[zerox80]

Description

Multiple access tokens can be valid for one session, for example after a refresh. A backchannel logout only removed the cached claims of the token in the session lookup, so older tokens stayed accepted. With PROXY_OIDC_SKIP_USER_INFO=true, even the removed token was accepted again, because a still valid JWT was verified anew.

Each verified logout now writes a marker for the session or, if the logout token has no sid, for the subject. The marker holds the logout token's iat as a cutoff. Every authenticated request is checked against the markers, and a token is rejected if a marker covers it and iat <= cutoff. This also covers tokens the proxy hasn't seen before the logout.

This is the marker design @rhafer suggested, with two differences:

• Session markers also use the iat cutoff. Keycloak keeps issuing tokens with the same sid to a client with an offline token after a logout, and reuses the sid for a new login after a consent revocation. A plain marker would reject those tokens until it expires.
• The cutoff is the logout token's iat, not the time of receipt. Both timestamps then come from the IDP clock, and replaying a logout token doesn't change the cutoff.

Behavior changes

• Logout tokens without iat are rejected.
• Access tokens need sid, sub and iat. A token without sid isn't covered by a session logout. A token without iat is rejected if a logout covers it.
• New setting PROXY_OIDC_MAX_ACCESS_TOKEN_LIFETIME (default 24h). Markers are kept for this duration plus 5 minutes. A JWT access token that lives longer or has no exp is still accepted, and the proxy logs a warning because the token can outlive its marker.
• If the marker store can't be read, the proxy ignores the cached claims and asks the userinfo endpoint. With PROXY_OIDC_SKIP_USER_INFO=true there is nothing to ask, so the request is rejected. A corrupt marker always rejects.
• The default cache database is renamed from cache-userinfo to cache-oidc-tokens, which starts with a fresh cache. Markers use their own database with the suffix -logout-markers, because nats-js-kv only supports one TTL per bucket.
• Publishing the logout event and removing cached records are now best effort, so a failed publish no longer blocks the logout.

The README lists PROXY_OIDC_SKIP_USER_INFO=false as a requirement for a reliable logout.

Cost

Every authenticated request reads up to two markers, one for the session and one for the subject, also on a cache hit. With nats-js-kv these are two more round trips per request. I haven't measured this.

Open questions

• The iat cutoff for session markers. My comment from Nov 4 has the comparison with other implementations.
• Should the proxy honor revoke_offline_access in the logout token?
• Only web has a backchannel logout URL in the realms of this repo and of opencloud-compose. Should the desktop and mobile clients get one?

Commit sequence

1. Add backchannel logout markers. The marker store and its unit tests.
2. Reject tokens covered by a backchannel logout. Authentication, the logout route, configuration, documentation and regression tests.

Related Issue

• Fixes Backchannel logout leaves older access tokens of the session valid #3947
• Fixes Backchannel logout has no lasting effect with PROXY_OIDC_SKIP_USER_INFO=true #3948
• Fixes Backchannel logout keeps the cached claims if the logout event cannot be published #3949

Motivation and Context

Backchannel logout must invalidate the tokens of the affected session, including tokens issued before a refresh and tokens verified without a userinfo request.

Keycloak also sends no logout token in some cases, so the proxy can't react to them. These gaps exist today and this PR doesn't fix them: clients without a backchannel URL, a realm-wide "logout all", and on 26.8.1 a browser logout of a session with only an offline client.

How Has This Been Tested?

Unit and HTTP regression tests:

go test -race -short ./pkg/oidc/... ./services/proxy/... ./services/clientlog/...

End to end with a real OpenCloud server and Keycloak 25.0.0 and 26.8.1. On current main, tokens stay accepted after logout. With this PR they are rejected, and other sessions and users aren't affected. These runs are from before the last two changes, the lifetime warning and the userinfo fallback. Both are covered by unit and HTTP tests only.

End-to-end runs in detail

Test copy of the realm (no LDAP, plain HTTP, extra test clients), scripted clients. Memory, NATS and noop stores, each with PROXY_OIDC_SKIP_USER_INFO on and off. In all 12 runs:

• Old and new tokens are rejected after a refresh and logout, including tokens the proxy hadn't seen.
• Other sessions and users aren't affected.
• An offline desktop client recovers with a refresh after a web logout.
• After a subject-only logout, a session that is still alive recovers with a refresh.
• A new login works after a logout, and after a consent revocation that reuses the sid.
• With NATS, markers survive a server restart and apply on a second proxy.
• With PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=none, results are identical to main.

