From 339d4ec355b65b3e52e0478dffe610e7e30fd7f6 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 13 Mar 2021 15:18:37 +0000
Subject: [PATCH] Fixed misalignment of page and chapter parent book

Could occur when a chapter was moved with deleted pages.
Fixes #2632
---
 app/Auth/Permissions/PermissionService.php | 17 +++++++----------
 app/Entities/Models/BookChild.php          |  5 +----
 tests/Entity/SortTest.php                  | 18 ++++++++++++++++++
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/app/Auth/Permissions/PermissionService.php b/app/Auth/Permissions/PermissionService.php
index 89c8a5fbb..342be543e 100644
--- a/app/Auth/Permissions/PermissionService.php
+++ b/app/Auth/Permissions/PermissionService.php
@@ -3,8 +3,10 @@
 use BookStack\Auth\Permissions;
 use BookStack\Auth\Role;
 use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\EntityProvider;
+use BookStack\Entities\Models\Page;
 use BookStack\Model;
 use BookStack\Traits\HasCreatorAndUpdater;
 use BookStack\Traits\HasOwner;
@@ -112,21 +114,16 @@ class PermissionService
 
     /**
      * Get a chapter via ID, Checks local cache
-     * @param $chapterId
-     * @return \BookStack\Entities\Models\Book
      */
-    protected function getChapter($chapterId)
+    protected function getChapter(int $chapterId): ?Chapter
     {
         if (isset($this->entityCache['chapter']) && $this->entityCache['chapter']->has($chapterId)) {
             return $this->entityCache['chapter']->get($chapterId);
         }
 
-        $chapter = $this->entityProvider->chapter->find($chapterId);
-        if ($chapter === null) {
-            $chapter = false;
-        }
-
-        return $chapter;
+        return $this->entityProvider->chapter->newQuery()
+            ->withTrashed()
+            ->find($chapterId);
     }
 
     /**
@@ -460,7 +457,7 @@ class PermissionService
         $hasPermissiveAccessToParents = !$book->restricted;
 
         // For pages with a chapter, Check if explicit permissions are set on the Chapter
-        if ($entity->isA('page') && $entity->chapter_id !== 0 && $entity->chapter_id !== '0') {
+        if ($entity instanceof Page && intval($entity->chapter_id) !== 0) {
             $chapter = $this->getChapter($entity->chapter_id);
             $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapter->restricted;
             if ($chapter->restricted) {
diff --git a/app/Entities/Models/BookChild.php b/app/Entities/Models/BookChild.php
index 8b968cc8b..c73fa3959 100644
--- a/app/Entities/Models/BookChild.php
+++ b/app/Entities/Models/BookChild.php
@@ -1,8 +1,5 @@
 <?php namespace BookStack\Entities\Models;
 
-use BookStack\Entities\Models\Chapter;
-use BookStack\Entities\Models\Entity;
-use BookStack\Entities\Models\Book;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
@@ -49,7 +46,7 @@ abstract class BookChild extends Entity
 
         // Update all child pages if a chapter
         if ($this instanceof Chapter) {
-            foreach ($this->pages as $page) {
+            foreach ($this->pages()->withTrashed()->get() as $page) {
                 $page->changeBook($newBookId);
             }
         }
diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php
index d75a134ea..d3838c53c 100644
--- a/tests/Entity/SortTest.php
+++ b/tests/Entity/SortTest.php
@@ -196,6 +196,24 @@ class SortTest extends TestCase
         $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 */
+        $chapter = Chapter::query()->whereHas('pages')->first();
+        $currentBook = $chapter->book;
+        $pageToCheck = $chapter->pages->first();
+        $newBook = Book::query()->where('id', '!=', $currentBook->id)->first();
+
+        $pageToCheck->delete();
+
+        $this->asEditor()->put($chapter->getUrl('/move'), [
+            'entity_selection' => 'book:' . $newBook->id
+        ]);
+
+        $pageToCheck->refresh();
+        $this->assertEquals($newBook->id, $pageToCheck->book_id);
+    }
+
     public function test_book_sort()
     {
         $oldBook = Book::query()->first();