fix(auth): redact secrets in Debug output for StoredCredentials and StoredAuthorizationState (#744)
* fix(auth): redact secrets in Debug output for StoredCredentials and StoredAuthorizationState
Removes `Debug` from the derive macros on `StoredCredentials` and
`StoredAuthorizationState` and replaces them with manual `Debug` impls
that print `[REDACTED]` for sensitive fields (access/refresh tokens,
PKCE verifiers, and CSRF tokens), preventing accidental credential
leakage via `{:?}` formatters, log calls, and error chains.
Fixes #741
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(auth): assert Debug output redacts secrets for credential types
Adds regression tests for the fix in the previous commit, verifying
that `{:?}` formatting of `StoredAuthorizationState` and
`StoredCredentials` does not emit plaintext secrets.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(auth): address review feedback on debug redaction tests
- Remove redundant VendorExtraTokenFields from use super:: in
test_stored_credentials_debug_redacts_token_response (already
imported at module scope)
- Add assert!(debug_output.contains("created_at")) to
test_stored_authorization_state_debug_redacts_secrets to verify
non-secret fields remain visible in Debug output
- Run cargo fmt
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Update crates/rmcp/src/transport/auth.rs
Co-authored-by: Dale Seo <5466341+DaleSeo@users.noreply.github.com>
* fix: remaining formatting issue
* fix: formatting
* fix: formatting
* fix: please
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Dale Seo <5466341+DaleSeo@users.noreply.github.com>
This commit is contained in:
parent
1a4a52a173
commit
66712db808
1 changed files with 64 additions and 2 deletions
|
|
@ -59,7 +59,7 @@ impl<'c> AsyncHttpClient<'c> for OAuthReqwestClient {
|
|||
const DEFAULT_EXCHANGE_URL: &str = "http://localhost";
|
||||
|
||||
/// Stored credentials for OAuth2 authorization
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Clone, Serialize, Deserialize)]
|
||||
pub struct StoredCredentials {
|
||||
pub client_id: String,
|
||||
pub token_response: Option<OAuthTokenResponse>,
|
||||
|
|
@ -69,6 +69,20 @@ pub struct StoredCredentials {
|
|||
pub token_received_at: Option<u64>,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for StoredCredentials {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("StoredCredentials")
|
||||
.field("client_id", &self.client_id)
|
||||
.field(
|
||||
"token_response",
|
||||
&self.token_response.as_ref().map(|_| "[REDACTED]"),
|
||||
)
|
||||
.field("granted_scopes", &self.granted_scopes)
|
||||
.field("token_received_at", &self.token_received_at)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
/// Trait for storing and retrieving OAuth2 credentials
|
||||
///
|
||||
/// Implementations of this trait can provide custom storage backends
|
||||
|
|
@ -119,13 +133,23 @@ impl CredentialStore for InMemoryCredentialStore {
|
|||
}
|
||||
|
||||
/// Stored authorization state for OAuth2 PKCE flow
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Clone, Serialize, Deserialize)]
|
||||
pub struct StoredAuthorizationState {
|
||||
pub pkce_verifier: String,
|
||||
pub csrf_token: String,
|
||||
pub created_at: u64,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for StoredAuthorizationState {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("StoredAuthorizationState")
|
||||
.field("pkce_verifier", &"[REDACTED]")
|
||||
.field("csrf_token", &"[REDACTED]")
|
||||
.field("created_at", &self.created_at)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
/// A transparent wrapper around a JSON object that captures any extra fields returned by the
|
||||
/// authorization server during token exchange that are not part of the standard OAuth 2.0 token
|
||||
/// response.
|
||||
|
|
@ -2776,6 +2800,44 @@ mod tests {
|
|||
assert_eq!(deserialized.csrf_token, "my-csrf");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stored_authorization_state_debug_redacts_secrets() {
|
||||
let pkce = PkceCodeVerifier::new("super-secret-verifier".to_string());
|
||||
let csrf = CsrfToken::new("super-secret-csrf".to_string());
|
||||
let state = StoredAuthorizationState::new(&pkce, &csrf);
|
||||
let debug_output = format!("{:?}", state);
|
||||
|
||||
assert!(!debug_output.contains("super-secret-verifier"));
|
||||
assert!(!debug_output.contains("super-secret-csrf"));
|
||||
assert!(debug_output.contains("[REDACTED]"));
|
||||
assert!(debug_output.contains("created_at"));
|
||||
assert!(debug_output.contains("created_at"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stored_credentials_debug_redacts_token_response() {
|
||||
use oauth2::{AccessToken, basic::BasicTokenType};
|
||||
|
||||
use super::{OAuthTokenResponse, StoredCredentials};
|
||||
|
||||
let token_response = OAuthTokenResponse::new(
|
||||
AccessToken::new("super-secret-access-token".to_string()),
|
||||
BasicTokenType::Bearer,
|
||||
VendorExtraTokenFields::default(),
|
||||
);
|
||||
let creds = StoredCredentials {
|
||||
client_id: "my-client".to_string(),
|
||||
token_response: Some(token_response),
|
||||
granted_scopes: vec![],
|
||||
token_received_at: None,
|
||||
};
|
||||
let debug_output = format!("{:?}", creds);
|
||||
|
||||
assert!(!debug_output.contains("super-secret-access-token"));
|
||||
assert!(debug_output.contains("[REDACTED]"));
|
||||
assert!(debug_output.contains("my-client"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stored_authorization_state_into_pkce_verifier() {
|
||||
let pkce = PkceCodeVerifier::new("original-verifier".to_string());
|
||||
|
|
|
|||
Loading…
Reference in a new issue