From cc69f300c2ede788b96737cb44eab1f8fce9de85 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 18 May 2021 01:56:37 -0700
Subject: [PATCH 01/27] (REF) Consolidate the 'dispatchSubevent()' methods

---
 Civi/Core/Container.php       | 11 +++++++++--
 Civi/Core/Event/PostEvent.php | 11 -----------
 Civi/Core/Event/PreEvent.php  | 11 -----------
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
index e6d1cff3b322..2785d04da870 100644
--- a/Civi/Core/Container.php
+++ b/Civi/Core/Container.php
@@ -371,13 +371,20 @@ public function createEventDispatcher() {
     /** @var CiviEventDispatcher $dispatcher */
     $dispatcher = static::getBootService('dispatcher.boot');
 
+    // Sometimes, you have a generic event ('hook_pre') and wish to fire more targeted aliases ('hook_pre::MyEntity') to allow shorter subscriber lists.
+    $aliasEvent = function($eventName, $fieldName) {
+      return function($e) use ($eventName, $fieldName) {
+        \Civi::dispatcher()->dispatch($eventName . "::" . $e->{$fieldName}, $e);
+      };
+    };
+
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\InstallationCanary', 'check']);
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\DatabaseInitializer', 'initialize']);
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\LocalizationInitializer', 'initialize']);
     $dispatcher->addListener('hook_civicrm_post', ['\CRM_Core_Transaction', 'addPostCommit'], -1000);
-    $dispatcher->addListener('hook_civicrm_pre', ['\Civi\Core\Event\PreEvent', 'dispatchSubevent'], 100);
+    $dispatcher->addListener('hook_civicrm_pre', $aliasEvent('hook_civicrm_pre', 'entity'), 100);
     $dispatcher->addListener('civi.dao.preDelete', ['\CRM_Core_BAO_EntityTag', 'preDeleteOtherEntity']);
-    $dispatcher->addListener('hook_civicrm_post', ['\Civi\Core\Event\PostEvent', 'dispatchSubevent'], 100);
+    $dispatcher->addListener('hook_civicrm_post', $aliasEvent('hook_civicrm_post', 'entity'), 100);
     $dispatcher->addListener('hook_civicrm_post::Activity', ['\Civi\CCase\Events', 'fireCaseChange']);
     $dispatcher->addListener('hook_civicrm_post::Case', ['\Civi\CCase\Events', 'fireCaseChange']);
     $dispatcher->addListener('hook_civicrm_caseChange', ['\Civi\CCase\Events', 'delegateToXmlListeners']);
diff --git a/Civi/Core/Event/PostEvent.php b/Civi/Core/Event/PostEvent.php
index 100be7b7547a..f3885af444ca 100644
--- a/Civi/Core/Event/PostEvent.php
+++ b/Civi/Core/Event/PostEvent.php
@@ -17,17 +17,6 @@
  */
 class PostEvent extends GenericHookEvent {
 
-  /**
-   * This adapter automatically emits a narrower event.
-   *
-   * For example, `hook_civicrm_pre(Contact, ...)` will also dispatch `hook_civicrm_pre::Contact`.
-   *
-   * @param \Civi\Core\Event\PostEvent $event
-   */
-  public static function dispatchSubevent(PostEvent $event) {
-    \Civi::dispatcher()->dispatch("hook_civicrm_post::" . $event->entity, $event);
-  }
-
   /**
    * One of: 'create'|'edit'|'delete'
    *
diff --git a/Civi/Core/Event/PreEvent.php b/Civi/Core/Event/PreEvent.php
index dc08f1d0c98c..db25c344e095 100644
--- a/Civi/Core/Event/PreEvent.php
+++ b/Civi/Core/Event/PreEvent.php
@@ -17,17 +17,6 @@
  */
 class PreEvent extends GenericHookEvent {
 
-  /**
-   * This adapter automatically emits a narrower event.
-   *
-   * For example, `hook_civicrm_pre(Contact, ...)` will also dispatch `hook_civicrm_pre::Contact`.
-   *
-   * @param \Civi\Core\Event\PreEvent $event
-   */
-  public static function dispatchSubevent(PreEvent $event) {
-    \Civi::dispatcher()->dispatch("hook_civicrm_pre::" . $event->entity, $event);
-  }
-
   /**
    * One of: 'create'|'edit'|'delete'
    *

From 8047edec7bea76c2ed4e8e32684f8069fa60a7b6 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 19 May 2021 16:45:51 -0700
Subject: [PATCH 02/27] (REF) APIv4 BatchAction - Split 'getBatchRecords()' in
 two

The original form (`getBatchRecords()`) still returns an array of items.

The alternate form (`getBatchAction()`) returns an API call for fetching the
batchs - but this API call may be further refined (e.g.  selecting different
fields or data-pages).
---
 Civi/Api4/Generic/AbstractBatchAction.php | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Civi/Api4/Generic/AbstractBatchAction.php b/Civi/Api4/Generic/AbstractBatchAction.php
index 7a0e6e8c19e2..f0701590f14b 100644
--- a/Civi/Api4/Generic/AbstractBatchAction.php
+++ b/Civi/Api4/Generic/AbstractBatchAction.php
@@ -54,9 +54,23 @@ public function __construct($entityName, $actionName, $select = 'id') {
   }
 
   /**
+   * Get a list of records for this batch.
+   *
    * @return array
    */
   protected function getBatchRecords() {
+    return (array) $this->getBatchAction()->execute();
+  }
+
+  /**
+   * Get a query which resolves the list of records for this batch.
+   *
+   * This is similar to `getBatchRecords()`, but you may further refine the
+   * API call (e.g. selecting different fields or data-pages) before executing.
+   *
+   * @return \Civi\Api4\Generic\AbstractGetAction
+   */
+  protected function getBatchAction() {
     $params = [
       'checkPermissions' => $this->checkPermissions,
       'where' => $this->where,
@@ -67,8 +81,7 @@ protected function getBatchRecords() {
     if (empty($this->reload)) {
       $params['select'] = $this->select;
     }
-
-    return (array) civicrm_api4($this->getEntityName(), 'get', $params);
+    return \Civi\API\Request::create($this->getEntityName(), 'get', ['version' => 4] + $params);
   }
 
   /**

From a9aac3bf03a73a553b96031be604de933cb14dfd Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 19 May 2021 15:21:23 -0700
Subject: [PATCH 03/27] (REF) APIv4 UpdateAction - Add skeletal
 'validateValues()` method

---
 Civi/Api4/Generic/AbstractUpdateAction.php | 7 +++++++
 Civi/Api4/Generic/BasicUpdateAction.php    | 1 +
 Civi/Api4/Generic/DAOUpdateAction.php      | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/Civi/Api4/Generic/AbstractUpdateAction.php b/Civi/Api4/Generic/AbstractUpdateAction.php
index 91e8c3feac70..0260fb2a89a5 100644
--- a/Civi/Api4/Generic/AbstractUpdateAction.php
+++ b/Civi/Api4/Generic/AbstractUpdateAction.php
@@ -70,4 +70,11 @@ public function addValue(string $fieldName, $value) {
     return $this;
   }
 
+  /**
+   * @throws \API_Exception
+   */
+  protected function validateValues() {
+    // Placeholder
+  }
+
 }
diff --git a/Civi/Api4/Generic/BasicUpdateAction.php b/Civi/Api4/Generic/BasicUpdateAction.php
index a7bc7e9bb22b..4fd8fe6c156b 100644
--- a/Civi/Api4/Generic/BasicUpdateAction.php
+++ b/Civi/Api4/Generic/BasicUpdateAction.php
@@ -58,6 +58,7 @@ public function __construct($entityName, $actionName, $select = 'id', $setter =
    */
   public function _run(Result $result) {
     $this->formatWriteValues($this->values);
+    $this->validateValues();
     foreach ($this->getBatchRecords() as $item) {
       $result[] = $this->writeRecord($this->values + $item);
     }
diff --git a/Civi/Api4/Generic/DAOUpdateAction.php b/Civi/Api4/Generic/DAOUpdateAction.php
index a3c86dbfe622..aef6618729f5 100644
--- a/Civi/Api4/Generic/DAOUpdateAction.php
+++ b/Civi/Api4/Generic/DAOUpdateAction.php
@@ -61,6 +61,7 @@ public function _run(Result $result) {
     if ($this->getSelect() === ['id'] && count($this->where) === 1 && $this->where[0][0] === 'id' && $this->where[0][1] === '=' && !empty($this->where[0][2])) {
       $this->values['id'] = $this->where[0][2];
       $items = [$this->values];
+      $this->validateValues();
       $result->exchangeArray($this->writeObjects($items));
       return;
     }
@@ -71,6 +72,7 @@ public function _run(Result $result) {
       $item = $this->values + $item;
     }
 
+    $this->validateValues();
     $result->exchangeArray($this->writeObjects($items));
   }
 

From 48872a5738c50268b9761dadf248c5885a63e4c8 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 19 May 2021 19:34:16 -0700
Subject: [PATCH 04/27] LazyArray - Add helper class for lazily-loaded lists

---
 CRM/Utils/Array.php                       |  25 +++++
 CRM/Utils/LazyArray.php                   |  95 ++++++++++++++++++
 tests/phpunit/CRM/Utils/LazyArrayTest.php | 111 ++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 CRM/Utils/LazyArray.php
 create mode 100644 tests/phpunit/CRM/Utils/LazyArrayTest.php

diff --git a/CRM/Utils/Array.php b/CRM/Utils/Array.php
index f2260f4502d4..06aa5a8faf63 100644
--- a/CRM/Utils/Array.php
+++ b/CRM/Utils/Array.php
@@ -17,6 +17,31 @@
  */
 class CRM_Utils_Array {
 
+  /**
+   * Cast a value to an array.
+   *
+   * This is similar to PHP's `(array)`, but it also converts iterators.
+   *
+   * @param mixed $value
+   * @return array
+   */
+  public static function cast($value) {
+    if (is_array($value)) {
+      return $value;
+    }
+    if ($value instanceof CRM_Utils_LazyArray || $value instanceof ArrayObject) {
+      // iterator_to_array() would work here, but getArrayCopy() doesn't require actual iterations.
+      return $value->getArrayCopy();
+    }
+    if (is_iterable($value)) {
+      return iterator_to_array($value);
+    }
+    if (is_scalar($value)) {
+      return [$value];
+    }
+    throw new \RuntimeException(sprintf("Cannot cast %s to array", gettype($value)));
+  }
+
   /**
    * Returns $list[$key] if such element exists, or a default value otherwise.
    *
diff --git a/CRM/Utils/LazyArray.php b/CRM/Utils/LazyArray.php
new file mode 100644
index 000000000000..4ea6ab165995
--- /dev/null
+++ b/CRM/Utils/LazyArray.php
@@ -0,0 +1,95 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * A lazy-array works much like a regular array or ArrayObject. However, it is
+ * initially empty - and it is only populated if used.
+ */
+class CRM_Utils_LazyArray implements ArrayAccess, IteratorAggregate, Countable {
+
+  /**
+   * A function which generates a list of values.
+   *
+   * @var callable
+   *   function(): iterable
+   */
+  private $func;
+
+  /**
+   * Cached values
+   *
+   * @var array|null
+   */
+  private $cache;
+
+  /**
+   * CRM_Utils_LazyList constructor.
+   *
+   * @param callable $func
+   *   Function which provides a list of values (array/iterator/generator).
+   */
+  public function __construct($func) {
+    $this->func = $func;
+  }
+
+  /**
+   * Determine if the content has been fetched.
+   *
+   * @return bool
+   */
+  public function isLoaded() {
+    return $this->cache !== NULL;
+  }
+
+  public function load($force = FALSE) {
+    if ($this->cache === NULL || $force) {
+      $this->cache = CRM_Utils_Array::cast(call_user_func($this->func));
+    }
+    return $this;
+  }
+
+  public function offsetExists($offset) {
+    return isset($this->load()->cache[$offset]);
+  }
+
+  public function &offsetGet($offset) {
+    return $this->load()->cache[$offset];
+  }
+
+  public function offsetSet($offset, $value) {
+    if ($offset === NULL) {
+      $this->load()->cache[] = $value;
+    }
+    else {
+      $this->load()->cache[$offset] = $value;
+    }
+  }
+
+  public function offsetUnset($offset) {
+    unset($this->load()->cache[$offset]);
+  }
+
+  public function getIterator() {
+    return new ArrayIterator($this->load()->cache);
+  }
+
+  /**
+   * @return array
+   */
+  public function getArrayCopy() {
+    return $this->load()->cache;
+  }
+
+  public function count() {
+    return count($this->load()->cache);
+  }
+
+}
diff --git a/tests/phpunit/CRM/Utils/LazyArrayTest.php b/tests/phpunit/CRM/Utils/LazyArrayTest.php
new file mode 100644
index 000000000000..c48b1455db9f
--- /dev/null
+++ b/tests/phpunit/CRM/Utils/LazyArrayTest.php
@@ -0,0 +1,111 @@
+<?php
+
+/**
+ * Class CRM_Utils_LazyArrayTest
+ * @group headless
+ */
+class CRM_Utils_LazyArrayTest extends CiviUnitTestCase {
+
+  public function testAssoc() {
+    $l = $this->createFruitBasket();
+    $this->assertFalse($l->isLoaded());
+
+    $this->assertEquals('apple', $l['a']);
+    $this->assertEquals('banana', $l['b']);
+    $this->assertTrue($l->isLoaded());
+    $this->assertEquals(3, count($l));
+    $this->assertTrue(isset($l['c']));
+    $this->assertFalse(isset($l['d']));
+
+    $l['a'] = 'apricot';
+    $this->assertEquals('apricot', $l['a']);
+    $this->assertEquals(3, count($l));
+
+    $l['d'] = 'date';
+    $this->assertEquals('date', $l['d']);
+    $this->assertEquals(4, count($l));
+
+    $keys = [];
+    foreach ($l as $key => $value) {
+      $keys[] = $key;
+    }
+    $this->assertEquals(['a', 'b', 'c', 'd'], $keys);
+    $this->assertEquals(['a', 'b', 'c', 'd'], array_keys(CRM_Utils_Array::cast($l)));
+  }
+
+  public function testNumeric() {
+    $l = $this->createSeaRecords();
+    $this->assertFalse($l->isLoaded());
+
+    $this->assertEquals('aegean', $l[0]['name']);
+    $this->assertEquals('caspian', $l[2]['name']);
+    $this->assertTrue($l->isLoaded());
+    $this->assertEquals(3, count($l));
+    $this->assertTrue(isset($l[2]));
+    $this->assertFalse(isset($l[3]));
+
+    $l[2]['name'] = 'coral';
+    $this->assertEquals(['name' => 'coral', 'area' => 371], $l['2']);
+    $this->assertEquals(3, count($l));
+
+    $l[] = ['name' => 'weddell', 'area' => 2800];
+    $this->assertEquals('weddell', $l[3]['name']);
+    $this->assertEquals(4, count($l));
+
+    $keys = [];
+    foreach ($l as $key => $value) {
+      $keys[] = $key;
+    }
+    $this->assertEquals([0, 1, 2, 3], $keys);
+    $this->assertEquals([0, 1, 2, 3], array_keys(CRM_Utils_Array::cast($l)));
+  }
+
+  public function testBasicInspections() {
+    $l = $this->createFruitBasket();
+    $this->assertFalse($l->isLoaded());
+
+    $this->assertTrue($l !== NULL);
+    $this->assertTrue($l instanceof CRM_Utils_LazyArray);
+    $this->assertTrue(is_iterable($l));
+    $this->assertTrue(!is_array($l));
+
+    $this->assertFalse($l->isLoaded());
+
+    $this->assertEquals(3, count($l));
+    $this->assertTrue($l->isLoaded());
+  }
+
+  public function testCopy() {
+    $l = $this->createFruitBasket();
+    $copy = $l->getArrayCopy();
+    $copy['d'] = 'date';
+
+    $this->assertEquals(3, count($l));
+    $this->assertEquals(4, count($copy));
+  }
+
+  /**
+   * @return \CRM_Utils_LazyArray
+   */
+  private function createFruitBasket(): \CRM_Utils_LazyArray {
+    return new CRM_Utils_LazyArray(function () {
+      yield 'a' => 'apple';
+      yield 'b' => 'banana';
+      yield 'c' => 'cherry';
+    });
+  }
+
+  /**
+   * @return \CRM_Utils_LazyArray
+   */
+  private function createSeaRecords(): \CRM_Utils_LazyArray {
+    return new CRM_Utils_LazyArray(function () {
+      return [
+        ['name' => 'aegean', 'area' => 214],
+        ['name' => 'baltic', 'area' => 377],
+        ['name' => 'caspian', 'area' => 371],
+      ];
+    });
+  }
+
+}

From 7645fa5db4b8db5368cdee71a325b59963002ae4 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 3 Jun 2021 21:38:04 -0700
Subject: [PATCH 05/27] (REF) Civi/API/Event - Extract RequestTrait

---
 Civi/API/Event/Event.php        | 32 ++--------------
 Civi/API/Event/PrepareEvent.php |  6 +--
 Civi/API/Event/RequestTrait.php | 67 +++++++++++++++++++++++++++++++++
 Civi/API/Event/ResolveEvent.php |  3 +-
 4 files changed, 75 insertions(+), 33 deletions(-)
 create mode 100644 Civi/API/Event/RequestTrait.php

diff --git a/Civi/API/Event/Event.php b/Civi/API/Event/Event.php
index 9a68b743390a..c5ad2cc92b65 100644
--- a/Civi/API/Event/Event.php
+++ b/Civi/API/Event/Event.php
@@ -17,6 +17,8 @@
  */
 class Event extends \Symfony\Component\EventDispatcher\Event {
 
+  use RequestTrait;
+
   /**
    * @var \Civi\API\Kernel
    */
@@ -28,14 +30,6 @@ class Event extends \Symfony\Component\EventDispatcher\Event {
    */
   protected $apiProvider;
 
-  /**
-   * @var array
-   *   The full description of the API request.
-   *
-   * @see \Civi\API\Request::create
-   */
-  protected $apiRequest;
-
   /**
    * @param \Civi\API\Provider\ProviderInterface $apiProvider
    *   The API responsible for executing the request.
@@ -46,7 +40,7 @@ class Event extends \Symfony\Component\EventDispatcher\Event {
   public function __construct($apiProvider, $apiRequest, $apiKernel) {
     $this->apiKernel = $apiKernel;
     $this->apiProvider = $apiProvider;
-    $this->apiRequest = $apiRequest;
+    $this->setApiRequest($apiRequest);
   }
 
   /**
@@ -65,24 +59,4 @@ public function getApiProvider() {
     return $this->apiProvider;
   }
 
-  /**
-   * @return array
-   */
-  public function getApiRequest() {
-    return $this->apiRequest;
-  }
-
-  /**
-   * Create a brief string identifying the entity/action. Useful for
-   * pithy matching/switching.
-   *
-   * Ex: if ($e->getApiRequestSig() === '3.contact.get') { ... }
-   *
-   * @return string
-   *   Ex: '3.contact.get'
-   */
-  public function getApiRequestSig() {
-    return mb_strtolower($this->apiRequest['version'] . '.' . $this->apiRequest['entity'] . '.' . $this->apiRequest['action']);
-  }
-
 }
diff --git a/Civi/API/Event/PrepareEvent.php b/Civi/API/Event/PrepareEvent.php
index 1c6576173c89..452ee4c9c267 100644
--- a/Civi/API/Event/PrepareEvent.php
+++ b/Civi/API/Event/PrepareEvent.php
@@ -26,11 +26,11 @@ class PrepareEvent extends Event {
   /**
    * @param array $apiRequest
    *   The full description of the API request.
-   * @return PrepareEvent
+   * @return static
    */
   public function setApiRequest($apiRequest) {
-    $this->apiRequest = $apiRequest;
-    return $this;
+    // Elevate from 'protected' to 'public'.
+    return parent::setApiRequest($apiRequest);
   }
 
   /**
diff --git a/Civi/API/Event/RequestTrait.php b/Civi/API/Event/RequestTrait.php
new file mode 100644
index 000000000000..b216c5e1d754
--- /dev/null
+++ b/Civi/API/Event/RequestTrait.php
@@ -0,0 +1,67 @@
+<?php
+
+namespace Civi\API\Event;
+
+/**
+ * Trait RequestTrait
+ * @package Civi\API\Event
+ *
+ * Most events emitted by the API subsystem should include information about the active API request.
+ */
+trait RequestTrait {
+
+  /**
+   * @var \Civi\Api4\Generic\AbstractAction|array
+   *   The full description of the API request.
+   *
+   * @see \Civi\API\Request::create
+   */
+  protected $apiRequest;
+
+  /**
+   * @return \Civi\Api4\Generic\AbstractAction|array
+   */
+  public function getApiRequest() {
+    return $this->apiRequest;
+  }
+
+  /**
+   * @param \Civi\Api4\Generic\AbstractAction|array $apiRequest
+   *   The full description of the API request.
+   * @return static
+   */
+  protected function setApiRequest($apiRequest) {
+    $this->apiRequest = $apiRequest;
+    return $this;
+  }
+
+  /**
+   * Create a brief string identifying the entity/action. Useful for
+   * pithy matching/switching.
+   *
+   * Ex: if ($e->getApiRequestSig() === '3.contact.get') { ... }
+   *
+   * @return string
+   *   Ex: '3.contact.get'
+   */
+  public function getApiRequestSig(): string {
+    return mb_strtolower($this->apiRequest['version'] . '.' . $this->apiRequest['entity'] . '.' . $this->apiRequest['action']);
+  }
+
+  /**
+   * @return string
+   *   Ex: 'Contact', 'Activity'
+   */
+  public function getEntityName(): string {
+    return $this->apiRequest['entity'];
+  }
+
+  /**
+   * @return string
+   *   Ex: 'create', 'update'
+   */
+  public function getActionName(): string {
+    return $this->apiRequest['action'];
+  }
+
+}
diff --git a/Civi/API/Event/ResolveEvent.php b/Civi/API/Event/ResolveEvent.php
index 8d574a032782..cd2cba839eb4 100644
--- a/Civi/API/Event/ResolveEvent.php
+++ b/Civi/API/Event/ResolveEvent.php
@@ -46,7 +46,8 @@ public function setApiProvider($apiProvider) {
    *   The full description of the API request.
    */
   public function setApiRequest($apiRequest) {
-    $this->apiRequest = $apiRequest;
+    // Elevate from 'protected' to 'public'.
+    return parent::setApiRequest($apiRequest);
   }
 
 }

From 4bf92107ce114979357dbda9edeede077638dfb8 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 27 Apr 2021 14:38:38 -0700
Subject: [PATCH 06/27] APIv4 - When running validateValues(), fire an event

---
 Civi/Api4/Event/ValidateValuesEvent.php       | 141 ++++++++++++++++++
 Civi/Api4/Generic/AbstractCreateAction.php    |   8 +
 Civi/Api4/Generic/AbstractSaveAction.php      |   8 +
 Civi/Api4/Generic/AbstractUpdateAction.php    |   9 +-
 Civi/Core/Container.php                       |   7 +
 .../phpunit/api/v4/Entity/ConformanceTest.php |  12 ++
 6 files changed, 184 insertions(+), 1 deletion(-)
 create mode 100644 Civi/Api4/Event/ValidateValuesEvent.php

diff --git a/Civi/Api4/Event/ValidateValuesEvent.php b/Civi/Api4/Event/ValidateValuesEvent.php
new file mode 100644
index 000000000000..146f29f2cb1d
--- /dev/null
+++ b/Civi/Api4/Event/ValidateValuesEvent.php
@@ -0,0 +1,141 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+namespace Civi\Api4\Event;
+
+use Civi\API\Event\RequestTrait;
+use Civi\Core\Event\GenericHookEvent;
+
+/**
+ * The ValidateValuesEvent ('civi.api4.validate') is emitted when creating or saving an entire record via APIv4.
+ * It is emitted once for every record is updated.
+ *
+ * Example #1: Walk each record and validate some fields
+ *
+ * function(ValidateValuesEvent $e) {
+ *   if ($e->entity !== 'Foozball') return;
+ *   foreach ($e->records as $r => $record) {
+ *     if (strtotime($record['start_time']) < CRM_Utils_Time::time()) {
+ *       $e->addError($r, 'start_time', 'past', ts('Start time has already passed.'));
+ *     }
+ *     if ($record['length'] * $record['width'] * $record['height'] > VOLUME_LIMIT) {
+ *       $e->addError($r, ['length', 'width', 'height'], 'excessive_volume', ts('The record is too big.'));
+ *     }
+ *   }
+ * }
+ *
+ * Example #2: Prohibit recording `Contribution` records on `Student` contacts.
+ *
+ * function(ValidateValuesEvent $e) {
+ *   if ($e->entity !== 'Contribution') return;
+ *   $contactSubTypes = CRM_Utils_SQL_Select::from('civicrm_contact')
+ *     ->where('id IN (#ids)', ['ids' => array_column($e->records, 'contact_id')])
+ *     ->select('id, contact_sub_type')
+ *     ->execute()->fetchMap('id', 'contact_sub_type');
+ *   foreach ($e->records as $r => $record) {
+ *     if ($contactSubTypes[$record['contact_id']] === 'Student') {
+ *       $e->addError($r, 'contact_id', 'student_prohibited', ts('Donations from student records are strictly prohibited.'));
+ *     }
+ *   }
+ * }
+ */
+class ValidateValuesEvent extends GenericHookEvent {
+
+  use RequestTrait;
+
+  /**
+   * List of updated records.
+   *
+   * The list of `$records` reflects only the list of new values assigned
+   * by this action. It may or may not correspond to an existing row in the database.
+   * It is similar to the `$records` list used by `save()`.
+   *
+   * @var array|\CRM_Utils_LazyArray
+   * @see \Civi\Api4\Generic\AbstractSaveAction::$records
+   */
+  public $records;
+
+  /**
+   * List of error messages.
+   *
+   * @var array
+   *   Array(string $errorName => string $errorMessage)
+   *   Note:
+   */
+  public $errors = [];
+
+  /**
+   * ValidateValuesEvent constructor.
+   *
+   * @param \Civi\Api4\Generic\AbstractAction $apiRequest
+   * @param array|\CRM_Utils_LazyArray $records
+   *   List of updates (akin to SaveAction::$records).
+   */
+  public function __construct($apiRequest, $records) {
+    $this->setApiRequest($apiRequest);
+    $this->records = $records;
+    $this->errors = [];
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public function getHookValues() {
+    return [$this->getApiRequest(), $this->records, &$this->errors];
+  }
+
+  /**
+   * Add an error.
+   *
+   * @param string|int $recordKey
+   *   The validator may work with multiple records. This should identify the specific record.
+   *   Each record is identified by its offset (`$records[$recordKey] === [...the record...]`).
+   * @param string|array $field
+   *   The name of the field which has an error.
+   *   If the error is multi-field (e.g. mismatched password-confirmation), then use an array.
+   *   If the error is independent of any field, then use [].
+   * @param string $name
+   * @param string|NULL $message
+   * @return $this
+   */
+  public function addError($recordKey, $field, string $name, string $message = NULL): self {
+    $this->errors[] = [
+      'record' => $recordKey,
+      'fields' => (array) $field,
+      'name' => $name,
+      'message' => $message ?: ts('Error code (%1)', [1 => $name]),
+    ];
+    return $this;
+  }
+
+  /**
+   * Convert the list of errors an exception.
+   *
+   * @return \API_Exception
+   */
+  public function toException() {
+    // We should probably have a better way to report the errors in a structured/list format.
+    return new \API_Exception(ts('Found %1 error(s) in submitted %2 record(s) of type "%3": %4', [
+      1 => count($this->errors),
+      2 => count(array_unique(array_column($this->errors, 'record'))),
+      3 => $this->getEntityName(),
+      4 => implode(', ', array_column($this->errors, 'message')),
+    ]));
+  }
+
+}
diff --git a/Civi/Api4/Generic/AbstractCreateAction.php b/Civi/Api4/Generic/AbstractCreateAction.php
index c77236e9797a..5299741a8f04 100644
--- a/Civi/Api4/Generic/AbstractCreateAction.php
+++ b/Civi/Api4/Generic/AbstractCreateAction.php
@@ -19,6 +19,8 @@
 
 namespace Civi\Api4\Generic;
 
+use Civi\Api4\Event\ValidateValuesEvent;
+
 /**
  * Base class for all `Create` api actions.
  *
@@ -59,10 +61,16 @@ public function addValue(string $fieldName, $value) {
    * @throws \API_Exception
    */
   protected function validateValues() {
+    // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
     $unmatched = $this->checkRequiredFields($this->getValues());
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
+    $e = new ValidateValuesEvent($this, [$this->getValues()]);
+    \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
+    if (!empty($e->errors)) {
+      throw $e->toException();
+    }
   }
 
 }
diff --git a/Civi/Api4/Generic/AbstractSaveAction.php b/Civi/Api4/Generic/AbstractSaveAction.php
index b12dc332049f..d67fa3c2c5e0 100644
--- a/Civi/Api4/Generic/AbstractSaveAction.php
+++ b/Civi/Api4/Generic/AbstractSaveAction.php
@@ -19,6 +19,8 @@
 
 namespace Civi\Api4\Generic;
 
+use Civi\Api4\Event\ValidateValuesEvent;
+
 /**
  * Create or update one or more $ENTITIES.
  *
@@ -93,6 +95,7 @@ public function __construct($entityName, $actionName, $idField = 'id') {
    * @throws \API_Exception
    */
   protected function validateValues() {
+    // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
     $unmatched = [];
     foreach ($this->records as $record) {
       if (empty($record[$this->idField])) {
@@ -102,6 +105,11 @@ protected function validateValues() {
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
+    $e = new ValidateValuesEvent($this, $this->records);
+    \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
+    if (!empty($e->errors)) {
+      throw $e->toException();
+    }
   }
 
   /**
diff --git a/Civi/Api4/Generic/AbstractUpdateAction.php b/Civi/Api4/Generic/AbstractUpdateAction.php
index 0260fb2a89a5..f4fb2ec8f8db 100644
--- a/Civi/Api4/Generic/AbstractUpdateAction.php
+++ b/Civi/Api4/Generic/AbstractUpdateAction.php
@@ -19,6 +19,8 @@
 
 namespace Civi\Api4\Generic;
 
+use Civi\Api4\Event\ValidateValuesEvent;
+
 /**
  * Base class for all `Update` api actions
  *
@@ -74,7 +76,12 @@ public function addValue(string $fieldName, $value) {
    * @throws \API_Exception
    */
   protected function validateValues() {
-    // Placeholder
+    // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
+    $e = new ValidateValuesEvent($this, [$this->values]);
+    \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
+    if (!empty($e->errors)) {
+      throw $e->toException();
+    }
   }
 
 }
diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
index 2785d04da870..8e2e926d1bd8 100644
--- a/Civi/Core/Container.php
+++ b/Civi/Core/Container.php
@@ -377,6 +377,13 @@ public function createEventDispatcher() {
         \Civi::dispatcher()->dispatch($eventName . "::" . $e->{$fieldName}, $e);
       };
     };
+    $aliasMethodEvent = function($eventName, $methodName) {
+      return function($e) use ($eventName, $methodName) {
+        \Civi::dispatcher()->dispatch($eventName . "::" . $e->{$methodName}(), $e);
+      };
+    };
+
+    $dispatcher->addListener('civi.api4.validate', $aliasMethodEvent('civi.api4.validate', 'getEntityName'), 100);
 
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\InstallationCanary', 'check']);
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\DatabaseInitializer', 'initialize']);
diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php
index 5ad31a132f9b..1ad972ab13db 100644
--- a/tests/phpunit/api/v4/Entity/ConformanceTest.php
+++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php
@@ -21,6 +21,7 @@
 
 use Civi\Api4\Entity;
 use api\v4\UnitTestCase;
+use Civi\Api4\Event\ValidateValuesEvent;
 use Civi\Api4\Utils\CoreUtil;
 
 /**
@@ -205,6 +206,13 @@ protected function checkActions($entityClass): array {
    * @return mixed
    */
   protected function checkCreation($entity, $entityClass) {
+    $hookLog = [];
+    $onValidate = function(ValidateValuesEvent $e) use (&$hookLog) {
+      $hookLog[$e->entity][$e->action] = 1 + ($hookLog[$e->entity][$e->action] ?? 0);
+    };
+    \Civi::dispatcher()->addListener('civi.api4.validate', $onValidate);
+    \Civi::dispatcher()->addListener('civi.api4.validate::' . $entity, $onValidate);
+
     $requiredParams = $this->creationParamProvider->getRequired($entity);
     $createResult = $entityClass::create()
       ->setValues($requiredParams)
@@ -217,6 +225,10 @@ protected function checkCreation($entity, $entityClass) {
 
     $this->assertGreaterThanOrEqual(1, $id, "$entity ID not positive");
 
+    $this->assertEquals(2, $hookLog[$entity]['create']);
+    \Civi::dispatcher()->removeListener('civi.api4.validate', $onValidate);
+    \Civi::dispatcher()->removeListener('civi.api4.validate::' . $entity, $onValidate);
+
     return $id;
   }
 

From 9f04ea33de122a5087450444fa8ffcea55aeab12 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 19 May 2021 22:11:29 -0700
Subject: [PATCH 07/27] ValidateValues - Provide optional access to complete
 records

---
 Civi/Api4/Event/ValidateValuesEvent.php    | 23 +++++++++++++++++++++-
 Civi/Api4/Generic/AbstractCreateAction.php |  4 +++-
 Civi/Api4/Generic/AbstractSaveAction.php   | 15 +++++++++++++-
 Civi/Api4/Generic/AbstractUpdateAction.php |  9 ++++++++-
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Civi/Api4/Event/ValidateValuesEvent.php b/Civi/Api4/Event/ValidateValuesEvent.php
index 146f29f2cb1d..6444ad3b6739 100644
--- a/Civi/Api4/Event/ValidateValuesEvent.php
+++ b/Civi/Api4/Event/ValidateValuesEvent.php
@@ -70,6 +70,24 @@ class ValidateValuesEvent extends GenericHookEvent {
    */
   public $records;
 
+  /**
+   * Detailed, side-by-side comparison of old and new values.
+   *
+   * This requires loading the list of old values from the database. Consequently,
+   * reading `$diffs` is more expensive than reading `$records`, so you should only use it if
+   * really necessary.
+   *
+   * The list of $diffs may be important if you are enforcing a rule that involves
+   * multiple fields. (Ex: "Validate that the state_id and country_id match.")
+   *
+   * When possible, $records and $diffs will have the same number of items (with corresponding
+   * keys). However, in the case of a batch `update()`, the list of diffs will be longer.
+   *
+   * @var array|\CRM_Utils_LazyArray
+   *   Each item is a record of the form ['old' => $fieldValues, 'new' => $fieldValues]
+   */
+  public $diffs;
+
   /**
    * List of error messages.
    *
@@ -85,10 +103,13 @@ class ValidateValuesEvent extends GenericHookEvent {
    * @param \Civi\Api4\Generic\AbstractAction $apiRequest
    * @param array|\CRM_Utils_LazyArray $records
    *   List of updates (akin to SaveAction::$records).
+   * @param array|\CRM_Utils_LazyArray $diffs
+   *   List of differences (comparing old values and new values).
    */
-  public function __construct($apiRequest, $records) {
+  public function __construct($apiRequest, $records, $diffs) {
     $this->setApiRequest($apiRequest);
     $this->records = $records;
+    $this->diffs = $diffs;
     $this->errors = [];
   }
 
diff --git a/Civi/Api4/Generic/AbstractCreateAction.php b/Civi/Api4/Generic/AbstractCreateAction.php
index 5299741a8f04..153e26ce6f59 100644
--- a/Civi/Api4/Generic/AbstractCreateAction.php
+++ b/Civi/Api4/Generic/AbstractCreateAction.php
@@ -66,7 +66,9 @@ protected function validateValues() {
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
-    $e = new ValidateValuesEvent($this, [$this->getValues()]);
+    $e = new ValidateValuesEvent($this, [$this->getValues()], new \CRM_Utils_LazyArray(function () {
+      return [['old' => NULL, 'new' => $this->getValues()]];
+    }));
     \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
     if (!empty($e->errors)) {
       throw $e->toException();
diff --git a/Civi/Api4/Generic/AbstractSaveAction.php b/Civi/Api4/Generic/AbstractSaveAction.php
index d67fa3c2c5e0..29f329cc661e 100644
--- a/Civi/Api4/Generic/AbstractSaveAction.php
+++ b/Civi/Api4/Generic/AbstractSaveAction.php
@@ -105,7 +105,20 @@ protected function validateValues() {
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
-    $e = new ValidateValuesEvent($this, $this->records);
+    $e = new ValidateValuesEvent($this, $this->records, new \CRM_Utils_LazyArray(function() {
+      $existingIds = array_column($this->records, $this->idField);
+      $existing = civicrm_api4($this->getEntityName(), 'get', [
+        'checkPermissions' => $this->checkPermissions,
+        'where' => [[$this->idField, 'IN', $existingIds]],
+      ], $this->idField);
+
+      $result = [];
+      foreach ($this->records as $k => $new) {
+        $old = isset($new[$this->idField]) ? $existing[$new[$this->idField]] : NULL;
+        $result[$k] = ['old' => $old, 'new' => $new];
+      }
+      return $result;
+    }));
     \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
     if (!empty($e->errors)) {
       throw $e->toException();
diff --git a/Civi/Api4/Generic/AbstractUpdateAction.php b/Civi/Api4/Generic/AbstractUpdateAction.php
index f4fb2ec8f8db..42a61cccf7a0 100644
--- a/Civi/Api4/Generic/AbstractUpdateAction.php
+++ b/Civi/Api4/Generic/AbstractUpdateAction.php
@@ -77,7 +77,14 @@ public function addValue(string $fieldName, $value) {
    */
   protected function validateValues() {
     // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
-    $e = new ValidateValuesEvent($this, [$this->values]);
+    $e = new ValidateValuesEvent($this, [$this->values], new \CRM_Utils_LazyArray(function () {
+      $existing = $this->getBatchAction()->setSelect(['*'])->execute();
+      $result = [];
+      foreach ($existing as $record) {
+        $result[] = ['old' => $record, 'new' => $this->values];
+      }
+      return $result;
+    }));
     \Civi::dispatcher()->dispatch('civi.api4.validate', $e);
     if (!empty($e->errors)) {
       throw $e->toException();

From 61e8985d11e888fd6a1a2ebc2e32bc8348c44e9f Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 19 May 2021 23:17:25 -0700
Subject: [PATCH 08/27] Add ValidateValuesTest

---
 .../api/v4/Entity/ValidateValuesTest.php      | 214 ++++++++++++++++++
 1 file changed, 214 insertions(+)
 create mode 100644 tests/phpunit/api/v4/Entity/ValidateValuesTest.php

diff --git a/tests/phpunit/api/v4/Entity/ValidateValuesTest.php b/tests/phpunit/api/v4/Entity/ValidateValuesTest.php
new file mode 100644
index 000000000000..3cc1a835010f
--- /dev/null
+++ b/tests/phpunit/api/v4/Entity/ValidateValuesTest.php
@@ -0,0 +1,214 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+namespace api\v4\Entity;
+
+use Civi\Api4\Contact;
+use api\v4\UnitTestCase;
+use Civi\Api4\Event\ValidateValuesEvent;
+use Civi\Test\TransactionalInterface;
+
+/**
+ * Assert that 'validateValues' runs during
+ *
+ * @group headless
+ */
+class ValidateValuesTest extends UnitTestCase implements TransactionalInterface {
+
+  private $lastValidator;
+
+  protected function setUp(): void {
+    $this->lastValidator = NULL;
+    parent::setUp();
+  }
+
+  protected function tearDown(): void {
+    $this->setValidator(NULL);
+    parent::tearDown();
+  }
+
+  /**
+   * Fire ValidateValuesEvent several times - and ensure it conveys the
+   * expected data.
+   *
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  public function testHookData() {
+    $hookCount = 0;
+
+    // Step 1: `create()` a record for Ms. Alice Alison.
+    $this->setValidator(function (ValidateValuesEvent $e) use (&$hookCount) {
+      $this->assertWellFormedEvent($e);
+      if ($e->getEntityName() !== 'Contact') {
+        return;
+      }
+
+      $hookCount++;
+      $this->assertEquals('create', $e->getActionName());
+      $this->assertEquals('Alice', $e->records[0]['first_name']);
+      $this->assertEquals('Alison', $e->records[0]['last_name']);
+
+      $this->assertFalse($e->diffs->isLoaded());
+      $this->assertEquals(1, count($e->diffs));
+      $this->assertEquals(NULL, $e->diffs[0]['old']);
+      $this->assertEquals('Alice', $e->diffs[0]['new']['first_name']);
+      $this->assertEquals('Alison', $e->diffs[0]['new']['last_name']);
+
+    });
+    $created = Contact::create()->setValues([
+      'contact_type' => 'Individual',
+      'first_name' => 'Alice',
+      'last_name' => 'Alison',
+    ])->execute()->single();
+    $this->assertTrue(is_numeric($created['id']));
+    $this->assertEquals(1, $hookCount);
+
+    // Step 2: `save()` a couple records for Ms. Alice Alison and Mr. Bob Bobmom.
+    $this->setValidator(function (ValidateValuesEvent $e) use (&$hookCount) {
+      $this->assertWellFormedEvent($e);
+      if ($e->getEntityName() !== 'Contact') {
+        return;
+      }
+
+      $hookCount++;
+      $this->assertEquals('save', $e->getActionName());
+      $this->assertEquals('Alicia', $e->records[0]['first_name']);
+      $this->assertEquals('Bob', $e->records[1]['first_name']);
+
+      $this->assertFalse($e->diffs->isLoaded());
+      $this->assertEquals(2, count($e->diffs));
+      $this->assertEquals('Alice', $e->diffs[0]['old']['first_name']);
+      $this->assertEquals('Alicia', $e->diffs[0]['new']['first_name']);
+      $this->assertEquals(NULL, $e->diffs[1]['old']);
+      $this->assertEquals('Bob', $e->diffs[1]['new']['first_name']);
+
+    });
+    $saved = Contact::save()->setRecords([
+      ['id' => $created['id'], 'first_name' => 'Alicia'],
+      ['contact_type' => 'Individual', 'first_name' => 'Bob', 'last_name' => 'Bobmom'],
+    ])->execute();
+    $this->assertEquals(2, $saved->count());
+    $this->assertEquals(2, $hookCount);
+
+    // Step 3: `update()` a record for Mr. Bob Bobmom
+    $this->setValidator(function (ValidateValuesEvent $e) use (&$hookCount) {
+      $this->assertWellFormedEvent($e);
+      if ($e->getEntityName() !== 'Contact') {
+        return;
+      }
+
+      $hookCount++;
+      $this->assertEquals('update', $e->getActionName());
+      $this->assertEquals('Bobby', $e->records[0]['first_name']);
+
+      $this->assertFalse($e->diffs->isLoaded());
+      $this->assertEquals(1, count($e->diffs));
+      $this->assertEquals('Bob', $e->diffs[0]['old']['first_name']);
+      $this->assertEquals('Bobmom', $e->diffs[0]['old']['last_name']);
+      $this->assertEquals('Bobby', $e->diffs[0]['new']['first_name']);
+    });
+    $updated = Contact::update()
+      ->setValues(['first_name' => 'Bobby'])
+      ->addWhere('last_name', '=', 'Bobmom')
+      ->execute();
+    $this->assertEquals(1, $updated->count());
+    $this->assertEquals(3, $hookCount);
+  }
+
+  public function testRaiseError() {
+    $this->setValidator(function (ValidateValuesEvent $e) use (&$hookCount) {
+      $this->assertWellFormedEvent($e);
+      if ($e->getEntityName() !== 'Contact') {
+        return;
+      }
+
+      foreach ($e->records as $k => $record) {
+        $e->addError($k, 'first_name', 'not-namey-enough', ts('The first name is not sufficiently namey.'));
+        $e->addError($k, ['first_name', 'last_name'], 'tongue-twister', ts('When the names are put together, they become a tongue twister.'));
+        $e->errors[] = [
+          'record' => $k,
+          'fields' => ['last_name'],
+          'name' => 'misspelled',
+          'message' => ts('I disagree with the spelling of your name.'),
+        ];
+      }
+
+      $hookCount++;
+    });
+
+    try {
+      Contact::create()->setValues([
+        'contact_type' => 'Individual',
+        'first_name' => 'Alice',
+        'last_name' => 'Alison',
+      ])->execute();
+      $this->fail('Expected an exception due to validation error');
+    }
+    catch (\API_Exception $e) {
+      $this->assertEquals(1, $hookCount);
+      $this->assertRegExp(';not sufficiently namey;', $e->getMessage());
+      $this->assertRegExp(';tongue twister;', $e->getMessage());
+      $this->assertRegExp(';disagree with the spelling;', $e->getMessage());
+    }
+  }
+
+  /**
+   * Add/replace the validator.
+   *
+   * @param callable $func
+   */
+  protected function setValidator($func) {
+    $dispatcher = \Civi::dispatcher();
+    if ($this->lastValidator) {
+      $dispatcher->removeListener('civi.api4.validate', $this->lastValidator);
+    }
+    if ($func) {
+      $dispatcher->addListener('civi.api4.validate', $func);
+    }
+    $this->lastValidator = $func;
+  }
+
+  protected function assertWellFormedEvent(ValidateValuesEvent $e) {
+    $this->assertRegExp('/Contact/', $e->getEntityName());
+    $this->assertRegExp('/create|save|update/', $e->getActionName());
+    $this->assertTrue(count($e->records) > 0);
+    foreach ($e->records as $record) {
+      $this->assertWellFormedFields($record);
+    }
+
+    // We want to let the main test do some assertions on the lazy-array, so we'll peek a clone.
+    $peekAtDiffs = clone $e->diffs;
+    $this->assertTrue(count($peekAtDiffs) > 0);
+    foreach ($peekAtDiffs as $diff) {
+      $this->assertTrue(is_array($diff['new']));
+      $this->assertWellFormedFields($diff['new']);
+      if ($diff['old'] !== NULL) {
+        $this->assertTrue(is_array($diff['old']));
+        $this->assertWellFormedFields($diff['old']);
+      }
+    }
+  }
+
+  protected function assertWellFormedFields($record) {
+    foreach ($record as $field => $value) {
+      $this->assertRegExp('/^[a-zA-Z0-9_]+$/', $field);
+    }
+  }
+
+}

From 3f0340c8e9fe79987f76ff4e67a78a83db2d34b4 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 3 Jun 2021 20:13:48 -0700
Subject: [PATCH 09/27] FinancialItem - Provide defaults so that stricter
 ConformanceTest will pas

Context: There were three separate, concurrent PRs - two added more tests
and events to APIv4, and the third added a new entity (FinancialItem).
FinancialItem got merged first.  I'm working reconciling the other two...
and discovered that `FinancialItem` isn't passing.

Problem: When the `ConformanceTest` creates a `FinancialItem`, it doesn't
fill in valid values for `entity_table,entity_id`. These values are
important to the access-control criteria used in reading-back data.
---
 .../FinancialItemCreationSpecProvider.php     | 52 +++++++++++++++++++
 .../Service/TestCreationParameterProvider.php | 13 ++++-
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 Civi/Api4/Service/Spec/Provider/FinancialItemCreationSpecProvider.php

diff --git a/Civi/Api4/Service/Spec/Provider/FinancialItemCreationSpecProvider.php b/Civi/Api4/Service/Spec/Provider/FinancialItemCreationSpecProvider.php
new file mode 100644
index 000000000000..4567d2bed19b
--- /dev/null
+++ b/Civi/Api4/Service/Spec/Provider/FinancialItemCreationSpecProvider.php
@@ -0,0 +1,52 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+
+namespace Civi\Api4\Service\Spec\Provider;
+
+use Civi\Api4\Service\Spec\RequestSpec;
+
+class FinancialItemCreationSpecProvider implements Generic\SpecProviderInterface {
+
+  // I'm not sure it makes sense to have a default `entity_table`... actually, I don't even know if it makes
+  // sense to expose `FinancialItem` as a public API, for what that's worth. But it's there, so clearly it does.
+  //  And the ConformanceTests require that you be able to create (and read-back) a record using metadata.
+
+  const DEFAULT_TABLE = 'civicrm_line_item';
+  const DEFAULT_ENTITY = 'LineItem';
+
+  /**
+   * @param \Civi\Api4\Service\Spec\RequestSpec $spec
+   */
+  public function modifySpec(RequestSpec $spec) {
+    $spec->getFieldByName('entity_table')->setRequired(TRUE);
+    $spec->getFieldByName('entity_id')->setRequired(TRUE);
+    $spec->getFieldByName('entity_table')->setDefaultValue(self::DEFAULT_TABLE);
+  }
+
+  /**
+   * @param string $entity
+   * @param string $action
+   *
+   * @return bool
+   */
+  public function applies($entity, $action) {
+    return $entity === 'FinancialItem' && $action === 'create';
+  }
+
+}
diff --git a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php
index 632945ed4934..757258b48e68 100644
--- a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php
+++ b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php
@@ -20,6 +20,7 @@
 namespace api\v4\Service;
 
 use Civi\Api4\Service\Spec\FieldSpec;
+use Civi\Api4\Service\Spec\Provider\FinancialItemCreationSpecProvider;
 use Civi\Api4\Service\Spec\SpecGatherer;
 
 class TestCreationParameterProvider {
@@ -82,9 +83,19 @@ private function getRequiredValue(FieldSpec $field) {
     elseif ($field->getDefaultValue()) {
       return $field->getDefaultValue();
     }
-    elseif (in_array($field->getName(), ['entity_id', 'contact_id'])) {
+    elseif ($field->getName() === 'contact_id') {
       return $this->getFkID($field, 'Contact');
     }
+    elseif ($field->getName() === 'entity_id') {
+      // What could possibly go wrong with this?
+      switch ($field->getTableName()) {
+        case 'civicrm_financial_item':
+          return $this->getFkID($field, FinancialItemCreationSpecProvider::DEFAULT_ENTITY);
+
+        default:
+          return $this->getFkID($field, 'Contact');
+      }
+    }
 
     $randomValue = $this->getRandomValue($field->getDataType());
 

From 572b221126f5f603215f415811fdb83056a6bf24 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 3 Jun 2021 23:05:19 -0700
Subject: [PATCH 10/27] DAOCreateAction - Fill defaults before validating write

---
 Civi/Api4/Generic/DAOCreateAction.php | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Civi/Api4/Generic/DAOCreateAction.php b/Civi/Api4/Generic/DAOCreateAction.php
index 336b26be7502..cf88e31910cc 100644
--- a/Civi/Api4/Generic/DAOCreateAction.php
+++ b/Civi/Api4/Generic/DAOCreateAction.php
@@ -33,11 +33,10 @@ class DAOCreateAction extends AbstractCreateAction {
    */
   public function _run(Result $result) {
     $this->formatWriteValues($this->values);
+    $this->fillDefaults($this->values);
     $this->validateValues();
-    $params = $this->values;
-    $this->fillDefaults($params);
-    $items = [$params];
 
+    $items = [$this->values];
     $result->exchangeArray($this->writeObjects($items));
   }
 

From d291b3694deaaa4d3c220b300d7b213f1b833afd Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 3 Jun 2021 23:12:23 -0700
Subject: [PATCH 11/27] UFJoin - Update addSelectWhereClause

---
 CRM/Core/BAO/UFJoin.php | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/CRM/Core/BAO/UFJoin.php b/CRM/Core/BAO/UFJoin.php
index b3514d9f9835..df3643984d50 100644
--- a/CRM/Core/BAO/UFJoin.php
+++ b/CRM/Core/BAO/UFJoin.php
@@ -175,4 +175,15 @@ public static function entityTables() {
     ];
   }
 
+  /**
+   * Override base method which assumes permissions should be based on entity_table.
+   *
+   * @return array
+   */
+  public function addSelectWhereClause() {
+    $clauses = [];
+    CRM_Utils_Hook::selectWhereClause($this, $clauses);
+    return $clauses;
+  }
+
 }

From 9cef60668e2373de629f43d8c32c9c8a16964b18 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 12 Apr 2021 08:22:52 +1200
Subject: [PATCH 12/27] Add BAO function and hook for checkAccess

This adds a static ::checkAccess function to all BAOs, which dispatches to
a protected _checkAccess function in that BAO, as well as a new hook:
hook_civicrm_checkAccess($entity, $action, $record, $contactID, &$granted)
---
 CRM/Core/DAO.php                              | 40 ++++++++++++++++++-
 CRM/Core/DAO/AllCoreTables.php                |  2 +-
 CRM/Financial/BAO/FinancialType.php           |  6 +--
 CRM/Utils/Hook.php                            | 25 ++++++++++++
 api/v3/Contribution.php                       | 16 ++------
 ext/financialacls/financialacls.php           | 26 ++++++++++++
 tests/phpunit/api/v3/FinancialTypeACLTest.php |  6 ++-
 7 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 80271dd4496d..d8dbad2760a2 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -2993,7 +2993,7 @@ public function addSelectWhereClause() {
         $clauses[$fieldName] = CRM_Utils_SQL::mergeSubquery('Contact');
       }
       // Clause for an entity_table/entity_id combo
-      if ($fieldName == 'entity_id' && isset($fields['entity_table'])) {
+      if ($fieldName === 'entity_id' && isset($fields['entity_table'])) {
         $relatedClauses = [];
         $relatedEntities = $this->buildOptions('entity_table', 'get');
         foreach ((array) $relatedEntities as $table => $ent) {
@@ -3040,9 +3040,45 @@ public static function getSelectWhereClause($tableAlias = NULL) {
     return $clauses;
   }
 
+  /**
+   * Check whether action can be performed on a given record.
+   *
+   * Dispatches to internal BAO function ('static::_checkAccess())` and `hook_civicrm_checkAccess`.
+   *
+   * @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 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);
+    // 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)
+    ) {
+      throw new CRM_Core_Exception('Function checkAccess must be called on a BAO class');
+    }
+    $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);
+    }
+    // Dispatch to hook
+    CRM_Utils_Hook::checkAccess($entityName, $action, $record, $userID, $granted);
+    return $granted;
+  }
+
   /**
    * ensure database name is 'safe', i.e. only contains word characters (includes underscores)
-   * and dashes, and contains at least one [a-z] case insenstive.
+   * and dashes, and contains at least one [a-z] case insensitive.
    *
    * @param $database
    *
diff --git a/CRM/Core/DAO/AllCoreTables.php b/CRM/Core/DAO/AllCoreTables.php
index 04e4fe32dc81..f117d68148ae 100644
--- a/CRM/Core/DAO/AllCoreTables.php
+++ b/CRM/Core/DAO/AllCoreTables.php
@@ -203,7 +203,7 @@ public static function getCanonicalClassName($baoName) {
    */
   public static function getBAOClassName($daoName) {
     $baoName = str_replace('_DAO_', '_BAO_', $daoName);
-    return class_exists($baoName) ? $baoName : $daoName;
+    return $daoName === $baoName || class_exists($baoName) ? $baoName : $daoName;
   }
 
   /**
diff --git a/CRM/Financial/BAO/FinancialType.php b/CRM/Financial/BAO/FinancialType.php
index 9003eb8d1630..86d3528066f0 100644
--- a/CRM/Financial/BAO/FinancialType.php
+++ b/CRM/Financial/BAO/FinancialType.php
@@ -420,20 +420,20 @@ public static function buildPermissionedClause(&$whereClauses, $component = NULL
    * @param string $op
    *   the mode of operation, can be add, view, edit, delete
    * @param bool $force
+   * @param int $contactID
    *
    * @return bool
    */
-  public static function checkPermissionedLineItems($id, $op, $force = TRUE) {
+  public static function checkPermissionedLineItems($id, $op, $force = TRUE, $contactID = NULL) {
     if (!self::isACLFinancialTypeStatus()) {
       return TRUE;
     }
     $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($id);
     $flag = FALSE;
     foreach ($lineItems as $items) {
-      if (!CRM_Core_Permission::check($op . ' contributions of type ' . CRM_Contribute_PseudoConstant::financialType($items['financial_type_id']))) {
+      if (!CRM_Core_Permission::check($op . ' contributions of type ' . CRM_Contribute_PseudoConstant::financialType($items['financial_type_id']), $contactID)) {
         if ($force) {
           throw new CRM_Core_Exception(ts('You do not have permission to access this page.'));
-          break;
         }
         $flag = FALSE;
         break;
diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
index 82877f3feb08..e2907bd9a6fd 100644
--- a/CRM/Utils/Hook.php
+++ b/CRM/Utils/Hook.php
@@ -2100,6 +2100,31 @@ public static function permission_check($permission, &$granted, $contactId) {
     );
   }
 
+  /**
+   * Check whether the given contact has access to perform the given action.
+   *
+   * If the contact cannot perform the action the
+   *
+   * @param string $entity
+   *   APIv4 entity name.
+   *   Ex: 'Contact', 'Email', 'Event'
+   * @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|null $contactID
+   *   Contact ID of the active user (whose access we must check).
+   * @param bool $granted
+   */
+  public static function checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+    self::singleton()->invoke(['entity', 'action', 'record', 'contactID', 'granted'], $entity, $action, $record,
+      $contactID, $granted, self::$_nullObject,
+      'civicrm_checkAccess'
+    );
+  }
+
   /**
    * Rotate the cryptographic key used in the database.
    *
diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index a4a358136157..eeae8e5855b0 100644
--- a/api/v3/Contribution.php
+++ b/api/v3/Contribution.php
@@ -204,23 +204,13 @@ 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'];
-  // First check contribution financial type
-  $financialType = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionID, 'financial_type_id');
-  // Now check permissioned lineitems & permissioned contribution
-  if (!empty($params['check_permissions']) && CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus() &&
-    (
-      !CRM_Core_Permission::check('delete contributions of type ' . CRM_Contribute_PseudoConstant::financialType($financialType))
-      || !CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributionID, 'delete', FALSE)
-    )
-  ) {
+  if (!empty($params['check_permissions']) && !CRM_Contribute_BAO_Contribution::checkAccess('delete', ['id' => $contributionID])) {
     throw new API_Exception('You do not have permission to delete this contribution');
   }
   if (CRM_Contribute_BAO_Contribution::deleteContribution($contributionID)) {
     return civicrm_api3_create_success([$contributionID => 1]);
   }
-  else {
-    throw new API_Exception('Could not delete contribution');
-  }
+  throw new API_Exception('Could not delete contribution');
 }
 
 /**
@@ -671,7 +661,7 @@ function _ipn_process_transaction($params, $contribution, $input, $ids) {
     static $domainFromName;
     static $domainFromEmail;
     if (empty($domainFromEmail) && (empty($params['receipt_from_name']) || empty($params['receipt_from_email']))) {
-      list($domainFromName, $domainFromEmail) = CRM_Core_BAO_Domain::getNameAndEmail(TRUE);
+      [$domainFromName, $domainFromEmail] = CRM_Core_BAO_Domain::getNameAndEmail(TRUE);
     }
     $input['receipt_from_name'] = CRM_Utils_Array::value('receipt_from_name', $params, $domainFromName);
     $input['receipt_from_email'] = CRM_Utils_Array::value('receipt_from_email', $params, $domainFromEmail);
diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php
index ddd51b22a252..569346d72fcc 100644
--- a/ext/financialacls/financialacls.php
+++ b/ext/financialacls/financialacls.php
@@ -286,6 +286,32 @@ function financialacls_civicrm_permission(&$permissions) {
   ];
 }
 
+/**
+ * @param string $entity
+ * @param string $action
+ * @param array $record
+ * @param int|null $contactID
+ * @param bool $granted
+ *
+ * @throws \CRM_Core_Exception
+ */
+function financialacls_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+  if (!financialacls_is_acl_limiting_enabled()) {
+    return;
+  }
+  if ($action === 'delete' && $entity === 'Contribution') {
+    $contributionID = $record['id'];
+    // First check contribution financial type
+    $financialType = CRM_Core_PseudoConstant::getName('CRM_Contribute_DAO_Contribution', 'financial_type_id', CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionID, 'financial_type_id'));
+    // Now check permissioned line items & permissioned contribution
+    if (!CRM_Core_Permission::check('delete contributions of type ' . $financialType, $contactID) ||
+      !CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributionID, 'delete', FALSE, $contactID)
+    ) {
+      $granted = FALSE;
+    }
+  }
+}
+
 /**
  * Remove unpermitted financial types from field Options in search context.
  *
diff --git a/tests/phpunit/api/v3/FinancialTypeACLTest.php b/tests/phpunit/api/v3/FinancialTypeACLTest.php
index 38b6eb83d78b..b019ed7eeaa3 100644
--- a/tests/phpunit/api/v3/FinancialTypeACLTest.php
+++ b/tests/phpunit/api/v3/FinancialTypeACLTest.php
@@ -291,8 +291,10 @@ public function testEditACLContribution() {
 
   /**
    * Test that acl contributions can be deleted.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testDeleteACLContribution() {
+  public function testDeleteACLContribution(): void {
     $this->enableFinancialACLs();
 
     $this->setPermissions([
@@ -313,7 +315,7 @@ public function testDeleteACLContribution() {
     $this->addFinancialAclPermissions([['delete', 'Donation']]);
     $contribution = $this->callAPISuccess('Contribution', 'delete', $params);
 
-    $this->assertEquals($contribution['count'], 1);
+    $this->assertEquals(1, $contribution['count']);
   }
 
   public function testMembershipTypeACLFinancialTypeACL() {

From 929a9585fec8683805087f134d6185a0cc84322b Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 27 Apr 2021 14:51:02 -0400
Subject: [PATCH 13/27] APIv4 - Add checkAccess action

Call checkAccess action before creating, updating or deleting
---
 Civi/Api4/CustomValue.php                     |  7 ++
 Civi/Api4/Generic/AbstractCreateAction.php    |  8 ++
 Civi/Api4/Generic/AbstractEntity.php          |  7 ++
 Civi/Api4/Generic/AbstractSaveAction.php      | 13 +++
 Civi/Api4/Generic/BasicBatchAction.php        |  5 ++
 Civi/Api4/Generic/BasicUpdateAction.php       |  8 +-
 Civi/Api4/Generic/CheckAccessAction.php       | 80 +++++++++++++++++++
 Civi/Api4/Generic/DAODeleteAction.php         |  6 ++
 Civi/Api4/Generic/DAOUpdateAction.php         |  9 +++
 Civi/Api4/Utils/CoreUtil.php                  | 37 +++++++++
 .../api/v4/Action/BasicActionsTest.php        |  2 +-
 11 files changed, 180 insertions(+), 2 deletions(-)
 create mode 100644 Civi/Api4/Generic/CheckAccessAction.php

diff --git a/Civi/Api4/CustomValue.php b/Civi/Api4/CustomValue.php
index 40b4d9616081..e036aa48e844 100644
--- a/Civi/Api4/CustomValue.php
+++ b/Civi/Api4/CustomValue.php
@@ -122,6 +122,13 @@ public static function getActions($customGroup = NULL, $checkPermissions = TRUE)
       ->setCheckPermissions($checkPermissions);
   }
 
+  /**
+   * @return \Civi\Api4\Generic\CheckAccessAction
+   */
+  public static function checkAccess($customGroup) {
+    return new Generic\CheckAccessAction("Custom_$customGroup", __FUNCTION__);
+  }
+
   /**
    * @see \Civi\Api4\Generic\AbstractEntity::permissions()
    * @return array
diff --git a/Civi/Api4/Generic/AbstractCreateAction.php b/Civi/Api4/Generic/AbstractCreateAction.php
index 153e26ce6f59..b08cb3e39c2f 100644
--- a/Civi/Api4/Generic/AbstractCreateAction.php
+++ b/Civi/Api4/Generic/AbstractCreateAction.php
@@ -20,6 +20,8 @@
 namespace Civi\Api4\Generic;
 
 use Civi\Api4\Event\ValidateValuesEvent;
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
 
 /**
  * Base class for all `Create` api actions.
@@ -59,6 +61,7 @@ public function addValue(string $fieldName, $value) {
 
   /**
    * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
   protected function validateValues() {
     // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
@@ -66,6 +69,11 @@ protected function validateValues() {
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
+
+    if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $this->getValues())) {
+      throw new UnauthorizedException("ACL check failed");
+    }
+
     $e = new ValidateValuesEvent($this, [$this->getValues()], new \CRM_Utils_LazyArray(function () {
       return [['old' => NULL, 'new' => $this->getValues()]];
     }));
diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php
index 72ef2bab19b4..348015fb3c39 100644
--- a/Civi/Api4/Generic/AbstractEntity.php
+++ b/Civi/Api4/Generic/AbstractEntity.php
@@ -55,6 +55,13 @@ public static function getActions($checkPermissions = TRUE) {
    */
   abstract public static function getFields();
 
+  /**
+   * @return \Civi\Api4\Generic\CheckAccessAction
+   */
+  public static function checkAccess() {
+    return new CheckAccessAction(self::getEntityName(), __FUNCTION__);
+  }
+
   /**
    * Returns a list of permissions needed to access the various actions in this api.
    *
diff --git a/Civi/Api4/Generic/AbstractSaveAction.php b/Civi/Api4/Generic/AbstractSaveAction.php
index 29f329cc661e..aefd3725edbb 100644
--- a/Civi/Api4/Generic/AbstractSaveAction.php
+++ b/Civi/Api4/Generic/AbstractSaveAction.php
@@ -20,6 +20,8 @@
 namespace Civi\Api4\Generic;
 
 use Civi\Api4\Event\ValidateValuesEvent;
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
 
 /**
  * Create or update one or more $ENTITIES.
@@ -93,6 +95,7 @@ public function __construct($entityName, $actionName, $idField = 'id') {
 
   /**
    * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
   protected function validateValues() {
     // FIXME: There should be a protocol to report a full list of errors... Perhaps a subclass of API_Exception?
@@ -105,6 +108,16 @@ protected function validateValues() {
     if ($unmatched) {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
+
+    if ($this->checkPermissions) {
+      foreach ($this->records as $record) {
+        $action = empty($record[$this->idField]) ? 'create' : 'update';
+        if (!CoreUtil::checkAccess($this->getEntityName(), $action, $record)) {
+          throw new UnauthorizedException("ACL check failed");
+        }
+      }
+    }
+
     $e = new ValidateValuesEvent($this, $this->records, new \CRM_Utils_LazyArray(function() {
       $existingIds = array_column($this->records, $this->idField);
       $existing = civicrm_api4($this->getEntityName(), 'get', [
diff --git a/Civi/Api4/Generic/BasicBatchAction.php b/Civi/Api4/Generic/BasicBatchAction.php
index d1787bd44a12..1806f9165bef 100644
--- a/Civi/Api4/Generic/BasicBatchAction.php
+++ b/Civi/Api4/Generic/BasicBatchAction.php
@@ -20,6 +20,8 @@
 namespace Civi\Api4\Generic;
 
 use Civi\API\Exception\NotImplementedException;
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
 
 /**
  * $ACTION one or more $ENTITIES.
@@ -65,6 +67,9 @@ public function __construct($entityName, $actionName, $select = 'id', $doer = NU
    */
   public function _run(Result $result) {
     foreach ($this->getBatchRecords() as $item) {
+      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+        throw new UnauthorizedException("ACL check failed");
+      }
       $result[] = $this->doTask($item);
     }
   }
diff --git a/Civi/Api4/Generic/BasicUpdateAction.php b/Civi/Api4/Generic/BasicUpdateAction.php
index 4fd8fe6c156b..f00dfe358f48 100644
--- a/Civi/Api4/Generic/BasicUpdateAction.php
+++ b/Civi/Api4/Generic/BasicUpdateAction.php
@@ -20,6 +20,8 @@
 namespace Civi\Api4\Generic;
 
 use Civi\API\Exception\NotImplementedException;
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
 
 /**
  * Update one or more $ENTITY with new values.
@@ -60,7 +62,11 @@ public function _run(Result $result) {
     $this->formatWriteValues($this->values);
     $this->validateValues();
     foreach ($this->getBatchRecords() as $item) {
-      $result[] = $this->writeRecord($this->values + $item);
+      $record = $this->values + $item;
+      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $record)) {
+        throw new UnauthorizedException("ACL check failed");
+      }
+      $result[] = $this->writeRecord($record);
     }
   }
 
diff --git a/Civi/Api4/Generic/CheckAccessAction.php b/Civi/Api4/Generic/CheckAccessAction.php
new file mode 100644
index 000000000000..cc26a5e6e555
--- /dev/null
+++ b/Civi/Api4/Generic/CheckAccessAction.php
@@ -0,0 +1,80 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+
+namespace Civi\Api4\Generic;
+
+use Civi\Api4\Utils\CoreUtil;
+
+/**
+ * Check if current user is authorized to perform specified action on a given $ENTITY.
+ *
+ * @method $this setAction(string $action)
+ * @method string getAction()
+ * @method $this setValues(array $values)
+ * @method array getValues()
+ */
+class CheckAccessAction extends AbstractAction {
+
+  /**
+   * @var string
+   * @required
+   */
+  protected $action;
+
+  /**
+   * @var array
+   * @required
+   */
+  protected $values = [];
+
+  /**
+   * @param \Civi\Api4\Generic\Result $result
+   */
+  public function _run(Result $result) {
+    // Prevent circular checks
+    if ($this->action === 'checkAccess') {
+      $granted = TRUE;
+    }
+    else {
+      $granted = CoreUtil::checkAccess($this->getEntityName(), $this->action, $this->values);
+    }
+    $result->exchangeArray([['access' => $granted]]);
+  }
+
+  /**
+   * This action is always allowed
+   *
+   * @return bool
+   */
+  public function isAuthorized() {
+    return TRUE;
+  }
+
+  /**
+   * Add an item to the values array
+   * @param string $fieldName
+   * @param mixed $value
+   * @return $this
+   */
+  public function addValue(string $fieldName, $value) {
+    $this->values[$fieldName] = $value;
+    return $this;
+  }
+
+}
diff --git a/Civi/Api4/Generic/DAODeleteAction.php b/Civi/Api4/Generic/DAODeleteAction.php
index ecd9c91c1316..bf335c0f404c 100644
--- a/Civi/Api4/Generic/DAODeleteAction.php
+++ b/Civi/Api4/Generic/DAODeleteAction.php
@@ -19,6 +19,9 @@
 
 namespace Civi\Api4\Generic;
 
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
+
 /**
  * Delete one or more $ENTITIES.
  *
@@ -53,6 +56,9 @@ protected function deleteObjects($items) {
 
     if ($this->getCheckPermissions()) {
       foreach (array_keys($items) as $key) {
+        if (!CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $items[$key])) {
+          throw new UnauthorizedException("ACL check failed");
+        }
         $items[$key]['check_permissions'] = TRUE;
         $this->checkContactPermissions($baoName, $items[$key]);
       }
diff --git a/Civi/Api4/Generic/DAOUpdateAction.php b/Civi/Api4/Generic/DAOUpdateAction.php
index aef6618729f5..b545fbc22540 100644
--- a/Civi/Api4/Generic/DAOUpdateAction.php
+++ b/Civi/Api4/Generic/DAOUpdateAction.php
@@ -19,6 +19,9 @@
 
 namespace Civi\Api4\Generic;
 
+use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Utils\CoreUtil;
+
 /**
  * Update one or more $ENTITY with new values.
  *
@@ -60,6 +63,9 @@ public function _run(Result $result) {
     // Update a single record by ID unless select requires more than id
     if ($this->getSelect() === ['id'] && count($this->where) === 1 && $this->where[0][0] === 'id' && $this->where[0][1] === '=' && !empty($this->where[0][2])) {
       $this->values['id'] = $this->where[0][2];
+      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $this->values)) {
+        throw new UnauthorizedException("ACL check failed");
+      }
       $items = [$this->values];
       $this->validateValues();
       $result->exchangeArray($this->writeObjects($items));
@@ -70,6 +76,9 @@ public function _run(Result $result) {
     $items = $this->getBatchRecords();
     foreach ($items as &$item) {
       $item = $this->values + $item;
+      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+        throw new UnauthorizedException("ACL check failed");
+      }
     }
 
     $this->validateValues();
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index c5b0d5e82957..65a3d81ed9fd 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -19,6 +19,7 @@
 
 namespace Civi\Api4\Utils;
 
+use Civi\API\Request;
 use CRM_Core_DAO_AllCoreTables as AllCoreTables;
 
 class CoreUtil {
@@ -151,4 +152,40 @@ private static function isCustomEntity($customGroupName) {
     return $customGroupName && \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $customGroupName, 'is_multiple', 'name');
   }
 
+  /**
+   * Check if current user is authorized to perform specified action on a given entity.
+   *
+   * @param string $entityName
+   * @param string $actionName
+   * @param array $record
+   * @return bool
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \Civi\API\Exception\NotImplementedException
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  public static function checkAccess(string $entityName, string $actionName, array $record) {
+    $action = Request::create($entityName, $actionName, ['version' => 4]);
+    // This checks gatekeeper permissions
+    $granted = $action->isAuthorized();
+    // For get actions, just run a get and ACLs will be applied to the query.
+    // It's a cheap trick and not as efficient as not running the query at all,
+    // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
+    if (is_a($action, '\Civi\Api4\Generic\DAOGetAction')) {
+      $granted = $granted && $action->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
+    }
+    else {
+      $baoName = self::getBAOFromApiName($entityName);
+      // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
+      if ($baoName && strpos($baoName, '_BAO_')) {
+        $baoName::checkAccess($actionName, $record, NULL, $granted);
+      }
+      // Otherwise, call the hook directly
+      else {
+        \CRM_Utils_Hook::checkAccess($entityName, $actionName, $record, NULL, $granted);
+      }
+    }
+    return $granted;
+  }
+
 }
diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php
index 40048d6e4bba..68fcf7c341e4 100644
--- a/tests/phpunit/api/v4/Action/BasicActionsTest.php
+++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php
@@ -260,7 +260,7 @@ public function testWildcardSelect() {
 
   public function testEmptyAndNullOperators() {
     $records = [
-      [],
+      ['id' => NULL],
       ['color' => '', 'weight' => 0],
       ['color' => 'yellow', 'weight' => 100000000000],
     ];

From 1d3cbc3c535f826daed52f9e0f976cf0a815b329 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Wed, 5 May 2021 15:30:03 -0400
Subject: [PATCH 14/27] Implement _checkAccess for Contact BAO and related
 entities (email, phone, etc.)

Implements the _checkAccess BAO callback for contacts and the related entities
listed in _civicrm_api3_check_edit_permissions.

Switch APIv4 to stop using _civicrm_api3_check_edit_permissions
now that the checks are implemented in the BAO.

Also fixes a couple permission check functions to respect $userID variable.
---
 CRM/Contact/AccessTrait.php                 | 43 +++++++++++++++++++++
 CRM/Contact/BAO/Contact.php                 | 28 ++++++++++++++
 CRM/Contact/BAO/Contact/Permission.php      | 43 ++++++++++-----------
 CRM/Core/BAO/Address.php                    |  1 +
 CRM/Core/BAO/Email.php                      |  1 +
 CRM/Core/BAO/IM.php                         |  1 +
 CRM/Core/BAO/OpenID.php                     |  1 +
 CRM/Core/BAO/Phone.php                      |  1 +
 CRM/Core/BAO/Website.php                    |  1 +
 Civi/Api4/Generic/DAODeleteAction.php       | 19 +++++----
 Civi/Api4/Generic/Traits/DAOActionTrait.php | 26 -------------
 Civi/Api4/Utils/CoreUtil.php                |  2 +-
 tests/phpunit/api/v3/ContactTest.php        |  7 +++-
 13 files changed, 114 insertions(+), 60 deletions(-)
 create mode 100644 CRM/Contact/AccessTrait.php

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
new file mode 100644
index 000000000000..d3a5e396618f
--- /dev/null
+++ b/CRM/Contact/AccessTrait.php
@@ -0,0 +1,43 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+/**
+ * Trait shared with entities attached to the contact record.
+ */
+trait CRM_Contact_AccessTrait {
+
+  /**
+   * @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) {
+    $cid = $record['contact_id'] ?? NULL;
+    if (!$cid && !empty($record['id'])) {
+      $cid = CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'contact_id');
+    }
+    if (!$cid) {
+      // With no contact id this must be part of an event locblock
+      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($action, ['id' => $cid], $userID);
+  }
+
+}
diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index fb35924582af..5275f5a9fd99 100644
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -3728,4 +3728,32 @@ public static function getEntityRefFilters() {
     ];
   }
 
+  /**
+   * @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 {
+    switch ($action) {
+      case 'create':
+        return CRM_Core_Permission::check('add contacts', $userID);
+
+      case 'get':
+        $actionType = CRM_Core_Permission::VIEW;
+        break;
+
+      case 'delete':
+        $actionType = CRM_Core_Permission::DELETE;
+        break;
+
+      default:
+        $actionType = CRM_Core_Permission::EDIT;
+        break;
+    }
+
+    return CRM_Contact_BAO_Contact_Permission::allow($record['id'], $actionType, $userID);
+  }
+
 }
diff --git a/CRM/Contact/BAO/Contact/Permission.php b/CRM/Contact/BAO/Contact/Permission.php
index 1aae3cea9a7d..5143831b6cc2 100644
--- a/CRM/Contact/BAO/Contact/Permission.php
+++ b/CRM/Contact/BAO/Contact/Permission.php
@@ -136,37 +136,39 @@ public static function allowList($contact_ids, $type = CRM_Core_Permission::VIEW
    * @param int $id
    *   Contact id.
    * @param int|string $type the type of operation (view|edit)
+   * @param int $userID
+   *   Contact id of user to check (defaults to current logged-in user)
    *
    * @return bool
    *   true if the user has permission, false otherwise
    */
-  public static function allow($id, $type = CRM_Core_Permission::VIEW) {
-    // get logged in user
-    $contactID = CRM_Core_Session::getLoggedInContactID();
+  public static function allow($id, $type = CRM_Core_Permission::VIEW, $userID = NULL) {
+    // Default to logged in user if not supplied
+    $userID = $userID ?? CRM_Core_Session::getLoggedInContactID();
 
     // first: check if contact is trying to view own contact
-    if ($contactID == $id && ($type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view my contact')
-     || $type == CRM_Core_Permission::EDIT && CRM_Core_Permission::check('edit my contact'))
+    if ($userID == $id && ($type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view my contact')
+     || $type == CRM_Core_Permission::EDIT && CRM_Core_Permission::check('edit my contact', $userID))
       ) {
       return TRUE;
     }
 
     // FIXME: push this somewhere below, to not give this permission so many rights
     $isDeleted = (bool) CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $id, 'is_deleted');
-    if (CRM_Core_Permission::check('access deleted contacts') && $isDeleted) {
+    if (CRM_Core_Permission::check('access deleted contacts', $userID) && $isDeleted) {
       return TRUE;
     }
 
     // short circuit for admin rights here so we avoid unneeeded queries
     // some duplication of code, but we skip 3-5 queries
-    if (CRM_Core_Permission::check('edit all contacts') ||
-      ($type == CRM_ACL_API::VIEW && CRM_Core_Permission::check('view all contacts'))
+    if (CRM_Core_Permission::check('edit all contacts', $userID) ||
+      ($type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view all contacts', $userID))
     ) {
       return TRUE;
     }
 
     // check permission based on relationship, CRM-2963
-    if (self::relationshipList([$id], $type)) {
+    if (self::relationshipList([$id], $type, $userID)) {
       return TRUE;
     }
 
@@ -175,7 +177,7 @@ public static function allow($id, $type = CRM_Core_Permission::VIEW) {
     $tables = [];
     $whereTables = [];
 
-    $permission = CRM_ACL_API::whereClause($type, $tables, $whereTables, NULL, FALSE, FALSE, TRUE);
+    $permission = CRM_ACL_API::whereClause($type, $tables, $whereTables, $userID, FALSE, FALSE, TRUE);
     $from = CRM_Contact_BAO_Query::fromClause($whereTables);
 
     $query = "
@@ -185,10 +187,7 @@ public static function allow($id, $type = CRM_Core_Permission::VIEW) {
   LIMIT 1
 ";
 
-    if (CRM_Core_DAO::singleValueQuery($query, [1 => [$id, 'Integer']])) {
-      return TRUE;
-    }
-    return FALSE;
+    return (bool) CRM_Core_DAO::singleValueQuery($query, [1 => [$id, 'Integer']]);
   }
 
   /**
@@ -362,18 +361,18 @@ public static function cacheSubquery() {
 
   /**
    * Filter a list of contact_ids by the ones that the
-   *  currently active user as a permissioned relationship with
+   * user as a permissioned relationship with
    *
    * @param array $contact_ids
    *   List of contact IDs to be filtered
-   *
    * @param int $type
    *   access type CRM_Core_Permission::VIEW or CRM_Core_Permission::EDIT
+   * @param int $userID
    *
    * @return array
    *   List of contact IDs that the user has permissions for
    */
-  public static function relationshipList($contact_ids, $type) {
+  public static function relationshipList($contact_ids, $type, $userID = NULL) {
     $result_set = [];
 
     // no processing empty lists (avoid SQL errors as well)
@@ -381,9 +380,9 @@ public static function relationshipList($contact_ids, $type) {
       return [];
     }
 
-    // get the currently logged in user
-    $contactID = CRM_Core_Session::getLoggedInContactID();
-    if (empty($contactID)) {
+    // Default to currently logged in user
+    $userID = $userID ?? CRM_Core_Session::getLoggedInContactID();
+    if (empty($userID)) {
       return [];
     }
 
@@ -418,7 +417,7 @@ public static function relationshipList($contact_ids, $type) {
 SELECT civicrm_relationship.{$contact_id_column} AS contact_id
   FROM civicrm_relationship
   {$LEFT_JOIN_DELETED}
- WHERE civicrm_relationship.{$user_id_column} = {$contactID}
+ WHERE civicrm_relationship.{$user_id_column} = {$userID}
    AND civicrm_relationship.{$contact_id_column} IN ({$contact_id_list})
    AND civicrm_relationship.is_active = 1
    AND civicrm_relationship.is_permission_{$direction['from']}_{$direction['to']} {$is_perm_condition}
@@ -444,7 +443,7 @@ public static function relationshipList($contact_ids, $type) {
   FROM civicrm_relationship first_degree_relationship
   LEFT JOIN civicrm_relationship second_degree_relationship ON first_degree_relationship.contact_id_{$first_direction['to']} = second_degree_relationship.contact_id_{$second_direction['from']}
   {$LEFT_JOIN_DELETED}
- WHERE first_degree_relationship.contact_id_{$first_direction['from']} = {$contactID}
+ WHERE first_degree_relationship.contact_id_{$first_direction['from']} = {$userID}
    AND second_degree_relationship.contact_id_{$second_direction['to']} IN ({$contact_id_list})
    AND first_degree_relationship.is_active = 1
    AND first_degree_relationship.is_permission_{$first_direction['from']}_{$first_direction['to']} {$is_perm_condition}
diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php
index 283709aba6bd..636294b42683 100644
--- a/CRM/Core/BAO/Address.php
+++ b/CRM/Core/BAO/Address.php
@@ -19,6 +19,7 @@
  * This is class to handle address related functions.
  */
 class CRM_Core_BAO_Address extends CRM_Core_DAO_Address {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Takes an associative array and creates a address.
diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php
index f09051a9b02a..1e947294a574 100644
--- a/CRM/Core/BAO/Email.php
+++ b/CRM/Core/BAO/Email.php
@@ -19,6 +19,7 @@
  * This class contains functions for email handling.
  */
 class CRM_Core_BAO_Email extends CRM_Core_DAO_Email {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Create email address.
diff --git a/CRM/Core/BAO/IM.php b/CRM/Core/BAO/IM.php
index bca4ce1365df..12fc6238d433 100644
--- a/CRM/Core/BAO/IM.php
+++ b/CRM/Core/BAO/IM.php
@@ -19,6 +19,7 @@
  * This class contain function for IM handling
  */
 class CRM_Core_BAO_IM extends CRM_Core_DAO_IM {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Create or update IM record.
diff --git a/CRM/Core/BAO/OpenID.php b/CRM/Core/BAO/OpenID.php
index f2000435737d..33a7c269fc3c 100644
--- a/CRM/Core/BAO/OpenID.php
+++ b/CRM/Core/BAO/OpenID.php
@@ -19,6 +19,7 @@
  * This class contains function for Open Id
  */
 class CRM_Core_BAO_OpenID extends CRM_Core_DAO_OpenID {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Create or update OpenID record.
diff --git a/CRM/Core/BAO/Phone.php b/CRM/Core/BAO/Phone.php
index dd73b9f9b739..c5a110e2eeba 100644
--- a/CRM/Core/BAO/Phone.php
+++ b/CRM/Core/BAO/Phone.php
@@ -19,6 +19,7 @@
  * Class contains functions for phone.
  */
 class CRM_Core_BAO_Phone extends CRM_Core_DAO_Phone {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Create phone object - note that the create function calls 'add' but
diff --git a/CRM/Core/BAO/Website.php b/CRM/Core/BAO/Website.php
index d5f386e8fdb8..d6b420a6bf52 100644
--- a/CRM/Core/BAO/Website.php
+++ b/CRM/Core/BAO/Website.php
@@ -19,6 +19,7 @@
  * This class contain function for Website handling.
  */
 class CRM_Core_BAO_Website extends CRM_Core_DAO_Website {
+  use CRM_Contact_AccessTrait;
 
   /**
    * Create or update Website record.
diff --git a/Civi/Api4/Generic/DAODeleteAction.php b/Civi/Api4/Generic/DAODeleteAction.php
index bf335c0f404c..de5ed1a07f0c 100644
--- a/Civi/Api4/Generic/DAODeleteAction.php
+++ b/Civi/Api4/Generic/DAODeleteAction.php
@@ -40,6 +40,15 @@ public function _run(Result $result) {
     }
 
     $items = $this->getBatchRecords();
+
+    if ($this->getCheckPermissions()) {
+      foreach ($items as $key => $item) {
+        if (!CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+          throw new UnauthorizedException("ACL check failed");
+        }
+        $items[$key]['check_permissions'] = TRUE;
+      }
+    }
     if ($items) {
       $result->exchangeArray($this->deleteObjects($items));
     }
@@ -54,16 +63,6 @@ protected function deleteObjects($items) {
     $ids = [];
     $baoName = $this->getBaoName();
 
-    if ($this->getCheckPermissions()) {
-      foreach (array_keys($items) as $key) {
-        if (!CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $items[$key])) {
-          throw new UnauthorizedException("ACL check failed");
-        }
-        $items[$key]['check_permissions'] = TRUE;
-        $this->checkContactPermissions($baoName, $items[$key]);
-      }
-    }
-
     if ($this->getEntityName() !== 'EntityTag' && method_exists($baoName, 'del')) {
       foreach ($items as $item) {
         $args = [$item['id']];
diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php
index 88d1a612cd4f..af611354480c 100644
--- a/Civi/Api4/Generic/Traits/DAOActionTrait.php
+++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php
@@ -136,10 +136,6 @@ protected function writeObjects(&$items) {
         $item['contact_id'] = $entityId;
       }
 
-      if ($this->getCheckPermissions()) {
-        $this->checkContactPermissions($baoName, $item);
-      }
-
       if ($this->getEntityName() === 'Address') {
         $createResult = $baoName::$method($item, $this->fixAddress);
       }
@@ -259,26 +255,4 @@ protected function getCustomFieldInfo(string $fieldExpr) {
     return isset($info[$fieldName]) ? ['suffix' => $suffix] + $info[$fieldName] : NULL;
   }
 
-  /**
-   * Check edit/delete permissions for contacts and related entities.
-   *
-   * @param string $baoName
-   * @param array $item
-   *
-   * @throws \Civi\API\Exception\UnauthorizedException
-   */
-  protected function checkContactPermissions($baoName, $item) {
-    if ($baoName === 'CRM_Contact_BAO_Contact' && !empty($item['id'])) {
-      $permission = $this->getActionName() === 'delete' ? \CRM_Core_Permission::DELETE : \CRM_Core_Permission::EDIT;
-      if (!\CRM_Contact_BAO_Contact_Permission::allow($item['id'], $permission)) {
-        throw new \Civi\API\Exception\UnauthorizedException('Permission denied to modify contact record');
-      }
-    }
-    else {
-      // Fixme: decouple from v3
-      require_once 'api/v3/utils.php';
-      _civicrm_api3_check_edit_permissions($baoName, $item);
-    }
-  }
-
 }
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 65a3d81ed9fd..e62bcfe007a9 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -178,7 +178,7 @@ public static function checkAccess(string $entityName, string $actionName, array
       $baoName = self::getBAOFromApiName($entityName);
       // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
       if ($baoName && strpos($baoName, '_BAO_')) {
-        $baoName::checkAccess($actionName, $record, NULL, $granted);
+        $granted = $baoName::checkAccess($actionName, $record, NULL, $granted);
       }
       // Otherwise, call the hook directly
       else {
diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php
index 4bce7f933383..1f381ca7908a 100644
--- a/tests/phpunit/api/v3/ContactTest.php
+++ b/tests/phpunit/api/v3/ContactTest.php
@@ -3211,7 +3211,12 @@ public function testContactUpdatePermissions(int $version): void {
 
     $config->userPermissionClass->permissions = ['access CiviCRM'];
     $result = $this->callAPIFailure('contact', 'update', $params);
-    $this->assertEquals('Permission denied to modify contact record', $result['error_message']);
+    if ($version == 3) {
+      $this->assertEquals('Permission denied to modify contact record', $result['error_message']);
+    }
+    else {
+      $this->assertEquals('ACL check failed', $result['error_message']);
+    }
 
     $config->userPermissionClass->permissions = [
       'access CiviCRM',

From 01c65aabe0fe7107d52db901dbf7ddd6faf451d4 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 6 May 2021 13:39:21 -0400
Subject: [PATCH 15/27] Implement checkAccess for custom entities

---
 CRM/Core/BAO/CustomValue.php | 28 ++++++++++++++++++++++++++++
 Civi/Api4/Utils/CoreUtil.php |  8 ++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index ebf1f75aa09a..e2e3034ca004 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -221,4 +221,32 @@ public function addSelectWhereClause() {
     return $clauses;
   }
 
+  /**
+   * Special checkAccess function for multi-record custom pseudo-entities
+   *
+   * @param string $action
+   * @param array $record
+   * @param null $userID
+   * @param bool $granted
+   * @param string $groupName
+   * @return bool
+   */
+  public static function checkAccess(string $action, array $record, $userID = NULL, $granted = TRUE, $groupName = NULL): bool {
+    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');
+    }
+    // Currently, multi-record custom data always extends Contacts
+    $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
+    CRM_Utils_Hook::checkAccess("Custom_$groupName", $action, $record, $userID, $granted);
+    return $granted;
+  }
+
 }
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index e62bcfe007a9..3f984656a18f 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -175,9 +175,13 @@ public static function checkAccess(string $entityName, string $actionName, array
       $granted = $granted && $action->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
     }
     else {
-      $baoName = self::getBAOFromApiName($entityName);
       // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
-      if ($baoName && strpos($baoName, '_BAO_')) {
+      $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, NULL, $granted, substr($entityName, 7));
+      }
+      elseif ($baoName) {
         $granted = $baoName::checkAccess($actionName, $record, NULL, $granted);
       }
       // Otherwise, call the hook directly

From 3d3a7160c1cb239e12baa708c536d0bc6d6aa89d Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 7 May 2021 20:20:43 -0400
Subject: [PATCH 16/27] Implement checkAccess for EntityTags and Notes

---
 CRM/Contact/AccessTrait.php                |  2 +-
 CRM/Core/BAO/EntityTag.php                 |  1 +
 CRM/Core/BAO/Note.php                      |  1 +
 CRM/Core/DynamicFKAccessTrait.php          | 46 ++++++++++++++++++++++
 tests/phpunit/api/v3/ACLPermissionTest.php |  3 --
 5 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 CRM/Core/DynamicFKAccessTrait.php

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
index d3a5e396618f..466198a3a79e 100644
--- a/CRM/Contact/AccessTrait.php
+++ b/CRM/Contact/AccessTrait.php
@@ -37,7 +37,7 @@ public static function _checkAccess(string $action, array $record, $userID) {
       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($action, ['id' => $cid], $userID);
+    return CRM_Contact_BAO_Contact::checkAccess(CRM_Core_Permission::EDIT, ['id' => $cid], $userID);
   }
 
 }
diff --git a/CRM/Core/BAO/EntityTag.php b/CRM/Core/BAO/EntityTag.php
index cbd60e2cd62e..35950c5cf16f 100644
--- a/CRM/Core/BAO/EntityTag.php
+++ b/CRM/Core/BAO/EntityTag.php
@@ -16,6 +16,7 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
+  use CRM_Core_DynamicFKAccessTrait;
 
   /**
    * Given a contact id, it returns an array of tag id's the contact belongs to.
diff --git a/CRM/Core/BAO/Note.php b/CRM/Core/BAO/Note.php
index d63d8dea2713..a08dbacf4faa 100644
--- a/CRM/Core/BAO/Note.php
+++ b/CRM/Core/BAO/Note.php
@@ -19,6 +19,7 @@
  * BAO object for crm_note table.
  */
 class CRM_Core_BAO_Note extends CRM_Core_DAO_Note {
+  use CRM_Core_DynamicFKAccessTrait;
 
   /**
    * Const the max number of notes we display at any given time.
diff --git a/CRM/Core/DynamicFKAccessTrait.php b/CRM/Core/DynamicFKAccessTrait.php
new file mode 100644
index 000000000000..1fe7baae4d9c
--- /dev/null
+++ b/CRM/Core/DynamicFKAccessTrait.php
@@ -0,0 +1,46 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+/**
+ * Trait for with entities with an entity_table + entity_id dynamic FK.
+ */
+trait CRM_Core_DynamicFKAccessTrait {
+
+  /**
+   * @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 {
+    $eid = $record['entity_id'] ?? NULL;
+    $table = $record['entity_table'] ?? NULL;
+    if (!$eid && !empty($record['id'])) {
+      $eid = CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'entity_id');
+    }
+    if ($eid && !$table && !empty($record['id'])) {
+      $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);
+    }
+    return TRUE;
+  }
+
+}
diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index 8641e253b250..35ef4147be83 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -228,9 +228,6 @@ public function testRelatedEntityPermissions($version) {
       ]);
       $this->assertGreaterThan(0, $results['count']);
     }
-    if ($version == 4) {
-      $this->markTestIncomplete('Skipping entity_id related perms in api4 for now.');
-    }
     $newTag = civicrm_api3('Tag', 'create', [
       'name' => 'Foo123',
     ]);

From 63ee6673cb8ff13666ffb5e3b50a36d6011eabfd Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 3 Jun 2021 22:59:48 -0700
Subject: [PATCH 17/27] ConformanceTest - Add coverage for checkAccess

---
 .../phpunit/api/v4/Entity/ConformanceTest.php | 110 ++++++++++++++++--
 .../api/v4/Traits/CheckAccessTrait.php        |  62 ++++++++++
 2 files changed, 162 insertions(+), 10 deletions(-)
 create mode 100644 tests/phpunit/api/v4/Traits/CheckAccessTrait.php

diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php
index 1ad972ab13db..ab112e9b1d0a 100644
--- a/tests/phpunit/api/v4/Entity/ConformanceTest.php
+++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php
@@ -19,16 +19,19 @@
 
 namespace api\v4\Entity;
 
+use Civi\API\Exception\UnauthorizedException;
 use Civi\Api4\Entity;
 use api\v4\UnitTestCase;
 use Civi\Api4\Event\ValidateValuesEvent;
 use Civi\Api4\Utils\CoreUtil;
+use Civi\Test\HookInterface;
 
 /**
  * @group headless
  */
-class ConformanceTest extends UnitTestCase {
+class ConformanceTest extends UnitTestCase implements HookInterface {
 
+  use \api\v4\Traits\CheckAccessTrait;
   use \api\v4\Traits\TableDropperTrait;
   use \api\v4\Traits\OptionCleanupTrait {
     setUp as setUpOptionCleanup;
@@ -58,6 +61,7 @@ public function setUp(): void {
     $this->loadDataSet('ConformanceTest');
     $this->creationParamProvider = \Civi::container()->get('test.param_provider');
     parent::setUp();
+    $this->resetCheckAccess();
   }
 
   /**
@@ -137,13 +141,16 @@ public function testConformance($entity): void {
     }
 
     $this->checkFields($entityClass, $entity);
-    $id = $this->checkCreation($entity, $entityClass);
+    $this->checkCreationDenied($entity, $entityClass);
+    $id = $this->checkCreationAllowed($entity, $entityClass);
     $this->checkGet($entityClass, $id, $entity);
+    $this->checkGetAllowed($entityClass, $id, $entity);
     $this->checkGetCount($entityClass, $id, $entity);
     $this->checkUpdateFailsFromCreate($entityClass, $id);
     $this->checkWrongParamType($entityClass);
     $this->checkDeleteWithNoId($entityClass);
-    $this->checkDeletion($entityClass, $id);
+    $this->checkDeletionDenied($entityClass, $id, $entity);
+    $this->checkDeletionAllowed($entityClass, $id, $entity);
     $this->checkPostDelete($entityClass, $id, $entity);
   }
 
@@ -205,25 +212,29 @@ protected function checkActions($entityClass): array {
    *
    * @return mixed
    */
-  protected function checkCreation($entity, $entityClass) {
+  protected function checkCreationAllowed($entity, $entityClass) {
     $hookLog = [];
     $onValidate = function(ValidateValuesEvent $e) use (&$hookLog) {
-      $hookLog[$e->entity][$e->action] = 1 + ($hookLog[$e->entity][$e->action] ?? 0);
+      $hookLog[$e->getEntityName()][$e->getActionName()] = 1 + ($hookLog[$e->getEntityName()][$e->getActionName()] ?? 0);
     };
     \Civi::dispatcher()->addListener('civi.api4.validate', $onValidate);
     \Civi::dispatcher()->addListener('civi.api4.validate::' . $entity, $onValidate);
 
+    $this->setCheckAccessGrants(["{$entity}::create" => TRUE]);
+    $this->assertEquals(0, $this->checkAccessCounts["{$entity}::create"]);
+
     $requiredParams = $this->creationParamProvider->getRequired($entity);
     $createResult = $entityClass::create()
       ->setValues($requiredParams)
-      ->setCheckPermissions(FALSE)
+      ->setCheckPermissions(TRUE)
       ->execute()
       ->first();
 
     $this->assertArrayHasKey('id', $createResult, "create missing ID");
     $id = $createResult['id'];
-
     $this->assertGreaterThanOrEqual(1, $id, "$entity ID not positive");
+    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    $this->resetCheckAccess();
 
     $this->assertEquals(2, $hookLog[$entity]['create']);
     \Civi::dispatcher()->removeListener('civi.api4.validate', $onValidate);
@@ -232,6 +243,33 @@ protected function checkCreation($entity, $entityClass) {
     return $id;
   }
 
+  /**
+   * @param string $entity
+   * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
+   *
+   * @return mixed
+   */
+  protected function checkCreationDenied($entity, $entityClass) {
+    $this->setCheckAccessGrants(["{$entity}::create" => FALSE]);
+    $this->assertEquals(0, $this->checkAccessCounts["{$entity}::create"]);
+
+    $requiredParams = $this->creationParamProvider->getRequired($entity);
+
+    try {
+      $entityClass::create()
+        ->setValues($requiredParams)
+        ->setCheckPermissions(TRUE)
+        ->execute()
+        ->first();
+      $this->fail("{$entityClass}::create() should throw an authorization failure.");
+    }
+    catch (UnauthorizedException $e) {
+      // OK, expected exception
+    }
+    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    $this->resetCheckAccess();
+  }
+
   /**
    * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
    * @param int $id
@@ -264,6 +302,26 @@ protected function checkGet($entityClass, $id, $entity) {
     $this->assertEquals(1, $getResult->count(), $errMsg);
   }
 
+  /**
+   * Use a permissioned request for `get()`, with access grnted
+   * via checkAccess event.
+   *
+   * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
+   * @param int $id
+   * @param string $entity
+   */
+  protected function checkGetAllowed($entityClass, $id, $entity) {
+    $this->setCheckAccessGrants(["{$entity}::get" => TRUE]);
+    $getResult = $entityClass::get()
+      ->addWhere('id', '=', $id)
+      ->execute();
+
+    $errMsg = sprintf('Failed to fetch a %s after creation', $entity);
+    $this->assertEquals($id, $getResult->first()['id'], $errMsg);
+    $this->assertEquals(1, $getResult->count(), $errMsg);
+    $this->resetCheckAccess();
+  }
+
   /**
    * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
    * @param int $id
@@ -280,7 +338,6 @@ protected function checkGetCount($entityClass, $id, $entity): void {
     $getResult = $entityClass::get(FALSE)
       ->selectRowCount()
       ->execute();
-    $errMsg = sprintf('%s getCount failed', $entity);
     $this->assertGreaterThanOrEqual(1, $getResult->count(), $errMsg);
   }
 
@@ -317,16 +374,49 @@ protected function checkWrongParamType($entityClass) {
   }
 
   /**
+   * Delete an entity - while having a targeted grant (hook_civirm_checkAccess).
+   *
    * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
    * @param int $id
+   * @param string $entity
    */
-  protected function checkDeletion($entityClass, $id) {
-    $deleteResult = $entityClass::delete(FALSE)
+  protected function checkDeletionAllowed($entityClass, $id, $entity) {
+    $this->setCheckAccessGrants(["{$entity}::delete" => TRUE]);
+    $this->assertEquals(0, $this->checkAccessCounts["{$entity}::delete"]);
+
+    $deleteResult = $entityClass::delete()
       ->addWhere('id', '=', $id)
       ->execute();
 
     // should get back an array of deleted id
     $this->assertEquals([['id' => $id]], (array) $deleteResult);
+    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::delete"]);
+    $this->resetCheckAccess();
+  }
+
+  /**
+   * Attempt to delete an entity while having explicitly denied permission (hook_civicrm_checkAccess).
+   *
+   * @param \Civi\Api4\Generic\AbstractEntity|string $entityClass
+   * @param int $id
+   * @param string $entity
+   */
+  protected function checkDeletionDenied($entityClass, $id, $entity) {
+    $this->setCheckAccessGrants(["{$entity}::delete" => FALSE]);
+    $this->assertEquals(0, $this->checkAccessCounts["{$entity}::delete"]);
+
+    try {
+      $entityClass::delete()
+        ->addWhere('id', '=', $id)
+        ->execute();
+      $this->fail("{$entity}::delete should throw an authorization failure.");
+    }
+    catch (UnauthorizedException $e) {
+      // OK
+    }
+
+    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::delete"]);
+    $this->resetCheckAccess();
   }
 
   /**
diff --git a/tests/phpunit/api/v4/Traits/CheckAccessTrait.php b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
new file mode 100644
index 000000000000..ddc7826b17f7
--- /dev/null
+++ b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
@@ -0,0 +1,62 @@
+<?php
+namespace api\v4\Traits;
+
+/**
+ * Define an implementation of `hook_civicrm_checkAccess` in which access-control decisions are
+ * based on a predefined list. For example:
+ *
+ *   $this->setCheckAccessGrants(['Contact::create' => TRUE]);
+ *   Contact::create()->setValues(...)->execute();
+ *
+ * Note: Any class which uses this should implement the `HookInterface` so that the hook is picked up.
+ */
+trait CheckAccessTrait {
+
+  /**
+   * Specify whether to grant access to an entity-action via hook_checkAccess.
+   *
+   * @var array
+   *   Array(string $entityAction => bool $grant).
+   *   TRUE=>Allow. FALSE=>Deny. Undefined=>No preference.
+   */
+  private $checkAccessGrants = [];
+
+  /**
+   * Number of times hook_checkAccess has fired.
+   * @var array
+   */
+  protected $checkAccessCounts = [];
+
+  /**
+   * @param string $entity
+   * @param string $action
+   * @param array $record
+   * @param int|null $contactID
+   * @param bool $granted
+   * @see \CRM_Utils_Hook::checkAccess()
+   */
+  public function hook_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+    $key = "{$entity}::{$action}";
+    if (isset($this->checkAccessGrants[$key])) {
+      $granted = $this->checkAccessGrants[$key];
+      $this->checkAccessCounts[$key]++;
+    }
+  }
+
+  /**
+   * @param array $list
+   *   Ex: ["Event::delete" => TRUE, "Contribution::delete" => FALSE]
+   */
+  protected function setCheckAccessGrants($list) {
+    $this->checkAccessGrants = $this->checkAccessCounts = [];
+    foreach ($list as $key => $grant) {
+      $this->checkAccessGrants[$key] = $grant;
+      $this->checkAccessCounts[$key] = 0;
+    }
+  }
+
+  protected function resetCheckAccess() {
+    $this->setCheckAccessGrants([]);
+  }
+
+}

From cb72c80e967c878519a03626f639840adecaa8c7 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Mon, 7 Jun 2021 04:40:44 -0700
Subject: [PATCH 18/27] ConformanceTest - Add support for read-only entities

---
 .../phpunit/api/v4/Entity/ConformanceTest.php  | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php
index ab112e9b1d0a..43d8e78da1e0 100644
--- a/tests/phpunit/api/v4/Entity/ConformanceTest.php
+++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php
@@ -31,6 +31,8 @@
  */
 class ConformanceTest extends UnitTestCase implements HookInterface {
 
+  const READ_ONLY_ENTITIES = '/^(FinancialItem)$/';
+
   use \api\v4\Traits\CheckAccessTrait;
   use \api\v4\Traits\TableDropperTrait;
   use \api\v4\Traits\OptionCleanupTrait {
@@ -142,7 +144,7 @@ public function testConformance($entity): void {
 
     $this->checkFields($entityClass, $entity);
     $this->checkCreationDenied($entity, $entityClass);
-    $id = $this->checkCreationAllowed($entity, $entityClass);
+    $id = $this->checkCreation($entity, $entityClass);
     $this->checkGet($entityClass, $id, $entity);
     $this->checkGetAllowed($entityClass, $id, $entity);
     $this->checkGetCount($entityClass, $id, $entity);
@@ -212,7 +214,9 @@ protected function checkActions($entityClass): array {
    *
    * @return mixed
    */
-  protected function checkCreationAllowed($entity, $entityClass) {
+  protected function checkCreation($entity, $entityClass) {
+    $isReadOnly = preg_match(static::READ_ONLY_ENTITIES, $entity);
+
     $hookLog = [];
     $onValidate = function(ValidateValuesEvent $e) use (&$hookLog) {
       $hookLog[$e->getEntityName()][$e->getActionName()] = 1 + ($hookLog[$e->getEntityName()][$e->getActionName()] ?? 0);
@@ -226,14 +230,16 @@ protected function checkCreationAllowed($entity, $entityClass) {
     $requiredParams = $this->creationParamProvider->getRequired($entity);
     $createResult = $entityClass::create()
       ->setValues($requiredParams)
-      ->setCheckPermissions(TRUE)
+      ->setCheckPermissions(!$isReadOnly)
       ->execute()
       ->first();
 
     $this->assertArrayHasKey('id', $createResult, "create missing ID");
     $id = $createResult['id'];
     $this->assertGreaterThanOrEqual(1, $id, "$entity ID not positive");
-    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    if (!$isReadOnly) {
+      $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    }
     $this->resetCheckAccess();
 
     $this->assertEquals(2, $hookLog[$entity]['create']);
@@ -266,7 +272,9 @@ protected function checkCreationDenied($entity, $entityClass) {
     catch (UnauthorizedException $e) {
       // OK, expected exception
     }
-    $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    if (!preg_match(static::READ_ONLY_ENTITIES, $entity)) {
+      $this->assertEquals(1, $this->checkAccessCounts["{$entity}::create"]);
+    }
     $this->resetCheckAccess();
   }
 

From 63a2492e2fbd35bfbe70c3a7e9b76fd52a12ff4f Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Sun, 6 Jun 2021 17:53:49 -0700
Subject: [PATCH 19/27] (REF) AuthorizeEvent - Extract AuthorizedTrait

The primary purpose of this is to provide a trait (`AuthorizedTrait`) to
describe the common semantics of of coarse-grained authorization check and
the upcoming fine-grained authorization check.

The extracted trait makes a few small changes:

* Change the default value from `FALSE` to `NULL`.  In grepping universe for
  consumers of `isAuthorized(0`, I could only find consumers that used
  bool-ish values.  So this should be the same for them.  However, for
  future cases, it will allow some distinction between NULL/FALSE.

* Use more type-hints. The type should be nullable-boolean.

* Mutators should be amenable to fluent style (e.g. `$event->authorize()->stopPropagation()`).
---
 Civi/API/Event/AuthorizeEvent.php  | 19 +-----------
 Civi/API/Event/AuthorizedTrait.php | 48 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 18 deletions(-)
 create mode 100644 Civi/API/Event/AuthorizedTrait.php

diff --git a/Civi/API/Event/AuthorizeEvent.php b/Civi/API/Event/AuthorizeEvent.php
index 9d146a9b40f6..164677a01b24 100644
--- a/Civi/API/Event/AuthorizeEvent.php
+++ b/Civi/API/Event/AuthorizeEvent.php
@@ -22,24 +22,7 @@
  * Event name: 'civi.api.authorize'
  */
 class AuthorizeEvent extends Event {
-  /**
-   * @var bool
-   */
-  private $authorized = FALSE;
 
-  /**
-   * Mark the request as authorized.
-   */
-  public function authorize() {
-    $this->authorized = TRUE;
-  }
-
-  /**
-   * @return bool
-   *   TRUE if the request has been authorized.
-   */
-  public function isAuthorized() {
-    return $this->authorized;
-  }
+  use AuthorizedTrait;
 
 }
diff --git a/Civi/API/Event/AuthorizedTrait.php b/Civi/API/Event/AuthorizedTrait.php
new file mode 100644
index 000000000000..af3367af1dd9
--- /dev/null
+++ b/Civi/API/Event/AuthorizedTrait.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace Civi\API\Event;
+
+/**
+ * Trait AuthorizedTrait
+ * @package Civi\API\Event
+ */
+trait AuthorizedTrait {
+
+  /**
+   * @var bool|null
+   *   - TRUE: The action is explicitly authorized.
+   *   - FALSE: The action is explicitly prohibited.
+   *   - NULL: The authorization status has not been determined.
+   */
+  private $authorized = NULL;
+
+  /**
+   * Mark the request as authorized.
+   *
+   * @return static
+   */
+  public function authorize() {
+    $this->authorized = TRUE;
+    return $this;
+  }
+
+  /**
+   * @return bool|null
+   *   TRUE if the request has been authorized.
+   */
+  public function isAuthorized(): ?bool {
+    return $this->authorized;
+  }
+
+  /**
+   * Change the authorization status.
+   *
+   * @param bool|null $authorized
+   * @return static
+   */
+  public function setAuthorized(?bool $authorized) {
+    $this->authorized = $authorized;
+    return $this;
+  }
+
+}

From 399eff1aa1af75058765ef6196287f506abe5a68 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Fri, 4 Jun 2021 16:42:29 -0700
Subject: [PATCH 20/27] CoreUtil::checkAccess() - Accept optional argument
 $userID

Technically, there is an inheritable contract-change here - modifying
`isAuthorized()` to accept the current user ID. However, I grepped
universe for references:

```
[bknix-min:~/bknix/build/universe] grep -ri isAuthorized $( find -name Civi )
```

And all references were internal to `civicrm-core.git`.  This makes some
sense, given the available alternative extension-points
(`Civi\Api4\$ENTITY::permissions()` and `civi.api.authorize`).
---
 Civi/Api4/Action/GetActions.php                      |  2 +-
 .../Event/Subscriber/PermissionCheckSubscriber.php   |  2 +-
 Civi/Api4/Generic/AbstractAction.php                 |  6 ++++--
 Civi/Api4/Generic/CheckAccessAction.php              |  2 +-
 Civi/Api4/Utils/CoreUtil.php                         | 12 +++++++-----
 .../OAuthContactToken/OnlyModifyOwnTokensTrait.php   |  7 +++----
 6 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/Civi/Api4/Action/GetActions.php b/Civi/Api4/Action/GetActions.php
index e0dfbb597b3f..1720ee701655 100644
--- a/Civi/Api4/Action/GetActions.php
+++ b/Civi/Api4/Action/GetActions.php
@@ -79,7 +79,7 @@ private function loadAction($actionName, $method = NULL) {
     try {
       if (!isset($this->_actions[$actionName]) && (!$this->_actionsToGet || in_array($actionName, $this->_actionsToGet))) {
         $action = \Civi\API\Request::create($this->getEntityName(), $actionName, ['version' => 4]);
-        if (is_object($action) && (!$this->checkPermissions || $action->isAuthorized())) {
+        if (is_object($action) && (!$this->checkPermissions || $action->isAuthorized(\CRM_Core_Session::singleton()->getLoggedInContactID()))) {
           $this->_actions[$actionName] = ['name' => $actionName];
           if ($this->_isFieldSelected('description', 'comment', 'see')) {
             $vars = ['entity' => $this->getEntityName(), 'action' => $actionName];
diff --git a/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php b/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php
index 70fecac90789..e71e6707e017 100644
--- a/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php
+++ b/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php
@@ -40,7 +40,7 @@ public function onApiAuthorize(\Civi\API\Event\AuthorizeEvent $event) {
     /* @var \Civi\Api4\Generic\AbstractAction $apiRequest */
     $apiRequest = $event->getApiRequest();
     if ($apiRequest['version'] == 4) {
-      if (!$apiRequest->getCheckPermissions() || $apiRequest->isAuthorized()) {
+      if (!$apiRequest->getCheckPermissions() || $apiRequest->isAuthorized(\CRM_Core_Session::singleton()->getLoggedInContactID())) {
         $event->authorize();
         $event->stopPropagation();
       }
diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php
index d73a78cd5c51..275bd690774a 100644
--- a/Civi/Api4/Generic/AbstractAction.php
+++ b/Civi/Api4/Generic/AbstractAction.php
@@ -389,11 +389,13 @@ public function offsetUnset($offset) {
    *
    * This function is called if checkPermissions is set to true.
    *
+   * @param int|null $userID
+   *   Contact ID of the user we are testing, or NULL for the default/active user.
    * @return bool
    */
-  public function isAuthorized() {
+  public function isAuthorized(?int $userID): bool {
     $permissions = $this->getPermissions();
-    return \CRM_Core_Permission::check($permissions);
+    return \CRM_Core_Permission::check($permissions, $userID);
   }
 
   /**
diff --git a/Civi/Api4/Generic/CheckAccessAction.php b/Civi/Api4/Generic/CheckAccessAction.php
index cc26a5e6e555..bdbc82156701 100644
--- a/Civi/Api4/Generic/CheckAccessAction.php
+++ b/Civi/Api4/Generic/CheckAccessAction.php
@@ -62,7 +62,7 @@ public function _run(Result $result) {
    *
    * @return bool
    */
-  public function isAuthorized() {
+  public function isAuthorized(?int $userID): bool {
     return TRUE;
   }
 
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 3f984656a18f..e0c6061bfa50 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -158,16 +158,18 @@ private static function isCustomEntity($customGroupName) {
    * @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 default/active user.
    * @return bool
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \Civi\API\Exception\NotImplementedException
    * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public static function checkAccess(string $entityName, string $actionName, array $record) {
+  public static function checkAccess(string $entityName, string $actionName, array $record, $userID = NULL) {
     $action = Request::create($entityName, $actionName, ['version' => 4]);
     // This checks gatekeeper permissions
-    $granted = $action->isAuthorized();
+    $granted = $action->isAuthorized($userID);
     // For get actions, just run a get and ACLs will be applied to the query.
     // It's a cheap trick and not as efficient as not running the query at all,
     // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
@@ -179,14 +181,14 @@ public static function checkAccess(string $entityName, string $actionName, array
       $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, NULL, $granted, substr($entityName, 7));
+        $granted = \CRM_Core_BAO_CustomValue::checkAccess($actionName, $record, $userID, $granted, substr($entityName, 7));
       }
       elseif ($baoName) {
-        $granted = $baoName::checkAccess($actionName, $record, NULL, $granted);
+        $granted = $baoName::checkAccess($actionName, $record, $userID, $granted);
       }
       // Otherwise, call the hook directly
       else {
-        \CRM_Utils_Hook::checkAccess($entityName, $actionName, $record, NULL, $granted);
+        \CRM_Utils_Hook::checkAccess($entityName, $actionName, $record, $userID, $granted);
       }
     }
     return $granted;
diff --git a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
index 77817ab4aa15..749b296d114f 100644
--- a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
+++ b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
@@ -4,14 +4,13 @@
 
 trait OnlyModifyOwnTokensTrait {
 
-  public function isAuthorized(): bool {
-    if (\CRM_Core_Permission::check(['manage all OAuth contact tokens'])) {
+  public function isAuthorized(?int $loggedInContactID): bool {
+    if (\CRM_Core_Permission::check(['manage all OAuth contact tokens'], $loggedInContactID)) {
       return TRUE;
     }
-    if (!\CRM_Core_Permission::check(['manage my OAuth contact tokens'])) {
+    if (!\CRM_Core_Permission::check(['manage my OAuth contact tokens'], $loggedInContactID)) {
       return FALSE;
     }
-    $loggedInContactID = \CRM_Core_Session::singleton()->getLoggedInContactID();
     foreach ($this->where as $clause) {
       [$field, $op, $val] = $clause;
       if ($field !== 'contact_id') {

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 21/27] (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 466198a3a79e..372a8d50dcf4 100644
--- a/CRM/Contact/AccessTrait.php
+++ b/CRM/Contact/AccessTrait.php
@@ -37,7 +37,7 @@ public static function _checkAccess(string $action, array $record, $userID) {
       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 e2e3034ca004..0ad75b12a466 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -237,15 +237,23 @@ public static function checkAccess(string $action, array $record, $userID = NULL
       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 1fe7baae4d9c..ce8dec38d44d 100644
--- a/CRM/Core/DynamicFKAccessTrait.php
+++ b/CRM/Core/DynamicFKAccessTrait.php
@@ -37,8 +37,8 @@ public static function _checkAccess(string $action, array $record, $userID): boo
       $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 e0c6061bfa50..2d723f05c59d 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -194,4 +194,23 @@ public static function checkAccess(string $entityName, string $actionName, array
     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 eeae8e5855b0..bf5335a6e24b 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)) {

From a5d0f31af7fea240ee802a6eb7b692428dc772e4 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Sun, 6 Jun 2021 23:05:40 -0700
Subject: [PATCH 22/27] (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
---
 CRM/Contact/AccessTrait.php       |  3 ++-
 CRM/Contact/BAO/Contact.php       |  3 ++-
 CRM/Core/BAO/CustomValue.php      |  9 ++++++---
 CRM/Core/DAO.php                  | 11 ++++++-----
 CRM/Core/DynamicFKAccessTrait.php |  3 ++-
 Civi/Api4/Utils/CoreUtil.php      |  8 ++------
 6 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
index 372a8d50dcf4..d5206e04b0a1 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 5275f5a9fd99..c76c28883660 100644
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -3729,13 +3729,14 @@ public static function getEntityRefFilters() {
   }
 
   /**
+   * @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 0ad75b12a466..02f7eb41043d 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -224,14 +224,17 @@ public function addSelectWhereClause() {
   /**
    * 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 d8dbad2760a2..25d88a379f99 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -3045,21 +3045,22 @@ public static function getSelectWhereClause($tableAlias = NULL) {
    *
    * 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 @@ public static function checkAccess(string $action, array $record, $userID = NULL
     $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 ce8dec38d44d..ebf83825539b 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 2d723f05c59d..adc2a1670d16 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -179,12 +179,8 @@ public static function checkAccess(string $entityName, string $actionName, array
     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 {

From 849354a5e1bcdd1812f3848095b1571ffd082676 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Sun, 6 Jun 2021 23:28:43 -0700
Subject: [PATCH 23/27] (REF) Change CoreUtil::checkAccess() to
 CoreUtil::checkAccessRecord()

This change invovles a few things:

1. Pass the `AbstractAction $apiRequest` instead of the tuple `string $entity, string $action`.

2. There are a couple cases where we don't actually want to re-use the current `$apiRequest`.
   Switch these using `checkAccessDelegated()`.

3. Always resolve the userID before calling `checkAccessRecord()`. `$userID===null` can mean
   two different things (ie "active user" vs "anonymous user").  By
   resolving this once before we do any work with `checkAccess()`, we ensure that it will
   consistently mean "anonymous user" (even if there are multiple rounds of delegation).

3. Change the name from `checkAccess()` to `checkAccessRecord`. There are a few flavors of
   `...checkAccess...`, and this makes it easier to differentiate when skimming.
---
 CRM/Core/DAO.php                           |  1 -
 Civi/Api4/Generic/AbstractCreateAction.php |  2 +-
 Civi/Api4/Generic/AbstractSaveAction.php   |  2 +-
 Civi/Api4/Generic/BasicBatchAction.php     |  2 +-
 Civi/Api4/Generic/BasicUpdateAction.php    |  2 +-
 Civi/Api4/Generic/CheckAccessAction.php    |  2 +-
 Civi/Api4/Generic/DAODeleteAction.php      |  2 +-
 Civi/Api4/Generic/DAOUpdateAction.php      |  4 +--
 Civi/Api4/Utils/CoreUtil.php               | 31 +++++++++++-----------
 9 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 25d88a379f99..89340462d5fe 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -3067,7 +3067,6 @@ public static function checkAccess(string $entityName, string $action, array $re
     ) {
       throw new CRM_Core_Exception('Function checkAccess must be called on a BAO class');
     }
-    $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($entityName, $action, $record, $userID);
diff --git a/Civi/Api4/Generic/AbstractCreateAction.php b/Civi/Api4/Generic/AbstractCreateAction.php
index b08cb3e39c2f..f6bfb2f3d300 100644
--- a/Civi/Api4/Generic/AbstractCreateAction.php
+++ b/Civi/Api4/Generic/AbstractCreateAction.php
@@ -70,7 +70,7 @@ protected function validateValues() {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
 
-    if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $this->getValues())) {
+    if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->getValues(), \CRM_Core_Session::getLoggedInContactID())) {
       throw new UnauthorizedException("ACL check failed");
     }
 
diff --git a/Civi/Api4/Generic/AbstractSaveAction.php b/Civi/Api4/Generic/AbstractSaveAction.php
index aefd3725edbb..a0a65a964a68 100644
--- a/Civi/Api4/Generic/AbstractSaveAction.php
+++ b/Civi/Api4/Generic/AbstractSaveAction.php
@@ -112,7 +112,7 @@ protected function validateValues() {
     if ($this->checkPermissions) {
       foreach ($this->records as $record) {
         $action = empty($record[$this->idField]) ? 'create' : 'update';
-        if (!CoreUtil::checkAccess($this->getEntityName(), $action, $record)) {
+        if (!CoreUtil::checkAccessDelegated($this->getEntityName(), $action, $record, \CRM_Core_Session::getLoggedInContactID())) {
           throw new UnauthorizedException("ACL check failed");
         }
       }
diff --git a/Civi/Api4/Generic/BasicBatchAction.php b/Civi/Api4/Generic/BasicBatchAction.php
index 1806f9165bef..aa0dd475413d 100644
--- a/Civi/Api4/Generic/BasicBatchAction.php
+++ b/Civi/Api4/Generic/BasicBatchAction.php
@@ -67,7 +67,7 @@ public function __construct($entityName, $actionName, $select = 'id', $doer = NU
    */
   public function _run(Result $result) {
     foreach ($this->getBatchRecords() as $item) {
-      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
         throw new UnauthorizedException("ACL check failed");
       }
       $result[] = $this->doTask($item);
diff --git a/Civi/Api4/Generic/BasicUpdateAction.php b/Civi/Api4/Generic/BasicUpdateAction.php
index f00dfe358f48..f192978636e1 100644
--- a/Civi/Api4/Generic/BasicUpdateAction.php
+++ b/Civi/Api4/Generic/BasicUpdateAction.php
@@ -63,7 +63,7 @@ public function _run(Result $result) {
     $this->validateValues();
     foreach ($this->getBatchRecords() as $item) {
       $record = $this->values + $item;
-      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $record)) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $record, \CRM_Core_Session::getLoggedInContactID())) {
         throw new UnauthorizedException("ACL check failed");
       }
       $result[] = $this->writeRecord($record);
diff --git a/Civi/Api4/Generic/CheckAccessAction.php b/Civi/Api4/Generic/CheckAccessAction.php
index bdbc82156701..85d3e92a0846 100644
--- a/Civi/Api4/Generic/CheckAccessAction.php
+++ b/Civi/Api4/Generic/CheckAccessAction.php
@@ -52,7 +52,7 @@ public function _run(Result $result) {
       $granted = TRUE;
     }
     else {
-      $granted = CoreUtil::checkAccess($this->getEntityName(), $this->action, $this->values);
+      $granted = CoreUtil::checkAccessDelegated($this->getEntityName(), $this->action, $this->values, \CRM_Core_Session::getLoggedInContactID());
     }
     $result->exchangeArray([['access' => $granted]]);
   }
diff --git a/Civi/Api4/Generic/DAODeleteAction.php b/Civi/Api4/Generic/DAODeleteAction.php
index de5ed1a07f0c..6101de9e807a 100644
--- a/Civi/Api4/Generic/DAODeleteAction.php
+++ b/Civi/Api4/Generic/DAODeleteAction.php
@@ -43,7 +43,7 @@ public function _run(Result $result) {
 
     if ($this->getCheckPermissions()) {
       foreach ($items as $key => $item) {
-        if (!CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+        if (!CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
           throw new UnauthorizedException("ACL check failed");
         }
         $items[$key]['check_permissions'] = TRUE;
diff --git a/Civi/Api4/Generic/DAOUpdateAction.php b/Civi/Api4/Generic/DAOUpdateAction.php
index b545fbc22540..a359f1889002 100644
--- a/Civi/Api4/Generic/DAOUpdateAction.php
+++ b/Civi/Api4/Generic/DAOUpdateAction.php
@@ -63,7 +63,7 @@ public function _run(Result $result) {
     // Update a single record by ID unless select requires more than id
     if ($this->getSelect() === ['id'] && count($this->where) === 1 && $this->where[0][0] === 'id' && $this->where[0][1] === '=' && !empty($this->where[0][2])) {
       $this->values['id'] = $this->where[0][2];
-      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $this->values)) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->values, \CRM_Core_Session::getLoggedInContactID())) {
         throw new UnauthorizedException("ACL check failed");
       }
       $items = [$this->values];
@@ -76,7 +76,7 @@ public function _run(Result $result) {
     $items = $this->getBatchRecords();
     foreach ($items as &$item) {
       $item = $this->values + $item;
-      if ($this->checkPermissions && !CoreUtil::checkAccess($this->getEntityName(), $this->getActionName(), $item)) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
         throw new UnauthorizedException("ACL check failed");
       }
     }
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index adc2a1670d16..902f5cbabcee 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -155,36 +155,33 @@ private static function isCustomEntity($customGroupName) {
   /**
    * Check if current user is authorized to perform specified action on a given entity.
    *
-   * @param string $entityName
-   * @param string $actionName
+   * @param \Civi\Api4\Generic\AbstractAction $apiRequest
    * @param array $record
    * @param int|null $userID
-   *   Contact ID of the user we are testing, or NULL for the default/active user.
+   *   Contact ID of the user we are testing, or NULL for the anonymous user.
    * @return bool
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \Civi\API\Exception\NotImplementedException
    * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public static function checkAccess(string $entityName, string $actionName, array $record, $userID = NULL) {
-    $action = Request::create($entityName, $actionName, ['version' => 4]);
-    // This checks gatekeeper permissions
-    $granted = $action->isAuthorized($userID);
+  public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiRequest, array $record, ?int $userID) {
+    $granted = TRUE;
     // For get actions, just run a get and ACLs will be applied to the query.
     // It's a cheap trick and not as efficient as not running the query at all,
     // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
-    if (is_a($action, '\Civi\Api4\Generic\DAOGetAction')) {
-      $granted = $granted && $action->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
+    if (is_a($apiRequest, '\Civi\Api4\Generic\DAOGetAction')) {
+      $granted = $granted && $apiRequest->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
     }
     else {
       // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
-      $baoName = self::getBAOFromApiName($entityName);
+      $baoName = self::getBAOFromApiName($apiRequest->getEntityName());
       if ($baoName) {
-        $granted = $baoName::checkAccess($entityName, $actionName, $record, $userID, $granted);
+        $granted = $baoName::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
       }
       // Otherwise, call the hook directly
       else {
-        \CRM_Utils_Hook::checkAccess($entityName, $actionName, $record, $userID, $granted);
+        \CRM_Utils_Hook::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
       }
     }
     return $granted;
@@ -204,9 +201,13 @@ public static function checkAccess(string $entityName, string $actionName, array
    * @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);
+  public static function checkAccessDelegated(string $entityName, string $actionName, array $record, ?int $userID) {
+    $apiRequest = Request::create($entityName, $actionName, ['version' => 4]);
+    // TODO: Should probably emit civi.api.authorize for checking guardian permission; but in APIv4 with std cfg, this is de-facto equivalent.
+    if (!$apiRequest->isAuthorized($userID)) {
+      return FALSE;
+    }
+    return static::checkAccessRecord($apiRequest, $record, $userID);
   }
 
 }

From e294cebf4a8cf872a022eaae82250b5231dbf27e Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Mon, 7 Jun 2021 03:12:10 -0700
Subject: [PATCH 24/27] (REF) Consolidate calls to `Hook::checkAccess()`.
 Define initial value `$granted=NULL`.

Regarding invocations:

* Before: There are three different ways `Hook::checkAccess()` may be invoked, e.g.
    * `CRM_Core_DAO::checkAccess()`, which sprinkles in a call to `static::_checkAccess()` before `Hook::checkAccess()`
    * `CRM_Core_BAO_CustomValue::checkAccess()`, which sprinkles in a call to `checkAccessDelegated()` after `Hook::checkAccess()`
    * `CoreUtil::checkAccessRecord()`, which delegates to one of the above (if appropriate) or else calls `Hook::checkAccess()`
    * `CoreUtil::checkAccessRecord()` is the most general entry-point
* After: There is one way to invoke `Hook::checkAccess()`, and it incorporates some qausi/unofficial listeners.
    * `CoreUtil::checkAccessRecord()` is still the most general entry-point.
    * `CoreUtil::checkAccessRecord()` fires `Hook::checkAccess()` unconditionally
    * `CoreUtil::checkAccessRecord()` calls `CRM_Core_DAO::checkAccess()` and/or `CRM_Core_BAO_CustomValue::_checkAccess()`,
      which are now quasi/unofficial listeners for the hook

Regarding initialization and passing of `$granted`:

* Before: The value of `$granted` defaults to `TRUE`. Listeners may flip between `TRUE`/`FALSE`. The value of `$granted` is passed to each listener.
* After: The value of `$granted` defaults to `NULL`. Listeners may flip to `TRUE`/`FALSE`. If it remains `NULL` until the end, then it's treated as `TRUE`.
  The value of `$granted` is not passed to each listener.
* Comment: IMHO, this is an overall simplification.  If you pass in `$granted`, then each listener has to decide
  whether/how to mix the inputted value with its own decision.  (Ex: Should it be `return $grantedInput &&
  $myGrantedDecision` or `return $grantedInput || $myGrantedDecision` or `return $myGrantedDecision`? That choice appears to be
  carefully informed by the context of what steps ran before.) In the updated protocol, each `_checkAccess()` a smaller scope.
---
 CRM/Core/BAO/CustomValue.php                  | 12 +++---------
 CRM/Core/DAO.php                              | 17 +++++++----------
 CRM/Utils/Hook.php                            |  5 +++--
 Civi/Api4/Utils/CoreUtil.php                  | 19 +++++++------------
 ext/financialacls/financialacls.php           |  4 ++--
 .../api/v4/Traits/CheckAccessTrait.php        |  2 +-
 6 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index 02f7eb41043d..2b42b91d7d59 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -230,10 +230,10 @@ public function addSelectWhereClause() {
    * @param array $record
    * @param int|null $userID
    *   Contact ID of the active user (whose access we must check). NULL for anonymous.
-   * @param bool $granted
    * @return bool
+   *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
    */
-  public static function checkAccess(string $entityName, string $action, array $record, $userID, $granted = TRUE): bool {
+  public static function _checkAccess(string $entityName, string $action, array $record, $userID): ?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.
@@ -251,13 +251,7 @@ public static function checkAccess(string $entityName, string $action, array $re
       $cid = CRM_Core_DAO::singleValueQuery("SELECT entity_id FROM `$tableName` WHERE id = " . (int) $record['id']);
     }
 
-    // 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;
+    return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contact', 'update', ['id' => $cid], $userID);
   }
 
 }
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 89340462d5fe..07f21edfe552 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -3055,25 +3055,22 @@ public static function getSelectWhereClause($tableAlias = NULL) {
    *   The record should provide an 'id' but may otherwise be incomplete; guard accordingly.
    * @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
+   *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
    */
-  public static function checkAccess(string $entityName, string $action, array $record, $userID, $granted = TRUE): bool {
+  public static function checkAccess(string $entityName, string $action, array $record, $userID): ?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)
     ) {
       throw new CRM_Core_Exception('Function checkAccess must be called on a BAO class');
     }
-    // Dispatch to protected function _checkAccess in this BAO
-    if ($granted && method_exists(static::class, '_checkAccess')) {
-      $granted = static::_checkAccess($entityName, $action, $record, $userID);
+    if (method_exists(static::class, '_checkAccess')) {
+      return static::_checkAccess($entityName, $action, $record, $userID);
+    }
+    else {
+      return TRUE;
     }
-    // Dispatch to hook
-    CRM_Utils_Hook::checkAccess($entityName, $action, $record, $userID, $granted);
-    return $granted;
   }
 
   /**
diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
index e2907bd9a6fd..f93c14e28226 100644
--- a/CRM/Utils/Hook.php
+++ b/CRM/Utils/Hook.php
@@ -2116,9 +2116,10 @@ public static function permission_check($permission, &$granted, $contactId) {
    *   The record should provide an 'id' but may otherwise be incomplete; guard accordingly.
    * @param int|null $contactID
    *   Contact ID of the active user (whose access we must check).
-   * @param bool $granted
+   * @param bool|null $granted
+   *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
    */
-  public static function checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+  public static function checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
     self::singleton()->invoke(['entity', 'action', 'record', 'contactID', 'granted'], $entity, $action, $record,
       $contactID, $granted, self::$_nullObject,
       'civicrm_checkAccess'
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 902f5cbabcee..e39e32e854ac 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -166,23 +166,18 @@ private static function isCustomEntity($customGroupName) {
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiRequest, array $record, ?int $userID) {
-    $granted = TRUE;
     // For get actions, just run a get and ACLs will be applied to the query.
     // It's a cheap trick and not as efficient as not running the query at all,
     // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
     if (is_a($apiRequest, '\Civi\Api4\Generic\DAOGetAction')) {
-      $granted = $granted && $apiRequest->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
+      return (bool) $apiRequest->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
     }
-    else {
-      // If entity has a BAO, run the BAO::checkAccess function, which will call the hook
-      $baoName = self::getBAOFromApiName($apiRequest->getEntityName());
-      if ($baoName) {
-        $granted = $baoName::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
-      }
-      // Otherwise, call the hook directly
-      else {
-        \CRM_Utils_Hook::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
-      }
+
+    $granted = NULL;
+    \CRM_Utils_Hook::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
+    $baoName = self::getBAOFromApiName($apiRequest->getEntityName());
+    if ($granted === NULL && $baoName) {
+      $granted = $baoName::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID);
     }
     return $granted;
   }
diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php
index 569346d72fcc..a53d36f7effd 100644
--- a/ext/financialacls/financialacls.php
+++ b/ext/financialacls/financialacls.php
@@ -291,11 +291,11 @@ function financialacls_civicrm_permission(&$permissions) {
  * @param string $action
  * @param array $record
  * @param int|null $contactID
- * @param bool $granted
+ * @param bool|null $granted
  *
  * @throws \CRM_Core_Exception
  */
-function financialacls_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+function financialacls_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
   if (!financialacls_is_acl_limiting_enabled()) {
     return;
   }
diff --git a/tests/phpunit/api/v4/Traits/CheckAccessTrait.php b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
index ddc7826b17f7..d91aaae64523 100644
--- a/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
+++ b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
@@ -35,7 +35,7 @@ trait CheckAccessTrait {
    * @param bool $granted
    * @see \CRM_Utils_Hook::checkAccess()
    */
-  public function hook_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, bool &$granted) {
+  public function hook_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
     $key = "{$entity}::{$action}";
     if (isset($this->checkAccessGrants[$key])) {
       $granted = $this->checkAccessGrants[$key];

From af4cccf728aaf85d323397fe43ee5c4ae2ec499f Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Fri, 4 Jun 2021 11:57:55 -0700
Subject: [PATCH 25/27] Convert hook_civicrm_checkAccess to
 civi.api4.authorizeRecord

---
 CRM/Core/DAO.php                              | 33 -------
 CRM/Utils/Hook.php                            | 26 ------
 Civi/Api4/Event/AuthorizeRecordEvent.php      | 91 +++++++++++++++++++
 Civi/Api4/Utils/CoreUtil.php                  | 20 ++--
 Civi/Core/Container.php                       |  1 +
 ext/financialacls/financialacls.php           | 28 +++---
 .../api/v4/Traits/CheckAccessTrait.php        | 19 ++--
 7 files changed, 132 insertions(+), 86 deletions(-)
 create mode 100644 Civi/Api4/Event/AuthorizeRecordEvent.php

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 07f21edfe552..c05658b10674 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -3040,39 +3040,6 @@ public static function getSelectWhereClause($tableAlias = NULL) {
     return $clauses;
   }
 
-  /**
-   * Check whether action can be performed on a given record.
-   *
-   * 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|null $userID
-   *   Contact ID of the active user (whose access we must check). NULL for anonymous.
-   * @return bool
-   *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
-   */
-  public static function checkAccess(string $entityName, string $action, array $record, $userID): ?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)
-    ) {
-      throw new CRM_Core_Exception('Function checkAccess must be called on a BAO class');
-    }
-    if (method_exists(static::class, '_checkAccess')) {
-      return static::_checkAccess($entityName, $action, $record, $userID);
-    }
-    else {
-      return TRUE;
-    }
-  }
-
   /**
    * ensure database name is 'safe', i.e. only contains word characters (includes underscores)
    * and dashes, and contains at least one [a-z] case insensitive.
diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
index f93c14e28226..82877f3feb08 100644
--- a/CRM/Utils/Hook.php
+++ b/CRM/Utils/Hook.php
@@ -2100,32 +2100,6 @@ public static function permission_check($permission, &$granted, $contactId) {
     );
   }
 
-  /**
-   * Check whether the given contact has access to perform the given action.
-   *
-   * If the contact cannot perform the action the
-   *
-   * @param string $entity
-   *   APIv4 entity name.
-   *   Ex: 'Contact', 'Email', 'Event'
-   * @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|null $contactID
-   *   Contact ID of the active user (whose access we must check).
-   * @param bool|null $granted
-   *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
-   */
-  public static function checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
-    self::singleton()->invoke(['entity', 'action', 'record', 'contactID', 'granted'], $entity, $action, $record,
-      $contactID, $granted, self::$_nullObject,
-      'civicrm_checkAccess'
-    );
-  }
-
   /**
    * Rotate the cryptographic key used in the database.
    *
diff --git a/Civi/Api4/Event/AuthorizeRecordEvent.php b/Civi/Api4/Event/AuthorizeRecordEvent.php
new file mode 100644
index 000000000000..7e7fc15b41d7
--- /dev/null
+++ b/Civi/Api4/Event/AuthorizeRecordEvent.php
@@ -0,0 +1,91 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+namespace Civi\Api4\Event;
+
+use Civi\API\Event\AuthorizedTrait;
+use Civi\API\Event\RequestTrait;
+use Civi\Core\Event\GenericHookEvent;
+
+/**
+ * Determine if the a user has access to a given record.
+ *
+ * Event name: 'civi.api4.authorizeRecord'
+ */
+class AuthorizeRecordEvent extends GenericHookEvent {
+
+  use RequestTrait;
+  use AuthorizedTrait;
+
+  /**
+   * All (known/loaded) values of individual record being accessed.
+   * The record should provide an 'id' but may otherwise be incomplete; guard accordingly.
+   *
+   * @var array
+   */
+  private $record;
+
+  /**
+   * Contact ID of the active/target user (whose access we must check).
+   * NULL for anonymous.
+   *
+   * @var int|null
+   */
+  private $userID;
+
+  /**
+   * CheckAccessEvent constructor.
+   *
+   * @param \Civi\Api4\Generic\AbstractAction $apiRequest
+   * @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|null $userID
+   *   Contact ID of the active/target user (whose access we must check).
+   *   NULL for anonymous.
+   */
+  public function __construct($apiRequest, array $record, ?int $userID) {
+    $this->setApiRequest($apiRequest);
+    $this->record = $record;
+    $this->userID = $userID;
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public function getHookValues() {
+    return [$this->getApiRequest(), $this->record, &$this->authorized];
+  }
+
+  /**
+   * @return array
+   */
+  public function getRecord(): array {
+    return $this->record;
+  }
+
+  /**
+   * @return int|null
+   *   Contact ID of the active/target user (whose access we must check).
+   *   NULL for anonymous.
+   */
+  public function getUserID(): ?int {
+    return $this->userID;
+  }
+
+}
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index e39e32e854ac..5ae51d7ac7ac 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -173,13 +173,21 @@ public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiR
       return (bool) $apiRequest->addSelect('id')->addWhere('id', '=', $record['id'])->execute()->count();
     }
 
-    $granted = NULL;
-    \CRM_Utils_Hook::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID, $granted);
-    $baoName = self::getBAOFromApiName($apiRequest->getEntityName());
-    if ($granted === NULL && $baoName) {
-      $granted = $baoName::checkAccess($apiRequest->getEntityName(), $apiRequest->getActionName(), $record, $userID);
+    $event = new \Civi\Api4\Event\AuthorizeRecordEvent($apiRequest, $record, $userID);
+    \Civi::dispatcher()->dispatch('civi.api4.authorizeRecord', $event);
+
+    // Note: $bao::_checkAccess() is a quasi-listener. TODO: Convert to straight-up listener.
+    if ($event->isAuthorized() === NULL) {
+      $baoName = self::getBAOFromApiName($apiRequest->getEntityName());
+      if ($baoName && method_exists($baoName, '_checkAccess')) {
+        $authorized = $baoName::_checkAccess($event->getEntityName(), $event->getActionName(), $event->getRecord(), $event->getUserID());
+        $event->setAuthorized($authorized);
+      }
+      else {
+        $event->setAuthorized(TRUE);
+      }
     }
-    return $granted;
+    return $event->isAuthorized();
   }
 
   /**
diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php
index 8e2e926d1bd8..0632c649c440 100644
--- a/Civi/Core/Container.php
+++ b/Civi/Core/Container.php
@@ -384,6 +384,7 @@ public function createEventDispatcher() {
     };
 
     $dispatcher->addListener('civi.api4.validate', $aliasMethodEvent('civi.api4.validate', 'getEntityName'), 100);
+    $dispatcher->addListener('civi.api4.authorizeRecord', $aliasMethodEvent('civi.api4.authorizeRecord', 'getEntityName'), 100);
 
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\InstallationCanary', 'check']);
     $dispatcher->addListener('civi.core.install', ['\Civi\Core\DatabaseInitializer', 'initialize']);
diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php
index a53d36f7effd..3451baeca8d5 100644
--- a/ext/financialacls/financialacls.php
+++ b/ext/financialacls/financialacls.php
@@ -14,6 +14,15 @@ function financialacls_civicrm_config(&$config) {
   _financialacls_civix_civicrm_config($config);
 }
 
+/**
+ * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
+ */
+function financialacls_civicrm_container($container) {
+  $dispatcherDefn = $container->getDefinition('dispatcher');
+  $container->addResource(new \Symfony\Component\Config\Resource\FileResource(__FILE__));
+  $dispatcherDefn->addMethodCall('addListener', ['civi.api4.authorizeRecord::Contribution', '_financialacls_civi_api4_authorizeContribution']);
+}
+
 /**
  * Implements hook_civicrm_xmlMenu().
  *
@@ -287,27 +296,24 @@ function financialacls_civicrm_permission(&$permissions) {
 }
 
 /**
- * @param string $entity
- * @param string $action
- * @param array $record
- * @param int|null $contactID
- * @param bool|null $granted
+ * Listener for 'civi.api4.authorizeRecord::Contribution'
  *
+ * @param \Civi\Api4\Event\AuthorizeRecordEvent $e
  * @throws \CRM_Core_Exception
  */
-function financialacls_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
+function _financialacls_civi_api4_authorizeContribution(\Civi\Api4\Event\AuthorizeRecordEvent $e) {
   if (!financialacls_is_acl_limiting_enabled()) {
     return;
   }
-  if ($action === 'delete' && $entity === 'Contribution') {
-    $contributionID = $record['id'];
+  if ($e->getActionName() === 'delete' && $e->getEntityName() === 'Contribution') {
+    $contributionID = $e->getRecord()['id'];
     // First check contribution financial type
     $financialType = CRM_Core_PseudoConstant::getName('CRM_Contribute_DAO_Contribution', 'financial_type_id', CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionID, 'financial_type_id'));
     // Now check permissioned line items & permissioned contribution
-    if (!CRM_Core_Permission::check('delete contributions of type ' . $financialType, $contactID) ||
-      !CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributionID, 'delete', FALSE, $contactID)
+    if (!CRM_Core_Permission::check('delete contributions of type ' . $financialType, $e->getUserID()) ||
+      !CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributionID, 'delete', FALSE, $e->getUserID())
     ) {
-      $granted = FALSE;
+      $e->setAuthorized(FALSE);
     }
   }
 }
diff --git a/tests/phpunit/api/v4/Traits/CheckAccessTrait.php b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
index d91aaae64523..709be2c9ea6c 100644
--- a/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
+++ b/tests/phpunit/api/v4/Traits/CheckAccessTrait.php
@@ -1,8 +1,10 @@
 <?php
 namespace api\v4\Traits;
 
+use Civi\Api4\Event\AuthorizeRecordEvent;
+
 /**
- * Define an implementation of `hook_civicrm_checkAccess` in which access-control decisions are
+ * Define an implementation of `civi.api4.authorizeRecord` in which access-control decisions are
  * based on a predefined list. For example:
  *
  *   $this->setCheckAccessGrants(['Contact::create' => TRUE]);
@@ -28,17 +30,14 @@ trait CheckAccessTrait {
   protected $checkAccessCounts = [];
 
   /**
-   * @param string $entity
-   * @param string $action
-   * @param array $record
-   * @param int|null $contactID
-   * @param bool $granted
-   * @see \CRM_Utils_Hook::checkAccess()
+   * Listen to 'civi.api4.authorizeRecord'. Override decisions with specified grants.
+   *
+   * @param \Civi\Api4\Event\AuthorizeRecordEvent $e
    */
-  public function hook_civicrm_checkAccess(string $entity, string $action, array $record, ?int $contactID, ?bool &$granted) {
-    $key = "{$entity}::{$action}";
+  public function on_civi_api4_authorizeRecord(AuthorizeRecordEvent $e): void {
+    $key = $e->getEntityName() . '::' . $e->getActionName();
     if (isset($this->checkAccessGrants[$key])) {
-      $granted = $this->checkAccessGrants[$key];
+      $e->setAuthorized($this->checkAccessGrants[$key]);
       $this->checkAccessCounts[$key]++;
     }
   }

From 70da392777d9c663ea79755b43cfec2e347b5f04 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 8 Jun 2021 19:46:17 -0700
Subject: [PATCH 26/27] Partially rollback changes to `$userID`. Merely lay
 groundwork for future update.

Context: AuthorizeEvent did not allow tracking userID.  AuthorizeRecordEvent
is spec'd to track userID.  This is a step toward supporting checks when the
target user is non-present (ie not the user in the browser/session).
However, this step is not *sufficient* - additional work is also needed to
support non-present users.

Original: AuthorizeEvent and AbstractAction::isAuthorized did not report
current userID.  However, the wiring for AuthorizeRecordEvent is spec'd
to allow userID.

Previous: Made a breaking change in the signature of
AuthorizeEvent/AbstractAction::isAuthorized() to report userID.  However,
even with the break, it's not clear if this is the best approach.

Revised:
* Both AuthorizeEvent and AuthorizeRecordEvent report `userID`. This allows consumers to start using
  this information -- laying the groundwork for future changes.
* If an existing event-consumer ignores the `userID`, it will still work as correctly as before. This is
  because we guarantee that the userID matches the session-user.
* The signature of `AbstractAction::isAuthorized()` matches its original. No BC break. However, the method
  is flagged `@internal` to warn about the prospect of future changes.
* In the future, after we do more legwork on to ensure that the overall
  system makes sense, we may flip this and start doing non-present users.
---
 CRM/Contact/AccessTrait.php                   |  4 +-
 CRM/Core/BAO/CustomValue.php                  |  6 +--
 CRM/Core/DynamicFKAccessTrait.php             |  4 +-
 Civi/API/Event/AuthorizeEvent.php             |  9 ++++
 Civi/API/Kernel.php                           |  2 +-
 Civi/Api4/Event/ActiveUserTrait.php           | 43 +++++++++++++++++++
 Civi/Api4/Event/AuthorizeRecordEvent.php      | 26 +++--------
 Civi/Api4/Generic/AbstractAction.php          |  7 ++-
 Civi/Api4/Generic/AbstractCreateAction.php    |  2 +-
 Civi/Api4/Generic/AbstractSaveAction.php      |  2 +-
 Civi/Api4/Generic/BasicBatchAction.php        |  2 +-
 Civi/Api4/Generic/BasicUpdateAction.php       |  2 +-
 Civi/Api4/Generic/CheckAccessAction.php       |  4 +-
 Civi/Api4/Generic/DAODeleteAction.php         |  2 +-
 Civi/Api4/Generic/DAOUpdateAction.php         |  4 +-
 Civi/Api4/Utils/CoreUtil.php                  | 14 +++---
 api/v3/Contribution.php                       |  2 +-
 .../OnlyModifyOwnTokensTrait.php              |  7 +--
 18 files changed, 89 insertions(+), 53 deletions(-)
 create mode 100644 Civi/Api4/Event/ActiveUserTrait.php

diff --git a/CRM/Contact/AccessTrait.php b/CRM/Contact/AccessTrait.php
index d5206e04b0a1..d7bfd66fb9fe 100644
--- a/CRM/Contact/AccessTrait.php
+++ b/CRM/Contact/AccessTrait.php
@@ -24,11 +24,11 @@ trait CRM_Contact_AccessTrait {
    * @param string $entityName
    * @param string $action
    * @param array $record
-   * @param int|NULL $userID
+   * @param int $userID
    * @return bool
    * @see CRM_Core_DAO::checkAccess
    */
-  public static function _checkAccess(string $entityName, string $action, array $record, $userID) {
+  public static function _checkAccess(string $entityName, string $action, array $record, int $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/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index 2b42b91d7d59..3f1760d799c0 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -228,12 +228,12 @@ public function addSelectWhereClause() {
    *   Ex: 'Contact' or 'Custom_Foobar'
    * @param string $action
    * @param array $record
-   * @param int|null $userID
-   *   Contact ID of the active user (whose access we must check). NULL for anonymous.
+   * @param int $userID
+   *   Contact ID of the active user (whose access we must check). 0 for anonymous.
    * @return bool
    *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
    */
-  public static function _checkAccess(string $entityName, string $action, array $record, $userID): ?bool {
+  public static function _checkAccess(string $entityName, string $action, array $record, int $userID): ?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.
diff --git a/CRM/Core/DynamicFKAccessTrait.php b/CRM/Core/DynamicFKAccessTrait.php
index ebf83825539b..041d53717d56 100644
--- a/CRM/Core/DynamicFKAccessTrait.php
+++ b/CRM/Core/DynamicFKAccessTrait.php
@@ -24,11 +24,11 @@ trait CRM_Core_DynamicFKAccessTrait {
    * @param string $entityName
    * @param string $action
    * @param array $record
-   * @param int|NULL $userID
+   * @param int $userID
    * @return bool
    * @see CRM_Core_DAO::checkAccess
    */
-  public static function _checkAccess(string $entityName, string $action, array $record, $userID): bool {
+  public static function _checkAccess(string $entityName, string $action, array $record, int $userID): bool {
     $eid = $record['entity_id'] ?? NULL;
     $table = $record['entity_table'] ?? NULL;
     if (!$eid && !empty($record['id'])) {
diff --git a/Civi/API/Event/AuthorizeEvent.php b/Civi/API/Event/AuthorizeEvent.php
index 164677a01b24..d50411fb65cf 100644
--- a/Civi/API/Event/AuthorizeEvent.php
+++ b/Civi/API/Event/AuthorizeEvent.php
@@ -11,8 +11,11 @@
 
 namespace Civi\API\Event;
 
+use Civi\Api4\Event\ActiveUserTrait;
+
 /**
  * Class AuthorizeEvent
+ *
  * @package Civi\API\Event
  *
  * Determine whether the API request is allowed for the current user.
@@ -24,5 +27,11 @@
 class AuthorizeEvent extends Event {
 
   use AuthorizedTrait;
+  use ActiveUserTrait;
+
+  public function __construct($apiProvider, $apiRequest, $apiKernel, int $userID) {
+    parent::__construct($apiProvider, $apiRequest, $apiKernel);
+    $this->setUser($userID);
+  }
 
 }
diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php
index f3cab8ed09c2..70395abd7681 100644
--- a/Civi/API/Kernel.php
+++ b/Civi/API/Kernel.php
@@ -218,7 +218,7 @@ public function resolve($apiRequest) {
    */
   public function authorize($apiProvider, $apiRequest) {
     /** @var \Civi\API\Event\AuthorizeEvent $event */
-    $event = $this->dispatcher->dispatch('civi.api.authorize', new AuthorizeEvent($apiProvider, $apiRequest, $this));
+    $event = $this->dispatcher->dispatch('civi.api.authorize', new AuthorizeEvent($apiProvider, $apiRequest, $this, \CRM_Core_Session::getLoggedInContactID() ?: 0));
     if (!$event->isAuthorized()) {
       throw new \Civi\API\Exception\UnauthorizedException("Authorization failed");
     }
diff --git a/Civi/Api4/Event/ActiveUserTrait.php b/Civi/Api4/Event/ActiveUserTrait.php
new file mode 100644
index 000000000000..3cb8c82352aa
--- /dev/null
+++ b/Civi/Api4/Event/ActiveUserTrait.php
@@ -0,0 +1,43 @@
+<?php
+
+namespace Civi\Api4\Event;
+
+trait ActiveUserTrait {
+
+  /**
+   * Contact ID of the active/target user (whose access we must check).
+   * 0 for anonymous.
+   *
+   * @var int
+   */
+  private $userID;
+
+  /**
+   * @param int|null $userID
+   *   Contact ID of the active/target user (whose access we must check).
+   *   0 for anonymous.
+   * @return $this
+   */
+  protected function setUser(int $userID) {
+    $loggedInContactID = \CRM_Core_Session::getLoggedInContactID() ?: 0;
+    if ($userID !== $loggedInContactID) {
+      throw new \RuntimeException("The API subsystem does not yet fully support variable user IDs.");
+      // Traditionally, the API events did not emit information about the current user; it was assumed
+      // that the user was the logged-in user. This may be expanded in the future to support some more edge-cases.
+      // For now, the semantics are unchanged - but we've begun reporting the active userID so that
+      // consumers can start adopting it.
+    }
+    $this->userID = $userID;
+    return $this;
+  }
+
+  /**
+   * @return int
+   *   Contact ID of the active/target user (whose access we must check).
+   *   0 for anonymous.
+   */
+  public function getUserID(): int {
+    return $this->userID;
+  }
+
+}
diff --git a/Civi/Api4/Event/AuthorizeRecordEvent.php b/Civi/Api4/Event/AuthorizeRecordEvent.php
index 7e7fc15b41d7..ba480de84226 100644
--- a/Civi/Api4/Event/AuthorizeRecordEvent.php
+++ b/Civi/Api4/Event/AuthorizeRecordEvent.php
@@ -31,6 +31,7 @@ class AuthorizeRecordEvent extends GenericHookEvent {
 
   use RequestTrait;
   use AuthorizedTrait;
+  use ActiveUserTrait;
 
   /**
    * All (known/loaded) values of individual record being accessed.
@@ -40,14 +41,6 @@ class AuthorizeRecordEvent extends GenericHookEvent {
    */
   private $record;
 
-  /**
-   * Contact ID of the active/target user (whose access we must check).
-   * NULL for anonymous.
-   *
-   * @var int|null
-   */
-  private $userID;
-
   /**
    * CheckAccessEvent constructor.
    *
@@ -55,14 +48,14 @@ class AuthorizeRecordEvent extends GenericHookEvent {
    * @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|null $userID
+   * @param int $userID
    *   Contact ID of the active/target user (whose access we must check).
-   *   NULL for anonymous.
+   *   0 for anonymous.
    */
-  public function __construct($apiRequest, array $record, ?int $userID) {
+  public function __construct($apiRequest, array $record, int $userID) {
     $this->setApiRequest($apiRequest);
     $this->record = $record;
-    $this->userID = $userID;
+    $this->setUser($userID);
   }
 
   /**
@@ -79,13 +72,4 @@ public function getRecord(): array {
     return $this->record;
   }
 
-  /**
-   * @return int|null
-   *   Contact ID of the active/target user (whose access we must check).
-   *   NULL for anonymous.
-   */
-  public function getUserID(): ?int {
-    return $this->userID;
-  }
-
 }
diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php
index 275bd690774a..a91e85853ffc 100644
--- a/Civi/Api4/Generic/AbstractAction.php
+++ b/Civi/Api4/Generic/AbstractAction.php
@@ -389,13 +389,12 @@ public function offsetUnset($offset) {
    *
    * This function is called if checkPermissions is set to true.
    *
-   * @param int|null $userID
-   *   Contact ID of the user we are testing, or NULL for the default/active user.
    * @return bool
+   * @internal Implement/override in civicrm-core.git only. Signature may evolve.
    */
-  public function isAuthorized(?int $userID): bool {
+  public function isAuthorized(): bool {
     $permissions = $this->getPermissions();
-    return \CRM_Core_Permission::check($permissions, $userID);
+    return \CRM_Core_Permission::check($permissions);
   }
 
   /**
diff --git a/Civi/Api4/Generic/AbstractCreateAction.php b/Civi/Api4/Generic/AbstractCreateAction.php
index f6bfb2f3d300..c8b75eb870e3 100644
--- a/Civi/Api4/Generic/AbstractCreateAction.php
+++ b/Civi/Api4/Generic/AbstractCreateAction.php
@@ -70,7 +70,7 @@ protected function validateValues() {
       throw new \API_Exception("Mandatory values missing from Api4 {$this->getEntityName()}::{$this->getActionName()}: " . implode(", ", $unmatched), "mandatory_missing", ["fields" => $unmatched]);
     }
 
-    if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->getValues(), \CRM_Core_Session::getLoggedInContactID())) {
+    if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->getValues(), \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
       throw new UnauthorizedException("ACL check failed");
     }
 
diff --git a/Civi/Api4/Generic/AbstractSaveAction.php b/Civi/Api4/Generic/AbstractSaveAction.php
index a0a65a964a68..578e1dfc7238 100644
--- a/Civi/Api4/Generic/AbstractSaveAction.php
+++ b/Civi/Api4/Generic/AbstractSaveAction.php
@@ -112,7 +112,7 @@ protected function validateValues() {
     if ($this->checkPermissions) {
       foreach ($this->records as $record) {
         $action = empty($record[$this->idField]) ? 'create' : 'update';
-        if (!CoreUtil::checkAccessDelegated($this->getEntityName(), $action, $record, \CRM_Core_Session::getLoggedInContactID())) {
+        if (!CoreUtil::checkAccessDelegated($this->getEntityName(), $action, $record, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
           throw new UnauthorizedException("ACL check failed");
         }
       }
diff --git a/Civi/Api4/Generic/BasicBatchAction.php b/Civi/Api4/Generic/BasicBatchAction.php
index aa0dd475413d..25b662e56bc9 100644
--- a/Civi/Api4/Generic/BasicBatchAction.php
+++ b/Civi/Api4/Generic/BasicBatchAction.php
@@ -67,7 +67,7 @@ public function __construct($entityName, $actionName, $select = 'id', $doer = NU
    */
   public function _run(Result $result) {
     foreach ($this->getBatchRecords() as $item) {
-      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
         throw new UnauthorizedException("ACL check failed");
       }
       $result[] = $this->doTask($item);
diff --git a/Civi/Api4/Generic/BasicUpdateAction.php b/Civi/Api4/Generic/BasicUpdateAction.php
index f192978636e1..1eaabfe28c3f 100644
--- a/Civi/Api4/Generic/BasicUpdateAction.php
+++ b/Civi/Api4/Generic/BasicUpdateAction.php
@@ -63,7 +63,7 @@ public function _run(Result $result) {
     $this->validateValues();
     foreach ($this->getBatchRecords() as $item) {
       $record = $this->values + $item;
-      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $record, \CRM_Core_Session::getLoggedInContactID())) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $record, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
         throw new UnauthorizedException("ACL check failed");
       }
       $result[] = $this->writeRecord($record);
diff --git a/Civi/Api4/Generic/CheckAccessAction.php b/Civi/Api4/Generic/CheckAccessAction.php
index 85d3e92a0846..c1f0e919d399 100644
--- a/Civi/Api4/Generic/CheckAccessAction.php
+++ b/Civi/Api4/Generic/CheckAccessAction.php
@@ -52,7 +52,7 @@ public function _run(Result $result) {
       $granted = TRUE;
     }
     else {
-      $granted = CoreUtil::checkAccessDelegated($this->getEntityName(), $this->action, $this->values, \CRM_Core_Session::getLoggedInContactID());
+      $granted = CoreUtil::checkAccessDelegated($this->getEntityName(), $this->action, $this->values, \CRM_Core_Session::getLoggedInContactID() ?: 0);
     }
     $result->exchangeArray([['access' => $granted]]);
   }
@@ -62,7 +62,7 @@ public function _run(Result $result) {
    *
    * @return bool
    */
-  public function isAuthorized(?int $userID): bool {
+  public function isAuthorized(): bool {
     return TRUE;
   }
 
diff --git a/Civi/Api4/Generic/DAODeleteAction.php b/Civi/Api4/Generic/DAODeleteAction.php
index 6101de9e807a..902d28838a2d 100644
--- a/Civi/Api4/Generic/DAODeleteAction.php
+++ b/Civi/Api4/Generic/DAODeleteAction.php
@@ -43,7 +43,7 @@ public function _run(Result $result) {
 
     if ($this->getCheckPermissions()) {
       foreach ($items as $key => $item) {
-        if (!CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
+        if (!CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
           throw new UnauthorizedException("ACL check failed");
         }
         $items[$key]['check_permissions'] = TRUE;
diff --git a/Civi/Api4/Generic/DAOUpdateAction.php b/Civi/Api4/Generic/DAOUpdateAction.php
index a359f1889002..33adb73a2d9c 100644
--- a/Civi/Api4/Generic/DAOUpdateAction.php
+++ b/Civi/Api4/Generic/DAOUpdateAction.php
@@ -63,7 +63,7 @@ public function _run(Result $result) {
     // Update a single record by ID unless select requires more than id
     if ($this->getSelect() === ['id'] && count($this->where) === 1 && $this->where[0][0] === 'id' && $this->where[0][1] === '=' && !empty($this->where[0][2])) {
       $this->values['id'] = $this->where[0][2];
-      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->values, \CRM_Core_Session::getLoggedInContactID())) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $this->values, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
         throw new UnauthorizedException("ACL check failed");
       }
       $items = [$this->values];
@@ -76,7 +76,7 @@ public function _run(Result $result) {
     $items = $this->getBatchRecords();
     foreach ($items as &$item) {
       $item = $this->values + $item;
-      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID())) {
+      if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
         throw new UnauthorizedException("ACL check failed");
       }
     }
diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php
index 5ae51d7ac7ac..8399142500b7 100644
--- a/Civi/Api4/Utils/CoreUtil.php
+++ b/Civi/Api4/Utils/CoreUtil.php
@@ -157,15 +157,15 @@ private static function isCustomEntity($customGroupName) {
    *
    * @param \Civi\Api4\Generic\AbstractAction $apiRequest
    * @param array $record
-   * @param int|null $userID
-   *   Contact ID of the user we are testing, or NULL for the anonymous user.
+   * @param int|string $userID
+   *   Contact ID of the user we are testing,. 0 for the anonymous user.
    * @return bool
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \Civi\API\Exception\NotImplementedException
    * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiRequest, array $record, ?int $userID) {
+  public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiRequest, array $record, int $userID) {
     // For get actions, just run a get and ACLs will be applied to the query.
     // It's a cheap trick and not as efficient as not running the query at all,
     // but BAO::checkAccess doesn't consistently check permissions for the "get" action.
@@ -197,17 +197,17 @@ public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiR
    * @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.
+   * @param int $userID
+   *   Contact ID of the user we are testing, or 0 for the anonymous user.
    *
    * @return bool
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
    */
-  public static function checkAccessDelegated(string $entityName, string $actionName, array $record, ?int $userID) {
+  public static function checkAccessDelegated(string $entityName, string $actionName, array $record, int $userID) {
     $apiRequest = Request::create($entityName, $actionName, ['version' => 4]);
     // TODO: Should probably emit civi.api.authorize for checking guardian permission; but in APIv4 with std cfg, this is de-facto equivalent.
-    if (!$apiRequest->isAuthorized($userID)) {
+    if (!$apiRequest->isAuthorized()) {
       return FALSE;
     }
     return static::checkAccessRecord($apiRequest, $record, $userID);
diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index bf5335a6e24b..80961ba5e313 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']) && !\Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contribution', 'delete', ['id' => $contributionID], CRM_Core_Session::getLoggedInContactID())) {
+  if (!empty($params['check_permissions']) && !\Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contribution', 'delete', ['id' => $contributionID], CRM_Core_Session::getLoggedInContactID() ?: 0)) {
     throw new API_Exception('You do not have permission to delete this contribution');
   }
   if (CRM_Contribute_BAO_Contribution::deleteContribution($contributionID)) {
diff --git a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
index 749b296d114f..77817ab4aa15 100644
--- a/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
+++ b/ext/oauth-client/Civi/Api4/Action/OAuthContactToken/OnlyModifyOwnTokensTrait.php
@@ -4,13 +4,14 @@
 
 trait OnlyModifyOwnTokensTrait {
 
-  public function isAuthorized(?int $loggedInContactID): bool {
-    if (\CRM_Core_Permission::check(['manage all OAuth contact tokens'], $loggedInContactID)) {
+  public function isAuthorized(): bool {
+    if (\CRM_Core_Permission::check(['manage all OAuth contact tokens'])) {
       return TRUE;
     }
-    if (!\CRM_Core_Permission::check(['manage my OAuth contact tokens'], $loggedInContactID)) {
+    if (!\CRM_Core_Permission::check(['manage my OAuth contact tokens'])) {
       return FALSE;
     }
+    $loggedInContactID = \CRM_Core_Session::singleton()->getLoggedInContactID();
     foreach ($this->where as $clause) {
       [$field, $op, $val] = $clause;
       if ($field !== 'contact_id') {

From b87406e2cd0706f292248bdd1374cc9a8e795692 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 9 Jun 2021 03:24:21 -0700
Subject: [PATCH 27/27] Expand CustomValue::_checkAccess()

Before: Reports that access is available based one delegated-check for `Contact.update`

After: Reports that access is available based on multiple checks:

  1. The user must have access to the relevant CustomGroup (by way of ACL or perms)
  2. The user must have acces to the underlying entity (by way of checkAccessDelgated)

Comments: I did a bit of testing with `Custom_*.get`, and it does seem to
give access to single-value CustomGroups. So I removed a comment about
multi-value CustomGroups and expanded to a larger list of entities.
---
 CRM/Core/BAO/CustomValue.php | 40 ++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php
index 3f1760d799c0..ca4d0ea388d6 100644
--- a/CRM/Core/BAO/CustomValue.php
+++ b/CRM/Core/BAO/CustomValue.php
@@ -234,24 +234,46 @@ public function addSelectWhereClause() {
    *   TRUE if granted. FALSE if prohibited. NULL if indeterminate.
    */
   public static function _checkAccess(string $entityName, string $action, array $record, int $userID): ?bool {
+    // This check implements two rules: you must have access to the specific custom-data-group - and to the underlying record (e.g. Contact).
+
     $groupName = substr($entityName, 0, 7) === 'Custom_' ? substr($entityName, 7) : NULL;
+    $extends = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $groupName, 'extends', 'name');
+    $id = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $groupName, 'id', 'name');
     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');
+      throw new CRM_Core_Exception('Missing required group-name 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}.");
+
+    if (empty($extends) || empty($id)) {
+      throw new CRM_Core_Exception('Received invalid group-name in CustomValue::checkAccess');
     }
 
-    $cid = $record['entity_id'] ?? NULL;
-    if (!$cid) {
+    $customGroups = [$id => $id];
+    $defaultGroups = CRM_Core_Permission::customGroupAdmin() ? [$id] : [];
+    // FIXME: Per current onscreen help (Admin=>ACLs=>Add ACLs), CustomGroup ACLs treat VIEW and EDIT as the same. Skimming code, it appears that existing checks use VIEW.
+    $accessList = CRM_ACL_API::group(CRM_Core_Permission::VIEW, $userID, 'civicrm_custom_group', $customGroups, $defaultGroups);
+    if (empty($accessList)) {
+      return FALSE;
+    }
+
+    $eid = $record['entity_id'] ?? NULL;
+    if (!$eid) {
       $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']);
+      $eid = CRM_Core_DAO::singleValueQuery("SELECT entity_id FROM `$tableName` WHERE id = " . (int) $record['id']);
     }
 
-    return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contact', 'update', ['id' => $cid], $userID);
+    // Do we have access to the target record?
+    if (in_array($extends, ['Contact', 'Individual', 'Organization', 'Household'])) {
+      return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated('Contact', 'update', ['id' => $eid], $userID);
+    }
+    elseif (\Civi\Api4\Utils\CoreUtil::getApiClass($extends)) {
+      // For most entities (Activity, Relationship, Contribution, ad nauseum), we acn just use an eponymous API.
+      return \Civi\Api4\Utils\CoreUtil::checkAccessDelegated($extends, 'update', ['id' => $eid], $userID);
+    }
+    else {
+      // Do you need to add a special case for some oddball custom-group type?
+      throw new CRM_Core_Exception("Cannot assess delegated permissions for group {$groupName}.");
+    }
   }
 
 }