dev/core#421 Fix issue where creating user driven message templates w…

…as 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
civicrm · Oct 8, 2018 · 781ed31 · 781ed31
1 parent 412c1f4
commit 781ed31
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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]);
+  }
+
 }