BookStackApp · ssddanbrown · Oct 14, 2022 · Oct 2, 2022 · Oct 2, 2022 · Oct 2, 2022
diff --git a/app/Auth/Permissions/EntityPermission.php b/app/Auth/Permissions/EntityPermission.php
@@ -2,20 +2,41 @@
 
 namespace BookStack\Auth\Permissions;
 
+use BookStack\Auth\Role;
 use BookStack\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\Relations\MorphTo;
 
+/**
+ * @property int $id
+ * @property int $role_id
+ * @property int $entity_id
+ * @property string $entity_type
+ * @property boolean $view
+ * @property boolean $create
+ * @property boolean $update
+ * @property boolean $delete
+ */
 class EntityPermission extends Model
 {
-    protected $fillable = ['role_id', 'action'];
+    public const PERMISSIONS = ['view', 'create', 'update', 'delete'];
+
+    protected $fillable = ['role_id', 'view', 'create', 'update', 'delete'];
     public $timestamps = false;
 
     /**
-     * Get all this restriction's attached entity.
-     *
-     * @return \Illuminate\Database\Eloquent\Relations\MorphTo
+     * Get this restriction's attached entity.
      */
-    public function restrictable()
+    public function restrictable(): MorphTo
     {
         return $this->morphTo('restrictable');
     }
+
+    /**
+     * Get the role assigned to this entity permission.
+     */
+    public function role(): BelongsTo
+    {
+        return $this->belongsTo(Role::class);
+    }
 }
diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -40,7 +40,7 @@ public function rebuildForAll()
         });
 
         // Chunk through all bookshelves
-        Bookshelf::query()->withTrashed()->select(['id', 'restricted', 'owned_by'])
+        Bookshelf::query()->withTrashed()->select(['id', 'owned_by'])
             ->chunk(50, function (EloquentCollection $shelves) use ($roles) {
                 $this->createManyJointPermissions($shelves->all(), $roles);
             });
@@ -92,7 +92,7 @@ public function rebuildForRole(Role $role)
         });
 
         // Chunk through all bookshelves
-        Bookshelf::query()->select(['id', 'restricted', 'owned_by'])
+        Bookshelf::query()->select(['id', 'owned_by'])
             ->chunk(50, function ($shelves) use ($roles) {
                 $this->createManyJointPermissions($shelves->all(), $roles);
             });
