Remote entity profile: Allow to restrict access with join

Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandler.php

@@ -31,6 +31,7 @@
 use Civi\RemoteTools\Api4\Action\RemoteValidateCreateFormAction;
 use Civi\RemoteTools\Api4\Action\RemoteValidateUpdateFormAction;
 use Civi\RemoteTools\Api4\Api4Interface;
+use Civi\RemoteTools\Api4\Query\Join;
 use Civi\RemoteTools\Api4\Query\QueryApplier;
 use Civi\RemoteTools\EntityProfile\EntityProfileOptionSuffixDecorator;
 use Civi\RemoteTools\EntityProfile\EntityProfilePermissionDecorator;
@@ -363,6 +364,11 @@ protected function convertToSubmitFormActionResult(
    * @throws \CRM_Core_Exception
    */
   protected function getEntityById(int $id, string $actionName, ?int $contactId): ?array {
+    $join = array_map(
+      fn (Join $join) => $join->toArray(),
+      $this->profile->getJoins($actionName, $contactId)
+    );
+
     $where = [['id', '=', $id]];
     $filter = $this->profile->getFilter($actionName, $contactId);
     if (NULL !== $filter) {
@@ -371,6 +377,7 @@ protected function getEntityById(int $id, string $actionName, ?int $contactId):
 
     return $this->api4->execute($this->profile->getEntityName(), 'get', [
       'select' => $this->profile->getSelectFieldNames(['*'], $actionName, [], $contactId),
+      'join' => $join,
       'where' => $where,
       'checkPermissions' => $this->profile->isCheckApiPermissions($contactId),
     ])->first();

Civi/RemoteTools/Api4/Query/ConditionInterface.php

@@ -23,15 +23,16 @@
  * @phpstan-import-type comparisonT from Comparison
  * @phpstan-import-type compositeConditionT from CompositeCondition
  *
+ * @phpstan-type conditionT comparisonT|compositeConditionT
+ *
  * @api
  */
 interface ConditionInterface {
 
   public function getOperator(): string;
 
   /**
-   * @return array
-   * @phpstan-return comparisonT|compositeConditionT
+   * @phpstan-return conditionT
    */
   public function toArray(): array;
 

Civi/RemoteTools/Api4/Query/Join.php

@@ -22,33 +22,63 @@
 use Webmozart\Assert\Assert;
 
 /**
- * @phpstan-type ComparisonT array{string, string, 2?: scalar|array<scalar>}
- * Actually this should be: array{string, array<int, ComparisonT|CompositeConditionT>}, so that is not possible
- * @phpstan-type CompositeConditionT array{string, array<int, array>}
- * @phpstan-type ConditionT ComparisonT|CompositeConditionT
+ * @phpstan-import-type conditionT from \Civi\RemoteTools\Api4\Query\ConditionInterface
+ *
+ * @phpstan-type joinTypeT 'INNER'|'LEFT'|'EXCLUDE'
+ *
+ * @phpstan-type joinT list<string|conditionT>
+ *   The above type hint is required because of the workaround used. Otherwise,
+ *   the type hint would be:
+ *   array{string, joinTypeT, conditionT}|array{string, joinTypeT, string, 3?: conditionT}
+ *   The second option is when using a bridge.
+ *
+ * @see self::generateConditionsWorkaround()
+ *
+ * @api
  */
 final class Join {
 
   private string $entityName;
 
   private string $alias;
 
+  /**
+   * @phpstan-var joinTypeT
+   */
   private string $type;
 
   private ?string $bridge;
 
   private ?ConditionInterface $condition;
 
+  /**
+   * @phpstan-param joinTypeT $type
+   *
+   * If a value in a condition is not a field name, it must be enclosed by '"',
+   * e.g. '"my_string"'.
+   */
   public static function new(string $entityName, string $alias, string $type, ConditionInterface $condition): self {
     return new self($entityName, $alias, $type, NULL, $condition);
   }
 
+  /**
+   * @phpstan-param joinTypeT $type
+   *
+   *  If a value in a condition is not a field name, it must be enclosed by '"',
+   *  e.g. '"my_string"'.
+   */
   public static function newWithBridge(string $entityName, string $alias, string $type, string $bridge,
     ?ConditionInterface $condition = NULL
   ): self {
     return new self($entityName, $alias, $type, $bridge, $condition);
   }
 
+  /**
+   * @phpstan-param joinTypeT $type
+   *
+   *  If a value in a condition is not a field name, it must be enclosed by '"',
+   *  e.g. '"my_string"'.
+   */
   public function __construct(string $entityName, string $alias, string $type, ?string $bridge,
     ?ConditionInterface $condition
   ) {
@@ -69,6 +99,9 @@ public function getAlias(): string {
     return $this->alias;
   }
 
+  /**
+   * @phpstan-return joinTypeT
+   */
   public function getType(): string {
     return $this->type;
   }
@@ -82,21 +115,36 @@ public function getCondition(): ?ConditionInterface {
   }
 
   /**
-   * Actually this should be array{string, string, string|ConditionT, ...ConditionT}, so this is not possible
-   * @phpstan-return array<int, string|ConditionT>
-   *
-   * @return array
+   * @phpstan-return joinT
    */
   public function toArray(): array {
     $join = [$this->entityName . ' AS ' . $this->alias, $this->type];
     if (NULL !== $this->bridge) {
       $join[] = $this->bridge;
+      if (NULL !== $this->condition) {
+        $join = array_merge($join, self::generateConditionsWorkaround($this->condition));
+      }
     }
-    if (NULL !== $this->condition) {
-      $join[] = $this->condition->toArray();
+    else {
+      // @phpstan-ignore argument.type
+      $join = array_merge($join, self::generateConditionsWorkaround($this->condition));
     }
 
     return $join;
   }
 
+  /**
+   * This is a workaround and (in some cases) necessary because of
+   * https://lab.civicrm.org/dev/core/-/issues/5500
+   *
+   * @phpstan-return list<conditionT>
+   */
+  private static function generateConditionsWorkaround(ConditionInterface $condition): array {
+    if ($condition instanceof CompositeCondition && 'AND' === $condition->getOperator()) {
+      return array_map(fn (ConditionInterface $subCondition) => $subCondition->toArray(), $condition->getConditions());
+    }
+
+    return [$condition->toArray()];
+  }
+
 }

Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfile.php

@@ -58,6 +58,13 @@ public function getSelectFieldNames(array $select, string $actionName, array $re
     return $select;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public function getJoins(string $actionName, ?int $contactId): array {
+    return [];
+  }
+
   /**
    * @inheritDoc
    */

Civi/RemoteTools/EntityProfile/AbstractRemoteEntityProfileDecorator.php

@@ -98,6 +98,13 @@ public function getFilter(string $actionName, ?int $contactId): ?ConditionInterf
     return $this->profile->getFilter($actionName, $contactId);
   }
 
+  /**
+   * @inheritDoc
+   */
+  public function getJoins(string $actionName, ?int $contactId): array {
+    return $this->profile->getJoins($actionName, $contactId);
+  }
+
   /**
    * @inheritDoc
    */

Civi/RemoteTools/EntityProfile/Helper/ProfileEntityDeleter.php

@@ -22,6 +22,7 @@
 use Civi\RemoteTools\Api4\Action\RemoteDeleteAction;
 use Civi\RemoteTools\Api4\Api4Interface;
 use Civi\RemoteTools\Api4\Query\Comparison;
+use Civi\RemoteTools\Api4\Query\Join;
 use Civi\RemoteTools\EntityProfile\RemoteEntityProfileInterface;
 use Civi\RemoteTools\Helper\WhereFactoryInterface;
 use Webmozart\Assert\Assert;
@@ -55,10 +56,15 @@ public function delete(RemoteEntityProfileInterface $profile, RemoteDeleteAction
     ])->indexBy('name')->getArrayCopy();
     $remoteFields = $profile->getRemoteFields($entityFields, $action->getResolvedContactId());
 
+    $join = array_map(
+      fn (Join $join) => $join->toArray(),
+      $profile->getJoins('delete', $action->getResolvedContactId())
+    );
     $where = $this->createWhereForDelete($profile, $action, $entityFields, $remoteFields);
 
     $getResult = $this->api4->execute($profile->getEntityName(), 'get', [
       'select' => $profile->getSelectFieldNames(['*'], 'delete', [], $action->getResolvedContactId()),
+      'join' => $join,
       'where' => $where,
       'checkPermissions' => $profile->isCheckApiPermissions($action->getResolvedContactId()),
     ]);

Civi/RemoteTools/EntityProfile/Helper/ProfileEntityLoader.php

@@ -23,6 +23,7 @@
 use Civi\RemoteTools\Api4\Action\RemoteGetAction;
 use Civi\RemoteTools\Api4\Api4Interface;
 use Civi\RemoteTools\Api4\Query\Comparison;
+use Civi\RemoteTools\Api4\Query\Join;
 use Civi\RemoteTools\Api4\Query\QueryApplier;
 use Civi\RemoteTools\EntityProfile\RemoteEntityProfileInterface;
 use Civi\RemoteTools\Helper\SelectFactoryInterface;
@@ -60,6 +61,10 @@ public function get(RemoteEntityProfileInterface $profile, RemoteGetAction $acti
     $remoteFields = $profile->getRemoteFields($entityFields, $action->getResolvedContactId());
 
     $selects = $this->createSelectsForGet($profile, $action, $entityFields, $remoteFields);
+    $join = array_map(
+      fn (Join $join) => $join->toArray(),
+      $profile->getJoins('get', $action->getResolvedContactId())
+    );
     $where = $this->createWhereForGet($profile, $action, $entityFields, $remoteFields);
 
     $entityOrderBy = [];
@@ -74,6 +79,7 @@ public function get(RemoteEntityProfileInterface $profile, RemoteGetAction $acti
 
     $result = $this->api4->execute($profile->getEntityName(), 'get', [
       'select' => $selects['entity'],
+      'join' => $join,
       'where' => $where,
       'orderBy' => $entityOrderBy,
       'limit' => $action->getLimit(),

Civi/RemoteTools/EntityProfile/RemoteEntityProfileInterface.php

@@ -163,6 +163,18 @@ public function getSelectFieldNames(array $select, string $actionName, array $re
    */
   public function getFilter(string $actionName, ?int $contactId): ?ConditionInterface;
 
+  /**
+   * Might be used to restrict access using related entities.
+   *
+   * @phpstan-param 'delete'|'get'|'update' $actionName
+   *
+   * @phpstan-return list<\Civi\RemoteTools\Api4\Query\Join>
+   *   Joins added to the query.
+   *
+   * @api
+   */
+  public function getJoins(string $actionName, ?int $contactId): array;
+
   /**
    * @phpstan-param array<string, mixed> $entityValues
    * @phpstan-param array<string> $select

phpstan.neon.dist

@@ -42,7 +42,6 @@ parameters:
 
 		- '#^Method Civi\\RemoteTools\\Api4\\Action\\[^\s]+Action::getHandlerResult\(\) has Civi\\RemoteTools\\Exception\\ActionHandlerNotFoundException in PHPDoc @throws tag but it.s not thrown.$#'
 		- '#^Method Civi\\RemoteTools\\Api4\\Api4::createAction\(\) should return Civi\\Api4\\Generic\\AbstractAction but returns array|Civi\Api4\Generic\AbstractAction.$#'
-		- '#^Method Civi\\RemoteTools\\Api4\\Query\\[^\\]+::toArray\(\) return type has no value type specified in iterable type array.$#'
 
 		- '#^Method Civi\\RemoteTools\\Fixture\\[^\s]+Fixture::[^\s]+ should return array\{id: int\} but returns array.$#'
 		-

tests/phpunit/Civi/RemoteTools/ActionHandler/AbstractProfileEntityActionsHandlerTest.php

@@ -15,6 +15,8 @@
 use Civi\RemoteTools\Api4\Action\RemoteValidateCreateFormAction;
 use Civi\RemoteTools\Api4\Action\RemoteValidateUpdateFormAction;
 use Civi\RemoteTools\Api4\Api4Interface;
+use Civi\RemoteTools\Api4\Query\Comparison;
+use Civi\RemoteTools\Api4\Query\Join;
 use Civi\RemoteTools\EntityProfile\Authorization\GrantResult;
 use Civi\RemoteTools\EntityProfile\EntityProfilePermissionDecorator;
 use Civi\RemoteTools\EntityProfile\Helper\ProfileEntityDeleterInterface;
@@ -92,7 +94,6 @@ protected function setUp(): void {
   public function testDelete(): void {
     $actionMock = $this->createMock(RemoteDeleteAction::class);
 
-    $result = new Result();
     $this->entityDeleterMock->method('delete')
       ->with(static::isInstanceOf(EntityProfilePermissionDecorator::class), $actionMock)
       ->willReturn([['id' => 1]]);
@@ -338,6 +339,7 @@ public function testGetUpdateForm(): void {
 
     $this->mockApi4Execute('Entity', 'get', [
       'select' => ['foo'],
+      'join' => [],
       'where' => [['id', '=', 12]],
       'checkPermissions' => FALSE,
     ], new Result([$entityValues]));
@@ -361,6 +363,16 @@ public function testGetUpdateForm(): void {
   }
 
   public function testGetUpdateFormDeniedWithForm(): void {
+    $profileJoin = Join::newWithBridge('JoinedEntity', 'j', 'INNER', 'bridge');
+    $this->profileMock->method('getJoins')
+      ->with('update', self::RESOLVED_CONTACT_ID)
+      ->willReturn([$profileJoin]);
+
+    $profileFilter = Comparison::new('extra', '!=', 'abc');
+    $this->profileMock->method('getFilter')
+      ->with('update', self::RESOLVED_CONTACT_ID)
+      ->willReturn($profileFilter);
+
     $actionMock = $this->createActionMock(RemoteGetUpdateFormAction::class);
     $actionMock->setId(12);
 
@@ -376,7 +388,8 @@ public function testGetUpdateFormDeniedWithForm(): void {
 
     $this->mockApi4Execute('Entity', 'get', [
       'select' => ['foo'],
-      'where' => [['id', '=', 12]],
+      'join' => [['JoinedEntity AS j', 'INNER', 'bridge']],
+      'where' => [['id', '=', 12], ['extra', '!=', 'abc']],
       'checkPermissions' => FALSE,
     ], new Result([$entityValues]));
 
@@ -410,6 +423,7 @@ public function testGetUpdateFormDenied(): void {
     $this->api4Mock->method('execute')
       ->with('Entity', 'get', [
         'select' => ['foo'],
+        'join' => [],
         'where' => [['id', '=', 12]],
         'checkPermissions' => FALSE,
       ])
@@ -439,6 +453,7 @@ public function testValidateUpdateForm(): void {
 
     $this->mockApi4Execute('Entity', 'get', [
       'select' => ['foo', 'bar'],
+      'join' => [],
       'where' => [['id', '=', 12]],
       'checkPermissions' => FALSE,
     ], new Result([$entityValues]));
@@ -490,6 +505,7 @@ public function testSubmitUpdateForm(): void {
 
     $this->mockApi4Execute('Entity', 'get', [
       'select' => ['foo', 'bar'],
+      'join' => [],
       'where' => [['id', '=', 12]],
       'checkPermissions' => FALSE,
     ], new Result([$entityValues]));

tests/phpunit/Civi/RemoteTools/EntityProfile/Helper/ProfileEntityLoaderTest.php

@@ -7,6 +7,7 @@
 use Civi\RemoteTools\Api4\Action\RemoteGetAction;
 use Civi\RemoteTools\Api4\Api4Interface;
 use Civi\RemoteTools\Api4\Query\Comparison;
+use Civi\RemoteTools\Api4\Query\Join;
 use Civi\RemoteTools\EntityProfile\RemoteEntityProfileInterface;
 use Civi\RemoteTools\Helper\SelectFactoryInterface;
 use Civi\RemoteTools\Helper\WhereFactoryInterface;
@@ -100,6 +101,12 @@ public function testGet(): void {
       ->with($entitySelect, 'get', $remoteSelect, $resolvedContactId)
       ->willReturn($profileEntitySelect);
 
+    $profileJoin = Join::newWithBridge('JoinedEntity', 'j', 'INNER', 'bridge');
+    $profileMock->method('getJoins')
+      ->with('get', $resolvedContactId)
+      ->willReturn([$profileJoin]);
+    $entityJoin = [['JoinedEntity AS j', 'INNER', 'bridge']];
+
     $profileFilter = Comparison::new('extra', '!=', 'abc');
     $profileMock->method('getFilter')
       ->with('get', $resolvedContactId)
@@ -135,6 +142,7 @@ public function testGet(): void {
           'get',
           [
             'select' => $profileEntitySelect,
+            'join' => $entityJoin,
             'where' => $entityWhereWithFilter,
             'orderBy' => ['foo' => 'DESC'],
             'limit' => 10,