From a5b7d91da8bf4f94bda948232e0fae78abf75c05 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Sat, 29 Mar 2025 17:23:58 +0100 Subject: [PATCH 01/30] wip - compiling --- rust/operator-binary/src/controller.rs | 21 ++++ .../src/crd/user_info_fetcher.rs | 34 ++++++- rust/user-info-fetcher/src/backend/entra.rs | 96 +++++++++++++++++++ rust/user-info-fetcher/src/backend/mod.rs | 1 + rust/user-info-fetcher/src/main.rs | 13 +++ 5 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 rust/user-info-fetcher/src/backend/entra.rs diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index b47ee8c6..a1b847fa 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1030,6 +1030,27 @@ fn build_server_rolegroup_daemonset( .add_volumes_and_mounts(&mut pb, vec![&mut cb_user_info_fetcher]) .context(UserInfoFetcherTlsVolumeAndMountsSnafu)?; } + user_info_fetcher::v1alpha1::Backend::Entra(entra) => { + pb.add_volume( + VolumeBuilder::new(USER_INFO_FETCHER_CREDENTIALS_VOLUME_NAME) + .secret(SecretVolumeSource { + secret_name: Some(entra.client_credentials_secret.clone()), + ..Default::default() + }) + .build(), + ) + .context(AddVolumeSnafu)?; + cb_user_info_fetcher + .add_volume_mount( + USER_INFO_FETCHER_CREDENTIALS_VOLUME_NAME, + USER_INFO_FETCHER_CREDENTIALS_DIR, + ) + .context(AddVolumeMountSnafu)?; + entra + .tls + .add_volumes_and_mounts(&mut pb, vec![&mut cb_user_info_fetcher]) + .context(UserInfoFetcherTlsVolumeAndMountsSnafu)?; + } } pb.add_container(cb_user_info_fetcher.build()); diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index d3a7d688..38b852f7 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::{collections::BTreeMap, str::FromStr}; use serde::{Deserialize, Serialize}; use stackable_operator::{ @@ -38,6 +38,10 @@ pub mod versioned { /// Backend that fetches user information from Active Directory #[serde(rename = "experimentalActiveDirectory")] ActiveDirectory(v1alpha1::ActiveDirectoryBackend), + + /// Backend that fetches user information from Microsoft Entra + #[serde(rename = "experimentalEntraBackend")] + Entra(v1alpha1::EntraBackend), } #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] @@ -110,6 +114,30 @@ pub mod versioned { pub additional_group_attribute_filters: BTreeMap, } + #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct EntraBackend { + /// Hostname of the identity provider, e.g. `login.microsoft.com`. + #[serde(default = "default_entra_host")] + pub hostname: HostName, + + /// Port of the identity provider. If TLS is used defaults to `443`, otherwise to `80`. + pub port: Option, + + /// Root HTTP path of the identity provider. Defaults to `/`. + #[serde(default = "default_root_path")] + pub tenant_id: String, + + /// Use a TLS connection. If not specified no TLS will be used. + #[serde(flatten)] + pub tls: TlsClientDetails, + + /// Name of a Secret that contains client credentials of a Entra account with permission to read user metadata. + /// + /// Must contain the fields `clientId` and `clientSecret`. + pub client_credentials_secret: String, + } + #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct Cache { @@ -129,6 +157,10 @@ fn default_root_path() -> String { "/".to_string() } +fn default_entra_host() -> HostName { + HostName::from_str("login.microsoft.com").unwrap() +} + fn aas_default_port() -> u16 { 5000 } diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs new file mode 100644 index 00000000..4a9c8e37 --- /dev/null +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -0,0 +1,96 @@ +use std::collections::HashMap; + +use hyper::StatusCode; +use serde::Deserialize; +use snafu::{OptionExt, ResultExt, Snafu}; +use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; +use stackable_operator::commons::authentication::oidc; + +use crate::{http_error, utils::http::send_json_request, Credentials, UserInfo, UserInfoRequest}; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to get access_token"))] + AccessToken { source: crate::utils::http::Error }, + + #[snafu(display("failed to search for user"))] + SearchForUser { source: crate::utils::http::Error }, + + #[snafu(display("unable to find user with id {user_id:?}"))] + UserNotFoundById { + source: crate::utils::http::Error, + user_id: String, + }, + + #[snafu(display("unable to find user with username {username:?}"))] + UserNotFoundByName { username: String }, + + #[snafu(display("more than one user was returned when there should be one or none"))] + TooManyUsersReturned, + + #[snafu(display( + "failed to request groups for user with username {username:?} (user_id: {user_id:?})" + ))] + RequestUserGroups { + source: crate::utils::http::Error, + username: String, + user_id: String, + }, + + #[snafu(display("failed to parse OIDC endpoint url"))] + ParseOidcEndpointUrl { source: oidc::Error }, + + #[snafu(display("failed to construct OIDC endpoint path"))] + ConstructOidcEndpointPath { source: url::ParseError }, +} + +impl http_error::Error for Error { + fn status_code(&self) -> StatusCode { + match self { + Self::AccessToken { .. } => StatusCode::BAD_GATEWAY, + Self::SearchForUser { .. } => StatusCode::BAD_GATEWAY, + Self::UserNotFoundById { .. } => StatusCode::NOT_FOUND, + Self::UserNotFoundByName { .. } => StatusCode::NOT_FOUND, + Self::TooManyUsersReturned {} => StatusCode::INTERNAL_SERVER_ERROR, + Self::RequestUserGroups { .. } => StatusCode::BAD_GATEWAY, + Self::ParseOidcEndpointUrl { .. } => StatusCode::INTERNAL_SERVER_ERROR, + Self::ConstructOidcEndpointPath { .. } => StatusCode::INTERNAL_SERVER_ERROR, + } + } +} + +#[derive(Deserialize)] +struct OAuthResponse { + access_token: String, +} + +/// The minimal structure of [UserRepresentation] that is returned by [`/users`][users] and [`/users/{id}`][user-by-id]. +///
Some fields, such as `groups` are never present. See [keycloak/keycloak#20292][issue-20292]
+/// +/// [users]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#_get_adminrealmsrealmusers +/// [user-by-id]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#_get_adminrealmsrealmusersid +/// [UserRepresentation]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#UserRepresentation +/// [issue-20292]: https://github.com/keycloak/keycloak/issues/20294 +#[derive(Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct UserMetadata { + id: String, + username: String, + #[serde(default)] + attributes: HashMap, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct GroupMembership { + path: String, +} + +pub(crate) async fn get_user_info( + req: &UserInfoRequest, + http: &reqwest::Client, + credentials: &Credentials, + config: &v1alpha1::EntraBackend, +) -> Result { + Ok(UserInfo::default()) +} diff --git a/rust/user-info-fetcher/src/backend/mod.rs b/rust/user-info-fetcher/src/backend/mod.rs index 4540c6bf..c9a32709 100644 --- a/rust/user-info-fetcher/src/backend/mod.rs +++ b/rust/user-info-fetcher/src/backend/mod.rs @@ -1,3 +1,4 @@ pub mod active_directory; +pub mod entra; pub mod keycloak; pub mod xfsc_aas; diff --git a/rust/user-info-fetcher/src/main.rs b/rust/user-info-fetcher/src/main.rs index e5a0a079..dc2969d8 100644 --- a/rust/user-info-fetcher/src/main.rs +++ b/rust/user-info-fetcher/src/main.rs @@ -122,6 +122,10 @@ async fn main() -> Result<(), StartupError> { client_id: "".to_string(), client_secret: "".to_string(), }, + v1alpha1::Backend::Entra(_) => Credentials { + client_id: read_config_file(&args.credentials_dir.join("clientId")).await?, + client_secret: read_config_file(&args.credentials_dir.join("clientSecret")).await?, + }, }); let mut client_builder = ClientBuilder::new(); @@ -231,6 +235,9 @@ enum GetUserInfoError { ActiveDirectory { source: backend::active_directory::Error, }, + + #[snafu(display("failed to get user information from Entra"))] + Entra { source: backend::entra::Error }, } impl http_error::Error for GetUserInfoError { @@ -245,6 +252,7 @@ impl http_error::Error for GetUserInfoError { Self::Keycloak { source } => source.status_code(), Self::ExperimentalXfscAas { source } => source.status_code(), Self::ActiveDirectory { source } => source.status_code(), + Self::Entra { source } => source.status_code(), } } } @@ -305,6 +313,11 @@ async fn get_user_info( .await .context(get_user_info_error::ActiveDirectorySnafu) } + v1alpha1::Backend::Entra(entra) => { + backend::entra::get_user_info(&req, &http, &credentials, entra) + .await + .context(get_user_info_error::EntraSnafu) + } } }) .await?, From aa254e4697472f0ab1e5b24a5069d1e5ab0cd52c Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 31 Mar 2025 22:19:10 +0200 Subject: [PATCH 02/30] wip --- rust/user-info-fetcher/src/backend/entra.rs | 75 ++++++++++++++++++++- 1 file changed, 72 insertions(+), 3 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 4a9c8e37..49e78704 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -75,7 +75,9 @@ struct OAuthResponse { #[serde(rename_all = "camelCase")] struct UserMetadata { id: String, - username: String, + //username: String, + mail: String, + display_name: String, #[serde(default)] attributes: HashMap, } @@ -83,7 +85,8 @@ struct UserMetadata { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct GroupMembership { - path: String, + id: String, + displayName: String, } pub(crate) async fn get_user_info( @@ -92,5 +95,71 @@ pub(crate) async fn get_user_info( credentials: &Credentials, config: &v1alpha1::EntraBackend, ) -> Result { - Ok(UserInfo::default()) + let v1alpha1::EntraBackend { + client_credentials_secret: _, + hostname, + port, + tenant_id, + tls, + } = config; + + // TODO: tls + let host_port = port.unwrap_or(443); + let token_url = format!("http://{hostname}:{host_port}/{tenant_id}/oauth2/v2.0/token"); + + // -H "Content-Type: application/x-www-form-urlencoded" \ + // -d "client_id=${CLIENT_ID}" \ + // -d "client_secret=${CLIENT_SECRET}" \ + // -d "scope=https://graph.microsoft.com/.default" \ + // -d "grant_type=client_credentials" | jq -r .access_token) + + let authn = send_json_request::(http.post(token_url).form(&[ + ("client_id", &credentials.client_id), + ("client_secret", &credentials.client_secret), + ("scope", &"https://graph.microsoft.com/.default".to_string()), + ("grant_type", &"client_credentials".to_string()), + ])) + .await + .context(AccessTokenSnafu)?; + + let tok = &authn.access_token; + tracing::warn!("Got token: {tok}"); + + let users_base_url = format!("http://{hostname}:{host_port}/v1.0/users"); + + let user_info = match req { + UserInfoRequest::UserInfoRequestById(req) => { + let user_id = req.id.clone(); + send_json_request::( + http.get(format!("{users_base_url}/{user_id}")) + .bearer_auth(&authn.access_token), + ) + .await + .context(UserNotFoundByIdSnafu { user_id })? + } + UserInfoRequest::UserInfoRequestByName(req) => { + let username = &req.username; + let users_url = format!("{users_base_url}/{username}"); + send_json_request::(http.get(users_url).bearer_auth(&authn.access_token)) + .await + .context(SearchForUserSnafu)? + } + }; + + let groups = send_json_request::>( + http.get(format!("{users_base_url}/{id}/memberOf", id = user_info.id)) + .bearer_auth(&authn.access_token), + ) + .await + .context(RequestUserGroupsSnafu { + username: user_info.display_name.clone(), + user_id: user_info.id.clone(), + })?; + + Ok(UserInfo { + id: Some(user_info.id), + username: Some(user_info.display_name), + groups: groups.into_iter().map(|g| g.displayName).collect(), + custom_attributes: user_info.attributes, + }) } From 6f38bb91d5d552c6db924dc427f32ebe93d6e25c Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 4 Apr 2025 11:03:25 +0200 Subject: [PATCH 03/30] wip --- deploy/helm/opa-operator/crds/crds.yaml | 78 ++++++++++++ .../src/crd/user_info_fetcher.rs | 25 ++-- rust/user-info-fetcher/src/backend/entra.rs | 116 +++++++++++------- rust/user-info-fetcher/src/main.rs | 4 + 4 files changed, 170 insertions(+), 53 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 95b0f804..0b3acc3b 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -64,6 +64,8 @@ spec: - experimentalXfscAas - required: - experimentalActiveDirectory + - required: + - experimentalEntraBackend properties: experimentalActiveDirectory: description: Backend that fetches user information from Active Directory @@ -137,6 +139,82 @@ spec: - kerberosSecretClassName - ldapServer type: object + experimentalEntraBackend: + description: Backend that fetches user information from Microsoft Entra + properties: + clientCredentialsSecret: + description: |- + Name of a Secret that contains client credentials of a Entra account with permission to read user metadata. + + Must contain the fields `clientId` and `clientSecret`. + type: string + hostname: + default: login.microsoft.com + description: Hostname of the identity provider, defaults to `login.microsoft.com`. + type: string + port: + default: 443 + description: Port of the identity provider. Defaults to 443. + format: uint16 + minimum: 0.0 + type: integer + tenantId: + description: The Microsoft Entra tenant ID. + type: string + tls: + default: + tls: + verification: + server: + caCert: + webPki: {} + description: Use a TLS connection. Should usually be set to WebPki. + properties: + tls: + description: Use a TLS connection. If not specified no TLS will be used. + nullable: true + properties: + verification: + description: The verification method used to verify the certificates of the server and/or the client. + oneOf: + - required: + - none + - required: + - server + properties: + none: + description: Use TLS but don't verify certificates. + type: object + server: + description: Use TLS and a CA certificate to verify the server. + properties: + caCert: + description: CA cert to verify the server. + oneOf: + - required: + - webPki + - required: + - secretClass + properties: + secretClass: + description: Name of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) which will provide the CA certificate. Note that a SecretClass does not need to have a key but can also work with just a CA certificate, so if you got provided with a CA cert but don't have access to the key you can still use this method. + type: string + webPki: + description: Use TLS and the CA certificates trusted by the common web browsers to verify the server. This can be useful when you e.g. use public AWS S3 or other public available services. + type: object + type: object + required: + - caCert + type: object + type: object + required: + - verification + type: object + type: object + required: + - clientCredentialsSecret + - tenantId + type: object experimentalXfscAas: description: Backend that fetches user information from the Gaia-X Cross Federation Services Components (XFSC) Authentication & Authorization Service. properties: diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 38b852f7..6f71c163 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -2,7 +2,10 @@ use std::{collections::BTreeMap, str::FromStr}; use serde::{Deserialize, Serialize}; use stackable_operator::{ - commons::{networking::HostName, tls_verification::TlsClientDetails}, + commons::{ + networking::HostName, + tls_verification::{CaCert, Tls, TlsClientDetails, TlsServerVerification, TlsVerification}, + }, schemars::{self, JsonSchema}, time::Duration, }; @@ -117,18 +120,18 @@ pub mod versioned { #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct EntraBackend { - /// Hostname of the identity provider, e.g. `login.microsoft.com`. - #[serde(default = "default_entra_host")] + /// Hostname of the identity provider, defaults to `login.microsoft.com`. + #[serde(default = "entra_default_host")] pub hostname: HostName, - /// Port of the identity provider. If TLS is used defaults to `443`, otherwise to `80`. - pub port: Option, + /// Port of the identity provider. Defaults to 443. + #[serde(default = "entra_default_port")] + pub port: u16, - /// Root HTTP path of the identity provider. Defaults to `/`. - #[serde(default = "default_root_path")] + /// The Microsoft Entra tenant ID. pub tenant_id: String, - /// Use a TLS connection. If not specified no TLS will be used. + /// Use a TLS connection. Should usually be set to WebPki. #[serde(flatten)] pub tls: TlsClientDetails, @@ -157,10 +160,14 @@ fn default_root_path() -> String { "/".to_string() } -fn default_entra_host() -> HostName { +fn entra_default_host() -> HostName { HostName::from_str("login.microsoft.com").unwrap() } +fn entra_default_port() -> u16 { + 443 +} + fn aas_default_port() -> u16 { 5000 } diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 49e78704..c4f593fd 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -2,9 +2,9 @@ use std::collections::HashMap; use hyper::StatusCode; use serde::Deserialize; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; -use stackable_operator::commons::authentication::oidc; +use stackable_operator::commons::{networking::HostName, tls_verification::TlsClientDetails}; use crate::{http_error, utils::http::send_json_request, Credentials, UserInfo, UserInfoRequest}; @@ -22,12 +22,6 @@ pub enum Error { user_id: String, }, - #[snafu(display("unable to find user with username {username:?}"))] - UserNotFoundByName { username: String }, - - #[snafu(display("more than one user was returned when there should be one or none"))] - TooManyUsersReturned, - #[snafu(display( "failed to request groups for user with username {username:?} (user_id: {user_id:?})" ))] @@ -36,12 +30,6 @@ pub enum Error { username: String, user_id: String, }, - - #[snafu(display("failed to parse OIDC endpoint url"))] - ParseOidcEndpointUrl { source: oidc::Error }, - - #[snafu(display("failed to construct OIDC endpoint path"))] - ConstructOidcEndpointPath { source: url::ParseError }, } impl http_error::Error for Error { @@ -50,11 +38,7 @@ impl http_error::Error for Error { Self::AccessToken { .. } => StatusCode::BAD_GATEWAY, Self::SearchForUser { .. } => StatusCode::BAD_GATEWAY, Self::UserNotFoundById { .. } => StatusCode::NOT_FOUND, - Self::UserNotFoundByName { .. } => StatusCode::NOT_FOUND, - Self::TooManyUsersReturned {} => StatusCode::INTERNAL_SERVER_ERROR, Self::RequestUserGroups { .. } => StatusCode::BAD_GATEWAY, - Self::ParseOidcEndpointUrl { .. } => StatusCode::INTERNAL_SERVER_ERROR, - Self::ConstructOidcEndpointPath { .. } => StatusCode::INTERNAL_SERVER_ERROR, } } } @@ -86,7 +70,14 @@ struct UserMetadata { #[serde(rename_all = "camelCase")] struct GroupMembership { id: String, - displayName: String, + display_name: String, +} + +struct EntraEndpoint { + hostname: HostName, + port: u16, + tenant_id: String, + protocol: String, } pub(crate) async fn get_user_info( @@ -103,35 +94,23 @@ pub(crate) async fn get_user_info( tls, } = config; - // TODO: tls - let host_port = port.unwrap_or(443); - let token_url = format!("http://{hostname}:{host_port}/{tenant_id}/oauth2/v2.0/token"); - - // -H "Content-Type: application/x-www-form-urlencoded" \ - // -d "client_id=${CLIENT_ID}" \ - // -d "client_secret=${CLIENT_SECRET}" \ - // -d "scope=https://graph.microsoft.com/.default" \ - // -d "grant_type=client_credentials" | jq -r .access_token) + let entra_endpoint = EntraEndpoint::new(hostname.clone(), port.clone(), tenant_id.clone(), tls); + let token_url = entra_endpoint.oauth2_token(); let authn = send_json_request::(http.post(token_url).form(&[ - ("client_id", &credentials.client_id), - ("client_secret", &credentials.client_secret), - ("scope", &"https://graph.microsoft.com/.default".to_string()), - ("grant_type", &"client_credentials".to_string()), + ("client_id", credentials.client_id.as_str()), + ("client_secret", credentials.client_secret.as_str()), + ("scope", "https://graph.microsoft.com/.default"), + ("grant_type", "client_credentials"), ])) .await .context(AccessTokenSnafu)?; - let tok = &authn.access_token; - tracing::warn!("Got token: {tok}"); - - let users_base_url = format!("http://{hostname}:{host_port}/v1.0/users"); - let user_info = match req { UserInfoRequest::UserInfoRequestById(req) => { let user_id = req.id.clone(); send_json_request::( - http.get(format!("{users_base_url}/{user_id}")) + http.get(entra_endpoint.users(&user_id)) .bearer_auth(&authn.access_token), ) .await @@ -139,15 +118,17 @@ pub(crate) async fn get_user_info( } UserInfoRequest::UserInfoRequestByName(req) => { let username = &req.username; - let users_url = format!("{users_base_url}/{username}"); - send_json_request::(http.get(users_url).bearer_auth(&authn.access_token)) - .await - .context(SearchForUserSnafu)? + send_json_request::( + http.get(entra_endpoint.users(&username)) + .bearer_auth(&authn.access_token), + ) + .await + .context(SearchForUserSnafu)? } }; let groups = send_json_request::>( - http.get(format!("{users_base_url}/{id}/memberOf", id = user_info.id)) + http.get(entra_endpoint.member_of(&user_info.id)) .bearer_auth(&authn.access_token), ) .await @@ -159,7 +140,54 @@ pub(crate) async fn get_user_info( Ok(UserInfo { id: Some(user_info.id), username: Some(user_info.display_name), - groups: groups.into_iter().map(|g| g.displayName).collect(), + groups: groups.into_iter().map(|g| g.display_name).collect(), custom_attributes: user_info.attributes, }) } + +impl EntraEndpoint { + pub fn new(hostname: HostName, port: u16, tenant_id: String, tls: &TlsClientDetails) -> Self { + Self { + hostname, + port, + tenant_id, + protocol: if tls.uses_tls() { + "https".to_string() + } else { + "http".to_string() + }, + } + } + + pub fn oauth2_token(&self) -> String { + format!( + "{base_url}/{tenant_id}/oauth2/v2.0/token", + base_url = self.base_url(), + tenant_id = self.tenant_id + ) + } + + pub fn users(&self, user: &str) -> String { + format!("{base_url}/v1.0/users/{user}", base_url = self.base_url()) + } + + pub fn member_of(&self, user: &str) -> String { + format!( + "{base_url}/v1.0/users/{user}/memberOf", + base_url = self.base_url() + ) + } + + fn base_url(&self) -> String { + format!( + "{protocol}://{hostname}{opt_port}", + opt_port = if self.port == 443 || self.port == 80 { + "".to_string() + } else { + format!(":{port}", port = self.port) + }, + hostname = self.hostname, + protocol = self.protocol + ) + } +} diff --git a/rust/user-info-fetcher/src/main.rs b/rust/user-info-fetcher/src/main.rs index dc2969d8..75667930 100644 --- a/rust/user-info-fetcher/src/main.rs +++ b/rust/user-info-fetcher/src/main.rs @@ -138,6 +138,10 @@ async fn main() -> Result<(), StartupError> { client_builder = utils::tls::configure_reqwest(&keycloak.tls, client_builder) .await .context(ConfigureTlsSnafu)?; + } else if let v1alpha1::Backend::Entra(entra) = &config.backend { + client_builder = utils::tls::configure_reqwest(&entra.tls, client_builder) + .await + .context(ConfigureTlsSnafu)?; } let http = client_builder.build().context(ConstructHttpClientSnafu)?; From fc7b35ab143a9253d22401526cde2617a70e2787 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Sun, 13 Apr 2025 19:10:47 +0200 Subject: [PATCH 04/30] cleanup & test --- .../src/crd/user_info_fetcher.rs | 5 +- rust/user-info-fetcher/src/backend/entra.rs | 96 ++++++++++++++++--- 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 6f71c163..03a7ee9d 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -2,10 +2,7 @@ use std::{collections::BTreeMap, str::FromStr}; use serde::{Deserialize, Serialize}; use stackable_operator::{ - commons::{ - networking::HostName, - tls_verification::{CaCert, Tls, TlsClientDetails, TlsServerVerification, TlsVerification}, - }, + commons::{networking::HostName, tls_verification::TlsClientDetails}, schemars::{self, JsonSchema}, time::Duration, }; diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index c4f593fd..64269938 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -59,9 +59,7 @@ struct OAuthResponse { #[serde(rename_all = "camelCase")] struct UserMetadata { id: String, - //username: String, - mail: String, - display_name: String, + user_principal_name: String, #[serde(default)] attributes: HashMap, } @@ -69,7 +67,6 @@ struct UserMetadata { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct GroupMembership { - id: String, display_name: String, } @@ -110,7 +107,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestById(req) => { let user_id = req.id.clone(); send_json_request::( - http.get(entra_endpoint.users(&user_id)) + http.get(entra_endpoint.user_info(&user_id)) .bearer_auth(&authn.access_token), ) .await @@ -119,7 +116,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestByName(req) => { let username = &req.username; send_json_request::( - http.get(entra_endpoint.users(&username)) + http.get(entra_endpoint.user_info(&username)) .bearer_auth(&authn.access_token), ) .await @@ -128,18 +125,18 @@ pub(crate) async fn get_user_info( }; let groups = send_json_request::>( - http.get(entra_endpoint.member_of(&user_info.id)) + http.get(entra_endpoint.group_info(&user_info.id)) .bearer_auth(&authn.access_token), ) .await .context(RequestUserGroupsSnafu { - username: user_info.display_name.clone(), + username: user_info.user_principal_name.clone(), user_id: user_info.id.clone(), })?; Ok(UserInfo { id: Some(user_info.id), - username: Some(user_info.display_name), + username: Some(user_info.user_principal_name), groups: groups.into_iter().map(|g| g.display_name).collect(), custom_attributes: user_info.attributes, }) @@ -167,11 +164,12 @@ impl EntraEndpoint { ) } - pub fn users(&self, user: &str) -> String { + // Works both with id/oid and userPrincipalName + pub fn user_info(&self, user: &str) -> String { format!("{base_url}/v1.0/users/{user}", base_url = self.base_url()) } - pub fn member_of(&self, user: &str) -> String { + pub fn group_info(&self, user: &str) -> String { format!( "{base_url}/v1.0/users/{user}/memberOf", base_url = self.base_url() @@ -191,3 +189,79 @@ impl EntraEndpoint { ) } } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use stackable_operator::commons::tls_verification::{ + CaCert, Tls, TlsServerVerification, TlsVerification, + }; + + use super::*; + + #[test] + fn test_defaults() { + let entra_endpoint = EntraEndpoint::new( + HostName::from_str("login.microsoft.com").expect("Could not parse hostname"), + 443, + "1234-5678".to_string(), + &TlsClientDetails { + tls: Some(Tls { + verification: TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::WebPki {}, + }), + }), + }, + ); + + assert_eq!( + entra_endpoint.oauth2_token(), + "https://login.microsoft.com/1234-5678/oauth2/v2.0/token" + ); + assert_eq!( + entra_endpoint.user_info("0000-0000"), + "https://login.microsoft.com/v1.0/users/0000-0000" + ); + assert_eq!( + entra_endpoint.group_info("0000-0000"), + "https://login.microsoft.com/v1.0/users/0000-0000/memberOf" + ); + } + + #[test] + fn test_non_defaults_tls() { + let entra_endpoint = EntraEndpoint::new( + HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + 8443, + "1234-5678".to_string(), + &TlsClientDetails { + tls: Some(Tls { + verification: TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::WebPki {}, + }), + }), + }, + ); + + assert_eq!( + entra_endpoint.oauth2_token(), + "https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token" + ); + } + + #[test] + fn test_non_defaults_non_tls() { + let entra_endpoint = EntraEndpoint::new( + HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + 8080, + "1234-5678".to_string(), + &TlsClientDetails { tls: None }, + ); + + assert_eq!( + entra_endpoint.oauth2_token(), + "http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token" + ); + } +} From 261df99551a329f9df16c4d5bca53e4269c968ca Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 08:55:30 +0200 Subject: [PATCH 05/30] remove comments --- rust/user-info-fetcher/src/backend/entra.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 64269938..ac091929 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -48,13 +48,6 @@ struct OAuthResponse { access_token: String, } -/// The minimal structure of [UserRepresentation] that is returned by [`/users`][users] and [`/users/{id}`][user-by-id]. -///
Some fields, such as `groups` are never present. See [keycloak/keycloak#20292][issue-20292]
-/// -/// [users]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#_get_adminrealmsrealmusers -/// [user-by-id]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#_get_adminrealmsrealmusersid -/// [UserRepresentation]: https://www.keycloak.org/docs-api/22.0.1/rest-api/index.html#UserRepresentation -/// [issue-20292]: https://github.com/keycloak/keycloak/issues/20294 #[derive(Clone, Deserialize)] #[serde(rename_all = "camelCase")] struct UserMetadata { @@ -91,7 +84,7 @@ pub(crate) async fn get_user_info( tls, } = config; - let entra_endpoint = EntraEndpoint::new(hostname.clone(), port.clone(), tenant_id.clone(), tls); + let entra_endpoint = EntraEndpoint::new(hostname.clone(), *port, tenant_id.clone(), tls); let token_url = entra_endpoint.oauth2_token(); let authn = send_json_request::(http.post(token_url).form(&[ @@ -116,7 +109,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestByName(req) => { let username = &req.username; send_json_request::( - http.get(entra_endpoint.user_info(&username)) + http.get(entra_endpoint.user_info(username)) .bearer_auth(&authn.access_token), ) .await From 41d7eec67656bfba9b4a904e31c8d1e4c7979dfe Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 08:55:43 +0200 Subject: [PATCH 06/30] adapt entra name --- deploy/helm/opa-operator/crds/crds.yaml | 74 ++++++++----------- .../src/crd/user_info_fetcher.rs | 2 +- 2 files changed, 33 insertions(+), 43 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 0b3acc3b..92ca1d0d 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -65,7 +65,7 @@ spec: - required: - experimentalActiveDirectory - required: - - experimentalEntraBackend + - experimentalEntra properties: experimentalActiveDirectory: description: Backend that fetches user information from Active Directory @@ -139,7 +139,7 @@ spec: - kerberosSecretClassName - ldapServer type: object - experimentalEntraBackend: + experimentalEntra: description: Backend that fetches user information from Microsoft Entra properties: clientCredentialsSecret: @@ -162,54 +162,44 @@ spec: description: The Microsoft Entra tenant ID. type: string tls: - default: - tls: - verification: - server: - caCert: - webPki: {} - description: Use a TLS connection. Should usually be set to WebPki. + description: Use a TLS connection. If not specified no TLS will be used. + nullable: true properties: - tls: - description: Use a TLS connection. If not specified no TLS will be used. - nullable: true + verification: + description: The verification method used to verify the certificates of the server and/or the client. + oneOf: + - required: + - none + - required: + - server properties: - verification: - description: The verification method used to verify the certificates of the server and/or the client. - oneOf: - - required: - - none - - required: - - server + none: + description: Use TLS but don't verify certificates. + type: object + server: + description: Use TLS and a CA certificate to verify the server. properties: - none: - description: Use TLS but don't verify certificates. - type: object - server: - description: Use TLS and a CA certificate to verify the server. + caCert: + description: CA cert to verify the server. + oneOf: + - required: + - webPki + - required: + - secretClass properties: - caCert: - description: CA cert to verify the server. - oneOf: - - required: - - webPki - - required: - - secretClass - properties: - secretClass: - description: Name of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) which will provide the CA certificate. Note that a SecretClass does not need to have a key but can also work with just a CA certificate, so if you got provided with a CA cert but don't have access to the key you can still use this method. - type: string - webPki: - description: Use TLS and the CA certificates trusted by the common web browsers to verify the server. This can be useful when you e.g. use public AWS S3 or other public available services. - type: object + secretClass: + description: Name of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) which will provide the CA certificate. Note that a SecretClass does not need to have a key but can also work with just a CA certificate, so if you got provided with a CA cert but don't have access to the key you can still use this method. + type: string + webPki: + description: Use TLS and the CA certificates trusted by the common web browsers to verify the server. This can be useful when you e.g. use public AWS S3 or other public available services. type: object - required: - - caCert type: object + required: + - caCert type: object - required: - - verification type: object + required: + - verification type: object required: - clientCredentialsSecret diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 03a7ee9d..194ff7e2 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -40,7 +40,7 @@ pub mod versioned { ActiveDirectory(v1alpha1::ActiveDirectoryBackend), /// Backend that fetches user information from Microsoft Entra - #[serde(rename = "experimentalEntraBackend")] + #[serde(rename = "experimentalEntra")] Entra(v1alpha1::EntraBackend), } From d52c9aa285edf17eebfe765086bb4c091f4151c7 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 10:37:28 +0200 Subject: [PATCH 07/30] set correct default --- deploy/helm/opa-operator/crds/crds.yaml | 11 +++- rust/operator-binary/src/controller.rs | 12 ++-- .../src/crd/user_info_fetcher.rs | 57 +++++++++++++++++-- rust/user-info-fetcher/src/backend/entra.rs | 7 ++- rust/user-info-fetcher/src/main.rs | 12 +++- 5 files changed, 82 insertions(+), 17 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 92ca1d0d..42e11d2e 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -65,7 +65,7 @@ spec: - required: - experimentalActiveDirectory - required: - - experimentalEntra + - experimentalEntraBackend properties: experimentalActiveDirectory: description: Backend that fetches user information from Active Directory @@ -139,7 +139,7 @@ spec: - kerberosSecretClassName - ldapServer type: object - experimentalEntra: + experimentalEntraBackend: description: Backend that fetches user information from Microsoft Entra properties: clientCredentialsSecret: @@ -162,7 +162,12 @@ spec: description: The Microsoft Entra tenant ID. type: string tls: - description: Use a TLS connection. If not specified no TLS will be used. + default: + verification: + server: + caCert: + webPki: {} + description: Use a TLS connection. Should usually be set to WebPki. nullable: true properties: verification: diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index a1b847fa..b5a5fcf8 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -31,7 +31,7 @@ use stackable_operator::{ product_image_selection::ResolvedProductImage, rbac::build_rbac_resources, secret_class::{SecretClassVolume, SecretClassVolumeScope}, - tls_verification::TlsClientDetailsError, + tls_verification::{TlsClientDetails, TlsClientDetailsError}, }, k8s_openapi::{ api::{ @@ -1046,10 +1046,12 @@ fn build_server_rolegroup_daemonset( USER_INFO_FETCHER_CREDENTIALS_DIR, ) .context(AddVolumeMountSnafu)?; - entra - .tls - .add_volumes_and_mounts(&mut pb, vec![&mut cb_user_info_fetcher]) - .context(UserInfoFetcherTlsVolumeAndMountsSnafu)?; + + TlsClientDetails { + tls: entra.tls.clone(), + } + .add_volumes_and_mounts(&mut pb, vec![&mut cb_user_info_fetcher]) + .context(UserInfoFetcherTlsVolumeAndMountsSnafu)?; } } diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 194ff7e2..63851900 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -1,12 +1,16 @@ use std::{collections::BTreeMap, str::FromStr}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use stackable_operator::{ - commons::{networking::HostName, tls_verification::TlsClientDetails}, + commons::{ + networking::HostName, + tls_verification::{CaCert, Tls, TlsClientDetails, TlsServerVerification, TlsVerification}, + }, schemars::{self, JsonSchema}, time::Duration, }; use stackable_versioned::versioned; +use v1alpha1::EntraBackend; #[versioned(version(name = "v1alpha1"))] pub mod versioned { @@ -40,7 +44,7 @@ pub mod versioned { ActiveDirectory(v1alpha1::ActiveDirectoryBackend), /// Backend that fetches user information from Microsoft Entra - #[serde(rename = "experimentalEntra")] + #[serde(rename = "experimentalEntraBackend")] Entra(v1alpha1::EntraBackend), } @@ -129,8 +133,8 @@ pub mod versioned { pub tenant_id: String, /// Use a TLS connection. Should usually be set to WebPki. - #[serde(flatten)] - pub tls: TlsClientDetails, + #[serde(default = "default_tls_web_pki")] + pub tls: Option, /// Name of a Secret that contains client credentials of a Entra account with permission to read user metadata. /// @@ -165,6 +169,14 @@ fn entra_default_port() -> u16 { 443 } +fn default_tls_web_pki() -> Option { + Some(Tls { + verification: TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::WebPki {}, + }), + }) +} + fn aas_default_port() -> u16 { 5000 } @@ -182,3 +194,38 @@ impl Default for v1alpha1::Cache { } } } + +// #[derive(Deserialize)] +// #[serde(rename_all = "camelCase")] +// struct EntraBackendHelper { +// #[serde(default = "entra_default_host")] +// hostname: HostName, +// #[serde(default = "entra_default_port")] +// port: u16, +// tenant_id: String, +// #[serde(flatten)] +// tls: Option, +// client_credentials_secret: String, +// } + +// impl<'de> Deserialize<'de> for EntraBackend { +// fn deserialize(deserializer: D) -> Result +// where +// D: Deserializer<'de>, +// { +// let helper = EntraBackendHelper::deserialize(deserializer)?; +// Ok(EntraBackend { +// hostname: helper.hostname, +// port: helper.port, +// tenant_id: helper.tenant_id, +// tls: helper.tls.unwrap_or_else(|| TlsClientDetails { +// tls: Some(Tls { +// verification: TlsVerification::Server(TlsServerVerification { +// ca_cert: CaCert::WebPki {}, +// }), +// }), +// }), +// client_credentials_secret: helper.client_credentials_secret, +// }) +// } +// } diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index ac091929..90a9b4fe 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -84,7 +84,12 @@ pub(crate) async fn get_user_info( tls, } = config; - let entra_endpoint = EntraEndpoint::new(hostname.clone(), *port, tenant_id.clone(), tls); + let entra_endpoint = EntraEndpoint::new( + hostname.clone(), + *port, + tenant_id.clone(), + &TlsClientDetails { tls: tls.clone() }, + ); let token_url = entra_endpoint.oauth2_token(); let authn = send_json_request::(http.post(token_url).form(&[ diff --git a/rust/user-info-fetcher/src/main.rs b/rust/user-info-fetcher/src/main.rs index 75667930..76a2127f 100644 --- a/rust/user-info-fetcher/src/main.rs +++ b/rust/user-info-fetcher/src/main.rs @@ -13,6 +13,7 @@ use reqwest::ClientBuilder; use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; +use stackable_operator::commons::tls_verification::TlsClientDetails; use tokio::net::TcpListener; mod backend; @@ -139,9 +140,14 @@ async fn main() -> Result<(), StartupError> { .await .context(ConfigureTlsSnafu)?; } else if let v1alpha1::Backend::Entra(entra) = &config.backend { - client_builder = utils::tls::configure_reqwest(&entra.tls, client_builder) - .await - .context(ConfigureTlsSnafu)?; + client_builder = utils::tls::configure_reqwest( + &TlsClientDetails { + tls: entra.tls.clone(), + }, + client_builder, + ) + .await + .context(ConfigureTlsSnafu)?; } let http = client_builder.build().context(ConstructHttpClientSnafu)?; From 6f75fc7d155fabb0fa40e2a199d801e5a01f8a37 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 10:50:16 +0200 Subject: [PATCH 08/30] merge main --- .../src/crd/user_info_fetcher.rs | 39 +------------------ rust/operator-binary/src/main.rs | 11 ++++-- rust/user-info-fetcher/src/backend/entra.rs | 2 +- 3 files changed, 9 insertions(+), 43 deletions(-) diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 6add761e..745a1a14 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -1,6 +1,6 @@ use std::{collections::BTreeMap, str::FromStr}; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Serialize}; use stackable_operator::{ commons::{ networking::HostName, @@ -10,8 +10,6 @@ use stackable_operator::{ time::Duration, versioned::versioned, }; -use stackable_versioned::versioned; -use v1alpha1::EntraBackend; #[versioned(version(name = "v1alpha1"))] pub mod versioned { @@ -195,38 +193,3 @@ impl Default for v1alpha1::Cache { } } } - -// #[derive(Deserialize)] -// #[serde(rename_all = "camelCase")] -// struct EntraBackendHelper { -// #[serde(default = "entra_default_host")] -// hostname: HostName, -// #[serde(default = "entra_default_port")] -// port: u16, -// tenant_id: String, -// #[serde(flatten)] -// tls: Option, -// client_credentials_secret: String, -// } - -// impl<'de> Deserialize<'de> for EntraBackend { -// fn deserialize(deserializer: D) -> Result -// where -// D: Deserializer<'de>, -// { -// let helper = EntraBackendHelper::deserialize(deserializer)?; -// Ok(EntraBackend { -// hostname: helper.hostname, -// port: helper.port, -// tenant_id: helper.tenant_id, -// tls: helper.tls.unwrap_or_else(|| TlsClientDetails { -// tls: Some(Tls { -// verification: TlsVerification::Server(TlsServerVerification { -// ca_cert: CaCert::WebPki {}, -// }), -// }), -// }), -// client_credentials_secret: helper.client_credentials_secret, -// }) -// } -// } diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 8cb6f0e2..e780afb6 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -131,10 +131,13 @@ async fn create_controller( .owns(configmaps_api, watcher::Config::default()) .owns(services_api, watcher::Config::default()); - let event_recorder = Arc::new(Recorder::new(client.as_kube_client(), Reporter { - controller: OPA_FULL_CONTROLLER_NAME.to_string(), - instance: None, - })); + let event_recorder = Arc::new(Recorder::new( + client.as_kube_client(), + Reporter { + controller: OPA_FULL_CONTROLLER_NAME.to_string(), + instance: None, + }, + )); controller .run( controller::reconcile_opa, diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 90a9b4fe..9996b946 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -6,7 +6,7 @@ use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; use stackable_operator::commons::{networking::HostName, tls_verification::TlsClientDetails}; -use crate::{http_error, utils::http::send_json_request, Credentials, UserInfo, UserInfoRequest}; +use crate::{Credentials, UserInfo, UserInfoRequest, http_error, utils::http::send_json_request}; #[derive(Snafu, Debug)] pub enum Error { From 04c31174c29efe18876ddc88636c042bfc72d348 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 10:58:35 +0200 Subject: [PATCH 09/30] fmt --- rust/operator-binary/src/main.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index e780afb6..8cb6f0e2 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -131,13 +131,10 @@ async fn create_controller( .owns(configmaps_api, watcher::Config::default()) .owns(services_api, watcher::Config::default()); - let event_recorder = Arc::new(Recorder::new( - client.as_kube_client(), - Reporter { - controller: OPA_FULL_CONTROLLER_NAME.to_string(), - instance: None, - }, - )); + let event_recorder = Arc::new(Recorder::new(client.as_kube_client(), Reporter { + controller: OPA_FULL_CONTROLLER_NAME.to_string(), + instance: None, + })); controller .run( controller::reconcile_opa, From dc5130ff1c6fa77f265236933cd9a9c465c7844a Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 11:59:38 +0200 Subject: [PATCH 10/30] test fix entra endpoint --- deploy/helm/opa-operator/crds/crds.yaml | 6 ++-- .../src/crd/user_info_fetcher.rs | 4 +-- rust/operator-binary/src/main.rs | 11 ++++--- rust/user-info-fetcher/src/backend/entra.rs | 31 +++++++++++++------ 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 42e11d2e..68f0ad43 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -65,7 +65,7 @@ spec: - required: - experimentalActiveDirectory - required: - - experimentalEntraBackend + - experimentalEntra properties: experimentalActiveDirectory: description: Backend that fetches user information from Active Directory @@ -139,7 +139,7 @@ spec: - kerberosSecretClassName - ldapServer type: object - experimentalEntraBackend: + experimentalEntra: description: Backend that fetches user information from Microsoft Entra properties: clientCredentialsSecret: @@ -149,7 +149,7 @@ spec: Must contain the fields `clientId` and `clientSecret`. type: string hostname: - default: login.microsoft.com + default: microsoft.com description: Hostname of the identity provider, defaults to `login.microsoft.com`. type: string port: diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 745a1a14..b236307a 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -43,7 +43,7 @@ pub mod versioned { ActiveDirectory(v1alpha1::ActiveDirectoryBackend), /// Backend that fetches user information from Microsoft Entra - #[serde(rename = "experimentalEntraBackend")] + #[serde(rename = "experimentalEntra")] Entra(v1alpha1::EntraBackend), } @@ -161,7 +161,7 @@ fn default_root_path() -> String { } fn entra_default_host() -> HostName { - HostName::from_str("login.microsoft.com").unwrap() + HostName::from_str("microsoft.com").unwrap() } fn entra_default_port() -> u16 { diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 8cb6f0e2..e780afb6 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -131,10 +131,13 @@ async fn create_controller( .owns(configmaps_api, watcher::Config::default()) .owns(services_api, watcher::Config::default()); - let event_recorder = Arc::new(Recorder::new(client.as_kube_client(), Reporter { - controller: OPA_FULL_CONTROLLER_NAME.to_string(), - instance: None, - })); + let event_recorder = Arc::new(Recorder::new( + client.as_kube_client(), + Reporter { + controller: OPA_FULL_CONTROLLER_NAME.to_string(), + instance: None, + }, + )); controller .run( controller::reconcile_opa, diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 9996b946..4408042f 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -157,26 +157,29 @@ impl EntraEndpoint { pub fn oauth2_token(&self) -> String { format!( "{base_url}/{tenant_id}/oauth2/v2.0/token", - base_url = self.base_url(), + base_url = self.base_url("login"), tenant_id = self.tenant_id ) } // Works both with id/oid and userPrincipalName pub fn user_info(&self, user: &str) -> String { - format!("{base_url}/v1.0/users/{user}", base_url = self.base_url()) + format!( + "{base_url}/v1.0/users/{user}", + base_url = self.base_url("graph"), + ) } pub fn group_info(&self, user: &str) -> String { format!( "{base_url}/v1.0/users/{user}/memberOf", - base_url = self.base_url() + base_url = self.base_url("graph"), ) } - fn base_url(&self) -> String { + fn base_url(&self, prefix: &str) -> String { format!( - "{protocol}://{hostname}{opt_port}", + "{protocol}://{prefix}.{hostname}{opt_port}", opt_port = if self.port == 443 || self.port == 80 { "".to_string() } else { @@ -201,7 +204,7 @@ mod tests { #[test] fn test_defaults() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.microsoft.com").expect("Could not parse hostname"), + HostName::from_str("microsoft.com").expect("Could not parse hostname"), 443, "1234-5678".to_string(), &TlsClientDetails { @@ -219,18 +222,18 @@ mod tests { ); assert_eq!( entra_endpoint.user_info("0000-0000"), - "https://login.microsoft.com/v1.0/users/0000-0000" + "https://graph.microsoft.com/v1.0/users/0000-0000" ); assert_eq!( entra_endpoint.group_info("0000-0000"), - "https://login.microsoft.com/v1.0/users/0000-0000/memberOf" + "https://graph.microsoft.com/v1.0/users/0000-0000/memberOf" ); } #[test] fn test_non_defaults_tls() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + HostName::from_str("myentra.com").expect("Could not parse hostname"), 8443, "1234-5678".to_string(), &TlsClientDetails { @@ -246,12 +249,16 @@ mod tests { entra_endpoint.oauth2_token(), "https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token" ); + assert_eq!( + entra_endpoint.user_info("0000-0000"), + "https://graph.myentra.com:8443/v1.0/users/0000-0000" + ); } #[test] fn test_non_defaults_non_tls() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + HostName::from_str("myentra.com").expect("Could not parse hostname"), 8080, "1234-5678".to_string(), &TlsClientDetails { tls: None }, @@ -261,5 +268,9 @@ mod tests { entra_endpoint.oauth2_token(), "http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token" ); + assert_eq!( + entra_endpoint.user_info("0000-0000"), + "http://graph.myentra.com:8080/v1.0/users/0000-0000" + ); } } From 29202b9fc10216fe3486d16495c60f0f017b91ab Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 12:03:48 +0200 Subject: [PATCH 11/30] fmt --- rust/operator-binary/src/main.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index e780afb6..8cb6f0e2 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -131,13 +131,10 @@ async fn create_controller( .owns(configmaps_api, watcher::Config::default()) .owns(services_api, watcher::Config::default()); - let event_recorder = Arc::new(Recorder::new( - client.as_kube_client(), - Reporter { - controller: OPA_FULL_CONTROLLER_NAME.to_string(), - instance: None, - }, - )); + let event_recorder = Arc::new(Recorder::new(client.as_kube_client(), Reporter { + controller: OPA_FULL_CONTROLLER_NAME.to_string(), + instance: None, + })); controller .run( controller::reconcile_opa, From 5a4860e863e7f9d8fbee040440b082e42584f76e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 12:34:53 +0200 Subject: [PATCH 12/30] split token and host endpoint --- deploy/helm/opa-operator/crds/crds.yaml | 10 +++-- .../src/crd/user_info_fetcher.rs | 18 +++++--- rust/user-info-fetcher/src/backend/entra.rs | 42 ++++++++++++------- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 68f0ad43..4bb9c5df 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -148,9 +148,13 @@ spec: Must contain the fields `clientId` and `clientSecret`. type: string - hostname: - default: microsoft.com - description: Hostname of the identity provider, defaults to `login.microsoft.com`. + hostnameGraph: + default: graph.microsoft.com + description: Hostname of the user info provider, defaults to `graph.microsoft.com`. + type: string + hostnameToken: + default: login.microsoft.com + description: Hostname of the token provider, defaults to `login.microsoft.com`. type: string port: default: 443 diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index b236307a..3155e5de 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -120,9 +120,13 @@ pub mod versioned { #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct EntraBackend { - /// Hostname of the identity provider, defaults to `login.microsoft.com`. - #[serde(default = "entra_default_host")] - pub hostname: HostName, + /// Hostname of the token provider, defaults to `login.microsoft.com`. + #[serde(default = "entra_default_host_token")] + pub hostname_token: HostName, + + /// Hostname of the user info provider, defaults to `graph.microsoft.com`. + #[serde(default = "entra_default_host_graph")] + pub hostname_graph: HostName, /// Port of the identity provider. Defaults to 443. #[serde(default = "entra_default_port")] @@ -160,8 +164,12 @@ fn default_root_path() -> String { "/".to_string() } -fn entra_default_host() -> HostName { - HostName::from_str("microsoft.com").unwrap() +fn entra_default_host_token() -> HostName { + HostName::from_str("login.microsoft.com").unwrap() +} + +fn entra_default_host_graph() -> HostName { + HostName::from_str("graph.microsoft.com").unwrap() } fn entra_default_port() -> u16 { diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 4408042f..c897b563 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -64,7 +64,8 @@ struct GroupMembership { } struct EntraEndpoint { - hostname: HostName, + hostname_token: HostName, + hostname_graph: HostName, port: u16, tenant_id: String, protocol: String, @@ -78,16 +79,18 @@ pub(crate) async fn get_user_info( ) -> Result { let v1alpha1::EntraBackend { client_credentials_secret: _, - hostname, + hostname_token, + hostname_graph, port, tenant_id, tls, } = config; let entra_endpoint = EntraEndpoint::new( - hostname.clone(), + hostname_token.clone(), + hostname_graph.clone(), *port, - tenant_id.clone(), + tenant_id.to_string(), &TlsClientDetails { tls: tls.clone() }, ); let token_url = entra_endpoint.oauth2_token(); @@ -141,9 +144,16 @@ pub(crate) async fn get_user_info( } impl EntraEndpoint { - pub fn new(hostname: HostName, port: u16, tenant_id: String, tls: &TlsClientDetails) -> Self { + pub fn new( + hostname_token: HostName, + hostname_graph: HostName, + port: u16, + tenant_id: String, + tls: &TlsClientDetails, + ) -> Self { Self { - hostname, + hostname_token, + hostname_graph, port, tenant_id, protocol: if tls.uses_tls() { @@ -157,7 +167,7 @@ impl EntraEndpoint { pub fn oauth2_token(&self) -> String { format!( "{base_url}/{tenant_id}/oauth2/v2.0/token", - base_url = self.base_url("login"), + base_url = self.base_url(&self.hostname_token), tenant_id = self.tenant_id ) } @@ -166,26 +176,25 @@ impl EntraEndpoint { pub fn user_info(&self, user: &str) -> String { format!( "{base_url}/v1.0/users/{user}", - base_url = self.base_url("graph"), + base_url = self.base_url(&self.hostname_graph), ) } pub fn group_info(&self, user: &str) -> String { format!( "{base_url}/v1.0/users/{user}/memberOf", - base_url = self.base_url("graph"), + base_url = self.base_url(&self.hostname_graph), ) } - fn base_url(&self, prefix: &str) -> String { + fn base_url(&self, hostname: &HostName) -> String { format!( - "{protocol}://{prefix}.{hostname}{opt_port}", + "{protocol}://{hostname}{opt_port}", opt_port = if self.port == 443 || self.port == 80 { "".to_string() } else { format!(":{port}", port = self.port) }, - hostname = self.hostname, protocol = self.protocol ) } @@ -204,7 +213,8 @@ mod tests { #[test] fn test_defaults() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("microsoft.com").expect("Could not parse hostname"), + HostName::from_str("login.microsoft.com").expect("Could not parse hostname"), + HostName::from_str("graph.microsoft.com").expect("Could not parse hostname"), 443, "1234-5678".to_string(), &TlsClientDetails { @@ -233,7 +243,8 @@ mod tests { #[test] fn test_non_defaults_tls() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("myentra.com").expect("Could not parse hostname"), + HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + HostName::from_str("graph.myentra.com").expect("Could not parse hostname"), 8443, "1234-5678".to_string(), &TlsClientDetails { @@ -258,7 +269,8 @@ mod tests { #[test] fn test_non_defaults_non_tls() { let entra_endpoint = EntraEndpoint::new( - HostName::from_str("myentra.com").expect("Could not parse hostname"), + HostName::from_str("login.myentra.com").expect("Could not parse hostname"), + HostName::from_str("graph.myentra.com").expect("Could not parse hostname"), 8080, "1234-5678".to_string(), &TlsClientDetails { tls: None }, From 6b85d1b30b5c35df04aaf3f3b286379799e79939 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 13:02:38 +0200 Subject: [PATCH 13/30] fix group response --- rust/user-info-fetcher/src/backend/entra.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index c897b563..7fdb4eb8 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -57,10 +57,16 @@ struct UserMetadata { attributes: HashMap, } +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct GroupMembershipResponse { + value: Vec, +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct GroupMembership { - display_name: String, + display_name: Option, } struct EntraEndpoint { @@ -125,7 +131,7 @@ pub(crate) async fn get_user_info( } }; - let groups = send_json_request::>( + let groups = send_json_request::( http.get(entra_endpoint.group_info(&user_info.id)) .bearer_auth(&authn.access_token), ) @@ -133,12 +139,17 @@ pub(crate) async fn get_user_info( .context(RequestUserGroupsSnafu { username: user_info.user_principal_name.clone(), user_id: user_info.id.clone(), - })?; + })? + .value; Ok(UserInfo { id: Some(user_info.id), username: Some(user_info.user_principal_name), - groups: groups.into_iter().map(|g| g.display_name).collect(), + groups: groups + .into_iter() + .map(|g| g.display_name) + .flatten() + .collect(), custom_attributes: user_info.attributes, }) } From f438b6363ddf1360f556f73c8f7ff4ec804436e1 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 13:04:49 +0200 Subject: [PATCH 14/30] clippy --- rust/user-info-fetcher/src/backend/entra.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 7fdb4eb8..a5f3f70a 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -145,11 +145,7 @@ pub(crate) async fn get_user_info( Ok(UserInfo { id: Some(user_info.id), username: Some(user_info.user_principal_name), - groups: groups - .into_iter() - .map(|g| g.display_name) - .flatten() - .collect(), + groups: groups.into_iter().filter_map(|g| g.display_name).collect(), custom_attributes: user_info.attributes, }) } From 4ed2a9284cab92475d8015c088db15d8a6cb6635 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 16:53:04 +0200 Subject: [PATCH 15/30] use url in entra endpoint --- .../src/crd/user_info_fetcher.rs | 20 +- rust/user-info-fetcher/src/backend/entra.rs | 211 +++++++++--------- 2 files changed, 120 insertions(+), 111 deletions(-) diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 3155e5de..3d981c44 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -121,12 +121,12 @@ pub mod versioned { #[serde(rename_all = "camelCase")] pub struct EntraBackend { /// Hostname of the token provider, defaults to `login.microsoft.com`. - #[serde(default = "entra_default_host_token")] - pub hostname_token: HostName, + #[serde(default = "entra_default_token_endpoint")] + pub token_endpoint: HostName, /// Hostname of the user info provider, defaults to `graph.microsoft.com`. - #[serde(default = "entra_default_host_graph")] - pub hostname_graph: HostName, + #[serde(default = "entra_default_user_info_endpoint")] + pub user_info_endpoint: HostName, /// Port of the identity provider. Defaults to 443. #[serde(default = "entra_default_port")] @@ -136,10 +136,16 @@ pub mod versioned { pub tenant_id: String, /// Use a TLS connection. Should usually be set to WebPki. + // We do not use the flattened `TlsClientDetails` here since we cannot + // default to WebPki using a default and flatten + // https://github.com/serde-rs/serde/issues/1626 + // This means we have to wrap `Tls` in `TlsClientDetails` to its + // method like `uses_tls()`. #[serde(default = "default_tls_web_pki")] pub tls: Option, - /// Name of a Secret that contains client credentials of a Entra account with permission to read user metadata. + /// Name of a Secret that contains client credentials of an Entra account with + /// permissions `User.ReadAll` and `GroupMemberShip.ReadAll`. /// /// Must contain the fields `clientId` and `clientSecret`. pub client_credentials_secret: String, @@ -164,11 +170,11 @@ fn default_root_path() -> String { "/".to_string() } -fn entra_default_host_token() -> HostName { +fn entra_default_token_endpoint() -> HostName { HostName::from_str("login.microsoft.com").unwrap() } -fn entra_default_host_graph() -> HostName { +fn entra_default_user_info_endpoint() -> HostName { HostName::from_str("graph.microsoft.com").unwrap() } diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index a5f3f70a..23b54f4e 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -4,7 +4,8 @@ use hyper::StatusCode; use serde::Deserialize; use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; -use stackable_operator::commons::{networking::HostName, tls_verification::TlsClientDetails}; +use stackable_operator::commons::tls_verification::TlsClientDetails; +use url::Url; use crate::{Credentials, UserInfo, UserInfoRequest, http_error, utils::http::send_json_request}; @@ -16,7 +17,7 @@ pub enum Error { #[snafu(display("failed to search for user"))] SearchForUser { source: crate::utils::http::Error }, - #[snafu(display("unable to find user with id {user_id:?}"))] + #[snafu(display("failed to search for user with id {user_id:?}"))] UserNotFoundById { source: crate::utils::http::Error, user_id: String, @@ -30,6 +31,12 @@ pub enum Error { username: String, user_id: String, }, + + #[snafu(display("failed to to build entra endpoint for {endpoint}"))] + BuildEntraEndpointFailed { + source: url::ParseError, + endpoint: String, + }, } impl http_error::Error for Error { @@ -39,6 +46,7 @@ impl http_error::Error for Error { Self::SearchForUser { .. } => StatusCode::BAD_GATEWAY, Self::UserNotFoundById { .. } => StatusCode::NOT_FOUND, Self::RequestUserGroups { .. } => StatusCode::BAD_GATEWAY, + Self::BuildEntraEndpointFailed { .. } => StatusCode::BAD_REQUEST, } } } @@ -69,14 +77,6 @@ struct GroupMembership { display_name: Option, } -struct EntraEndpoint { - hostname_token: HostName, - hostname_graph: HostName, - port: u16, - tenant_id: String, - protocol: String, -} - pub(crate) async fn get_user_info( req: &UserInfoRequest, http: &reqwest::Client, @@ -85,22 +85,22 @@ pub(crate) async fn get_user_info( ) -> Result { let v1alpha1::EntraBackend { client_credentials_secret: _, - hostname_token, - hostname_graph, + token_endpoint, + user_info_endpoint, port, tenant_id, tls, } = config; - let entra_endpoint = EntraEndpoint::new( - hostname_token.clone(), - hostname_graph.clone(), + let entra_endpoint = EntraBackend::try_new( + &token_endpoint.as_url_host(), + &user_info_endpoint.as_url_host(), *port, - tenant_id.to_string(), - &TlsClientDetails { tls: tls.clone() }, - ); - let token_url = entra_endpoint.oauth2_token(); + tenant_id, + TlsClientDetails { tls: tls.clone() }.uses_tls(), + )?; + let token_url = entra_endpoint.oauth2_token(); let authn = send_json_request::(http.post(token_url).form(&[ ("client_id", credentials.client_id.as_str()), ("client_secret", credentials.client_secret.as_str()), @@ -150,145 +150,148 @@ pub(crate) async fn get_user_info( }) } -impl EntraEndpoint { - pub fn new( - hostname_token: HostName, - hostname_graph: HostName, +struct EntraBackend { + token_endpoint_url: Url, + user_info_endpoint_url: Url, +} + +impl EntraBackend { + pub fn try_new( + token_endpoint: &str, + user_info_endpoint: &str, port: u16, - tenant_id: String, - tls: &TlsClientDetails, - ) -> Self { - Self { - hostname_token, - hostname_graph, - port, - tenant_id, - protocol: if tls.uses_tls() { - "https".to_string() - } else { - "http".to_string() - }, - } + tenant_id: &str, + uses_tls: bool, + ) -> Result { + let schema = if uses_tls { "https" } else { "http" }; + + let token_endpoint = + format!("{schema}://{token_endpoint}:{port}/{tenant_id}/oauth2/v2.0/token"); + let token_endpoint_url = + Url::parse(&token_endpoint).context(BuildEntraEndpointFailedSnafu { + endpoint: token_endpoint, + })?; + + let user_info_endpoint = format!("{schema}://{user_info_endpoint}:{port}"); + let user_info_endpoint_url = + Url::parse(&user_info_endpoint).context(BuildEntraEndpointFailedSnafu { + endpoint: user_info_endpoint, + })?; + + Ok(Self { + token_endpoint_url, + user_info_endpoint_url, + }) } pub fn oauth2_token(&self) -> String { - format!( - "{base_url}/{tenant_id}/oauth2/v2.0/token", - base_url = self.base_url(&self.hostname_token), - tenant_id = self.tenant_id - ) + self.token_endpoint_url.to_string() } // Works both with id/oid and userPrincipalName pub fn user_info(&self, user: &str) -> String { - format!( - "{base_url}/v1.0/users/{user}", - base_url = self.base_url(&self.hostname_graph), - ) + let mut user_info_url = self.user_info_endpoint_url.clone(); + user_info_url.set_path(&format!("/v1.0/users/{user}")); + + user_info_url.to_string() } pub fn group_info(&self, user: &str) -> String { - format!( - "{base_url}/v1.0/users/{user}/memberOf", - base_url = self.base_url(&self.hostname_graph), - ) - } + let mut user_info_url = self.user_info_endpoint_url.clone(); + user_info_url.set_path(&format!("/v1.0/users/{user}/memberOf")); - fn base_url(&self, hostname: &HostName) -> String { - format!( - "{protocol}://{hostname}{opt_port}", - opt_port = if self.port == 443 || self.port == 80 { - "".to_string() - } else { - format!(":{port}", port = self.port) - }, - protocol = self.protocol - ) + user_info_url.to_string() } } #[cfg(test)] mod tests { - use std::str::FromStr; - - use stackable_operator::commons::tls_verification::{ - CaCert, Tls, TlsServerVerification, TlsVerification, - }; - use super::*; #[test] fn test_defaults() { - let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.microsoft.com").expect("Could not parse hostname"), - HostName::from_str("graph.microsoft.com").expect("Could not parse hostname"), + let entra = EntraBackend::try_new( + "login.microsoft.com", + "graph.microsoft.com", 443, - "1234-5678".to_string(), - &TlsClientDetails { - tls: Some(Tls { - verification: TlsVerification::Server(TlsServerVerification { - ca_cert: CaCert::WebPki {}, - }), - }), - }, - ); + "1234-5678", + true, + ) + .unwrap(); assert_eq!( - entra_endpoint.oauth2_token(), + entra.oauth2_token(), "https://login.microsoft.com/1234-5678/oauth2/v2.0/token" ); assert_eq!( - entra_endpoint.user_info("0000-0000"), + entra.user_info("0000-0000"), "https://graph.microsoft.com/v1.0/users/0000-0000" ); assert_eq!( - entra_endpoint.group_info("0000-0000"), + entra.group_info("0000-0000"), "https://graph.microsoft.com/v1.0/users/0000-0000/memberOf" ); } #[test] fn test_non_defaults_tls() { - let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.myentra.com").expect("Could not parse hostname"), - HostName::from_str("graph.myentra.com").expect("Could not parse hostname"), + let entra = EntraBackend::try_new( + "login.myentra.com", + "graph.myentra.com", 8443, - "1234-5678".to_string(), - &TlsClientDetails { - tls: Some(Tls { - verification: TlsVerification::Server(TlsServerVerification { - ca_cert: CaCert::WebPki {}, - }), - }), - }, - ); + "1234-5678", + true, + ) + .unwrap(); assert_eq!( - entra_endpoint.oauth2_token(), + entra.oauth2_token(), "https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token" ); assert_eq!( - entra_endpoint.user_info("0000-0000"), + entra.user_info("0000-0000"), "https://graph.myentra.com:8443/v1.0/users/0000-0000" ); } + #[test] + fn test_defaults_non_tls() { + let entra = EntraBackend::try_new( + "login.myentra.com", + "graph.myentra.com", + 80, + "1234-5678", + false, + ) + .unwrap(); + + assert_eq!( + entra.oauth2_token(), + "http://login.myentra.com/1234-5678/oauth2/v2.0/token" + ); + assert_eq!( + entra.user_info("0000-0000"), + "http://graph.myentra.com/v1.0/users/0000-0000" + ); + } + #[test] fn test_non_defaults_non_tls() { - let entra_endpoint = EntraEndpoint::new( - HostName::from_str("login.myentra.com").expect("Could not parse hostname"), - HostName::from_str("graph.myentra.com").expect("Could not parse hostname"), + let entra = EntraBackend::try_new( + "login.myentra.com", + "graph.myentra.com", 8080, - "1234-5678".to_string(), - &TlsClientDetails { tls: None }, - ); + "1234-5678", + false, + ) + .unwrap(); assert_eq!( - entra_endpoint.oauth2_token(), + entra.oauth2_token(), "http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token" ); assert_eq!( - entra_endpoint.user_info("0000-0000"), + entra.user_info("0000-0000"), "http://graph.myentra.com:8080/v1.0/users/0000-0000" ); } From c26952fee5abb7621beca0b83165722aabbfbe45 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 16:55:04 +0200 Subject: [PATCH 16/30] regenerate charts --- deploy/helm/opa-operator/crds/crds.yaml | 18 +++++++++--------- rust/user-info-fetcher/src/backend/entra.rs | 2 -- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 4bb9c5df..d16f5c6f 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -144,18 +144,10 @@ spec: properties: clientCredentialsSecret: description: |- - Name of a Secret that contains client credentials of a Entra account with permission to read user metadata. + Name of a Secret that contains client credentials of an Entra account with permissions `User.ReadAll` and `GroupMemberShip.ReadAll`. Must contain the fields `clientId` and `clientSecret`. type: string - hostnameGraph: - default: graph.microsoft.com - description: Hostname of the user info provider, defaults to `graph.microsoft.com`. - type: string - hostnameToken: - default: login.microsoft.com - description: Hostname of the token provider, defaults to `login.microsoft.com`. - type: string port: default: 443 description: Port of the identity provider. Defaults to 443. @@ -210,6 +202,14 @@ spec: required: - verification type: object + tokenEndpoint: + default: login.microsoft.com + description: Hostname of the token provider, defaults to `login.microsoft.com`. + type: string + userInfoEndpoint: + default: graph.microsoft.com + description: Hostname of the user info provider, defaults to `graph.microsoft.com`. + type: string required: - clientCredentialsSecret - tenantId diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 23b54f4e..f889afe5 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -192,14 +192,12 @@ impl EntraBackend { pub fn user_info(&self, user: &str) -> String { let mut user_info_url = self.user_info_endpoint_url.clone(); user_info_url.set_path(&format!("/v1.0/users/{user}")); - user_info_url.to_string() } pub fn group_info(&self, user: &str) -> String { let mut user_info_url = self.user_info_endpoint_url.clone(); user_info_url.set_path(&format!("/v1.0/users/{user}/memberOf")); - user_info_url.to_string() } } From 888076d2ee1b872c4791cf651778867b7ffa19bf Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 15 Apr 2025 17:41:40 +0200 Subject: [PATCH 17/30] extend test, improve erros --- rust/user-info-fetcher/src/backend/entra.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index f889afe5..af0d99e6 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -14,8 +14,11 @@ pub enum Error { #[snafu(display("failed to get access_token"))] AccessToken { source: crate::utils::http::Error }, - #[snafu(display("failed to search for user"))] - SearchForUser { source: crate::utils::http::Error }, + #[snafu(display("failed to search for user with username {username:?}"))] + SearchForUser { + source: crate::utils::http::Error, + username: String, + }, #[snafu(display("failed to search for user with id {user_id:?}"))] UserNotFoundById { @@ -127,7 +130,7 @@ pub(crate) async fn get_user_info( .bearer_auth(&authn.access_token), ) .await - .context(SearchForUserSnafu)? + .context(SearchForUserSnafu { username })? } }; @@ -250,6 +253,10 @@ mod tests { entra.user_info("0000-0000"), "https://graph.myentra.com:8443/v1.0/users/0000-0000" ); + assert_eq!( + entra.group_info("0000-0000"), + "https://graph.myentra.com:8443/v1.0/users/0000-0000/memberOf" + ); } #[test] @@ -271,6 +278,10 @@ mod tests { entra.user_info("0000-0000"), "http://graph.myentra.com/v1.0/users/0000-0000" ); + assert_eq!( + entra.group_info("0000-0000"), + "http://graph.myentra.com/v1.0/users/0000-0000/memberOf" + ); } #[test] @@ -292,5 +303,9 @@ mod tests { entra.user_info("0000-0000"), "http://graph.myentra.com:8080/v1.0/users/0000-0000" ); + assert_eq!( + entra.group_info("0000-0000"), + "http://graph.myentra.com:8080/v1.0/users/0000-0000/memberOf" + ); } } From b974eb6a435820d29c5e235af94771eacefc70c6 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 16 Apr 2025 09:54:56 +0200 Subject: [PATCH 18/30] use with_context --- rust/user-info-fetcher/src/backend/entra.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index af0d99e6..45e31f50 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -139,7 +139,7 @@ pub(crate) async fn get_user_info( .bearer_auth(&authn.access_token), ) .await - .context(RequestUserGroupsSnafu { + .with_context(|_| RequestUserGroupsSnafu { username: user_info.user_principal_name.clone(), user_id: user_info.id.clone(), })? From 61baffb5169886e13e46e308ed9cdec495253b11 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 16 Apr 2025 11:02:34 +0200 Subject: [PATCH 19/30] use with context 2 --- rust/user-info-fetcher/src/backend/entra.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 45e31f50..f49f602b 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -115,13 +115,15 @@ pub(crate) async fn get_user_info( let user_info = match req { UserInfoRequest::UserInfoRequestById(req) => { - let user_id = req.id.clone(); + let user_id = &req.id; send_json_request::( http.get(entra_endpoint.user_info(&user_id)) .bearer_auth(&authn.access_token), ) .await - .context(UserNotFoundByIdSnafu { user_id })? + .with_context(|_| UserNotFoundByIdSnafu { + user_id: user_id.clone(), + })? } UserInfoRequest::UserInfoRequestByName(req) => { let username = &req.username; @@ -130,7 +132,9 @@ pub(crate) async fn get_user_info( .bearer_auth(&authn.access_token), ) .await - .context(SearchForUserSnafu { username })? + .with_context(|_| SearchForUserSnafu { + username: username.clone(), + })? } }; From c85e2b3f2788d8c7afddd4459fe92ecb62eb234d Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 16 Apr 2025 11:25:12 +0200 Subject: [PATCH 20/30] clippy --- rust/user-info-fetcher/src/backend/entra.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index f49f602b..8e1eec20 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -117,7 +117,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestById(req) => { let user_id = &req.id; send_json_request::( - http.get(entra_endpoint.user_info(&user_id)) + http.get(entra_endpoint.user_info(user_id)) .bearer_auth(&authn.access_token), ) .await From b1913ca28f010aa12e846c6571d6f5a630283729 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 13:13:34 +0200 Subject: [PATCH 21/30] change endpoint to hostname in CRD --- deploy/helm/opa-operator/crds/crds.yaml | 4 ++-- rust/operator-binary/src/crd/user_info_fetcher.rs | 12 ++++++------ rust/user-info-fetcher/src/backend/entra.rs | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index d16f5c6f..331b3289 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -202,11 +202,11 @@ spec: required: - verification type: object - tokenEndpoint: + tokenHostname: default: login.microsoft.com description: Hostname of the token provider, defaults to `login.microsoft.com`. type: string - userInfoEndpoint: + userInfoHostname: default: graph.microsoft.com description: Hostname of the user info provider, defaults to `graph.microsoft.com`. type: string diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 3d981c44..69eeb559 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -121,12 +121,12 @@ pub mod versioned { #[serde(rename_all = "camelCase")] pub struct EntraBackend { /// Hostname of the token provider, defaults to `login.microsoft.com`. - #[serde(default = "entra_default_token_endpoint")] - pub token_endpoint: HostName, + #[serde(default = "entra_default_token_hostname")] + pub token_hostname: HostName, /// Hostname of the user info provider, defaults to `graph.microsoft.com`. - #[serde(default = "entra_default_user_info_endpoint")] - pub user_info_endpoint: HostName, + #[serde(default = "entra_default_user_info_hostname")] + pub user_info_hostname: HostName, /// Port of the identity provider. Defaults to 443. #[serde(default = "entra_default_port")] @@ -170,11 +170,11 @@ fn default_root_path() -> String { "/".to_string() } -fn entra_default_token_endpoint() -> HostName { +fn entra_default_token_hostname() -> HostName { HostName::from_str("login.microsoft.com").unwrap() } -fn entra_default_user_info_endpoint() -> HostName { +fn entra_default_user_info_hostname() -> HostName { HostName::from_str("graph.microsoft.com").unwrap() } diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 8e1eec20..406b7bdb 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -88,16 +88,16 @@ pub(crate) async fn get_user_info( ) -> Result { let v1alpha1::EntraBackend { client_credentials_secret: _, - token_endpoint, - user_info_endpoint, + token_hostname, + user_info_hostname, port, tenant_id, tls, } = config; let entra_endpoint = EntraBackend::try_new( - &token_endpoint.as_url_host(), - &user_info_endpoint.as_url_host(), + &token_hostname.as_url_host(), + &user_info_hostname.as_url_host(), *port, tenant_id, TlsClientDetails { tls: tls.clone() }.uses_tls(), From cad745a782383332cb96338f5806832b809bc55d Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 13:14:38 +0200 Subject: [PATCH 22/30] consolidate naming --- rust/user-info-fetcher/src/backend/entra.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 406b7bdb..92a9bf06 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -95,7 +95,7 @@ pub(crate) async fn get_user_info( tls, } = config; - let entra_endpoint = EntraBackend::try_new( + let entra_backend = EntraBackend::try_new( &token_hostname.as_url_host(), &user_info_hostname.as_url_host(), *port, @@ -103,7 +103,7 @@ pub(crate) async fn get_user_info( TlsClientDetails { tls: tls.clone() }.uses_tls(), )?; - let token_url = entra_endpoint.oauth2_token(); + let token_url = entra_backend.oauth2_token(); let authn = send_json_request::(http.post(token_url).form(&[ ("client_id", credentials.client_id.as_str()), ("client_secret", credentials.client_secret.as_str()), @@ -117,7 +117,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestById(req) => { let user_id = &req.id; send_json_request::( - http.get(entra_endpoint.user_info(user_id)) + http.get(entra_backend.user_info(user_id)) .bearer_auth(&authn.access_token), ) .await @@ -128,7 +128,7 @@ pub(crate) async fn get_user_info( UserInfoRequest::UserInfoRequestByName(req) => { let username = &req.username; send_json_request::( - http.get(entra_endpoint.user_info(username)) + http.get(entra_backend.user_info(username)) .bearer_auth(&authn.access_token), ) .await @@ -139,7 +139,7 @@ pub(crate) async fn get_user_info( }; let groups = send_json_request::( - http.get(entra_endpoint.group_info(&user_info.id)) + http.get(entra_backend.group_info(&user_info.id)) .bearer_auth(&authn.access_token), ) .await From 8c07822b12e180ed8b13abfc25bd0996d0c6eee7 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 13:27:11 +0200 Subject: [PATCH 23/30] change entrabackend to use url consistently --- rust/user-info-fetcher/src/backend/entra.rs | 72 +++++++++++---------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 92a9bf06..bcf61770 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -4,7 +4,7 @@ use hyper::StatusCode; use serde::Deserialize; use snafu::{ResultExt, Snafu}; use stackable_opa_operator::crd::user_info_fetcher::v1alpha1; -use stackable_operator::commons::tls_verification::TlsClientDetails; +use stackable_operator::commons::{networking::HostName, tls_verification::TlsClientDetails}; use url::Url; use crate::{Credentials, UserInfo, UserInfoRequest, http_error, utils::http::send_json_request}; @@ -96,8 +96,8 @@ pub(crate) async fn get_user_info( } = config; let entra_backend = EntraBackend::try_new( - &token_hostname.as_url_host(), - &user_info_hostname.as_url_host(), + token_hostname, + user_info_hostname, *port, tenant_id, TlsClientDetails { tls: tls.clone() }.uses_tls(), @@ -164,8 +164,8 @@ struct EntraBackend { impl EntraBackend { pub fn try_new( - token_endpoint: &str, - user_info_endpoint: &str, + token_endpoint: &HostName, + user_info_endpoint: &HostName, port: u16, tenant_id: &str, uses_tls: bool, @@ -191,33 +191,35 @@ impl EntraBackend { }) } - pub fn oauth2_token(&self) -> String { - self.token_endpoint_url.to_string() + pub fn oauth2_token(&self) -> Url { + self.token_endpoint_url.clone() } // Works both with id/oid and userPrincipalName - pub fn user_info(&self, user: &str) -> String { + pub fn user_info(&self, user: &str) -> Url { let mut user_info_url = self.user_info_endpoint_url.clone(); user_info_url.set_path(&format!("/v1.0/users/{user}")); - user_info_url.to_string() + user_info_url } - pub fn group_info(&self, user: &str) -> String { + pub fn group_info(&self, user: &str) -> Url { let mut user_info_url = self.user_info_endpoint_url.clone(); user_info_url.set_path(&format!("/v1.0/users/{user}/memberOf")); - user_info_url.to_string() + user_info_url } } #[cfg(test)] mod tests { + use std::str::FromStr; + use super::*; #[test] - fn test_defaults() { + fn test_entra_defaults() { let entra = EntraBackend::try_new( - "login.microsoft.com", - "graph.microsoft.com", + &HostName::from_str("login.microsoft.com").unwrap(), + &HostName::from_str("graph.microsoft.com").unwrap(), 443, "1234-5678", true, @@ -226,23 +228,23 @@ mod tests { assert_eq!( entra.oauth2_token(), - "https://login.microsoft.com/1234-5678/oauth2/v2.0/token" + Url::parse("https://login.microsoft.com/1234-5678/oauth2/v2.0/token").unwrap() ); assert_eq!( entra.user_info("0000-0000"), - "https://graph.microsoft.com/v1.0/users/0000-0000" + Url::parse("https://graph.microsoft.com/v1.0/users/0000-0000").unwrap() ); assert_eq!( entra.group_info("0000-0000"), - "https://graph.microsoft.com/v1.0/users/0000-0000/memberOf" + Url::parse("https://graph.microsoft.com/v1.0/users/0000-0000/memberOf").unwrap() ); } #[test] - fn test_non_defaults_tls() { + fn test_entra_non_default_host_non_default_port_tls() { let entra = EntraBackend::try_new( - "login.myentra.com", - "graph.myentra.com", + &HostName::from_str("login.myentra.com").unwrap(), + &HostName::from_str("graph.myentra.com").unwrap(), 8443, "1234-5678", true, @@ -251,23 +253,23 @@ mod tests { assert_eq!( entra.oauth2_token(), - "https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token" + Url::parse("https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token").unwrap() ); assert_eq!( entra.user_info("0000-0000"), - "https://graph.myentra.com:8443/v1.0/users/0000-0000" + Url::parse("https://graph.myentra.com:8443/v1.0/users/0000-0000").unwrap() ); assert_eq!( entra.group_info("0000-0000"), - "https://graph.myentra.com:8443/v1.0/users/0000-0000/memberOf" + Url::parse("https://graph.myentra.com:8443/v1.0/users/0000-0000/memberOf").unwrap() ); } #[test] - fn test_defaults_non_tls() { + fn test_entra_non_default_host_default_port_non_tls() { let entra = EntraBackend::try_new( - "login.myentra.com", - "graph.myentra.com", + &HostName::from_str("login.myentra.com").unwrap(), + &HostName::from_str("graph.myentra.com").unwrap(), 80, "1234-5678", false, @@ -276,23 +278,23 @@ mod tests { assert_eq!( entra.oauth2_token(), - "http://login.myentra.com/1234-5678/oauth2/v2.0/token" + Url::parse("http://login.myentra.com/1234-5678/oauth2/v2.0/token").unwrap() ); assert_eq!( entra.user_info("0000-0000"), - "http://graph.myentra.com/v1.0/users/0000-0000" + Url::parse("http://graph.myentra.com/v1.0/users/0000-0000").unwrap() ); assert_eq!( entra.group_info("0000-0000"), - "http://graph.myentra.com/v1.0/users/0000-0000/memberOf" + Url::parse("http://graph.myentra.com/v1.0/users/0000-0000/memberOf").unwrap() ); } #[test] - fn test_non_defaults_non_tls() { + fn test_entra_non_default_host_non_default_port_non_tls() { let entra = EntraBackend::try_new( - "login.myentra.com", - "graph.myentra.com", + &HostName::from_str("login.myentra.com").unwrap(), + &HostName::from_str("graph.myentra.com").unwrap(), 8080, "1234-5678", false, @@ -301,15 +303,15 @@ mod tests { assert_eq!( entra.oauth2_token(), - "http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token" + Url::parse("http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token").unwrap() ); assert_eq!( entra.user_info("0000-0000"), - "http://graph.myentra.com:8080/v1.0/users/0000-0000" + Url::parse("http://graph.myentra.com:8080/v1.0/users/0000-0000").unwrap() ); assert_eq!( entra.group_info("0000-0000"), - "http://graph.myentra.com:8080/v1.0/users/0000-0000/memberOf" + Url::parse("http://graph.myentra.com:8080/v1.0/users/0000-0000/memberOf").unwrap() ); } } From e6a531a615d838f9929fc7e7f2169fd9a0fa177e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 13:51:58 +0200 Subject: [PATCH 24/30] remove obsolete tests --- rust/user-info-fetcher/src/backend/entra.rs | 102 +++++++------------- 1 file changed, 35 insertions(+), 67 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index bcf61770..71ebb42c 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -216,102 +216,70 @@ mod tests { use super::*; #[test] - fn test_entra_defaults() { + fn test_entra_defaults_id() { + let tenant_id = "1234-5678-1234-5678"; + let user = "1234-5678-1234-5678"; + let entra = EntraBackend::try_new( &HostName::from_str("login.microsoft.com").unwrap(), &HostName::from_str("graph.microsoft.com").unwrap(), 443, - "1234-5678", + tenant_id, true, ) .unwrap(); assert_eq!( entra.oauth2_token(), - Url::parse("https://login.microsoft.com/1234-5678/oauth2/v2.0/token").unwrap() + Url::parse(&format!( + "https://login.microsoft.com/{tenant_id}/oauth2/v2.0/token" + )) + .unwrap() ); assert_eq!( - entra.user_info("0000-0000"), - Url::parse("https://graph.microsoft.com/v1.0/users/0000-0000").unwrap() + entra.user_info(user), + Url::parse(&format!("https://graph.microsoft.com/v1.0/users/{user}")).unwrap() ); assert_eq!( - entra.group_info("0000-0000"), - Url::parse("https://graph.microsoft.com/v1.0/users/0000-0000/memberOf").unwrap() + entra.group_info(user), + Url::parse(&format!( + "https://graph.microsoft.com/v1.0/users/{user}/memberOf" + )) + .unwrap() ); } #[test] - fn test_entra_non_default_host_non_default_port_tls() { - let entra = EntraBackend::try_new( - &HostName::from_str("login.myentra.com").unwrap(), - &HostName::from_str("graph.myentra.com").unwrap(), - 8443, - "1234-5678", - true, - ) - .unwrap(); + fn test_entra_defaults_email() { + let tenant_id = "1234-5678-1234-5678"; + let user = "test@stackable.tech"; - assert_eq!( - entra.oauth2_token(), - Url::parse("https://login.myentra.com:8443/1234-5678/oauth2/v2.0/token").unwrap() - ); - assert_eq!( - entra.user_info("0000-0000"), - Url::parse("https://graph.myentra.com:8443/v1.0/users/0000-0000").unwrap() - ); - assert_eq!( - entra.group_info("0000-0000"), - Url::parse("https://graph.myentra.com:8443/v1.0/users/0000-0000/memberOf").unwrap() - ); - } - - #[test] - fn test_entra_non_default_host_default_port_non_tls() { let entra = EntraBackend::try_new( - &HostName::from_str("login.myentra.com").unwrap(), - &HostName::from_str("graph.myentra.com").unwrap(), - 80, - "1234-5678", - false, - ) - .unwrap(); - - assert_eq!( - entra.oauth2_token(), - Url::parse("http://login.myentra.com/1234-5678/oauth2/v2.0/token").unwrap() - ); - assert_eq!( - entra.user_info("0000-0000"), - Url::parse("http://graph.myentra.com/v1.0/users/0000-0000").unwrap() - ); - assert_eq!( - entra.group_info("0000-0000"), - Url::parse("http://graph.myentra.com/v1.0/users/0000-0000/memberOf").unwrap() - ); - } - - #[test] - fn test_entra_non_default_host_non_default_port_non_tls() { - let entra = EntraBackend::try_new( - &HostName::from_str("login.myentra.com").unwrap(), - &HostName::from_str("graph.myentra.com").unwrap(), - 8080, - "1234-5678", - false, + &HostName::from_str("login.microsoft.com").unwrap(), + &HostName::from_str("graph.microsoft.com").unwrap(), + 443, + tenant_id, + true, ) .unwrap(); assert_eq!( entra.oauth2_token(), - Url::parse("http://login.myentra.com:8080/1234-5678/oauth2/v2.0/token").unwrap() + Url::parse(&format!( + "https://login.microsoft.com/{tenant_id}/oauth2/v2.0/token" + )) + .unwrap() ); assert_eq!( - entra.user_info("0000-0000"), - Url::parse("http://graph.myentra.com:8080/v1.0/users/0000-0000").unwrap() + entra.user_info(user), + Url::parse(&format!("https://graph.microsoft.com/v1.0/users/{user}")).unwrap() ); assert_eq!( - entra.group_info("0000-0000"), - Url::parse("http://graph.myentra.com:8080/v1.0/users/0000-0000/memberOf").unwrap() + entra.group_info(user), + Url::parse(&format!( + "https://graph.microsoft.com/v1.0/users/{user}/memberOf" + )) + .unwrap() ); } } From 45cf3ccd8df775d4fd82c86cffddb8c3b5807445 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 13:53:31 +0200 Subject: [PATCH 25/30] reduce tests further --- rust/user-info-fetcher/src/backend/entra.rs | 34 --------------------- 1 file changed, 34 deletions(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 71ebb42c..501ca62e 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -248,38 +248,4 @@ mod tests { .unwrap() ); } - - #[test] - fn test_entra_defaults_email() { - let tenant_id = "1234-5678-1234-5678"; - let user = "test@stackable.tech"; - - let entra = EntraBackend::try_new( - &HostName::from_str("login.microsoft.com").unwrap(), - &HostName::from_str("graph.microsoft.com").unwrap(), - 443, - tenant_id, - true, - ) - .unwrap(); - - assert_eq!( - entra.oauth2_token(), - Url::parse(&format!( - "https://login.microsoft.com/{tenant_id}/oauth2/v2.0/token" - )) - .unwrap() - ); - assert_eq!( - entra.user_info(user), - Url::parse(&format!("https://graph.microsoft.com/v1.0/users/{user}")).unwrap() - ); - assert_eq!( - entra.group_info(user), - Url::parse(&format!( - "https://graph.microsoft.com/v1.0/users/{user}/memberOf" - )) - .unwrap() - ); - } } From d0c127a1a04268a9142ec5b1019c077de09039a4 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 17:10:20 +0200 Subject: [PATCH 26/30] add documentation --- .../pages/usage-guide/user-info-fetcher.adoc | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc b/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc index 101b722f..1fb8cd04 100644 --- a/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc +++ b/docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc @@ -55,6 +55,7 @@ Currently the following backends are supported: * xref:#backend-keycloak[] * xref:#backend-activedirectory[] +* xref:#backend-entra[] [#backends] == Backends @@ -109,6 +110,29 @@ spec: <7> The name of the SecretClass that knows how to create Kerberos keytabs trusted by Active Directory <8> The name of the SecretClass that contains the Active Directory's root CA certificate(s) +[#backend-entra] +=== Entra + +WARNING: The Entra backend is experimental, and subject to change. + +Fetch groups but not roles for a user from Entra. + +NOTE: The client in Entra must use the `client_credentials` flow and requires the `User.ReadAll` and `GroupMemberShip.ReadAll` permissions. + +[source,yaml] +---- +spec: + clusterConfig: + userInfo: + backend: + experimentalEntra: # <1> + tenantId: 00000000-0000-0000-0000-000000000000 # <2> + clientCredentialsSecret: user-info-fetcher-client-credentials # <3> +---- +<1> Enables the Entra backend +<2> The Entra tenant ID +<3> A secret containing the `clientId` and `clientSecret` keys + == User info fetcher API User information can be retrieved from regorules using the functions `userInfoByUsername(username)` and `userInfoById(id)` in `data.stackable.opa.userinfo.v1`. From c93f2392f9e7fc0a5cb24054973a3b56c7635bde Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 17:12:21 +0200 Subject: [PATCH 27/30] adapted changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 903cbb21..7bae6f16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - Log the startup event for bundle-builder and user-info-fetcher ([#703]). +- Support expermimental user-info-fetcher Entra backend to fetch user groups ([#712]). ### Changed @@ -38,6 +39,7 @@ All notable changes to this project will be documented in this file. [#707]: https://github.com/stackabletech/opa-operator/pull/707 [#709]: https://github.com/stackabletech/opa-operator/pull/709 [#710]: https://github.com/stackabletech/opa-operator/pull/710 +[#712]: https://github.com/stackabletech/opa-operator/pull/712 ## [25.3.0] - 2025-03-21 From 616b51463411d0baacff6f797c8e57b1efa54d57 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 17 Apr 2025 17:12:50 +0200 Subject: [PATCH 28/30] fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bae6f16..db613bff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - Log the startup event for bundle-builder and user-info-fetcher ([#703]). -- Support expermimental user-info-fetcher Entra backend to fetch user groups ([#712]). +- Support experimental user-info-fetcher Entra backend to fetch user groups ([#712]). ### Changed From 4d10eaaaf8d0e83ab31a41d580335b087c89e082 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 22 Apr 2025 15:01:51 +0200 Subject: [PATCH 29/30] make port optional --- deploy/helm/opa-operator/crds/crds.yaml | 4 +- .../src/crd/user_info_fetcher.rs | 11 ++---- rust/user-info-fetcher/src/backend/entra.rs | 39 ++++++++++++++++++- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/deploy/helm/opa-operator/crds/crds.yaml b/deploy/helm/opa-operator/crds/crds.yaml index 331b3289..bfe07e38 100644 --- a/deploy/helm/opa-operator/crds/crds.yaml +++ b/deploy/helm/opa-operator/crds/crds.yaml @@ -149,10 +149,10 @@ spec: Must contain the fields `clientId` and `clientSecret`. type: string port: - default: 443 - description: Port of the identity provider. Defaults to 443. + description: Port of the identity provider. If TLS is used defaults to `443`, otherwise to `80`. format: uint16 minimum: 0.0 + nullable: true type: integer tenantId: description: The Microsoft Entra tenant ID. diff --git a/rust/operator-binary/src/crd/user_info_fetcher.rs b/rust/operator-binary/src/crd/user_info_fetcher.rs index 69eeb559..6bed814e 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher.rs @@ -128,9 +128,8 @@ pub mod versioned { #[serde(default = "entra_default_user_info_hostname")] pub user_info_hostname: HostName, - /// Port of the identity provider. Defaults to 443. - #[serde(default = "entra_default_port")] - pub port: u16, + /// Port of the identity provider. If TLS is used defaults to `443`, otherwise to `80`. + pub port: Option, /// The Microsoft Entra tenant ID. pub tenant_id: String, @@ -139,7 +138,7 @@ pub mod versioned { // We do not use the flattened `TlsClientDetails` here since we cannot // default to WebPki using a default and flatten // https://github.com/serde-rs/serde/issues/1626 - // This means we have to wrap `Tls` in `TlsClientDetails` to its + // This means we have to wrap `Tls` in `TlsClientDetails` to use its // method like `uses_tls()`. #[serde(default = "default_tls_web_pki")] pub tls: Option, @@ -178,10 +177,6 @@ fn entra_default_user_info_hostname() -> HostName { HostName::from_str("graph.microsoft.com").unwrap() } -fn entra_default_port() -> u16 { - 443 -} - fn default_tls_web_pki() -> Option { Some(Tls { verification: TlsVerification::Server(TlsServerVerification { diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 501ca62e..0e72c3c0 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -166,11 +166,12 @@ impl EntraBackend { pub fn try_new( token_endpoint: &HostName, user_info_endpoint: &HostName, - port: u16, + port: Option, tenant_id: &str, uses_tls: bool, ) -> Result { let schema = if uses_tls { "https" } else { "http" }; + let port = port.unwrap_or_else(|| if uses_tls { 443 } else { 80 }); let token_endpoint = format!("{schema}://{token_endpoint}:{port}/{tenant_id}/oauth2/v2.0/token"); @@ -223,7 +224,7 @@ mod tests { let entra = EntraBackend::try_new( &HostName::from_str("login.microsoft.com").unwrap(), &HostName::from_str("graph.microsoft.com").unwrap(), - 443, + None, tenant_id, true, ) @@ -248,4 +249,38 @@ mod tests { .unwrap() ); } + + #[test] + fn test_entra_custom_id() { + let tenant_id = "1234-5678-1234-5678"; + let user = "1234-5678-1234-5678"; + + let entra = EntraBackend::try_new( + &HostName::from_str("login.mock.com").unwrap(), + &HostName::from_str("graph.mock.com").unwrap(), + Some(8080), + tenant_id, + false, + ) + .unwrap(); + + assert_eq!( + entra.oauth2_token(), + Url::parse(&format!( + "http://login.mock.com:8080/{tenant_id}/oauth2/v2.0/token" + )) + .unwrap() + ); + assert_eq!( + entra.user_info(user), + Url::parse(&format!("http://graph.mock.com:8080/v1.0/users/{user}")).unwrap() + ); + assert_eq!( + entra.group_info(user), + Url::parse(&format!( + "http://graph.mock.com:8080/v1.0/users/{user}/memberOf" + )) + .unwrap() + ); + } } From cd1599951764ee8108fc851fee4fd997aebe01a1 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 22 Apr 2025 15:03:53 +0200 Subject: [PATCH 30/30] clippy --- rust/user-info-fetcher/src/backend/entra.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/user-info-fetcher/src/backend/entra.rs b/rust/user-info-fetcher/src/backend/entra.rs index 0e72c3c0..f261df50 100644 --- a/rust/user-info-fetcher/src/backend/entra.rs +++ b/rust/user-info-fetcher/src/backend/entra.rs @@ -171,7 +171,7 @@ impl EntraBackend { uses_tls: bool, ) -> Result { let schema = if uses_tls { "https" } else { "http" }; - let port = port.unwrap_or_else(|| if uses_tls { 443 } else { 80 }); + let port = port.unwrap_or(if uses_tls { 443 } else { 80 }); let token_endpoint = format!("{schema}://{token_endpoint}:{port}/{tenant_id}/oauth2/v2.0/token");