fix(oauth): DCR skip-invalid redirect URIs + drop duplicate RFC 8707 resource param

.github/workflows/e2e-desktop.yml

@@ -131,16 +131,54 @@ jobs:
         if: matrix.os == 'windows-latest'
         shell: pwsh
         run: |
-          # Tauri uses the WebView2 runtime, not Edge directly.
-          # msedgedriver must match the WebView2 runtime version, not the Edge browser version.
+          # Tauri uses the WebView2 runtime, not Edge directly, so msedgedriver must be
+          # compatible with the WebView2 runtime (matching the major version is sufficient).
+          # Microsoft does not publish a driver for every exact runtime build, so try the
+          # exact version first, then fall back to the latest driver for the same major
+          # version, then LATEST_STABLE. This keeps the job from breaking whenever the
+          # runner's WebView2 build outpaces the published Edge drivers.
+          $ErrorActionPreference = 'Stop'
           $wv2Key = 'HKLM:\SOFTWARE\WOW6432Node\Microsoft\EdgeUpdate\Clients\{F3017226-FE2A-4295-8BDF-00C3A9A7E4C5}'
           if (-not (Test-Path $wv2Key)) { $wv2Key = 'HKLM:\SOFTWARE\Microsoft\EdgeUpdate\Clients\{F3017226-FE2A-4295-8BDF-00C3A9A7E4C5}' }
           $wv2Version = (Get-ItemProperty $wv2Key).pv
-          Write-Host "WebView2 runtime version: $wv2Version"
-          $driverUrl = "https://msedgewebdriverstorage.blob.core.windows.net/edgewebdriver/$wv2Version/edgedriver_win64.zip"
+          $wv2Major = $wv2Version.Split('.')[0]
+          Write-Host "WebView2 runtime version: $wv2Version (major $wv2Major)"
+
+          # Microsoft locked down the old msedgewebdriverstorage blob (now returns HTTP 409
+          # "Public access is not permitted"); the current host is msedgedriver.microsoft.com.
+          $base = "https://msedgedriver.microsoft.com"
+
+          # Ordered list of candidate driver versions to try.
+          $candidates = [System.Collections.Generic.List[string]]::new()
+          $candidates.Add($wv2Version)
+          foreach ($marker in @("LATEST_RELEASE_${wv2Major}_WINDOWS", "LATEST_STABLE")) {
+            try {
+              $resolved = (Invoke-WebRequest -UseBasicParsing -Uri "$base/$marker").Content
+              # Markers are UTF-16/BOM encoded; keep only version characters.
+              $resolved = ($resolved -replace '[^0-9\.]', '').Trim()
+              if ($resolved) { $candidates.Add($resolved); Write-Host "marker $marker -> $resolved" }
+            } catch {
+              Write-Host "marker $marker unavailable: $($_.Exception.Message)"
+            }
+          }
+
           $zipPath = "$env:TEMP\edgedriver.zip"
           $extractDir = "$env:TEMP\edgedriver"
-          Invoke-WebRequest -Uri $driverUrl -OutFile $zipPath
+          $ok = $false
+          foreach ($v in ($candidates | Select-Object -Unique)) {
+            $driverUrl = "$base/$v/edgedriver_win64.zip"
+            Write-Host "Trying EdgeDriver $v -> $driverUrl"
+            try {
+              Invoke-WebRequest -UseBasicParsing -Uri $driverUrl -OutFile $zipPath
+              Write-Host "Downloaded EdgeDriver $v"
+              $ok = $true
+              break
+            } catch {
+              Write-Host "  not available: $($_.Exception.Message)"
+            }
+          }
+          if (-not $ok) { throw "No compatible msedgedriver found for WebView2 $wv2Version (tried: $($candidates -join ', '))" }
+
           Expand-Archive -Path $zipPath -DestinationPath $extractDir -Force
           $driverDir = "$extractDir\edgedriver_win64"
           if (-not (Test-Path "$driverDir\msedgedriver.exe")) { $driverDir = $extractDir }

crates/mcpmux-gateway/src/oauth/dcr.rs

@@ -176,16 +176,24 @@ 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(
             "At least one redirect_uri is required",
         ));
     }
 
+    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.

crates/mcpmux-gateway/src/pool/oauth.rs

@@ -255,36 +255,6 @@ impl OutboundOAuthManager {
         scopes.iter().map(|s| s.as_str()).collect()
     }
 
