diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index aa8f89ea4..f1645bb4b 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -155,7 +155,7 @@ class BookController extends Controller $book = $this->entityRepo->getBySlug('book', $bookSlug); $this->checkOwnablePermission('book-update', $book); $bookChildren = $this->entityRepo->getBookChildren($book, true); - $books = $this->entityRepo->getAll('book', false); + $books = $this->entityRepo->getAll('book', false, 'update'); $this->setPageTitle(trans('entities.books_sort_named', ['bookName'=>$book->getShortName()])); return view('books/sort', ['book' => $book, 'current' => $book, 'books' => $books, 'bookChildren' => $bookChildren]); } @@ -190,42 +190,56 @@ class BookController extends Controller } // Sort pages and chapters - $sortedBooks = []; - $updatedModels = collect(); - $sortMap = json_decode($request->get('sort-tree')); - $defaultBookId = $book->id; + $sortMap = collect(json_decode($request->get('sort-tree'))); + $bookIdsInvolved = collect([$book->id]); - // Loop through contents of provided map and update entities accordingly - foreach ($sortMap as $bookChild) { - $priority = $bookChild->sort; - $id = intval($bookChild->id); - $isPage = $bookChild->type == 'page'; - $bookId = $this->entityRepo->exists('book', $bookChild->book) ? intval($bookChild->book) : $defaultBookId; - $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter); - $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); + // Load models into map + $sortMap->each(function($mapItem) use ($bookIdsInvolved) { + $mapItem->type = ($mapItem->type === 'page' ? 'page' : 'chapter'); + $mapItem->model = $this->entityRepo->getById($mapItem->type, $mapItem->id); + // Store source and target books + $bookIdsInvolved->push(intval($mapItem->model->book_id)); + $bookIdsInvolved->push(intval($mapItem->book)); + }); - // Update models only if there's a change in parent chain or ordering. - if ($model->priority !== $priority || $model->book_id !== $bookId || ($isPage && $model->chapter_id !== $chapterId)) { - $this->entityRepo->changeBook($isPage?'page':'chapter', $bookId, $model); - $model->priority = $priority; - if ($isPage) $model->chapter_id = $chapterId; + // Get the books involved in the sort + $bookIdsInvolved = $bookIdsInvolved->unique()->toArray(); + $booksInvolved = $this->entityRepo->book->newQuery()->whereIn('id', $bookIdsInvolved)->get(); + // Throw permission error if invalid ids or inaccessible books given. + if (count($bookIdsInvolved) !== count($booksInvolved)) { + $this->showPermissionError(); + } + // Check permissions of involved books + $booksInvolved->each(function(Book $book) { + $this->checkOwnablePermission('book-update', $book); + }); + + // Perform the sort + $sortMap->each(function($mapItem) { + $model = $mapItem->model; + + $priorityChanged = intval($model->priority) !== intval($mapItem->sort); + $bookChanged = intval($model->book_id) !== intval($mapItem->book); + $chapterChanged = ($mapItem->type === 'page') && intval($model->chapter_id) !== $mapItem->parentChapter; + + if ($bookChanged) { + $this->entityRepo->changeBook($mapItem->type, $mapItem->book, $model); + } + if ($chapterChanged) { + $model->chapter_id = intval($mapItem->parentChapter); $model->save(); - $updatedModels->push($model); } - - // Store involved books to be sorted later - if (!in_array($bookId, $sortedBooks)) { - $sortedBooks[] = $bookId; + if ($priorityChanged) { + $model->priority = intval($mapItem->sort); + $model->save(); } - } + }); - // Add activity for books - foreach ($sortedBooks as $bookId) { - /** @var Book $updatedBook */ - $updatedBook = $this->entityRepo->getById('book', $bookId); - $this->entityRepo->buildJointPermissionsForBook($updatedBook); - Activity::add($updatedBook, 'book_sort', $updatedBook->id); - } + // Rebuild permissions and add activity for involved books. + $booksInvolved->each(function(Book $book) { + $this->entityRepo->buildJointPermissionsForBook($book); + Activity::add($book, 'book_sort', $book->id); + }); return redirect($book->getUrl()); } diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index 24c680234..2c92e1907 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -113,9 +113,9 @@ class EntityRepo * @param bool $allowDrafts * @return \Illuminate\Database\Query\Builder */ - protected function entityQuery($type, $allowDrafts = false) + protected function entityQuery($type, $allowDrafts = false, $permission = 'view') { - $q = $this->permissionService->enforceEntityRestrictions($type, $this->getEntity($type), 'view'); + $q = $this->permissionService->enforceEntityRestrictions($type, $this->getEntity($type), $permission); if (strtolower($type) === 'page' && !$allowDrafts) { $q = $q->where('draft', '=', false); } @@ -196,14 +196,15 @@ class EntityRepo } /** - * Get all entities of a type limited by count unless count if false. + * Get all entities of a type with the given permission, limited by count unless count is false. * @param string $type * @param integer|bool $count + * @param string $permission * @return Collection */ - public function getAll($type, $count = 20) + public function getAll($type, $count = 20, $permission = 'view') { - $q = $this->entityQuery($type)->orderBy('name', 'asc'); + $q = $this->entityQuery($type, false, $permission)->orderBy('name', 'asc'); if ($count !== false) $q = $q->take($count); return $q->get(); } diff --git a/tests/BrowserKitTest.php b/tests/BrowserKitTest.php index 1eabc7417..d5c9911f8 100644 --- a/tests/BrowserKitTest.php +++ b/tests/BrowserKitTest.php @@ -3,7 +3,6 @@ use BookStack\Entity; use BookStack\Role; use BookStack\Services\PermissionService; -use BookStack\User; use Illuminate\Contracts\Console\Kernel; use Illuminate\Foundation\Testing\DatabaseTransactions; use Laravel\BrowserKitTesting\TestCase; diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 218b7a0d8..8f37b2517 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -3,6 +3,7 @@ use BookStack\Book; use BookStack\Services\PermissionService; use BookStack\User; +use BookStack\Repos\EntityRepo; class RestrictionsTest extends BrowserKitTest { @@ -554,4 +555,70 @@ class RestrictionsTest extends BrowserKitTest $this->dontSee(substr($bookChapter->name, 0, 15)); } + public function test_book_sort_view_permission() + { + $firstBook = Book::first(); + $secondBook = Book::find(2); + $thirdBook = Book::find(3); + + $this->setEntityRestrictions($firstBook, ['view', 'update']); + $this->setEntityRestrictions($secondBook, ['view']); + $this->setEntityRestrictions($thirdBook, ['view', 'update']); + + // Test sort page visibility + $this->actingAs($this->user)->visit($secondBook->getUrl() . '/sort') + ->see('You do not have permission') + ->seePageIs('/'); + + // Check sort page on first book + $this->actingAs($this->user)->visit($firstBook->getUrl() . '/sort') + ->see($thirdBook->name) + ->dontSee($secondBook->name); + } + + public function test_book_sort_permission() { + $firstBook = Book::first(); + $secondBook = Book::find(2); + + $this->setEntityRestrictions($firstBook, ['view', 'update']); + $this->setEntityRestrictions($secondBook, ['view']); + + $firstBookChapter = $this->app[EntityRepo::class]->createFromInput('chapter', + ['name' => 'first book chapter'], $firstBook); + $secondBookChapter = $this->app[EntityRepo::class]->createFromInput('chapter', + ['name' => 'second book chapter'], $secondBook); + + // Create request data + $reqData = [ + [ + 'id' => $firstBookChapter->id, + 'sort' => 0, + 'parentChapter' => false, + 'type' => 'chapter', + 'book' => $secondBook->id + ] + ]; + + // Move chapter from first book to a second book + $this->actingAs($this->user)->put($firstBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]) + ->followRedirects() + ->see('You do not have permission') + ->seePageIs('/'); + + $reqData = [ + [ + 'id' => $secondBookChapter->id, + 'sort' => 0, + 'parentChapter' => false, + 'type' => 'chapter', + 'book' => $firstBook->id + ] + ]; + + // Move chapter from second book to first book + $this->actingAs($this->user)->put($firstBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]) + ->followRedirects() + ->see('You do not have permission') + ->seePageIs('/'); + } }