Skip to content

fix(auth): merge configured and server-required OAuth scopes - #1102

Draft
tsarlandie-oai wants to merge 1 commit into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-configured-scope-union
Draft

fix(auth): merge configured and server-required OAuth scopes#1102
tsarlandie-oai wants to merge 1 commit into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-configured-scope-union

Conversation

@tsarlandie-oai

Copy link
Copy Markdown
Contributor

Motivation and Context

Reactive OAuth discovery records scopes required by the server's
WWW-Authenticate challenge and validated protected-resource metadata. However,
AuthorizationSession::new bypasses the existing scope-selection policy whenever
AuthorizationRequest::with_scopes supplies explicit client scopes:

if request.scopes.is_empty() {
    request.scopes = auth_manager.select_scopes(None, &[]);
} else {
    auth_manager.add_offline_access_if_supported(&mut request.scopes);
}

As a result, a client configured with configured shared does not request
challenge or resource when the Bearer challenge and protected-resource
metadata require them. Duplicate configured scopes are also preserved, and
authorization can fail even though the SDK discovered the scopes necessary for
the operation.

What changed

  • Route both configured and implicit scopes through the existing stable,
    deduplicating scope-selection policy.
  • Seed that policy with explicitly requested scopes before accumulating
    previously requested scopes, Bearer challenge scopes, and protected-resource
    scopes.
  • Keep authorization-server scopes_supported as a fallback only when no
    caller-requested or operationally required scopes exist.
  • Preserve the existing offline_access policy and apply the same resulting
    scope set to preregistered clients, dynamic registration, and the authorization
    URL.
  • Update public documentation to explain that configured scopes are combined
    with server-required scopes.

The public select_scopes API is unchanged. Discovery remains reactive,
consistent with #1052: this change adds no synthetic POST, ping, initialize
request, or additional discovery endpoint.

Regression coverage

Two end-to-end authorization tests cover preregistered and dynamically
registered clients with:

configured scopes:           configured shared configured
Bearer challenge scopes:     challenge shared
protected-resource scopes:   resource shared
authorization-server scopes: fallback-only offline_access

requested scopes:            configured shared challenge resource offline_access

The tests verify stable ordering, deduplication, exclusion of unrelated
authorization-server scopes, preserved offline_access, reactive GET-only
metadata discovery for preregistered clients, and identical scopes in the
dynamic-registration request and authorization URL.

Validation

  • cargo test -p rmcp --features auth --lib transport::auth::tests — 173 OAuth
    tests passed.
  • cargo test -p rmcp --features auth,client,transport-io — full package tests
    and doctests passed.
  • cargo clippy -p rmcp --features auth,client,transport-io --lib -- -D warnings.
  • rustfmt --check --edition 2024 crates/rmcp/src/transport/auth.rs.
  • git diff --check.

Breaking Changes

None. Existing public APIs, registration priority, metadata fallback, and
reactive discovery behavior are preserved.

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

Labels

T-core Core library changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant