From 7249f917d6e95c60fe35417b80bbd672ce66b948 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 4 Nov 2024 17:44:29 +0100 Subject: [PATCH] fix: TLS CA cert path used in REQUESTS_CA_BUNDLE --- CHANGELOG.md | 3 +- rust/crd/src/authentication.rs | 158 +++++++++++++++++++++++---- rust/operator-binary/src/config.rs | 3 + rust/operator-binary/src/env_vars.rs | 18 +-- 4 files changed, 153 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68f9cbac..9a6832ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Use the env var `KUBERNETES_CLUSTER_DOMAIN` or the operator Helm chart property `kubernetesClusterDomain` to set a non-default cluster domain ([#518]). - Support for `2.9.3` ([#494]). - Experimental Support for `2.10.2` ([#512]). -- Add support for OpenID Connect ([#524]) +- Add support for OpenID Connect ([#524], [#529]) ### Changed @@ -32,6 +32,7 @@ [#518]: https://github.com/stackabletech/airflow-operator/pull/518 [#520]: https://github.com/stackabletech/airflow-operator/pull/520 [#524]: https://github.com/stackabletech/airflow-operator/pull/524 +[#529]: https://github.com/stackabletech/airflow-operator/pull/529 ## [24.7.0] - 2024-07-24 diff --git a/rust/crd/src/authentication.rs b/rust/crd/src/authentication.rs index d769f163..d9297ae4 100644 --- a/rust/crd/src/authentication.rs +++ b/rust/crd/src/authentication.rs @@ -4,10 +4,13 @@ use serde::{Deserialize, Serialize}; use snafu::{ensure, ResultExt, Snafu}; use stackable_operator::{ client::Client, - commons::authentication::{ - ldap, - oidc::{self, IdentityProviderHint}, - AuthenticationClass, AuthenticationClassProvider, ClientAuthenticationDetails, + commons::{ + authentication::{ + ldap, + oidc::{self, IdentityProviderHint}, + AuthenticationClass, AuthenticationClassProvider, ClientAuthenticationDetails, + }, + tls_verification::TlsClientDetails, }, schemars::{self, JsonSchema}, }; @@ -78,6 +81,9 @@ pub enum Error { supported: String, auth_class_name: String, }, + + #[snafu(display("Currently only one CA certificate is supported."))] + MultipleCaCertsNotSupported, } type Result = std::result::Result; @@ -130,17 +136,7 @@ pub struct AirflowClientAuthenticationDetailsResolved { pub user_registration: bool, pub user_registration_role: String, pub sync_roles_at: FlaskRolesSyncMoment, -} - -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum AirflowAuthenticationClassResolved { - Ldap { - provider: ldap::AuthenticationProvider, - }, - Oidc { - provider: oidc::AuthenticationProvider, - oidc: oidc::ClientAuthenticationOptions<()>, - }, + pub tls_ca_cert_mount_path: Option, } impl AirflowClientAuthenticationDetailsResolved { @@ -254,12 +250,24 @@ impl AirflowClientAuthenticationDetailsResolved { None => sync_roles_at = Some(entry.sync_roles_at.to_owned()), } } + + let mut tls_ca_cert_mount_paths = resolved_auth_classes + .iter() + .filter_map(AirflowAuthenticationClassResolved::tls_ca_cert_mount_path) + .collect::>(); + let tls_ca_cert_mount_path = tls_ca_cert_mount_paths.pop_first(); + ensure!( + tls_ca_cert_mount_paths.is_empty(), + MultipleCaCertsNotSupportedSnafu + ); + Ok(AirflowClientAuthenticationDetailsResolved { authentication_classes_resolved: resolved_auth_classes, user_registration: user_registration.unwrap_or_else(default_user_registration), user_registration_role: user_registration_role .unwrap_or_else(default_user_registration_role), sync_roles_at: sync_roles_at.unwrap_or_else(FlaskRolesSyncMoment::default), + tls_ca_cert_mount_path, }) } @@ -314,6 +322,35 @@ impl AirflowClientAuthenticationDetailsResolved { } } +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum AirflowAuthenticationClassResolved { + Ldap { + provider: ldap::AuthenticationProvider, + }, + Oidc { + provider: oidc::AuthenticationProvider, + oidc: oidc::ClientAuthenticationOptions<()>, + }, +} + +impl AirflowAuthenticationClassResolved { + pub fn tls_ca_cert_mount_path(&self) -> Option { + self.tls_client_details().tls_ca_cert_mount_path() + } + + pub fn tls_client_details(&self) -> &TlsClientDetails { + match self { + AirflowAuthenticationClassResolved::Ldap { + provider: ldap::AuthenticationProvider { tls, .. }, + } => tls, + AirflowAuthenticationClassResolved::Oidc { + provider: oidc::AuthenticationProvider { tls, .. }, + .. + } => tls, + } + } +} + #[cfg(test)] mod tests { use std::pin::Pin; @@ -336,7 +373,8 @@ mod tests { authentication_classes_resolved: Vec::default(), user_registration: default_user_registration(), user_registration_role: default_user_registration_role(), - sync_roles_at: FlaskRolesSyncMoment::default() + sync_roles_at: FlaskRolesSyncMoment::default(), + tls_ca_cert_mount_path: None, }, auth_details_resolved ); @@ -362,6 +400,11 @@ mod tests { provider: ldap: hostname: my.ldap.server + tls: + verification: + server: + caCert: + secretClass: tls "}, ) .await; @@ -369,11 +412,20 @@ mod tests { assert_eq!( AirflowClientAuthenticationDetailsResolved { authentication_classes_resolved: vec![AirflowAuthenticationClassResolved::Ldap { - provider: serde_yaml::from_str("hostname: my.ldap.server").unwrap() + provider: serde_yaml::from_str(indoc! {" + hostname: my.ldap.server + tls: + verification: + server: + caCert: + secretClass: tls + "}) + .unwrap() }], user_registration: false, user_registration_role: "Gamma".into(), - sync_roles_at: FlaskRolesSyncMoment::Login + sync_roles_at: FlaskRolesSyncMoment::Login, + tls_ca_cert_mount_path: Some("/stackable/secrets/tls/ca.crt".into()), }, auth_details_resolved ); @@ -437,6 +489,11 @@ mod tests { - openid - email - profile + tls: + verification: + server: + caCert: + secretClass: tls "}, ) .await; @@ -471,7 +528,13 @@ mod tests { HostName::try_from("second.oidc.server".to_string()).unwrap(), None, "/realms/test".into(), - TlsClientDetails { tls: None }, + TlsClientDetails { + tls: Some(Tls { + verification: TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::SecretClass("tls".into()) + }) + }) + }, "preferred_username".into(), vec!["openid".into(), "email".into(), "profile".into()], None @@ -485,7 +548,8 @@ mod tests { ], user_registration: false, user_registration_role: "Gamma".into(), - sync_roles_at: FlaskRolesSyncMoment::Login + sync_roles_at: FlaskRolesSyncMoment::Login, + tls_ca_cert_mount_path: Some("/stackable/secrets/tls/ca.crt".into()), }, auth_details_resolved ); @@ -829,6 +893,60 @@ mod tests { ); } + #[tokio::test] + async fn reject_different_tls_ca_certs() { + let error_message = test_resolve_and_expect_error( + indoc! {" + - authenticationClass: oidc1 + oidc: + clientCredentialsSecret: airflow-oidc-client1 + - authenticationClass: oidc2 + oidc: + clientCredentialsSecret: airflow-oidc-client2 + "}, + indoc! {" + --- + apiVersion: authentication.stackable.tech/v1alpha1 + kind: AuthenticationClass + metadata: + name: oidc1 + spec: + provider: + oidc: + hostname: first.oidc.server + principalClaim: preferred_username + scopes: [] + tls: + verification: + server: + caCert: + secretClass: tls1 + --- + apiVersion: authentication.stackable.tech/v1alpha1 + kind: AuthenticationClass + metadata: + name: oidc2 + spec: + provider: + oidc: + hostname: second.oidc.server + principalClaim: preferred_username + scopes: [] + tls: + verification: + server: + caCert: + secretClass: tls2 + "}, + ) + .await; + + assert_eq!( + "Currently only one CA certificate is supported.", + error_message + ); + } + /// Call `AirflowClientAuthenticationDetailsResolved::resolve` with /// the given lists of `AirflowClientAuthenticationDetails` and /// `AuthenticationClass`es and return the diff --git a/rust/operator-binary/src/config.rs b/rust/operator-binary/src/config.rs index 2d7feff7..2ed28a7d 100644 --- a/rust/operator-binary/src/config.rs +++ b/rust/operator-binary/src/config.rs @@ -271,6 +271,7 @@ mod tests { user_registration: true, user_registration_role: "User".to_string(), sync_roles_at: FlaskRolesSyncMoment::Registration, + tls_ca_cert_mount_path: None, }; let mut result = BTreeMap::new(); @@ -314,6 +315,7 @@ mod tests { user_registration: true, user_registration_role: "Admin".to_string(), sync_roles_at: FlaskRolesSyncMoment::Registration, + tls_ca_cert_mount_path: Some("/stackable/secrets/openldap-tls/ca.crt".to_string()), }; let mut result = BTreeMap::new(); @@ -394,6 +396,7 @@ mod tests { user_registration: default_user_registration(), user_registration_role: "Admin".to_string(), sync_roles_at: default_sync_roles_at(), + tls_ca_cert_mount_path: Some("keycloak-ca-cert".to_string()), }; let mut result = BTreeMap::new(); diff --git a/rust/operator-binary/src/env_vars.rs b/rust/operator-binary/src/env_vars.rs index 5d581090..8b160cc6 100644 --- a/rust/operator-binary/src/env_vars.rs +++ b/rust/operator-binary/src/env_vars.rs @@ -200,14 +200,16 @@ pub fn build_airflow_statefulset_envs( AirflowRole::Webserver => { let auth_vars = authentication_env_vars(auth_config); env.extend(auth_vars.into_iter().map(|var| (var.name.to_owned(), var))); - env.insert( - "REQUESTS_CA_BUNDLE".into(), - EnvVar { - name: "REQUESTS_CA_BUNDLE".to_string(), - value: Some("/stackable/secrets/tls/ca.crt".to_string()), - ..Default::default() - }, - ); + if let Some(tls_ca_cert_mount_path) = &auth_config.tls_ca_cert_mount_path { + env.insert( + "REQUESTS_CA_BUNDLE".into(), + EnvVar { + name: "REQUESTS_CA_BUNDLE".to_string(), + value: Some(tls_ca_cert_mount_path.to_owned()), + ..Default::default() + }, + ); + } } _ => {} }