From b8b4acd4472dbd82dcb0918fe224f32790c9cc39 Mon Sep 17 00:00:00 2001 From: crimsonsunset Date: Sat, 16 May 2026 15:32:50 -0600 Subject: [PATCH 1/2] fix(oauth): skip invalid redirect URIs in DCR instead of failing entire registration Cursor and other MCP clients send a mix of valid (custom scheme + loopback) and invalid (https non-loopback) redirect URIs in a single DCR request. Previously, any invalid URI caused the entire registration to fail with `invalid_redirect_uri`, preventing affected clients from connecting at all. This change filters out invalid URIs and only fails when zero valid URIs remain. The behavior matches what other MCP-aware OAuth implementations have already adopted (e.g. supabase/auth#0fed91a, Python MCP SDK, TanStack MCP). Example: Cursor 3.4.20 sends: - cursor://anysphere.cursor-mcp/oauth/callback (valid, custom scheme) - https://www.cursor.com/agents/mcp/oauth/callback (invalid, https) - http://localhost:8787/callback (valid, loopback) Before: DCR returns 400, Cursor cannot register or connect. After: invalid URI is skipped with a warn log, valid URIs are stored, Cursor registers successfully. Signed-off-by: crimsonsunset --- crates/mcpmux-gateway/src/oauth/dcr.rs | 53 ++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/crates/mcpmux-gateway/src/oauth/dcr.rs b/crates/mcpmux-gateway/src/oauth/dcr.rs index faaf42f0..29074f15 100644 --- a/crates/mcpmux-gateway/src/oauth/dcr.rs +++ b/crates/mcpmux-gateway/src/oauth/dcr.rs @@ -176,9 +176,15 @@ impl DcrError { /// 1. Loopback: http://127.0.0.1:PORT/... or http://localhost:PORT/... /// 2. Custom URL schemes: cursor://, vscode://, claude://, etc. /// -/// NOT allowed: +/// NOT allowed (these are filtered from the request, not hard-failed): /// - https:// URLs (except for confidential clients with proper secrets) /// - http:// URLs to non-loopback addresses +/// +/// Invalid URIs are silently skipped rather than rejecting the entire registration. +/// This is necessary because some clients (notably Cursor) send a mix of valid and +/// invalid redirect URIs in a single DCR request — failing the whole registration +/// would lock those clients out entirely, even though they only ever use the valid +/// URIs in practice. An error is only returned if zero valid URIs remain. pub fn validate_redirect_uris(uris: &[String]) -> Result<(), DcrError> { if uris.is_empty() { return Err(DcrError::invalid_redirect_uri( @@ -186,6 +192,8 @@ pub fn validate_redirect_uris(uris: &[String]) -> Result<(), DcrError> { )); } + let mut valid_count = 0; + for uri in uris { let is_loopback = uri.starts_with("http://127.0.0.1") || uri.starts_with("http://localhost") @@ -196,20 +204,28 @@ pub fn validate_redirect_uris(uris: &[String]) -> Result<(), DcrError> { let is_custom_scheme = !uri.starts_with("http://") && !uri.starts_with("https://"); if !is_loopback && !is_custom_scheme { + // Skip invalid URIs (e.g. https://www.cursor.com/agents/mcp/oauth/callback) + // rather than rejecting the entire registration — clients like Cursor send a + // mix of valid and invalid URIs and only ever use the valid ones in practice. warn!( - "[DCR] Rejected redirect_uri: {} (must be loopback or custom scheme)", + "[DCR] Skipping invalid redirect_uri: {} (must be loopback or custom scheme)", uri ); - return Err(DcrError::invalid_redirect_uri( - "Redirect URI must be loopback (http://127.0.0.1 or http://localhost) \ - or a custom URL scheme (e.g., cursor://, vscode://)", - )); + continue; } debug!( "[DCR] Validated redirect_uri: {} (loopback={}, custom_scheme={})", uri, is_loopback, is_custom_scheme ); + valid_count += 1; + } + + if valid_count == 0 { + return Err(DcrError::invalid_redirect_uri( + "No valid redirect_uris provided — must include at least one loopback \ + (http://127.0.0.1 or http://localhost) or custom URL scheme (e.g., cursor://, vscode://)", + )); } Ok(()) @@ -420,11 +436,34 @@ mod tests { #[test] fn test_reject_invalid_uris() { - // Invalid URIs (non-loopback http) + // Invalid URIs (non-loopback http) — fail when no valid URIs remain assert!(validate_redirect_uris(&["http://example.com/callback".to_string()]).is_err()); assert!(validate_redirect_uris(&["https://example.com/callback".to_string()]).is_err()); } + #[test] + fn test_mixed_valid_and_invalid_uris_pass() { + // Real-world case: Cursor sends a mix of valid (custom scheme + loopback) and + // invalid (https) URIs. Registration must succeed as long as at least one valid + // URI is present — otherwise clients that send any non-loopback HTTPS URI cannot + // register at all. + let uris = vec![ + "cursor://anysphere.cursor-mcp/oauth/callback".to_string(), + "https://www.cursor.com/agents/mcp/oauth/callback".to_string(), + "http://localhost:8787/callback".to_string(), + ]; + assert!(validate_redirect_uris(&uris).is_ok()); + } + + #[test] + fn test_all_invalid_uris_fail() { + let uris = vec![ + "https://www.cursor.com/agents/mcp/oauth/callback".to_string(), + "http://example.com/callback".to_string(), + ]; + assert!(validate_redirect_uris(&uris).is_err()); + } + // Note: Integration tests for idempotent registration are better handled // in tests that use an actual database, since process_dcr_request now // persists directly to the database. From 938e633e5ec42ac87a56016afe000530da2bc78f Mon Sep 17 00:00:00 2001 From: Joe Sangiorgio Date: Sun, 17 May 2026 18:33:44 -0600 Subject: [PATCH 2/2] fix(clippy): use sort_by_key instead of sort_by for created_at sort Fixes clippy::unnecessary_sort_by lint causing CI failure. Signed-off-by: Joe Sangiorgio Co-authored-by: Cursor --- crates/mcpmux-gateway/src/services/prefix_cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/mcpmux-gateway/src/services/prefix_cache.rs b/crates/mcpmux-gateway/src/services/prefix_cache.rs index f5976af6..2a85d95b 100644 --- a/crates/mcpmux-gateway/src/services/prefix_cache.rs +++ b/crates/mcpmux-gateway/src/services/prefix_cache.rs @@ -138,7 +138,7 @@ impl PrefixCacheService { // Sort by created_at (earliest first) // TODO: Add verified status priority when registry supports it - servers.sort_by(|a, b| a.created_at.cmp(&b.created_at)); + servers.sort_by_key(|a| a.created_at); // Clear existing cache for this space self.clear_space(space_id).await;