From 00ca5573406fe77c365b263ebf4ffadeebbc30c4 Mon Sep 17 00:00:00 2001 From: Maurus Decimus <11444311+mdecimus@users.noreply.github.com> Date: Sat, 4 Jul 2026 07:39:16 +0200 Subject: [PATCH] Fix OIDC: Allow ports in `redirect_uri` for loopback addresses --- crates/common/src/auth/oauth/registration.rs | 35 +++++++++++++++++--- crates/http/src/auth/oauth/auth.rs | 22 +----------- crates/http/src/auth/oauth/registration.rs | 9 +++-- tests/src/system/oidc.rs | 29 ++++++++++------ 4 files changed, 57 insertions(+), 38 deletions(-) diff --git a/crates/common/src/auth/oauth/registration.rs b/crates/common/src/auth/oauth/registration.rs index 6e4b5be3..56d7711c 100644 --- a/crates/common/src/auth/oauth/registration.rs +++ b/crates/common/src/auth/oauth/registration.rs @@ -207,6 +207,36 @@ impl ClientRegistrationError { } } +pub fn loopback_redirect_parts(uri: &str) -> Option<(&str, &str)> { + let uri = uri.strip_prefix("http://")?; + + for host in ["127.0.0.1", "[::1]"] { + if let Some(rest) = uri.strip_prefix(host) { + if let Some(path) = rest.strip_prefix('/') { + return Some((host, path)); + } else if let Some(after_colon) = rest.strip_prefix(':') + && let Some((port, path)) = after_colon.split_once('/') + && !port.is_empty() + && port.bytes().all(|b| b.is_ascii_digit()) + { + return Some((host, path)); + } + } + } + None +} + +pub fn redirect_uri_matches(registered: &str, presented: &str) -> bool { + registered == presented + || matches!( + ( + loopback_redirect_parts(registered), + loopback_redirect_parts(presented), + ), + (Some(reg), Some(pres)) if reg == pres + ) +} + pub fn validate_redirect_uri(uri: &str) -> Result<(), ClientRegistrationError> { if uri.contains('#') { return Err(ClientRegistrationError::invalid_redirect_uri( @@ -216,10 +246,7 @@ pub fn validate_redirect_uri(uri: &str) -> Result<(), ClientRegistrationError> { return Err(ClientRegistrationError::invalid_redirect_uri( "Redirect URI must not contain consecutive dots.", )); - } else if uri.starts_with("http://127.0.0.1/") - || uri.starts_with("http://[::1]/") - || uri.starts_with("https://") - { + } else if uri.starts_with("https://") || loopback_redirect_parts(uri).is_some() { return Ok(()); } else if let Some((scheme, _)) = uri.split_once(':') && scheme.contains('.') diff --git a/crates/http/src/auth/oauth/auth.rs b/crates/http/src/auth/oauth/auth.rs index b84d26aa..4f0e58bc 100644 --- a/crates/http/src/auth/oauth/auth.rs +++ b/crates/http/src/auth/oauth/auth.rs @@ -16,6 +16,7 @@ use common::{ CLIENT_ID_MAX_LEN, DEVICE_CODE_LEN, SUPPORTED_SCOPES, USER_CODE_ALPHABET, USER_CODE_LEN, client_id::{decode_client_id, scopes_to_mask}, + registration::redirect_uri_matches, }, }, }; @@ -569,27 +570,6 @@ impl OAuthApiHandler for Server { } } -fn redirect_uri_matches(registered: &str, presented: &str) -> bool { - registered == presented || loopback_redirect_matches(registered, presented) -} - -fn loopback_redirect_matches(registered: &str, presented: &str) -> bool { - for host in ["http://127.0.0.1", "http://[::1]"] { - if let (Some(reg_path), Some(pres_rest)) = - (registered.strip_prefix(host), presented.strip_prefix(host)) - && let Some(after_port) = pres_rest.strip_prefix(':') - && let Some(slash) = after_port.find('/') - { - let (port, pres_path) = after_port.split_at(slash); - if !port.is_empty() && port.bytes().all(|b| b.is_ascii_digit()) && pres_path == reg_path - { - return true; - } - } - } - false -} - fn grant_scope(requested: Option<&str>, registered_mask: u64) -> Option { let mut granted = String::new(); for scope in requested.unwrap_or_default().split_ascii_whitespace() { diff --git a/crates/http/src/auth/oauth/registration.rs b/crates/http/src/auth/oauth/registration.rs index 72e4a10f..33e4945b 100644 --- a/crates/http/src/auth/oauth/registration.rs +++ b/crates/http/src/auth/oauth/registration.rs @@ -14,7 +14,8 @@ use common::{ client_id::{ClientMeta, decode_client_id, encode_client_id, scopes_to_mask}, registration::{ ClientRegistrationError, ClientRegistrationRequest, ClientRegistrationResponse, - TokenEndpointAuthMethod, validate_grant_metadata, validate_redirect_uri, + TokenEndpointAuthMethod, redirect_uri_matches, validate_grant_metadata, + validate_redirect_uri, }, }, }, @@ -247,7 +248,11 @@ impl ClientRegistrationHandler for Server { .caused_by(trc::location!()) .ctx(trc::Key::Id, client_id.id().id()) })?; - if client.redirect_uris.iter().any(|uri| uri == redirect_uri) { + if client + .redirect_uris + .iter() + .any(|uri| redirect_uri_matches(uri, redirect_uri)) + { return Ok(None); } } else { diff --git a/tests/src/system/oidc.rs b/tests/src/system/oidc.rs index e34b739b..336c366f 100644 --- a/tests/src/system/oidc.rs +++ b/tests/src/system/oidc.rs @@ -240,17 +240,24 @@ pub async fn test(test: &mut TestServer) { assert_eq!(body["error"], "invalid_redirect_uri", "for {bad_uri}"); } - // A loopback redirect URI is accepted and registration returns 201 Created - let (status, _) = post_json_raw( - &metadata.registration_endpoint, - &ClientRegistrationRequest { - redirect_uris: vec!["http://127.0.0.1/cb".to_string()], - scope: Some(PROFILE_SCOPE.to_string()), - ..Default::default() - }, - ) - .await; - assert_eq!(status, 201, "registration should return 201 Created"); + // A loopback redirect URI is accepted and registration returns 201 Created, + // including loopback URIs that specify an ephemeral port (RFC 8252 ยง7.3). + for good_uri in [ + "http://127.0.0.1/cb", + "http://127.0.0.1:54321/cb", + "http://[::1]:8080/cb", + ] { + let (status, body) = post_json_raw( + &metadata.registration_endpoint, + &ClientRegistrationRequest { + redirect_uris: vec![good_uri.to_string()], + scope: Some(PROFILE_SCOPE.to_string()), + ..Default::default() + }, + ) + .await; + assert_eq!(status, 201, "registration should return 201 for {good_uri}: {body}"); + } // Register the client used for the flow with a private-use scheme redirect URI let registration: ClientRegistrationResponse = post_json(