Fix OIDC: Allow ports in redirect_uri for loopback addresses

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/")
+    } else if uri.starts_with("https://") || loopback_redirect_parts(uri).is_some() {
-        || uri.starts_with("http://[::1]/")
-        || uri.starts_with("https://")
-    {
         return Ok(());
     } else if let Some((scheme, _)) = uri.split_once(':')
         && scheme.contains('.')

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<String> {
     let mut granted = String::new();
     for scope in requested.unwrap_or_default().split_ascii_whitespace() {

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 {

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
+    // A loopback redirect URI is accepted and registration returns 201 Created,
-    let (status, _) = post_json_raw(
+    // including loopback URIs that specify an ephemeral port (RFC 8252 §7.3).
-        &metadata.registration_endpoint,
+    for good_uri in [
-        &ClientRegistrationRequest {
+        "http://127.0.0.1/cb",
-            redirect_uris: vec!["http://127.0.0.1/cb".to_string()],
+        "http://127.0.0.1:54321/cb",
-            scope: Some(PROFILE_SCOPE.to_string()),
+        "http://[::1]:8080/cb",
-            ..Default::default()
+    ] {
-        },
+        let (status, body) = post_json_raw(
-    )
+            &metadata.registration_endpoint,
-    .await;
+            &ClientRegistrationRequest {
-    assert_eq!(status, 201, "registration should return 201 Created");
+                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(