Not covered or not resolved:

• Logout event: In my end-to-end setup, the backchannel-logout event didn't arrive on an SSE stream, on main or with this PR. I don't know the cause yet. A unit test confirms the event is published with the right user and session.
• Not tested: the real desktop client, Redis Sentinel, other IDPs, clock skew, and a store outage end to end.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation added

Assisted-By: LLM

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 134 complexity

Metric	Results
Complexity	134

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[zerox80]

Could you mark this finding as a false positive and rerun Codacy? It occurs in an integration test: the executable name is fixed to redis-server, arguments are passed without a shell, and the socket path comes from t.TempDir(). No untrusted request input reaches this call.

[dschmidt]

This means you're logged out from every session on every browser/device, right?
Is that really desirable/expected?

[zerox80]

This means you're logged out from every session on every browser/device, right? Is that really desirable/expected?

the scope depends on the logout token. When it contains a sid, only the tracked tokens belonging to that session are revoked; other sessions of the same user remain authenticated. When it contains only sub, all tracked sessions for that user are targeted, as specified by oidc back-channel logout.
That distinction already existed before this PR. The fix ensures that all previously accepted tokens within the targeted session(s), including tokens issued before a refresh, are invalidated. I verified the session isolation with HTTP regression tests.

[dschmidt]

Thanks for the clarification!

[rhafer]

@zerox80 Could you provide some more details about what issue this PR is actually trying to address? Under what circumstances (config) is there actually a problem?

[zerox80]

@rhafer The issue is in the proxy's backchannel logout handling. It can happen with the default OIDC config (PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt, PROXY_OIDC_SKIP_USER_INFO=false, memory cache) if the IdP sends a backchannel logout and a refresh leaves two unexpired access tokens for the same session. Token A is cached, then token B is used and replaces A in the single session lookup. A's cached claims remain, so logout removes B but A can still authenticate until its cache entry expires.

With PROXY_OIDC_SKIP_USER_INFO=true, even a single token can be accepted again after logout: deleting cached claims doesn't revoke the still-valid JWT. This PR keeps a separate lookup and revocation record for each token the proxy has already accepted.

[rhafer]

@zerox80 Thanks again for bringing this up. And sorry for having it hanging around for so long. I think I understand the issue(s) now. However, I somehow have the feeling we're making our lives more complicated that needed here (partially already before your change).

The main issue you're bringing up is valid. Multiple tokens for the same session can still be valid at a given time (e.g. if a client refresh it's token before it expires). And our current implementation is not safe in that case (the old token stays in the userinfo cache).

I’m less sure we need a per-token index and revocation list to handle it, though. I think it might be simpler to just record logout markers by sessionid and/or subject and check those during authentication:

Session-specific logout

1. On a verified backchannel logout containing sid, write a marker such for that session id
2. On every authentication, obtain the token’s verified session ID and check for that marker.
3. Reject the token if the marker exists.

That covers the case where multiple valid tokens exist. Even for tokens that OpenCloud has not seen before. It also avoids enumerating tokens.

This assumes the IdP supplies a trustworthy sid in access tokens and assigns a new sid to a new login. Which I think is fine. We should just document that requirement.

Subject-only logout

A logout can contain sub without sid, meaning all sessions for that subject should end. A permanent subject marker would also block future logins, so this needs a cutoff:

