Delegate authorize-time scope policy to the provider - #3222
Draft
maxisbey wants to merge 2 commits into
Draft
Conversation
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
Contributor
📚 Documentation preview
|
This comment was marked as spam.
This comment was marked as spam.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #2301. Fixes #2216, fixes #977.
Motivation and Context
The SDK-hosted authorization server (
create_auth_routes) rejected any/authorizerequest whose scope wasn't in the client's registeredscopemetadata, and read a registeredscope=Noneas an empty allowlist. Three consequences:scope(the default DCR path whendefault_scopesis unset) was rejected for every scoped request (Bug: validate_scope rejects client scopes when required scopes in None #2216).403 insufficient_scopeclients (this SDK's included, per SEP-2350) re-authorize for prior ∪ challenged scopes without re-registering, which the check rejected as unregistered.scope=parameter, which some clients send, was rejected (Empty but defined scope query param ("&scope=") validates as [""] and not [] during authorization. #977).RFC 7591 §2 makes registered
scopeself-asserted metadata the client "can use", not an allowlist the authorization server must enforce, and RFC 6749 §4.1.2.1'sinvalid_scopemeans "invalid, unknown, or malformed", not "outside this client's registration". The authorize handler now enforces only the server-wideClientRegistrationOptions.valid_scopes(the list already advertised asscopes_supportedand enforced at registration); per-client scope policy belongs inOAuthAuthorizationServerProvider.authorize()viaAuthorizeError(error="invalid_scope"). An emptyscopeparameter is treated as omitted.How Has This Been Tested?
./scripts/test(full suite, 100% coverage). New interaction requirementhosting: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 outsidevalid_scopesrejected withinvalid_scope, emptyscope=accepted, and a provider-raisedinvalid_scopereaching the redirect.Breaking Changes
OAuthClientInformationFull.validate_scope()andmcp.shared.auth.InvalidScopeErrorare removed; both existed only to implement the deleted check.docs/migration.mdshows the provider-side replacement.Types of changes
Checklist
Additional context
Design note versus #2301: rather than deleting scope enforcement at
/authorizeoutright, 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-scopeallowlist 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=Nonecase on #2216.AI Disclaimer