From bc6e64730f7008cf897a5e27316c2411d8357d3d Mon Sep 17 00:00:00 2001
From: Seamus Lee <seamuslee001@gmail.com>
Date: Thu, 22 Jun 2023 08:57:20 +1000
Subject: [PATCH] Add weight column to ACLs and support weighted ACL rules

Add in upgrade step

Rename field to be priority and add in upgrade guards as per Coleman

Updates as per discussion with coleman
---
 CRM/ACL/BAO/ACL.php                           | 34 +++++++--
 CRM/ACL/DAO/ACL.php                           | 39 +++++++++-
 CRM/ACL/Form/ACL.php                          |  2 +
 CRM/ACL/Page/ACL.php                          |  1 +
 CRM/Upgrade/Incremental/php/FiveSixtyFour.php |  1 +
 templates/CRM/ACL/Form/ACL.tpl                |  6 ++
 templates/CRM/ACL/Page/ACL.tpl                |  2 +
 tests/phpunit/api/v3/ACLPermissionTest.php    | 72 +++++++++++++++++++
 xml/schema/ACL/ACL.xml                        | 14 ++++
 9 files changed, 166 insertions(+), 5 deletions(-)

diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php
index 977f3d3997..ac8c86c78d 100644
--- a/CRM/ACL/BAO/ACL.php
+++ b/CRM/ACL/BAO/ACL.php
@@ -529,23 +529,49 @@ ORDER BY a.object_id
     $acls = CRM_ACL_BAO_Cache::build($contactID);
     $aclKeys = array_keys($acls);
     $aclKeys = implode(',', $aclKeys);
+    $select = "a.operation, a.object_id";
+    $hasPriorty = FALSE;
+    if (array_key_exists('priority', CRM_ACL_BAO_ACL::getSupportedFields())) {
+      $select .= ",a.priority";
+      $hasPriority = TRUE;
+    }
     $query = "
-SELECT   a.operation, a.object_id
+SELECT   {$select}
   FROM   civicrm_acl_cache c, civicrm_acl a
  WHERE   c.acl_id       =  a.id
    AND   a.is_active    =  1
    AND   a.object_table = %1
    AND   a.id        IN ( $aclKeys )
    AND   a.deny         = 1
-GROUP BY a.operation,a.object_id
-ORDER BY a.object_id
 ";
     $params = [1 => [$tableName, 'String']];
     $dao = CRM_Core_DAO::executeQuery($query, $params);
     while ($dao->fetch()) {
       if ($dao->object_id) {
         if (self::matchType($type, $dao->operation)) {
-          $ids[] = $dao->object_id;
+          if ($hasPriority) {
+            $permittedRules = CRM_Core_DAO::singleValueQuery("SELECT count(a.id)
+              FROM civicrm_acl a
+              INNER JOIN civicrm_acl_cache c ON c.acl_id = a.id
+              WHERE a.id IN ( $aclKeys )
+              AND a.is_active = 1
+              AND a.object_table = %1
+              AND a.deny = 0
+              AND a.priority > %2
+              AND a.object_id = %3", [
+                1 => [$tableName, 'String'],
+                2 => [(int) $dao->priority, 'Integer'],
+                3 => [$dao->object_id, 'Integer'],
+              ]);
+            if (empty($permittedRules) && !in_array($dao->object_id, $ids, TRUE)) {
+              $ids[] = $dao->object_id;
+            }
+          }
+          else {
+            if (!in_array($dao->object_id, $ids, TRUE)) {
+              $ids[] = $dao->object_id;
+            }
+          }
         }
       }
     }
diff --git a/CRM/ACL/DAO/ACL.php b/CRM/ACL/DAO/ACL.php
index 16e184652a..4abc03ab4c 100644
--- a/CRM/ACL/DAO/ACL.php
+++ b/CRM/ACL/DAO/ACL.php
@@ -6,7 +6,7 @@
  *
  * Generated from xml/schema/CRM/ACL/ACL.xml
  * DO NOT EDIT.  Generated by CRM_Core_CodeGen
- * (GenCodeChecksum:f21ef3073d6247d130341cd182793ea6)
+ * (GenCodeChecksum:9d50ed80344474830f87df285dc6cbf2)
  */
 
 /**
@@ -129,6 +129,13 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO {
    */
   public $is_active;
 
+  /**
+   * @var int|string
+   *   (SQL type: int)
+   *   Note that values will be retrieved from the database as a string.
+   */
+  public $priority;
+
   /**
    * Class constructor.
    */
@@ -403,6 +410,28 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO {
           ],
           'add' => '1.6',
         ],
