Fixed a couple of non-intended logical permission issues

Both caught in tests:
Fixed loss of permissions for admin users when entity restrictions were
active, since there are no entity-restrictions for the admin role but
we'd force generate them in joint permissions, which would be queried.
Fixed new role permission checks when permissions given with only the
action (eg. 'view'), since the type prefix would be required for role
permission checks. Was previously not needed as only the simpler form
was used in the jointpermissions after merge & calculation.

diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 91a7c72ae2bb439201939d01ab314dddb5b38ca6..4b648532a5bb43f9a81a4469d315d8ed9db7a49a 100644 (file)
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -25,11 +25,13 @@ class PermissionApplicator
     {
         $explodedPermission = explode('-', $permission);
         $action = $explodedPermission[1] ?? $explodedPermission[0];
+        $fullPermission = count($explodedPermission) > 1 ? $permission : $ownable->getMorphClass() . '-' . $permission;
+
         $user = $this->currentUser();
         $userRoleIds = $this->getCurrentUserRoleIds();
 
-        $allRolePermission = $user->can($permission . '-all');
-        $ownRolePermission = $user->can($permission . '-own');
+        $allRolePermission = $user->can($fullPermission . '-all');
+        $ownRolePermission = $user->can($fullPermission . '-own');
         $nonJointPermissions = ['restrictions', 'image', 'attachment', 'comment'];
         $ownerField = ($ownable instanceof Entity) ? 'owned_by' : 'created_by';
         $isOwner = $user->id === $ownable->getAttribute($ownerField);
@@ -40,23 +42,22 @@ class PermissionApplicator
             return $hasRolePermission;
         }
 
-        $entityPermissions = $this->getApplicableEntityPermissions($ownable, $userRoleIds, $action);
-        if (is_null($entityPermissions)) {
-            return $hasRolePermission;
-        }
+        $hasApplicableEntityPermissions = $this->hasEntityPermission($ownable, $userRoleIds, $action);
 
-        return count($entityPermissions) > 0;
+        return is_null($hasApplicableEntityPermissions) ? $hasRolePermission : $hasApplicableEntityPermissions;
     }
 
     /**
-     * Get the permissions that are applicable for the given entity item.
-     * Returns null when no entity permissions apply otherwise entity permissions
-     * are active, even if the returned array is empty.
-     *
-     * @returns EntityPermission[]
+     * 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 getApplicableEntityPermissions(Entity $entity, array $userRoleIds, string $action): ?array
+    protected function hasEntityPermission(Entity $entity, array $userRoleIds, string $action): ?bool
     {
+        $adminRoleId = Role::getSystemRole('admin')->id;
+        if (in_array($adminRoleId, $userRoleIds)) {
+            return true;
+        }
+
         $chain = [$entity];
         if ($entity instanceof Page && $entity->chapter_id) {
             $chain[] = $entity->chapter;
@@ -71,8 +72,7 @@ class PermissionApplicator
                 return $currentEntity->permissions()
                     ->whereIn('role_id', $userRoleIds)
                     ->where('action', '=', $action)
-                    ->get()
-                    ->all();
+                    ->count() > 0;
             }
         }
 

diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php
index b9ebab92ef88da243a9ef069f35cce0e5ddd0e14..b2dab252a05decab60f407b0fe68541b92f9063e 100644 (file)
--- a/app/Entities/Models/Bookshelf.php
+++ b/app/Entities/Models/Bookshelf.php
@@ -91,10 +91,6 @@ class Bookshelf extends Entity implements HasCoverImage
 
     /**
      * Check if this shelf contains the given book.
-     *
-     * @param Book $book
-     *
-     * @return bool
      */
     public function contains(Book $book): bool
     {
@@ -103,8 +99,6 @@ class Bookshelf extends Entity implements HasCoverImage
 
     /**
      * Add a book to the end of this shelf.
-     *
-     * @param Book $book
      */
     public function appendBook(Book $book)
     {

diff --git a/app/Entities/Tools/ShelfContext.php b/app/Entities/Tools/ShelfContext.php
index 50d7981716e68c7dd662e982e22b115d64a0b4f2..50c7047d9e39607c2ba3459fc5e6e845a33dbc13 100644 (file)
--- a/app/Entities/Tools/ShelfContext.php
+++ b/app/Entities/Tools/ShelfContext.php
@@ -20,6 +20,7 @@ class ShelfContext
             return null;
         }
 
+        /** @var Bookshelf $shelf */
         $shelf = Bookshelf::visible()->find($contextBookshelfId);
         $shelfContainsBook = $shelf && $shelf->contains($book);
 

diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php
index 18adaa62756d55d9ec904078d9680fd098267913..feb581c780579d470e5bf1437ac491f0591ebb9f 100644 (file)
--- a/app/Http/Controllers/BookshelfController.php
+++ b/app/Http/Controllers/BookshelfController.php
@@ -104,7 +104,7 @@ class BookshelfController extends Controller
     public function show(ActivityQueries $activities, string $slug)
     {
         $shelf = $this->bookshelfRepo->getBySlug($slug);
-        $this->checkOwnablePermission('book-view', $shelf);
+        $this->checkOwnablePermission('bookshelf-view', $shelf);
 
         $sort = setting()->getForCurrentUser('shelf_books_sort', 'default');
         $order = setting()->getForCurrentUser('shelf_books_sort_order', 'asc');