From fbd388ba4c5c1322038b362f8043532e7bd5596e Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 5 Jan 2022 16:11:11 +0000
Subject: [PATCH] Aligned chapter move permissions with page move permissions

---
 app/Entities/Repos/ChapterRepo.php         |  7 ++++--
 app/Entities/Repos/PageRepo.php            |  2 +-
 app/Http/Controllers/ChapterController.php |  5 +++--
 app/Http/Controllers/PageController.php    |  6 ++----
 tests/Entity/SortTest.php                  | 25 ++++++++++++++++++++++
 5 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php
index 672c2140c..2b81891af 100644
--- a/app/Entities/Repos/ChapterRepo.php
+++ b/app/Entities/Repos/ChapterRepo.php
@@ -10,6 +10,7 @@ use BookStack\Entities\Tools\BookContents;
 use BookStack\Entities\Tools\TrashCan;
 use BookStack\Exceptions\MoveOperationException;
 use BookStack\Exceptions\NotFoundException;
+use BookStack\Exceptions\PermissionsException;
 use BookStack\Facades\Activity;
 use Exception;
 
@@ -85,16 +86,18 @@ class ChapterRepo
      * 'book:<id>' (book:5).
      *
      * @throws MoveOperationException
+     * @throws PermissionsException
      */
     public function move(Chapter $chapter, string $parentIdentifier): Book
     {
-        /** @var Book $parent */
         $parent = $this->findParentByIdentifier($parentIdentifier);
         if (is_null($parent)) {
             throw new MoveOperationException('Book to move chapter into not found');
         }
 
-        // TODO - Check create permissions for new parent?
+        if (!userCan('chapter-create', $parent)) {
+            throw new PermissionsException('User does not have permission to create a chapter within the chosen book');
+        }
 
         $chapter->changeBook($parent->id);
         $chapter->rebuildPermissions();
diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index 992946461..828c4572f 100644
--- a/app/Entities/Repos/PageRepo.php
+++ b/app/Entities/Repos/PageRepo.php
@@ -328,7 +328,7 @@ class PageRepo
     public function move(Page $page, string $parentIdentifier): Entity
     {
         $parent = $this->findParentByIdentifier($parentIdentifier);
-        if ($parent === null) {
+        if (is_null($parent)) {
             throw new MoveOperationException('Book or chapter to move page into not found');
         }
 
diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php
index 5cd720f02..83b9bb692 100644
--- a/app/Http/Controllers/ChapterController.php
+++ b/app/Http/Controllers/ChapterController.php
@@ -11,6 +11,7 @@ use BookStack\Entities\Tools\NextPreviousContentLocator;
 use BookStack\Entities\Tools\PermissionsUpdater;
 use BookStack\Exceptions\MoveOperationException;
 use BookStack\Exceptions\NotFoundException;
+use BookStack\Exceptions\PermissionsException;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;
 use Throwable;
@@ -178,10 +179,10 @@ class ChapterController extends Controller
             return redirect($chapter->getUrl());
         }
 
-        // TODO - Check permissions against pages
-
         try {
             $newBook = $this->chapterRepo->move($chapter, $entitySelection);
+        } catch (PermissionsException $exception) {
+            $this->showPermissionError();
         } catch (MoveOperationException $exception) {
             $this->showErrorNotification(trans('errors.selected_book_not_found'));
 
diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php
index 3e57657da..fc4b463e1 100644
--- a/app/Http/Controllers/PageController.php
+++ b/app/Http/Controllers/PageController.php
@@ -412,11 +412,9 @@ class PageController extends Controller
 
         try {
             $parent = $this->pageRepo->move($page, $entitySelection);
+        } catch (PermissionsException $exception) {
+            $this->showPermissionError();
         } catch (Exception $exception) {
-            if ($exception instanceof PermissionsException) {
-                $this->showPermissionError();
-            }
-
             $this->showErrorNotification(trans('errors.selected_book_chapter_not_found'));
 
             return redirect()->back();
diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php
index dcca426f7..9ff75e700 100644
--- a/tests/Entity/SortTest.php
+++ b/tests/Entity/SortTest.php
@@ -198,6 +198,31 @@ class SortTest extends TestCase
         $this->assertTrue($chapter->book->id == $newBook->id, 'Page book is now the new book');
     }
 
+    public function test_chapter_move_requires_create_permissions_in_new_book()
+    {
+        $chapter = Chapter::query()->first();
+        $currentBook = $chapter->book;
+        $newBook = Book::query()->where('id', '!=', $currentBook->id)->first();
+        $editor = $this->getEditor();
+
+        $this->setEntityRestrictions($newBook, ['view', 'update', 'delete'], [$editor->roles->first()]);
+        $this->setEntityRestrictions($chapter, ['view', 'update', 'create', 'delete'], [$editor->roles->first()]);
+
+        $moveChapterResp = $this->actingAs($editor)->put($chapter->getUrl('/move'), [
+            'entity_selection' => 'book:' . $newBook->id,
+        ]);
+        $this->assertPermissionError($moveChapterResp);
+
+        $this->setEntityRestrictions($newBook, ['view', 'update', 'create', 'delete'], [$editor->roles->first()]);
+        $moveChapterResp = $this->put($chapter->getUrl('/move'), [
+            'entity_selection' => 'book:' . $newBook->id,
+        ]);
+
+        $chapter = Chapter::query()->find($chapter->id);
+        $moveChapterResp->assertRedirect($chapter->getUrl());
+        $this->assertTrue($chapter->book->id == $newBook->id, 'Page book is now the new book');
+    }
+
     public function test_chapter_move_changes_book_for_deleted_pages_within()
     {
         /** @var Chapter $chapter */