Skip to content

fix: default to allowing missing issuer #1051

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

Merged
jamadeo merged 1 commit into from
Jul 27, 2026
+63 −13
Merged

fix: default to allowing missing issuer #1051

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
76 changes: 63 additions & 13 deletions crates/rmcp/src/transport/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,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)]
Expand Down Expand Up @@ -1208,6 +1209,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)
Expand All @@ -1218,6 +1220,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) {

@alexhancock alexhancock Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide a public setter, or just announce the change and change the default at some point in the future? Providing a setter suggests to people they could set it to false and leave it that way when my understanding of this is that we'll want to eventually always require it.

@jamadeo jamadeo Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once we are at the point where we remove non-compliant server support, we'd remove the setter completely and that forces users to change their code anyway.

self.strict_issuer_validation = strict;
}

/// Set a custom credential store
///
/// This allows you to provide your own implementation of credential storage,
Expand Down Expand Up @@ -2156,7 +2169,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) => {
Expand Down Expand Up @@ -2218,6 +2231,7 @@ impl AuthorizationManager {
}

fn validate_authorization_metadata_issuer(
&self,
discovery_url: &Url,
metadata: &AuthorizationMetadata,
) -> Result<(), AuthError> {
Expand All @@ -2227,7 +2241,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 {
Expand Down Expand Up @@ -3938,8 +3955,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();
Expand All @@ -3949,8 +3966,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();
}

Expand All @@ -3966,8 +3987,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();
Expand All @@ -3977,13 +3998,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();
Expand All @@ -3994,9 +4019,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!(
Expand Down