From 781ed314a81b287b696404426c8e6a31a0ece9c7 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Fri, 5 Oct 2018 12:31:38 +1000 Subject: [PATCH] dev/core#421 Fix issue where creating user driven message templates was requireing the ssystem workflow message template permission as well Wrap permission checking in the check_permissions param Move Permission checking to BAO level from API Allow for the fact that edit message templates permission should still be able to work on both sets of templates without the granular permissions. Also reduce duplication of code abit --- CRM/Core/BAO/MessageTemplate.php | 27 +++++++++++++++++ CRM/Core/Permission.php | 4 +-- tests/phpunit/api/v3/MessageTemplateTest.php | 31 ++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php index 9d2b002aacb4..0fd69789cc69 100644 --- a/CRM/Core/BAO/MessageTemplate.php +++ b/CRM/Core/BAO/MessageTemplate.php @@ -83,6 +83,33 @@ public static function setIsActive($id, $is_active) { * @return object */ public static function add(&$params) { + // System Workflow Templates have a specific wodkflow_id in them but normal user end message templates don't + // If we have an id check to see if we are update, and need to check if original is a system workflow or not. + $systemWorkflowPermissionDeniedMessage = 'Editing or creating system workflow messages requires edit system workflow message templates permission or the edit message templates permission'; + $userWorkflowPermissionDeniedMessage = 'Editing or creating user driven workflow messages requires edit user-driven message templates or the edit message templates permission'; + if (!empty($params['check_permissions'])) { + if (!CRM_Core_Permission::check('edit message templates')) { + if (!empty($params['id'])) { + $details = civicrm_api3('MessageTemplate', 'getSingle', ['id' => $params['id']]); + if (!empty($details['workflow_id'])) { + if (!CRM_Core_Permission::check('edit system workflow message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $systemWorkflowPermissionDeniedMessage])); + } + } + elseif (!CRM_Core_Permission::check('edit user-driven message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $userWorkflowPermissionDeniedMessage])); + } + } + else { + if (!empty($params['workflow_id']) && !CRM_Core_Permission::check('edit system workflow message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $systemWorkflowPermissionDeniedMessage])); + } + elseif (!CRM_Core_Permission::check('edit user-driven message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $userWorkflowPermissionDeniedMessage])); + } + } + } + } $hook = empty($params['id']) ? 'create' : 'edit'; CRM_Utils_Hook::pre($hook, 'MessageTemplate', CRM_Utils_Array::value('id', $params), $params); diff --git a/CRM/Core/Permission.php b/CRM/Core/Permission.php index 49d003dd62c4..3448f7f0df46 100644 --- a/CRM/Core/Permission.php +++ b/CRM/Core/Permission.php @@ -1486,8 +1486,8 @@ public static function getEntityActionPermissions() { $permissions['message_template'] = array( 'get' => array('access CiviCRM'), - 'create' => array('edit message templates', 'edit user-driven message templates', 'edit system workflow message templates'), - 'update' => array('edit message templates', 'edit user-driven message templates', 'edit system workflow message templates'), + 'create' => array(array('edit message templates', 'edit user-driven message templates', 'edit system workflow message templates')), + 'update' => array(array('edit message templates', 'edit user-driven message templates', 'edit system workflow message templates')), ); $permissions['report_template']['update'] = 'save Report Criteria'; diff --git a/tests/phpunit/api/v3/MessageTemplateTest.php b/tests/phpunit/api/v3/MessageTemplateTest.php index d40173566e70..ab370117426f 100644 --- a/tests/phpunit/api/v3/MessageTemplateTest.php +++ b/tests/phpunit/api/v3/MessageTemplateTest.php @@ -55,6 +55,11 @@ public function setUp() { ); } + public function tearDown() { + parent::tearDown(); + unset(CRM_Core_Config::singleton()->userPermissionClass->permissions); + } + /** * Test create function succeeds. */ @@ -89,4 +94,30 @@ public function testDelete() { $this->assertEquals(0, $checkDeleted['count']); } + public function testPermissionChecks() { + $entity = $this->createTestEntity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('edit user-driven message templates'); + // Ensure that it cannot create a system message or update a system message tempalte given current permissions. + $this->callAPIFailure('MessageTemplate', 'create', ['id' => $entity['id'], 'msg_subject' => 'test msg permission subject', 'check_permissions' => TRUE]); + $testUserEntity = $entity['values'][$entity['id']]; + unset($testUserEntity['id']); + $testUserEntity['msg_subject'] = 'Test user message template'; + unset($testUserEntity['workflow_id']); + $testuserEntity['check_permissions'] = TRUE; + // ensure that it can create user templates; + $userEntity = $this->callAPISuccess('MessageTemplate', 'create', $testUserEntity); + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('edit system workflow message templates'); + // Now check that when its swapped around permissions that the correct reponses are detected. + $this->callAPIFailure('MessageTemplate', 'create', ['id' => $userEntity['id'], 'msg_subject' => 'User template updated by system message permission', 'check_permissions' => TRUE]); + $this->callAPISuccess('MessageTemplate', 'create', ['id' => $entity['id'], 'msg_subject' => 'test msg permission subject', 'check_permissions' => TRUE]); + // verify with all 3 permissions someone can do everything. + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('edit system workflow message templates', 'edit user-driven message templates'); + $this->callAPISuccess('MessageTemplate', 'create', ['id' => $userEntity['id'], 'msg_subject' => 'User template updated by system message permission', 'check_permissions' => TRUE]); + $this->callAPISuccess('MessageTemplate', 'create', ['id' => $entity['id'], 'msg_subject' => 'test msg permission subject', 'check_permissions' => TRUE]); + // Verify that the backwards compatabiltiy still works i.e. having edit message templates allows for editing of both kinds of message templates + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('edit message templates'); + $this->callAPISuccess('MessageTemplate', 'create', ['id' => $userEntity['id'], 'msg_subject' => 'User template updated by edit message permission', 'check_permissions' => TRUE]); + $this->callAPISuccess('MessageTemplate', 'create', ['id' => $entity['id'], 'msg_subject' => 'test msg permission subject backwards compatabilty', 'check_permissions' => TRUE]); + } + }