Fixed failed permission checks due to non-loaded fields

Added additional exceptions to prevent such cases in the future, so
that they are caught in dev ideally.
Added test case specifically for reported favourite scenario.
This commit is contained in:
Dan Brown 2022-08-10 08:06:48 +01:00
parent 219da9da9b
commit 16eedc8264
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
6 changed files with 42 additions and 6 deletions

View File

@ -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)

View File

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

View File

@ -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 */

View File

@ -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(['*']);
}

View File

@ -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) {

View File

@ -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 = [