Merge pull request #3632 from BookStackApp/ownable_permission_fix

Fixed failed permission checks due to non-loaded fields

app/Auth/Permissions/PermissionApplicator.php

@@ -34,7 +34,13 @@ class PermissionApplicator
         $ownRolePermission = $user->can($fullPermission . '-own');
         $nonJointPermissions = ['restrictions', 'image', 'attachment', 'comment'];
         $ownerField = ($ownable instanceof Entity) ? 'owned_by' : 'created_by';
-        $isOwner = $user->id === $ownable->getAttribute($ownerField);
+        $ownableFieldVal = $ownable->getAttribute($ownerField);
+
+        if (is_null($ownableFieldVal)) {
+            throw new InvalidArgumentException("{$ownerField} field used but has not been loaded");
+        }
+
+        $isOwner = $user->id === $ownableFieldVal;
         $hasRolePermission = $allRolePermission || ($isOwner && $ownRolePermission);
 
         // Handle non entity specific jointPermissions
@@ -68,6 +74,11 @@ class PermissionApplicator
         }
 
         foreach ($chain as $currentEntity) {
+
+            if (is_null($currentEntity->restricted)) {
+                throw new InvalidArgumentException("Entity restricted field used but has not been loaded");
+            }
+
             if ($currentEntity->restricted) {
                 return $currentEntity->permissions()
                     ->whereIn('role_id', $userRoleIds)

app/Entities/Repos/BaseRepo.php

@@ -38,6 +38,7 @@ class BaseRepo
             $this->tagRepo->saveTagsToEntity($entity, $input['tags']);
         }
 
+        $entity->refresh();
         $entity->rebuildPermissions();
         $entity->indexForSearch();
     }

app/Entities/Repos/BookshelfRepo.php

@@ -140,7 +140,7 @@ class BookshelfRepo
     public function copyDownPermissions(Bookshelf $shelf, $checkUserPermissions = true): int
     {
         $shelfPermissions = $shelf->permissions()->get(['role_id', 'action'])->toArray();
-        $shelfBooks = $shelf->books()->get(['id', 'restricted']);
+        $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
         $updatedBookCount = 0;
 
         /** @var Book $book */

app/Entities/Tools/SearchRunner.php

@@ -163,7 +163,7 @@ class SearchRunner
         $entityQuery = $entityModelInstance->newQuery()->scopes('visible');
 
         if ($entityModelInstance instanceof Page) {
-            $entityQuery->select($entityModelInstance::$listAttributes);
+            $entityQuery->select(array_merge($entityModelInstance::$listAttributes, ['restricted', 'owned_by']));
         } else {
             $entityQuery->select(['*']);
         }

app/Http/Controllers/FavouriteController.php

@@ -87,7 +87,7 @@ class FavouriteController extends Controller
 
         $modelInstance = $model->newQuery()
             ->where('id', '=', $modelInfo['id'])
-            ->first(['id', 'name']);
+            ->first(['id', 'name', 'restricted', 'owned_by']);
 
         $inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance));
         if (is_null($modelInstance) || $inaccessibleEntity) {

tests/FavouriteTest.php

@@ -1,11 +1,11 @@
-<?php
+<?php namespace Tests;
 
 use BookStack\Actions\Favourite;
+use BookStack\Auth\User;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
-use Tests\TestCase;
 
 class FavouriteTest extends TestCase
 {
@@ -58,6 +58,30 @@ class FavouriteTest extends TestCase
         ]);
     }
 
+    public function test_favourite_flow_with_own_permissions()
+    {
+        /** @var Book $book */
+        $book = Book::query()->first();
+        $user = User::factory()->create();
+        $book->owned_by = $user->id;
+        $book->save();
+
+        $this->giveUserPermissions($user, ['book-view-own']);
+
+        $this->actingAs($user)->get($book->getUrl());
+        $resp = $this->post('/favourites/add', [
+            'type' => get_class($book),
+            'id'   => $book->id,
+        ]);
+        $resp->assertRedirect($book->getUrl());
+
+        $this->assertDatabaseHas('favourites', [
+            'user_id'           => $user->id,
+            'favouritable_type' => $book->getMorphClass(),
+            'favouritable_id'   => $book->id,
+        ]);
+    }
+
     public function test_book_chapter_shelf_pages_contain_favourite_button()
     {
         $entities = [