(REF) Consolidate calls to `Hook::checkAccess()`. Define initial value `$granted...
authorTim Otten <totten@civicrm.org>
Mon, 7 Jun 2021 10:12:10 +0000 (03:12 -0700)
committerTim Otten <totten@civicrm.org>
Tue, 8 Jun 2021 04:10:03 +0000 (21:10 -0700)
commite294cebf4a8cf872a022eaae82250b5231dbf27e
tree2add4c88df83e5959c67aae5b60f3f0bcb108f59
parent849354a5e1bcdd1812f3848095b1571ffd082676
(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
CRM/Core/DAO.php
CRM/Utils/Hook.php
Civi/Api4/Utils/CoreUtil.php
ext/financialacls/financialacls.php
tests/phpunit/api/v4/Traits/CheckAccessTrait.php