From: Tim Otten <totten@civicrm.org>
Date: Mon, 7 Jun 2021 06:05:40 +0000 (-0700)
Subject: (REF) Consistently pass `string $entity` to all flavors of checkAccess
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=a5d0f31af7fea240ee802a6eb7b692428dc772e4;p=civicrm-core.git

(REF) Consistently pass `string $entity` to all flavors of checkAccess

1. This removes the special-case where `CustomValue::checkAccess()` needs an extra parameter
   to identify the target entity.
2. This lines things up to do the swap from `_checkAccess()` to a hook/event listener
---

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
index 372a8d50dc..d5206e04b0 100644
--- a/CRM/Contact/AccessTrait.php
+++ b/CRM/Contact/AccessTrait.php
@@ -21,13 +21,14 @@
 trait CRM_Contact_AccessTrait {
 
   /**
+   * @param string $entityName
    * @param string $action
    * @param array $record
    * @param int|NULL $userID
    * @return bool
    * @see CRM_Core_DAO::checkAccess
    */
-  public static function _checkAccess(string $action, array $record, $userID) {
+  public static function _checkAccess(string $entityName, string $action, array $record, $userID) {
     $cid = $record['contact_id'] ?? NULL;
     if (!$cid && !empty($record['id'])) {
       $cid = CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'contact_id');
diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index 5275f5a9fd..c76c288836 100644
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -3729,13 +3729,14 @@ LEFT JOIN civicrm_address ON ( civicrm_address.contact_id = civicrm_contact.id )
   }
 
   /**
+   * @param string $entityName
    * @param string $action
    * @param array $record
    * @param $userID
    * @return bool
    * @see CRM_Core_DAO::checkAccess
    */
-  public static function _checkAccess(string $action, array $record, $userID): bool {
+  public static function _checkAccess(string $entityName, string $action, array $record, $userID): bool {
     switch ($action) {
       case 'create':
         return CRM_Core_Permission::check('add contacts', $userID);
diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index 0ad75b12a4..02f7eb4104 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -224,14 +224,17 @@ class CRM_Core_BAO_CustomValue extends CRM_Core_DAO {
   /**
    * Special checkAccess function for multi-record custom pseudo-entities
    *
+   * @param string $entityName
+   *   Ex: 'Contact' or 'Custom_Foobar'
    * @param string $action
    * @param array $record
-   * @param null $userID
+   * @param int|null $userID
+   *   Contact ID of the active user (whose access we must check). NULL for anonymous.
    * @param bool $granted
-   * @param string $groupName
    * @return bool
    */
-  public static function checkAccess(string $action, array $record, $userID = NULL, $granted = TRUE, $groupName = NULL): bool {
+  public static function checkAccess(string $entityName, string $action, array $record, $userID, $granted = TRUE): bool {
+    $groupName = substr($entityName, 0, 7) === 'Custom_' ? substr($entityName, 7) : NULL;
     if (!$groupName) {
       // $groupName is required but the function signature has to match the parent.
       throw new CRM_Core_Exception('Missing required $groupName in CustomValue::checkAccess');
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index d8dbad2760..25d88a379f 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -3045,21 +3045,22 @@ SELECT contact_id
    *
    * Dispatches to internal BAO function ('static::_checkAccess())` and `hook_civicrm_checkAccess`.
    *
+   * @param string $entityName
+   *   Ex: 'Contact' or 'Custom_Foobar'
    * @param string $action
    *   APIv4 action name.
    *   Ex: 'create', 'get', 'delete'
    * @param array $record
    *   All (known/loaded) values of individual record being accessed.
    *   The record should provide an 'id' but may otherwise be incomplete; guard accordingly.
-   * @param int $userID
-   *   Contact ID of the active user (whose access we must check).
+   * @param int|null $userID
+   *   Contact ID of the active user (whose access we must check). NULL for anonymous.
    * @param bool $granted
    *   Initial value (usually TRUE, but the API might pass FALSE if gatekeeper permissions fail)
    *
    * @return bool
    */
-  public static function checkAccess(string $action, array $record, $userID = NULL, $granted = TRUE): bool {
-    $entityName = CRM_Core_DAO_AllCoreTables::getBriefName(static::class);
+  public static function checkAccess(string $entityName, string $action, array $record, $userID, $granted = TRUE): bool {
     // Ensure this function was either called on a BAO class or a DAO that has no BAO
     if (!$entityName ||
       (!strpos(static::class, '_BAO_') && CRM_Core_DAO_AllCoreTables::getBAOClassName(static::class) !== static::class)
@@ -3069,7 +3070,7 @@ SELECT contact_id
     $userID = isset($userID) ? (int) $userID : CRM_Core_Session::getLoggedInContactID();
     // Dispatch to protected function _checkAccess in this BAO
     if ($granted && method_exists(static::class, '_checkAccess')) {
-      $granted = static::_checkAccess($action, $record, $userID);
+      $granted = static::_checkAccess($entityName, $action, $record, $userID);
     }
     // Dispatch to hook
     CRM_Utils_Hook::checkAccess($entityName, $action, $record, $userID, $granted);
diff --git a/CRM/Core/DynamicFKAccessTrait.php b/CRM/Core/DynamicFKAccessTrait.php
index ce8dec38d4..ebf8382553 100644
--- a/CRM/Core/DynamicFKAccessTrait.php
+++ b/CRM/Core/DynamicFKAccessTrait.php
@@ -21,13 +21,14 @@
 trait CRM_Core_DynamicFKAccessTrait {
 
   /**
+   * @param string $entityName
    * @param string $action
    * @param array $record
    * @param int|NULL $userID
    * @return bool
    * @see CRM_Core_DAO::checkAccess
    */
-  public static function _checkAccess(string $action, array $record, $userID): bool {
+  public static function _checkAccess(string $entityName, string $action, array $record, $userID): bool {
     $eid = $record['entity_id'] ?? NULL;
     $table = $record['entity_table'] ?? NULL;
     if (!$eid && !empty($record['id'])) {
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 2d723f05c5..adc2a1670d 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -179,12 +179,8 @@ class CoreUtil {
     else {
       // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
       $baoName = self::getBAOFromApiName($entityName);
-      // CustomValue also requires the name of the group
-      if ($baoName === 'CRM_Core_BAO_CustomValue') {
-        $granted = \CRM_Core_BAO_CustomValue::checkAccess($actionName, $record, $userID, $granted, substr($entityName, 7));
-      }
-      elseif ($baoName) {
-        $granted = $baoName::checkAccess($actionName, $record, $userID, $granted);
+      if ($baoName) {
+        $granted = $baoName::checkAccess($entityName, $actionName, $record, $userID, $granted);
       }
       // Otherwise, call the hook directly
       else {