Merge pull request #12576 from colemanw/userPerm

Fix CRM_ACL_API::whereClause to respect $contactId param

CRM/ACL/API.php

@@ -113,18 +113,18 @@ public static function whereClause(
       }
     }
 
-    // first see if the contact has edit / view all contacts
-    if (CRM_Core_Permission::check('edit all contacts') ||
-      ($type == self::VIEW && CRM_Core_Permission::check('view all contacts'))
-    ) {
-      return $deleteClause;
-    }
-
     if (!$contactID) {
       $contactID = CRM_Core_Session::getLoggedInContactID();
     }
     $contactID = (int) $contactID;
 
+    // first see if the contact has edit / view all permission
+    if (CRM_Core_Permission::check('edit all contacts', $contactID) ||
+      ($type == self::VIEW && CRM_Core_Permission::check('view all contacts', $contactID))
+    ) {
+      return $deleteClause;
+    }
+
     $where = implode(' AND ',
       array(
         CRM_ACL_BAO_ACL::whereClause($type,

CRM/Core/Permission.php

@@ -83,9 +83,11 @@ public static function getPermission() {
 
   /**
    * Given a permission string or array, check for access requirements
-   * @param mixed $permissions
+   * @param string|array $permissions
    *   The permission to check as an array or string -see examples.
-   *   arrays
+   *
+   * @param int $contactId
+   *   Contact id to check permissions for. Defaults to current logged-in user.
    *
    *  Ex 1
    *
@@ -117,15 +119,20 @@ public static function getPermission() {
    * @return bool
    *   true if yes, else false
    */
-  public static function check($permissions) {
+  public static function check($permissions, $contactId = NULL) {
     $permissions = (array) $permissions;
+    $userId = NULL;
+    if ($contactId) {
+      $userId = CRM_Core_BAO_UFMatch::getUFId($contactId);
+    }
 
+    /** @var CRM_Core_Permission_Temp $tempPerm */
     $tempPerm = CRM_Core_Config::singleton()->userPermissionTemp;
 
     foreach ($permissions as $permission) {
       if (is_array($permission)) {
         foreach ($permission as $orPerm) {
-          if (self::check($orPerm)) {
+          if (self::check($orPerm, $contactId)) {
             //one of our 'or' permissions has succeeded - stop checking this permission
             return TRUE;
           }
@@ -135,9 +142,9 @@ public static function check($permissions) {
       }
       else {
         // This is an individual permission
-        $granted = CRM_Core_Config::singleton()->userPermissionClass->check($permission);
+        $granted = CRM_Core_Config::singleton()->userPermissionClass->check($permission, $userId);
         // Call the permission_check hook to permit dynamic escalation (CRM-19256)
-        CRM_Utils_Hook::permission_check($permission, $granted);
+        CRM_Utils_Hook::permission_check($permission, $granted, $contactId);
         if (
           !$granted
           && !($tempPerm && $tempPerm->check($permission))

CRM/Core/Permission/Backdrop.php

@@ -68,12 +68,12 @@ class CRM_Core_Permission_Backdrop extends CRM_Core_Permission_DrupalBase {
    * @param string $str
    *   The permission to check.
    *
-   * @param int $contactID
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str, $contactID = NULL) {
+  public function check($str, $userId = NULL) {
     $str = $this->translatePermission($str, 'Drupal', array(
       'view user account' => 'access user profiles',
       'administer users' => 'administer users',
@@ -85,7 +85,11 @@ public function check($str, $contactID = NULL) {
       return TRUE;
     }
     if (function_exists('user_access')) {
-      return user_access($str) ? TRUE : FALSE;
+      $account = NULL;
+      if ($userId) {
+        $account = user_load($userId);
+      }
+      return user_access($str, $account);
     }
     return TRUE;
   }

CRM/Core/Permission/Base.php

@@ -157,9 +157,10 @@ public function groupClause($type, &$tables, &$whereTables) {
    *
    * @param string $str
    *   The permission to check.
+   * @param int $userId
    *
    */
-  public function check($str) {
+  public function check($str, $userId = NULL) {
     //no default behaviour
   }
 

CRM/Core/Permission/Drupal.php

@@ -68,12 +68,12 @@ class CRM_Core_Permission_Drupal extends CRM_Core_Permission_DrupalBase {
    * @param string $str
    *   The permission to check.
    *
-   * @param int $contactID
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str, $contactID = NULL) {
+  public function check($str, $userId = NULL) {
     $str = $this->translatePermission($str, 'Drupal', array(
       'view user account' => 'access user profiles',
       'administer users' => 'administer users',
@@ -85,7 +85,11 @@ public function check($str, $contactID = NULL) {
       return TRUE;
     }
     if (function_exists('user_access')) {
-      return user_access($str) ? TRUE : FALSE;
+      $account = NULL;
+      if ($userId) {
+        $account = user_load($userId);
+      }
+      return user_access($str, $account);
     }
     return TRUE;
   }

CRM/Core/Permission/Drupal6.php

@@ -67,12 +67,12 @@ class CRM_Core_Permission_Drupal6 extends CRM_Core_Permission_DrupalBase {
    * @param string $str
    *   The permission to check.
    *
-   * @param int $contactID
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str, $contactID = NULL) {
+  public function check($str, $userId = NULL) {
     $str = $this->translatePermission($str, 'Drupal6', array(
       'view user account' => 'access user profiles',
       'administer users' => 'administer users',
@@ -84,7 +84,11 @@ public function check($str, $contactID = NULL) {
       return TRUE;
     }
     if (function_exists('user_access')) {
-      return user_access($str) ? TRUE : FALSE;
+      $account = NULL;
+      if ($userId) {
+        $account = user_load($userId);
+      }
+      return user_access($str, $account);
     }
     return TRUE;
   }

CRM/Core/Permission/Drupal8.php

@@ -43,11 +43,11 @@ class CRM_Core_Permission_Drupal8 extends CRM_Core_Permission_DrupalBase {
    * @param string $str
    *   The permission to check.
    *
-   * @param null $contactID
+   * @param int $userId
    *
    * @return bool
    */
-  public function check($str, $contactID = NULL) {
+  public function check($str, $userId = NULL) {
     $str = $this->translatePermission($str, 'Drupal', array(
       'view user account' => 'access user profiles',
     ));
@@ -58,7 +58,8 @@ public function check($str, $contactID = NULL) {
     if ($str == CRM_Core_Permission::ALWAYS_ALLOW_PERMISSION) {
       return TRUE;
     }
-    return \Drupal::currentUser()->hasPermission($str);
+    $acct = $userId ? \Drupal\user\Entity\User::load($userId) : \Drupal::currentUser();
+    return $acct->hasPermission($str);
   }
 
   /**

CRM/Core/Permission/Joomla.php

@@ -42,12 +42,17 @@ class CRM_Core_Permission_Joomla extends CRM_Core_Permission_Base {
    *
    * @param string $str
    *   The permission to check.
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str) {
+  public function check($str, $userId = NULL) {
     $config = CRM_Core_Config::singleton();
+    // JFactory::getUser does strict type checking, so convert falesy values to NULL
+    if (!$userId) {
+      $userId = NULL;
+    }
 
     $translated = $this->translateJoomlaPermission($str);
     if ($translated === CRM_Core_Permission::ALWAYS_DENY_PERMISSION) {
@@ -61,7 +66,7 @@ public function check($str) {
     // we've not yet figured out how to bootstrap joomla, so we should
     // not execute hooks if joomla is not loaded
     if (defined('_JEXEC')) {
-      $user = JFactory::getUser();
+      $user = JFactory::getUser($userId);
       $api_key    = CRM_Utils_Request::retrieve('api_key', 'String', $store, FALSE, NULL, 'REQUEST');
 
       // If we are coming from REST we don't have a user but we do have the api_key for a user.

CRM/Core/Permission/Soap.php

@@ -43,11 +43,12 @@ class CRM_Core_Permission_Soap extends CRM_Core_Permission_Base {
    *
    * @param string $str
    *   The permission to check.
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str) {
+  public function check($str, $userId = NULL) {
     return TRUE;
   }
 

CRM/Core/Permission/UnitTests.php

@@ -46,11 +46,12 @@ class CRM_Core_Permission_UnitTests extends CRM_Core_Permission_Base {
    *
    * @param string $str
    *   The permission to check.
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str) {
+  public function check($str, $userId = NULL) {
     if ($str == CRM_Core_Permission::ALWAYS_DENY_PERMISSION) {
       return FALSE;
     }

CRM/Core/Permission/WordPress.php

@@ -42,11 +42,12 @@ class CRM_Core_Permission_WordPress extends CRM_Core_Permission_Base {
    *
    * @param string $str
    *   The permission to check.
+   * @param int $userId
    *
    * @return bool
    *   true if yes, else false
    */
-  public function check($str) {
+  public function check($str, $userId = NULL) {
     // Generic cms 'administer users' role tranlates to users with the 'edit_users' capability' in WordPress
     $str = $this->translatePermission($str, 'WordPress', array(
       'administer users' => 'edit_users',
@@ -74,16 +75,18 @@ public function check($str) {
       return TRUE;
     }
 
-    if (current_user_can('super admin') || current_user_can('administrator')) {
+    $user = $userId ? get_userdata($userId) : wp_get_current_user();
+
+    if ($user->has_cap('super admin') || $user->has_cap('administrator')) {
       return TRUE;
     }
 
     // Make string lowercase and convert spaces into underscore
     $str = CRM_Utils_String::munge(strtolower($str));
 
-    if (is_user_logged_in()) {
+    if ($user->exists()) {
       // Check whether the logged in user has the capabilitity
-      if (current_user_can($str)) {
+      if ($user->has_cap($str)) {
         return TRUE;
       }
     }

CRM/Utils/Hook.php

@@ -1820,13 +1820,15 @@ public static function permission(&$permissions) {
    *   The name of an atomic permission, ie. 'access deleted contacts'
    * @param bool $granted
    *   Whether this permission is currently granted. The hook can change this value.
+   * @param int $contactId
+   *   Contact whose permissions we are checking (if null, assume current user).
    *
    * @return null
    *   The return value is ignored
    */
-  public static function permission_check($permission, &$granted) {
-    return self::singleton()->invoke(array('permission', 'granted'), $permission, $granted,
-      self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject,
+  public static function permission_check($permission, &$granted, $contactId) {
+    return self::singleton()->invoke(array('permission', 'granted', 'contactId'), $permission, $granted, $contactId,
+      self::$_nullObject, self::$_nullObject, self::$_nullObject,
       'civicrm_permission_check'
     );
   }