Skip to content

feat(auth): bind DCR client credentials to issuing authorization server (SEP-2352)#998

Merged
DaleSeo merged 2 commits into
mainfrom
alexhancock/sep-2352-as-binding
Jul 18, 2026
Merged

feat(auth): bind DCR client credentials to issuing authorization server (SEP-2352)#998
DaleSeo merged 2 commits into
mainfrom
alexhancock/sep-2352-as-binding

Conversation

@alexhancock

Copy link
Copy Markdown
Contributor

Motivation and Context

Per modelcontextprotocol/modelcontextprotocol#2352 clients SHOULD maintain separate registration state per auth server and MUST NOT assume cross-AS credential validity

How Has This Been Tested?

Conformance tests

cargo build -p mcp-conformance
npx -y @modelcontextprotocol/conformance@0.2.0-alpha.9 client --command ./target/debug/conformance-client --scenario auth/authorization-server-migration -o results

Breaking Changes

None. StoredCredentials is #[non_exhaustive] and has a new constructor

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

Closes #879

@alexhancock
alexhancock requested a review from a team as a code owner July 16, 2026 15:58
@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Jul 16, 2026
token_response: Some(token_result.clone()),
granted_scopes,
token_received_at: Some(Self::now_epoch_secs()),
issuer: self.metadata_issuer(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now stamp issuer: self.metadata_issuer() here, but nothing reads it back. After a migration, what stops a static pre-registered credential from being silently sent to the new AS, the case where SEP-2352 says clients "SHOULD surface an error"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. initialize_from_store now returns an AuthorizationServerMismatch error on issuer change

Comment thread crates/rmcp/src/transport/auth.rs Outdated
Comment thread crates/rmcp/src/transport/auth.rs
Comment thread crates/rmcp/src/transport/auth.rs
Comment thread crates/rmcp/src/transport/auth.rs
@alexhancock
alexhancock force-pushed the alexhancock/sep-2352-as-binding branch 2 times, most recently from 0c9c067 to 7696cf8 Compare July 17, 2026 20:46
Comment thread crates/rmcp/src/transport/auth.rs Outdated
DaleSeo
DaleSeo previously approved these changes Jul 17, 2026
@alexhancock
alexhancock force-pushed the alexhancock/sep-2352-as-binding branch from d051669 to 6dac387 Compare July 18, 2026 00:45
@alexhancock

Copy link
Copy Markdown
Contributor Author

I applied your suggestion @DaleSeo but had to squash to satisfy commitlint. Can you re-stamp? TY!

@DaleSeo
DaleSeo merged commit 8efc142 into main Jul 18, 2026
21 checks passed
@DaleSeo
DaleSeo deleted the alexhancock/sep-2352-as-binding branch July 18, 2026 10:44
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.

Implement SEP-2352: Clarify authorization server binding and migration

2 participants