fix: default to allowing missing issuer (#1051)
This commit is contained in:
parent
14298b72e0
commit
92c0793247
1 changed files with 63 additions and 13 deletions
|
|
@ -1000,6 +1000,7 @@ pub struct AuthorizationManager {
|
|||
resource_scopes: RwLock<Vec<String>>,
|
||||
/// OIDC Dynamic Client Registration `application_type` (SEP-837)
|
||||
application_type: Option<String>,
|
||||
strict_issuer_validation: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
|
|
@ -1245,6 +1246,7 @@ impl AuthorizationManager {
|
|||
www_auth_scopes: RwLock::new(Vec::new()),
|
||||
resource_scopes: RwLock::new(Vec::new()),
|
||||
application_type: Some(DEFAULT_APPLICATION_TYPE.to_string()),
|
||||
strict_issuer_validation: false,
|
||||
};
|
||||
|
||||
Ok(manager)
|
||||
|
|
@ -1255,6 +1257,17 @@ impl AuthorizationManager {
|
|||
self.scope_upgrade_config = config;
|
||||
}
|
||||
|
||||
/// Configure whether authorization server metadata discovery requires the
|
||||
/// metadata `issuer` field.
|
||||
///
|
||||
/// The default is `false` to preserve compatibility with legacy
|
||||
/// authorization servers that omit `issuer`. Set this to `true` to enforce
|
||||
/// the RFC 8414/OIDC requirement that discovered metadata include `issuer`
|
||||
/// whenever the expected issuer can be derived from the discovery URL.
|
||||
pub fn set_strict_issuer_validation(&mut self, strict: bool) {
|
||||
self.strict_issuer_validation = strict;
|
||||
}
|
||||
|
||||
/// Set a custom credential store
|
||||
///
|
||||
/// This allows you to provide your own implementation of credential storage,
|
||||
|
|
@ -2226,7 +2239,7 @@ impl AuthorizationManager {
|
|||
|
||||
match serde_json::from_slice::<AuthorizationMetadata>(response.body()) {
|
||||
Ok(metadata) => {
|
||||
Self::validate_authorization_metadata_issuer(discovery_url, &metadata)?;
|
||||
self.validate_authorization_metadata_issuer(discovery_url, &metadata)?;
|
||||
Ok(Some(metadata))
|
||||
}
|
||||
Err(err) => {
|
||||
|
|
@ -2288,6 +2301,7 @@ impl AuthorizationManager {
|
|||
}
|
||||
|
||||
fn validate_authorization_metadata_issuer(
|
||||
&self,
|
||||
discovery_url: &Url,
|
||||
metadata: &AuthorizationMetadata,
|
||||
) -> Result<(), AuthError> {
|
||||
|
|
@ -2297,7 +2311,10 @@ impl AuthorizationManager {
|
|||
return Ok(());
|
||||
};
|
||||
let Some(received_issuer) = metadata.issuer.as_deref() else {
|
||||
return Err(AuthError::AuthorizationServerMissingIssuer { expected_issuer });
|
||||
if self.strict_issuer_validation {
|
||||
return Err(AuthError::AuthorizationServerMissingIssuer { expected_issuer });
|
||||
}
|
||||
return Ok(());
|
||||
};
|
||||
if !Self::issuer_identifiers_match(received_issuer, &expected_issuer) {
|
||||
return Err(AuthError::AuthorizationServerMismatch {
|
||||
|
|
@ -4006,8 +4023,8 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorization_metadata_accepts_oidc_path_appended_issuer() {
|
||||
#[tokio::test]
|
||||
async fn authorization_metadata_accepts_oidc_path_appended_issuer() {
|
||||
let discovery_url =
|
||||
Url::parse("https://auth.example.com/tenant1/.well-known/openid-configuration")
|
||||
.unwrap();
|
||||
|
|
@ -4017,8 +4034,12 @@ mod tests {
|
|||
token_endpoint: "https://auth.example.com/tenant1/token".to_string(),
|
||||
..Default::default()
|
||||
};
|
||||
let manager = AuthorizationManager::new("https://mcp.example.com/")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
AuthorizationManager::validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
manager
|
||||
.validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
|
|
@ -4034,8 +4055,8 @@ mod tests {
|
|||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorization_metadata_accepts_oidc_path_inserted_issuer() {
|
||||
#[tokio::test]
|
||||
async fn authorization_metadata_accepts_oidc_path_inserted_issuer() {
|
||||
let discovery_url =
|
||||
Url::parse("https://auth.example.com/.well-known/openid-configuration/tenant1")
|
||||
.unwrap();
|
||||
|
|
@ -4045,13 +4066,17 @@ mod tests {
|
|||
token_endpoint: "https://auth.example.com/tenant1/token".to_string(),
|
||||
..Default::default()
|
||||
};
|
||||
let manager = AuthorizationManager::new("https://mcp.example.com/")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
AuthorizationManager::validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
manager
|
||||
.validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorization_metadata_rejects_missing_issuer_for_standard_discovery_url() {
|
||||
#[tokio::test]
|
||||
async fn authorization_metadata_allows_missing_issuer_by_default() {
|
||||
let discovery_url =
|
||||
Url::parse("https://auth.example.com/.well-known/openid-configuration/tenant1")
|
||||
.unwrap();
|
||||
|
|
@ -4062,9 +4087,34 @@ mod tests {
|
|||
..Default::default()
|
||||
};
|
||||
|
||||
let error =
|
||||
AuthorizationManager::validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
.unwrap_err();
|
||||
let manager = AuthorizationManager::new("https://mcp.example.com/")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
manager
|
||||
.validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authorization_metadata_rejects_missing_issuer_when_required() {
|
||||
let discovery_url =
|
||||
Url::parse("https://auth.example.com/.well-known/openid-configuration/tenant1")
|
||||
.unwrap();
|
||||
let metadata = AuthorizationMetadata {
|
||||
issuer: None,
|
||||
authorization_endpoint: "https://auth.example.com/tenant1/authorize".to_string(),
|
||||
token_endpoint: "https://auth.example.com/tenant1/token".to_string(),
|
||||
..Default::default()
|
||||
};
|
||||
let mut manager = AuthorizationManager::new("https://mcp.example.com/")
|
||||
.await
|
||||
.unwrap();
|
||||
manager.set_strict_issuer_validation(true);
|
||||
|
||||
let error = manager
|
||||
.validate_authorization_metadata_issuer(&discovery_url, &metadata)
|
||||
.unwrap_err();
|
||||
|
||||
assert!(
|
||||
matches!(
|
||||
|
|
|
|||
Loading…
Reference in a new issue