From 5623bf2a4a5f32235400a19afa66a7220ff2ecba Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Tue, 15 Jun 2021 02:25:51 -0400 Subject: [PATCH] SearchKit - Enable super-admins to disable Search Display access checks This allows users with 'all CiviCRM permissions and ACLs' to configure a search display to bypass permission checks and display all records to the user. Once a display is set to bypass ACLs, it can only be edited by a super-admin, ordinary admin users will not be able to edit the display or the saved search. Such a display will not automatically appear on its own page; it must be embedded in an Afform, and the Afform will act as gatekeeper for users to view the display. --- Civi/Api4/Utils/CoreUtil.php | 6 + css/civicrm.css | 2 +- .../CRM/Search/BAO/SearchDisplay.php | 43 ++++ .../Civi/Api4/Action/SearchDisplay/Run.php | 11 +- ext/search_kit/ang/crmSearchAdmin.ang.php | 1 + ext/search_kit/ang/crmSearchAdmin.module.js | 1 + .../crmSearchAdminDisplay.component.js | 3 + .../crmSearchAdmin/crmSearchAdminDisplay.html | 13 ++ .../crmSearchAdmin/searchList.controller.js | 1 + .../ang/crmSearchAdmin/searchList.html | 6 +- ext/search_kit/css/crmSearchAdmin.css | 9 + ext/search_kit/search_kit.php | 11 ++ .../api/v4/SearchDisplay/SearchRunTest.php | 183 ++++++++++++++++++ .../api/v4/Traits/CheckAccessTrait.php | 4 + 14 files changed, 288 insertions(+), 6 deletions(-) diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php index 8399142500..a43bfeb6f7 100644 --- a/Civi/Api4/Utils/CoreUtil.php +++ b/Civi/Api4/Utils/CoreUtil.php @@ -166,6 +166,12 @@ class CoreUtil { * @throws \Civi\API\Exception\UnauthorizedException */ public static function checkAccessRecord(\Civi\Api4\Generic\AbstractAction $apiRequest, array $record, int $userID) { + + // Super-admins always have access to everything + if (\CRM_Core_Permission::check('all CiviCRM permissions and ACLs', $userID)) { + return TRUE; + } + // 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. diff --git a/css/civicrm.css b/css/civicrm.css index c91c5a000c..27277e73b7 100644 --- a/css/civicrm.css +++ b/css/civicrm.css @@ -2781,7 +2781,7 @@ tbody.scrollContent tr.alternateRow { } .crm-container .disabled, -.crm-container .disabled td, +.crm-container .disabled *, .crm-container .cancelled, .crm-container .cancelled td, .crm-container li.disabled a.ui-tabs-anchor, diff --git a/ext/search_kit/CRM/Search/BAO/SearchDisplay.php b/ext/search_kit/CRM/Search/BAO/SearchDisplay.php index 0d6f6651d5..48776d96eb 100644 --- a/ext/search_kit/CRM/Search/BAO/SearchDisplay.php +++ b/ext/search_kit/CRM/Search/BAO/SearchDisplay.php @@ -14,4 +14,47 @@ */ class CRM_Search_BAO_SearchDisplay extends CRM_Search_DAO_SearchDisplay { + /** + * Ensure only super-admins are allowed to create or update displays with acl_bypass + * + * @param string $entityName + * @param string $action + * @param array $record + * @param int $userCID + * @return bool + */ + public static function _checkAccess(string $entityName, string $action, array $record, int $userCID) { + // If we hit this function at all, the user is not a super-admin + // But they must be at least a regular administrator + if (!CRM_Core_Permission::check('administer CiviCRM data')) { + return FALSE; + } + if (in_array($action, ['create', 'update'], TRUE)) { + // Do not allow acl_bypass to be set to TRUE + if (!empty($record['acl_bypass'])) { + return FALSE; + } + // Do not allow edits to an existing record with acl_bypass = TRUE + if (!empty($record['id'])) { + return !CRM_Core_DAO::getFieldValue(__CLASS__, $record['id'], 'acl_bypass'); + } + } + return TRUE; + } + + /** + * Ensure only super-admins may update SavedSearches linked to displays with acl_bypass + * + * @param \Civi\Api4\Event\AuthorizeRecordEvent $e + */ + public static function savedSearchCheckAccessByDisplay(\Civi\Api4\Event\AuthorizeRecordEvent $e) { + if ($e->getActionName() === 'update') { + $id = (int) $e->getRecord()['id']; + $sql = "SELECT COUNT(id) FROM civicrm_search_display WHERE acl_bypass = 1 AND saved_search_id = $id"; + if (CRM_Core_DAO::singleValueQuery($sql)) { + $e->setAuthorized(FALSE); + } + } + } + } diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php index dcf36703c5..3d22fea9cc 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php @@ -75,7 +75,7 @@ class Run extends \Civi\Api4\Generic\AbstractAction { */ public function _run(\Civi\Api4\Generic\Result $result) { // Only administrators can use this in unsecured "preview mode" - if (!(is_string($this->savedSearch) && is_string($this->display)) && $this->checkPermissions && !\CRM_Core_Permission::check('administer CiviCRM')) { + if (!(is_string($this->savedSearch) && is_string($this->display)) && $this->checkPermissions && !\CRM_Core_Permission::check('administer CiviCRM data')) { throw new UnauthorizedException('Access denied'); } if (is_string($this->savedSearch)) { @@ -93,9 +93,16 @@ class Run extends \Civi\Api4\Generic\AbstractAction { if (!$this->savedSearch || !$this->display) { throw new \API_Exception("Error: SearchDisplay not found."); } + // Displays with acl_bypass must be embedded on an afform which the user has access to + if ( + $this->checkPermissions && !empty($this->display['acl_bypass']) && + !\CRM_Core_Permission::check('all CiviCRM permissions and ACLs') && !$this->loadAfform() + ) { + throw new UnauthorizedException('Access denied'); + } $entityName = $this->savedSearch['api_entity']; $apiParams =& $this->savedSearch['api_params']; - $apiParams['checkPermissions'] = $this->checkPermissions; + $apiParams['checkPermissions'] = empty($this->display['acl_bypass']); $apiParams += ['where' => []]; $settings = $this->display['settings']; $page = NULL; diff --git a/ext/search_kit/ang/crmSearchAdmin.ang.php b/ext/search_kit/ang/crmSearchAdmin.ang.php index a65b495313..31ff79962f 100644 --- a/ext/search_kit/ang/crmSearchAdmin.ang.php +++ b/ext/search_kit/ang/crmSearchAdmin.ang.php @@ -16,4 +16,5 @@ return [ 'basePages' => ['civicrm/admin/search'], 'requires' => ['crmUi', 'crmUtil', 'ngRoute', 'ui.sortable', 'ui.bootstrap', 'api4', 'crmSearchTasks', 'crmRouteBinder'], 'settingsFactory' => ['\Civi\Search\Admin', 'getAdminSettings'], + 'permissions' => ['all CiviCRM permissions and ACLs'], ]; diff --git a/ext/search_kit/ang/crmSearchAdmin.module.js b/ext/search_kit/ang/crmSearchAdmin.module.js index dfa3910d79..0ac61bf896 100644 --- a/ext/search_kit/ang/crmSearchAdmin.module.js +++ b/ext/search_kit/ang/crmSearchAdmin.module.js @@ -30,6 +30,7 @@ 'GROUP_CONCAT(display.name ORDER BY display.id) AS display_name', 'GROUP_CONCAT(display.label ORDER BY display.id) AS display_label', 'GROUP_CONCAT(display.type:icon ORDER BY display.id) AS display_icon', + 'GROUP_CONCAT(display.acl_bypass ORDER BY display.id) AS display_acl_bypass', 'GROUP_CONCAT(DISTINCT group.title) AS groups' ], join: [['SearchDisplay AS display'], ['Group AS group']], diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.component.js b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.component.js index d6d673be87..a15f452b67 100644 --- a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.component.js +++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.component.js @@ -36,6 +36,9 @@ var ts = $scope.ts = CRM.ts('org.civicrm.search_kit'), ctrl = this; + this.isSuperAdmin = CRM.checkPerm('all CiviCRM permissions and ACLs'); + this.aclBypassHelp = ts('Only users with "all CiviCRM permissions and ACLs" can disable permission checks.'); + this.preview = this.stale = false; this.colTypes = { diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.html b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.html index 41c63274b9..6393bf24b9 100644 --- a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.html +++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdminDisplay.html @@ -2,5 +2,18 @@
+
+ +
+
+ + {{:: ts('Anyone who can view this display will be able to see all results, regardless of their permission level.') }} +
+ diff --git a/ext/search_kit/ang/crmSearchAdmin/searchList.controller.js b/ext/search_kit/ang/crmSearchAdmin/searchList.controller.js index be17c652da..2266b65d7c 100644 --- a/ext/search_kit/ang/crmSearchAdmin/searchList.controller.js +++ b/ext/search_kit/ang/crmSearchAdmin/searchList.controller.js @@ -13,6 +13,7 @@ _.each(savedSearches, function(search) { search.entity_title = searchMeta.getEntity(search.api_entity).title_plural; + search.permissionToEdit = CRM.checkPerm('all CiviCRM permissions and ACLs') || !_.includes(search.display_acl_bypass, true); search.afform_count = 0; }); diff --git a/ext/search_kit/ang/crmSearchAdmin/searchList.html b/ext/search_kit/ang/crmSearchAdmin/searchList.html index a698477772..debbbd0d9e 100644 --- a/ext/search_kit/ang/crmSearchAdmin/searchList.html +++ b/ext/search_kit/ang/crmSearchAdmin/searchList.html @@ -62,8 +62,8 @@ {{:: search.display_name.length === 1 ? ts('1 Display') : ts('%1 Displays', {1: search.display_name.length}) }}