Aligned chapter move permissions with page move permissions

diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php
index 672c2140c3714c9efb970af0c8c56436449b041f..2b81891af63ce5fd67410bf62517c728e1e1d3bd 100644 (file)
--- 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\Entities\Tools\TrashCan;
 use BookStack\Exceptions\MoveOperationException;
 use BookStack\Exceptions\NotFoundException;

+use BookStack\Exceptions\PermissionsException;

 use BookStack\Facades\Activity;
 use Exception;
 
 use BookStack\Facades\Activity;
 use Exception;
 


@@ -85,16 +86,18 @@ class ChapterRepo
      * 'book:<id>' (book:5).
      *
      * @throws MoveOperationException
      * 'book:<id>' (book:5).
      *
      * @throws MoveOperationException

+     * @throws PermissionsException

      */
     public function move(Chapter $chapter, string $parentIdentifier): Book
     {
      */
     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');
         }
 
         $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();
 
         $chapter->changeBook($parent->id);
         $chapter->rebuildPermissions();

diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index 99294646145168a87aebe18c4fc0258911329b7e..828c4572fd100f7355d02077477a85a8728bc216 100644 (file)
--- 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);
     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');
         }
 
             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 5cd720f02b97ac360ad0fc65e4e61dcd38500c6a..83b9bb692da6c3173305c538ab79a6a8a836da86 100644 (file)
--- 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\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;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;
 use Throwable;


@@ -178,10 +179,10 @@ class ChapterController extends Controller
             return redirect($chapter->getUrl());
         }
 
             return redirect($chapter->getUrl());
         }
 

-        // TODO - Check permissions against pages
-

         try {
             $newBook = $this->chapterRepo->move($chapter, $entitySelection);
         try {
             $newBook = $this->chapterRepo->move($chapter, $entitySelection);

+        } catch (PermissionsException $exception) {
+            $this->showPermissionError();

         } catch (MoveOperationException $exception) {
             $this->showErrorNotification(trans('errors.selected_book_not_found'));
 
         } 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 3e57657da7199e6b5da0046b1c6f6008c50c932b..fc4b463e15f421fcd386ba054a6ae7919d09cd92 100644 (file)
--- 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);
 
         try {
             $parent = $this->pageRepo->move($page, $entitySelection);

+        } catch (PermissionsException $exception) {
+            $this->showPermissionError();

         } catch (Exception $exception) {
         } catch (Exception $exception) {

-            if ($exception instanceof PermissionsException) {
-                $this->showPermissionError();
-            }
-

             $this->showErrorNotification(trans('errors.selected_book_chapter_not_found'));
 
             return redirect()->back();
             $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 dcca426f77b79cf6a6c69946808b273856993ed2..9ff75e7000ccc78762b9be14a10e7d5036e9c7e8 100644 (file)
--- 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');
     }
 
         $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 */
     public function test_chapter_move_changes_book_for_deleted_pages_within()
     {
         /** @var Chapter $chapter */