Skip to content

feat(auth): bind stored credentials to authorization server issuer (SEP-2352)#965

Draft
alexhancock wants to merge 1 commit into
mainfrom
sep-2352-as-binding
Draft

feat(auth): bind stored credentials to authorization server issuer (SEP-2352)#965
alexhancock wants to merge 1 commit into
mainfrom
sep-2352-as-binding

Conversation

@alexhancock

@alexhancock alexhancock commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the authorization-server binding rule from SEP-2352 (Clarify authorization server binding and migration). OAuth credentials are now bound to the issuer of the authorization server (AS) that minted them, and get_access_token() refuses to hand out a token when the manager is configured against a different known issuer. This prevents credentials obtained from one AS from being silently reused against another, which SEP-2352 forbids.

Refs #879.

What changed

All changes are confined to crates/rmcp/src/transport/auth.rs.

  • StoredCredentials.issuer: Option<String> — new field recording the AS issuer the credentials were minted for.
    • #[serde(default)] so credentials persisted before this change still deserialize (issuer becomes None).
    • StoredCredentials::new() signature is unchanged (still sets issuer: None); added a with_issuer() builder.
    • Debug impl updated to include the issuer (no secrets exposed).
  • AuthorizationManager::stored_credentials() — new helper that stamps the currently-configured issuer (from discovered AuthorizationMetadata) onto persisted credentials. Wired into every token-save site: authorization-code exchange, refresh, client-credentials, client-credentials-JWT, and OAuthState::set_credentials (which now discovers metadata before persisting so the issuer is available).
  • AuthorizationManager::check_issuer_binding() — guard invoked at the top of get_access_token(). On a known-issuer mismatch it returns the new AuthError::AuthorizationServerMismatch { stored, active }.
  • Re-register on mismatch (SEP-2352: MUST NOT reuse / MUST re-register) — on a detected mismatch, get_access_token() clears the stale credentials before returning the error, so the next authorization flow re-registers with the new AS rather than reusing credentials invalid there.
  • CIMD portability carve-out — CIMD client_ids (HTTPS URLs) are portable across authorization servers, so they are exempt from the binding check and never trigger a mismatch.

Backward compatibility

Non-breaking:

  • Legacy credentials with issuer == None are treated as unbound and pass through unchanged.
  • If the active manager has no issuer in its metadata, no mismatch can be proven, so bound credentials still work.
  • StoredCredentials and AuthError are both #[non_exhaustive], and the public new() signature is preserved.

Tests

Added 9 tests:

  • mismatch is rejected with AuthorizationServerMismatch
  • stale credentials are cleared on mismatch (re-registration path)
  • portable CIMD client_id (HTTPS URL) passes through despite differing issuer
  • matching issuer passes through
  • legacy credentials (issuer None) pass through
  • unknown active issuer passes through
  • stored_credentials() stamps the active issuer
  • no issuer stamped when metadata is absent
  • with_issuer() builder sets the issuer

cargo build, cargo test, cargo clippy, and cargo fmt all pass (feature auth); 90 auth tests green.

Scope / notes

Focused, self-contained slice of the SEP-2352 work: issuer binding, mismatch enforcement + re-registration on the token-use path, and the CIMD portability carve-out. The remaining broader work — per-AS keying of custom credential/state stores so multiple AS credentials can coexist (rather than a single active set) — is intentionally left for follow-up. Marked as draft for early review.

@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Jul 8, 2026
…EP-2352)

Implements the authorization-server binding rule from SEP-2352: OAuth
credentials are now stamped with the issuer of the authorization server
that minted them, and get_access_token refuses to hand out a token when
the manager is configured against a different known issuer.

- Add StoredCredentials.issuer (Option<String>, #[serde(default)]) plus a
  with_issuer() builder; keep StoredCredentials::new() signature stable.
- Populate the issuer from AuthorizationMetadata at all token-persistence
  sites via a new AuthorizationManager::stored_credentials() helper
  (auth code, refresh, client-credentials, client-credentials-JWT, and
  OAuthState::set_credentials).
- Guard get_access_token() with check_issuer_binding(); on mismatch return
  the new AuthError::AuthorizationServerMismatch { stored, active }.
- Legacy credentials (issuer == None) and an unknown active issuer are
  treated as unbound and pass through, preserving backward compatibility.
- Add tests covering mismatch rejection, matching pass-through, legacy
  credentials, unknown active issuer, and issuer stamping.

Refs #879
@alexhancock alexhancock force-pushed the sep-2352-as-binding branch from 8f8833f to 752c82f Compare July 9, 2026 13:39
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