dev/core#690 - Civi\API - Fix entity permission check for trusted calls

This changes the permission check in DynamicFKAuthorization to
short-circuit on trusted API calls (check_permissions = 0) so that
no exception is thrown when it's used with entities that are not
allowed delegates.

diff --git a/Civi/API/Subscriber/DynamicFKAuthorization.php b/Civi/API/Subscriber/DynamicFKAuthorization.php
index a00802e65e52a720e50f623d36dd8fc1ec8da482..aade5b2968a863c449c1ae7c430da936b971c702 100644 (file)
--- a/Civi/API/Subscriber/DynamicFKAuthorization.php
+++ b/Civi/API/Subscriber/DynamicFKAuthorization.php
@@ -206,6 +206,10 @@ class DynamicFKAuthorization implements EventSubscriberInterface {
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function authorizeDelegate($action, $entityTable, $entityId, $apiRequest) {
+    if ($this->isTrusted($apiRequest)) {
+      return;
+    }
+
     $entity = $this->getDelegatedEntityName($entityTable);
     if (!$entity) {
       throw new \API_Exception("Failed to run permission check: Unrecognized target entity table ($entityTable)");
@@ -214,10 +218,6 @@ class DynamicFKAuthorization implements EventSubscriberInterface {
       throw new \Civi\API\Exception\UnauthorizedException("Authorization failed on ($entity): Missing entity_id");
     }
 
-    if ($this->isTrusted($apiRequest)) {
-      return;
-    }
-
     /**
      * @var \Exception $exception
      */

diff --git a/tests/phpunit/Civi/API/Subscriber/DynamicFKAuthorizationTest.php b/tests/phpunit/Civi/API/Subscriber/DynamicFKAuthorizationTest.php
index df0b5becf50062563aafe351fcdfddfbd54a3890..3e0a32f47044c016e6269102305ec051bbc11d61 100644 (file)
--- a/tests/phpunit/Civi/API/Subscriber/DynamicFKAuthorizationTest.php
+++ b/tests/phpunit/Civi/API/Subscriber/DynamicFKAuthorizationTest.php
@@ -11,6 +11,8 @@ class DynamicFKAuthorizationTest extends \CiviUnitTestCase {
 
   const FILE_FORBIDDEN_ID = 11;
 
+  const FILE_UNDELEGATED_ENTITY = 12;
+
   const WIDGET_ID = 20;
 
   const FORBIDDEN_ID = 30;
@@ -214,4 +216,28 @@ class DynamicFKAuthorizationTest extends \CiviUnitTestCase {
     $this->assertRegExp($expectedError, $result['error_message']);
   }
 
+  /**
+   * Test whether trusted API calls bypass the permission check
+   *
+   */
+  public function testNotDelegated() {
+    $entity = 'FakeFile';
+    $action = 'create';
+    $params = [
+      'entity_id' => self::FILE_UNDELEGATED_ENTITY,
+      'entity_table' => 'civicrm_membership',
+      'version' => 3,
+      'debug' => 1,
+      'check_permissions' => 1,
+    ];
+    // run with permission check
+    $result = $this->kernel->run('FakeFile', 'create', $params);
+    $this->assertTrue((bool) $result['is_error'], 'Undelegated entity with check_permissions = 1 should fail');
+    $this->assertRegExp('/Unrecognized target entity table \(civicrm_membership\)/', $result['error_message']);
+    // repeat without permission check
+    $params['check_permissions'] = 0;
+    $result = $this->kernel->run('FakeFile', 'create', $params);
+    $this->assertFalse((bool) $result['is_error'], 'Undelegated entity with check_permissions = 0 should succeed');
+  }
+
 }