Skip to content

feat: accumulate client-side scopes during step-up authorization (SEP-2350) #888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stefanoamorelli wants to merge 1 commit into
base: main
Choose a base branch
from
+110 −25
Open

feat: accumulate client-side scopes during step-up authorization (SEP-2350) #888

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 110 additions & 25 deletions crates/rmcp/src/transport/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,11 +1042,18 @@ impl AuthorizationManager {

/// compute the union of current scopes and required scopes
fn compute_scope_union(current: &[String], required: &str) -> Vec<String> {
let mut scope_set: std::collections::HashSet<String> = current.iter().cloned().collect();
for scope in required.split_whitespace() {
scope_set.insert(scope.to_string());
}
scope_set.into_iter().collect()
let mut scopes = current.to_vec();
scopes.extend(required.split_whitespace().map(|s| s.to_string()));
Self::dedup_scopes(scopes)
}

/// deduplicate scopes preserving first-seen order (SEP-2350: stable for testability)
fn dedup_scopes(scopes: Vec<String>) -> Vec<String> {
let mut seen = std::collections::HashSet::new();
scopes
.into_iter()
.filter(|s| seen.insert(s.clone()))
.collect()
}

/// check if a scope upgrade is possible and allowed
Expand All @@ -1069,35 +1076,42 @@ impl AuthorizationManager {
scopes
}

/// select scopes based on SEP-835 priority:
/// 1. scope from WWW-Authenticate header (argument or stored from initial 401 probe)
/// 2. scopes_supported from protected resource metadata (RFC 9728)
/// 3. scopes_supported from authorization server metadata
/// 4. provided default scopes
/// select scopes following SEP-2350: re-authorization requests the union of the
/// previously requested scopes and the newly challenged scopes. Server-reported
/// scopes (WWW-Authenticate challenge, protected resource metadata) are operational
/// requirements for the current operation, never an exclusive directive, so they
/// accumulate rather than replace. The AS metadata and caller defaults only seed the
/// request when nothing has been requested or challenged yet.
fn select_base_scopes(
&self,
www_authenticate_scope: Option<&str>,
default_scopes: &[&str],
) -> Vec<String> {
if let Some(scope) = www_authenticate_scope {
return scope.split_whitespace().map(|s| s.to_string()).collect();
let mut accumulated: Vec<String> = Vec::new();

// previously requested scopes
if let Ok(guard) = self.current_scopes.try_read() {
accumulated.extend(guard.iter().cloned());
}

// use scopes from initial 401 WWW-Authenticate header
// newly challenged scopes for the current operation (RFC 6750 §3.1)
if let Some(scope) = www_authenticate_scope {
accumulated.extend(scope.split_whitespace().map(|s| s.to_string()));
}
if let Ok(guard) = self.www_auth_scopes.try_read() {
if !guard.is_empty() {
return guard.clone();
}
accumulated.extend(guard.iter().cloned());
}

// use scopes_supported from protected resource metadata (RFC 9728)
// scopes required for the current operation per protected resource metadata (RFC 9728)
if let Ok(guard) = self.resource_scopes.try_read() {
if !guard.is_empty() {
return guard.clone();
}
accumulated.extend(guard.iter().cloned());
}

if !accumulated.is_empty() {
return Self::dedup_scopes(accumulated);
}

// use scopes_supported from authorization server metadata
// nothing requested or challenged yet: seed from AS metadata, then caller defaults
if let Some(metadata) = &self.metadata {
if let Some(scopes_supported) = &metadata.scopes_supported {
if !scopes_supported.is_empty() {
Expand Down Expand Up @@ -1225,10 +1239,13 @@ impl AuthorizationManager {

debug!("exchange token result: {:?}", token_result);

let granted_scopes: Vec<String> = token_result
.scopes()
.map(|scopes| scopes.iter().map(|s| s.to_string()).collect())
.unwrap_or_default();
// SEP-2350: an explicit scope list is authoritative and may narrow the grant, but a
// server that grants the requested scopes may omit `scope` (RFC 6749 §5.1); when it
// does, retain the accumulated set rather than clearing it.
let granted_scopes: Vec<String> = match token_result.scopes() {
Some(scopes) => scopes.iter().map(|s| s.to_string()).collect(),
None => self.current_scopes.read().await.clone(),
};

*self.current_scopes.write().await = granted_scopes.clone();
*self.scope_upgrade_attempts.write().await = 0;
Expand Down Expand Up @@ -3506,6 +3523,74 @@ mod tests {
assert!(scopes.contains(&"email".to_string()));
}

// -- SEP-2350: client-side scope accumulation in step-up authorization --

#[tokio::test]
async fn select_scopes_unions_challenge_with_previously_requested() {
let mgr = manager_with_metadata(None).await;
*mgr.current_scopes.write().await = vec!["read".to_string()];

let scopes = mgr.select_scopes(Some("write"), &[]);

assert_eq!(scopes, vec!["read".to_string(), "write".to_string()]);
}

#[tokio::test]
async fn select_scopes_does_not_replace_previously_requested_with_challenge() {
let mgr = manager_with_metadata(None).await;
*mgr.current_scopes.write().await = vec!["read".to_string(), "profile".to_string()];

let scopes = mgr.select_scopes(Some("write"), &[]);

assert!(scopes.contains(&"read".to_string()));
assert!(scopes.contains(&"profile".to_string()));
assert!(scopes.contains(&"write".to_string()));
}

#[tokio::test]
async fn select_scopes_accumulates_across_multiple_step_up_rounds() {
let mgr = manager_with_metadata(None).await;
*mgr.current_scopes.write().await = vec!["read".to_string()];

// round one: server challenges for "write"
let round_one = mgr.select_scopes(Some("write"), &[]);
assert_eq!(round_one, vec!["read".to_string(), "write".to_string()]);
*mgr.current_scopes.write().await = round_one;

// round two: server challenges for "admin", earlier scopes are retained
let round_two = mgr.select_scopes(Some("admin"), &[]);
assert_eq!(
round_two,
vec!["read".to_string(), "write".to_string(), "admin".to_string()]
);
}

#[tokio::test]
async fn select_scopes_deduplicates_challenge_already_requested() {
let mgr = manager_with_metadata(None).await;
*mgr.current_scopes.write().await = vec!["read".to_string(), "write".to_string()];

let scopes = mgr.select_scopes(Some("write admin"), &[]);

assert_eq!(
scopes,
vec!["read".to_string(), "write".to_string(), "admin".to_string()]
);
}

#[tokio::test]
async fn select_scopes_unions_resource_metadata_as_operational_requirement() {
let mgr = manager_with_metadata(None).await;
*mgr.current_scopes.write().await = vec!["read".to_string()];
*mgr.resource_scopes.write().await = vec!["profile".to_string()];

let scopes = mgr.select_scopes(Some("write"), &[]);

assert!(scopes.contains(&"read".to_string()));
assert!(scopes.contains(&"write".to_string()));
assert!(scopes.contains(&"profile".to_string()));
}

#[tokio::test]
async fn add_offline_access_if_supported_works_with_explicit_scopes() {
let mgr = manager_with_metadata(Some(AuthorizationMetadata {
Expand Down