@@ -138,12 +138,11 @@ protected function getChapter(int $chapterId): SimpleEntityData
     protected function bookFetchQuery(): Builder
     {
         return Book::query()->withTrashed()
-            ->select(['id', 'restricted', 'owned_by'])->with([
+            ->select(['id', 'owned_by'])->with([
                 'chapters' => function ($query) {
-                    $query->withTrashed()->select(['id', 'restricted', 'owned_by', 'book_id']);
                 },
                 'pages' => function ($query) {
-                    $query->withTrashed()->select(['id', 'restricted', 'owned_by', 'book_id', 'chapter_id']);
+                    $query->withTrashed()->select(['id', 'owned_by', 'book_id', 'chapter_id']);
                 },
             ]);
     }
@@ -218,7 +217,6 @@ protected function entitiesToSimpleEntities(array $entities): array
             $simple = new SimpleEntityData();
             $simple->id = $attrs['id'];
             $simple->type = $entity->getMorphClass();
-            $simple->restricted = boolval($attrs['restricted'] ?? 0);
             $simple->owned_by = $attrs['owned_by'] ?? 0;
             $simple->book_id = $attrs['book_id'] ?? null;
             $simple->chapter_id = $attrs['chapter_id'] ?? null;
@@ -240,21 +238,14 @@ protected function createManyJointPermissions(array $originalEntities, array $ro
         $this->readyEntityCache($entities);
         $jointPermissions = [];
 
-        // Create a mapping of entity restricted statuses
-        $entityRestrictedMap = [];
-        foreach ($entities as $entity) {
-            $entityRestrictedMap[$entity->type . ':' . $entity->id] = $entity->restricted;
-        }
-
         // Fetch related entity permissions
         $permissions = $this->getEntityPermissionsForEntities($entities);
 
         // Create a mapping of explicit entity permissions
         $permissionMap = [];
         foreach ($permissions as $permission) {
-            $key = $permission->restrictable_type . ':' . $permission->restrictable_id . ':' . $permission->role_id;
-            $isRestricted = $entityRestrictedMap[$permission->restrictable_type . ':' . $permission->restrictable_id];
-            $permissionMap[$key] = $isRestricted;
+            $key = $permission->entity_type . ':' . $permission->entity_id . ':' . $permission->role_id;
+            $permissionMap[$key] = $permission->view;
         }
 
         // Create a mapping of role permissions
@@ -319,11 +310,10 @@ protected function getEntityPermissionsForEntities(array $entities): array
     {
         $idsByType = $this->entitiesToTypeIdMap($entities);
         $permissionFetch = EntityPermission::query()
-            ->where('action', '=', 'view')
             ->where(function (Builder $query) use ($idsByType) {
                 foreach ($idsByType as $type => $ids) {
                     $query->orWhere(function (Builder $query) use ($type, $ids) {
-                        $query->where('restrictable_type', '=', $type)->whereIn('restrictable_id', $ids);
+                        $query->where('entity_type', '=', $type)->whereIn('entity_id', $ids);
                     });
                 }
             });
@@ -345,7 +335,7 @@ protected function createJointPermissionData(SimpleEntityData $entity, int $role
             return $this->createJointPermissionDataArray($entity, $roleId, true, true);
         }
 
-        if ($entity->restricted) {
+        if ($this->entityPermissionsActiveForRole($permissionMap, $entity, $roleId)) {
             $hasAccess = $this->mapHasActiveRestriction($permissionMap, $entity, $roleId);
 
             return $this->createJointPermissionDataArray($entity, $roleId, $hasAccess, $hasAccess);
@@ -358,13 +348,14 @@ protected function createJointPermissionData(SimpleEntityData $entity, int $role
         // 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 = !$book->restricted;
+        $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);
-            $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapter->restricted;
-            if ($chapter->restricted) {
+            $chapterRestricted = $this->entityPermissionsActiveForRole($permissionMap, $chapter, $roleId);
+            $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapterRestricted;
+            if ($chapterRestricted) {
                 $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $chapter, $roleId);
             }
         }
@@ -377,14 +368,25 @@ protected function createJointPermissionData(SimpleEntityData $entity, int $role
         );
     }
 
+    /**
+     * 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
     {
-        $key = $entity->type . ':' . $entity->id . ':' . $roleId;
+        $roleKey = $entity->type . ':' . $entity->id . ':' . $roleId;
+        $defaultKey = $entity->type . ':' . $entity->id . ':0';
 
-        return $entityMap[$key] ?? false;
+        return $entityMap[$roleKey] ?? $entityMap[$defaultKey] ?? false;
     }
 
     /**

diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
@@ -59,11 +59,15 @@ public function checkOwnableUserAccess(Model $ownable, string $permission): bool
      */
     protected function hasEntityPermission(Entity $entity, array $userRoleIds, string $action): ?bool
     {
+        $this->ensureValidEntityAction($action);
+
         $adminRoleId = Role::getSystemRole('admin')->id;
         if (in_array($adminRoleId, $userRoleIds)) {
             return true;
         }
 
+        // The chain order here is very important due to the fact we walk up the chain
+        // in the loop below. Earlier items in the chain have higher priority.
         $chain = [$entity];
         if ($entity instanceof Page && $entity->chapter_id) {
             $chain[] = $entity->chapter;
@@ -74,16 +78,26 @@ protected function hasEntityPermission(Entity $entity, array $userRoleIds, strin
         }
 
         foreach ($chain as $currentEntity) {
-            if (is_null($currentEntity->restricted)) {
-                throw new InvalidArgumentException('Entity restricted field used but has not been loaded');
+            $allowedByRoleId = $currentEntity->permissions()
+                ->whereIn('role_id', [0, ...$userRoleIds])
+                ->pluck($action, 'role_id');
+
+            // Continue up the chain if no applicable entity permission overrides.
+            if ($allowedByRoleId->isEmpty()) {
+                continue;
             }
 
-            if ($currentEntity->restricted) {
-                return $currentEntity->permissions()
-                    ->whereIn('role_id', $userRoleIds)
-                    ->where('action', '=', $action)
-                    ->count() > 0;
+            // If we have user-role-specific permissions set, allow if any of those
+            // role permissions allow access.
+            $hasDefault = $allowedByRoleId->has(0);
+            if (!$hasDefault || $allowedByRoleId->count() > 1) {
+                return $allowedByRoleId->search(function (bool $allowed, int $roleId) {
+                        return $roleId !== 0 && $allowed;
+                }) !== false;
             }
+
+            // Otherwise, return the default "Other roles" fallback value.
+            return $allowedByRoleId->get(0);
         }
 
         return null;
@@ -95,18 +109,16 @@ protected function hasEntityPermission(Entity $entity, array $userRoleIds, strin
      */
     public function checkUserHasEntityPermissionOnAny(string $action, string $entityClass = ''): bool
     {
-        if (strpos($action, '-') !== false) {
-            throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
-        }
+        $this->ensureValidEntityAction($action);
 
         $permissionQuery = EntityPermission::query()
-            ->where('action', '=', $action)
+            ->where($action, '=', true)
             ->whereIn('role_id', $this->getCurrentUserRoleIds());
 
         if (!empty($entityClass)) {
             /** @var Entity $entityInstance */
             $entityInstance = app()->make($entityClass);
-            $permissionQuery = $permissionQuery->where('restrictable_type', '=', $entityInstance->getMorphClass());
+            $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass());
         }
 
         $hasPermission = $permissionQuery->count() > 0;
@@ -255,4 +267,16 @@ protected function getCurrentUserRoleIds(): array
 
         return $this->currentUser()->roles->pluck('id')->values()->all();
     }
+
+    /**
+     * Ensure the given action is a valid and expected entity action.
+     * Throws an exception if invalid otherwise does nothing.
+     * @throws InvalidArgumentException
+     */
+    protected function ensureValidEntityAction(string $action): void
+    {
+        if (!in_array($action, EntityPermission::PERMISSIONS)) {
+            throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
+        }
+    }
 }
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
@@ -0,0 +1,68 @@
+<?php
+
+namespace BookStack\Auth\Permissions;
+
+use BookStack\Auth\Role;
+use BookStack\Entities\Models\Entity;
+
+class PermissionFormData
+{
+    protected Entity $entity;
+
+    public function __construct(Entity $entity)
+    {
+        $this->entity = $entity;
+    }
+
+    /**
+     * Get the permissions with assigned roles.
+     */
+    public function permissionsWithRoles(): array
+    {
+        return $this->entity->permissions()
+            ->with('role')
+            ->where('role_id', '!=', 0)
+            ->get()
+            ->sortBy('role.display_name')
+            ->all();
+    }
+
+    /**
+     * Get the roles that don't yet have specific permissions for the
+     * entity we're managing permissions for.
+     */
+    public function rolesNotAssigned(): array
+    {
+        $assigned = $this->entity->permissions()->pluck('role_id');
+        return Role::query()
+            ->where('system_name', '!=', 'admin')
+            ->whereNotIn('id', $assigned)
+            ->orderBy('display_name', 'asc')
+            ->get()
+            ->all();
+    }
+
+    /**
+     * Get the entity permission for the "Everyone Else" option.
+     */
+    public function everyoneElseEntityPermission(): EntityPermission
+    {
+        /** @var ?EntityPermission $permission */
+        $permission = $this->entity->permissions()
+            ->where('role_id', '=', 0)
+            ->first();
+        return $permission ?? (new EntityPermission());
+    }
+
+    /**
+     * Get the "Everyone Else" role entry.
+     */
+    public function everyoneElseRole(): Role
+    {
+        return (new Role())->forceFill([
+            'id' => 0,
+            'display_name' => trans('entities.permissions_role_everyone_else'),
+            'description' => trans('entities.permissions_role_everyone_else_desc'),
+        ]);
+    }
+}
diff --git a/app/Auth/Permissions/PermissionsRepo.php b/app/Auth/Permissions/PermissionsRepo.php
@@ -139,6 +139,7 @@ public function deleteRole($roleId, $migrateRoleId)
             }
         }
 
+        $role->entityPermissions()->delete();
         $role->jointPermissions()->delete();
         Activity::add(ActivityType::ROLE_DELETE, $role);
         $role->delete();

diff --git a/app/Auth/Permissions/SimpleEntityData.php b/app/Auth/Permissions/SimpleEntityData.php
@@ -6,7 +6,6 @@ class SimpleEntityData
 {
     public int $id;
     public string $type;
-    public bool $restricted;
     public int $owned_by;
     public ?int $book_id;
     public ?int $chapter_id;

diff --git a/app/Auth/Role.php b/app/Auth/Role.php
@@ -2,6 +2,7 @@
 
 namespace BookStack\Auth;
 
+use BookStack\Auth\Permissions\EntityPermission;
 use BookStack\Auth\Permissions\JointPermission;
 use BookStack\Auth\Permissions\RolePermission;
 use BookStack\Interfaces\Loggable;
@@ -54,6 +55,14 @@ public function permissions(): BelongsToMany
         return $this->belongsToMany(RolePermission::class, 'permission_role', 'role_id', 'permission_id');
     }
 
+    /**
+     * Get the entity permissions assigned to this role.
+     */
+    public function entityPermissions(): HasMany
+    {
+        return $this->hasMany(EntityPermission::class);
+    }
+
     /**
      * Check if this role has a permission.
      */
@@ -109,17 +118,6 @@ public static function visible(): Collection
         return static::query()->where('hidden', '=', false)->orderBy('name')->get();
     }
 
-    /**
-     * Get the roles that can be restricted.
-     */
-    public static function restrictable(): Collection
-    {
-        return static::query()
-            ->where('system_name', '!=', 'admin')
-            ->orderBy('display_name', 'asc')
-            ->get();
-    }
-
     /**
      * {@inheritdoc}
      */