archive/BookStackApp_BookStack
0
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-04-28 05:42:28 +00:00
Code Issues Projects Releases Wiki Activity

Altered sort permission checking and started tests

Browse source 
Previous implemenations were hard to read so changing to be more
logically simplistic. Still needs further coverage in tests and
review/alignment of permissions to use.
This commit is contained in:
Dan Brown 2022-01-05 14:39:21 +00:00
parent d8c45f5746
commit 553954ad18
No known key found for this signature in database
GPG key ID: 46D9F943C24A2EF9
2 changed files with 136 additions and 26 deletions
app/Entities/Tools
BookContents.php
tests/Entity
SortTest.php

75
app/Entities/Tools/BookContents.php
View file

@ -116,8 +116,17 @@ class BookContents
// Load models into map
$modelMap = $this->loadModelsFromSortMap($sortMap);
// Sort our changes from our map to be chapters first
// Since they need to be process to ensure book alignment for child page changes.
$sortMapItems = $sortMap->all();
usort($sortMapItems, function(BookSortMapItem $itemA, BookSortMapItem $itemB) {
$aScore = $itemA->type === 'page' ? 2 : 1;
$bScore = $itemB->type === 'page' ? 2 : 1;
return $aScore - $bScore;
});
// Perform the sort
foreach ($sortMap->all() as $item) {
foreach ($sortMapItems as $item) {
$this->applySortUpdates($item, $modelMap);
}
@ -163,32 +172,23 @@ class BookContents
$currentParentKey = 'chapter:' . $model->chapter_id;
}
$currentParent = $modelMap[$currentParentKey];
$currentParent = $modelMap[$currentParentKey] ?? null;
/** @var Book $newBook */
$newBook = $modelMap['book:' . $sortMapItem->parentBookId] ?? null;
$newBook = $modelMap['book:' . $sortMapItem->parentBookId];
/** @var ?Chapter $newChapter */
$newChapter = $sortMapItem->parentChapterId ? ($modelMap['chapter:' . $sortMapItem->parentChapterId] ?? null) : null;
// Check permissions of our changes to be made
if (!$currentParent || !$newBook) {
return;
} else if (!userCan('chapter-update', $currentParent) && !userCan('book-update', $currentParent)) {
return;
} else if ($bookChanged && !$newChapter && !userCan('book-update', $newBook)) {
return;
} else if ($newChapter && !userCan('chapter-update', $newChapter)) {
return;
} else if (($chapterChanged || $bookChanged) && $newChapter && $newBook->id !== $newChapter->book_id) {
if (!$this->isSortChangePermissible($sortMapItem, $model, $currentParent, $newBook, $newChapter)) {
return;
}
// Action the required changes
if ($bookChanged) {
$model->changeBook($sortMapItem->parentBookId);
$model->changeBook($newBook->id);
}
if ($chapterChanged) {
$model->chapter_id = $sortMapItem->parentChapterId ?? 0;
$model->chapter_id = $newChapter->id ?? 0;
}
if ($priorityChanged) {
@ -200,6 +200,51 @@ class BookContents
}
}
/**
* Check if the current user has permissions to apply the given sorting change.
*/
protected function isSortChangePermissible(BookSortMapItem $sortMapItem, Entity $model, ?Entity $currentParent, ?Entity $newBook, ?Entity $newChapter): bool
{
// TODO - Move operations check for create permissions, Needs these also/instead?
// Stop if we can't see the current parent or new book.
if (!$currentParent || !$newBook) {
return false;
}
if ($model instanceof Chapter) {
$hasPermission = userCan('book-update', $currentParent)
&& userCan('book-update', $newBook);
if (!$hasPermission) {
return false;
}
}
if ($model instanceof Page) {
$parentPermission = ($currentParent instanceof Chapter) ? 'chapter-update' : 'book-update';
$hasCurrentParentPermission = userCan($parentPermission, $currentParent);
// This needs to check if there was an intended chapter location in the original sort map
// rather than inferring from the $newChapter since that variable may be null
// due to other reasons (Visibility).
$newParent = $sortMapItem->parentChapterId ? $newChapter : $newBook;
if (!$newParent) {
return false;
}
$newParentInRightLocation = ($newParent instanceof Book || $newParent->book_id === $newBook->id);
$newParentPermission = ($newParent instanceof Chapter) ? 'chapter-update' : 'book-update';
$hasNewParentPermission = userCan($newParentPermission, $newParent);
$hasPermission = $hasCurrentParentPermission && $newParentInRightLocation && $hasNewParentPermission;
if (!$hasPermission) {
return false;
}
}
return true;
}
/**
* Load models from the database into the given sort map.
* @return array<string, Entity>

87
tests/Entity/SortTest.php
View file

@ -239,20 +239,20 @@ class SortTest extends TestCase
// Create request data
$reqData = [
[
'id' => $chapterToMove->id,
'sort' => 0,
'id' => $chapterToMove->id,
'sort' => 0,
'parentChapter' => false,
'type' => 'chapter',
'book' => $newBook->id,
'type' => 'chapter',
'book' => $newBook->id,
],
];
foreach ($pagesToMove as $index => $page) {
$reqData[] = [
'id' => $page->id,
'sort' => $index,
'id' => $page->id,
'sort' => $index,
'parentChapter' => $index === count($pagesToMove) - 1 ? $chapterToMove->id : false,
'type' => 'page',
'book' => $newBook->id,
'type' => 'page',
'book' => $newBook->id,
];
}
@ -260,18 +260,83 @@ class SortTest extends TestCase
$sortResp->assertRedirect($newBook->getUrl());
$sortResp->assertStatus(302);
$this->assertDatabaseHas('chapters', [
'id' => $chapterToMove->id,
'book_id' => $newBook->id,
'id' => $chapterToMove->id,
'book_id' => $newBook->id,
'priority' => 0,
]);
$this->assertTrue($newBook->chapters()->count() === 1);
$this->assertTrue($newBook->chapters()->first()->pages()->count() === 1);
$checkPage = $pagesToMove[1];
$checkResp = $this->get(Page::find($checkPage->id)->getUrl());
$checkResp = $this->get($checkPage->refresh()->getUrl());
$checkResp->assertSee($newBook->name);
}
public function test_book_sort_makes_no_changes_if_new_chapter_does_not_align_with_new_book()
{
/** @var Page $page */
$page = Page::query()->where('chapter_id', '!=', 0)->first();
$otherChapter = Chapter::query()->where('book_id', '!=', $page->book_id)->first();
$sortData = [
'id' => $page->id,
'sort' => 0,
'parentChapter' => $otherChapter->id,
'type' => 'page',
'book' => $page->book_id,
];
$this->asEditor()->put($page->book->getUrl('/sort'), ['sort-tree' => json_encode([$sortData])])->assertRedirect();
$this->assertDatabaseHas('pages', [
'id' => $page->id, 'chapter_id' => $page->chapter_id, 'book_id' => $page->book_id,
]);
}
public function test_book_sort_makes_no_changes_if_no_view_permissions_on_new_chapter()
{
/** @var Page $page */
$page = Page::query()->where('chapter_id', '!=', 0)->first();
/** @var Chapter $otherChapter */
$otherChapter = Chapter::query()->where('book_id', '!=', $page->book_id)->first();
$this->setEntityRestrictions($otherChapter);
$sortData = [
'id' => $page->id,
'sort' => 0,
'parentChapter' => $otherChapter->id,
'type' => 'page',
'book' => $otherChapter->book_id,
];
$this->asEditor()->put($page->book->getUrl('/sort'), ['sort-tree' => json_encode([$sortData])])->assertRedirect();
$this->assertDatabaseHas('pages', [
'id' => $page->id, 'chapter_id' => $page->chapter_id, 'book_id' => $page->book_id,
]);
}
public function test_book_sort_makes_no_changes_if_no_update_permissions_on_new_chapter()
{
/** @var Page $page */
$page = Page::query()->where('chapter_id', '!=', 0)->first();
/** @var Chapter $otherChapter */
$otherChapter = Chapter::query()->where('book_id', '!=', $page->book_id)->first();
$editor = $this->getEditor();
$this->setEntityRestrictions($otherChapter, ['view'], [$editor->roles()->first()]);
$sortData = [
'id' => $page->id,
'sort' => 0,
'parentChapter' => $otherChapter->id,
'type' => 'page',
'book' => $otherChapter->book_id,
];
$this->actingAs($editor)->put($page->book->getUrl('/sort'), ['sort-tree' => json_encode([$sortData])])->assertRedirect();
$this->assertDatabaseHas('pages', [
'id' => $page->id, 'chapter_id' => $page->chapter_id, 'book_id' => $page->book_id,
]);
}
public function test_book_sort_item_returns_book_content()
{
$books = Book::all();