You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Restore potentially lost entries of revoked items into the cache - #54353
This is a best-effort defense-in-depth measure, not a guarantee.
Rationale
Instead of adding a database lookup to every cache miss on the token-validation path, the cache is reconciled with the database when the cluster membership changes.
Token validation stays cache-only and fast, and the cost is paid only in the rare event of a node leaving or a partition merging.
Decisions
Only the coordinator scans the database, as writes to the distributed cache restore the entries for the whole cluster.
Nodes joining the cluster don't trigger a reload, so scaling up doesn't scan the database.
Existing entries are never overwritten, so the lifespan of entries written by other nodes isn't restarted.
No retry: if the reload fails, a warning is logged and the LOADED marker is removed, so the next node that starts up loads the persisted revocations again. Nodes that are already running keep their state until then.
Assumptions
Entries are lost when the owning node leaves, not while the cache rebalances, so the reload doesn't need to wait for the rebalancing to finish.
The database stays available during the reload. If it doesn't, the best-effort fallback above applies.
This PR addresses #54347 by restoring persisted, unexpired token revocations that may be lost from the Infinispan cache during cluster membership changes.
Changes:
Adds asynchronous, coordinator-only recovery after member departures and cluster merges.
Updates JPA revocation queries to project results directly and use long-valued timestamps.
Adds a split-brain regression test for restoring lost revocations after a merge.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
Factory cleanup also runs after initialization fails, when persistRevokedTokens can be true but postInit() has not assigned singleUseObjectCache (see KeycloakModelTest.createKeycloakSessionFactory()). This dereference then throws a NullPointerException, masking the startup failure and aborting remaining provider cleanup; guard against a null cache before removing the listener.
On a long reload, this fixed timestamp makes later entries live past their persisted expiry and can reinsert tokens that expired while the scan was running. Compute the current time per record, as before, so recovery skips expired rows and does not extend cache retention by the scan duration.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Security-sensitive revocation recovery relies on interactions between Infinispan membership notifications and cache state transfer, requiring clustered runtime validation of merge recovery, owner departure, and reload-failure fallback.
I reviewed the residual behavior of this best-effort mitigation against the redesign tracked in #54373.
There is no need to fail all token introspection requests closed while a reload is running. With the action token cache configured with two owners, an ordinary rolling update removes one owner at a time and the surviving owner retains the revocation entry. The residual replay window requires concurrent loss of both owners, or equivalent exceptional cache loss, and lasts until the asynchronous reload restores the entry. A reload failure can extend that window until another node starts.
Those residual architectural limitations are covered by #54373:
each node builds its node-local revocation state from the database before it is marked ready and accepts dependent requests, with targeted database confirmation as the alternative during startup;
later rebuilds keep the existing local state active, incorporate concurrent revocation events, and replace or augment it without globally blocking introspection;
node loss no longer loses revocations through distributed-cache ownership;
one-time action tokens and SAML assertions use a database-arbitrated consume-once operation;
failed or missed rebuilds are recovered by subsequent triggers and hourly
10000
reconciliation; and
concurrent rebuild triggers are coalesced so topology churn does not queue repeated full scans.
Conclusion: I do not see an additional merge-blocking security finding in this PR within its explicitly documented best-effort scope. The remaining replay exposure and recovery limitations require the redesign already tracked by #54373.
ahus1
changed the title
Restore potentially lost expired entries into the cache
Restore potentially lost entries of revoked or used items into the cache
Nov 3, 2026
ahus1
changed the title
Restore potentially lost entries of revoked or used items into the cache
Restore potentially lost entries of revoked items into the cache
Nov 3, 2026
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cluster recovery across failures and membership changes warrants human assessment beyond the published review.
No open issues 🎉
Review effort: Balanced
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
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.
Closes #54347
This is a best-effort defense-in-depth measure, not a guarantee.
Rationale
Instead of adding a database lookup to every cache miss on the token-validation path, the cache is reconciled with the database when the cluster membership changes.
Token validation stays cache-only and fast, and the cost is paid only in the rare event of a node leaving or a partition merging.
Decisions
LOADEDmarker is removed, so the next node that starts up loads the persisted revocations again. Nodes that are already running keep their state until then.Assumptions