Adds a mock OpenID provider issuing signed ID tokens so AuthenticateCallback
can be tested end to end, and pins the current behaviour for OIDC-created
users (email/name/avatar/team sync, disabled user refused). The proxy test
reuses the same mock instead of its inline copy.
Every login matched through emailfallback or usernamefallback now warns that both options will be removed and the user has to link the provider from their account settings.
Address review decision: the post-commit avatar sync still blocked the
LDAP/OIDC login response for the download and up to 30s of decode
queueing. It now runs in a goroutine on a context that outlives the
request, with panics recovered and failures logged.
Address review decision: the provider avatar was decoded inside the login
transaction after the user row was written, holding it while queued for
decode capacity, and a later rollback (TOTP, disabled user) discarded the
work. Logins now commit first; the avatar is decoded without a session
and saved in its own short transaction.
Address review decision: a decode queued for capacity held the request's
transaction for up to 30s, so on SQLite a concurrent write made the
upload fail with database is locked. Uploads and avatar reads now decode
and encode first; only the encoded PNG enters the transaction.
Address pr-swarm finding: both logins copied crop, store and flush, had
diverged, decoded every avatar twice, and imaging.Fill cloned images
thinner than 100 px at full resolution outside the decode budget.
Identity-provider avatars are cropped through imageutils.Decode. When the
server is busy the avatar is skipped and logged, like any other avatar
sync error, so the login still succeeds.
The memory keyvalue store only shallow-copies Provider, so Oauth2Config
was shared between the cache and all requests. Concurrent callbacks
overwrote each other's RedirectURL, causing intermittent login failures.
A provider's discovery document could advertise a javascript: URL as
end_session_endpoint, which the frontend then navigated to on logout,
running script on Vikunja's origin. Only absolute http(s) URLs are now
accepted, both when caching the discovered endpoint and when building
the logout URL. Invalid endpoints are dropped so logout stays local.
Add auth.ldap.attribute.id. When set (e.g. objectGUID on AD, entryUUID
on OpenLDAP), users and teams are matched by that attribute instead of
the login name and the group DN, so renaming or moving them in the
directory no longer creates duplicate accounts or new teams without the
old shares.
AD's binary objectGUID is formatted as a canonical GUID string; other
attributes must be printable text. Existing accounts and teams are
switched to the new id on the next login. An empty option keeps the old
behaviour.
Support {userdn} and {username} placeholders in auth.ldap.groupsyncfilter.
When present, every matching group is a membership and the member
attribute is not fetched, so the cost scales with the user's own
memberships and nested AD groups can be resolved with
LDAP_MATCHING_RULE_IN_CHAIN. Filters without a placeholder keep the
previous client-side member matching.
The group search now uses paged results in both modes so directories
with more groups than the server's size limit no longer fail the login.
Over https the refresh cookie used SameSite=None. CORS allows localhost
origins by default for the desktop app, so a page on any localhost port
could refresh a session with the user's cookie and read the new token.
The cookie is now always SameSite=Lax. SameSite=None existed for the
desktop app's cookie refresh, which it replaced with OAuth over IPC in
v2.3.0.
Changelog:
- Desktop v2.2.x has to log in again more often; upgrade to v2.3.0 or later.
- Deployments with the frontend and API on different registrable domains
lose cookie refresh, as before v2.2.0.
The socket only checked the token's signature and expiry. A token of a
deleted session or of a disabled user still authenticated, and an open
socket kept receiving notifications after logout, session revocation,
password change or account deletion.
Auth now requires the token's session to exist for that user and the
user to be active; the connection registers first and is checked once,
so a concurrent revocation can't slip in between. Transient errors close
with try-again-later instead of invalid_token. Each socket closes when
its token expires, which bounds exposure even on nodes that miss a
revocation event.
Deleting sessions dispatches SessionsRevokedEvent on commit, every caller
flushes it after committing, and the hub closes the matching sockets.
Disabling or locking a user and deleting a user revoke their sessions,
also from the CLI.
The proxy dial exemption was keyed on addresses learned lazily from earlier
proxied requests, so a direct request (loopback, NO_PROXY or an unproxied
scheme) or a redirect to the proxy's host:port was dialed unguarded. Derive
the exempt addresses once from the configured or environment proxy URLs and
reject direct requests to them with ssrf.ErrProhibitedIP.
Address pr-swarm finding: SSRF guard bypass via the proxy address
Both `usernamefallback` + `emailfallback` on: lookup required username
AND email match, so local user with different username never linked,
duplicate account created. Now verified-email-only lookup always tried
last.
Fixes#2022
## How to verify
1. Create a local user `localuser` with email `me@example.com`.
2. Configure an OIDC provider with `usernamefallback: true` and
`emailfallback: true`. In the IdP, create a user whose email is
`me@example.com` (sent with `email_verified: true`) and whose
username/sub is different from `localuser`.
3. Log in to Vikunja through that OIDC provider.
4. **Expected:** You are logged in as `localuser`, and `vikunja user
list` shows no new user for the provider's issuer.
**Before this PR:** a second user with a random or IdP-provided username
was created.
---------
Co-authored-by: kolaente <k@knt.li>
`POST /api/v1/oauth/authorize` and `POST /api/v1/oauth/token` are served
but missing from the generated OpenAPI docs: the handlers in
`pkg/modules/auth/oauth2server` carry no swagger annotations, so neither
path is in `swagger.json` — 126 paths, no `oauth` among them.
Found while building a third-party client. The API answers fine; the
docs say the endpoints don't exist, so a client that validates its calls
against them can't use OAuth at all.
Adds annotations for both handlers and regenerates `pkg/swagger`. No
behaviour change — comments only, plus generated output. `mage
generate:swagger-docs` produces 521 insertions and 0 deletions, so
nothing else in the spec moved.
Checked against a 2.5.0 instance: `OPTIONS` on both paths returns
`Allow: OPTIONS, POST`, and `POST /api/v1/oauth/token` with no grant
type returns code 17007 naming `authorization_code` and `refresh_token`.
One judgement call worth a look: I annotated the token endpoint `@Accept
json`, matching the comment on `TokenRequest` that v1 binds JSON and v2
takes the form-encoded body. In practice v1 binds a form body too —
`Content-Type: application/x-www-form-urlencoded` with `grant_type=nope`
returns the same 17007 — so if you'd rather document both, say the word
and I'll widen it.
The `/api/v2` routes are untouched — happy to cover those too if you'd
like them in.
AI-assisted: annotations drafted with Claude, reviewed and tested by me.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
db.NewSession begins a transaction, so every request holds one pooled
connection for its whole life: across the API's own work and every round
trip in between. At 20,000 simulated users that filled a 192-connection
pool with 170 backends idle in transaction while 13 were active, and the
database sat at 40 % CPU with p99 at 94 ms.
NewReadSession attaches the session memo but does not Begin; each statement
borrows a connection only for itself. Commit and Rollback are no-ops on it,
so DoReadOne, DoReadAll and ValidateAPITokenString keep their shape. A read
path that writes (position healing) now commits per statement.
GetAuthFromClaims opened a session and loaded the token owner by id on
every call, although the token middleware had already put the user into
the context. With the metrics middleware and the handler both resolving
the auth, that was two extra transactions and user lookups per request
authenticated with an API token.
The metrics middleware called GetAuthFromClaims on every request and logged
its error when no JWT was in context. Unauthenticated routes (login, token
refresh, /info, websocket) hit that path constantly, flooding the log at
WARNING level and burying real errors.
Skip the active-user update when the request carries no credentials.
Only an expired session or a disabled account invalidates the cookie. A transient failure must not clear it, and neither must a token rotated away by a concurrent refresh: that would delete the cookie the winning refresh just set.
It returned *echo.HTTPError from a transport-agnostic module, which v2 had
to shim back into a huma error. Domain errors render on both APIs and let
callers tell the failure modes apart.
address pr-swarm finding
The cookie was path-scoped to /api/v1/user/token/refresh only, so browsers never sent it to the v2 endpoint and every v2 refresh returned 401. One cookie per API version path now, with the route literals shared as consts.
Vikunja leaves `oauth2.Config.Endpoint.AuthStyle` at
`AuthStyleAutoDetect`, so `golang.org/x/oauth2` tries
`client_secret_basic`, retries with `client_secret_post` on *any* error,
and reports only the second error — turning every token endpoint failure
(a wrong client secret, say) into a bogus "the client registration does
not allow client_secret_post" complaint.
Style now comes from the provider's
`token_endpoint_auth_methods_supported` discovery value. Provider
advertises neither: keep autodetect, so nothing changes for providers
with a thin discovery document.
Side effect: one token request per login instead of two, no more
spurious failed-auth entry in provider logs on every successful login.
Reported at
https://community.vikunja.io/t/authelia-could-not-authenticate-against-third-party/4770
## How to verify
1. Register a Vikunja client in an OIDC provider that only accepts
`client_secret_basic` (Authelia: `token_endpoint_auth_method:
'client_secret_basic'`).
2. Configure that provider under `auth.openid.providers` in Vikunja, but
put a deliberately wrong value in `clientsecret`.
3. Log in through that provider and read the Vikunja log.
4. **Expected:** the error names the real problem — an `invalid_client`
/ "client secret did not match" message from the provider.
**Before this PR:** the error claimed the client registration does not
allow `client_secret_post`, which was never configured on either side.
5. Fix `clientsecret` to the correct value and log in again.
6. **Expected:** login succeeds, and the provider's access log shows a
single `POST /token` per login rather than a rejected one followed by an
accepted one.
---------
Co-authored-by: kolaente <k@knt.li>
GetUserFromClaims read the id claim straight into User.ID without checking the
token type. A link share JWT carries the raw, positive share id in that claim
and reaches every authenticated route, so the only thing preventing
impersonation was the incidental absence of a username claim in link share
tokens. Adding one would have reintroduced the confusion with a positive id,
bypassing the GetID negation entirely.
AuthTypeUser moves to pkg/user, which parses the claims and cannot import
pkg/modules/auth, so the value exists once.
Three Debug-level log calls in the token-exchange path logged the
raw response body/full oauth2.Token on error. The full token struct
has no String() override, so %v printed AccessToken/RefreshToken in
clear text; the raw response body could similarly carry sensitive
request/response context. Log the RFC 6749 error/error_description
fields (successful-exchange-but-missing-id_token path: the token
type) instead - the intended human-readable diagnostic.
kolaenteandTink
s-seidelandGitHub
kolaenteandkolaente
Mauandkolaente