Skip to content

feat(sep-1932): DPoP discovery signals (stacked on #528) - #529

Open
nbarbettini wants to merge 6 commits into
modelcontextprotocol:mainfrom
nbarbettini:feat/1932-server-dpop-discovery
Open

nbarbettini wants to merge 6 commits into
modelcontextprotocol:mainfrom
nbarbettini:feat/1932-server-dpop-discovery

Conversation

@nbarbettini

@nbarbettini nbarbettini commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #528 -> #527 -> #395 (PieterKas/conformance:dpop-server, head dd9fd77). This branch contains those commits plus 5d213b9. Rebase onto main once #528, #527, and #395 merge.

The new commit (5d213b9) is the only one to review.

What each check asserts

#395 checks that a malformed proof is rejected. It does not check how a client that has never met the server learns that DPoP is required, or that the advertisement matches what the server enforces.

This assumes SEP-1932 will require the server to advertise DPoP - which @PieterKas proposed (and I agree with!). With that assumption, a missing DPoP challenge or a missing Protected Resource Metadata field is a failure. Missing algs stays a WARNING because RFC 9449 §7.1 says SHOULD.

sep-1932-server-advertises-dpop

RFC 9449 §7.1, graded SHOULD for algs from "An algs parameter SHOULD be included to signal to the client the JWS algorithms that are acceptable for the DPoP proof JWT. The value of the parameter is a space-delimited list of JWS alg (Algorithm) header values ([RFC7515], Section 4.1.1)."

An unauthenticated POST, no Authorization and no DPoP header.

  • 401 with a DPoP challenge that includes algs: SUCCESS
  • algs missing: WARNING
  • No DPoP scheme: FAILURE

The resource_metadata parameter is captured from the DPoP challenge, or from a sibling challenge when the DPoP one does not carry it (RFC 9728 §5.1).

sep-1932-server-prm-dpop

RFC 9728 §2 marks both fields OPTIONAL. Assuming SEP-1932 requires them, absence is a failure.

Fetched from resource_metadata when the challenge has it, otherwise the path-based well-known URL, otherwise the root one. Same order as the client metadata-discovery scenarios. A resource_metadata URL that does not return a document is not replaced by a different well-known document.

  • dpop_signing_alg_values_supported present, non-empty, and asymmetric only: the field half is satisfied. none or HS* is FAILURE, same rule as sep-1932-as-no-none-alg (RFC 9449 §11.6). Absent or empty: FAILURE
  • dpop_bound_access_tokens_required present as a boolean: the field half is satisfied. Absent: FAILURE
  • Metadata unreachable: not testable (FAILURE)

sep-1932-server-prm-consistency

An unbound token (omitCnf), presented as Bearer with no DPoP header. Gated on the positive proof.

  • Metadata says required and the server accepts: FAILURE
  • Metadata says required and the server rejects with 401: SUCCESS
  • Metadata says not required, or is silent, and the server rejects with 401: FAILURE ("server requires DPoP but does not advertise it")
  • Metadata says not required, or is silent, and the server accepts: INFO

Testing

  • DPoP server scenario tests pass (16), including the omit-fields, unbound-bearer, none, and missing-algs fixtures
  • eslint + prettier — clean on the touched files

Made with Cursor

PieterKas and others added 6 commits September 9, 2026 18:52
…ocol#369)

Follow-up on the DPoP client PR (shared foundation). Adds the server-role
conformance for SEP-1932 / RFC 9449: the framework acts as a DPoP client
against the MCP server under test and emits the sep-1932-server-* checks
across the RFC 9449 §4.3 validation surface — proof validation, the ±5-minute
iat window, asymmetric-only algorithms, the 401 + WWW-Authenticate challenge,
token audience validation under DPoP, and the optional server-provided nonce.

- src/scenarios/server/auth/dpop.ts (+ test, spec-references): the scenario.
- examples/servers/typescript/sep-1932-{compliant,broken}-server.ts: passing
  and failing fixtures proving every check passes and fails.
- Registered in the pending + all-client scenario lists.

Depends only on the shared DPoP foundation (dpopProof/dpopToken); independent
of the authorization-server PR. No DPoP-capable MCP SDK exists yet, so
correctness rests on RFC vectors + an independent verifier + the fixtures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Refresh the held DPoP nonce from every response's DPoP-Nonce header
  (RFC 9449 §8.2 newest-wins) instead of capturing it once, so a server that
  rotates or single-uses its nonce no longer turns the negative probes into
  false "not testable" failures (or vacuous passes). (modelcontextprotocol#1)
- Widen the stale iat probe to -303 s (matching the future side) so the ±1 s
  quantization of the whole-second Date header can't pull it onto the ±300 s
  boundary and be false-accepted. (modelcontextprotocol#2)
- Update the isNonceChallenge comment and the untestable message: probes now
  carry the held nonce, so a use_dpop_nonce challenge is about the nonce
  lifetime (rotated/stale/single-use), not a missing nonce. (modelcontextprotocol#3)
- Fixture: parse integer env vars NaN-safely (intEnv helper) so a malformed
  DPOP_CLOCK_OFFSET_SECONDS / DPOP_IAT_SKEW_SECONDS falls back to its default
  instead of silently disabling the iat window. (modelcontextprotocol#4)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Soften the heldNonce comment: newest-wins refresh is correct for ROTATING
  servers; a strict single-use server that re-arms only via challenges can still
  push alternate negatives to untestable (correctly reported, not mis-scored) —
  the previous "rotate or single-use" over-claimed. (R5)
- Fix the clock-skew test comment: the stale probe is now -303s (~273s old),
  not -301s/~271s. (R5)

Comment-only; no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A crash or 404 must not count as correctly refusing a DPoP-bound token presented as Bearer.

Co-authored-by: Cursor <cursoragent@cursor.com>
A challenge with no error code was passing. The code is a SHOULD, so a wrong or missing value is a warning and the existing MUST checks stay as they are.

Co-authored-by: Cursor <cursoragent@cursor.com>
A client that has never met the server has to learn the requirement from the 401 challenge and from protected resource metadata. A missing challenge or metadata field is a failure, and the advertisement has to match what the server enforces.

Co-authored-by: Cursor <cursoragent@cursor.com>
@nbarbettini
nbarbettini force-pushed the feat/1932-server-dpop-discovery branch from f20bf07 to 5d213b9 Compare September 25, 2026 18:39
@PieterKas

Copy link
Copy Markdown
Contributor

@nbarbettini I am reviewing the PR and also revisiting the SEP1932 text. I think the following is overly strict (or at least as I think the tests are implemented):

With that assumption, a missing DPoP challenge or a missing Protected Resource Metadata field is a failure.

Here are my thoughts process. I think if the MCP server supports DPoP it MUST advertise that support in protected resource metadata (PRM). However, there will be a lot of MCP servers out there that does not support it and won't advertise that they don't support it, they just won't have that field set. According to RFC9728, absence of a field means "not supported". I don''t think we want to register absence of the field as a failure, but rather as being equivalent to being set to "false". I feel like this is a more reasonable approach and avoids a lot of servers having to advertise "false" when they can achieve the same by just not advertising support.

DISCOVERY_REFERENCES
);
}
if (!algsOk || !requiredPresent) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grading change needed here: an absent dpop_bound_access_tokens_required is a valid state, not a violation (see proposed update to SEP-1932 modelcontextprotocol/ext-auth#33).

RFC 9728 §2 defines the field as OPTIONAL with "If omitted, the default value is false" — so silence is a defined answer ("DPoP-bound tokens not required, bearer OK"), not a missing one. SEP-1932 now states this explicitly in the new "MCP Server DPoP Advertisement" section added in modelcontextprotocol/ext-auth#33: "When the field is absent, the default behavior defined in RFC 9728 applies and DPoP-bound access tokens are not required." Requiring the field's presence would also fail every metadata generator that correctly follows the RFC.

As written, requiredPresent folds absence into this FAILURE gate, so a fully conformant bearer-accepting server — correct dpop_signing_alg_values_supported, flag validly omitted — is failed with "metadata is missing dpop_bound_access_tokens_required" (reproduced against a live fixture). The consistency check downstream already handles the absent case correctly via its own arms, so nothing else depends on presence here.

Suggested reshape of this gate:

  • absent → SUCCESS (keep recording dpop_bound_access_tokens_required: null in details, which you already do);
  • present but not a boolean → keep FAILURE, but as "malformed", not "missing";
  • and while splitting the message: !algsOk also fires for a present-but-empty (or non-array) dpop_signing_alg_values_supported, which this message reports as "missing" — worth separate wording ("empty"/"malformed") so operators aren't sent looking for a field that's already there.

Generated with the aid of Claude (Fable 5).

algs: algs ?? null,
resourceMetadata: resourceMetadataParam(res.wwwAuthenticate) ?? null
};
if (!challenged) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grading change: "401 without a DPoP challenge" needs two tiers, keyed off the metadata this scenario already fetches.

SEP-1932's new advertisement section (modelcontextprotocol/ext-auth#33) grades the challenge differently depending on the server's declared posture:

  • A server that merely supports DPoP — dpop_signing_alg_values_supported published, dpop_bound_access_tokens_required absent or false (bearer tokens accepted per the RFC 9728 default) — is under a SHOULD: "an MCP server supporting this extension SHOULD include a DPoP challenge…". For it, a textbook RFC 6750 Bearer-only challenge is conformant (and discovery still works — resourceMetadataParam() in this function reads resource_metadata from the Bearer challenge, since RFC 9728 §5.1's parameter is scheme-independent). Missing DPoP scheme → WARNING.

  • A server that requires DPoP — dpop_bound_access_tokens_required: true — is under a MUST: "An MCP server that sets dpop_bound_access_tokens_required to true … MUST include a DPoP challenge in every WWW-Authenticate response it sends." For it, a Bearer-only challenge invites authentication it will always reject. Missing DPoP scheme → FAILURE.

As written, this arm hard-FAILs both postures, which over-grades the first (a conformant server fails) and under-specifies the second (the FAILURE is right but for a reason the message doesn't state). Since the PRM document is fetched later in run() anyway, the simplest shape is probably: record the challenge observation here, grade it after discovery — WARNING when the metadata is silent/false/unreachable (the MUST tier can't be established), FAILURE with a "required=true but no DPoP challenge" message when the metadata says true. The catch-path untestableCheck for this check (~line 887) should likewise default to WARNING severity per the #248 keyword convention, since the unconditional obligation is only a SHOULD.

Note the SEP also expressly permits other challenges alongside the DPoP one (e.g. the RFC 9449 §7.2 Bearer error carrier) — so the predicate should stay "DPoP challenge present among the challenges", never "DPoP only".

Generated with the aid of Claude (Fable 5).

CONSISTENCY_NAME,
CONSISTENCY_DESC,
'FAILURE',
'server requires DPoP but does not advertise it',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This arm can assert something the run never observed. saysDpopRequired is initialized to false (line 859) and only becomes true when a metadata document was successfully fetched and read — so "we read the metadata and it doesn't advertise DPoP-required" and "we never managed to read any metadata" arrive here looking identical. In the second case this check still returns a definitive FAILURE saying the server "does not advertise it", and the details carry dpop_bound_access_tokens_required: false — a value no document ever supplied.

Reproduced against a live fixture whose PRM endpoint 404s while the backend requires DPoP: the run reports PrmDpop = "Not testable: … unreachable" and PrmConsistency = FAILURE "server requires DPoP but does not advertise it" — two red marks from one root cause, with the second claiming knowledge of a document that was never retrieved. An operator reading that report would hunt for a metadata field problem when their actual issue is that the metadata endpoint is down.

Fix (the document-was-read case is correct and unchanged): track three states instead of two — required / not required / no document, e.g. boolean | null — and when no document was read, report this check via untestableCheck ("cannot evaluate advertisement consistency: protected resource metadata was unreachable") instead of this arm.

Generated with the aid of Claude (Fable 5).

];
for (const candidate of candidates) {
const doc = await fetchJsonDocument(candidate.url);
if (doc) return { doc, source: candidate.source, url: candidate.url };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing gate: before grading the DPoP fields, check that the document is one a conformant client would be allowed to use.

RFC 9728 §3.3 requires every metadata consumer to verify the document's resource field identifies the resource it asked about, and on mismatch "the data contained in the response MUST NOT be used" (§7.3 explains the impersonation attack this prevents). A server whose metadata fails that gate is serving metadata that's unusable to conformant clients — however correct the DPoP fields inside it are.

This function skips the gate: reproduced with a fixture serving otherwise-valid metadata with resource: "https://attacker.example/mcp" — the document is consumed as-is, PrmDpop grades SUCCESS, and the consistency check trusts its dpop_bound_access_tokens_required. So the check currently asserts "the right fields exist in some JSON at the right URL" rather than what we actually want: "a conformant client following this server's discovery would obtain usable metadata and find correct DPoP advertisement in it."

Fix is small: at both successful-fetch returns (line 494 and here), compare doc.resource to this function's resourceUrl (per §3.3 that's the right comparand on both discovery paths), and on mismatch return a { reason } naming the two values instead of the document. The existing reason-handling then does the rest: PrmDpop reports the mismatch, and the consistency check has no disqualified value to trust.

Generated with the aid of Claude (Fable 5).

res: Response
): ConformanceCheck {
const accepted = isAccepted(res.statusCode);
const rejected = res.statusCode === 401;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SEP's MUST says "reject", not "401" — so a server that refuses the unbound Bearer token with 403 or 400 satisfies the requirement but currently fails the says-required arm.

Suggested split, since this shared predicate serves two arms with different jobs: for the says-required arm (grading the MUST from modelcontextprotocol/ext-auth#33), count any non-2xx as rejection; for the silent arm (inferring unadvertised DPoP enforcement), keep 401-only and add an isNonceChallenge guard like rejectionCheck has — a use_dpop_nonce 401 says nothing about the Bearer token, and an accusation needs stronger evidence than a MUST-compliance pass.

Generated with the aid of Claude (Fable 5).

} catch {
return undefined;
}
if (res.statusCode < 200 || res.statusCode >= 300) return undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A healthy server whose metadata URL answers with an ordinary redirect (e.g. the trailing-slash 301/308 that nginx/Express add by default) is reported "metadata unreachable" and fails the discovery checks — undici's request doesn't follow 3xx, so the redirect lands in this non-2xx branch as undefined, even though any real client resolves it in one hop. Fix: allow a redirect hop or two (maxRedirections).

Two smaller gaps in the same function, both against RFC 9728 §3.2 ("A successful response MUST use the 200 OK HTTP status code and return a JSON object using the application/json content type"): the status gate accepts any 2xx where the spec requires exactly 200, and the Content-Type header is never checked, so a response labeled text/html whose body happens to parse as JSON would be accepted and graded. Both are one-line guards, worth adding while this function is being touched for the redirect fix.

Generated with the aid of Claude (Fable 5).

): ConformanceCheck {
const accepted = isAccepted(res.statusCode);
const rejected = res.statusCode === 401;
const details = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: details carry only the status code — adding the response's WWW-Authenticate value (or its parsed error code) would make this check's verdicts auditable from the report, same as the other rejection checks.

Generated with the aid of Claude (Fable 5).

Comment thread src/seps/sep-1932.yaml
Comment on lines +22 to +33
# Quoted from RFC 9449 §7.1. Missing `algs` is WARNING (SHOULD).
# No DPoP scheme is a failure: SEP-1932 requires the server to advertise DPoP.
- check: sep-1932-server-advertises-dpop
text: 'An `algs` parameter SHOULD be included to signal to the client the JWS algorithms that are acceptable for the DPoP proof JWT. The value of the parameter is a space-delimited list of JWS `alg` (Algorithm) header values ([RFC7515], Section 4.1.1).'
# Quoted from RFC 9728 §2, where both fields are OPTIONAL. SEP-1932 requires
# them, so a missing field is a failure. A listed `none` or `HS*` value is
# a failure per RFC 9449 §11.6, matching sep-1932-as-no-none-alg.
- check: sep-1932-server-prm-dpop
text: '`dpop_signing_alg_values_supported`: OPTIONAL. JSON array containing a list of the JWS `alg` values (from the "JSON Web Signature and Encryption Algorithms" registry [IANA.JOSE]) supported by the resource server for validating Demonstrating Proof of Possession (DPoP) proof JWTs [RFC9449]. / `dpop_bound_access_tokens_required`: OPTIONAL. Boolean value specifying whether the protected resource always requires the use of DPoP-bound access tokens [RFC9449]. If omitted, the default value is false.'
# The boolean's meaning is the RFC 9728 §2 sentence above. The SEP text does not contain it yet.
- check: sep-1932-server-prm-consistency
text: '`dpop_bound_access_tokens_required`: OPTIONAL. Boolean value specifying whether the protected resource always requires the use of DPoP-bound access tokens [RFC9449]. If omitted, the default value is false.'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three rows quote RFC 9449/9728 text and attribute it to SEP-1932 — including sentences whose own keywords (SHOULD, OPTIONAL) contradict the FAILURE grading the checks apply — because the SEP sentences didn't exist when this was written. They exist now: SEP-1932's "MCP Server DPoP Advertisement" section (modelcontextprotocol/ext-auth#33) has exact sentences for all three checks, so per this file's verbatim-from-SEP convention the rows should quote those (and the comment below saying "The SEP text does not contain it yet" is no longer true):

Suggested change
# Quoted from RFC 9449 §7.1. Missing `algs` is WARNING (SHOULD).
# No DPoP scheme is a failure: SEP-1932 requires the server to advertise DPoP.
- check: sep-1932-server-advertises-dpop
text: 'An `algs` parameter SHOULD be included to signal to the client the JWS algorithms that are acceptable for the DPoP proof JWT. The value of the parameter is a space-delimited list of JWS `alg` (Algorithm) header values ([RFC7515], Section 4.1.1).'
# Quoted from RFC 9728 §2, where both fields are OPTIONAL. SEP-1932 requires
# them, so a missing field is a failure. A listed `none` or `HS*` value is
# a failure per RFC 9449 §11.6, matching sep-1932-as-no-none-alg.
- check: sep-1932-server-prm-dpop
text: '`dpop_signing_alg_values_supported`: OPTIONAL. JSON array containing a list of the JWS `alg` values (from the "JSON Web Signature and Encryption Algorithms" registry [IANA.JOSE]) supported by the resource server for validating Demonstrating Proof of Possession (DPoP) proof JWTs [RFC9449]. / `dpop_bound_access_tokens_required`: OPTIONAL. Boolean value specifying whether the protected resource always requires the use of DPoP-bound access tokens [RFC9449]. If omitted, the default value is false.'
# The boolean's meaning is the RFC 9728 §2 sentence above. The SEP text does not contain it yet.
- check: sep-1932-server-prm-consistency
text: '`dpop_bound_access_tokens_required`: OPTIONAL. Boolean value specifying whether the protected resource always requires the use of DPoP-bound access tokens [RFC9449]. If omitted, the default value is false.'
# Quoted from the SEP's "MCP Server DPoP Advertisement" section. Challenge
# advertisement is two-tier: SHOULD for any server supporting the extension,
# MUST when dpop_bound_access_tokens_required is true.
- check: sep-1932-server-advertises-dpop
text: 'When responding to an unauthenticated or failed request with HTTP 401, an MCP server supporting this extension SHOULD include a `DPoP` challenge in the `WWW-Authenticate` header, and that challenge SHOULD include the `algs` parameter signaling the JWS algorithms acceptable for DPoP proof JWTs, as described in RFC 9449 Section 7.1. / An MCP server that sets `dpop_bound_access_tokens_required` to `true` in its Protected Resource Metadata MUST include a `DPoP` challenge in every `WWW-Authenticate` response it sends.'
- check: sep-1932-server-prm-dpop
text: 'MCP servers supporting this extension MUST include the `dpop_signing_alg_values_supported` field in their Protected Resource Metadata (RFC 9728 Section 2). This field MUST contain a non-empty JSON array of registered JWS algorithm values (from the IANA JSON Web Signature and Encryption Algorithms registry) that the server accepts for DPoP proof JWTs. Only asymmetric digital signature algorithms are permitted. The `none` algorithm and symmetric (MAC) algorithms MUST NOT be included.'
# Absence of the boolean is valid (RFC 9728 default) — see the grading
# comments on dpop.ts.
- check: sep-1932-server-prm-consistency
text: 'An MCP server that requires DPoP-bound access tokens (that is, one that does not accept bearer tokens) MUST set the `dpop_bound_access_tokens_required` field to `true` in its Protected Resource Metadata. Conversely, an MCP server that sets `dpop_bound_access_tokens_required` to `true` MUST reject requests that do not present a DPoP-bound access token with a valid DPoP proof. When the field is absent, the default behavior defined in RFC 9728 applies and DPoP-bound access tokens are not required.'

One dependency to be aware of: these quotes match the current text of ext-auth#33; if its wording shifts in auth-wg review, this file should be re-synced to the merged version.

Generated with the aid of Claude (Fable 5).

@PieterKas

Copy link
Copy Markdown
Contributor

Big-picture: the test fixtures (the fake servers the suite runs against) have no way to trigger the situations described in the other review comments — so the grading fixes those comments ask for can't be tested, and could silently regress later. This comment collects the fixture modes and test cases needed to close that gap.

Fixture modes (sep-1932-compliant-server.ts):

  • Split DPOP_PRM_OMIT_FIELDS: it currently drops both DPoP fields at once, but under the corrected grading those two omissions have opposite meanings — missing dpop_signing_alg_values_supported is a MUST violation (FAILURE), while an absent dpop_bound_access_tokens_required is a valid configuration (SUCCESS). One mode per field (DPOP_PRM_OMIT_ALGS, DPOP_PRM_OMIT_REQUIRED), each flipping exactly its targeted outcome.
  • DPOP_PRM_DISABLED (no PRM route, DPoP-enforcing backend): pins the metadata-unreachable paths — PrmDpop's unreachable reason and the consistency check reporting "cannot evaluate" instead of a fabricated verdict (line 666 comment).
  • A wrong-resource mode (metadata served with resource naming a different URL): pins the RFC 9728 §3.3 gate (line 507 comment).
  • A Bearer-only-challenge mode (PRM correct, 401 carries no DPoP scheme): pins the two-tier advertises grading (line 524 comment) — WARNING when required is absent/false, FAILURE when the same fixture also sets required: true.

Test arms with no coverage today, worth pinning while in here:

  • dpop_signing_alg_values_supported: [] and non-array values → FAILURE (and the message distinguishing "empty"/"malformed" from "missing");
  • an HS256 entry in the PRM algs list → FAILURE (only none is currently exercised);
  • present-but-non-boolean dpop_bound_access_tokens_required → FAILURE;
  • consistency says-required arm satisfied by a 403 rejection (line 622 comment);
  • the well-known fallback fetch exercised end-to-end — every fixture 401 currently carries resource_metadata, so the path-based/root candidate loop only ever runs in URL-builder unit tests; a mode that omits the challenge parameter would cover it.

Each new mode should flip exactly its targeted check and leave the rest green, per the suite's one-defect convention.

Generated with the aid of Claude (Fable 5).

@PieterKas PieterKas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thorough review done (adversarial pass with the aid of Claude, Fable 5 — findings verified empirically against live fixtures). The architecture is good: discovery probes are cleanly separated, the consistency probe reuses the positive token minus cnf so rejections are attributable, and the suites run green.

Requesting changes for two themes, detailed in 9 comments:

  1. Grading drift against the proposed SEP text: the "MCP Server DPoP Advertisement" section proposed in modelcontextprotocol/ext-auth#33 grades these behaviors differently than this PR assumes — absent dpop_bound_access_tokens_required is valid (line 591), the challenge is two-tier SHOULD/MUST (line 524), "reject" isn't "401" (line 622), and the yaml rows should quote the SEP sentences (yaml comment, with suggestion).
  2. Verdicts without evidence: the consistency check asserts metadata contents when no document was read (line 666), and documents are consumed without the RFC 9728 §3.3 resource check (line 507); plus fetcher robustness (line 456) and a details nit (line 623).

The Conversation-tab comment collects the fixture modes and tests needed to pin all of the above. Sequencing: ext-auth#33 needs to land first so the yaml quotes track its final wording; happy to re-review quickly after.

@nbarbettini

Copy link
Copy Markdown
Contributor Author

@nbarbettini I am reviewing the PR and also revisiting the SEP1932 text. I think the following is overly strict (or at least as I think the tests are implemented):

With that assumption, a missing DPoP challenge or a missing Protected Resource Metadata field is a failure.

Here are my thoughts process. I think if the MCP server supports DPoP it MUST advertise that support in protected resource metadata (PRM). However, there will be a lot of MCP servers out there that does not support it and won't advertise that they don't support it, they just won't have that field set. According to RFC9728, absence of a field means "not supported". I don''t think we want to register absence of the field as a failure, but rather as being equivalent to being set to "false". I feel like this is a more reasonable approach and avoids a lot of servers having to advertise "false" when they can achieve the same by just not advertising support.

I completely agree - I was too eager here and didn't mean to imply that all servers MUST advertise their DPoP stance. I meant it in the same way you described above: "if the MCP server supports DPoP it MUST advertise that support in protected resource metadata (PRM)"

I will take a pass at updating this PR with your feedback, thanks!

This branch has not been deployed

No deployments
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