fix(spring): clear the SecurityContext when a caller's token is rejected - #37
Merged
Conversation
Adds the CONTRACT.md §10.1 rule-8 guardrail regression tests, and fixes
a real gap the new tests exposed in AxiamAuthenticationFilter.
Rule 8 — "subject of the decision" — asks not whether the token is good
but whether it is the token the decision is even about. SEC-085
satisfied rules 1-7 and was still an authentication bypass: the PHP
guard routed a failed verification into a second, successful one
against the application's own session.
The Spring filter rejected correctly (401, chain not invoked) but never
cleared the ambient SecurityContext. A caller presenting a failed token
therefore left behind whatever identity was already on the thread —
placed there by an earlier filter, or left over on a pooled container
thread. A later read of SecurityContextHolder then sees an identity the
caller never authenticated as, which is the servlet analogue of the
SEC-085 substitution. The filter's own comment already stated the
intent ("never let an unexpected exception fall through to an
authenticated SecurityContext"); it only ever avoided *setting* one,
not clearing a pre-existing one.
writeJsonError now clears the context. It runs only on rejection paths:
a request presenting no credential at all returns earlier and never
reaches it, so session-authenticated traffic carrying no AXIAM token is
untouched.
Found by writing the guardrail test, and falsified by it — the test
fails against the filter as it stood.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SkTHvZQMV47t3UwkEtmB1D
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
2 participants
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.
Adds the CONTRACT.md §10.1 rule-8 guardrail regression tests — and fixes a real gap the new tests exposed.
The rule
Rules 1-7 ask whether the token is good. Rule 8 asks whether it is the token the decision is even about. SEC-085 satisfied all seven and was still an authentication bypass: the PHP guard routed a failed verification into a second, successful one against the application's own session, admitting the caller as the app's service account.
The gap this found
AxiamAuthenticationFilterrejected correctly — 401, chain not invoked — but never cleared the ambientSecurityContext. A caller presenting a failed token left behind whatever identity was already on the thread: placed there by an earlier filter, or left over on a pooled container thread. A later read ofSecurityContextHolderthen sees an identity the caller never authenticated as — the servlet analogue of the SEC-085 substitution.The filter's own comment already stated the intent — "never let an unexpected exception fall through to an authenticated SecurityContext" — but the code only ever avoided setting one, never cleared a pre-existing one.
writeJsonErrornow clears the context. It runs only on rejection paths: a request presenting no credential at all returns earlier and never reaches it, so session-authenticated traffic that carries no AXIAM token is untouched.Tests (4)
aFailedCallerTokenIsRejectedAndNoIdentityIsAuthenticated— expired token, correctly signed, right tenant. Fails rule 2 and nothing else.aRejectedCallerDoesNotInheritAPreexistingAuthentication— the one that found the bug. Primes the ambient context with the app's own identity, asserts the precondition that it really is present, then asserts a rejected caller does not inherit it.theAuthenticatedIdentityIsAlwaysTheCallersOwn— the positive half. A guard preferring an ambient credential would pass the negative tests while still being wrong.theFilterConstructorExposesNoSecondCredential— pins the dependency surface (JwksVerifier+ tenant id, nothing more) and asserts no field is session-shaped, so the property can't be quietly undone later.Falsified by construction: test 2 failed against the filter as it stood, which is how the gap surfaced.
Verified locally:
mvn testgreen (full suite).🤖 Generated with Claude Code
https://claude.ai/code/session_01SkTHvZQMV47t3UwkEtmB1D
Generated by Claude Code