+        'priority' => [
+          'name' => 'priority',
+          'type' => CRM_Utils_Type::T_INT,
+          'title' => ts('Priority'),
+          'required' => TRUE,
+          'usage' => [
+            'import' => FALSE,
+            'export' => FALSE,
+            'duplicate_matching' => FALSE,
+            'token' => FALSE,
+          ],
+          'where' => 'civicrm_acl.priority',
+          'default' => '0',
+          'table_name' => 'civicrm_acl',
+          'entity' => 'ACL',
+          'bao' => 'CRM_ACL_BAO_ACL',
+          'localizable' => 0,
+          'html' => [
+            'type' => 'Number',
+          ],
+          'add' => '5.64',
+        ],
       ];
       CRM_Core_DAO_AllCoreTables::invoke(__CLASS__, 'fields_callback', Civi::$statics[__CLASS__]['fields']);
     }
@@ -481,6 +510,14 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO {
         'localizable' => FALSE,
         'sig' => 'civicrm_acl::0::acl_id',
       ],
+      'index_priority' => [
+        'name' => 'index_priority',
+        'field' => [
+          0 => 'priority',
+        ],
+        'localizable' => FALSE,
+        'sig' => 'civicrm_acl::0::priority',
+      ],
     ];
     return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices;
   }
diff --git a/CRM/ACL/Form/ACL.php b/CRM/ACL/Form/ACL.php
index 10e6369f50..4ac78946d1 100644
--- a/CRM/ACL/Form/ACL.php
+++ b/CRM/ACL/Form/ACL.php
@@ -29,6 +29,7 @@ class CRM_ACL_Form_ACL extends CRM_Admin_Form {
 
     if ($this->_action & CRM_Core_Action::ADD) {
       $defaults['object_type'] = 1;
+      $defaults['priority'] = 0;
     }
 
     $showHide = new CRM_Core_ShowHideBlocks();
@@ -164,6 +165,7 @@ class CRM_ACL_Form_ACL extends CRM_Admin_Form {
       0 => ts('Allow'),
       1 => ts('Deny'),
     ]);
+    $this->add('number', 'priority', ts('Priority'));
 
     $this->addFormRule(['CRM_ACL_Form_ACL', 'formRule']);
   }
diff --git a/CRM/ACL/Page/ACL.php b/CRM/ACL/Page/ACL.php
index 8c3b3b79a1..b5d3cc8955 100644
--- a/CRM/ACL/Page/ACL.php
+++ b/CRM/ACL/Page/ACL.php
@@ -137,6 +137,7 @@ ORDER BY entity_id
       $acl[$dao->id]['object_id'] = $dao->object_id;
       $acl[$dao->id]['is_active'] = $dao->is_active;
       $acl[$dao->id]['deny'] = $dao->deny;
+      $acl[$dao->id]['priority'] = $dao->priority;
 
       if ($acl[$dao->id]['entity_id']) {
         $acl[$dao->id]['entity'] = $roles[$acl[$dao->id]['entity_id']] ?? NULL;
diff --git a/CRM/Upgrade/Incremental/php/FiveSixtyFour.php b/CRM/Upgrade/Incremental/php/FiveSixtyFour.php
index b6a5ead8b4..fdd22f1757 100644
--- a/CRM/Upgrade/Incremental/php/FiveSixtyFour.php
+++ b/CRM/Upgrade/Incremental/php/FiveSixtyFour.php
@@ -28,6 +28,7 @@ class CRM_Upgrade_Incremental_php_FiveSixtyFour extends CRM_Upgrade_Incremental_
    *   The version number matching this function name
    */
   public function upgrade_5_64_alpha1($rev): void {
+    $this->addTask('Add priority column onto ACL table', 'addColumn', 'civicrm_acl', 'priority', 'int NOT NULL DEFAULT 0');
     $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
     $this->addTask('Update post_URL/cancel_URL in logging tables', 'updateLogging');
   }
diff --git a/templates/CRM/ACL/Form/ACL.tpl b/templates/CRM/ACL/Form/ACL.tpl
index 583be52ac0..d51c24a97f 100644
--- a/templates/CRM/ACL/Form/ACL.tpl
+++ b/templates/CRM/ACL/Form/ACL.tpl
@@ -49,6 +49,12 @@
        <td class="label">{$form.deny.label}</td>
        <td>{$form.deny.html}</td>
      </tr>
+     <tr class="crm-acl-form-block-priority">
+       <td class="label">{$form.priority.label}</td>
+       <td>{$form.priority.label}<br />
+         <span class="description">{ts}Higher priority ACL rules will override lower priority rules{/ts}</span>
+       </td>
+     </tr>
   </table>
   <div id="id-group-acl">
    <table  class="form-layout-compressed">
diff --git a/templates/CRM/ACL/Page/ACL.tpl b/templates/CRM/ACL/Page/ACL.tpl
index f5b54b9c26..3f3850fe29 100644
--- a/templates/CRM/ACL/Page/ACL.tpl
+++ b/templates/CRM/ACL/Page/ACL.tpl
@@ -31,6 +31,7 @@
               <th>{ts}Description{/ts}</th>
               <th>{ts}Enabled?{/ts}</th>
               <th>{ts}Mode{/ts}</th>
+              <th>{ts}Priority{/ts}</th>
               <th></th>
             </tr>
             </thead>
@@ -44,6 +45,7 @@
                 <td class="crm-acl-name crm-editable" data-field="name">{$row.name}</td>
                 <td class="crm-acl-is_active" id="row_{$aclID}_status">{if $row.is_active eq 1} {ts}Yes{/ts} {else} {ts}No{/ts} {/if}</td>
                 <td class="crm-acl-deny" id="row_{$aclID}_deny">{if $row.deny}{ts}Deny{/ts}{else}{ts}Allow{/ts}{/if}</td>
+                <td class="crm-acl-priority" id="row_{$aclID}_priority">{$row.priority}</td>
                 <td>{$row.action|replace:'xx':$aclID}</td>
               </tr>
             {/foreach}
diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index 5be3166570..de46201180 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -1316,6 +1316,78 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
     Civi::$statics['CRM_ACL_BAO_ACL'] = [];
   }
 
