Reported against 2.2.1 by someone reading the source. Every finding held.
**Sign-in had no ceiling.** Failures were logged with the caller's address
and nothing more. Two counters now — one per address, one per account —
because the two attacks look different: one source working through many
accounts is caught by the first, many sources working on one account by the
second, and behind a proxy only the second still means anything.
The count is kept in the process that serves the request. PLANKA needs no
Redis and the stock deployment is one container; run several and each keeps
its own count, which multiplies the ceiling by their number. That trade is
written where the limits are configured.
**The second factor could be guessed at leisure.** Six digits, and a
pending token that stayed valid for its full ten minutes however many codes
were wrong. Wrong codes are now counted on the session row — in the
database, so the count survives a restart and holds across every process —
and when the budget is spent the session is destroyed. After that even the
right code is refused and the login starts over from the password.
**Avatars, background images and favicons** checked the token's signature
and nothing else, so a revoked session, a deactivated account or a changed
password all kept working there for as long as the signature lasted, which
is a year by default. The five checks the API makes now live in one helper
that both use, rather than the shortened copy that had drifted from it.
**A link attachment's favicon** was fetched from wherever the URL pointed.
Storing a link is harmless — it is a string the user typed — but fetching
its icon is a request the server makes to an address the user chose, and
whether an icon came back reported on what is reachable from inside the
network. Server-side fetches now refuse private, loopback and link-local
addresses, `169.254.169.254` among them. The attachment is still created:
linking to an internal wiki is a legitimate thing to do, and it was the
server's own request that had to stop.
**The signing key.** Our own compose file ships `notsecretkey`, and it is
printed in the documentation — so on any instance that copied it, anyone can
sign a token for any account. PLANKA now says so on every start, and keeps
saying it, along with a key that is missing or shorter than 32 characters.
The placeholder carries the warning inline, where it is copied from.
**The backup script** wrote password hashes, live sessions, TOTP secrets and
SMTP credentials to an unencrypted archive. `BACKUP_PASSPHRASE` now encrypts
it, and without one the script says what it just put on disk. It also says
what it is — an example for the stock compose stack, not a backup concept —
and names the window between the database dump and the file copy, which no
ordering closes.
Adds TOTP setup with QR code, login challenge, recovery codes and
trusted devices that let a browser skip the second factor for 30
days. Admins can reset another user's second factor by confirming
with their own password.
The OIDC implementation merged in https://github.com/plankanban/planka/pull/491 is flawed for multiple reasons.
It assumes that the access_token returned by the IDP has to be a JWT parseable by the RP which is not the case [1].
Many major IDPs do issue tokens which are not JWTs and RPs should not rely on the contents of these at all.
The only signed token which has a standardized format for direct RP consumption is the OIDC ID token (id_token), but this by default doesn't contain many claims, especially role claims are omitted from them by default for size reasons. To get these additional claims into the ID token, one needs an IDP with support for the "claims" parameter.
It requires manual specification of the JWKS URL which is mandatory in any OIDC discovery document and thus never needs to be manually specified.
It also makes the questionable decision to use a client-side code flow with PKCE where a normal code flow would be much more appropriate as all user data is processed in the backend which can securely hold a client secret (confidential client). This has far wider IDP support, is safer (due to direct involvement of the IDP in obtaining user information) and doesn't require working with ID tokens and claim parameters.
By using a server-side code flow we can also offload most complexity to the server alone, no longer requiring an additional OIDC library on the web client.
Also silent logout doesn't work on most IDPs for security reasons, one needs to actually redirect the user over to the IDP, which then prompts them once more if they actually want to log out.
This implementation should work with any OIDC-compliant IDP and even OAuth 2.0-only IDPs as long as they serve and OIDC discovery document.
[1] rfc-editor.org/rfc/rfc6749#section-5.1
Daniel Hiller