feat(auth): bind stored credentials to authorization server issuer (SEP-2352)#965
Draft
alexhancock wants to merge 1 commit into
Draft
feat(auth): bind stored credentials to authorization server issuer (SEP-2352)#965alexhancock wants to merge 1 commit into
alexhancock wants to merge 1 commit into
Conversation
…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
8f8833f to
752c82f
Compare
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
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.
Summary
Implements the authorization-server binding rule from SEP-2352 (
Clarify authorization server binding and migration). OAuth credentials are now bound to theissuerof the authorization server (AS) that minted them, andget_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 becomesNone).StoredCredentials::new()signature is unchanged (still setsissuer: None); added awith_issuer()builder.Debugimpl updated to include the issuer (no secrets exposed).AuthorizationManager::stored_credentials()— new helper that stamps the currently-configured issuer (from discoveredAuthorizationMetadata) onto persisted credentials. Wired into every token-save site: authorization-code exchange, refresh, client-credentials, client-credentials-JWT, andOAuthState::set_credentials(which now discovers metadata before persisting so the issuer is available).AuthorizationManager::check_issuer_binding()— guard invoked at the top ofget_access_token(). On a known-issuer mismatch it returns the newAuthError::AuthorizationServerMismatch { stored, active }.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.Backward compatibility
Non-breaking:
issuer == Noneare treated as unbound and pass through unchanged.StoredCredentialsandAuthErrorare both#[non_exhaustive], and the publicnew()signature is preserved.Tests
Added 9 tests:
AuthorizationServerMismatchNone) pass throughstored_credentials()stamps the active issuerwith_issuer()builder sets the issuercargo build,cargo test,cargo clippy, andcargo fmtall pass (featureauth); 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.