From: Seamus Lee <seamuslee001@gmail.com>
Date: Fri, 23 Jun 2023 00:17:27 +0000 (+1000)
Subject: Retain the concept of an all groups rule == all contacts access
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=5a5f698d1e72ecae19408c7c2f1a9ab94a5ed888;p=civicrm-core.git

Retain the concept of an all groups rule == all contacts access
---

diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php
index 1a9c66ae3b..c3dee1f460 100644
--- a/CRM/ACL/BAO/ACL.php
+++ b/CRM/ACL/BAO/ACL.php
@@ -9,8 +9,6 @@
  +--------------------------------------------------------------------+
  */
 
-use Civi\Api4\Group;
-
 /**
  *
  * @package CRM
@@ -225,6 +223,7 @@ SELECT count( a.id )
     $acls = CRM_ACL_BAO_Cache::build($contactID);
 
     $whereClause = NULL;
+    $allInclude = $allExclude = FALSE;
     $clauses = [];
 
     if (!empty($acls)) {
@@ -247,54 +246,37 @@ ORDER BY {$orderBy}
       $dao = CRM_Core_DAO::executeQuery($query);
 
       // do an or of all the where clauses u see
-      $ids = [];
+      $ids = $excludeIds = [];
       while ($dao->fetch()) {
         // make sure operation matches the type TODO
         if (self::matchType($type, $dao->operation)) {
           if (!$dao->deny) {
             if (empty($dao->object_id)) {
-              $ids = array_merge($ids, Group::get(FALSE)->execute()->column('id'));
+              $allInclude = TRUE;
             }
             else {
               $ids[] = $dao->object_id;
             }
           }
           else {
-            $ids = array_diff($ids, [$dao->object_id]);
+            if (empty($dao->object_id)) {
+              $allExclude = TRUE;
+            }
+            else {
+              $excludeIds[] = $dao->object_id;
+            }
           }
         }
       }
+      if (!empty($excludeIds) && !$allInclude) {
+        $ids = array_diff($ids, $excludeIds);
+      }
+      elseif (!empty($excludeIds) && $allInclude) {
+        $ids = [];
+        $clauses[] = self::getGroupClause($excludeIds, 'NOT IN');
+      }
       if (!empty($ids)) {
-        $ids = implode(',', $ids);
-        $query = "
-SELECT g.*
-  FROM civicrm_group g
- WHERE g.id IN ( $ids )
- AND   g.is_active = 1
-";
-        $dao = CRM_Core_DAO::executeQuery($query);
-        $groupIDs = [];
-        $groupContactCacheClause = FALSE;
-        while ($dao->fetch()) {
-          $groupIDs[] = $dao->id;
-
-          if (($dao->saved_search_id || $dao->children || $dao->parents)) {
-            if ($dao->cache_date == NULL) {
-              CRM_Contact_BAO_GroupContactCache::load($dao);
-            }
-            $groupContactCacheClause = " UNION SELECT contact_id FROM civicrm_group_contact_cache WHERE group_id IN (" . implode(', ', $groupIDs) . ")";
-          }
-
-        }
-
-        if ($groupIDs) {
-          $clauses[] = "(
-            `contact_a`.id IN (
-               SELECT contact_id FROM civicrm_group_contact WHERE group_id IN (" . implode(', ', $groupIDs) . ") AND status = 'Added'
-               $groupContactCacheClause
-             )
-          )";
-        }
+        $clauses[] = self::getGroupClause($ids, 'IN');
       }
     }
 
@@ -515,4 +497,36 @@ ORDER BY {$orderBy}
     return $ids;
   }
 
+  private static function getGroupClause(array $groupIDs, string $operation): string {
+    $ids = implode(',', $groupIDs);
+    $query = "
+SELECT g.*
+  FROM civicrm_group g
+ WHERE g.id IN ( $ids )
+ AND   g.is_active = 1
+";
+    $dao = CRM_Core_DAO::executeQuery($query);
+    $foundGroupIDs = [];
+    $groupContactCacheClause = FALSE;
+    while ($dao->fetch()) {
+      $foundGroupIDs[] = $dao->id;
+      if (($dao->saved_search_id || $dao->children || $dao->parents)) {
+        if ($dao->cache_date == NULL) {
+          CRM_Contact_BAO_GroupContactCache::load($dao);
+        }
+        $groupContactCacheClause = " UNION SELECT contact_id FROM civicrm_group_contact_cache WHERE group_id IN (" . implode(', ', $foundGroupIDs) . ")";
+      }
+    }
+
+    if ($groupIDs) {
+      return "(
+        `contact_a`.id $operation (
+         SELECT contact_id FROM civicrm_group_contact WHERE group_id IN (" . implode(', ', $foundGroupIDs) . ") AND status = 'Added'
+           $groupContactCacheClause
+         )
+      )";
+    }
+    return '';
+  }
+
 }
diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index ea2166ddb8..751f5984ae 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -1400,6 +1400,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
     ]);
     $contact1 = $this->individualCreate();
     $contact2 = $this->individualCreate();
+    $contact3 = $this->individualCreate();
     $excludeGroup = $this->groupCreate(['title' => 'exclude group', 'name' => 'exclude group']);
     $otherGroup = $this->groupCreate(['title' => 'Other group', 'name' => 'other group']);
     $this->callAPISuccess('GroupContact', 'create', [
@@ -1425,7 +1426,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
       'entity_table' => 'civicrm_acl_role',
       'entity_id' => 6,
       'operation' => 'Edit',
-      'object_table' => 'civicrm_saved_search',
+      'object_table' => 'civicrm_group',
       'object_id' => 0,
     ]);
     $this->callAPISuccess('Acl', 'create', [
@@ -1434,7 +1435,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
       'entity_table' => 'civicrm_acl_role',
       'entity_id' => 6,
       'operation' => 'Edit',
-      'object_table' => 'civicrm_saved_search',
+      'object_table' => 'civicrm_group',
       'object_id' => $excludeGroup,
       'deny' => 1,
     ]);
@@ -1451,9 +1452,10 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
     $this->cleanupCachedPermissions();
     Civi::cache('metadata')->clear();
     Civi::$statics['CRM_ACL_BAO_ACL'] = [];
-    $contacts = Contact::get()->addWhere('id', 'IN', [$contact1, $contact2])->execute();
-    $this->assertCount(1, $contacts);
+    $contacts = Contact::get()->addWhere('id', 'IN', [$contact1, $contact2, $contact3])->execute();
+    $this->assertCount(2, $contacts);
     $this->assertEquals($contact2, $contacts[0]['id']);
+    $this->assertEquals($contact3, $contacts[1]['id']);
   }
 
   /**