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

examples/clients/typescript/helpers/dpopClientFlow.ts

@@ -55,6 +55,9 @@ export async function runDpopClient(
   );
   const prm = await (await fetch(prmUrl.toString())).json();
   const authServerUrl: string = prm.authorization_servers[0];
+  // RFC 8707 resource indicator — sent in both the authorization request and
+  // the token request, per the MCP authorization spec.
+  const resource: string = prm.resource;
 
   // 2. Authorization server metadata.
   const asMeta = await (
@@ -96,7 +99,8 @@ export async function runDpopClient(
     state,
     redirect_uri: REDIRECT_URI,
     code_challenge: codeChallenge,
-    code_challenge_method: 'S256'
+    code_challenge_method: 'S256',
+    resource
   }).toString()}`;
   const authorizeResponse = await request(authorizeUrl, { method: 'GET' });
   await authorizeResponse.body.text().catch(() => undefined);
@@ -105,8 +109,20 @@ export async function runDpopClient(
   if (!locationStr) {
     throw new Error('Authorization endpoint did not redirect with a code');
   }
-  const code = new URL(locationStr).searchParams.get('code');
+  const callbackParams = new URL(locationStr).searchParams;
+  const code = callbackParams.get('code');
   if (!code) throw new Error('No authorization code in redirect');
+  // CSRF binding: the callback must echo our state (OAuth 2.1 §7.1).
+  if (callbackParams.get('state') !== state) {
+    throw new Error('Authorization response state does not match the request');
+  }
+  // RFC 9207: when the AS returns iss, it must equal the metadata issuer.
+  const callbackIss = callbackParams.get('iss');
+  if (callbackIss !== null && callbackIss !== asMeta.issuer) {
+    throw new Error(
+      `Authorization response iss "${callbackIss}" does not match the metadata issuer "${asMeta.issuer}"`
+    );
+  }
 
   // 5. Token request with a DPoP proof → DPoP-bound access token. The broken
   // `sendTokenRequestProof:false` variant omits the proof, so the AS issues an
@@ -116,7 +132,8 @@ export async function runDpopClient(
     code,
     redirect_uri: REDIRECT_URI,
     code_verifier: codeVerifier,
-    client_id: clientId
+    client_id: clientId,
+    resource
   }).toString();
   const requestToken = async (nonce?: string): Promise<Response> => {
     const headers: Record<string, string> = {
@@ -156,7 +173,17 @@ export async function runDpopClient(
     }
   }
   if (!tokenResponse.ok) {
-    throw new Error(`Token request failed: HTTP ${tokenResponse.status}`);
+    // Surface the OAuth error body — error_description carries the actionable
+    // detail (e.g. which DPoP proof claim the AS rejected).
+    const body = await tokenResponse
+      .json()
+      .catch(() => ({}) as { error?: string; error_description?: string });
+    const detail = [body.error, body.error_description]
+      .filter(Boolean)
+      .join(': ');
+    throw new Error(
+      `Token request failed: HTTP ${tokenResponse.status}${detail ? ` (${detail})` : ''}`
+    );
   }
   const tokenBody = await tokenResponse.json();
   const accessToken: string = tokenBody.access_token;

src/scenarios/client/auth/dpop.ts

@@ -17,6 +17,8 @@ import {
   newDpopClientObservations,
   type DpopClientObservations
 } from './helpers/dpopResourceAuth';
+import { DPOP_ASYMMETRIC_ALGS } from './helpers/dpopAlgs';
+import { generateIssuerKey } from './helpers/dpopToken';
 import { SpecReferences } from './spec-references';
 
 const PRM_PATH = '/.well-known/oauth-protected-resource/mcp';
@@ -174,10 +176,16 @@ export class DPoPClientScenario implements Scenario {
     this.obs = newDpopClientObservations();
     this.tokenReqObs = newTokenReqObs();
 
+    // The scenario owns the issuer key: the AS mints tokens with it and the
+    // resource judge verifies presented tokens against it.
+    const issuerKey = await generateIssuerKey();
     const authApp = createAuthServer(ctx, this.checks, this.authServer.getUrl, {
-      dpopSigningAlgValuesSupported: ['ES256'],
+      // Advertise exactly what the validators enforce, so a client honoring
+      // RFC 9449 §5.1 alg negotiation is graded the same as one that doesn't.
+      dpopSigningAlgValuesSupported: DPOP_ASYMMETRIC_ALGS,
       dpopTokenRequestObs: this.tokenReqObs,
-      dpopRequireNonce: this.requireNonce
+      dpopRequireNonce: this.requireNonce,
+      dpopIssuerKey: issuerKey
     });
     await this.authServer.start(authApp);
 
@@ -191,8 +199,22 @@ export class DPoPClientScenario implements Scenario {
           this.obs,
           () => `${this.server.getUrl()}/mcp`,
           () => `${this.server.getUrl()}${PRM_PATH}`,
-          this.requireNonce
-        )
+          this.requireNonce,
+          {
+            issuerKey,
+            getIssuer: () => this.authServer.getUrl(),
+            getBoundJkt: () => this.tokenReqObs.jkt
+          }
+        ),
+        // RFC 9728 §2: tell discovery-driven clients this resource expects
+        // DPoP-bound tokens (the wire-level enablement signal for DPoP).
+        prmDpop: {
+          signingAlgValuesSupported: DPOP_ASYMMETRIC_ALGS,
+          boundAccessTokensRequired: true
+        },
+        // SEP-1932's per-request-proof requirement is not POST-scoped, so
+        // observe proofs on GET /mcp too (answered 405 — no SSE stream here).
+        observeGetMcp: true
       }
     );
     await this.server.start(app);
@@ -326,7 +348,11 @@ export class DPoPClientScenario implements Scenario {
       errorMessage = 'Client never presented an access token to the MCP server';
     } else if (!this.obs.allProofsWellFormed) {
       status = 'FAILURE';
-      errorMessage = `DPoP proof was missing or malformed: ${this.obs.proofError}`;
+      // Attribute the failure to the right layer: a defect in the proof itself
+      // vs. an access token the proof cannot be validated against.
+      errorMessage = this.obs.proofError
+        ? `DPoP proof was missing or malformed: ${this.obs.proofError}`
+        : `DPoP proof could not be validated against the presented access token: ${this.obs.tokenError}`;
     } else if (this.obs.replayDetected) {
       status = 'FAILURE';
       errorMessage =

src/scenarios/client/auth/helpers/createAuthServer.ts

@@ -7,6 +7,7 @@ import { createRequestLogger } from '../../../request-logger';
 import { SpecReferences } from '../spec-references';
 import { MockTokenVerifier } from './mockTokenVerifier';
 import * as jose from 'jose';
+import { DPOP_ASYMMETRIC_ALGS } from './dpopAlgs';
 import {
   generateIssuerKey,
   mintDpopBoundToken,
@@ -29,31 +30,16 @@ function computeS256Challenge(codeVerifier: string): string {
 // Fixed nonce the test AS hands out when `dpopRequireNonce` is set (RFC 9449 §8).
 const AS_DPOP_NONCE = 'conformance-as-dpop-nonce';
 
-// Asymmetric JWS algorithms acceptable for a DPoP proof (RFC 9449 §4.3, §11.6).
-const DPOP_ASYMMETRIC_ALGS = [
-  'ES256',
-  'ES384',
-  'ES512',
-  'RS256',
-  'RS384',
-  'RS512',
-  'PS256',
-  'PS384',
-  'PS512',
-  'EdDSA'
-];
-
 /**
- * Canonicalize an `htu` for comparison per RFC 9449 §4.3 (RFC 3986 scheme-based
- * normalization): lowercase scheme/host, drop the default port, ignore a
- * trailing slash. Query/fragment are rejected by the caller (RFC 9449 §4.2),
- * not silently stripped here.
+ * Canonicalize an `htu` for comparison per RFC 9449 §4.3 (RFC 3986
+ * syntax-based normalization): lowercase scheme/host, drop the default port,
+ * empty path → `/`. No leniency beyond that — `/mcp/` and `/mcp` are distinct
+ * URIs. Query/fragment are rejected by the caller (RFC 9449 §4.2), not
+ * silently stripped here.
  */
 function normalizeHtu(u: string): string {
   try {
-    const url = new URL(u);
-    // Tolerate a single trailing slash only; `/mcp//` is a distinct path.
-    return `${url.protocol}//${url.host}${url.pathname.replace(/\/$/, '')}`;
+    return new URL(u).href;
   } catch {
     return u;
   }
