diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index a50306552..20ab96133 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -161,6 +161,7 @@ class ChapterController extends Controller $chapter = $this->entityRepo->getBySlug('chapter', $chapterSlug, $bookSlug); $this->setPageTitle(trans('entities.chapters_move_named', ['chapterName' => $chapter->getShortName()])); $this->checkOwnablePermission('chapter-update', $chapter); + $this->checkOwnablePermission('chapter-delete', $chapter); return view('chapters/move', [ 'chapter' => $chapter, 'book' => $chapter->book @@ -179,6 +180,7 @@ class ChapterController extends Controller { $chapter = $this->entityRepo->getBySlug('chapter', $chapterSlug, $bookSlug); $this->checkOwnablePermission('chapter-update', $chapter); + $this->checkOwnablePermission('chapter-delete', $chapter); $entitySelection = $request->get('entity_selection', null); if ($entitySelection === null || $entitySelection === '') { diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 74595443b..b68655241 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -586,6 +586,7 @@ class PageController extends Controller { $page = $this->pageRepo->getPageBySlug($pageSlug, $bookSlug); $this->checkOwnablePermission('page-update', $page); + $this->checkOwnablePermission('page-delete', $page); return view('pages/move', [ 'book' => $page->book, 'page' => $page @@ -604,6 +605,7 @@ class PageController extends Controller { $page = $this->pageRepo->getPageBySlug($pageSlug, $bookSlug); $this->checkOwnablePermission('page-update', $page); + $this->checkOwnablePermission('page-delete', $page); $entitySelection = $request->get('entity_selection', null); if ($entitySelection === null || $entitySelection === '') { diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index ae450b8ee..f5f990145 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -20,11 +20,11 @@ @if(userCan('chapter-update', $chapter)) <a href="{{ $chapter->getUrl('/edit') }}" class="text-primary text-button">@icon('edit'){{ trans('common.edit') }}</a> @endif - @if(userCan('chapter-update', $chapter) || userCan('restrictions-manage', $chapter) || userCan('chapter-delete', $chapter)) + @if((userCan('chapter-update', $chapter) && userCan('chapter-delete', $chapter) )|| userCan('restrictions-manage', $chapter) || userCan('chapter-delete', $chapter)) <div dropdown class="dropdown-container"> <a dropdown-toggle class="text-primary text-button">@icon('more') {{ trans('common.more') }}</a> <ul> - @if(userCan('chapter-update', $chapter)) + @if(userCan('chapter-update', $chapter) && userCan('chapter-delete', $chapter)) <li><a href="{{ $chapter->getUrl('/move') }}" class="text-primary">@icon('folder'){{ trans('common.move') }}</a></li> @endif @if(userCan('restrictions-manage', $chapter)) diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 0b6aa7d14..afe007d45 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -23,7 +23,9 @@ <ul> @if(userCan('page-update', $page)) <li><a href="{{ $page->getUrl('/copy') }}" class="text-primary" >@icon('copy'){{ trans('common.copy') }}</a></li> - <li><a href="{{ $page->getUrl('/move') }}" class="text-primary" >@icon('folder'){{ trans('common.move') }}</a></li> + @if(userCan('page-delete', $page)) + <li><a href="{{ $page->getUrl('/move') }}" class="text-primary" >@icon('folder'){{ trans('common.move') }}</a></li> + @endif <li><a href="{{ $page->getUrl('/revisions') }}" class="text-primary">@icon('history'){{ trans('entities.revisions') }}</a></li> @endif @if(userCan('restrictions-manage', $page)) diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php index 5b23acfd5..11294f7df 100644 --- a/tests/Entity/SortTest.php +++ b/tests/Entity/SortTest.php @@ -3,7 +3,6 @@ use BookStack\Entities\Book; use BookStack\Entities\Chapter; use BookStack\Entities\Page; -use BookStack\Entities\Repos\EntityRepo; use BookStack\Entities\Repos\PageRepo; class SortTest extends TestCase @@ -58,14 +57,14 @@ class SortTest extends TestCase $newBook = Book::where('id', '!=', $currentBook->id)->first(); $editor = $this->getEditor(); - $this->setEntityRestrictions($newBook, ['view', 'edit', 'delete'], $editor->roles); + $this->setEntityRestrictions($newBook, ['view', 'update', 'delete'], $editor->roles); $movePageResp = $this->actingAs($editor)->put($page->getUrl('/move'), [ 'entity_selection' => 'book:' . $newBook->id ]); $this->assertPermissionError($movePageResp); - $this->setEntityRestrictions($newBook, ['view', 'edit', 'delete', 'create'], $editor->roles); + $this->setEntityRestrictions($newBook, ['view', 'update', 'delete', 'create'], $editor->roles); $movePageResp = $this->put($page->getUrl('/move'), [ 'entity_selection' => 'book:' . $newBook->id ]); @@ -76,6 +75,33 @@ class SortTest extends TestCase $this->assertTrue($page->book->id == $newBook->id, 'Page book is now the new book'); } + public function test_page_move_requires_delete_permissions() + { + $page = Page::first(); + $currentBook = $page->book; + $newBook = Book::where('id', '!=', $currentBook->id)->first(); + $editor = $this->getEditor(); + + $this->setEntityRestrictions($newBook, ['view', 'update', 'create', 'delete'], $editor->roles); + $this->setEntityRestrictions($page, ['view', 'update', 'create'], $editor->roles); + + $movePageResp = $this->actingAs($editor)->put($page->getUrl('/move'), [ + 'entity_selection' => 'book:' . $newBook->id + ]); + $this->assertPermissionError($movePageResp); + $pageView = $this->get($page->getUrl()); + $pageView->assertDontSee($page->getUrl('/move')); + + $this->setEntityRestrictions($page, ['view', 'update', 'create', 'delete'], $editor->roles); + $movePageResp = $this->put($page->getUrl('/move'), [ + 'entity_selection' => 'book:' . $newBook->id + ]); + + $page = Page::find($page->id); + $movePageResp->assertRedirect($page->getUrl()); + $this->assertTrue($page->book->id == $newBook->id, 'Page book is now the new book'); + } + public function test_chapter_move() { $chapter = Chapter::first(); @@ -104,6 +130,33 @@ class SortTest extends TestCase $pageCheckResp->assertSee($newBook->name); } + public function test_chapter_move_requires_delete_permissions() + { + $chapter = Chapter::first(); + $currentBook = $chapter->book; + $newBook = Book::where('id', '!=', $currentBook->id)->first(); + $editor = $this->getEditor(); + + $this->setEntityRestrictions($newBook, ['view', 'update', 'create', 'delete'], $editor->roles); + $this->setEntityRestrictions($chapter, ['view', 'update', 'create'], $editor->roles); + + $moveChapterResp = $this->actingAs($editor)->put($chapter->getUrl('/move'), [ + 'entity_selection' => 'book:' . $newBook->id + ]); + $this->assertPermissionError($moveChapterResp); + $pageView = $this->get($chapter->getUrl()); + $pageView->assertDontSee($chapter->getUrl('/move')); + + $this->setEntityRestrictions($chapter, ['view', 'update', 'create', 'delete'], $editor->roles); + $moveChapterResp = $this->put($chapter->getUrl('/move'), [ + 'entity_selection' => 'book:' . $newBook->id + ]); + + $chapter = Chapter::find($chapter->id); + $moveChapterResp->assertRedirect($chapter->getUrl()); + $this->assertTrue($chapter->book->id == $newBook->id, 'Page book is now the new book'); + } + public function test_book_sort() { $oldBook = Book::query()->first();