From 6ea81ac6742303f14cf07143379da6d5f7845792 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Sun, 6 Jun 2021 20:13:12 -0700
Subject: [PATCH] (REF) Isolate calls to $bao::checkAccess. Prefer
 CoreUtil::checkAccessDelegate.

Code paths:

* Before: There are many callers to `$bao::checkAccess()`.
* After: There is only one caller to `$bao::checkAccess()` (ie `CoreUtil`).

Delegation mechanics:

* Before: When delegating access-control to another entity, various things invoke  `$bao::checkAccess()`.
* After: When delegating access-control to another entity, various things invoke `CoreUtil::checkAccessDelegated()`
---
 CRM/Contact/AccessTrait.php       |  2 +-
 CRM/Core/BAO/CustomValue.php      | 10 +++++++++-
 CRM/Core/DynamicFKAccessTrait.php |  4 ++--
 Civi/Api4/Utils/CoreUtil.php      | 19 +++++++++++++++++++
 api/v3/Contribution.php           |  2 +-
 5 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
index 466198a3a7..372a8d50dc 100644
--- a/CRM/Contact/AccessTrait.php
+++ b/CRM/Contact/AccessTrait.php
@@ -37,7 +37,7 @@ trait CRM_Contact_AccessTrait {
       return in_array(__CLASS__, ['CRM_Core_BAO_Phone', 'CRM_Core_BAO_Email', 'CRM_Core_BAO_Address']) &&
         CRM_Core_Permission::check('edit all events', $userID);
     }
-    return CRM_Contact_BAO_Contact::checkAccess(CRM_Core_Permission::EDIT, ['id' => $cid], $userID);
+    return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contact', 'update', ['id' => $cid], $userID);
   }
 
 }
diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index e2e3034ca0..0ad75b12a4 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -237,15 +237,23 @@ class CRM_Core_BAO_CustomValue extends CRM_Core_DAO {
       throw new CRM_Core_Exception('Missing required $groupName in CustomValue::checkAccess');
     }
     // Currently, multi-record custom data always extends Contacts
+    $extends = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $groupName, 'extends', 'name');
+    if (!in_array($extends, ['Contact', 'Individual', 'Organization', 'Household'])) {
+      throw new CRM_Core_Exception("Cannot assess delegated permissions for group {$groupName}.");
+    }
+
     $cid = $record['entity_id'] ?? NULL;
     if (!$cid) {
       $tableName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $groupName, 'table_name', 'name');
       $cid = CRM_Core_DAO::singleValueQuery("SELECT entity_id FROM `$tableName` WHERE id = " . (int) $record['id']);
     }
-    $granted = CRM_Contact_BAO_Contact::checkAccess(CRM_Core_Permission::EDIT, ['id' => $cid], $userID, $granted);
 
     // Dispatch to hook
+    $granted = NULL;
     CRM_Utils_Hook::checkAccess("Custom_$groupName", $action, $record, $userID, $granted);
+    if ($granted === NULL) {
+      $granted = \Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contact', 'update', ['id' => $cid], $userID);
+    }
     return $granted;
   }
 
diff --git a/CRM/Core/DynamicFKAccessTrait.php b/CRM/Core/DynamicFKAccessTrait.php
index 1fe7baae4d..ce8dec38d4 100644
--- a/CRM/Core/DynamicFKAccessTrait.php
+++ b/CRM/Core/DynamicFKAccessTrait.php
@@ -37,8 +37,8 @@ trait CRM_Core_DynamicFKAccessTrait {
       $table = CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'entity_table');
     }
     if ($eid && $table) {
-      $bao = CRM_Core_DAO_AllCoreTables::getBAOClassName(CRM_Core_DAO_AllCoreTables::getClassForTable($table));
-      return $bao::checkAccess(CRM_Core_Permission::EDIT, ['id' => $eid], $userID);
+      $targetEntity = CRM_Core_DAO_AllCoreTables::getBriefName(CRM_Core_DAO_AllCoreTables::getClassForTable($table));
+      return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated($targetEntity, 'update', ['id' => $eid], $userID);
     }
     return TRUE;
   }
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index e0c6061bfa..2d723f05c5 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -194,4 +194,23 @@ class CoreUtil {
     return $granted;
   }
 
+  /**
+   * If the permissions of record $A are based on record $B, then use `checkAccessDelegated($B...)`
+   * to make see if access to $B is permitted.
+   *
+   * @param string $entityName
+   * @param string $actionName
+   * @param array $record
+   * @param int|null $userID
+   *   Contact ID of the user we are testing, or NULL for the anonymous user.
+   *
+   * @return bool
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   */
+  public static function checkAccessDelegated(string $entityName, string $actionName, array $record, $userID = NULL) {
+    // FIXME: Move isAuthorized check into here. It's redundant for normal checkAccess().
+    return static::checkAccess($entityName, $actionName, $record, $userID);
+  }
+
 }
diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index eeae8e5855..bf5335a6e2 100644
--- a/api/v3/Contribution.php
+++ b/api/v3/Contribution.php
@@ -204,7 +204,7 @@ function _civicrm_api3_contribution_create_legacy_support_45(&$params) {
 function civicrm_api3_contribution_delete($params) {
 
   $contributionID = !empty($params['contribution_id']) ? $params['contribution_id'] : $params['id'];
-  if (!empty($params['check_permissions']) && !CRM_Contribute_BAO_Contribution::checkAccess('delete', ['id' => $contributionID])) {
+  if (!empty($params['check_permissions']) && !\Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contribution', 'delete', ['id' => $contributionID], CRM_Core_Session::getLoggedInContactID())) {
     throw new API_Exception('You do not have permission to delete this contribution');
   }
   if (CRM_Contribute_BAO_Contribution::deleteContribution($contributionID)) {
-- 
2.25.1