diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 69ca8485f..0a99d1e78 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- Fixed URL handling related to OIDC and `rootPath` with and without trailing slashes. Also added a bunch of tests ([#910]). + +### Changed + +- BREAKING: Made `DEFAULT_OIDC_WELLKNOWN_PATH` private. Use `AuthenticationProvider::well_known_config_url` instead ([#910]). + +[#910]: https://github.com/stackabletech/operator-rs/pull/910 + ## [0.81.0] - 2024-11-05 ### Added @@ -12,7 +22,7 @@ All notable changes to this project will be documented in this file. ### Changed -- BREAKING: Split `ListenerClass.spec.preferred_address_type` into a new `PreferredAddressType` type. Use `resolve_preferred_address_type()` to access the `AddressType` as before. ([#903]) +- BREAKING: Split `ListenerClass.spec.preferred_address_type` into a new `PreferredAddressType` type. Use `resolve_preferred_address_type()` to access the `AddressType` as before ([#903]). [#903]: https://github.com/stackabletech/operator-rs/pull/903 diff --git a/crates/stackable-operator/src/commons/authentication/oidc.rs b/crates/stackable-operator/src/commons/authentication/oidc.rs index d707b5657..ed19eebdb 100644 --- a/crates/stackable-operator/src/commons/authentication/oidc.rs +++ b/crates/stackable-operator/src/commons/authentication/oidc.rs @@ -17,17 +17,19 @@ use crate::commons::{ pub type Result = std::result::Result; -pub const DEFAULT_OIDC_WELLKNOWN_PATH: &str = ".well-known/openid-configuration"; pub const CLIENT_ID_SECRET_KEY: &str = "clientId"; pub const CLIENT_SECRET_SECRET_KEY: &str = "clientSecret"; +/// Do *not* use this for [`Url::join`], as the leading slash will erase the existing path! +const DEFAULT_WELLKNOWN_OIDC_CONFIG_PATH: &str = "/.well-known/openid-configuration"; + #[derive(Debug, PartialEq, Snafu)] pub enum Error { #[snafu(display("failed to parse OIDC endpoint url"))] ParseOidcEndpointUrl { source: ParseError }, #[snafu(display( - "failed to set OIDC endpoint scheme '{scheme}' for endpoint url '{endpoint}'" + "failed to set OIDC endpoint scheme '{scheme}' for endpoint url \"{endpoint}\"" ))] SetOidcEndpointScheme { endpoint: Url, scheme: String }, } @@ -111,10 +113,10 @@ impl AuthenticationProvider { } } - /// Returns the OIDC endpoint [`Url`]. To append the default OIDC well-known - /// configuration path, use `url.join()`. This module provides the default - /// path at [`DEFAULT_OIDC_WELLKNOWN_PATH`]. - pub fn endpoint_url(&self) -> Result { + /// Returns the OIDC base [`Url`] without any path segments. + /// + /// The base url only contains the scheme, the host, and an optional port. + fn base_url(&self) -> Result { let mut url = Url::parse(&format!( "http://{host}:{port}", host = self.hostname.as_url_host(), @@ -132,7 +134,37 @@ impl AuthenticationProvider { })?; } - url.set_path(&self.root_path); + Ok(url) + } + + /// Returns the OIDC endpoint [`Url`] without a trailing slash. + /// + /// To retrieve the well-known OIDC configuration url, please use [`Self::well_known_config_url`]. + pub fn endpoint_url(&self) -> Result { + let mut url = self.base_url()?; + // Some tools can not cope with a trailing slash, so let's remove that + url.set_path(self.root_path.trim_end_matches('/')); + Ok(url) + } + + /// Returns the well-known OIDC configuration [`Url`] without a trailing slash. + /// + /// The returned url is a combination of [`Self::endpoint_url`] joined with + /// the well-known OIDC configuration path `DEFAULT_WELLKNOWN_OIDC_CONFIG_PATH`. + pub fn well_known_config_url(&self) -> Result { + let mut url = self.base_url()?; + + // Taken from https://docs.rs/url/latest/url/struct.Url.html#method.join: + // A trailing slash is significant. Without it, the last path component is considered to be + // a “file” name to be removed to get at the “directory” that is used as the base. + // + // Because of that behavior, we first need to make sure that the root path doesn't contain + // any trailing slashes to finally append the well-known config path to the url. The path + // already contains a prefixed slash. + let mut root_path_with_trailing_slash = self.root_path.trim_end_matches('/').to_string(); + root_path_with_trailing_slash.push_str(DEFAULT_WELLKNOWN_OIDC_CONFIG_PATH); + url.set_path(&root_path_with_trailing_slash); + Ok(url) } @@ -246,6 +278,8 @@ pub struct ClientAuthenticationOptions { #[cfg(test)] mod test { + use rstest::rstest; + use super::*; #[test] @@ -310,12 +344,8 @@ mod test { .unwrap(); assert_eq!( - oidc.endpoint_url() - .unwrap() - .join(DEFAULT_OIDC_WELLKNOWN_PATH) - .unwrap() - .as_str(), - "https://my.keycloak.server/.well-known/openid-configuration" + oidc.endpoint_url().unwrap().as_str(), + "https://my.keycloak.server/" ); } @@ -338,6 +368,70 @@ mod test { ); } + #[rstest] + #[case("/", "http://my.keycloak.server:1234/")] + #[case("/realms/sdp", "http://my.keycloak.server:1234/realms/sdp")] + #[case("/realms/sdp/", "http://my.keycloak.server:1234/realms/sdp")] + #[case("/realms/sdp//////", "http://my.keycloak.server:1234/realms/sdp")] + #[case( + "/realms/my/realm/with/slashes//////", + "http://my.keycloak.server:1234/realms/my/realm/with/slashes" + )] + fn root_path_endpoint_url(#[case] root_path: String, #[case] expected_endpoint_url: &str) { + let oidc = serde_yaml::from_str::(&format!( + " + hostname: my.keycloak.server + port: 1234 + rootPath: {root_path} + scopes: [openid] + principalClaim: preferred_username + " + )) + .unwrap(); + + assert_eq!(oidc.endpoint_url().unwrap().as_str(), expected_endpoint_url); + } + + #[rstest] + #[case("/", "https://my.keycloak.server/.well-known/openid-configuration")] + #[case( + "/realms/sdp", + "https://my.keycloak.server/realms/sdp/.well-known/openid-configuration" + )] + #[case( + "/realms/sdp/", + "https://my.keycloak.server/realms/sdp/.well-known/openid-configuration" + )] + #[case( + "/realms/sdp//////", + "https://my.keycloak.server/realms/sdp/.well-known/openid-configuration" + )] + #[case( + "/realms/my/realm/with/slashes//////", + "https://my.keycloak.server/realms/my/realm/with/slashes/.well-known/openid-configuration" + )] + fn root_path_well_known_url(#[case] root_path: String, #[case] expected_well_known_url: &str) { + let oidc = serde_yaml::from_str::(&format!( + " + hostname: my.keycloak.server + rootPath: {root_path} + scopes: [openid] + principalClaim: preferred_username + tls: + verification: + server: + caCert: + webPki: {{}} + " + )) + .unwrap(); + + assert_eq!( + oidc.well_known_config_url().unwrap().as_str(), + expected_well_known_url + ); + } + #[test] fn client_env_vars() { let secret_name = "my-keycloak-client";