From e5a5de8de37eade904f2b3ada71992e246a520af Mon Sep 17 00:00:00 2001
From: Firstyear <william@blackhats.net.au>
Date: Wed, 4 Sep 2024 22:19:40 +1000
Subject: [PATCH] MemberOf in search implies DirectMemberOf (#3024)

---
 server/lib/src/server/access/mod.rs      | 47 ++++++++++++++++++++++++
 server/lib/src/server/access/profiles.rs | 16 +++++++-
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/server/lib/src/server/access/mod.rs b/server/lib/src/server/access/mod.rs
index 1b51c4a5d..e8dd80fd4 100644
--- a/server/lib/src/server/access/mod.rs
+++ b/server/lib/src/server/access/mod.rs
@@ -3134,4 +3134,51 @@ mod tests {
         // Test reject delete, can not delete due to system protection
         test_acp_delete!(&de_account, vec![acp], &r_set, false);
     }
+
+    #[test]
+    fn test_access_sync_memberof_implies_directmemberof() {
+        sketching::test_init();
+
+        let ev1 = entry_init!(
+            (Attribute::Class, EntryClass::Object.to_value()),
+            (Attribute::Name, Value::new_iname("test_account_1")),
+            (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)),
+            (Attribute::MemberOf, Value::Refer(UUID_TEST_GROUP_1)),
+            (Attribute::DirectMemberOf, Value::Refer(UUID_TEST_GROUP_1))
+        )
+        .into_sealed_committed();
+        let r_set = vec![Arc::new(ev1)];
+
+        let exv1 = entry_init!(
+            (Attribute::Name, Value::new_iname("test_account_1")),
+            (Attribute::MemberOf, Value::Refer(UUID_TEST_GROUP_1)),
+            (Attribute::DirectMemberOf, Value::Refer(UUID_TEST_GROUP_1))
+        )
+        .into_sealed_committed();
+
+        let ex_anon_some = vec![exv1];
+
+        let se_anon_ro = SearchEvent::new_impersonate_identity(
+            Identity::from_impersonate_entry_readonly(E_TEST_ACCOUNT_1.clone()),
+            filter_all!(f_pres(Attribute::Name)),
+        );
+
+        let acp = AccessControlSearch::from_raw(
+            "test_acp",
+            Uuid::new_v4(),
+            // apply to all accounts.
+            UUID_TEST_GROUP_1,
+            // Allow anonymous to read only testperson1
+            filter_valid!(f_eq(
+                Attribute::Uuid,
+                PartialValue::Uuid(UUID_TEST_ACCOUNT_1)
+            )),
+            // May query on name, and see memberof. MemberOf implies direct
+            // memberof.
+            format!("{} {}", Attribute::Name, Attribute::MemberOf).as_str(),
+        );
+
+        // Finally test it!
+        test_acp_search_reduce!(&se_anon_ro, vec![acp], r_set, ex_anon_some);
+    }
 }
diff --git a/server/lib/src/server/access/profiles.rs b/server/lib/src/server/access/profiles.rs
index 6665fb4aa..a0911233a 100644
--- a/server/lib/src/server/access/profiles.rs
+++ b/server/lib/src/server/access/profiles.rs
@@ -35,7 +35,7 @@ impl AccessControlSearch {
             )));
         }
 
-        let attrs = value
+        let mut attrs: BTreeSet<_> = value
             .get_ava_iter_iutf8(Attribute::AcpSearchAttr)
             .ok_or_else(|| {
                 admin_error!("Missing {}", Attribute::AcpSearchAttr);
@@ -44,6 +44,11 @@ impl AccessControlSearch {
             .map(AttrString::from)
             .collect();
 
+        // Ability to search memberof, implies the ability to read directmemberof
+        if attrs.contains(Attribute::MemberOf.as_ref()) {
+            attrs.insert(Attribute::DirectMemberOf.into());
+        }
+
         let acp = AccessControlProfile::try_from(qs, value)?;
 
         Ok(AccessControlSearch { acp, attrs })
@@ -59,6 +64,13 @@ impl AccessControlSearch {
         targetscope: Filter<FilterValid>,
         attrs: &str,
     ) -> Self {
+        let mut attrs: BTreeSet<_> = attrs.split_whitespace().map(AttrString::from).collect();
+
+        // Ability to search memberof, implies the ability to read directmemberof
+        if attrs.contains(Attribute::MemberOf.as_ref()) {
+            attrs.insert(Attribute::DirectMemberOf.into());
+        }
+
         AccessControlSearch {
             acp: AccessControlProfile {
                 name: name.to_string(),
@@ -66,7 +78,7 @@ impl AccessControlSearch {
                 receiver: AccessControlReceiver::Group(btreeset!(receiver)),
                 target: AccessControlTarget::Scope(targetscope),
             },
-            attrs: attrs.split_whitespace().map(AttrString::from).collect(),
+            attrs,
         }
     }