Skip to content

Commit 985e6a4

Browse files
committed
fix(oauth,services): port #152 — DCR redirect URI tolerance + clippy
Cherry-picks the two changes from upstream PR #152 (#152) since `feat/workspace-root-routing` (PR #151, this branch's base) reworked `dcr.rs` extensively and a direct merge would conflict. - `validate_redirect_uris` now skips invalid URIs with a warn log and only fails registration when zero valid URIs remain. Unblocks Cursor 3.4.20, which sends a mixed-validity list (`cursor://`, `https://www.cursor.com`, `http://localhost`) in a single DCR request. - `PrefixCacheService` uses `sort_by_key` for the `created_at` sort so `clippy::unnecessary_sort_by` doesn't fail `cargo clippy -D warnings`. Adds two tests covering the mixed-validity case and the all-invalid case. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent deab680 commit 985e6a4

2 files changed

Lines changed: 39 additions & 6 deletions

File tree

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

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ pub fn validate_redirect_uris(uris: &[String]) -> Result<(), DcrError> {
227227
));
228228
}
229229

230+
let mut valid_count = 0;
231+
230232
for uri in uris {
231233
let is_loopback = uri.starts_with("http://127.0.0.1")
232234
|| uri.starts_with("http://localhost")
@@ -237,20 +239,28 @@ pub fn validate_redirect_uris(uris: &[String]) -> Result<(), DcrError> {
237239
let is_custom_scheme = !uri.starts_with("http://") && !uri.starts_with("https://");
238240

239241
if !is_loopback && !is_custom_scheme {
242+
// Skip invalid URIs (e.g. https://www.cursor.com/agents/mcp/oauth/callback)
243+
// rather than rejecting the entire registration — clients like Cursor send a
244+
// mix of valid and invalid URIs and only ever use the valid ones in practice.
240245
warn!(
241-
"[DCR] Rejected redirect_uri: {} (must be loopback or custom scheme)",
246+
"[DCR] Skipping invalid redirect_uri: {} (must be loopback or custom scheme)",
242247
uri
243248
);
244-
return Err(DcrError::invalid_redirect_uri(
245-
"Redirect URI must be loopback (http://127.0.0.1 or http://localhost) \
246-
or a custom URL scheme (e.g., cursor://, vscode://)",
247-
));
249+
continue;
248250
}
249251

250252
debug!(
251253
"[DCR] Validated redirect_uri: {} (loopback={}, custom_scheme={})",
252254
uri, is_loopback, is_custom_scheme
253255
);
256+
valid_count += 1;
257+
}
258+
259+
if valid_count == 0 {
260+
return Err(DcrError::invalid_redirect_uri(
261+
"No valid redirect_uris provided — must include at least one loopback \
262+
(http://127.0.0.1 or http://localhost) or custom URL scheme (e.g., cursor://, vscode://)",
263+
));
254264
}
255265

256266
Ok(())
@@ -462,6 +472,29 @@ mod tests {
462472
assert!(validate_redirect_uris(&["https://example.com/callback".to_string()]).is_err());
463473
}
464474

475+
#[test]
476+
fn test_mixed_valid_and_invalid_uris_pass() {
477+
// Real-world case: Cursor sends a mix of valid (custom scheme + loopback) and
478+
// invalid (https) URIs. Registration must succeed as long as at least one valid
479+
// URI is present — otherwise clients that send any non-loopback HTTPS URI cannot
480+
// register at all.
481+
let uris = vec![
482+
"cursor://anysphere.cursor-mcp/oauth/callback".to_string(),
483+
"https://www.cursor.com/agents/mcp/oauth/callback".to_string(),
484+
"http://localhost:8787/callback".to_string(),
485+
];
486+
assert!(validate_redirect_uris(&uris).is_ok());
487+
}
488+
489+
#[test]
490+
fn test_all_invalid_uris_fail() {
491+
let uris = vec![
492+
"https://www.cursor.com/agents/mcp/oauth/callback".to_string(),
493+
"http://example.com/callback".to_string(),
494+
];
495+
assert!(validate_redirect_uris(&uris).is_err());
496+
}
497+
465498
#[test]
466499
fn loopback_ignores_port_per_rfc_8252() {
467500
// Registered with one port, requested with another — must match.

crates/mcpmux-gateway/src/services/prefix_cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ impl PrefixCacheService {
138138

139139
// Sort by created_at (earliest first)
140140
// TODO: Add verified status priority when registry supports it
141-
servers.sort_by(|a, b| a.created_at.cmp(&b.created_at));
141+
servers.sort_by_key(|a| a.created_at);
142142

143143
// Clear existing cache for this space
144144
self.clear_space(space_id).await;

0 commit comments

Comments
 (0)