From faa79dbf28e2132ddd5911817dcd564a54dec0c6 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 28 Apr 2021 20:49:06 -0400 Subject: [PATCH] SearchKit - Pass-thu permission checks from SearchDisplay::run to underlying API The SearchDisplay::run api action essentially wraps api.get for the search entity, so it makes sense to allow open access to the run api but require permission checks on the delegated api call. --- .../Civi/Api4/Action/SearchDisplay/Run.php | 1 + ext/search/Civi/Api4/SearchDisplay.php | 6 + .../api/v4/SearchDisplay/SearchRunTest.php | 112 ++++++++++++++++++ .../phpunit/CRMTraits/ACL/PermissionTrait.php | 15 +++ 4 files changed, 134 insertions(+) diff --git a/ext/search/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search/Civi/Api4/Action/SearchDisplay/Run.php index 22725aee4d..57567b34e6 100644 --- a/ext/search/Civi/Api4/Action/SearchDisplay/Run.php +++ b/ext/search/Civi/Api4/Action/SearchDisplay/Run.php @@ -90,6 +90,7 @@ class Run extends \Civi\Api4\Generic\AbstractAction { } $entityName = $this->savedSearch['api_entity']; $apiParams =& $this->savedSearch['api_params']; + $apiParams['checkPermissions'] = $this->checkPermissions; $apiParams += ['where' => []]; $settings = $this->display['settings']; $page = NULL; diff --git a/ext/search/Civi/Api4/SearchDisplay.php b/ext/search/Civi/Api4/SearchDisplay.php index 85e65602f1..939e74664e 100644 --- a/ext/search/Civi/Api4/SearchDisplay.php +++ b/ext/search/Civi/Api4/SearchDisplay.php @@ -29,4 +29,10 @@ class SearchDisplay extends Generic\DAOEntity { ->setCheckPermissions($checkPermissions); } + public static function permissions() { + $permissions = parent::permissions(); + $permissions['run'] = []; + return $permissions; + } + } diff --git a/ext/search/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php index 6ffb5790a5..02506e6a8f 100644 --- a/ext/search/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php +++ b/ext/search/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php @@ -2,13 +2,20 @@ namespace api\v4\SearchDisplay; use Civi\Api4\Contact; +use Civi\Api4\SavedSearch; +use Civi\Api4\SearchDisplay; +use Civi\Api4\UFMatch; use Civi\Test\HeadlessInterface; use Civi\Test\TransactionalInterface; +// FIXME: This shouldn't be needed but the core classLoader doesn't seem present when this file loads +require_once 'tests/phpunit/CRMTraits/ACL/PermissionTrait.php'; + /** * @group headless */ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInterface, TransactionalInterface { + use \CRMTraits_ACL_PermissionTrait; public function setUpHeadless() { // Civi\Test has many helpers, like install(), uninstall(), sql(), and sqlFile(). @@ -102,4 +109,109 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->assertEquals('Two', $result[1]['first_name']); } + /** + * Test running a searchDisplay as a restricted user. + */ + public function testDisplayACLCheck() { + $lastName = uniqid(__FUNCTION__); + $sampleData = [ + ['first_name' => 'User', 'last_name' => uniqid('user')], + ['first_name' => 'One', 'last_name' => $lastName], + ['first_name' => 'Two', 'last_name' => $lastName], + ['first_name' => 'Three', 'last_name' => $lastName], + ['first_name' => 'Four', 'last_name' => $lastName], + ]; + $sampleData = Contact::save(FALSE) + ->setRecords($sampleData)->execute() + ->indexBy('first_name')->column('id'); + + // Create logged-in user + UFMatch::delete(FALSE) + ->addWhere('uf_id', '=', 6) + ->execute(); + UFMatch::create(FALSE)->setValues([ + 'contact_id' => $sampleData['User'], + 'uf_name' => 'superman', + 'uf_id' => 6, + ])->execute(); + + $session = \CRM_Core_Session::singleton(); + $session->set('userID', $sampleData['User']); + $hooks = \CRM_Utils_Hook::singleton(); + \CRM_Core_Config::singleton()->userPermissionClass->permissions = [ + 'access CiviCRM', + ]; + + $search = SavedSearch::create(FALSE) + ->setValues([ + 'name' => uniqid(__FUNCTION__), + 'api_entity' => 'Contact', + 'api_params' => [ + 'version' => 4, + 'select' => ['id', 'first_name', 'last_name'], + 'where' => [['last_name', '=', $lastName]], + ], + ]) + ->addChain('display', SearchDisplay::create() + ->setValues([ + 'type' => 'table', + 'label' => uniqid(__FUNCTION__), + 'saved_search_id' => '$id', + 'settings' => [ + 'limit' => 20, + 'pager' => TRUE, + 'columns' => [ + [ + 'key' => 'id', + 'label' => 'Contact ID', + 'dataType' => 'Integer', + 'type' => 'field', + ], + [ + 'key' => 'first_name', + 'label' => 'First Name', + 'dataType' => 'String', + 'type' => 'field', + ], + [ + 'key' => 'last_name', + 'label' => 'Last Name', + 'dataType' => 'String', + 'type' => 'field', + ], + ], + 'sort' => [ + ['id', 'ASC'], + ], + ], + ]), 0) + ->execute()->first(); + + $params = [ + 'return' => 'page:1', + 'savedSearch' => $search['name'], + 'display' => $search['display']['name'], + 'afform' => NULL, + ]; + + $hooks->setHook('civicrm_aclWhereClause', [$this, 'aclWhereHookNoResults']); + $result = civicrm_api4('SearchDisplay', 'run', $params); + $this->assertCount(0, $result); + + $this->allowedContactId = $sampleData['Two']; + $hooks->setHook('civicrm_aclWhereClause', [$this, 'aclWhereOnlyOne']); + $this->cleanupCachedPermissions(); + $result = civicrm_api4('SearchDisplay', 'run', $params); + $this->assertCount(1, $result); + $this->assertEquals($sampleData['Two'], $result[0]['id']); + + $hooks->setHook('civicrm_aclWhereClause', [$this, 'aclWhereGreaterThan']); + $this->cleanupCachedPermissions(); + $result = civicrm_api4('SearchDisplay', 'run', $params); + $this->assertCount(2, $result); + $this->assertEquals($sampleData['Three'], $result[0]['id']); + $this->assertEquals($sampleData['Four'], $result[1]['id']); + + } + } diff --git a/tests/phpunit/CRMTraits/ACL/PermissionTrait.php b/tests/phpunit/CRMTraits/ACL/PermissionTrait.php index dc0c2882e0..7622d4c844 100644 --- a/tests/phpunit/CRMTraits/ACL/PermissionTrait.php +++ b/tests/phpunit/CRMTraits/ACL/PermissionTrait.php @@ -94,6 +94,21 @@ trait CRMTraits_ACL_PermissionTrait { $where = " contact_a.id = " . $this->allowedContactId; } + /** + * Results after the allowedContact are returned. + * + * @implements CRM_Utils_Hook::aclWhereClause + * + * @param string $type + * @param array $tables + * @param array $whereTables + * @param int $contactID + * @param string $where + */ + public function aclWhereGreaterThan($type, &$tables, &$whereTables, &$contactID, &$where) { + $where = " contact_a.id > " . $this->allowedContactId; + } + /** * Set up a core ACL. * -- 2.25.1