Allow AD integration to filter LDAP groups (#693)

* Allow AD integration to filter LDAP groups

Fixes #691

* Rename `customGroupAttributeFilters` to `additionalGroupAttributeFilters`

* Docs

* Changelog

Author: nightkr

CHANGELOG.md

@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 - Run a `containerdebug` process in the background of each OPA container to collect debugging information ([#666]).
 - Added support for OPA `1.0.x` ([#677]) and ([#687]).
 - Aggregate emitted Kubernetes events on the CustomResources ([#675]).
+- Added support for filtering groups searched by Active Directory ([#693]).
 
 ### Removed
 
@@ -23,6 +24,7 @@ All notable changes to this project will be documented in this file.
 [#675]: https://github.com/stackabletech/opa-operator/pull/675
 [#677]: https://github.com/stackabletech/opa-operator/pull/677
 [#687]: https://github.com/stackabletech/opa-operator/pull/687
+[#693]: https://github.com/stackabletech/opa-operator/pull/693
 
 ## [24.11.1] - 2025-01-10
 

deploy/helm/opa-operator/crds/crds.yaml

@@ -68,6 +68,15 @@ spec:
                             experimentalActiveDirectory:
                               description: Backend that fetches user information from Active Directory
                               properties:
+                                additionalGroupAttributeFilters:
+                                  additionalProperties:
+                                    type: string
+                                  default: {}
+                                  description: |-
+                                    Attributes that groups must have to be returned.
+
+                                    These fields will be spliced into an LDAP Search Query, so wildcards can be used, but characters with a special meaning in LDAP will need to be escaped.
+                                  type: object
                                 baseDistinguishedName:
                                   description: The root Distinguished Name (DN) where users and groups are located.
                                   type: string

docs/modules/opa/pages/usage-guide/user-info-fetcher.adoc

@@ -89,12 +89,14 @@ spec:
           baseDistinguishedName: DC=sble,DC=test # <3>
           customAttributeMappings: # <4>
             country: c # <5>
-          kerberosSecretClassName: kerberos-ad # <6>
+          additionalGroupAttributeFilters: # <6>
+            foo: bar
+          kerberosSecretClassName: kerberos-ad # <7>
           tls:
             verification:
               server:
                 caCert:
-                  secretClass: tls-ad # <7>
+                  secretClass: tls-ad # <8>
       cache: # optional, enabled by default
         entryTimeToLive: 60s # optional, defaults to 60s
 ----
@@ -103,8 +105,9 @@ spec:
 <3> The distinguished name to search, users and groups outside of this will not be seen
 <4> Arbitrary LDAP attributes can be requested to be fetched
 <5> https://learn.microsoft.com/en-us/windows/win32/ad/address-book-properties[`c`] stores the ISO-3166 country code of the user
-<6> The name of the SecretClass that knows how to create Kerberos keytabs trusted by Active Directory
-<7> The name of the SecretClass that contains the Active Directory's root CA certificate(s)
+<6> Groups can be filtered by LDAP attributes to reduce the load in searching for membership. `*` can be used as a wildcard in these filters.
+<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)
 
 == User info fetcher API
 

rust/operator-binary/src/crd/user_info_fetcher.rs

@@ -101,6 +101,13 @@ pub mod versioned {
         /// Custom attributes, and their LDAP attribute names.
         #[serde(default)]
         pub custom_attribute_mappings: BTreeMap<String, String>,
+
+        /// Attributes that groups must have to be returned.
+        ///
+        /// These fields will be spliced into an LDAP Search Query, so wildcards can be used,
+        /// but characters with a special meaning in LDAP will need to be escaped.
+        #[serde(default)]
+        pub additional_group_attribute_filters: BTreeMap<String, String>,
     }
 
     #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]

rust/user-info-fetcher/src/backend/active_directory.rs

@@ -1,6 +1,6 @@
 use std::{
     collections::{BTreeMap, HashMap},
-    fmt::Display,
+    fmt::{Display, Write},
     io::{Cursor, Read},
     num::ParseIntError,
     str::FromStr,
@@ -90,13 +90,19 @@ const LDAP_FIELD_USER_NAME: &str = "userPrincipalName";
 const LDAP_FIELD_USER_PRIMARY_GROUP_RID: &str = "primaryGroupID";
 const LDAP_FIELD_GROUP_MEMBER: &str = "member";
 
-#[tracing::instrument(skip(tls, base_distinguished_name, custom_attribute_mappings))]
+#[tracing::instrument(skip(
+    tls,
+    base_distinguished_name,
+    custom_attribute_mappings,
+    additional_group_attribute_filters,
+))]
 pub(crate) async fn get_user_info(
     request: &UserInfoRequest,
     ldap_server: &str,
     tls: &TlsClientDetails,
     base_distinguished_name: &str,
     custom_attribute_mappings: &BTreeMap<String, String>,
+    additional_group_attribute_filters: &BTreeMap<String, String>,
 ) -> Result<UserInfo, Error> {
     let ldap_tls = utils::tls::configure_native_tls(tls)
         .await
@@ -168,16 +174,27 @@ pub(crate) async fn get_user_info(
         base_distinguished_name,
         &user,
         custom_attribute_mappings,
+        additional_group_attribute_filters,
     )
     .await
 }
 
-#[tracing::instrument(skip(ldap, base_dn, user, custom_attribute_mappings), fields(user.dn))]
+#[tracing::instrument(
+    skip(
+        ldap,
+        base_dn,
+        user,
+        custom_attribute_mappings,
+        additional_group_attribute_filters,
+    ),
+    fields(user.dn),
+)]
 async fn user_attributes(
     ldap: &mut Ldap,
     base_dn: &str,
     user: &SearchEntry,
     custom_attribute_mappings: &BTreeMap<String, String>,
+    additional_group_attribute_filters: &BTreeMap<String, String>,
 ) -> Result<UserInfo, Error> {
     let user_sid = user
         .bin_attrs
@@ -242,7 +259,14 @@ async fn user_attributes(
         })
         .collect::<HashMap<_, _>>();
     let groups = if let Some(user_sid) = &user_sid {
-        user_group_distinguished_names(ldap, base_dn, user, user_sid).await?
+        user_group_distinguished_names(
+            ldap,
+            base_dn,
+            user,
+            user_sid,
+            additional_group_attribute_filters,
+        )
+        .await?
     } else {
         tracing::debug!(user.dn, "user has no SID, cannot fetch groups...");
         Vec::new()
@@ -257,12 +281,13 @@ async fn user_attributes(
 }
 
 /// Gets the distinguished names of all of `user`'s groups, both primary and secondary.
-#[tracing::instrument(skip(ldap, base_dn, user, user_sid))]
+#[tracing::instrument(skip(ldap, base_dn, user, user_sid, additional_group_attribute_filters))]
 async fn user_group_distinguished_names(
     ldap: &mut Ldap,
     base_dn: &str,
     user: &SearchEntry,
     user_sid: &SecurityId,
+    additional_group_attribute_filters: &BTreeMap<String, String>,
 ) -> Result<Vec<String>, Error> {
     // User group memberships are tricky, because users have exactly one *primary* and any number of *secondary* groups.
     // Additionally groups can be members of other groups.
@@ -309,10 +334,22 @@ async fn user_group_distinguished_names(
         "({LDAP_FIELD_GROUP_MEMBER}{LDAP_MATCHING_RULE_IN_CHAIN}=<SID={primary_group_sid}>)"
     );
 
+    // Users can also specify custom filters via `group_attribute_filters`
+    let custom_group_filter =
+        additional_group_attribute_filters
+            .iter()
+            .fold(String::new(), |mut out, (k, v)| {
+                // NOTE: This is technically an LDAP injection vuln, but these are provided statically by the OPA administrator,
+                // who would be able to do plenty of other harm... (like providing their own OPA images that do whatever they want).
+                // We could base64 the value to "defuse" it entirely, but that would also prevent using wildcards.
+                write!(out, "({k}={v})").expect("string concatenation is infallible");
+                out
+            });
+
     // Let's put it all together, and make it go...
     let groups_filter =
         format!("(|{primary_group_filter}{primary_group_parents_filter}{secondary_groups_filter})");
-    let groups_query_filter = format!("(&(objectClass=group){groups_filter})");
+    let groups_query_filter = format!("(&(objectClass=group){custom_group_filter}{groups_filter})");
     let requested_group_attrs = [LDAP_FIELD_OBJECT_DISTINGUISHED_NAME];
     tracing::debug!(
         groups_query_filter,

rust/user-info-fetcher/src/main.rs

@@ -300,6 +300,7 @@ async fn get_user_info(
                             &ad.tls,
                             &ad.base_distinguished_name,
                             &ad.custom_attribute_mappings,
+                            &ad.additional_group_attribute_filters,
                         )
                         .await
                         .context(get_user_info_error::ActiveDirectorySnafu)