From: Rich Lott / Artful Robot <code.commits@artfulrobot.uk>
Date: Tue, 12 Dec 2023 14:34:29 +0000 (+0000)
Subject: Fix issue 4853: implied permission not applied
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=4f18a4972e0853d589cd2461e79d935f5afbaff3;p=civicrm-core.git

Fix issue 4853: implied permission not applied
---

diff --git a/CRM/Core/Permission/Base.php b/CRM/Core/Permission/Base.php
index 2ecb7e553d..ad59662886 100644
--- a/CRM/Core/Permission/Base.php
+++ b/CRM/Core/Permission/Base.php
@@ -154,7 +154,7 @@ class CRM_Core_Permission_Base {
 
       $groups = CRM_Core_PseudoConstant::allGroup($groupType, $excludeHidden);
 
-      if ($this->check('edit all contacts')) {
+      if (CRM_Core_Permission::check('edit all contacts')) {
         // this is the most powerful permission, so we return
         // immediately rather than dilute it further
         $this->_editAdminUser = $this->_viewAdminUser = TRUE;
@@ -163,7 +163,7 @@ class CRM_Core_Permission_Base {
         Civi::$statics['CRM_ACL_API']['viewPermissionedGroups_' . $domainId . '_' . $userId][$groupKey] = $groups;
         return Civi::$statics['CRM_ACL_API']['viewPermissionedGroups_' . $domainId . '_' . $userId][$groupKey];
       }
-      elseif ($this->check('view all contacts')) {
+      elseif (CRM_Core_Permission::check('view all contacts')) {
         $this->_viewAdminUser = TRUE;
         $this->_viewPermission = TRUE;
         Civi::$statics['CRM_ACL_API']['viewPermissionedGroups_' . $domainId . '_' . $userId][$groupKey] = $groups;
diff --git a/CRM/Core/Permission/Standalone.php b/CRM/Core/Permission/Standalone.php
index a201f698aa..bcc62bb2d6 100644
--- a/CRM/Core/Permission/Standalone.php
+++ b/CRM/Core/Permission/Standalone.php
@@ -16,6 +16,11 @@
  */
 
 /**
+ * Permissions class for Standalone.
+ *
+ * Note that CRM_Core_Permission_Base is unrelated to CRM_Core_Permission
+ * This class, and the _Base class, is to do with CMS permissions, whereas
+ * the CRM_Core_Permission class deals with Civi-specific permissioning.
  *
  */
 class CRM_Core_Permission_Standalone extends CRM_Core_Permission_Base {
@@ -27,7 +32,13 @@ class CRM_Core_Permission_Standalone extends CRM_Core_Permission_Base {
   public $permissions = NULL;
 
   /**
-   * Given a permission string, check for access requirements
+   * Given a permission string, check for access requirements.
+   *
+   * Note this differs from CRM_Core_Permission::check() which handles
+   * composite permissions (ORs etc) and Contacts.
+   *
+   * Some codepaths assume to be able to check a permission through this class;
+   * others through CRM_Core_Permission::check().
    *
    * @param string $str
    *   The permission to check.
@@ -37,19 +48,7 @@ class CRM_Core_Permission_Standalone extends CRM_Core_Permission_Base {
    *   true if yes, else false
    */
   public function check($str, $userId = NULL) {
-    if ($str == CRM_Core_Permission::ALWAYS_DENY_PERMISSION) {
-      return FALSE;
-    }
-    if ($str == CRM_Core_Permission::ALWAYS_ALLOW_PERMISSION) {
-      return TRUE;
-    }
-
-    if (class_exists(\Civi\Standalone\Security::class)) {
-      return \Civi\Standalone\Security::singleton()->checkPermission($this, $str, $userId);
-    }
-
-    // return the stubbed permission (defaulting to true if the array is missing)
-    return isset($this->permissions) && is_array($this->permissions) ? in_array($str, $this->permissions) : TRUE;
+    return \Civi\Standalone\Security::singleton()->checkPermission($this, $str, $userId);
   }
 
 }
diff --git a/ext/standaloneusers/Civi/Standalone/Security.php b/ext/standaloneusers/Civi/Standalone/Security.php
index 7b5313a0c7..f9fca9b68b 100644
--- a/ext/standaloneusers/Civi/Standalone/Security.php
+++ b/ext/standaloneusers/Civi/Standalone/Security.php
@@ -34,15 +34,21 @@ class Security {
    * @param string $permissionName
    *   The permission to check.
    *
-   * @param int $userID
-   *   It is unclear if this typehint is true: The Drupal version has a default NULL!
+   * @param ?int $userID
+   *   The User ID (not ContactID) to check. If NULL, current logged in user.
    *
    * @return bool
    *   true if yes, else false
    */
-  public function checkPermission(\CRM_Core_Permission_Standalone $permissionObject, string $permissionName, $userID) {
+  public function checkPermission(\CRM_Core_Permission_Standalone $permissionObject, string $permissionName, ?int $userID = NULL) {
+    if ($permissionName == \CRM_Core_Permission::ALWAYS_DENY_PERMISSION) {
+      return FALSE;
+    }
+    if ($permissionName == \CRM_Core_Permission::ALWAYS_ALLOW_PERMISSION) {
+      return TRUE;
+    }
 
-    // I think null means the current logged-in user
+    // NULL means the current logged-in user
     $userID = $userID ?? $this->getLoggedInUfID() ?? 0;
 
     if (!isset(\Civi::$statics[__METHOD__][$userID])) {
@@ -67,6 +73,8 @@ class Security {
       else {
         $permissionsPerRoleApiCall->addWhere('name', '=', 'everyone');
       }
+
+      // Get and cache an array of permission names for this user.
       $permissions = array_unique(array_merge(...$permissionsPerRoleApiCall->execute()->column('permissions')));
       \Civi::$statics[__METHOD__][$userID] = $permissions;
     }