@@ -63,12 +49,22 @@ function normalizeHtu(u: string): string {
  * Validate a DPoP proof presented at the token endpoint (the token-request
  * subset of RFC 9449 §4.3). Hand-rolled with jose primitives — deliberately an
  * INDEPENDENT code path from the suite's proof builder, so a shared bug
- * surfaces rather than hides. Returns the JWK thumbprint to bind on success.
+ * surfaces rather than hides. (Kept as a deliberate two-copy contract with
+ * validateResourceProof in dpopResourceAuth.ts — apply fixes to both.)
+ * Returns the JWK thumbprint to bind on success.
+ *
+ * `advertisedAlgs` is the AS's `dpop_signing_alg_values_supported`; proofs
+ * signed with an alg outside it are rejected (RFC 9449 §5.1), with an
+ * asymmetric-only floor (§11.6) regardless of what is advertised.
  */
 export async function validateDpopProofAtTokenEndpoint(
   proof: string,
-  tokenEndpointUrl: string
+  tokenEndpointUrl: string,
+  advertisedAlgs: string[] = DPOP_ASYMMETRIC_ALGS
 ): Promise<{ ok: true; jkt: string } | { ok: false; error: string }> {
+  const acceptedAlgs = advertisedAlgs.filter((a) =>
+    DPOP_ASYMMETRIC_ALGS.includes(a)
+  );
   let header: jose.ProtectedHeaderParameters;
   try {
     header = jose.decodeProtectedHeader(proof);
@@ -81,6 +77,12 @@ export async function validateDpopProofAtTokenEndpoint(
   if (!header.alg || !DPOP_ASYMMETRIC_ALGS.includes(header.alg)) {
     return { ok: false, error: 'alg must be a supported asymmetric algorithm' };
   }
+  if (!acceptedAlgs.includes(header.alg)) {
+    return {
+      ok: false,
+      error: `alg ${header.alg} is not among the advertised dpop_signing_alg_values_supported (${advertisedAlgs.join(', ')})`
+    };
+  }
   const jwk = header.jwk as jose.JWK | undefined;
   if (!jwk) {
     return { ok: false, error: 'missing jwk header parameter' };
@@ -91,20 +93,22 @@ export async function validateDpopProofAtTokenEndpoint(
   let claims: jose.JWTPayload;
   try {
     const key = await jose.importJWK(jwk, header.alg);
-    claims = (
-      await jose.jwtVerify(proof, key, { algorithms: DPOP_ASYMMETRIC_ALGS })
-    ).payload;
+    claims = (await jose.jwtVerify(proof, key, { algorithms: acceptedAlgs }))
+      .payload;
   } catch {
     return { ok: false, error: 'DPoP proof signature does not verify' };
   }
   if (typeof claims.jti !== 'string') {
     return { ok: false, error: 'missing jti claim' };
   }
   if (claims.htm !== 'POST') {
-    return { ok: false, error: 'htm does not match POST' };
+    return {
+      ok: false,
+      error: `htm "${claims.htm}" does not match the token request method "POST"`
+    };
   }
   if (typeof claims.htu !== 'string') {
-    return { ok: false, error: 'htu does not match the token endpoint' };
+    return { ok: false, error: 'missing or non-string htu claim' };
   }
   if (claims.htu.includes('?') || claims.htu.includes('#')) {
     return {
@@ -113,13 +117,20 @@ export async function validateDpopProofAtTokenEndpoint(
     };
   }
   if (normalizeHtu(claims.htu) !== normalizeHtu(tokenEndpointUrl)) {
-    return { ok: false, error: 'htu does not match the token endpoint' };
+    return {
+      ok: false,
+      error: `htu "${claims.htu}" does not match the token endpoint "${tokenEndpointUrl}" (compared after RFC 9449 §4.3 normalization)`
+    };
   }
-  if (
-    typeof claims.iat !== 'number' ||
-    Math.abs(Math.floor(Date.now() / 1000) - claims.iat) > 300
-  ) {
-    return { ok: false, error: 'iat outside the acceptable window' };
+  if (typeof claims.iat !== 'number') {
+    return { ok: false, error: 'missing or non-numeric iat claim' };
+  }
+  const iatSkew = Math.floor(Date.now() / 1000) - claims.iat;
+  if (Math.abs(iatSkew) > 300) {
+    return {
+      ok: false,
+      error: `iat ${claims.iat} is ${Math.abs(iatSkew)}s ${iatSkew > 0 ? 'behind' : 'ahead of'} server time; allowed window ±300s`
+    };
   }
   const jkt = await jose.calculateJwkThumbprint(jwk, 'sha256');
   return { ok: true, jkt };
@@ -152,6 +163,11 @@ export interface DpopTokenRequestObservation {
   asNonceChallengeIssued: boolean;
   /** The client retried the token request carrying the correct nonce. */
   asNonceHonored: boolean;
+  /**
+   * `cnf.jkt` bound into the most recently issued token, so the resource judge
+   * can assert the token presented at the MCP server is the one issued here.
+   */
+  jkt?: string;
 }
 
 export interface AuthServerOptions {
@@ -207,6 +223,12 @@ export interface AuthServerOptions {
     | 'unbound-token';
   /** Sink for the DPoP token-request observation; see the interface docstring. */
   dpopTokenRequestObs?: DpopTokenRequestObservation;
+  /**
+   * Issuer key for minting DPoP-bound tokens. Supply it when the scenario also
+   * needs to VERIFY the issued tokens (e.g. the resource judge); lazily
+   * generated when omitted.
+   */
+  dpopIssuerKey?: TokenIssuerKey;
   /**
    * When true, the token endpoint requires a DPoP nonce (RFC 9449 §8): a
    * proof-bearing request without the correct `nonce` claim is answered with
@@ -275,12 +297,19 @@ export function createAuthServer(
   let lastAuthorizationScopes: string[] = [];
   // Track PKCE code_challenge for verification in token request
   let storedCodeChallenge: string | undefined;
-  // Lazily-created issuer key for minting DPoP-bound JWT access tokens.
-  let dpopIssuerKey: TokenIssuerKey | undefined;
+  // Issuer key for minting DPoP-bound JWT access tokens (caller-supplied or
+  // lazily created).
+  let dpopIssuerKey: TokenIssuerKey | undefined = options.dpopIssuerKey;
   // DPoP behaviour is active only when the caller opts in (any DPoP option).
   const dpopEnabled =
     dpopSigningAlgValuesSupported !== undefined ||
     dpopMisbehavior !== undefined;
+  // Sub-options without DPoP enabled would be a silent no-op — fail fast.
+  if (!dpopEnabled && (dpopRequireNonce || dpopTokenRequestObs)) {
+    throw new Error(
+      'dpopRequireNonce/dpopTokenRequestObs require DPoP to be enabled (set dpopSigningAlgValuesSupported or dpopMisbehavior)'
+    );
+  }
 
   // Records whether the client presented a valid DPoP proof at its
   // authorization_code token request (RFC 9449 §5) into the caller's observation
@@ -602,7 +631,8 @@ export function createAuthServer(
       if (proof) {
         const result = await validateDpopProofAtTokenEndpoint(
           proof,
-          tokenEndpointUrl
+          tokenEndpointUrl,
+          dpopSigningAlgValuesSupported
         );
         if (!result.ok) {
           recordTokenRequestProof(grantType, false, result.error);
@@ -659,6 +689,11 @@ export function createAuthServer(
         if (!dpopIssuerKey) {
           dpopIssuerKey = await generateIssuerKey();
         }
+        // Record the bound thumbprint so the resource judge can assert the
+        // token presented at the MCP server is the one issued here.
+        if (grantType === 'authorization_code' && dpopTokenRequestObs) {
+          dpopTokenRequestObs.jkt = result.jkt;
+        }
         const boundToken = await mintDpopBoundToken({
           issuerKey: dpopIssuerKey,
           issuer: resolveIssuer(),