From 330ec60eb834b93be2a4460e6e273182ad7dc955 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 --- CRM/Core/BAO/MessageTemplate.php | 21 +++++++++++++++ CRM/Core/Permission.php | 4 +-- tests/phpunit/api/v3/MessageTemplateTest.php | 27 ++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php index 9d2b002aacb4..b6214cb4f586 100644 --- a/CRM/Core/BAO/MessageTemplate.php +++ b/CRM/Core/BAO/MessageTemplate.php @@ -83,6 +83,27 @@ 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. + if (!empty($params['id']) && !empty($params['check_permissions'])) { + $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('Editing or creating system workflow messages requires edit system workflow message templates permission as well as edit message templates')); + } + } + elseif (!CRM_Core_Permission::check('edit user-driven message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('Editing or creating user driven workflow messages requires edit user-driven message templates as well as edit message templates')); + } + } + elseif (!empty($params['check_permissions'])) { + if (!empty($params['workflow_id']) && !CRM_Core_Permission::check('edit system workflow message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('Editing or creating system workflow messages requires edit system workflow message templates permission as well as edit message templates')); + } + elseif (!CRM_Core_Permission::check('edit user-driven message templates')) { + throw new \Civi\API\Exception\UnauthorizedException(ts('Editing or creating user driven workflow messages requires edit user-driven message templates as well as edit message templates')); + } + } $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..087b11f65f6d 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('edit message templates'), + 'update' => array('edit 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..710881569bc4 100644 --- a/tests/phpunit/api/v3/MessageTemplateTest.php +++ b/tests/phpunit/api/v3/MessageTemplateTest.php @@ -55,6 +55,11 @@ public function setUp() { ); } + public function tareDown() { + parent::tareDown(); + unset(CRM_Core_Config::singleton()->userPermissionClass->permissions); + } + /** * Test create function succeeds. */ @@ -89,4 +94,26 @@ public function testDelete() { $this->assertEquals(0, $checkDeleted['count']); } + public function testPermissionChecks() { + $entity = $this->createTestEntity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('edit message templates', '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 message templates', '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 message templates', '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]); + } + }