Repository navigation
fix: prevent a stale id token from allowing session takeover - #54942
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Logout handling can delete unrelated login tabs and reuse another session’s logout state, potentially suppressing client logout notifications.
2 open issues
What changed in this PR
This PR addresses stale-ID-token session takeover by separating logout authentication roots from user-session IDs and consolidating session cleanup.
Changes:
- Uses random logout root IDs with explicit session bindings and earlier stale-token validation.
- Cleans up paired authentication roots and binds authorization codes to users.
- Adds regression coverage and documents rolling-upgrade behavior.
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/session/AdminSessionDeletionTest.java | Updates logout-root isolation assertions. |
| tests/base/src/test/java/org/keycloak/tests/oauth/AuthorizationCodeTest.java | Tests code exchange and user-binding rejection. |
| tests/base/src/test/java/org/keycloak/tests/forms/StaleAuthSessionLogoutTest.java | Adds fixation and orphaned-session regression coverage. |
| tests/base/src/test/java/org/keycloak/tests/forms/LogoutSessionReuseTest.java | Tests logout continuity, cookie safety, and cleanup. |
| services/src/test/java/org/keycloak/protocol/oidc/utils/OAuth2CodeTest.java | Tests user-ID serialization and legacy compatibility. |
| services/src/main/java/org/keycloak/services/resources/admin/ClientScopeEvaluateResource.java | Consolidates temporary-session cleanup. |
| services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java | Uses shared cleanup during application revocation. |
| services/src/main/java/org/keycloak/services/managers/UserSessionManager.java | Removes orphaned roots after offline-session revocation. |
| services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java | Adds random logout roots, explicit bindings, and paired cleanup. |
| services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2CodeParser.java | Validates authorization-code user binding. |
| services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2Code.java | Stores the user ID with authorization codes. |
| services/src/main/java/org/keycloak/protocol/oidc/refresh/DefaultRefreshTokenProvider.java | Resolves corresponding online sessions and cleans empty sessions. |
| services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java | Includes the user ID when issuing codes. |
| services/src/main/java/org/keycloak/protocol/oidc/grants/ResourceOwnerPasswordCredentialsGrantType.java | Removes paired roots for offline direct grants. |
| services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java | Reuses consolidated offline-access cleanup. |
| services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java | Validates hints earlier and clears reused logout request data. |
| services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java | Uses paired cleanup for evaluation sessions. |
| services/src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java | Restarts login when expiry cleanup removes its root. |
| server-spi/src/main/java/org/keycloak/sessions/AuthenticationSessionProvider.java | Adds root-session removal by ID. |
| docs/documentation/upgrading/topics/changes/changes-next-cve-2026-107124.adoc | Documents in-flight logout effects during rolling upgrades. |
Review effort: Balanced
shawkins
marked this pull request as draft
November 10, 2026 01:35
shawkins
marked this pull request as ready for review
November 10, 2026 12:44
mposolda
approved these changes
Nov 11, 2026
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive logout rebinding and cleanup span storage providers, browser/IdP callbacks, and mixed-version clusters, requiring runtime validation beyond static review.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
shawkins
enabled auto-merge (squash)
November 11, 2026 14:26
consolidated session cleanup, and changed how createOrJoinLogoutSession uses the userSession also adding more test coverage for orphaned state closes: keycloak#54691 closes: CVE-2026-107124 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
| // Never plant the id of a recovered root in the browser | ||
| boolean recovered = logoutRoot.getId().equals(session.getAttribute(RECOVERED_LOGOUT_ROOT_ID)); | ||
| if (!recovered && (cookieRoot == null || !cookieRoot.getId().equals(logoutRoot.getId()))) { | ||
| asm.setAuthSessionCookie(logoutRoot.getId()); |
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#unauthorizedSSOTestorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testMultipleTabsParallelLoginAfterAuthSessionExpirationorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#singleLoginAndLogoutSAMLTestorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostTestCompositeRoleForUserorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostEmptyConsumerPostURLorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testReloginWithInvalidAuthSessionCookieorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostTestorg.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testMultipleTabsParallelLogin |
1 of 2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
consolidated session cleanup, and changed how createOrJoinLogoutSession uses the userSession
also adding more test coverage for orphaned state
closes: #54691
closes: CVE-2026-107124