Skip to content

Delegate authorize-time scope policy to the provider - #3222

Draft
maxisbey wants to merge 2 commits into
mainfrom
fix/authorize-scope-policy
Draft

Delegate authorize-time scope policy to the provider#3222
maxisbey wants to merge 2 commits into
mainfrom
fix/authorize-scope-policy

Conversation

@maxisbey

Copy link
Copy Markdown
Contributor

Supersedes #2301. Fixes #2216, fixes #977.

Motivation and Context

The SDK-hosted authorization server (create_auth_routes) rejected any /authorize request whose scope wasn't in the client's registered scope metadata, and read a registered scope=None as an empty allowlist. Three consequences:

RFC 7591 §2 makes registered scope self-asserted metadata the client "can use", not an allowlist the authorization server must enforce, and RFC 6749 §4.1.2.1's invalid_scope means "invalid, unknown, or malformed", not "outside this client's registration". The authorize handler now enforces only the server-wide ClientRegistrationOptions.valid_scopes (the list already advertised as scopes_supported and enforced at registration); per-client scope policy belongs in OAuthAuthorizationServerProvider.authorize() via AuthorizeError(error="invalid_scope"). An empty scope parameter is treated as omitted.

How Has This Been Tested?

./scripts/test (full suite, 100% coverage). New interaction requirement hosting:auth:as:authorize-scope; the two step-up interaction tests now register the client with only the narrow scope, so they prove the wider re-authorization is accepted end to end. Also driven manually over HTTP against a mounted authorization server: step-up-shaped request accepted, scope outside valid_scopes rejected with invalid_scope, empty scope= accepted, and a provider-raised invalid_scope reaching the redirect.

Breaking Changes

OAuthClientInformationFull.validate_scope() and mcp.shared.auth.InvalidScopeError are removed; both existed only to implement the deleted check. docs/migration.md shows the provider-side replacement.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Design note versus #2301: rather than deleting scope enforcement at /authorize outright, this keeps the server-wide ceiling (valid_scopes) applied to every client, which preserves the transitive registration→authorization ceiling for DCR deployments; only the per-client registered-scope allowlist is gone. The TypeScript SDK removed its equivalent check in modelcontextprotocol/typescript-sdk#983.

Thanks to @nik1097 for the report and to everyone who opened fixes for the scope=None case on #2216.

AI Disclaimer

maxisbey added 2 commits July 29, 2026 15:39
The SDK-hosted authorization server rejected any /authorize request
whose scope was not in the client's registered `scope` metadata, and
treated a client registered without a scope as allowed to request
nothing at all. That broke the spec's step-up flow, in which a client
answers a 403 insufficient_scope challenge by re-authorizing for the
union of its previous and challenged scopes without re-registering, and
it rejected every scoped request from scope-less registrations (#2216)
as well as an empty `scope=` parameter (#977).

The authorize handler now enforces only the server-wide scope set,
`ClientRegistrationOptions.valid_scopes` (the list already advertised
as `scopes_supported` and enforced at registration). A requested scope
outside it redirects back with `invalid_scope`; when it is unset, every
requested scope reaches `provider.authorize()`, which can reject a
client's request by raising `AuthorizeError(error="invalid_scope")`.
An empty scope parameter is treated as omitted.

`OAuthClientInformationFull.validate_scope()` and `InvalidScopeError`
are removed; both existed only to implement the deleted check.

Github-Issue: #2216
Spell out on OAuthAuthorizationServerProvider.authorize() that the
requested scopes have only been checked against the server-wide
valid_scopes and that per-client scope policy is enforced here, so the
contract lives on the interface operators implement rather than only
in the migration guide.

No-Verification-Needed: docstring-only change
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation preview

Preview https://pr-3222.mcp-python-docs.pages.dev
Deployment https://e10118bc.mcp-python-docs.pages.dev
Commit b863e4c
Triggered by @maxisbey
Updated 2026-07-30 14:58:55 UTC

@keeltrace

Copy link
Copy Markdown

AI-assisted analysis; posting under human responsibility.

I think the new scope_tokens() parser is more permissive than RFC 6749 and can turn a malformed scope value into a valid-looking authorization request.

This PR adds:

def scope_tokens(scope: str | None) -> list[str] | None:
    tokens = scope.split() if scope else []
    return tokens or None

The no-argument form of Python str.split() treats all Unicode whitespace as separators and also collapses repeated whitespace. RFC 6749 §3.3 / Appendix A.4 is narrower:

scope       = scope-token *( SP scope-token )
scope-token = 1*NQCHAR

SP there is the ASCII space character. Tabs/newlines are not scope separators, and §4.1.2.1 defines invalid_scope to include a malformed requested scope.

So, for example, an authorization request containing:

scope=read%09admin

arrives as "read\tadmin". AuthorizationRequest.scope is currently an unconstrained str, so model validation accepts it; then scope_tokens() converts it to:

["read", "admin"]

If both tokens are in valid_scopes, the server accepts the request and passes those two scopes to the provider even though the wire scope value does not match the OAuth grammar.

The same normalization happens for repeated/leading/trailing whitespace, e.g. "read admin" becomes ["read", "admin"] rather than remaining detectably malformed.

This looks introduced by the refactor: the removed validate_scope() used split(" "), which did not reinterpret tabs/newlines as separators. I agree with the PR's main policy change (registered client scope should not be treated as a permanent allowlist); the concern is specifically the new syntax parser at that boundary.

Could scope_tokens() validate the RFC grammar before splitting — i.e. require only scope-token characters separated by literal single ASCII spaces — and return/raise an invalid_scope path for malformed values? A focused HTTP regression test for at least:

scope=read%09admin

with valid_scopes=["read", "admin"] would pin the distinction: it should be rejected rather than authorized as two scopes.

Checked against PR head b863e4c. This is a static standards/code-path review; I did not run the repository suite locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants