Skip to content

client/dpop: follow-ups from review (stacked on #394)#400

Draft
claude[bot] wants to merge 15 commits into
mainfrom
dpop-client-panel-fixes
Draft

client/dpop: follow-ups from review (stacked on #394)#400
claude[bot] wants to merge 15 commits into
mainfrom
dpop-client-panel-fixes

Conversation

@claude

@claude claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Requested by Paul Carleton · Slack thread

Follow-ups to #394 (now merged) from review feedback. Previously stacked on #401 (now merged), which registered the DPoP and WIF scenarios as the io.modelcontextprotocol/auth/dpop / io.modelcontextprotocol/auth/wif extensions — this PR carries only the remaining review fixes and now targets main directly.

Before / After

Before: the suite's spec link pointed at a personal working branch, the resource judge trusted whatever JWT the client presented (unverified decodeJwt), a spurious trailing slash in htu passed silently, the AS advertised ES256-only while enforcing ten algs, and failing implementers got rejection strings like "htu does not match the request URI" with no values to act on.

After: the spec link is commit-pinned, the judge verifies the presented token against the test AS issuer key and the exact cnf.jkt bound at the token endpoint, htu comparison is strict RFC 3986/9449 §4.3 normalization, advertisement matches enforcement (with unadvertised algs rejected per RFC 9449 §5.1), the PRM advertises dpop_bound_access_tokens_required / dpop_signing_alg_values_supported (RFC 9728 §2) so discovery-driven clients know DPoP is expected, and every htm/htu/iat rejection reports observed vs expected values (including the ±300s window).

How (one commit per fix)

  • Stable spec URLSpecReferences.DPOP_EXTENSION and sep-1932.yaml's spec_url now use a commit-pinned blob URL instead of the pieterkas-dpop-extension branch path, with a TODO to switch to the main-branch path once the doc merges.
  • Accurate failure attribution — an unverifiable access token is reported as a token problem ("could not be validated against the presented access token"), no longer as "DPoP proof was missing or malformed".
  • Reference client — sends the RFC 8707 resource parameter in both the authorization and token requests, validates state (and iss when returned) on the callback, and surfaces the OAuth error/error_description body in token-request failures instead of bare HTTP 400.
  • Strict htu normalization — dropped the trailing-slash leniency in both validators; only RFC 3986 syntax-based normalization (case, default port, empty path) is applied. Tests updated: the trailing-slash case now pins rejection, with a genuine-normalization acceptance case added.
  • Fail-fast AS options — setting dpopRequireNonce/dpopTokenRequestObs on a DPoP-disabled test AS now throws instead of silently doing nothing.
  • Alg-list coherence — one shared DPOP_ASYMMETRIC_ALGS constant; the scenario's AS metadata advertises exactly what the validators enforce, and the token-endpoint validator rejects proofs signed with an alg outside the advertised list (RFC 9449 §5.1) with an asymmetric-only floor (§11.6).
  • Negative-test isolation — the bearer/replay/no-token-proof tests now pin expectedSuccessSlugs, so a regression that breaks an unrelated check can't hide behind the subset assertion.
  • Resource judge token verification — the judge verifies the presented token's signature/issuer/exp against the test AS issuer key and cross-checks cnf.jkt against the jkt recorded at the token endpoint, closing the self-minted-token loophole.
  • sep-1932.yaml — the ten untested server/AS rows link their tracking issues (Server Auth: DPoP Proof Validation (SEP-1932) #369/Authorization Server Auth: DPoP Token Binding (SEP-1932) #370); the server-iat-window row is scoped to the spec's unconditional verbatim sentence (the stateless-server conditional moved to an excluded row); as-token-binding reworded to RFC 9449 §6's actual requirement (cnf.jkt for JWT tokens, introspection permitted).
  • Actionable rejection messages — htm/htu/iat rejections include observed and expected values and the ±300s window; missing htu/iat get their own messages distinct from mismatches.
  • PRM DPoP advertisement — the scenario's PRM carries dpop_bound_access_tokens_required: true and dpop_signing_alg_values_supported (RFC 9728 §2).
  • Non-extractable keysgenerateDpopKeyPair defaults to extractable: false (RFC 9449 §11 guidance); only the embedPrivateKey negative variants opt in.
  • Docstring fixesproof.test.tsdpopProof.test.ts; removed the pointer to nonexistent "project notes".
  • GET /mcp observation — the DPoP scenarios run the judge on GET /mcp and answer 405 (Streamable HTTP, no SSE stream), so non-POST proofs are observed; opt-in, other scenarios unaffected.
  • Observe-mode documentation — the judge's docstring now states that invalid proofs are recorded but served 200, and that real servers MUST 401 (RFC 9449 §7.2).

Skipped

  • §4.3 validator dedup refactor — the two validators are a deliberate two-copy design; added an explicit cross-reference comment in both instead of extracting a shared core.
  • SDK-capability prerequisite note (the reference client hand-rolls OAuth because SDKs lack a token-endpoint DPoP hook) — a docs/positioning question better decided by maintainers; no code change made.
  • Enforcing 401 on invalid proofs — kept observe-mode by design (documented instead), so sessions still complete and evidence accrues.

Testing

  • npm test — 37 files / 424 tests pass
  • npm run check (tsgo + eslint + prettier) — clean

Generated by Claude Code

@pkg-pr-new

pkg-pr-new Bot commented Jul 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

npx https://pkg.pr.new/@modelcontextprotocol/conformance@400

commit: 724563b

@claude claude Bot force-pushed the dpop-client-panel-fixes branch 3 times, most recently from 6ad0f76 to 9a1806c Compare July 10, 2026 16:41
claude added 15 commits July 10, 2026 16:45
…AS issuer key and the jkt bound at the token endpoint
…e iat-window to the spec's unconditional statement; align token-binding with RFC 9449 §6
@claude claude Bot force-pushed the dpop-client-panel-fixes branch from 9a1806c to 724563b Compare July 10, 2026 16:47
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.

1 participant