Skip to content

fix(client): DPoP follow-ups — apply at the fetch layer, fix edge cases - #1

Open
pcarleton wants to merge 2 commits into
gbshankar:feat/dpop-client-tokensfrom
modelcontextprotocol:paulc/pr2629-dpop-fetch-layer
Open

pcarleton wants to merge 2 commits into
gbshankar:feat/dpop-client-tokensfrom
modelcontextprotocol:paulc/pr2629-dpop-fetch-layer

Conversation

@pcarleton

Copy link
Copy Markdown

Follow-up to the DPoP (RFC 9449 / SEP-1932) client support, addressing review findings and reworking how the transports integrate it.

Integration shape

The initial version grew AuthProvider from {token, onUnauthorized} to five members (authorizeRequest / consumeChallenge / observeResponse + AuthRequestContext) and threaded a nonce-retry counter through every fetch site in both transports, each of which had to hand the request's method/URL to the provider and report the response back. The sites that got that wrong or were missed led to bugs e.g. SSE htu, DELETE-retry and 2xx-nonce bugs listed below. A fetch wrapper sees the real method/URL/response of every request by construction, and withDpop already existed, so:

  • AuthProvider is back to {token(); onUnauthorized?()}; adaptOAuthProvider is Bearer-only again; AuthRequestContext and the adaptOAuthProvider export are dropped.
  • withDpop(session, getToken) also accepts a lazy/provider-backed session source and passes the request through untouched when there is no session or token, so it can sit under a layer that already set Bearer. It now requires a fresh DPoP-Nonce before spending its retry and remembers a DPoP-Nonce from any response (RFC 9449 §8.2).
  • New withDpopFromProvider(provider): withDpop driven by provider.dpop() and provider.tokens(), upgrading to the DPoP scheme only for token_type=DPoP (RFC 9449 §7.1) and stamping provider failures as auth-seam errors.
  • StreamableHTTPClientTransport / SSEClientTransport wrap their resource-server fetch (including a caller-supplied fetch and SSE's eventSourceInit.fetch) with withDpopFromProvider when the OAuthClientProvider implements dpop(). The fetch handed to auth() stays unwrapped; token-endpoint DPoP remains executeTokenRequest's job. All hook plumbing and retry-counter threading is removed from the transports.
  • withOAuth and the conformance withOAuthRetry helper revert to Bearer + single re-auth and compose withDpopFromProvider underneath, which yields "nonce retry on every attempt, in either order" without a two-budget loop.

Behaviour fixes (each covered by a test that failed before) ----------------------------------------------------------

  • A token the AS issued as Bearer is presented as Bearer even when dpop() resolves (was always DPoP → hard 401 at a Bearer-only RS).
  • SSE POST proofs are bound to the announced message endpoint, not the /sse URL.
  • Session termination (DELETE) gets the same use_dpop_nonce retry as POST/GET.
  • A use_dpop_nonce challenge without a fresh DPoP-Nonce is not retried with the stale nonce.
  • A DPoP-Nonce delivered on a 2xx is used in the next proof.
  • addClientAuthentication runs per token-request attempt, so the AS nonce retry does not replay a private_key_jwt client_assertion (same jti).
  • auth() treats invalid_dpop_proof on refresh like invalid_grant (drop tokens, re-authorize) — e.g. a refresh token bound to a key no longer held. OAuthErrorCode gains InvalidDpopProof / UseDpopNonce.

Tests assert on the wire (real DpopSession behind an OAuthClientProvider, mocked fetch or a local HTTP server) rather than on hook invocations. Changeset updated to describe the final surface. Conformance auth/dpop 12/12, auth/dpop-nonce 14/14, auth suite 216/216 unchanged.

gbshankar and others added 2 commits August 9, 2026 10:52
Implements the client half of SEP-1932: DpopSession (key generation,
proof building, nonce tracking), optional AuthProvider.authorizeRequest/
consumeChallenge and OAuthClientProvider.dpop() hooks, DPoP wiring in the
token endpoint (auth.ts) and both transports (streamableHttp.ts, sse.ts),
and a standalone withDpop() middleware. Opt-in via provider.dpop() -
existing Bearer-only hosts are unaffected.

Also wires the conformance harness client side: registers auth/dpop and
auth/dpop-nonce against the pinned referee and removes them from
expected-failures.yaml.

Fixes a retry-ordering bug in withOAuth/withOAuthRetry found while
verifying against the real referee: the DPoP nonce challenge must be
re-checked *after* a credential re-authorization retry too, not only
before it (auth/dpop-nonce's actual sequence), so both are now tried in
whichever order the server presents them, bounded to one retry each.

Verified against the pinned @modelcontextprotocol/conformance referee:
auth/dpop 12/12, auth/dpop-nonce 14/14, full --suite all regression-clean.
Follow-up to the DPoP (RFC 9449 / SEP-1932) client support, addressing review
findings and reworking how the transports integrate it.

Integration shape
-----------------
The initial version grew AuthProvider from {token, onUnauthorized} to five
members (authorizeRequest / consumeChallenge / observeResponse +
AuthRequestContext) and threaded a nonce-retry counter through every fetch site
in both transports, each of which had to hand the request's method/URL to the
provider and report the response back. The sites that got that wrong or were
missed are exactly where the SSE htu, DELETE-retry and 2xx-nonce bugs below
came from. A fetch wrapper sees the real method/URL/response of every request
by construction, and withDpop already existed, so:

- AuthProvider is back to {token(); onUnauthorized?()}; adaptOAuthProvider is
  Bearer-only again; AuthRequestContext and the adaptOAuthProvider export are
  dropped.
- withDpop(session, getToken) also accepts a lazy/provider-backed session
  source and passes the request through untouched when there is no session or
  token, so it can sit under a layer that already set Bearer. It now requires a
  fresh DPoP-Nonce before spending its retry and remembers a DPoP-Nonce from
  any response (RFC 9449 §8.2).
- New withDpopFromProvider(provider): withDpop driven by provider.dpop() and
  provider.tokens(), upgrading to the DPoP scheme only for token_type=DPoP
  (RFC 9449 §7.1) and stamping provider failures as auth-seam errors.
- StreamableHTTPClientTransport / SSEClientTransport wrap their resource-server
  fetch (including a caller-supplied fetch and SSE's eventSourceInit.fetch) with
  withDpopFromProvider when the OAuthClientProvider implements dpop(). The fetch
  handed to auth() stays unwrapped; token-endpoint DPoP remains
  executeTokenRequest's job. All hook plumbing and retry-counter threading is
  removed from the transports.
- withOAuth and the conformance withOAuthRetry helper revert to Bearer + single
  re-auth and compose withDpopFromProvider underneath, which yields "nonce retry
  on every attempt, in either order" without a two-budget loop.

Behaviour fixes (each covered by a test that failed before)
----------------------------------------------------------
- A token the AS issued as Bearer is presented as Bearer even when dpop()
  resolves (was always DPoP → hard 401 at a Bearer-only RS).
- SSE POST proofs are bound to the announced message endpoint, not the /sse URL.
- Session termination (DELETE) gets the same use_dpop_nonce retry as POST/GET.
- A use_dpop_nonce challenge without a fresh DPoP-Nonce is not retried with the
  stale nonce.
- A DPoP-Nonce delivered on a 2xx is used in the next proof.
- addClientAuthentication runs per token-request attempt, so the AS nonce retry
  does not replay a private_key_jwt client_assertion (same jti).
- auth() treats invalid_dpop_proof on refresh like invalid_grant (drop tokens,
  re-authorize) — e.g. a refresh token bound to a key no longer held.
  OAuthErrorCode gains InvalidDpopProof / UseDpopNonce.

Tests assert on the wire (real DpopSession behind an OAuthClientProvider,
mocked fetch or a local HTTP server) rather than on hook invocations. Changeset
updated to describe the final surface. Conformance auth/dpop 12/12,
auth/dpop-nonce 14/14, auth suite 216/216 unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants