feat(sdk): handle DPoP-Nonce challenges (DSPX-3397) - #993
Draft
dmihalcik-virtru wants to merge 1 commit into
Draft
feat(sdk): handle DPoP-Nonce challenges (DSPX-3397)#993dmihalcik-virtru wants to merge 1 commit into
dmihalcik-virtru wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dmihalcik-virtru
force-pushed
the
dspx-3397-6-dpop-nonce
branch
from
August 10, 2026 16:38
85e74e1 to
06f9268
Compare
X-Test Failure Report |
dmihalcik-virtru
force-pushed
the
dspx-3397-6-dpop-nonce
branch
from
August 10, 2026 17:47
06f9268 to
a4e414f
Compare
dmihalcik-virtru
force-pushed
the
dspx-3397-5-cli-dpop-keys
branch
from
August 10, 2026 17:47
e6accca to
435b290
Compare
|
X-Test Failure Report |
RFC 9449 lets a server demand that DPoP proofs carry a server-issued nonce: an authorization server answers with HTTP 400 + `error=use_dpop_nonce` (§8), a resource server with HTTP 401 + `DPoP-Nonce` + `WWW-Authenticate: DPoP error="use_dpop_nonce"` (§9). Both replies include the nonce to use. The SDK ignored them, so against a nonce-requiring Keycloak or KAS every DPoP request failed and no retry was attempted. Adds an origin-keyed nonce cache (`src/auth/dpop-nonce.ts`) and wires it through the five places that mint or forward a DPoP proof: - `AccessToken.info` / `.doPost` (token + userinfo endpoints) - `access-fetch` (legacy REST rewrap and KAS-registry list) - `authTokenDPoPInterceptor` and `authProviderInterceptor` (Connect-RPC) Each sends with the cached nonce, and on a challenge adopts the server's nonce and retries exactly once. Two shared helpers -- `sendWithNonceRetry` (fetch) and `callWithNonceRetry` (Connect) -- own that logic so the five call sites stay a single implementation. Retries are capped at one; a server that omits the nonce or repeats the one we just sent gets a single warning naming the call site, and the original error propagates rather than looping. Connect errors do not reliably surface response headers, so `PlatformClient` wraps the transport's `fetch` to record `DPoP-Nonce` off the raw response. That only works if the transport and the auth layer share one cache instance, so the cache is exposed on `AuthProvider.nonceCache` and threaded from provider to interceptor to transport. `defaultNonceCache` is the shared fallback for custom providers; pass a dedicated `DPoPNonceCache` for per-client isolation. Decorators that wrap a provider must forward `nonceCache` -- the CLI's logging wrapper now does, and without it the two layers silently diverge and the retry never carries the nonce. Also fixes the three OIDC provider factories in `auth/providers.ts`, which dropped `dpopEnabled` and `signingKey` when constructing the `AccessToken`. DPoP was unreachable through the provider API regardless of nonce support. Test coverage: - `tests/server.ts` gains a strict RFC 9449 proof verifier (typ, alg, jwk shape, htm/htu, iat skew, jti replay, ath, cnf.jkt) and issues real nonce challenges from both the mock token endpoint and the KAS/policy RPC handlers. Strictness is the point: a regression in proof minting fails locally instead of only at xtest time. - New suites cover the cache, the token-endpoint retry, the rewrap retry, and the RPC retry, in both mocha and web-test-runner. - The roundtrip CI job sets `require_nonce: true` and marks the Keycloak clients `dpop.bound.access.tokens`, so the e2e path exercises the challenge rather than plain proof-of-possession. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
dspx-3397-6-dpop-nonce
branch
from
August 11, 2026 17:54
a4e414f to
83320a4
Compare
dmihalcik-virtru
force-pushed
the
dspx-3397-5-cli-dpop-keys
branch
from
August 11, 2026 17:54
435b290 to
e307560
Compare
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.
Stack: 5 of 6 — base is #991. This is the headline PR: everything beneath it is prefactoring split out of draft #939.
What
RFC 9449 lets a server demand that DPoP proofs carry a server-issued nonce — an AS answers HTTP 400 +
error=use_dpop_nonce(§8), an RS answers HTTP 401 +DPoP-Nonce+WWW-Authenticate: DPoP error="use_dpop_nonce"(§9). The SDK ignored both, so against a nonce-requiring Keycloak or KAS every DPoP request failed with no retry.This adds an origin-keyed nonce cache and wires it through the five places that mint or forward a proof:
AccessToken.doPost(token endpoint)DPoP-NonceAccessToken.info(userinfo)DPoP-Nonceaccess-fetch(legacy REST rewrap, KAS registry)DPoP-NonceauthTokenDPoPInterceptor(Connect)Code.UnauthenticatedauthProviderInterceptor(Connect)Code.UnauthenticatedTwo shared helpers own the retry so those five stay one implementation:
sendWithNonceRetry(fetch) andcallWithNonceRetry(Connect). Retry is capped at one. A server that omits the nonce or repeats the one we just sent produces a single warning naming the call site, then the original error propagates.Why the cache is threaded rather than global-only
Connect errors don't reliably surface response headers, so
PlatformClientwraps the transportfetchto captureDPoP-Nonceoff the raw response. That only works if the transport and the auth layer share one instance. HenceAuthProvider.nonceCache, threaded provider → interceptor → transport.defaultNonceCacheis the shared fallback so custom/legacy providers keep working; pass a dedicatedDPoPNonceCachefor per-client isolation.Decorators that wrap a provider must forward
nonceCache. The CLI's logging wrapper now does — without it the two layers diverge silently and the retry never carries the nonce.Drive-by fix
auth/providers.tsdroppeddpopEnabledandsigningKeywhen building theAccessTokenin all three factories, so DPoP was unreachable through the provider API regardless of nonce support.How to test
New suites:
dpop-nonce,dpop-rewrap-nonce,dpop-rpc-nonce(mocha) andauth/dpop-nonce,access/access-fetch,interceptors(web).tests/server.tsgains a strict RFC 9449 proof verifier — typ, alg, jwk shape, htm/htu, iat skew, jti replay,ath,cnf.jkt— and issues real nonce challenges from both the mock token endpoint and the KAS/policy RPC handlers. Strictness is deliberate: a proof-minting regression should fail locally rather than only at xtest time. It's a no-op for non-DPoP callers, so the existing Bearer tests are untouched.The roundtrip CI job sets
require_nonce: trueand marks the Keycloak clientsdpop.bound.access.tokens, so e2e exercises the challenge rather than plain proof-of-possession. xtest can't cover this: its nonce cases only run under the shareddpop-challengeinput, which also swaps in a platform config the other SDKs aren't ready for.Risk
Touches auth. The retry path is inert for non-DPoP providers and servers (they never emit a
DPoP-Nonce), and the cache falls back to a shared default when a provider doesn't expose one, so legacyAuthProviderimplementations are unaffected.