From 399eff1aa1af75058765ef6196287f506abe5a68 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Fri, 4 Jun 2021 16:42:29 -0700 Subject: [PATCH] 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 e0dfbb597b..1720ee7016 100644 --- a/Civi/Api4/Action/GetActions.php +++ b/Civi/Api4/Action/GetActions.php @@ -79,7 +79,7 @@ class GetActions extends BasicGetAction { 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 70fecac907..e71e6707e0 100644 --- a/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php +++ b/Civi/Api4/Event/Subscriber/PermissionCheckSubscriber.php @@ -40,7 +40,7 @@ class PermissionCheckSubscriber implements EventSubscriberInterface { /* @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 d73a78cd5c..275bd69077 100644 --- a/Civi/Api4/Generic/AbstractAction.php +++ b/Civi/Api4/Generic/AbstractAction.php @@ -389,11 +389,13 @@ abstract class AbstractAction implements \ArrayAccess { * * 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 cc26a5e6e5..bdbc821567 100644 --- a/Civi/Api4/Generic/CheckAccessAction.php +++ b/Civi/Api4/Generic/CheckAccessAction.php @@ -62,7 +62,7 @@ class CheckAccessAction extends AbstractAction { * * @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 3f984656a1..e0c6061bfa 100644 --- a/Civi/Api4/Utils/CoreUtil.php +++ b/Civi/Api4/Utils/CoreUtil.php @@ -158,16 +158,18 @@ class CoreUtil { * @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 @@ class CoreUtil { $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 77817ab4aa..749b296d11 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 @@ namespace Civi\Api4\Action\OAuthContactToken; 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') { -- 2.25.1