Skip to content

Commit aea1b88

Browse files
committed
feat: evict bearer token on a 401 challenge
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.
1 parent ea0cc81 commit aea1b88

4 files changed

Lines changed: 185 additions & 5 deletions

File tree

sdk-core/api/sdk-core.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ public class org/dexpace/sdk/core/http/pipeline/steps/BearerTokenAuthStep : org/
755755
public fun <init> (Lorg/dexpace/sdk/core/http/auth/BearerTokenProvider;Ljava/util/List;Ljava/time/Duration;Lorg/dexpace/sdk/core/util/Clock;)V
756756
public synthetic fun <init> (Lorg/dexpace/sdk/core/http/auth/BearerTokenProvider;Ljava/util/List;Ljava/time/Duration;Lorg/dexpace/sdk/core/util/Clock;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
757757
protected fun authorizeRequest (Lorg/dexpace/sdk/core/http/request/Request;)Lorg/dexpace/sdk/core/http/request/Request;
758+
protected fun authorizeRequestOnChallenge (Lorg/dexpace/sdk/core/http/request/Request;Lorg/dexpace/sdk/core/http/response/Response;)Lorg/dexpace/sdk/core/http/request/Request;
758759
}
759760

760761
public final class org/dexpace/sdk/core/http/pipeline/steps/DefaultAsyncInstrumentationStep : org/dexpace/sdk/core/http/pipeline/AsyncHttpStep {

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/BearerTokenAuthStep.kt

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import org.dexpace.sdk.core.http.auth.BearerToken
1111
import org.dexpace.sdk.core.http.auth.BearerTokenProvider
1212
import org.dexpace.sdk.core.http.common.HttpHeaderName
1313
import org.dexpace.sdk.core.http.request.Request
14+
import org.dexpace.sdk.core.http.response.Response
1415
import org.dexpace.sdk.core.util.Clock
1516
import java.time.Duration
1617
import java.util.concurrent.locks.ReentrantLock
@@ -37,6 +38,19 @@ import kotlin.concurrent.withLock
3738
* to decide when to refresh pre-emptively (default 30 seconds). A token whose `expiresAt`
3839
* is `now + 29s` with the default margin is considered expired and is refreshed.
3940
*
41+
* ## Eviction on 401
42+
*
43+
* The refresh margin only covers *local* expiry. A server may revoke a token before its
44+
* advertised expiry, and the cached token would otherwise keep being stamped until the
45+
* margin elapsed. To handle this, [authorizeRequestOnChallenge] evicts the rejected token
46+
* on a 401 + `WWW-Authenticate` response and re-stamps the request with a freshly fetched
47+
* one, so the single retry driven by [AuthStep] carries a new credential. Eviction is an
48+
* *additional* trigger layered on top of the margin check — it does not change pre-emptive
49+
* refresh. Only the token that produced the 401 is evicted; a token a concurrent request
50+
* already refreshed in the meantime is left in place. The retry runs regardless of HTTP
51+
* method (the in-step hook is not gated by retry-safety), so a non-idempotent POST is
52+
* re-authenticated too.
53+
*
4054
* ## Errors from [provider]
4155
*
4256
* - Throws → propagated. The exception is **not** cached, so a subsequent request
@@ -46,8 +60,9 @@ import kotlin.concurrent.withLock
4660
*
4761
* ## Open for subclassing
4862
*
49-
* Users wanting to override [authorizeRequestOnChallenge] (e.g. force a token refresh on
50-
* 401) can extend this class.
63+
* Eviction-on-401 is built in. Users wanting to customise challenge handling further (e.g.
64+
* inspect the `WWW-Authenticate` scheme before deciding to refresh) can override
65+
* [authorizeRequestOnChallenge].
5166
*
5267
* Thread-safety: see Caching above.
5368
* Cancellation: token fetch may block; the [provider] is expected to respect interrupts.
@@ -72,6 +87,40 @@ public open class BearerTokenAuthStep
7287
.build()
7388
}
7489