+  /**
+   * @throws \CRM_Core_Exception
+   */
+  public function testPriorityCustomGroupACL(): void {
+    // Create 2 multi-record custom entities and 2 regular custom fields
+    $customGroups = [];
+    foreach ([1, 2] as $i) {
+      $customGroups[$i] = CustomGroup::create(FALSE)
+        ->addValue('title', "priority_extra_group_$i")
+        ->addValue('extends', 'Contact')
+        ->addValue('is_multiple', FALSE)
+        ->addChain('field', CustomField::create()
+          ->addValue('label', "priority_extra_field_$i")
+          ->addValue('custom_group_id', '$id')
+          ->addValue('html_type', 'Text')
+          ->addValue('data_type', 'String')
+        )
+        ->execute()->single()['id'];
+      $this->callAPISuccess('Acl', 'create', [
+        'name' => 'Deny everyone to access custom group ' . $customGroups[$i],
+        'entity_table' => 'civicrm_acl_role',
+        'entity_id' => 0,
+        'operation' => 'Edit',
+        'object_table' => 'civicrm_custom_group',
+        'object_id' => $customGroups[$i],
+        'deny' => 1,
+      ]);
+    }
+
+    $this->callAPISuccess('OptionValue', 'create', [
+      'option_group_id' => 'acl_role',
+      'label' => 'Test Priority ACL Role',
+      'value' => 5,
+      'is_active' => 1,
+    ]);
+    $aclGroup = $this->groupCreate();
+    ACLEntityRole::create(FALSE)->setValues([
+      'acl_role_id' => 5,
+      'entity_table' => 'civicrm_group',
+      'entity_id' => $aclGroup,
+      'is_active' => 1,
+    ])->execute();
+    $this->callAPISuccess('Acl', 'create', [
+      'name' => 'Test Postive Priority ACL',
+      'priority' => 1,
+      'entity_table' => 'civicrm_acl_role',
+      'entity_id' => 5,
+      'operation' => 'Edit',
+      'object_table' => 'civicrm_custom_group',
+      'object_id' => $customGroups[2],
+    ]);
+    $userID = $this->createLoggedInUser();
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = [
+      'access CiviCRM',
+      'view my contact',
+    ];
+    $this->callAPISuccess('GroupContact', 'create', [
+      'contact_id' => $userID,
+      'group_id' => $aclGroup,
+      'status' => 'Added',
+    ]);
+    Civi::cache('metadata')->clear();
+    Civi::$statics['CRM_ACL_BAO_ACL'] = [];
+    $getFields = Contact::getFields()
+      ->addWhere('name', 'LIKE', 'priority_extra_group_%.priority_extra_field_%')
+      ->execute();
+    $this->assertCount(1, $getFields);
+
+    Civi::cache('metadata')->clear();
+    Civi::$statics['CRM_ACL_BAO_ACL'] = [];
+  }
+
   public function aclGroupHookAllResults($action, $contactID, $tableName, &$allGroups, &$currentGroups) {
     if ($tableName === $this->aclGroupHookType) {
       $currentGroups = array_keys($allGroups);
diff --git a/xml/schema/ACL/ACL.xml b/xml/schema/ACL/ACL.xml
index 326b69372b..d7a71d63be 100644
--- a/xml/schema/ACL/ACL.xml
+++ b/xml/schema/ACL/ACL.xml
@@ -126,4 +126,18 @@
       <label>Enabled</label>
     </html>
   </field>
+  <field>
+    <name>priority</name>
+    <type>int</type>
+    <default>0</default>
+    <required>true</required>
+    <add>5.64</add>
+    <html>
+      <type>Number</type>
+    </html>
+  </field>
+  <index>
+    <name>index_priority</name>
+    <fieldName>priority</fieldName>
+  </index>
 </table>
-- 
2.25.1