-    /// Add RFC 8707 'resource' parameter to authorization URL.
-    ///
-    /// The resource parameter tells the Authorization Server which protected resource
-    /// (MCP server) the client is requesting access to. This enables the AS to:
-    /// - Issue tokens scoped to the specific resource
-    /// - Apply resource-specific policies
-    /// - Prevent token replay at other resources
-    ///
-    /// Some servers (like Miro) require this parameter.
-    fn add_resource_parameter(auth_url: &str, server_url: &str) -> String {
-        use url::Url;
-
-        match Url::parse(auth_url) {
-            Ok(mut url) => {
-                // Add the resource parameter with the MCP server URL
-                url.query_pairs_mut().append_pair("resource", server_url);
-                info!("[OAuth] Added RFC 8707 resource parameter: {}", server_url);
-                url.to_string()
-            }
-            Err(e) => {
-                warn!(
-                    "[OAuth] Failed to parse auth URL to add resource parameter: {}",
-                    e
-                );
-                // Return original URL if parsing fails
-                auth_url.to_string()
-            }
-        }
-    }
-
     /// Subscribe to OAuth completion events
     pub fn subscribe(&self) -> tokio::sync::broadcast::Receiver<OAuthCompleteEvent> {
         self.completion_tx.subscribe()
@@ -1247,11 +1217,6 @@ impl OutboundOAuthManager {
             }
         };
 
-        // Add RFC 8707 'resource' parameter to the authorization URL.
-        // This tells the Authorization Server which protected resource (MCP server)
-        // the token is being requested for. Some servers (like Miro) require this.
-        let auth_url = Self::add_resource_parameter(&auth_url, server_url);
-
         // Extract state parameter from auth_url
         let state = match Self::extract_state_from_url(&auth_url) {
             Some(s) => s,
@@ -1684,3 +1649,54 @@ impl Default for OutboundOAuthManager {
         Self::new()
     }
 }
+
+#[cfg(test)]
+mod resource_param_tests {
+    use rmcp::transport::auth::{AuthorizationManager, AuthorizationMetadata};
+
+    /// The gateway delegates authorize-URL construction entirely to rmcp's
+    /// `AuthorizationManager::get_authorization_url` (via `create_auth_manager` /
+    /// `start_oauth_flow`). rmcp appends the RFC 8707 `resource` parameter itself, so the
+    /// gateway must NOT add a second one. This guards against re-introducing the removed
+    /// `add_resource_parameter` wrapper, which produced `?resource=...&resource=...` —
+    /// rejected by strict authorization servers (e.g. Supabase) and broke OAuth login.
+    #[tokio::test]
+    async fn authorize_url_has_exactly_one_resource_param() {
+        let base_url = "https://mcp.example.test/";
+
+        let mut manager = AuthorizationManager::new(base_url)
+            .await
+            .expect("construct AuthorizationManager");
+
+        manager.set_metadata(AuthorizationMetadata {
+            authorization_endpoint: "https://auth.example.test/authorize".to_string(),
+            token_endpoint: "https://auth.example.test/token".to_string(),
+            response_types_supported: Some(vec!["code".to_string()]),
+            code_challenge_methods_supported: Some(vec!["S256".to_string()]),
+            ..Default::default()
+        });
+        manager
+            .configure_client_id("test-client")
+            .expect("configure client id");
+
+        let auth_url = manager
+            .get_authorization_url(&["openid"])
+            .await
+            .expect("generate authorization url");
+
+        let parsed = url::Url::parse(&auth_url).expect("authorize url should parse");
+        let resource_values: Vec<String> = parsed
+            .query_pairs()
+            .filter(|(k, _)| k == "resource")
+            .map(|(_, v)| v.into_owned())
+            .collect();
+
+        assert_eq!(
+            resource_values.len(),
+            1,
+            "authorize URL must carry exactly one RFC 8707 resource param, \
+             got {resource_values:?} in {auth_url}"
+        );
+        assert_eq!(resource_values[0], base_url);
+    }
+}

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;

tests/rust/tests/oauth/dcr.rs

@@ -71,11 +71,23 @@ fn test_external_https_rejected() {
 }
 
 #[test]
-fn test_mixed_valid_invalid_rejected() {
-    // One invalid URI should fail the whole validation
+fn test_mixed_valid_invalid_passes() {
+    // Invalid URIs are skipped rather than failing the whole registration, as long as
+    // at least one valid URI remains. Clients like Cursor send a mix of valid (custom
+    // scheme + loopback) and invalid (non-loopback https) URIs in a single DCR request.
     let uris = vec![
         "http://127.0.0.1:8080/callback".to_string(),
-        "https://evil.com/steal".to_string(), // invalid
+        "https://evil.com/steal".to_string(), // invalid, skipped
+    ];
+    assert!(validate_redirect_uris(&uris).is_ok());
+}
+
+#[test]
+fn test_all_invalid_rejected() {
+    // When every URI is invalid, zero valid URIs remain and registration must fail.
+    let uris = vec![
+        "https://evil.com/steal".to_string(),
+        "http://example.com/callback".to_string(),
     ];
     assert!(validate_redirect_uris(&uris).is_err());
 }