(REF) Isolate calls to $bao::checkAccess. Prefer CoreUtil::checkAccessDelegate.
authorTim Otten <totten@civicrm.org>
Mon, 7 Jun 2021 03:13:12 +0000 (20:13 -0700)
committerTim Otten <totten@civicrm.org>
Tue, 8 Jun 2021 04:10:02 +0000 (21:10 -0700)
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
CRM/Core/BAO/CustomValue.php
CRM/Core/DynamicFKAccessTrait.php
Civi/Api4/Utils/CoreUtil.php
api/v3/Contribution.php

index 466198a3a79eae68be77215cd5014d16ff87dfea..372a8d50dcf41ff9767a04bd2103ac71e5abc7d7 100644 (file)
@@ -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);
   }
 
 }
index e2e3034ca0047694a35d249942a4b123404dc6ab..0ad75b12a466df2f240f6e4d79a947191a19076c 100644 (file)
@@ -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;
   }
 
index 1fe7baae4d9cb6d78909c5ddce9dac7f338ed5fe..ce8dec38d44db9e2d3a21b4819feaa2cab0f9195 100644 (file)
@@ -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;
   }
index e0c6061bfa504c6fef096fe7ec21e9afa5d87909..2d723f05c59d83e809d0a3157f050aa54df57319 100644 (file)
@@ -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);
+  }
+
 }
index eeae8e5855b037f06a9893b0eab36c856de352c1..bf5335a6e24bcf037b595e1072505dbd66486fed 100644 (file)
@@ -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)) {