Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 627
Add SEP-991 (CIMD) support for URL-based client IDs#570
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1fb0ef3
feat(auth): add cimd support for SEP-991
tanish111 2784a6d
test(auth): add unit tests for is_https_url helper
tanish111 45af2c2
feat(example): add CIMD OAuth server for SEP-991 testing
tanish111 b131bb1
fix(oauth): add CORS headers to token endpoint
tanish111 463b73a
refactor: improve is_https_url function and consolidate tests
tanish111 88f4c30
Merge remote-tracking branch 'origin/main' into feat/cimd
tanish111 51f0209
refactor: use map_err instead of match for error handling in auth.rs
tanish111 a8a5e92
feat: add client-metadata.json
tanish111 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "client_id": "https://raw.githubusercontent.com/modelcontextprotocol/rust-sdk/refs/heads/main/client-metadata.json", | ||
| "redirect_uris": ["http://localhost:4000/callback"], | ||
| "grant_types": ["authorization_code"], | ||
| "response_types": ["code"], | ||
| "token_endpoint_auth_method": "none" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -239,6 +239,15 @@ struct AuthorizationState { | ||
| csrf_token: CsrfToken, | ||
| } | ||
| /// SEP-991: URL-based Client IDs | ||
| /// Validate that the client_id is a valid URL with https scheme and non-root pathname | ||
| fn is_https_url(value: &str) -> bool { | ||
| Url::parse(value) | ||
| .ok() | ||
| .map(|url| url.scheme() == "https" && url.path() != "/" && url.host_str().is_some()) | ||
| .unwrap_or(false) | ||
| } | ||
| impl AuthorizationManager { | ||
| fn well_known_paths(base_path: &str, resource: &str) -> Vec<String> { | ||
| let trimmed = base_path.trim_start_matches('/').trim_end_matches('/'); | ||
| @@ -950,30 +959,57 @@ impl AuthorizationSession { | ||
| scopes: &[&str], | ||
| redirect_uri: &str, | ||
| client_name: Option<&str>, | ||
| client_metadata_url: Option<&str>, | ||
| ) -> Result<Self, AuthError> { | ||
| // Default client config | ||
| let config = OAuthClientConfig { | ||
| client_id: "mcp-client".to_string(), | ||
| client_secret: None, | ||
| scopes: scopes.iter().map(|s| s.to_string()).collect(), | ||
| redirect_uri: redirect_uri.to_string(), | ||
| }; | ||
| // try to dynamic register client | ||
| let config = match auth_manager | ||
| .register_client(client_name.unwrap_or("MCP Client"), redirect_uri) | ||
| .await | ||
| { | ||
| Ok(config) => config, | ||
| Err(e) => { | ||
| warn!( | ||
| "Dynamic registration failed: {}, fallback to default config", | ||
| e | ||
| ); | ||
| // fallback to default config | ||
| config | ||
| let metadata = auth_manager.metadata.as_ref(); | ||
| let supports_url_based_client_id = metadata | ||
| .and_then(|m| { | ||
| m.additional_fields | ||
| .get("client_id_metadata_document_supported") | ||
| }) | ||
| .and_then(|v| v.as_bool()) | ||
| .unwrap_or(false); | ||
| let config = if supports_url_based_client_id { | ||
| if let Some(client_metadata_url) = client_metadata_url { | ||
| if !is_https_url(client_metadata_url) { | ||
| return Err(AuthError::RegistrationFailed(format!( | ||
| "client_metadata_url must be a valid HTTPS URL with a non-root pathname, got: {}", | ||
| client_metadata_url | ||
| ))); | ||
| } | ||
| // SEP-991: URL-based Client IDs - use URL as client_id directly | ||
| OAuthClientConfig { | ||
| client_id: client_metadata_url.to_string(), | ||
| client_secret: None, | ||
| scopes: scopes.iter().map(|s| s.to_string()).collect(), | ||
| redirect_uri: redirect_uri.to_string(), | ||
| } | ||
| } else { | ||
| // Fallback to dynamic registration | ||
| auth_manager | ||
| .register_client(client_name.unwrap_or("MCP Client"), redirect_uri) | ||
| .await | ||
| .map_err(|e| { | ||
| AuthError::RegistrationFailed(format!("Dynamic registration failed: {}", e)) | ||
| })? | ||
| } | ||
| } else { | ||
| // Fallback to dynamic registration | ||
| match auth_manager | ||
| .register_client(client_name.unwrap_or("MCP Client"), redirect_uri) | ||
| .await | ||
| { | ||
| Ok(config) => config, | ||
| Err(e) => { | ||
| return Err(AuthError::RegistrationFailed(format!( | ||
| "Dynamic registration failed: {}", | ||
| e | ||
| ))); | ||
| } | ||
| } | ||
| }; | ||
| // reset client config | ||
| auth_manager.configure_client(config)?; | ||
| let auth_url = auth_manager.get_authorization_url(scopes).await?; | ||
| @@ -1125,6 +1161,18 @@ impl OAuthState { | ||
| scopes: &[&str], | ||
| redirect_uri: &str, | ||
| client_name: Option<&str>, | ||
| ) -> Result<(), AuthError> { | ||
tanish111 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| self.start_authorization_with_metadata_url(scopes, redirect_uri, client_name, None) | ||
| .await | ||
| } | ||
| /// start authorization with optional client metadata URL (SEP-991) | ||
| pub async fn start_authorization_with_metadata_url( | ||
| &mut self, | ||
| scopes: &[&str], | ||
| redirect_uri: &str, | ||
| client_name: Option<&str>, | ||
| client_metadata_url: Option<&str>, | ||
| ) -> Result<(), AuthError> { | ||
| if let OAuthState::Unauthorized(mut manager) = std::mem::replace( | ||
| self, | ||
| @@ -1134,8 +1182,14 @@ impl OAuthState { | ||
| let metadata = manager.discover_metadata().await?; | ||
| manager.metadata = Some(metadata); | ||
| debug!("start session"); | ||
| let session = | ||
| AuthorizationSession::new(manager, scopes, redirect_uri, client_name).await?; | ||
| let session = AuthorizationSession::new( | ||
| manager, | ||
| scopes, | ||
| redirect_uri, | ||
| client_name, | ||
| client_metadata_url, | ||
| ) | ||
| .await?; | ||
| *self = OAuthState::Session(session); | ||
| Ok(()) | ||
| } else { | ||
| @@ -1256,7 +1310,31 @@ impl OAuthState { | ||
| mod tests { | ||
| use url::Url; | ||
| use super::AuthorizationManager; | ||
| use super::{AuthorizationManager, is_https_url}; | ||
| // SEP-991: URL-based Client IDs | ||
tanish111 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Tests adapted from the TypeScript SDK's isHttpsUrl test suite | ||
| #[test] | ||
| fn test_is_https_url_scenarios() { | ||
| // Returns true for valid https url with path | ||
| assert!(is_https_url("https://example.com/client-metadata.json")); | ||
| // Returns true for https url with query params | ||
| assert!(is_https_url("https://example.com/metadata?version=1")); | ||
| // Returns false for https url without path | ||
| assert!(!is_https_url("https://example.com")); | ||
| assert!(!is_https_url("https://example.com/")); | ||
| assert!(!is_https_url("https://")); | ||
| // Returns false for http url | ||
| assert!(!is_https_url("http://example.com/metadata")); | ||
| // Returns false for non-url strings | ||
| assert!(!is_https_url("not a url")); | ||
| // Returns false for empty string | ||
| assert!(!is_https_url("")); | ||
| // Returns false for javascript scheme | ||
| assert!(!is_https_url("javascript:alert(1)")); | ||
| // Returns false for data scheme | ||
| assert!(!is_https_url("data:text/html,<script>alert(1)</script>")); | ||
| } | ||
| #[test] | ||
| fn parses_resource_metadata_parameter() { | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.