90+
/**
91+
* On a 401 challenge, evict the token that was just rejected and re-stamp [request]
92+
* with a freshly fetched one so [AuthStep]'s single retry carries a new credential.
93+
*
94+
* Eviction is scoped to the exact token that produced the 401 (matched against the
95+
* `Authorization` header [request] carried): if a concurrent request already refreshed
96+
* the cache, that newer token is preserved and reused for the retry rather than being
97+
* discarded. The subsequent [currentToken] call then re-fetches only when the cache is
98+
* actually empty, so a 401 storm across threads still funnels through the same
99+
* double-checked-locking fetch as the expiry-margin path.
100+
*/
101+
override fun authorizeRequestOnChallenge(
102+
request: Request,
103+
response: Response,
104+
): Request {
105+
evictRejectedToken(request.headers.get(HttpHeaderName.AUTHORIZATION))
106+
return authorizeRequest(request)
107+
}
108+
109+
/**
110+
* Clears [cachedToken] iff it is still the token whose stamped header is [rejectedHeader].
111+
* Guarded by the same [lock] as the refresh path so the read-compare-clear is atomic
112+
* against a concurrent refresh.
113+
*/
114+
private fun evictRejectedToken(rejectedHeader: String?) {
115+
if (rejectedHeader == null) return
116+
lock.withLock {
117+
val current = cachedToken ?: return
118+
if ("Bearer ${current.token}" == rejectedHeader) {
119+
cachedToken = null
120+
}
121+
}
122+
}
123+
75124
private fun currentToken(): BearerToken {
76125
// Fast path: lock-free volatile read; return the cached token if it's still valid.
77126
cachedToken?.takeIf { !it.isExpiredAt(clock.now(), refreshMargin) }?.let { return it }

sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/pipeline/steps/AuthStepTest.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,15 +519,16 @@ class AuthStepTest {
519519
// ----------------- 401 + WWW-Authenticate handling -----------------
520520

521521
@Test
522-
fun `401 with WWW-Authenticate returns the 401 unchanged by default`() {
523-
val provider = BearerTokenProvider { _, _ -> BearerToken("tk", null) }
522+
fun `401 with WWW-Authenticate returns the 401 unchanged when the step does not override the challenge hook`() {
523+
// KeyCredentialAuthStep does not override authorizeRequestOnChallenge, so it exercises
524+
// the AuthStep base default (returns null → no retry). The 401 surfaces unchanged.
524525
val fake =
525526
FakeHttpClient()
526527
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
527528

528529
val pipeline =
529530
HttpPipelineBuilder(fake)
530-
.append(BearerTokenAuthStep(provider, listOf("scope")))
531+
.append(KeyCredentialAuthStep(KeyCredential("k")))
531532
.build()
532533

533534
val response = pipeline.send(getHttpsRequest())

sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/pipeline/steps/BearerTokenAuthStepTest.kt

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,15 @@ import org.dexpace.sdk.core.http.common.HttpHeaderName
1313
import org.dexpace.sdk.core.http.pipeline.HttpPipelineBuilder
1414
import org.dexpace.sdk.core.http.request.Method
1515
import org.dexpace.sdk.core.http.request.Request
16+
import org.dexpace.sdk.core.http.request.RequestBody
17+
import org.dexpace.sdk.core.io.Io
1618
import org.dexpace.sdk.core.testing.FakeHttpClient
1719
import org.dexpace.sdk.core.testing.FixedClock
20+
import org.dexpace.sdk.io.OkioIoProvider
1821
import java.time.Duration
1922
import java.time.Instant
23+
import java.util.concurrent.atomic.AtomicInteger
24+
import kotlin.test.BeforeTest
2025
import kotlin.test.Test
2126
import kotlin.test.assertEquals
2227
import kotlin.test.assertFailsWith
@@ -28,6 +33,13 @@ class BearerTokenAuthStepTest {
2833
private val nowInstant: Instant = Instant.parse("2026-01-01T12:00:00Z")
2934
private val clock = FixedClock(nowInstant)
3035

36+
@BeforeTest
37+
fun setUp() {
38+
// The 401-challenge retry path closes the 401 response body; tests that exercise it
39+
// need an installed IoProvider so body materialization succeeds.
40+
Io.installProvider(OkioIoProvider)
41+
}
42+
3143
// ---- B14: fresh-token validation does NOT apply refreshMargin ----
3244

3345
/**
@@ -172,11 +184,128 @@ class BearerTokenAuthStepTest {
172184
assertEquals(2, calls, "provider must have been called again after prior exception")
173185
}
174186

187+
// ---- Evict-on-401 ----
188+
189+
@Test
190+
fun `401 challenge evicts the cached token and the retry carries a freshly fetched one`() {
191+
// A non-expiring token would normally be cached forever (no expiry margin ever fires).
192+
// The server rejects the first attempt with a 401 + WWW-Authenticate; the step must
193+
// evict the stale token and re-fetch so the single retry carries the fresh credential.
194+
val fetches = AtomicInteger(0)
195+
val provider =
196+
BearerTokenProvider { _, _ ->
197+
BearerToken("token-${fetches.incrementAndGet()}", futureExpiry)
198+
}
199+
val fake =
200+
FakeHttpClient()
201+
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
202+
.enqueue { status(200) }
203+
val pipeline =
204+
HttpPipelineBuilder(fake)
205+
.append(BearerTokenAuthStep(provider, listOf("scope"), clock = clock))
206+
.build()
207+
208+
val response = pipeline.send(getRequest())
209+
210+
assertEquals(200, response.status.code)
211+
assertEquals(2, fake.callCount, "exactly one retry after the 401")
212+
assertEquals(2, fetches.get(), "the 401 must trigger a re-fetch (eviction), not reuse the cached token")
213+
// First attempt carries the stale token; the retry carries the freshly fetched one.
214+
assertEquals("Bearer token-1", fake.requests[0].headers.get(HttpHeaderName.AUTHORIZATION))
215+
assertEquals("Bearer token-2", fake.requests[1].headers.get(HttpHeaderName.AUTHORIZATION))
216+
}
217+
218+
@Test
219+
fun `401 eviction works for a non-idempotent POST`() {
220+
// The in-step challenge hook is NOT gated by retry-safety, so a POST (non-idempotent)
221+
// still re-authenticates after a 401. A replayable body lets the retry re-send.
222+
val fetches = AtomicInteger(0)
223+
val provider =
224+
BearerTokenProvider { _, _ ->
225+
BearerToken("token-${fetches.incrementAndGet()}", futureExpiry)
226+
}
227+
val fake =
228+
FakeHttpClient()
229+
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
230+
.enqueue { status(200) }
231+
val pipeline =
232+
HttpPipelineBuilder(fake)
233+
.append(BearerTokenAuthStep(provider, listOf("scope"), clock = clock))
234+
.build()
235+
236+
val response = pipeline.send(postRequest())
237+
238+
assertEquals(200, response.status.code)
239+
assertEquals(2, fake.callCount, "the POST must be retried once after re-authentication")
240+
assertEquals(2, fetches.get())
241+
assertEquals("Bearer token-2", fake.requests[1].headers.get(HttpHeaderName.AUTHORIZATION))
242+
}
243+
244+
@Test
245+
fun `after a 401 eviction the next independent request still sees the refreshed token cached`() {
246+
// The retry re-fetches and re-caches; a subsequent unrelated request must reuse that
247+
// refreshed token rather than fetching a third time.
248+
val fetches = AtomicInteger(0)
249+
val provider =
250+
BearerTokenProvider { _, _ ->
251+
BearerToken("token-${fetches.incrementAndGet()}", futureExpiry)
252+
}
253+
val fake =
254+
FakeHttpClient()
255+
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
256+
.enqueue { status(200) }
257+
.enqueue { status(200) }
258+
val pipeline =
259+
HttpPipelineBuilder(fake)
260+
.append(BearerTokenAuthStep(provider, listOf("scope"), clock = clock))
261+
.build()
262+
263+
pipeline.send(getRequest()) // 401 → evict → retry with token-2
264+
pipeline.send(getRequest()) // reuses cached token-2
265+
266+
assertEquals(2, fetches.get(), "the refreshed token is cached; no third fetch")
267+
assertEquals("Bearer token-2", fake.requests[2].headers.get(HttpHeaderName.AUTHORIZATION))
268+
}
269+
270+
@Test
271+
fun `repeated 401s surface the second 401 without a second retry`() {
272+
// The base AuthStep retries exactly once. If the freshly fetched token is ALSO rejected,
273+
// the second 401 is surfaced as the operation result (no infinite refresh loop), and the
274+
// provider is consulted once per attempt (twice total).
275+
val fetches = AtomicInteger(0)
276+
val provider =
277+
BearerTokenProvider { _, _ ->
278+
BearerToken("token-${fetches.incrementAndGet()}", futureExpiry)
279+
}
280+
val fake =
281+
FakeHttpClient()
282+
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
283+
.enqueue { status(401).header("WWW-Authenticate", "Bearer realm=\"x\"") }
284+
val pipeline =
285+
HttpPipelineBuilder(fake)
286+
.append(BearerTokenAuthStep(provider, listOf("scope"), clock = clock))
287+
.build()
288+
289+
val response = pipeline.send(getRequest())
290+
291+
assertEquals(401, response.status.code, "the second 401 is surfaced, not retried again")
292+
assertEquals(2, fake.callCount)
293+
assertEquals(2, fetches.get())
294+
}
295+
175296
// ---- Helpers ----
176297

177298
private fun getRequest(): Request =
178299
Request.builder()
179300
.method(Method.GET)
180301
.url("https://api.example.com/resource")
181302
.build()
303+
304+
private fun postRequest(): Request =
305+
Request.builder()
306+
.method(Method.POST)
307+
.url("https://api.example.com/resource")
308+
// RequestBody.create(String) is replayable, so the non-idempotent retry can re-send.
309+
.body(RequestBody.create("payload"))
310+
.build()
182311
}

0 commit comments

Comments
 (0)