diff --git a/.changeset/feat_add_insufficient_scope.md b/.changeset/feat_add_insufficient_scope.md new file mode 100644 index 000000000..85c78b88b --- /dev/null +++ b/.changeset/feat_add_insufficient_scope.md @@ -0,0 +1,13 @@ +### feat(auth): Add 403 Forbidden `insufficient_scope` support per MCP Auth Spec 2025-11-25 and RFC 6750 (Section 3.1) - @gocamille PR #537 + +This adds HTTP 403 Forbidden responses with `error="insufficient_scope"` per [MCP Auth Spec 2025-11-25 Section 10: Error Handling](https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#error-handling) and [RFC 6750 Section 3.1](https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1). + +**Changes:** +- `www_authenticate.rs`: Added `BearerError::InsufficientScope` enum and [error](crates/apollo-mcp-server/src/auth/www_authenticate.rs#L135-L149) field to `WWW-Authenticate` header +- `valid_token.rs`: Extract [scope](crates/apollo-mcp-server/src/auth/valid_token.rs#L27-L38)/[scp](crates/apollo-mcp-server/src/auth/valid_token.rs#L503-L509) claims from JWTs (handles both standard OAuth and Azure AD) +- `auth.rs`: Scope validation with fail-closed behaviour—valid tokens lacking required scopes get `403` +- `headers.rs`: Updated tests for new `ValidToken` struct + +**Behavior:** +- **401 Unauthorized**: Missing or invalid token +- **403 Forbidden**: Valid token but insufficient scopes (includes `error="insufficient_scope"` in response) diff --git a/crates/apollo-mcp-server/src/auth.rs b/crates/apollo-mcp-server/src/auth.rs index 291690691..f99537f77 100644 --- a/crates/apollo-mcp-server/src/auth.rs +++ b/crates/apollo-mcp-server/src/auth.rs @@ -28,7 +28,7 @@ mod www_authenticate; use protected_resource::ProtectedResource; pub(crate) use valid_token::ValidToken; use valid_token::ValidateToken; -use www_authenticate::WwwAuthenticate; +use www_authenticate::{BearerError, WwwAuthenticate}; /// Errors that can occur when building a TLS-configured HTTP client #[derive(Debug, thiserror::Error)] @@ -191,11 +191,15 @@ async fn oauth_validate( ) -> Result)> { let auth_config = &auth_state.config; - // Consolidated unauthorized error for use with any fallible step in this process - let unauthorized_error = || { - let mut resource = auth_config.resource.clone(); - resource.set_path("/.well-known/oauth-protected-resource"); + // Helper to construct the resource metadata URL + let resource_metadata_url = || { + let mut url = auth_config.resource.clone(); + url.set_path("/.well-known/oauth-protected-resource"); + url + }; + // Unauthorized error for missing or invalid tokens + let unauthorized_error = || { let scope = if auth_config.scopes.is_empty() { None } else { @@ -205,8 +209,21 @@ async fn oauth_validate( ( StatusCode::UNAUTHORIZED, TypedHeader(WwwAuthenticate::Bearer { - resource_metadata: resource, + resource_metadata: resource_metadata_url(), scope, + error: None, + }), + ) + }; + + // Forbidden error for valid tokens with insufficient scopes (RFC 6750 Section 3.1) + let forbidden_error = |required_scopes: &[String]| { + ( + StatusCode::FORBIDDEN, + TypedHeader(WwwAuthenticate::Bearer { + resource_metadata: resource_metadata_url(), + scope: Some(required_scopes.join(" ")), + error: Some(BearerError::InsufficientScope), }), ) }; @@ -229,6 +246,27 @@ async fn oauth_validate( unauthorized_error() })?; + // Check if token has required scopes (fail-closed: missing scope claim = insufficient) + if !auth_config.scopes.is_empty() { + let missing_scopes: Vec<_> = auth_config + .scopes + .iter() + .filter(|required| !valid_token.scopes.iter().any(|s| s == *required)) + .collect(); + + if !missing_scopes.is_empty() { + tracing::warn!( + required = ?auth_config.scopes, + present = ?valid_token.scopes, + missing = ?missing_scopes, + "Token has insufficient scopes" + ); + tracing::Span::current().record("reason", "insufficient_scope"); + tracing::Span::current().record("status_code", StatusCode::FORBIDDEN.as_u16()); + return Err(forbidden_error(&auth_config.scopes)); + } + } + // Insert new context to ensure that handlers only use our enforced token verification // for propagation request.extensions_mut().insert(valid_token); @@ -336,6 +374,78 @@ mod tests { assert!(!www_auth.contains("scope=")); } + mod scope_validation { + use super::*; + + fn scopes_are_sufficient(required: &[String], present: &[String]) -> bool { + required.iter().all(|req| present.contains(req)) + } + + #[test] + fn insufficient_scopes_fails() { + let required = vec!["read".to_string(), "write".to_string()]; + let present = vec!["read".to_string()]; + assert!(!scopes_are_sufficient(&required, &present)); + } + + #[test] + fn all_required_scopes_succeeds() { + let required = vec!["read".to_string(), "write".to_string()]; + let present = vec!["read".to_string(), "write".to_string()]; + assert!(scopes_are_sufficient(&required, &present)); + } + + #[test] + fn no_scopes_when_required_fails() { + let required = vec!["read".to_string()]; + let present: Vec = vec![]; + assert!(!scopes_are_sufficient(&required, &present)); + } + + #[test] + fn superset_of_scopes_succeeds() { + let required = vec!["read".to_string()]; + let present = vec!["read".to_string(), "write".to_string(), "admin".to_string()]; + assert!(scopes_are_sufficient(&required, &present)); + } + + #[test] + fn empty_required_scopes_always_succeeds() { + let required: Vec = vec![]; + let present = vec!["read".to_string()]; + assert!(scopes_are_sufficient(&required, &present)); + + let present_empty: Vec = vec![]; + assert!(scopes_are_sufficient(&required, &present_empty)); + } + + #[test] + fn scope_order_does_not_matter() { + let required = vec!["write".to_string(), "read".to_string()]; + let present = vec!["read".to_string(), "write".to_string()]; + assert!(scopes_are_sufficient(&required, &present)); + } + + #[test] + fn forbidden_error_contains_insufficient_scope() { + let header = WwwAuthenticate::Bearer { + resource_metadata: Url::parse( + "https://test.com/.well-known/oauth-protected-resource", + ) + .unwrap(), + scope: Some("read write".to_string()), + error: Some(BearerError::InsufficientScope), + }; + + let mut values = Vec::new(); + headers::Header::encode(&header, &mut values); + let encoded = values.first().unwrap().to_str().unwrap(); + + assert!(encoded.contains(r#"error="insufficient_scope""#)); + assert!(encoded.contains(r#"scope="read write""#)); + } + } + mod tls_config { use super::*; use std::io::Write; diff --git a/crates/apollo-mcp-server/src/auth/valid_token.rs b/crates/apollo-mcp-server/src/auth/valid_token.rs index 02b2ae938..1e8ffc891 100644 --- a/crates/apollo-mcp-server/src/auth/valid_token.rs +++ b/crates/apollo-mcp-server/src/auth/valid_token.rs @@ -12,16 +12,28 @@ use url::Url; /// Note: This is used as a marker to ensure that we have validated this /// separately from just reading the header itself. #[derive(Clone, Debug, PartialEq)] -pub(crate) struct ValidToken(pub(crate) Authorization); +pub(crate) struct ValidToken { + pub(crate) token: Authorization, + pub(crate) scopes: Vec, +} impl Deref for ValidToken { type Target = Authorization; fn deref(&self) -> &Self::Target { - &self.0 + &self.token } } +/// Extract scopes from JWT claims. +/// +/// Scopes are expected as a space-separated string per RFC 6749. +pub(super) fn extract_scopes(scope: Option<&str>) -> Vec { + scope + .map(|s| s.split_whitespace().map(String::from).collect()) + .unwrap_or_default() +} + /// Trait to handle validation of tokens pub(super) trait ValidateToken { /// Whether to skip audience validation (allow any audience) @@ -51,6 +63,10 @@ pub(super) trait ValidateToken { /// The user who owns this token pub sub: String, + + /// OAuth scope claim (space-separated list per RFC 6749) + #[serde(default)] + pub scope: Option, } fn deserialize_audience<'de, D>(deserializer: D) -> Result, D::Error> @@ -111,8 +127,9 @@ pub(super) trait ValidateToken { }; match decode::(jwt, &jwk.decoding_key, &validation) { - Ok(_) => { - return Some(ValidToken(token)); + Ok(token_data) => { + let scopes = extract_scopes(token_data.claims.scope.as_deref()); + return Some(ValidToken { token, scopes }); } Err(e) => warn!("Token failed validation with error: {e}"), }; @@ -242,7 +259,7 @@ mod test { .validate(jwt) .await .expect("valid token") - .0 + .token .token(), token ); @@ -407,7 +424,7 @@ mod test { .validate(jwt) .await .expect("valid token") - .0 + .token .token(), token ); @@ -492,4 +509,42 @@ mod test { .ok_or("Expected warning for validation failure".to_string()) }); } + + // Tests for extract_scopes + mod extract_scopes_tests { + use super::super::extract_scopes; + + #[test] + fn returns_empty_when_none() { + assert_eq!(extract_scopes(None), Vec::::new()); + } + + #[test] + fn extracts_from_scope_claim() { + assert_eq!(extract_scopes(Some("read write")), vec!["read", "write"]); + } + + #[test] + fn handles_extra_whitespace() { + assert_eq!( + extract_scopes(Some(" read write ")), + vec!["read", "write"] + ); + } + + #[test] + fn handles_empty_string() { + assert_eq!(extract_scopes(Some("")), Vec::::new()); + } + + #[test] + fn handles_whitespace_only() { + assert_eq!(extract_scopes(Some(" ")), Vec::::new()); + } + + #[test] + fn handles_single_scope() { + assert_eq!(extract_scopes(Some("admin")), vec!["admin"]); + } + } } diff --git a/crates/apollo-mcp-server/src/auth/www_authenticate.rs b/crates/apollo-mcp-server/src/auth/www_authenticate.rs index 99baa8214..6c0ed985a 100644 --- a/crates/apollo-mcp-server/src/auth/www_authenticate.rs +++ b/crates/apollo-mcp-server/src/auth/www_authenticate.rs @@ -11,9 +11,17 @@ pub(super) enum WwwAuthenticate { Bearer { resource_metadata: Url, scope: Option, + error: Option, }, } +/// OAuth 2.0 Bearer Token error codes per RFC 6750 Section 3.1 +#[derive(Debug, Clone)] +pub(super) enum BearerError { + /// The request requires higher privileges than provided by the access token. + InsufficientScope, +} + impl Header for WwwAuthenticate { fn name() -> &'static http::HeaderName { &WWW_AUTHENTICATE @@ -33,11 +41,19 @@ impl Header for WwwAuthenticate { WwwAuthenticate::Bearer { resource_metadata, scope, + error, } => { let mut header = format!( r#"Bearer resource_metadata="{}""#, resource_metadata.as_str() ); + // Error must come before scope per RFC 6750 examples + if let Some(err) = error { + let error_str = match err { + BearerError::InsufficientScope => "insufficient_scope", + }; + header.push_str(&format!(r#", error="{}""#, error_str)); + } if let Some(scope) = scope { header.push_str(&format!(r#", scope="{}""#, scope)); } @@ -73,6 +89,7 @@ mod tests { resource_metadata: Url::parse("https://test.com/.well-known/oauth-protected-resource") .unwrap(), scope: None, + error: None, }; let encoded = encode_header(&header); @@ -90,6 +107,7 @@ mod tests { ) .unwrap(), scope: Some("read".to_string()), + error: None, }; let encoded = encode_header(&header); @@ -104,6 +122,7 @@ mod tests { resource_metadata: Url::parse("https://test.com/.well-known/oauth-protected-resource") .unwrap(), scope: Some("read write".to_string()), + error: None, }; let encoded = encode_header(&header); @@ -112,4 +131,20 @@ mod tests { r#"Bearer resource_metadata="https://test.com/.well-known/oauth-protected-resource", scope="read write""# ); } + + #[test] + fn encode_bearer_with_insufficient_scope_error() { + let header = WwwAuthenticate::Bearer { + resource_metadata: Url::parse("https://test.com/.well-known/oauth-protected-resource") + .unwrap(), + scope: Some("read write".to_string()), + error: Some(BearerError::InsufficientScope), + }; + + let encoded = encode_header(&header); + assert_eq!( + encoded, + r#"Bearer resource_metadata="https://test.com/.well-known/oauth-protected-resource", error="insufficient_scope", scope="read write""# + ); + } } diff --git a/crates/apollo-mcp-server/src/headers.rs b/crates/apollo-mcp-server/src/headers.rs index d028602d9..a72508915 100644 --- a/crates/apollo-mcp-server/src/headers.rs +++ b/crates/apollo-mcp-server/src/headers.rs @@ -124,7 +124,10 @@ mod tests { let incoming_headers = HeaderMap::new(); let mut extensions = Extensions::new(); - let token = ValidToken(Authorization::bearer("test-token").unwrap()); + let token = ValidToken { + token: Authorization::bearer("test-token").unwrap(), + scopes: vec![], + }; extensions.insert(token); let result = build_request_headers( @@ -146,7 +149,10 @@ mod tests { let incoming_headers = HeaderMap::new(); let mut extensions = Extensions::new(); - let token = ValidToken(Authorization::bearer("test-token").unwrap()); + let token = ValidToken { + token: Authorization::bearer("test-token").unwrap(), + scopes: vec![], + }; extensions.insert(token); let result = build_request_headers( @@ -201,7 +207,10 @@ mod tests { // OAuth token let mut extensions = Extensions::new(); - let token = ValidToken(Authorization::bearer("oauth-token").unwrap()); + let token = ValidToken { + token: Authorization::bearer("oauth-token").unwrap(), + scopes: vec![], + }; extensions.insert(token); let result = build_request_headers(