From 7f17ac7e2f50bedc9ae6dc1927b464daacdf6590 Mon Sep 17 00:00:00 2001 From: colemanw Date: Sun, 3 Sep 2023 19:36:16 -0400 Subject: [PATCH] Afform - Support multiple permissions in the GUI --- .../Civi/Api4/Action/Afform/LoadAdminData.php | 4 ++-- .../ang/afAdminFormSubmissionList.aff.json | 3 ++- ext/afform/admin/ang/afGuiEditor.css | 7 +++++++ .../ang/afGuiEditor/afGuiEditor.component.js | 2 ++ .../admin/ang/afGuiEditor/config-form.html | 13 +++++++++++-- ext/afform/core/CRM/Afform/AfformScanner.php | 4 ++-- .../core/Civi/Api4/Action/Afform/Get.php | 5 ++++- ext/afform/core/Civi/Api4/Afform.php | 6 ++++++ .../core/Civi/Api4/Utils/AfformSaveTrait.php | 3 +++ ext/afform/core/afform.php | 19 +++++++++++++------ .../mock/tests/phpunit/api/v4/AfformTest.php | 12 ++++++------ 11 files changed, 58 insertions(+), 20 deletions(-) diff --git a/ext/afform/admin/Civi/Api4/Action/Afform/LoadAdminData.php b/ext/afform/admin/Civi/Api4/Action/Afform/LoadAdminData.php index 0828b562b2..675bee1938 100644 --- a/ext/afform/admin/Civi/Api4/Action/Afform/LoadAdminData.php +++ b/ext/afform/admin/Civi/Api4/Action/Afform/LoadAdminData.php @@ -48,7 +48,7 @@ class LoadAdminData extends \Civi\Api4\Generic\AbstractAction { case 'form': $info['definition'] = $this->definition + [ 'title' => '', - 'permission' => 'access CiviCRM', + 'permission' => ['access CiviCRM'], 'layout' => [ [ '#tag' => 'af-form', @@ -70,7 +70,7 @@ class LoadAdminData extends \Civi\Api4\Generic\AbstractAction { case 'search': $info['definition'] = $this->definition + [ 'title' => '', - 'permission' => 'access CiviCRM', + 'permission' => ['access CiviCRM'], 'layout' => [ [ '#tag' => 'div', diff --git a/ext/afform/admin/ang/afAdminFormSubmissionList.aff.json b/ext/afform/admin/ang/afAdminFormSubmissionList.aff.json index 88d0a00294..3584ff5948 100644 --- a/ext/afform/admin/ang/afAdminFormSubmissionList.aff.json +++ b/ext/afform/admin/ang/afAdminFormSubmissionList.aff.json @@ -2,5 +2,6 @@ "type": "system", "title": "Submissions", "server_route": "civicrm/admin/afform/submissions", - "permission": [["administer CiviCRM", "administer afform"]] + "permission": ["administer CiviCRM", "administer afform"], + "permission_operator": "OR" } diff --git a/ext/afform/admin/ang/afGuiEditor.css b/ext/afform/admin/ang/afGuiEditor.css index 3aca35836f..b15d5892c7 100644 --- a/ext/afform/admin/ang/afGuiEditor.css +++ b/ext/afform/admin/ang/afGuiEditor.css @@ -346,6 +346,13 @@ body.af-gui-dragging { color: #0071bd; } +#afGuiEditor #af_config_form_permission { + width: calc(100% - 55px); +} +#afGuiEditor #af_config_form_permission_operator { + width: 45px; +} + #afGuiEditor .af-gui-layout-icon { width: 12px; height: 11px; diff --git a/ext/afform/admin/ang/afGuiEditor/afGuiEditor.component.js b/ext/afform/admin/ang/afGuiEditor/afGuiEditor.component.js index 73ab209afe..571d796006 100644 --- a/ext/afform/admin/ang/afGuiEditor/afGuiEditor.component.js +++ b/ext/afform/admin/ang/afGuiEditor/afGuiEditor.component.js @@ -136,6 +136,8 @@ editor.searchDisplays = getSearchDisplaysOnForm(); } + editor.afform.permission_operator = editor.afform.permission_operator || 'AND'; + // Initialize undo history undoAction = 'initialLoad'; undoHistory = [{ diff --git a/ext/afform/admin/ang/afGuiEditor/config-form.html b/ext/afform/admin/ang/afGuiEditor/config-form.html index bc858eef3e..a154730ebb 100644 --- a/ext/afform/admin/ang/afGuiEditor/config-form.html +++ b/ext/afform/admin/ang/afGuiEditor/config-form.html @@ -22,8 +22,17 @@ - -

{{:: ts('What permission is required to use this form?') }}

+
+ + +
+

+ {{:: ts('What permission is required to use this form?') }} + {{:: ts('Join multiple permissions with "And" to require all, or "Or" to require at least one.') }} +

diff --git a/ext/afform/core/CRM/Afform/AfformScanner.php b/ext/afform/core/CRM/Afform/AfformScanner.php index b7f3a5a47c..ae136c30c5 100644 --- a/ext/afform/core/CRM/Afform/AfformScanner.php +++ b/ext/afform/core/CRM/Afform/AfformScanner.php @@ -149,7 +149,7 @@ class CRM_Afform_AfformScanner { 'is_dashlet' => FALSE, 'is_public' => FALSE, 'is_token' => FALSE, - 'permission' => 'access CiviCRM', + 'permission' => ['access CiviCRM'], 'type' => 'system', ]; @@ -157,7 +157,7 @@ class CRM_Afform_AfformScanner { if ($metaFile !== NULL) { $r = array_merge($defaults, json_decode(file_get_contents($metaFile), 1)); // Previous revisions of GUI allowed permission==''. array_merge() doesn't catch all forms of missing-ness. - if ($r['permission'] === '') { + if (empty($r['permission'])) { $r['permission'] = $defaults['permission']; } return $r; diff --git a/ext/afform/core/Civi/Api4/Action/Afform/Get.php b/ext/afform/core/Civi/Api4/Action/Afform/Get.php index aa376d8a39..9976c73aa7 100644 --- a/ext/afform/core/Civi/Api4/Action/Afform/Get.php +++ b/ext/afform/core/Civi/Api4/Action/Afform/Get.php @@ -81,6 +81,9 @@ class Get extends \Civi\Api4\Generic\BasicGetAction { if ($getComputed) { $scanner->addComputedFields($afforms[$name]); } + if (isset($afforms[$name]['permission']) && is_string($afforms[$name]['permission'])) { + $afforms[$name]['permission'] = explode(',', $afforms[$name]['permission']); + } if ($getLayout || $getSearchDisplays) { // Autogenerated layouts will already be in values but can be overridden; scanner takes priority $afforms[$name]['layout'] = $scanner->getLayout($name) ?? $afforms[$name]['layout'] ?? ''; @@ -163,7 +166,7 @@ class Get extends \Civi\Api4\Generic\BasicGetAction { 'is_dashlet' => FALSE, 'is_public' => FALSE, 'is_token' => FALSE, - 'permission' => 'access CiviCRM', + 'permission' => ['access CiviCRM'], 'join_entity' => 'Custom_' . $custom['name'], 'entity_type' => $custom['extends'], ]; diff --git a/ext/afform/core/Civi/Api4/Afform.php b/ext/afform/core/Civi/Api4/Afform.php index 71ce29fa65..d1316c20c6 100644 --- a/ext/afform/core/Civi/Api4/Afform.php +++ b/ext/afform/core/Civi/Api4/Afform.php @@ -190,6 +190,12 @@ class Afform extends Generic\AbstractEntity { ], [ 'name' => 'permission', + 'data_type' => 'Array', + ], + [ + 'name' => 'permission_operator', + 'data_type' => 'String', + 'options' => \CRM_Core_SelectValues::andOr(), ], [ 'name' => 'redirect', diff --git a/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php b/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php index 06147458ea..42416f11e2 100644 --- a/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php +++ b/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php @@ -53,6 +53,9 @@ trait AfformSaveTrait { $meta = $item + (array) $orig; unset($meta['layout'], $meta['name']); + if (isset($meta['permission']) && is_string($meta['permission'])) { + $meta['permission'] = explode(',', $meta['permission']); + } if (!empty($meta)) { $metaPath = $scanner->createSiteLocalPath($item['name'], \CRM_Afform_AfformScanner::METADATA_FILE); \CRM_Utils_File::createDir(dirname($metaPath)); diff --git a/ext/afform/core/afform.php b/ext/afform/core/afform.php index 9aa670c910..007e52088a 100644 --- a/ext/afform/core/afform.php +++ b/ext/afform/core/afform.php @@ -136,8 +136,8 @@ function afform_civicrm_managed(&$entities, $modules) { 'values' => [ 'name' => $afform['name'], 'label' => $afform['navigation']['label'] ?: $afform['title'], - 'permission' => (array) $afform['permission'], - 'permission_operator' => 'OR', + 'permission' => $afform['permission'], + 'permission_operator' => $afform['permission_operator'] ?? 'AND', 'weight' => $afform['navigation']['weight'] ?? 0, 'url' => $afform['server_route'], 'is_active' => 1, @@ -443,14 +443,21 @@ function afform_civicrm_permission_check($permission, &$granted, $contactId) { if (preg_match('/^@afform:(.*)/', $permission, $m)) { $name = $m[1]; - $afform = \Civi\Api4\Afform::get() - ->setCheckPermissions(FALSE) + $afform = \Civi\Api4\Afform::get(FALSE) ->addWhere('name', '=', $name) - ->setSelect(['permission']) + ->addSelect('permission', 'permission_operator') ->execute() ->first(); + // No permissions found... this shouldn't happen but just in case, set default. + if ($afform && empty($afform['permission'])) { + $afform['permission'] = ['access CiviCRM']; + } if ($afform) { - $granted = CRM_Core_Permission::check($afform['permission'], $contactId); + $check = (array) $afform['permission']; + if ($afform['permission_operator'] === 'OR') { + $check = [$check]; + } + $granted = CRM_Core_Permission::check($check, $contactId); } } } diff --git a/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php b/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php index 8f093eac42..a673e680a8 100644 --- a/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php +++ b/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php @@ -31,10 +31,10 @@ class api_v4_AfformTest extends api_v4_AfformTestCase { public function getBasicDirectives() { return [ - ['mockPage', ['title' => '', 'description' => '', 'server_route' => 'civicrm/mock-page', 'permission' => 'access Foobar', 'is_dashlet' => TRUE]], - ['mockBareFile', ['title' => '', 'description' => '', 'permission' => 'access CiviCRM', 'is_dashlet' => FALSE]], - ['mockFoo', ['title' => '', 'description' => '', 'permission' => 'access CiviCRM']], - ['mock-weird-name', ['title' => 'Weird Name', 'description' => '', 'permission' => 'access CiviCRM']], + ['mockPage', ['title' => '', 'description' => '', 'server_route' => 'civicrm/mock-page', 'permission' => ['access Foobar'], 'is_dashlet' => TRUE]], + ['mockBareFile', ['title' => '', 'description' => '', 'permission' => ['access CiviCRM'], 'is_dashlet' => FALSE]], + ['mockFoo', ['title' => '', 'description' => '', 'permission' => ['access CiviCRM']]], + ['mock-weird-name', ['title' => 'Weird Name', 'description' => '', 'permission' => ['access CiviCRM']]], ]; } @@ -85,7 +85,7 @@ class api_v4_AfformTest extends api_v4_AfformTestCase { $result = Civi\Api4\Afform::update() ->addWhere('name', '=', $formName) ->addValue('description', 'The temporary description') - ->addValue('permission', 'access foo') + ->addValue('permission', ['access foo', 'access bar']) ->addValue('is_dashlet', empty($originalMetadata['is_dashlet'])) ->execute(); $this->assertEquals($formName, $result[0]['name'], $message); @@ -100,7 +100,7 @@ class api_v4_AfformTest extends api_v4_AfformTestCase { $this->assertEquals('The temporary description', $get($result[0], 'description'), $message); $this->assertEquals(empty($originalMetadata['is_dashlet']), $get($result[0], 'is_dashlet'), $message); $this->assertEquals($get($originalMetadata, 'server_route'), $get($result[0], 'server_route'), $message); - $this->assertEquals('access foo', $get($result[0], 'permission'), $message); + $this->assertEquals(['access foo', 'access bar'], $get($result[0], 'permission'), $message); $this->assertTrue(is_array($result[0]['layout']), $message); $this->assertEquals(TRUE, $get($result[0], 'has_base'), $message); $this->assertEquals(TRUE, $get($result[0], 'has_local'), $message); -- 2.25.1