Gate the async 401 re-challenge replay on body replayability and stop closing the caller-owned 401

[OmarAlJarrah]

Summary

The synchronous and asynchronous auth steps disagree on how they handle a 401 + WWW-Authenticate re-challenge whose replacement request carries a non-replayable body.

AuthStep.process gates the replay on RequestBody.isReplayable(): when the replacement body is present and non-replayable, it skips the replay, returns the original 401 unchanged, and deliberately does not close it because the caller owns and consumes it.

AsyncAuthStep.handleChallenge has no such gate. Once the challenge hook yields a non-null replacement, it unconditionally closes the original 401 and re-drives the replacement through a fresh chain copy, regardless of whether the replacement body can be replayed.

Two things go wrong on the async path when the replacement body is non-replayable:

1. It closes a Response the caller still owns (the caller never asked for a retry that could actually run).
2. It re-sends a single-use body. The second write trips the body's consume-once guard, and the resulting IOException from the downstream chain masks the real 401 the caller should have seen.

This is a sync/async parity defect: AuthStep satisfies the replayability gate and AsyncAuthStep does not. The AsyncAuthStep class documentation states that "the stamping and challenge semantics mirror [AuthStep] exactly" (AsyncAuthStep.kt:25), so this is a divergence from the step's own documented contract.

Impact

This is a platform-wide defect in a core pipeline primitive. AsyncAuthStep is public SDK surface (org.dexpace.sdk.core.http.pipeline.steps.AsyncAuthStep) and the base class for every async auth strategy, including the shipped AsyncBearerTokenAuthStep. Any consumer, generated service client, or downstream SDK that:

• runs on the async pipeline, and
• installs an auth step whose challenge hook returns a replacement request built from a streaming / single-use body (the common shape for re-issuing a mutating request after a token refresh),

will, on a 401 challenge, get its caller-owned 401 closed out from under it and see a spurious body-consumed IOException instead of the authentication failure. Because the behavior lives in the core primitive, consumers cannot work around it from outside: they cannot make the step honor the replayability gate, and they cannot recover the closed 401. Every async caller inherits the bug, and the async and sync pipelines produce different observable outcomes for the identical scenario, breaking the parity guarantee the two stacks are supposed to hold.

Where

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

handleChallenge (declared at line 86) drives the replacement with no replayability check:

return challengeFuture.handle<CompletableFuture<Response>> { retryRequest, hookError ->
    if (hookError != null) {
        response.close()
        return@handle Futures.failed<Response>(Futures.unwrap(hookError))
    }
    if (retryRequest == null) return@handle CompletableFuture.completedFuture(response)
    response.close()                       // line 110 — unconditional close of the caller-owned 401
    next.copy().processAsync(retryRequest) // line 111 — unconditional re-drive, may re-send a single-use body
}.thenCompose { it }

There is no reference to isReplayable anywhere in the file.

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

process (the sync sibling) applies the gate at lines 114-118:

val retryBody = retryRequest.body
if (retryBody != null && !retryBody.isReplayable()) return response // line 115 — skip replay, 401 left OPEN

response.close()                          // line 117 — only reached when the body is replayable
return next.copy().process(retryRequest)  // line 118

The primitive the gate relies on: RequestBody.isReplayable() — base default false at sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/RequestBody.kt:63, overridden to true on the in-memory / replayable body types (lines 243, 260, 288).

Expected vs. actual

Scenario: 401 with WWW-Authenticate; the async challenge hook returns a non-null replacement request whose body is present and not replayable.

• Expected (matching the sync AuthStep and AUTH-31): skip the replay, complete the returned future with the original 401 left open, and do not close it — the caller owns and consumes it.
• Actual: the async step closes the original 401 (line 110) and re-drives the replacement (line 111). The single-use body is written a second time, its consume-once guard trips, and the caller receives an IOException from the downstream chain instead of the 401 — with the 401 already closed and unrecoverable.

For a replayable body, both paths behave the same (close, then re-drive once), so the fix changes behavior only in the non-replayable case.

Proposed fix

Mirror the sync gate inside AsyncAuthStep.handleChallenge, immediately after the non-null retryRequest check and before the close/re-drive:

if (retryRequest == null) return@handle CompletableFuture.completedFuture(response)
val retryBody = retryRequest.body
if (retryBody != null && !retryBody.isReplayable()) {
    // Body cannot be replayed: skip the replay and surface the original 401 unchanged.
    // The caller owns and consumes it, so it MUST stay open.
    return@handle CompletableFuture.completedFuture(response)
}
response.close()
next.copy().processAsync(retryRequest)

Only close-and-re-drive when the replacement body is null or replayable. This reuses the existing RequestBody.isReplayable() and pulls in no new dependency, so it stays within sdk-core's near-zero-runtime-dependency constraint.

Update the documentation that currently describes an unconditional drive so it reflects the gate, matching the wording already on the sync AuthStep (class KDoc lines 40-44, method KDoc lines 130-143):

• the class-level "Challenge retry" bullet (AsyncAuthStep.kt lines 33-35),
• the KDoc on handleChallenge (lines 81-85),
• the KDoc on authorizeRequestOnChallengeAsync (lines 124-134).

Add an async test covering the non-replayable case (assert the returned future completes with the original 401 and that the response is not closed), paralleling the existing sync coverage.

Acceptance criteria

• With a non-replayable replacement body, AsyncAuthStep completes the returned future with the original 401 Response and does not close it (assert close() was not invoked, e.g. via a response whose body tracks consumption).
• With a replayable (or null) replacement body, AsyncAuthStep still closes the original 401 and drives the replacement through a fresh chain copy exactly once — unchanged from today.
• The non-replayable case no longer surfaces an IOException in place of the 401.
• A test asserts the sync (AuthStep) and async (AsyncAuthStep) paths produce the same observable outcome (same status returned, response left open) for the identical non-replayable-replacement scenario.
• The hook-error and sync-throw paths remain unchanged: the original 401 is still closed and the error propagated (see AUTH-32).

Note this is orthogonal to the hook-error path (AUTH-32), which must still close the 401 on a failed future or synchronous throw; those two branches (lines 105-108 and 100-101) stay as they are.

References

• AUTH-31 (MUST) — the 401 re-challenge replay must be gated on request-body replayability; on a non-replayable replacement body the step must skip the replay, surface the original 401 unchanged, and must not close it. The spec explicitly notes the gate is enforced on the sync auth step only and that a faithful port should apply it on both paths (docs/product-spec.md lines 442 and 1523).
• AUTH-30 (MUST) — the close-then-re-drive-once behavior this fix keeps for the replayable case (docs/product-spec.md lines 441, 1522).
• AUTH-32 (MUST) — the hook-error path that must still close the 401; unaffected by this change (docs/product-spec.md lines 441, 1524).
• AUTH-38 (SHOULD) — async errors delivered through the result channel; the skipped-replay outcome here is a normal completion, not an error (docs/product-spec.md lines 443, 1530).
• Sync reference implementation: AuthStep.process (sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/AuthStep.kt:114-118).