fix(auth): retain client secrets across credential restoration - #1255
Open
LSGoCards7 wants to merge 1 commit into
Open
fix(auth): retain client secrets across credential restoration#1255LSGoCards7 wants to merge 1 commit into
LSGoCards7 wants to merge 1 commit into
Conversation
github-actions
Bot
added
T-test
DaleSeo
reviewed
Sep 11, 2026
Comment on lines
+1483
to
+1485
| let mut config = OAuthClientConfig::new(&stored.client_id, self.base_url.to_string()); | ||
| config.client_secret = stored.client_secret.map(|secret| secret.secret().clone()); | ||
| self.configure_client(config)?; |
Member
There was a problem hiding this comment.
Was dropping it here intentional, or would threading the secret through that path keep both restore branches in step?
|
|
||
| let stored = StoredCredentials { | ||
| client_id: client_id.to_string(), | ||
| client_secret: None, |
Member
There was a problem hiding this comment.
How is a consumer that uses get_credentials and set_credentials instead of a CredentialStore expected to restore the secret on the next run?
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
2 participants
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.
Fixes #784.
A client registered with a secret can authorize successfully, but restoring its saved credentials reconstructs a client without that secret. Providers requiring client authentication then reject refresh after access-token expiry.
Persist an optional
oauth2::ClientSecretinStoredCredentials, retain it on the authorization manager, and restore it after the existing issuer check. Preserve it during authorization and refresh saves. Add typed accessors for consumers that manage their own credential persistence, including Codex. Keep the existing constructor andget_credentials()signature;StoredCredentialsis already non-exhaustive. Old records deserialize without a secret and public clients remain supported. Debug output redacts the field; empty secrets stay public.Regression coverage serializes credentials between new manager instances and requires both
client_secret_basicandclient_secret_poston two successive refreshes. It also covers public/legacy clients, rotated and omitted replacement refresh tokens, DCR/CIMD issuer changes, serialization, empty secrets, and redaction. Existing refresh locks, client binding, resource handling, and save-before-return behavior remain intact.Validation on base
3e636cab26c013eca5131103c03d20237f12c4df(RMCP 3.3.0):cargo +nightly fmt --all: passed.cargo +1.96.0 test -p rmcp --all-features transport::auth: 216 passed; 357 other library tests filtered out.cargo +1.96.0 clippy -p rmcp --all-targets --all-features -- -D warnings: passed.git diff --check: passed. No dependency or lockfile changes.Related consumer report: openai/codex#43216. An earlier isolated Codex 0.153.4/RMCP 3.1.3 candidate carrying this repair passed Supabase's natural access-token expiry, authenticated refresh with token rotation, an allowlisted read, and another process restart on 2026-09-10. That live result applies to the earlier candidate; this rebased SDK patch has the automated coverage above. No daily-client or released-fix claim is made.