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
…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).
…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
scopebranch 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_scopesis operator config (AuthSettings), never request-derived. - The claimed client-side consumers (
extract_scope_from_www_authfeeding scope selection and SEP-2350 step-up) — verified present insrc/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, andmissing-401correctly retains the still-open invalid_token-on-no-credentials gap.
Extended reasoning...
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.
Requested by Felix Weinberger · Slack thread
Fixes #3103
Problem
RequireAuthMiddleware._send_auth_errorbuilds theWWW-Authenticatechallenge for both the 401 (invalid_token) and 403 (insufficient_scope) responses witherror,error_description, and optionallyresource_metadata— but never thescopeattribute, even thoughrequired_scopesis already configured on the middleware instance.RFC 6750 §3 defines
scopeas a Bearer challenge attribute advertising "the scope necessary to access the protected resource", and §3.1 says theinsufficient_scoperesponse MAY carry it. The SDK's own client implements the consumer side on both paths —extract_scope_from_www_auth()feedsget_client_metadata_scopes(), where the challenge'sscopeis the highest-priority source in the MCP scope-selection strategy, both on initial authorization (401) and on SEP-2350 step-up after403 insufficient_scope. Since the server never emits it, that priority-1 source is always empty and clients fall back to PRMscopes_supported(which may not be advertised). This was also recorded as thehosting:auth:scope-403divergence in the interaction suite's requirements manifest ("a resource-server/client asymmetry").Reproduced in-process on
main(SDK streamable-HTTP app withrequired_scopes=["user", "admin"], SDK client helpers):Fix
_send_auth_errornow appendsscope="<space-delimited required_scopes>"to the challenge wheneverself.required_scopesis 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_scopecase, and §3 definesscopeas 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 resolvedDivergencerecords are removed (hosting:auth:scope-403, and the scope half ofmissing-401/invalid-401/expired-401), and the tutorial docs page showing the literal challenge is refreshed. Themissing-401divergence is narrowed, not removed: emittingerror="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
test_insufficient_scope_challenge_advertises_required_scopes— production wiring (AuthenticationMiddleware+BearerAuthBackend+RequireAuthMiddleware) over an in-process ASGI transport; asserts the exact 403 challenge value.test_unauthenticated_challenge_advertises_required_scopes— exact 401 challenge value with scopes configured.test_challenge_omits_scope_when_no_scopes_configured— pins the no-scopes branch (noscopeattribute emitted).test_403_step_up_consumes_scope_emitted_by_require_auth_middleware— end-to-end regression for RequireAuthMiddleware omits RFC 6750 scope in WWW-Authenticate on 401/403 #3103: an SDK server app requiringread adminrejects aread-scoped token; the server-produced challenge is replayed into the SDK client's auth flow, and the re-authorization request carries the SEP-2350 unionread adminwith no PRM fallback involved.tests/interaction/auth/test_bearer.py(401 snapshot/dict and the 403 test, renamed..._with_a_scope_param),tests/docs_src/test_authorization.py, plus stale docstrings intest_authorize_token.py/_harness.py.ruff format/ruff checkclean,pyright0 errors, full suite 5584 passed / 8 skipped / 1 xfailed (one deselected locally:test_sse_client_closes_all_streams_on_connection_error, IPv6-less sandbox — unrelated, fails identically on cleanmainthere), changed module at 100% line+branch coverage, no new pragmas.Relationship to #3130
#3130 (thanks @vishnujayvel, and @velias for the diagnosis) fixes the 403 case on the
v1.xmaintenance branch and explicitly deferredmain. This PR is themain(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