From 60d418118fdd5a680e836458a6df38d804ca5214 Mon Sep 17 00:00:00 2001 From: jeidnx Date: Sun, 8 Feb 2026 11:13:36 +0100 Subject: [PATCH] fix: url calculation in make_url --- src/core/config/mod.rs | 8 +++++--- src/service/oauth/providers.rs | 37 ++++++++++++++++++---------------- tuwunel-example.toml | 8 +++++--- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/core/config/mod.rs b/src/core/config/mod.rs index 2485f05a..612b0fdf 100644 --- a/src/core/config/mod.rs +++ b/src/core/config/mod.rs @@ -2698,9 +2698,11 @@ pub struct IdentityProvider { pub userid_claims: BTreeSet, /// Optional extra path components after the issuer_url leading to the - /// location of the `.well-known` directory used for discovery. This will be - /// empty for specification-compliant providers. We have supplied any known - /// values based on `brand` (e.g. `/login/oauth` for GitHub). + /// location of the `.well-known` directory used for discovery. If the path + /// starts with a slash it will be treated as absolute, meaning overwriting + /// any path in the issuer_url. The path needs to end with a slash. This + /// will be empty for specification-compliant providers. We have supplied + /// any known values based on `brand` (e.g. `login/oauth/` for GitHub). pub base_path: Option, /// Overrides the `.well-known` location where the provider's openid diff --git a/src/service/oauth/providers.rs b/src/service/oauth/providers.rs index 21a1556c..1d0de18d 100644 --- a/src/service/oauth/providers.rs +++ b/src/service/oauth/providers.rs @@ -136,7 +136,7 @@ async fn configure(&self, mut provider: Provider) -> Result { if provider.base_path.is_none() { provider.base_path = match provider.brand.as_str() { - | "github" => Some("/login/oauth".to_owned()), + | "github" => Some("login/oauth/".to_owned()), | _ => None, }; } @@ -157,7 +157,7 @@ async fn configure(&self, mut provider: Provider) -> Result { .and_then(JsonValue::as_str) .map(Url::parse) .transpose()? - .or_else(|| make_url(&provider, "/authorize").ok()) + .or_else(|| make_url(&provider, "authorize").ok()) .map(|url| provider.authorization_url.replace(url)); } @@ -167,7 +167,7 @@ async fn configure(&self, mut provider: Provider) -> Result { .and_then(JsonValue::as_str) .map(Url::parse) .transpose()? - .or_else(|| make_url(&provider, "/revocation").ok()) + .or_else(|| make_url(&provider, "revocation").ok()) .map(|url| provider.revocation_url.replace(url)); } @@ -177,7 +177,7 @@ async fn configure(&self, mut provider: Provider) -> Result { .and_then(JsonValue::as_str) .map(Url::parse) .transpose()? - .or_else(|| make_url(&provider, "/introspection").ok()) + .or_else(|| make_url(&provider, "introspection").ok()) .map(|url| provider.introspection_url.replace(url)); } @@ -189,7 +189,7 @@ async fn configure(&self, mut provider: Provider) -> Result { .transpose()? .or_else(|| match provider.brand.as_str() { | "github" => "https://api.github.com/user".try_into().ok(), - | _ => make_url(&provider, "/userinfo").ok(), + | _ => make_url(&provider, "userinfo").ok(), }) .map(|url| provider.userinfo_url.replace(url)); } @@ -202,9 +202,9 @@ async fn configure(&self, mut provider: Provider) -> Result { .transpose()? .or_else(|| { let path = if provider.brand == "github" { - "/access_token" + "access_token" } else { - "/token" + "token" }; make_url(&provider, path).ok() @@ -237,7 +237,7 @@ pub async fn discover(&self, provider: &Provider) -> Result { fn discovery_url(provider: &Provider) -> Result { let default_url = provider .discovery - .then(|| make_url(provider, "/.well-known/openid-configuration")) + .then(|| make_url(provider, ".well-known/openid-configuration")) .transpose()?; let Some(url) = provider @@ -288,14 +288,17 @@ fn make_url(provider: &Provider, path: &str) -> Result { let mut suffix = provider.base_path.clone().unwrap_or_default(); suffix.push_str(path); - let url = provider - .issuer_url - .as_ref() - .ok_or_else(|| { - let id = &provider.client_id; - err!(Config("issuer_url", "Provider {id:?} required field")) - })? - .join(&suffix)?; + let issuer = provider.issuer_url.as_ref().ok_or_else(|| { + let id = &provider.client_id; + err!(Config("issuer_url", "Provider {id:?} required field")) + })?; + let issuer_path = issuer.path(); - Ok(url) + if issuer_path.ends_with('/') { + Ok(issuer.join(suffix.as_str())?) + } else { + let mut url = issuer.to_owned(); + url.set_path((issuer_path.to_owned() + "/").as_str()); + Ok(url.join(&suffix)?) + } } diff --git a/tuwunel-example.toml b/tuwunel-example.toml index ec096900..6cffba53 100644 --- a/tuwunel-example.toml +++ b/tuwunel-example.toml @@ -2307,9 +2307,11 @@ #userid_claims = [] # Optional extra path components after the issuer_url leading to the -# location of the `.well-known` directory used for discovery. This will be -# empty for specification-compliant providers. We have supplied any known -# values based on `brand` (e.g. `/login/oauth` for GitHub). +# location of the `.well-known` directory used for discovery. If the path +# starts with a slash it will be treated as absolute, meaning overwriting +# any path in the issuer_url. The path needs to end with a slash. This +# will be empty for specification-compliant providers. We have supplied +# any known values based on `brand` (e.g. `login/oauth/` for GitHub). # #base_path =