From 2784eec60a1b8d9af871489eb040ee293eb873ae Mon Sep 17 00:00:00 2001
From: lafleur <lafleur@boum.org>
Date: Sat, 9 Aug 2025 01:29:43 +0200
Subject: [PATCH] fix oxide-auth's redirect_uri comparison

oxide-auth's `RegisteredUrl::IgnorePortOnLocalhost` doesn't work when the host is 127.0.0.1 or [::1].
This commit lets the authentication process translate the host. The new registrar already supports this.
---
 src/api/client/oidc/register.rs | 18 +++++++++++++-----
 src/service/oidc/registrar.rs   | 24 ++++++++++++++----------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/api/client/oidc/register.rs b/src/api/client/oidc/register.rs
index d39b6414..f920bf63 100644
--- a/src/api/client/oidc/register.rs
+++ b/src/api/client/oidc/register.rs
@@ -3,6 +3,7 @@ use conduwuit::{Result, err};
 use oxide_auth::primitives::prelude::Client;
 use reqwest::Url;
 use ruma::{DeviceId, identifiers_validation};
+use conduwuit_service::oidc::registrar::normalize_redirect;
 
 /// The required parameters to register a new client for OAuth2 application.
 #[derive(serde::Deserialize, Clone, Debug)]
@@ -61,11 +62,18 @@ pub(crate) async fn register_client(
 	Json(client): Json<ClientQuery>,
 ) -> Result<Json<ClientResponse>> {
 	tracing::trace!("processing OIDC device register request for client: {client:#?}");
-	let Some(redirect_uri) = client.redirect_uris.first().cloned() else {
-		return Err(err!(Request(Unknown(
-			"register request should contain at least a redirect_uri"
-		))));
-	};
+	if client.redirect_uris.len() == 0 {
+ 		return Err(err!(Request(Unknown(
+ 			"the client's registration request should contain at least a redirect_uri"
+ 		))));
+	}
+	let mut redirect_uris = client.redirect_uris.clone();
+	let redirect_uri = redirect_uris.pop().expect("at least one redirect_uri");
+	let redirect_uri = normalize_redirect(redirect_uri);
+	let remaining_uris = redirect_uris
+		.into_iter()
+		.map(|url| normalize_redirect(url))
+		.collect();
 	let device_id = DeviceId::new();
 	let scope = format!(
 		"urn:matrix:org.matrix.msc2967.client:api:* \
diff --git a/src/service/oidc/registrar.rs b/src/service/oidc/registrar.rs
index 893238de..2c4c98a1 100644
--- a/src/service/oidc/registrar.rs
+++ b/src/service/oidc/registrar.rs
@@ -20,8 +20,9 @@ pub fn normalize_redirect_hostname(url: Url) -> Url {
 	new_url
 }
 
-/// The redirect_uri has to be wrapped in an IgnorePortOnLocalhost for oxide-auth
-/// to ignore the port when comparing it with the registered ones.
+/// If `url` is a localhost (either 'localhost', '127.0.0.1' or '[::1]'), wrap it in an
+/// IgnorePortOnLocalhost, so that oxide-auth ignores the port when comparing it with the
+/// registered ones.
 pub fn normalize_redirect(url: Url) -> RegisteredUrl {
 	let new_url = normalize_redirect_hostname(url);
 
@@ -105,11 +106,13 @@ impl Registrar for ClientMap {
             Some(stored) => stored,
         };
 
-        // Perform exact matching as motivated in the rfc, just substitute "127.0.0.1" and
-		// "[::1]" for "localhost".
-		let redirect_uri = bound.redirect_uri
+        // Perform exact matching as motivated in the rfc, but substitute "127.0.0.1" and
+		// "[::1]" for "localhost" to let oxide-auth ignore their port.
+		let redirect_uri = bound.redirect_uri;
+		let normalized_uri = redirect_uri
+			.clone()
 			.map(|u| normalize_redirect(u.to_url()));
-        let registered_url = match redirect_uri {
+        let registered_url = match normalized_uri {
             None => client.redirect_uri.clone(),
             Some(url) => {
                 let original = std::iter::once(&client.redirect_uri);
@@ -118,7 +121,8 @@ impl Registrar for ClientMap {
                     .chain(alternatives)
                     .any(|registered| *registered == url)
                 {
-                    url.clone().into()
+					// If normalized_uri is Some(url), so is redirect_uri, so unwrap().
+                    redirect_uri.unwrap().into_owned().into()
                 } else {
 					tracing::debug!("the request's redirect url didn't match any registered. bound: {:?}, in client {:#?}", url, client);
                     return Err(RegistrarError::Unspecified);
@@ -153,9 +157,9 @@ impl Registrar for ClientMap {
 			.ok_or_else(|| {
 				tracing::debug!("this client is not registered yet: {client_id:?}.");
 				RegistrarError::Unspecified
-			}).and_then(|client| {
-                RegisteredClient::new(client, password_policy).check_authentication(passphrase)
-            })?;
+			}).and_then(|client|
+				RegisteredClient::new(client, password_policy).check_authentication(passphrase)
+			)?;
 
         Ok(())
     }