feat: evict a bearer token on a 401 challenge#101
Merged
Conversation
BearerTokenAuthStep only refreshed its cached token on the local expiry margin. When a server revokes a token before its advertised expiry, the step kept stamping the rejected token until the margin elapsed, so every request in that window failed with a 401. Override authorizeRequestOnChallenge so a 401 carrying WWW-Authenticate evicts the rejected token and re-stamps the request with a freshly fetched one, driving AuthStep's existing single retry with a new credential. Using the in-step hook (rather than throwing a retryable exception) means the retry is not gated by request idempotency, so a non-idempotent POST is re-authenticated too. Eviction is scoped to the token that actually produced the 401 (matched against the request's Authorization header) and runs under the same lock as the refresh path, so a token a concurrent request already refreshed is preserved rather than clobbered. This is an additional eviction trigger layered on top of the expiry-margin check, which is unchanged.
A cross-origin redirect re-issue reaches the AUTH stage stripped of its Authorization header and tagged with the internal cross-origin marker, so the stage forwards it credential-free. If the foreign host answered that request with a 401 + WWW-Authenticate, the challenge hook unconditionally re-stamped Authorization: Bearer <token> onto the server-chosen host and re-drove it through the chain — leaking the caller's token cross-origin and bypassing the HTTPS guard, which only the first pass through process() enforces. The challenge hook now returns null (surfacing the 401 unchanged) when the rejected request carried no Authorization header, since that only happens when the AUTH stage deliberately suppressed stamping. Route the bearer header value through a single bearerHeaderValue() helper shared by stamping and the eviction match so the two cannot drift, and document that a subclass customizing the header format must override it. Add a test for a cross-origin-marked request that 401s.
Resolve a conflict in BearerTokenAuthStepTest.kt: keep the IoProvider @BeforeTest setup added for the 401-challenge retry path, and adopt main's section comment.
Only evict and refetch the cached token when the 401's WWW-Authenticate header actually advertises a Bearer challenge. A 401 carrying only a non-bearer challenge (e.g. Basic), or an unparseable one, is surfaced unchanged: refreshing the bearer token cannot satisfy it, so retrying would only earn a second 401 after a wasted token fetch and round trip. Reuses AuthChallengeParser for RFC 7235 parsing, which also matches a Bearer challenge listed alongside others in a multi-challenge header.
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
1 participant
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.
Summary
When a bearer-authenticated request is rejected with a 401 challenge,
BearerTokenAuthStepnow evicts the rejected token and re-stamps the request with a freshly fetched one, instead of continuing to use a token that is still within its expiry margin but has already been refused by the server.It overrides the auth step's challenge hook, so the re-attempt uses the existing in-step single retry — which, unlike the throw-based retry path, is not gated by method idempotency, so it also works for a body-less
POST. Eviction is scoped to the exact token that produced the 401 (matched against the request'sAuthorizationheader) and runs under the same lock as the refresh path, so a token another thread already refreshed isn't clobbered and a 401 storm doesn't stampede the token provider. The existing expiry-margin refresh is unchanged — this is an additional eviction trigger on server rejection.New tests cover refetch-after-401, the non-idempotent
POSTcase, caching of the refreshed token for the next request, and that repeated 401s don't loop. The API snapshot is updated (the override is now a visibleprotectedmember).Closes #33