Fall back to the caller-configured scope when the server advertises none - #3172
Fall back to the caller-configured scope when the server advertises none#3172maxisbey wants to merge 3 commits into
Conversation
The OAuth client's scope-selection step overwrote the scope a caller set on the provider on every 401, and when the server advertised no scopes at all it left the token request with no scope. The configured scope is now the last-resort tier after the WWW-Authenticate challenge and the server's scopes_supported, matching the TypeScript SDK. A source that yields no scopes (absent, null, or an empty list) now falls through to the next tier instead of pinning an empty scope.
📚 Documentation preview
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/client/auth/utils.py">
<violation number="1" location="src/mcp/client/auth/utils.py:126">
P2: Clients that previously relied on an omitted `scope` will now send their configured value in this fallback case, but this behavior change is not documented in `docs/migration.md`. Adding a migration entry alongside the existing scope-provider guidance would let users identify and adjust deployments that reject or interpret the newly sent parameter differently.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| elif authorization_server_metadata is not None and authorization_server_metadata.scopes_supported: | ||
| selected_scope = " ".join(authorization_server_metadata.scopes_supported) | ||
| else: | ||
| selected_scope = configured_scope or None |
There was a problem hiding this comment.
P2: Clients that previously relied on an omitted scope will now send their configured value in this fallback case, but this behavior change is not documented in docs/migration.md. Adding a migration entry alongside the existing scope-provider guidance would let users identify and adjust deployments that reject or interpret the newly sent parameter differently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/auth/utils.py, line 126:
<comment>Clients that previously relied on an omitted `scope` will now send their configured value in this fallback case, but this behavior change is not documented in `docs/migration.md`. Adding a migration entry alongside the existing scope-provider guidance would let users identify and adjust deployments that reject or interpret the newly sent parameter differently.</comment>
<file context>
@@ -100,21 +100,30 @@ def get_client_metadata_scopes(
+ elif authorization_server_metadata is not None and authorization_server_metadata.scopes_supported:
selected_scope = " ".join(authorization_server_metadata.scopes_supported)
+ else:
+ selected_scope = configured_scope or None
# SEP-2207: append offline_access when the AS supports it and the client can use refresh tokens
</file context>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline nits, one other candidate was examined and ruled out: a quoted-empty WWW-Authenticate scope (scope="") parses via the unquoted regex branch to the literal string "", which was truthy under the old is not None check too — so the new truthy fall-through doesn't change behavior for that input; it's a pre-existing parsing quirk, not a regression from this PR.
Extended reasoning...
Two nit-level findings (metadata-object reuse polluting the configured_scope snapshot, and a missing docs/migration.md entry for the wire-behavior change) are already posted as inline comments. Separately, the quoted-empty WWW-Authenticate scope candidate was verified against src/mcp/client/auth/utils.py:30-35: scope="" matches the unquoted alternative as the literal "", which both the pre-PR is not None check and the new truthy check treat as present — identical outcome, so this PR neither introduces nor worsens it. Not approving because the PR modifies OAuth scope selection in the client auth flow, which is security-sensitive and warrants human review even though the tier-ordering logic is small, matches the TypeScript SDK, and is covered by unit and interaction tests.
…tadata Drop the authorization-server scopes_supported tier from scope selection. That list is the server's catalog rather than what the resource needs, so falling back to it could request every scope the server supports when the protected resource metadata published an empty list. The chain is now WWW-Authenticate scope, then PRM scopes_supported, then the caller-configured scope, then omit; an empty published list falls through instead of pinning an empty scope. AS metadata is still consulted for whether offline_access may be added. The provider now works on a copy of the caller's OAuthClientMetadata, so the flow's scope selection no longer rewrites the caller's model and the configured-scope snapshot cannot pick up another provider's discovered scopes when metadata is reused across providers.
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The migration entry named the wrong v1 trigger for the removed authorization-server fallback: it fired when the protected resource metadata was absent or omitted scopes_supported, not when it published an empty list. Also note that the offline_access append needs a base scope, so flows that previously drew only on the authorization server's list no longer request it. No-Verification-Needed: doc- and comment-only edits
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/migration.md">
<violation number="1" location="docs/migration.md:1999">
P2: Migration guidance incorrectly guarantees no refresh token when `offline_access` is omitted. Servers can still issue refresh tokens without that scope; describe this as losing the request for `offline_access` (and potentially the refresh token) instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| The client selects the scope to request from the `WWW-Authenticate` challenge, then the protected resource's `scopes_supported`, then the scope the caller configured on the provider (`ClientCredentialsOAuthProvider(scope=...)`, `PrivateKeyJWTOAuthProvider(scope=...)`, or `OAuthClientMetadata(scope=...)`), and otherwise omits it. Two v1 behaviours changed: | ||
|
|
||
| * The configured scope is now requested when the server publishes no scopes (its `scopes_supported` absent or empty). v1 silently dropped it and sent the token request scope-less. If a strict authorization server now answers `invalid_scope` for a configured scope it does not allowlist, drop or correct the `scope=` argument. | ||
| * The authorization server's `scopes_supported` no longer supplies the requested scope. When the protected resource metadata was absent or omitted `scopes_supported`, v1 requested that entire list — the server's whole catalog rather than what the resource needs. If you relied on it, configure the scope on the provider explicitly. The list is still consulted to decide whether `offline_access` may be added ([SEP-2207](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207)), but that append needs a base scope: a flow whose only scope source was the authorization server's list now also sends no `offline_access` and so receives no refresh token — configure the scope on the provider to keep requesting one. |
There was a problem hiding this comment.
P2: Migration guidance incorrectly guarantees no refresh token when offline_access is omitted. Servers can still issue refresh tokens without that scope; describe this as losing the request for offline_access (and potentially the refresh token) instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/migration.md, line 1999:
<comment>Migration guidance incorrectly guarantees no refresh token when `offline_access` is omitted. Servers can still issue refresh tokens without that scope; describe this as losing the request for `offline_access` (and potentially the refresh token) instead.</comment>
<file context>
@@ -1995,8 +1995,8 @@ ClientCredentialsOAuthProvider(..., scope="read write")
-* The configured scope is now requested when the server advertises no scopes. v1 silently dropped it and sent the token request scope-less. If a strict authorization server now answers `invalid_scope` for a configured scope it does not allowlist, drop or correct the `scope=` argument.
-* The authorization server's `scopes_supported` no longer supplies the requested scope. v1 fell back to that list, which is the server's whole catalog rather than what the resource needs, so a resource publishing an empty `scopes_supported` could trigger a request for every scope the authorization server supports. If you relied on it, configure the scope on the provider explicitly. The list is still consulted to decide whether `offline_access` may be added ([SEP-2207](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207)).
+* The configured scope is now requested when the server publishes no scopes (its `scopes_supported` absent or empty). v1 silently dropped it and sent the token request scope-less. If a strict authorization server now answers `invalid_scope` for a configured scope it does not allowlist, drop or correct the `scope=` argument.
+* The authorization server's `scopes_supported` no longer supplies the requested scope. When the protected resource metadata was absent or omitted `scopes_supported`, v1 requested that entire list — the server's whole catalog rather than what the resource needs. If you relied on it, configure the scope on the provider explicitly. The list is still consulted to decide whether `offline_access` may be added ([SEP-2207](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207)), but that append needs a base scope: a flow whose only scope source was the authorization server's list now also sends no `offline_access` and so receives no refresh token — configure the scope on the provider to keep requesting one.
### Client rejects authorization server metadata with a mismatched `issuer`
</file context>
| * The authorization server's `scopes_supported` no longer supplies the requested scope. When the protected resource metadata was absent or omitted `scopes_supported`, v1 requested that entire list — the server's whole catalog rather than what the resource needs. If you relied on it, configure the scope on the provider explicitly. The list is still consulted to decide whether `offline_access` may be added ([SEP-2207](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207)), but that append needs a base scope: a flow whose only scope source was the authorization server's list now also sends no `offline_access` and so receives no refresh token — configure the scope on the provider to keep requesting one. | |
| * The authorization server's `scopes_supported` no longer supplies the requested scope. When the protected resource metadata was absent or omitted `scopes_supported`, v1 requested that entire list — the server's whole catalog rather than what the resource needs. If you relied on it, configure the scope on the provider explicitly. The list is still consulted to decide whether `offline_access` may be added ([SEP-2207](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2207)), but that append needs a base scope: a flow whose only scope source was the authorization server's list now also sends no `offline_access`, which can prevent a refresh token from being issued — configure the scope on the provider to keep requesting one. |
There was a problem hiding this comment.
No new issues in this revision — everything my earlier reviews raised is addressed as of aeac39f, including the migration-note and docstring wording my last comment flagged (that comment ran against the previous commit and posted just after the fix landed; it's moot now). Leaving final sign-off to a human since this changes OAuth wire behavior on the client auth path.
What was reviewed this run:
- Tier ordering in
get_client_metadata_scopes(WWW-Authenticate → PRM → configured scope → omit), including the empty-list fall-through and the SEP-2207offline_accessappend no longer sourcing a base scope from the AS catalog. - The
model_copy()+configured_scopesnapshot — the cross-provider stale-scope leak is closed and regression-tested; the copy is shallow but nothing mutates shared sub-objects in place. - Both call sites (401 step 3 and the 403 step-up union) pass the configured scope consistently.
Extended reasoning...
Overview
The PR reorders the OAuth client's scope-selection chain in get_client_metadata_scopes so the caller-configured scope becomes the last-resort tier (replacing the AS scopes_supported catalog fallback), adds an empty-list fall-through via _join_scopes, snapshots configured_scope on OAuthContext, and copies the caller's OAuthClientMetadata at construction so the flow's in-place scope writes no longer mutate the caller's model. It touches oauth2.py, utils.py, the two client-credentials providers' docstrings, docs, and adds unit + interaction tests.
Security risks
This is auth-adjacent client code: it changes which scope string reaches /authorize and /token. The precedence change is scope-narrowing in spirit (advertised scopes still win; the AS catalog can no longer widen a request beyond what the resource or caller asked for), and the configured-scope fallback only fires when the server advertises nothing. No injection or credential-handling surface changes. The main risk class — stale discovered scopes leaking into the fallback tier — was raised in my first review and is closed by the model_copy() + snapshot fix, with a dedicated regression test.
Level of scrutiny
High: this is production OAuth flow logic in the SDK's client auth path, and a wire-behavior change (a previously dropped configured scope is now sent, which strict authorization servers can newly reject with invalid_scope). That behavior change is now documented in docs/migration.md per AGENTS.md, and the divergence from the spec's two-step chain is recorded in the interaction requirements with the TS SDK as precedent. Even with all prior findings addressed and no new bugs found, auth-path logic changes warrant a maintainer's sign-off rather than a shadow approval.
Other factors
Test coverage is solid: unit tests for each tier edge (configured fallback, advertised precedence, empty-list fall-through, no-mutation), an in-process interaction test proving scope=ops reaches the recorded /token body, and the updated offline_access test pinning that the AS catalog no longer supplies a base scope. One reviewer nit remains open (cubic's suggestion to soften the migration note's claim that omitting offline_access means no refresh token is issued) — a doc-prose accuracy point, not a code issue. My 2026-07-27 comment raced the aeac39f push and describes text that no longer exists; this message records that so the thread isn't misread as having unresolved findings.
1 participant
The OAuth client's scope-selection step overwrote the
scopea caller configured on the provider on every 401. When the server advertised no scopes at all (noWWW-Authenticatescope, noscopes_supported), the token request went out with no scope, soClientCredentialsOAuthProvider(scope=...),PrivateKeyJWTOAuthProvider(scope=...), andOAuthClientMetadata(scope=...)were silently dropped on the discovery-driven flow.The configured scope is now the last-resort tier — WWW-Authenticate → PRM
scopes_supported→ ASscopes_supported→ configured scope → omit — which matches the TypeScript SDK'srequestedScope || scopes_supported || clientMetadata.scope. A source that yields no scopes (absent, null, or an empty list) also falls through to the next tier instead of pinning an empty scope, again matching the TS chain. Advertised scopes still win over the configured one; that precedence is unchanged.Follow-up to the observation on #3166 that the just-renamed
scope=parameter never reached the token request on the real 401 flow.Motivation and Context
OAuthContext.configured_scopesnapshots the constructor'sclient_metadata.scope, because scope selection overwritesclient_metadata.scopeon every flow — reading that field back as the fallback would leak stale discovered scopes across flows.How Has This Been Tested?
Unit tests for the tier ordering (configured-scope fallback, advertised precedence, empty-list fall-through), plus an in-process interaction test where
ClientCredentialsOAuthProvider(scope="ops")runs against a server publishing no scopes and the recorded/tokenbody carriesscope=ops. Also driven end to end over loopback HTTP with theoauth_client_credentialsstory against a server variant that advertises no scopes.Breaking Changes
Behavior change, no API change: a configured scope that was previously dropped when the server advertised nothing is now sent.
Types of changes
Checklist