From e982f6209160f77d4b31733d8604f15a75f761c6 Mon Sep 17 00:00:00 2001 From: no-hup <19599684+no-hup@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:11:01 +0530 Subject: [PATCH 1/2] fix(auth): omit scope on refresh unless caller sets one Fixes #1408. --- crates/rmcp/src/transport/auth.rs | 95 ++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index 3d7816b54..a5e871960 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -1266,6 +1266,8 @@ pub struct AuthorizationManager { application_type: Option, allow_missing_issuer: bool, allow_private_metadata_addresses: bool, + /// Scopes the caller asked to send on refresh. `None` omits the parameter. + refresh_scopes: Option>, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1578,6 +1580,7 @@ impl AuthorizationManager { application_type: Some(DEFAULT_APPLICATION_TYPE.to_string()), allow_missing_issuer: false, allow_private_metadata_addresses: false, + refresh_scopes: None, }; Ok(manager) @@ -1588,6 +1591,19 @@ impl AuthorizationManager { self.scope_upgrade_config = config; } + /// Set the scopes to include on a refresh-token request. + /// + /// RFC 6749 §6 makes `scope` optional. Refresh omits it unless the caller + /// sets one here. A scope that was not part of the original grant is not + /// sent. An empty list omits the parameter again. + pub fn set_refresh_scopes(&mut self, scopes: I) + where + I: IntoIterator, + S: Into, + { + self.refresh_scopes = Some(scopes.into_iter().map(Into::into).collect()); + } + /// Configure whether authorization server metadata discovery tolerates a /// missing `issuer` field. /// @@ -2123,6 +2139,28 @@ impl AuthorizationManager { Self::dedup_scopes(scopes) } + /// Scopes to put on a refresh request. + /// + /// `None` or an empty list omits the parameter. Otherwise only scopes from + /// the original grant are kept. When that grant was not recorded, the + /// caller-provided scopes are used as-is. + fn refresh_request_scopes(explicit: Option<&[String]>, granted: &[String]) -> Vec { + let Some(explicit) = explicit.filter(|scopes| !scopes.is_empty()) else { + return Vec::new(); + }; + if granted.is_empty() { + return Self::dedup_scopes(explicit.to_vec()); + } + let granted_set: HashSet<&str> = granted.iter().map(String::as_str).collect(); + Self::dedup_scopes( + explicit + .iter() + .filter(|scope| granted_set.contains(scope.as_str())) + .cloned() + .collect(), + ) + } + /// deduplicate scopes preserving first-seen order (SEP-2350: stable for testability) fn dedup_scopes(scopes: Vec) -> Vec { let mut seen = std::collections::HashSet::new(); @@ -2561,10 +2599,21 @@ impl AuthorizationManager { .exchange_refresh_token(&refresh_token_value) // RFC 8707: the resource indicator is required on token requests, including refreshes .add_extra_param("resource", self.oauth_resource().await); - let requested_scopes = stored_credentials.granted_scopes.clone(); - for scope in requested_scopes.iter().cloned() { + // RFC 6749 §6: `scope` is optional on refresh and must not exceed the + // original grant. Omit it unless the caller set scopes explicitly. + let granted_scopes = stored_credentials.granted_scopes.clone(); + let scopes_to_send = + Self::refresh_request_scopes(self.refresh_scopes.as_deref(), &granted_scopes); + for scope in scopes_to_send.iter().cloned() { refresh_request = refresh_request.add_scope(Scope::new(scope)); } + // Omitting `scope` asks for the original grant. Keep that as the + // fallback when the token response also omits `scope` (RFC 6749 §5.1). + let requested_scopes = if scopes_to_send.is_empty() { + granted_scopes + } else { + scopes_to_send + }; let mut token_result = match refresh_request .request_async(&OAuth2HttpClient { client: self.http_client.as_ref(), @@ -10015,6 +10064,7 @@ mod tests { })) .await; manager.configure_client(test_client_config()).unwrap(); + manager.set_refresh_scopes(["read", "write"]); let stored = StoredCredentials { client_id: "my-client".to_string(), @@ -10091,6 +10141,7 @@ mod tests { })) .await; manager.configure_client(test_client_config()).unwrap(); + manager.set_refresh_scopes(["read"]); let stored = StoredCredentials { client_id: "my-client".to_string(), @@ -10130,6 +10181,7 @@ mod tests { })) .await; manager.configure_client(test_client_config()).unwrap(); + manager.set_refresh_scopes(["read", "offline_access"]); let stored = StoredCredentials { client_id: "my-client".to_string(), @@ -10193,6 +10245,45 @@ mod tests { ); } + #[tokio::test] + async fn refresh_token_omits_scope_unless_caller_sets_one() { + let (base_url, captured) = start_token_server().await; + + let mut manager = manager_with_metadata(Some(AuthorizationMetadata { + authorization_endpoint: format!("{}/authorize", base_url), + token_endpoint: format!("{}/token", base_url), + ..Default::default() + })) + .await; + manager.configure_client(test_client_config()).unwrap(); + + // The token response recorded these scopes. That must not put `scope` + // on the refresh by itself: Salesforce rejects the parameter, and + // RFC 6749 §6 says omitting it keeps the original grant. + let stored = StoredCredentials { + client_id: "my-client".to_string(), + token_response: Some(make_token_response_with_refresh( + "old-token", + "my-refresh-token", + )), + granted_scopes: vec!["mcp_api".to_string(), "refresh_token".to_string()], + token_received_at: Some(AuthorizationManager::now_epoch_secs()), + issuer: None, + }; + manager.credential_store.save(stored).await.unwrap(); + + manager.refresh_token().await.unwrap(); + + let body = captured.lock().unwrap().take().unwrap(); + let params: std::collections::HashMap<_, _> = url::form_urlencoded::parse(body.as_bytes()) + .into_owned() + .collect(); + assert!( + !params.contains_key("scope"), + "scope should be absent when the caller did not set one, body: {body}" + ); + } + #[tokio::test] async fn refresh_token_preserves_existing_refresh_token_when_response_omits_it() { use oauth2::TokenResponse; From f2df6675135dc0e8b8d30284d82abd7e16274247 Mon Sep 17 00:00:00 2001 From: no-hup <19599684+no-hup@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:14:43 +0530 Subject: [PATCH 2/2] fix(auth): keep original grant on narrowed refresh, update refresh docs --- crates/rmcp/src/transport/auth.rs | 56 ++++++++++++++++++++++++++++++- docs/OAUTH_SUPPORT.md | 2 +- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/rmcp/src/transport/auth.rs b/crates/rmcp/src/transport/auth.rs index a5e871960..815d8c506 100644 --- a/crates/rmcp/src/transport/auth.rs +++ b/crates/rmcp/src/transport/auth.rs @@ -2602,6 +2602,7 @@ impl AuthorizationManager { // RFC 6749 §6: `scope` is optional on refresh and must not exceed the // original grant. Omit it unless the caller set scopes explicitly. let granted_scopes = stored_credentials.granted_scopes.clone(); + let original_grant = granted_scopes.clone(); let scopes_to_send = Self::refresh_request_scopes(self.refresh_scopes.as_deref(), &granted_scopes); for scope in scopes_to_send.iter().cloned() { @@ -2659,11 +2660,19 @@ impl AuthorizationManager { *self.current_scopes.write().await = granted_scopes.clone(); + // A caller-narrowed refresh must not shrink the stored grant, or later + // refreshes could never ask for the dropped scopes again. + let stored_grant = if original_grant.is_empty() || self.refresh_scopes.is_none() { + granted_scopes + } else { + original_grant + }; + let client_id = oauth_client.client_id().to_string(); let stored = StoredCredentials { client_id, token_response: Some(token_result.clone()), - granted_scopes, + granted_scopes: stored_grant, token_received_at: Some(Self::now_epoch_secs()), issuer: self.metadata_issuer(), }; @@ -10284,6 +10293,51 @@ mod tests { ); } + #[tokio::test] + async fn narrowed_refresh_keeps_original_grant() { + let (base_url, captured) = start_token_server().await; + + let mut manager = manager_with_metadata(Some(AuthorizationMetadata { + authorization_endpoint: format!("{}/authorize", base_url), + token_endpoint: format!("{}/token", base_url), + ..Default::default() + })) + .await; + manager.configure_client(test_client_config()).unwrap(); + manager.set_refresh_scopes(["read"]); + + let stored = StoredCredentials { + client_id: "my-client".to_string(), + token_response: Some(make_token_response_with_refresh( + "old-token", + "my-refresh-token", + )), + granted_scopes: vec!["read".to_string(), "write".to_string()], + token_received_at: Some(AuthorizationManager::now_epoch_secs()), + issuer: None, + }; + manager.credential_store.save(stored).await.unwrap(); + + manager.refresh_token().await.unwrap(); + captured.lock().unwrap().take(); + + let saved = manager.credential_store.load().await.unwrap().unwrap(); + assert_eq!( + saved.granted_scopes, + vec!["read".to_string(), "write".to_string()], + "a narrowed refresh must not shrink the stored grant" + ); + + // A later refresh can still ask for the full grant. + manager.set_refresh_scopes(["read", "write"]); + manager.refresh_token().await.unwrap(); + let body = captured.lock().unwrap().take().unwrap(); + let params: std::collections::HashMap<_, _> = url::form_urlencoded::parse(body.as_bytes()) + .into_owned() + .collect(); + assert_eq!(params.get("scope").map(String::as_str), Some("read write")); + } + #[tokio::test] async fn refresh_token_preserves_existing_refresh_token_when_response_omits_it() { use oauth2::TokenResponse; diff --git a/docs/OAUTH_SUPPORT.md b/docs/OAUTH_SUPPORT.md index 816e97908..e3b2c3252 100644 --- a/docs/OAUTH_SUPPORT.md +++ b/docs/OAUTH_SUPPORT.md @@ -452,7 +452,7 @@ cargo run -p mcp-client-examples --example clients_client_credentials -- \ 6. **Authorization Request**: Build authorization URL with PKCE (S256) and RFC 8707 resource parameter 7. **Authorization Code Exchange**: After user authorization, exchange code for access token (with resource parameter) 8. **Token Usage**: Use access token for API calls via `AuthClient` or `AuthorizedHttpClient` -9. **Token Refresh**: Automatically use refresh token to get new access token when current one expires; previously granted scopes are forwarded in the refresh request so providers that require them (e.g. Azure AD v2) work correctly +9. **Token Refresh**: Automatically use refresh token to get new access token when current one expires; `scope` is omitted by default, which keeps the original grant (RFC 6749 §6). Providers that require it (e.g. Azure AD v2) can opt in with `AuthorizationManager::set_refresh_scopes`; only scopes from the original grant are sent, and a narrowed refresh does not shrink the stored grant 10. **Scope Upgrade**: On 403 insufficient_scope, compute scope union and re-authorize with upgraded scopes ## Security Considerations