fix(auth): include RFC 6750 scope attribute in WWW-Authenticate challenges - #3277
fix(auth): include RFC 6750 scope attribute in WWW-Authenticate challenges#3277claude[bot] wants to merge 3 commits into
Conversation
Sorry, something went wrong.
…enges RequireAuthMiddleware built its 401/403 WWW-Authenticate challenges with error/error_description (and optional resource_metadata) but never the scope attribute, even though required_scopes is configured on the middleware instance. Clients therefore could not discover the required scopes from the challenge: the SDK client reads scope from WWW-Authenticate as the highest-priority source both for initial authorization (401) and for SEP-2350 step-up on 403 insufficient_scope, so that path was always empty and fell back to protected resource metadata scopes_supported. Emit scope="<space-delimited required_scopes>" whenever required_scopes is non-empty, per RFC 6750 section 3 (section 3.1 for the insufficient_scope case). Fixes #3103
The interaction and docs_src suites pinned the old scope-less challenge as a recorded divergence (hosting:auth:scope-403 and the scope half of hosting:auth:missing-401 / invalid-401 / expired-401). Now that the middleware emits the RFC 6750 scope attribute, follow the divergence lifecycle: re-pin those tests to the spec-correct output, drop the resolved Divergence records, keep the still-open no-credentials error-code divergence on hosting:auth:missing-401, and refresh the docstrings and docs page that described the old behaviour.
There was a problem hiding this comment.
Beyond the two inline nits, this run confirmed the earlier blocking comment is fully resolved: all four previously-failing exact-equality tests are re-pinned with the correct attribute order (error, error_description, scope, resource_metadata), the hosting:auth:scope-403 / 401 divergence records are trimmed, and the doc literal matches tutorial001.py's required_scopes=["notes:read"]. Also ruled out: header injection via the new scope attribute (the value comes only from server-side required_scopes config, never request data), and a client/server format mismatch (the emitted attribute is exactly what extract_scope_from_www_auth parses at both the 401 scope-selection and 403 step-up sites in oauth2.py).
Extended reasoning...Bugs were found this run (two nits: a stale _FirstChallenge docstring and a private-path import in a test), so no approval verdict is issued — the PR also touches auth middleware, which warrants human sign-off regardless. This note only records what else was examined and ruled out: the prior run's blocking finding about four stale exact-assert tests, the divergence records, and the doc literal is verified addressed in the current diff; the new scope attribute was checked for injection (config-only value) and for consumer-side compatibility (the SDK client reads it verbatim on both the 401 and 403 paths).
Sorry, something went wrong.
…m docstring Use the public 'from mcp.server import Server' in tests/client/test_auth.py, matching __all__ and the sibling test files, and reword the _FirstChallenge docstring which still claimed the bearer middleware cannot emit scope= in its challenge (no longer true since the scope attribute fix).
There was a problem hiding this comment.
I reviewed this PR and found no bugs in the current revision — the findings from my earlier passes (the un-updated pinned tests, the stale _FirstChallenge docstring, and the private Server import) are all confirmed fixed in cd22f04 and 958b492. Because the change alters the wire-visible WWW-Authenticate challenge emitted by the auth middleware for every deployment with required_scopes, a human maintainer look is still worthwhile as a final step.
What was reviewed:
- The new scope branch in _send_auth_error — emitted on both 401 and 403 as the PR argues from RFC 6750 §3/§3.1, and pinned by exact-string tests including the no-scopes branch.
- Header-injection risk from the new interpolation — ruled out: required_scopes is operator config (AuthSettings), never request-derived.
- The claimed client-side consumers (extract_scope_from_www_auth feeding scope selection and SEP-2350 step-up) — verified present in src/mcp/client/auth, so the asymmetry the PR closes is real.
- The divergence-lifecycle bookkeeping in tests/interaction/_requirements.py — the removed records match what the fix resolves, and missing-401 correctly retains the still-open invalid_token-on-no-credentials gap.
Overview
The PR adds one conditional to RequireAuthMiddleware._send_auth_error so that WWW-Authenticate challenges (both the 401 invalid_token and 403 insufficient_scope forms) carry an RFC 6750 scope attribute listing the configured required_scopes. The remaining eight files are test re-pins, divergence-record updates in the interaction suite's requirements manifest, new regression tests (including an end-to-end #3103 step-up test), and a docs refresh of the literal challenge shown in the authorization tutorial.
Security risks
The change is in auth middleware but does not alter any authorization decision — it only adds an informational attribute to error responses. The interpolated values come from AuthSettings.required_scopes (operator configuration), not from request data, so there is no header-injection surface. Advertising required scopes in a challenge is exactly what RFC 6750 and the MCP authorization spec prescribe, and reveals nothing an unauthenticated caller cannot already read from the protected-resource metadata document (scopes_supported mirrors required_scopes).
Level of scrutiny
Moderate-to-high. The logic change is trivial and exhaustively tested (exact-string assertions on all three branches, production wiring through AuthenticationMiddleware + BearerAuthBackend, and an end-to-end test replaying the real server challenge into the SDK client's step-up flow). However, it changes the exact challenge bytes emitted by every scoped deployment, sits on the auth path, and involves a judgment call (emitting scope on the 401 as well as the 403) that a maintainer may want to own. Per my approval criteria, auth-touching changes are not shadow-approved regardless of how clean they look.
Other factors
My earlier review passes found three issues — the initially-missed pinned tests in tests/docs_src and tests/interaction (which broke CI as predicted), a stale harness docstring, and a private-module import — and all were fixed in follow-up commits that I verified in the current head. The bug hunting system found nothing further in this run. Test coverage of the changed module is at 100% line+branch per the PR's stated gates, and the divergence bookkeeping in the interaction suite was updated consistently with its documented lifecycle.
Sorry, something went wrong.
Requested by Felix Weinberger · Slack thread
Fixes #3103
Problem
RequireAuthMiddleware._send_auth_error builds the WWW-Authenticate challenge for both the 401 (invalid_token) and 403 (insufficient_scope) responses with error, error_description, and optionally resource_metadata — but never the scope attribute, even though required_scopes is already configured on the middleware instance.
RFC 6750 §3 defines scope as a Bearer challenge attribute advertising "the scope necessary to access the protected resource", and §3.1 says the insufficient_scope response MAY carry it. The SDK's own client implements the consumer side on both paths — extract_scope_from_www_auth() feeds get_client_metadata_scopes(), where the challenge's scope is the highest-priority source in the MCP scope-selection strategy, both on initial authorization (401) and on SEP-2350 step-up after 403 insufficient_scope. Since the server never emits it, that priority-1 source is always empty and clients fall back to PRM scopes_supported (which may not be advertised). This was also recorded as the hosting:auth:scope-403 divergence in the interaction suite's requirements manifest ("a resource-server/client asymmetry").
Reproduced in-process on main (SDK streamable-HTTP app with required_scopes=["user", "admin"], SDK client helpers):
Fix
_send_auth_error now appends scope="<space-delimited required_scopes>" to the challenge whenever self.required_scopes is non-empty. One conditional, no signature or config changes — the scopes are already on the middleware.
It is emitted on both challenge kinds deliberately: §3.1 explicitly covers the 403 insufficient_scope case, and §3 defines scope as a general challenge attribute, which is what lets a client request the right scopes on its initial authorization after a 401 (the SDK client's Step 3 scope selection reads it exactly there).
The second commit follows the interaction suite's divergence lifecycle (tests/interaction/README.md): the tests that pinned the old scope-less challenge as a recorded divergence are re-pinned to the spec-correct output, the resolved Divergence records are removed (hosting:auth:scope-403, and the scope half of missing-401/invalid-401/expired-401), and the tutorial docs page showing the literal challenge is refreshed. The missing-401 divergence is narrowed, not removed: emitting error="invalid_token" on the no-credentials case (RFC 6750 §3.1 SHOULD NOT) is a separate, still-open gap that this PR deliberately does not touch.
Testing
Relationship to #3130
#3130 (thanks @vishnujayvel, and @velias for the diagnosis) fixes the 403 case on the v1.x maintenance branch and explicitly deferred main. This PR is the main (v2) fix and additionally covers the 401 challenge per the issue's "Expected behavior". The two are complementary, not competing.
AI authorship disclosure
This PR was authored by Claude as part of a maintainer-requested triage workflow, and reviewed gates (tests, lint, types, coverage) were run before submission.
Generated by Claude Code