fix: TLS CA cert path used in REQUESTS_CA_BUNDLE

siegfriedweber · siegfriedweber · commit 7249f917d6e9 · 2024-11-04T20:47:33.000+01:00
diff --git 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
@@ -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<T, E = Error> = std::result::Result<T, E>;
@@ -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<String>,
 }
 
 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::<BTreeSet<_>>();
+        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<String> {
+        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,18 +400,32 @@ mod tests {
                   provider:
                     ldap:
                       hostname: my.ldap.server
+                      tls:
+                        verification:
+                          server:
+                            caCert:
+                              secretClass: tls
             "},
         )
         .await;
 
         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
@@ -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
@@ -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()
+                    },
+                );
+            }
         }
         _ => {}
     }