1. Record a verified logout time for the subject.
2. Reject tokens for that subject issued at or before that time.
3. Allow tokens from later logins.
   This requires a trusted token issuance time and handling of clock skew. Concurrent or replayed logout messages must not move the cutoff backwards.
   Tokens without a trustworthy iat for this to work. (Again something we'd need to document. I think it is fine if we do not support backchannel logout when opaque (i.e. non-jwt) access tokens are used

Retention and storage

Markers must remain until no token they cover can still be accepted. A practical simplification would be to require a configured maximum accepted token lifetime (+ some clock skew buffer maybe)

Other

• I don't think we need any migration here. In a multi instance setup we don't aim to support having multiple instance of the same service running in different version for a prolonged time. We should however switch the database name to start with a fresh cache after this change.
• As mentioned above it's fine to restrict backchannel logout to PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt setups
• It is also fine to have the documented requirement that the access token needs to contain a sid claim
• As a side effect this PR also fixes an issue that makes the userinfo cache invalidation fail when publishing the BackchannelLogoutEvent fails (we should not forget about that).

@zerox80 Do you feel comfortable implementing this?

[zerox80]

@rhafer Thanks for the review. Yes, I can implement this, and I agree that markers are simpler than tracking every token.

I tested the assumptions against Keycloak 25.0.0 and 26.8.1, using a copy of the realm from devtools/deployments/opencloud_full. Access tokens contain sid, iat and exp, sid stays the same across a refresh, and the old and new tokens are valid at the same time. Logout tokens contain iat and a matching sid and sub.

I also ran the logout end to end through a real OpenCloud server, using current main and this PR's unrebased branch. Each ran with memory and NATS caches, and with PROXY_OIDC_SKIP_USER_INFO on and off. The results were the same on both Keycloak versions:

• main: The old token stays accepted after logout in every configuration. With PROXY_OIDC_SKIP_USER_INFO=true, the new token stays accepted too.
• This PR's branch: Tokens the proxy has seen are rejected, and other sessions and users aren't affected. Clients whose IdP session is still alive recover with a refresh.
• Gap in this PR's branch: With PROXY_OIDC_SKIP_USER_INFO=true, a token the proxy hadn't seen before the logout is still accepted afterwards. Your marker approach would close this.

The marker design isn't implemented yet, so it hasn't been tested end to end.

One result changes the design, because a logged-out sid can stay in use:

• Web and an offline desktop client can share a session. After logging out of web, the offline token keeps working, and Keycloak issues new access tokens with the same sid.
• After a consent revocation on a client that requires consent, SSO stays active and the next login reuses the sid.

A plain session marker would reject those tokens until it expires. I suggest one rule for both marker types: reject a token if iat <= cutoff, using the logout token's iat as the cutoff. Both timestamps then come from the IdP clock, and replaying a logout token doesn't change the cutoff. It has to be <= because a token can be issued in the same second as the logout. With a subject cutoff, a login in that same second might need one refresh, which seems acceptable. Logout tokens without iat would be rejected, and so would access tokens without iat when a matching marker exists.

A few points I'd like to confirm:

1. Marker storage: nats-js-kv only supports a TTL per bucket, and the userinfo cache defaults to 10s, so markers can't live in that bucket. I plan to use a separate store with TTL = maximum token lifetime + buffer. Is that OK?
2. Maximum token lifetime: Should the proxy reject tokens that exceed the configured maximum or have no exp? Otherwise a marker can expire while a token it covers is still valid. I'd reject them.
3. Cutoff updates: The go-micro store has no compare-and-swap, so I'd read the current cutoff, keep the later value and write it back. Concurrent logouts for the same subject could still race, but only in a very small window. Is that acceptable?
4. Logout event: BackchannelLogout needs the user ID, which currently comes from the cached claims. I'd keep the existing session lookup for that only, as best effort. Authentication would depend only on the markers, and a failed publish would no longer block the logout.
5. Store errors: If a marker can't be read during authentication, I'd reject the request rather than risk accepting a logged-out token. The downside is that a store outage would cause 401s, so let me know if you'd prefer otherwise.

Keycloak also sends no logout token in these cases, so the proxy can't react to them. These gaps exist today and this change won't fix them, so I'd document them:

• Clients without a backchannel URL. In that realm config only web has one, so a desktop-only session isn't covered.
• A browser logout of a session with only an offline client, on 26.8.1 (25.0.0 does send one).
• A realm-wide "logout all".

[rhafer]

@rhafer Thanks for the review. Yes, I can implement this, and I agree that markers are simpler than tracking every token.

[..]

The marker design isn't implemented yet, so it hasn't been tested end to end.

One result changes the design, because a logged-out sid can stay in use:

* Web and an offline desktop client can share a session. After logging out of web, the offline token keeps working, and Keycloak issues new access tokens with the same `sid`.

* After a consent revocation on a client that requires consent, SSO stays active and the next login reuses the `sid`.

Hm, interesting corner case. I am not sure about the suggested solution, would also be interested to see how other implementations handle this case. In general my assumption would be that if a backchannel logout with a specific sid happens, all clients of that session are logged out. These needs a little more research.

A few points I'd like to confirm:

1. **Marker storage:** nats-js-kv only supports a TTL per bucket, and the userinfo cache defaults to 10s, so markers can't live in that bucket. I plan to use a separate store with TTL = maximum token lifetime + buffer. Is that OK?

Yes.

2. **Maximum token lifetime:** Should the proxy reject tokens that exceed the configured maximum or have no `exp`? Otherwise a marker can expire while a token it covers is still valid. I'd reject them.

Hm, I think we should issue a WARN level log message for these cases, for now. Also for cases like this it would IMO make sense to require PROXY_OIDC_SKIP_USER_INFO=false for backchannel logout to work more reliable, right? I'd be fine with that requirement.

3. **Cutoff updates:** The go-micro store has no compare-and-swap, so I'd read the current cutoff, keep the later value and write it back. Concurrent logouts for the same subject could still race, but only in a very small window. Is that acceptable?

Yes.

4. **Logout event:** `BackchannelLogout` needs the user ID, which currently comes from the cached claims. I'd keep the existing session lookup for that only, as best effort. Authentication would depend only on the markers, and a failed publish would no longer block the logout.

Not sure I understand this. Please elaborate.

5. **Store errors:** If a marker can't be read during authentication, I'd reject the request rather than risk accepting a logged-out token. The downside is that a store outage would cause 401s, so let me know if you'd prefer otherwise.

Would it help if we just fell back to an online userinfo lookup if reading the marker fails (only for failures caused by service outage of the store)

Keycloak also sends no logout token in these cases, so the proxy can't react to them. These gaps exist today and this change won't fix them, so I'd document them:

* Clients without a backchannel URL. In that realm config only `web` has one, so a desktop-only session isn't covered.

It's probably also worth checking why that is.

* A browser logout of a session with only an offline client, on 26.8.1 (25.0.0 does send one).

* A realm-wide "logout all".

agreed.

Some general remarks:

• It would be nice if you created bug issues for the problems that we're addressing here
• Review would be easier if you started new the implementation from scratch (based on current main). I don't think there is much left of your initial changes after switching to the marker based implemenation.

[zerox80]

@rhafer Thanks! The branch of this PR already contains a fresh implementation on top of main (two commits, none of the old ones), so you can review it as is. It's still a draft while I work in your feedback. I can also open a new PR if you'd prefer one without the old discussion.

sid and offline tokens: I compared this with other implementations, and the iat cutoff matches what they do.

• Offline tokens: Section 2.7 of the back-channel logout spec says: "Refresh tokens issued with the offline_access property normally SHOULD NOT be revoked." Keycloak does the same when it receives a logout token itself: LogoutEndpoint only ends offline sessions if the token has the revoke_offline_access event.
• Already issued tokens: Systems that have to reject stateless tokens use a stored cutoff. Keycloak's not-before policy compares iat, and AWS IAM's session revocation compares aws:TokenIssueTime.
• Session-based implementations: mod_auth_openidc, Spring Security and Nextcloud user_oidc end a local session by sid or sub. They don't have to decide this for bearer tokens checked on each request, so they aren't directly comparable.

For normal sessions, your assumption holds: refreshing fails after the logout, and the marker rejects the tokens of that session. Offline tokens are the exception, and the desktop client requests offline_access. With a plain marker, every new token of that client would be rejected until the marker expires. With the iat cutoff, its old token is rejected and a refresh recovers it. I verified that end to end on Keycloak 25.0.0 and 26.8.1.

I could also honor revoke_offline_access: if the logout token has that event, reject all tokens of the session until the marker expires. The proxy can't revoke the refresh token itself, so this only holds for as long as the marker exists. Would you want that?

Token lifetime: I'll change this to a WARN log and document PROXY_OIDC_SKIP_USER_INFO=false as a requirement.

Logout event: The markers only decide whether a token is rejected. The BackchannelLogout event also needs the user ID, but the logout token only has sub and sid. So the proxy gets the user from the existing session lookup and the cached claims, and I'd keep that lookup for this purpose only. The marker is written first, so a missing lookup or a failed publish only means that no event is sent. Today, a failed publish also skips deleting the cached claims. In my end-to-end setup the event didn't arrive on an SSE stream, on main either, and I'm still looking into why.

Store errors: The userinfo fallback makes sense, and I'll implement it for store read failures. With PROXY_OIDC_SKIP_USER_INFO=true there's no online check to fall back to, so I'd keep rejecting in that case. A corrupt marker would be rejected too. OK?

Backchannel URL: Only web has one, both in this repo's realms and in opencloud-compose, and the history I checked doesn't say why. Do you know the reason? Adding it to the desktop and mobile clients would cover more sessions, but I only tested that with a copy of the desktop client. On 26.8.1, a browser logout of an offline-only session still sends no logout token. A logout with the client's refresh token sends one on both versions.

I'll open bug issues for the problems this PR addresses and link them here.