From: Dan Brown <redacted>
Date: Mon, 23 Jan 2023 15:09:03 +0000 (+0000)
Subject: Shared entity permission logic across both query methods
X-Git-Tag: v23.01~1^2~13^2~6
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/91e613fe606777c0b036a2cfada94092b771dc22

Shared entity permission logic across both query methods

The runtime userCan() and the JointPermissionBuilder now share much of
the same logic for handling entity permission resolution.
---

diff --git a/app/Auth/Permissions/EntityPermissionEvaluator.php b/app/Auth/Permissions/EntityPermissionEvaluator.php
index 91596d02a..99e87d769 100644
--- a/app/Auth/Permissions/EntityPermissionEvaluator.php
+++ b/app/Auth/Permissions/EntityPermissionEvaluator.php
@@ -3,36 +3,36 @@
 namespace BookStack\Auth\Permissions;
 
 use BookStack\Auth\Role;
-use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Entity;
-use BookStack\Entities\Models\Page;
 use Illuminate\Database\Eloquent\Builder;
 
 class EntityPermissionEvaluator
 {
-    protected Entity $entity;
-    protected array $userRoleIds;
     protected string $action;
-    protected int $userId;
 
-    public function __construct(Entity $entity, int $userId, array $userRoleIds, string $action)
+    public function __construct(string $action)
     {
-        $this->entity = $entity;
-        $this->userId = $userId;
-        $this->userRoleIds = $userRoleIds;
         $this->action = $action;
     }
 
-    public function evaluate(): ?bool
+    public function evaluateEntityForUser(Entity $entity, array $userRoleIds): ?bool
     {
-        if ($this->isUserSystemAdmin()) {
+        if ($this->isUserSystemAdmin($userRoleIds)) {
             return true;
         }
 
-        $typeIdChain = $this->gatherEntityChainTypeIds();
-        $relevantPermissions = $this->getRelevantPermissionsMapByTypeId($typeIdChain);
+        $typeIdChain = $this->gatherEntityChainTypeIds(SimpleEntityData::fromEntity($entity));
+        $relevantPermissions = $this->getPermissionsMapByTypeId($typeIdChain, [...$userRoleIds, 0]);
         $permitsByType = $this->collapseAndCategorisePermissions($typeIdChain, $relevantPermissions);
 
+        return $this->evaluatePermitsByType($permitsByType);
+    }
+
+    /**
+     * @param array<string, array<string, int>> $permitsByType
+     */
+    protected function evaluatePermitsByType(array $permitsByType): ?bool
+    {
         // Return grant or reject from role-level if exists
         if (count($permitsByType['role']) > 0) {
             return boolval(max($permitsByType['role']));
@@ -73,21 +73,25 @@ class EntityPermissionEvaluator
      * @param string[] $typeIdChain
      * @return array<string, EntityPermission[]>
      */
-    protected function getRelevantPermissionsMapByTypeId(array $typeIdChain): array
+    protected function getPermissionsMapByTypeId(array $typeIdChain, array $filterRoleIds): array
     {
-        $relevantPermissions = EntityPermission::query()
-            ->where(function (Builder $query) use ($typeIdChain) {
-                foreach ($typeIdChain as $typeId) {
-                    $query->orWhere(function (Builder $query) use ($typeId) {
-                        [$type, $id] = explode(':', $typeId);
-                        $query->where('entity_type', '=', $type)
-                            ->where('entity_id', '=', $id);
-                    });
-                }
-            })->where(function (Builder $query) {
-                $query->whereIn('role_id', [...$this->userRoleIds, 0]);
-            })->get(['entity_id', 'entity_type', 'role_id', $this->action])
-            ->all();
+        $query = EntityPermission::query()->where(function (Builder $query) use ($typeIdChain) {
+            foreach ($typeIdChain as $typeId) {
+                $query->orWhere(function (Builder $query) use ($typeId) {
+                    [$type, $id] = explode(':', $typeId);
+                    $query->where('entity_type', '=', $type)
+                        ->where('entity_id', '=', $id);
+                });
+            }
+        });
+
+        if (!empty($filterRoleIds)) {
+            $query->where(function (Builder $query) use ($filterRoleIds) {
+                $query->whereIn('role_id', [...$filterRoleIds, 0]);
+            });
+        }
+
+        $relevantPermissions = $query->get(['entity_id', 'entity_type', 'role_id', $this->action])->all();
 
         $map = [];
         foreach ($relevantPermissions as $permission) {
@@ -105,27 +109,27 @@ class EntityPermissionEvaluator
     /**
      * @return string[]
      */
-    protected function gatherEntityChainTypeIds(): array
+    protected function gatherEntityChainTypeIds(SimpleEntityData $entity): array
     {
         // The array order here is very important due to the fact we walk up the chain
         // elsewhere in the class. Earlier items in the chain have higher priority.
 
-        $chain = [$this->entity->getMorphClass() . ':' . $this->entity->id];
+        $chain = [$entity->type . ':' . $entity->id];
 
-        if ($this->entity instanceof Page && $this->entity->chapter_id) {
-            $chain[] = 'chapter:' . $this->entity->chapter_id;
+        if ($entity->type === 'page' && $entity->chapter_id) {
+            $chain[] = 'chapter:' . $entity->chapter_id;
         }
 
-        if ($this->entity instanceof Page || $this->entity instanceof Chapter) {
-            $chain[] = 'book:' . $this->entity->book_id;
+        if ($entity->type === 'page' || $entity->type === 'chapter') {
+            $chain[] = 'book:' . $entity->book_id;
         }
 
         return $chain;
     }
 
-    protected function isUserSystemAdmin(): bool
+    protected function isUserSystemAdmin($userRoleIds): bool
     {
         $adminRoleId = Role::getSystemRole('admin')->id;
-        return in_array($adminRoleId, $this->userRoleIds);
+        return in_array($adminRoleId, $userRoleIds);
     }
 }
diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
index 114cff619..91207e3ab 100644
--- a/app/Auth/Permissions/JointPermissionBuilder.php
+++ b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -19,11 +19,6 @@ use Illuminate\Support\Facades\DB;
  */
 class JointPermissionBuilder
 {
-    /**
-     * @var array<string, array<int, SimpleEntityData>>
-     */
-    protected array $entityCache;
-
     /**
      * Re-generate all entity permission from scratch.
      */
@@ -98,40 +93,6 @@ class JointPermissionBuilder
             });
     }
 
-    /**
-     * Prepare the local entity cache and ensure it's empty.
-     *
-     * @param SimpleEntityData[] $entities
-     */
-    protected function readyEntityCache(array $entities)
-    {
-        $this->entityCache = [];
-
-        foreach ($entities as $entity) {
-            if (!isset($this->entityCache[$entity->type])) {
-                $this->entityCache[$entity->type] = [];
-            }
-
-            $this->entityCache[$entity->type][$entity->id] = $entity;
-        }
-    }
-
-    /**
-     * Get a book via ID, Checks local cache.
-     */
-    protected function getBook(int $bookId): SimpleEntityData
-    {
-        return $this->entityCache['book'][$bookId];
-    }
-
-    /**
-     * Get a chapter via ID, Checks local cache.
-     */
-    protected function getChapter(int $chapterId): SimpleEntityData
-    {
-        return $this->entityCache['chapter'][$chapterId];
-    }
-
     /**
      * Get a query for fetching a book with its children.
      */
@@ -214,13 +175,7 @@ class JointPermissionBuilder
         $simpleEntities = [];
 
         foreach ($entities as $entity) {
-            $attrs = $entity->getAttributes();
-            $simple = new SimpleEntityData();
-            $simple->id = $attrs['id'];
-            $simple->type = $entity->getMorphClass();
-            $simple->owned_by = $attrs['owned_by'] ?? 0;
-            $simple->book_id = $attrs['book_id'] ?? null;
-            $simple->chapter_id = $attrs['chapter_id'] ?? null;
+            $simple = SimpleEntityData::fromEntity($entity);
             $simpleEntities[] = $simple;
         }
 
@@ -236,18 +191,10 @@ class JointPermissionBuilder
     protected function createManyJointPermissions(array $originalEntities, array $roles)
     {
         $entities = $this->entitiesToSimpleEntities($originalEntities);
-        $this->readyEntityCache($entities);
         $jointPermissions = [];
 
         // Fetch related entity permissions
-        $permissions = $this->getEntityPermissionsForEntities($entities);
-
-        // Create a mapping of explicit entity permissions
-        $permissionMap = [];
-        foreach ($permissions as $permission) {
-            $key = $permission->entity_type . ':' . $permission->entity_id . ':' . $permission->role_id;
-            $permissionMap[$key] = $permission->view;
-        }
+        $permissions = new MassEntityPermissionEvaluator($entities, 'view');
 
         // Create a mapping of role permissions
         $rolePermissionMap = [];
@@ -260,13 +207,14 @@ class JointPermissionBuilder
         // Create Joint Permission Data
         foreach ($entities as $entity) {
             foreach ($roles as $role) {
-                $jointPermissions[] = $this->createJointPermissionData(
+                $jp = $this->createJointPermissionData(
                     $entity,
                     $role->getRawAttribute('id'),
-                    $permissionMap,
+                    $permissions,
                     $rolePermissionMap,
                     $role->system_name === 'admin'
                 );
+                $jointPermissions[] = $jp;
             }
         }
 
@@ -300,94 +248,28 @@ class JointPermissionBuilder
         return $idsByType;
     }
 
-    /**
-     * Get the entity permissions for all the given entities.
-     *
-     * @param SimpleEntityData[] $entities
-     *
-     * @return EntityPermission[]
-     */
-    protected function getEntityPermissionsForEntities(array $entities): array
-    {
-        $idsByType = $this->entitiesToTypeIdMap($entities);
-        $permissionFetch = EntityPermission::query()
-            ->where(function (Builder $query) use ($idsByType) {
-                foreach ($idsByType as $type => $ids) {
-                    $query->orWhere(function (Builder $query) use ($type, $ids) {
-                        $query->where('entity_type', '=', $type)->whereIn('entity_id', $ids);
-                    });
-                }
-            });
-
-        return $permissionFetch->get()->all();
-    }
-
     /**
      * Create entity permission data for an entity and role
      * for a particular action.
      */
-    protected function createJointPermissionData(SimpleEntityData $entity, int $roleId, array $permissionMap, array $rolePermissionMap, bool $isAdminRole): array
+    protected function createJointPermissionData(SimpleEntityData $entity, int $roleId, MassEntityPermissionEvaluator $permissionMap, array $rolePermissionMap, bool $isAdminRole): array
     {
-        $permissionPrefix = $entity->type . '-view';
-        $roleHasPermission = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-all']);
-        $roleHasPermissionOwn = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-own']);
-
+        // Ensure system admin role retains permissions
         if ($isAdminRole) {
             return $this->createJointPermissionDataArray($entity, $roleId, true, true);
         }
 
-        if ($this->entityPermissionsActiveForRole($permissionMap, $entity, $roleId)) {
-            $hasAccess = $this->mapHasActiveRestriction($permissionMap, $entity, $roleId);
-
-            return $this->createJointPermissionDataArray($entity, $roleId, $hasAccess, $hasAccess);
-        }
-
-        if ($entity->type === 'book' || $entity->type === 'bookshelf') {
-            return $this->createJointPermissionDataArray($entity, $roleId, $roleHasPermission, $roleHasPermissionOwn);
+        // Return evaluated entity permission status if it has an affect.
+        $entityPermissionStatus = $permissionMap->evaluateEntityForRole($entity, $roleId);
+        if ($entityPermissionStatus !== null) {
+            return $this->createJointPermissionDataArray($entity, $roleId, $entityPermissionStatus, $entityPermissionStatus);
         }
 
-        // For chapters and pages, Check if explicit permissions are set on the Book.
-        $book = $this->getBook($entity->book_id);
-        $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $book, $roleId);
-        $hasPermissiveAccessToParents = !$this->entityPermissionsActiveForRole($permissionMap, $book, $roleId);
-
-        // For pages with a chapter, Check if explicit permissions are set on the Chapter
-        if ($entity->type === 'page' && $entity->chapter_id !== 0) {
-            $chapter = $this->getChapter($entity->chapter_id);
-            $chapterRestricted = $this->entityPermissionsActiveForRole($permissionMap, $chapter, $roleId);
-            $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapterRestricted;
-            if ($chapterRestricted) {
-                $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $chapter, $roleId);
-            }
-        }
-
-        return $this->createJointPermissionDataArray(
-            $entity,
-            $roleId,
-            ($hasExplicitAccessToParents || ($roleHasPermission && $hasPermissiveAccessToParents)),
-            ($hasExplicitAccessToParents || ($roleHasPermissionOwn && $hasPermissiveAccessToParents))
-        );
-    }
-
-    /**
-     * Check if entity permissions are defined within the given map, for the given entity and role.
-     * Checks for the default `role_id=0` backup option as a fallback.
-     */
-    protected function entityPermissionsActiveForRole(array $permissionMap, SimpleEntityData $entity, int $roleId): bool
-    {
-        $keyPrefix = $entity->type . ':' . $entity->id . ':';
-        return isset($permissionMap[$keyPrefix . $roleId]) || isset($permissionMap[$keyPrefix . '0']);
-    }
-
-    /**
-     * Check for an active restriction in an entity map.
-     */
-    protected function mapHasActiveRestriction(array $entityMap, SimpleEntityData $entity, int $roleId): bool
-    {
-        $roleKey = $entity->type . ':' . $entity->id . ':' . $roleId;
-        $defaultKey = $entity->type . ':' . $entity->id . ':0';
-
-        return $entityMap[$roleKey] ?? $entityMap[$defaultKey] ?? false;
+        // Otherwise default to the role-level permissions
+        $permissionPrefix = $entity->type . '-view';
+        $roleHasPermission = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-all']);
+        $roleHasPermissionOwn = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-own']);
+        return $this->createJointPermissionDataArray($entity, $roleId, $roleHasPermission, $roleHasPermissionOwn);
     }
 
     /**
diff --git a/app/Auth/Permissions/MassEntityPermissionEvaluator.php b/app/Auth/Permissions/MassEntityPermissionEvaluator.php
new file mode 100644
index 000000000..1bd2ec44a
--- /dev/null
+++ b/app/Auth/Permissions/MassEntityPermissionEvaluator.php
@@ -0,0 +1,81 @@
+<?php
+
+namespace BookStack\Auth\Permissions;
+
+class MassEntityPermissionEvaluator extends EntityPermissionEvaluator
+{
+    /**
+     * @var SimpleEntityData[]
+     */
+    protected array $entitiesInvolved;
+    protected array $permissionMapCache;
+
+    public function __construct(array $entitiesInvolved, string $action)
+    {
+        $this->entitiesInvolved = $entitiesInvolved;
+        parent::__construct($action);
+    }
+
+    public function evaluateEntityForRole(SimpleEntityData $entity, int $roleId): ?bool
+    {
+        $typeIdChain = $this->gatherEntityChainTypeIds($entity);
+        $relevantPermissions = $this->getPermissionMapByTypeIdForChainAndRole($typeIdChain, $roleId);
+        $permitsByType = $this->collapseAndCategorisePermissions($typeIdChain, $relevantPermissions);
+
+        return $this->evaluatePermitsByType($permitsByType);
+    }
+
+    /**
+     * @param string[] $typeIdChain
+     * @return array<string, EntityPermission[]>
+     */
+    protected function getPermissionMapByTypeIdForChainAndRole(array $typeIdChain, int $roleId): array
+    {
+        $allPermissions = $this->getPermissionMapByTypeIdAndRoleForAllInvolved();
+        $relevantPermissions = [];
+
+        // Filter down permissions to just those for current typeId
+        // and current roleID or fallback permissions.
+        foreach ($typeIdChain as $typeId) {
+            $relevantPermissions[$typeId] = [
+                ...($allPermissions[$typeId][$roleId] ?? []),
+                ...($allPermissions[$typeId][0] ?? [])
+            ];
+        }
+
+        return $relevantPermissions;
+    }
+
+    /**
+     * @return array<string, array<int, EntityPermission[]>>
+     */
+    protected function getPermissionMapByTypeIdAndRoleForAllInvolved(): array
+    {
+        if (isset($this->permissionMapCache)) {
+            return $this->permissionMapCache;
+        }
+
+        $entityTypeIdChain = [];
+        foreach ($this->entitiesInvolved as $entity) {
+            $entityTypeIdChain[] = $entity->type . ':' . $entity->id;
+        }
+
+        $permissionMap = $this->getPermissionsMapByTypeId($entityTypeIdChain, []);
+
+       // Manipulate permission map to also be keyed by roleId.
+        foreach ($permissionMap as $typeId => $permissions) {
+            $permissionMap[$typeId] = [];
+            foreach ($permissions as $permission) {
+                $roleId = $permission->getRawAttribute('role_id');
+                if (!isset($permissionMap[$typeId][$roleId])) {
+                    $permissionMap[$typeId][$roleId] = [];
+                }
+                $permissionMap[$typeId][$roleId][] = $permission;
+            }
+        }
+
+        $this->permissionMapCache = $permissionMap;
+
+        return $this->permissionMapCache;
+    }
+}
diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 3855a283b..5326cc340 100644
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -47,7 +47,7 @@ class PermissionApplicator
             return $hasRolePermission;
         }
 
-        $hasApplicableEntityPermissions = $this->hasEntityPermission($ownable, $user->id, $userRoleIds, $action);
+        $hasApplicableEntityPermissions = $this->hasEntityPermission($ownable, $userRoleIds, $action);
 
         return is_null($hasApplicableEntityPermissions) ? $hasRolePermission : $hasApplicableEntityPermissions;
     }
@@ -56,11 +56,11 @@ class PermissionApplicator
      * Check if there are permissions that are applicable for the given entity item, action and roles.
      * Returns null when no entity permissions are in force.
      */
-    protected function hasEntityPermission(Entity $entity, int $userId, array $userRoleIds, string $action): ?bool
+    protected function hasEntityPermission(Entity $entity, array $userRoleIds, string $action): ?bool
     {
         $this->ensureValidEntityAction($action);
 
-        return (new EntityPermissionEvaluator($entity, $userId, $userRoleIds, $action))->evaluate();
+        return (new EntityPermissionEvaluator($action))->evaluateEntityForUser($entity, $userRoleIds);
     }
 
     /**
diff --git a/app/Auth/Permissions/SimpleEntityData.php b/app/Auth/Permissions/SimpleEntityData.php
index 62f5984f8..2128451fe 100644
--- a/app/Auth/Permissions/SimpleEntityData.php
+++ b/app/Auth/Permissions/SimpleEntityData.php
@@ -2,6 +2,8 @@
 
 namespace BookStack\Auth\Permissions;
 
+use BookStack\Entities\Models\Entity;
+
 class SimpleEntityData
 {
     public int $id;
@@ -9,4 +11,18 @@ class SimpleEntityData
     public int $owned_by;
     public ?int $book_id;
     public ?int $chapter_id;
+
+    public static function fromEntity(Entity $entity): self
+    {
+        $attrs = $entity->getAttributes();
+        $simple = new self();
+
+        $simple->id = $attrs['id'];
+        $simple->type = $entity->getMorphClass();
+        $simple->owned_by = $attrs['owned_by'] ?? 0;
+        $simple->book_id = $attrs['book_id'] ?? null;
+        $simple->chapter_id = $attrs['chapter_id'] ?? null;
+
+        return $simple;
+    }
 }