From cdb1c7ef88a0054c46ba9eb040464bdea274b095 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 14 Apr 2018 18:47:13 +0100
Subject: [PATCH] Added destination permission checking to entity move
---
app/Http/Controllers/PageController.php | 2 +
resources/views/chapters/move.blade.php | 2 +-
resources/views/pages/move.blade.php | 2 +-
tests/BrowserKitTest.php | 59 +---------
tests/Entity/SortTest.php | 35 +++++-
tests/Permissions/RestrictionsTest.php | 43 ++-----
tests/Permissions/RolesTest.php | 8 --
tests/SharedTestHelpers.php | 143 ++++++++++++++++++++++++
tests/TestCase.php | 110 ++----------------
9 files changed, 193 insertions(+), 211 deletions(-)
create mode 100644 tests/SharedTestHelpers.php
diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php
index f42cf63ec..221e21a99 100644
--- a/app/Http/Controllers/PageController.php
+++ b/app/Http/Controllers/PageController.php
@@ -585,6 +585,8 @@ class PageController extends Controller
return redirect()->back();
}
+ $this->checkOwnablePermission('page-create', $parent);
+
$this->entityRepo->changePageParent($page, $parent);
Activity::add($page, 'page_move', $page->book->id);
session()->flash('success', trans('entities.pages_move_success', ['parentName' => $parent->name]));
diff --git a/resources/views/chapters/move.blade.php b/resources/views/chapters/move.blade.php
index 143d048b9..0efc18adf 100644
--- a/resources/views/chapters/move.blade.php
+++ b/resources/views/chapters/move.blade.php
@@ -17,7 +17,7 @@
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
- @include('components.entity-selector', ['name' => 'entity_selection', 'selectorSize' => 'large', 'entityTypes' => 'book'])
+ @include('components.entity-selector', ['name' => 'entity_selection', 'selectorSize' => 'large', 'entityTypes' => 'book', 'entityPermission' => 'chapter-create'])
<div class="form-group text-right">
<a href="{{ $chapter->getUrl() }}" class="button outline">{{ trans('common.cancel') }}</a>
diff --git a/resources/views/pages/move.blade.php b/resources/views/pages/move.blade.php
index 2bbefaae0..5a5c7e3f9 100644
--- a/resources/views/pages/move.blade.php
+++ b/resources/views/pages/move.blade.php
@@ -17,7 +17,7 @@
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
- @include('components.entity-selector', ['name' => 'entity_selection', 'selectorSize' => 'large', 'entityTypes' => 'book,chapter'])
+ @include('components.entity-selector', ['name' => 'entity_selection', 'selectorSize' => 'large', 'entityTypes' => 'book,chapter', 'entityPermission' => 'page-create'])
<div class="form-group text-right">
<a href="{{ $page->getUrl() }}" class="button outline">{{ trans('common.cancel') }}</a>
diff --git a/tests/BrowserKitTest.php b/tests/BrowserKitTest.php
index a8ff03044..86f61a764 100644
--- a/tests/BrowserKitTest.php
+++ b/tests/BrowserKitTest.php
@@ -12,10 +12,7 @@ abstract class BrowserKitTest extends TestCase
{
use DatabaseTransactions;
-
- // Local user instances
- private $admin;
- private $editor;
+ use SharedTestHelpers;
/**
* The base URL to use while testing the application.
@@ -43,38 +40,6 @@ abstract class BrowserKitTest extends TestCase
return $app;
}
- /**
- * Set the current user context to be an admin.
- * @return $this
- */
- public function asAdmin()
- {
- return $this->actingAs($this->getAdmin());
- }
-
- /**
- * Get the current admin user.
- * @return mixed
- */
- public function getAdmin() {
- if($this->admin === null) {
- $adminRole = Role::getSystemRole('admin');
- $this->admin = $adminRole->users->first();
- }
- return $this->admin;
- }
-
- /**
- * Set the current editor context to be an editor.
- * @return $this
- */
- public function asEditor()
- {
- if ($this->editor === null) {
- $this->editor = $this->getEditor();
- }
- return $this->actingAs($this->editor);
- }
/**
* Get a user that's not a system user such as the guest user.
@@ -127,28 +92,6 @@ abstract class BrowserKitTest extends TestCase
$restrictionService->buildJointPermissionsForEntity($entity);
}
- /**
- * Get an instance of a user with 'editor' permissions
- * @param array $attributes
- * @return mixed
- */
- protected function getEditor($attributes = [])
- {
- $user = \BookStack\Role::getRole('editor')->users()->first();
- if (!empty($attributes)) $user->forceFill($attributes)->save();
- return $user;
- }
-
- /**
- * Get an instance of a user with 'viewer' permissions
- * @return mixed
- */
- protected function getViewer()
- {
- $user = \BookStack\Role::getRole('viewer')->users()->first();
- if (!empty($attributes)) $user->forceFill($attributes)->save();
- return $user;
- }
/**
* Quick way to create a new user without any permissions
diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php
index 6e2b7c34e..ea5ab665d 100644
--- a/tests/Entity/SortTest.php
+++ b/tests/Entity/SortTest.php
@@ -34,10 +34,10 @@ class SortTest extends TestCase
$currentBook = $page->book;
$newBook = Book::where('id', '!=', $currentBook->id)->first();
- $resp = $this->asAdmin()->get($page->getUrl() . '/move');
+ $resp = $this->asEditor()->get($page->getUrl('/move'));
$resp->assertSee('Move Page');
- $movePageResp = $this->put($page->getUrl() . '/move', [
+ $movePageResp = $this->put($page->getUrl('/move'), [
'entity_selection' => 'book:' . $newBook->id
]);
$page = Page::find($page->id);
@@ -50,6 +50,31 @@ class SortTest extends TestCase
$newBookResp->assertSee($page->name);
}
+ public function test_page_move_requires_create_permissions_on_parent()
+ {
+ $page = Page::first();
+ $currentBook = $page->book;
+ $newBook = Book::where('id', '!=', $currentBook->id)->first();
+ $editor = $this->getEditor();
+
+ $this->setEntityRestrictions($newBook, ['view', 'edit', '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);
+ $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();
@@ -57,10 +82,10 @@ class SortTest extends TestCase
$pageToCheck = $chapter->pages->first();
$newBook = Book::where('id', '!=', $currentBook->id)->first();
- $chapterMoveResp = $this->asAdmin()->get($chapter->getUrl() . '/move');
+ $chapterMoveResp = $this->asEditor()->get($chapter->getUrl('/move'));
$chapterMoveResp->assertSee('Move Chapter');
- $moveChapterResp = $this->put($chapter->getUrl() . '/move', [
+ $moveChapterResp = $this->put($chapter->getUrl('/move'), [
'entity_selection' => 'book:' . $newBook->id
]);
@@ -105,7 +130,7 @@ class SortTest extends TestCase
];
}
- $sortResp = $this->asAdmin()->put($newBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]);
+ $sortResp = $this->asEditor()->put($newBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]);
$sortResp->assertRedirect($newBook->getUrl());
$sortResp->assertStatus(302);
$this->assertDatabaseHas('chapters', [
diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php
index 433ae7ff9..53e7ad3f3 100644
--- a/tests/Permissions/RestrictionsTest.php
+++ b/tests/Permissions/RestrictionsTest.php
@@ -1,7 +1,7 @@
<?php namespace Tests;
use BookStack\Book;
-use BookStack\Services\PermissionService;
+use BookStack\Entity;
use BookStack\User;
use BookStack\Repos\EntityRepo;
@@ -18,49 +18,20 @@ class RestrictionsTest extends BrowserKitTest
*/
protected $viewer;
- /**
- * @var PermissionService
- */
- protected $permissionService;
-
public function setUp()
{
parent::setUp();
$this->user = $this->getEditor();
$this->viewer = $this->getViewer();
- $this->permissionService = $this->app[PermissionService::class];
}
- /**
- * Manually set some permissions on an entity.
- * @param \BookStack\Entity $entity
- * @param $actions
- */
- protected function setEntityRestrictions(\BookStack\Entity $entity, $actions)
+ protected function setEntityRestrictions(Entity $entity, $actions = [], $roles = [])
{
- $entity->restricted = true;
- $entity->permissions()->delete();
-
- $role = $this->user->roles->first();
- $viewerRole = $this->viewer->roles->first();
-
- $permissions = [];
- foreach ($actions as $action) {
- $permissions[] = [
- 'role_id' => $role->id,
- 'action' => strtolower($action)
- ];
- $permissions[] = [
- 'role_id' => $viewerRole->id,
- 'action' => strtolower($action)
- ];
- }
- $entity->permissions()->createMany($permissions);
-
- $entity->save();
- $entity->load('permissions');
- $this->permissionService->buildJointPermissionsForEntity($entity);
- $entity->load('jointPermissions');
+ $roles = [
+ $this->user->roles->first(),
+ $this->viewer->roles->first(),
+ ];
+ parent::setEntityRestrictions($entity, $actions, $roles);
}
public function test_book_view_restriction()
diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php
index 5bc66986b..f076e6734 100644
--- a/tests/Permissions/RolesTest.php
+++ b/tests/Permissions/RolesTest.php
@@ -16,14 +16,6 @@ class RolesTest extends BrowserKitTest
$this->user = $this->getViewer();
}
- protected function getViewer()
- {
- $role = \BookStack\Role::getRole('viewer');
- $viewer = $this->getNewBlankUser();
- $viewer->attachRole($role);;
- return $viewer;
- }
-
/**
* Give the given user some permissions.
* @param \BookStack\User $user
diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php
new file mode 100644
index 000000000..325979e74
--- /dev/null
+++ b/tests/SharedTestHelpers.php
@@ -0,0 +1,143 @@
+<?php namespace Tests;
+
+use BookStack\Book;
+use BookStack\Chapter;
+use BookStack\Entity;
+use BookStack\Repos\EntityRepo;
+use BookStack\Role;
+use BookStack\Services\PermissionService;
+use BookStack\Services\SettingService;
+
+trait SharedTestHelpers
+{
+
+ protected $admin;
+ protected $editor;
+
+ /**
+ * Set the current user context to be an admin.
+ * @return $this
+ */
+ public function asAdmin()
+ {
+ return $this->actingAs($this->getAdmin());
+ }
+
+ /**
+ * Get the current admin user.
+ * @return mixed
+ */
+ public function getAdmin() {
+ if($this->admin === null) {
+ $adminRole = Role::getSystemRole('admin');
+ $this->admin = $adminRole->users->first();
+ }
+ return $this->admin;
+ }
+
+ /**
+ * Set the current user context to be an editor.
+ * @return $this
+ */
+ public function asEditor()
+ {
+ return $this->actingAs($this->getEditor());
+ }
+
+
+ /**
+ * Get a editor user.
+ * @return mixed
+ */
+ protected function getEditor() {
+ if($this->editor === null) {
+ $editorRole = Role::getRole('editor');
+ $this->editor = $editorRole->users->first();
+ }
+ return $this->editor;
+ }
+
+ /**
+ * Get an instance of a user with 'viewer' permissions
+ * @param $attributes
+ * @return mixed
+ */
+ protected function getViewer($attributes = [])
+ {
+ $user = \BookStack\Role::getRole('viewer')->users()->first();
+ if (!empty($attributes)) $user->forceFill($attributes)->save();
+ return $user;
+ }
+
+ /**
+ * Create and return a new book.
+ * @param array $input
+ * @return Book
+ */
+ public function newBook($input = ['name' => 'test book', 'description' => 'My new test book']) {
+ return $this->app[EntityRepo::class]->createFromInput('book', $input, false);
+ }
+
+ /**
+ * Create and return a new test chapter
+ * @param array $input
+ * @param Book $book
+ * @return Chapter
+ */
+ public function newChapter($input = ['name' => 'test chapter', 'description' => 'My new test chapter'], Book $book) {
+ return $this->app[EntityRepo::class]->createFromInput('chapter', $input, $book);
+ }
+
+ /**
+ * Create and return a new test page
+ * @param array $input
+ * @return Chapter
+ */
+ public function newPage($input = ['name' => 'test page', 'html' => 'My new test page']) {
+ $book = Book::first();
+ $entityRepo = $this->app[EntityRepo::class];
+ $draftPage = $entityRepo->getDraftPage($book);
+ return $entityRepo->publishPageDraft($draftPage, $input);
+ }
+
+ /**
+ * Quickly sets an array of settings.
+ * @param $settingsArray
+ */
+ protected function setSettings($settingsArray)
+ {
+ $settings = app(SettingService::class);
+ foreach ($settingsArray as $key => $value) {
+ $settings->put($key, $value);
+ }
+ }
+
+ /**
+ * Manually set some permissions on an entity.
+ * @param Entity $entity
+ * @param array $actions
+ * @param array $roles
+ */
+ protected function setEntityRestrictions(Entity $entity, $actions = [], $roles = [])
+ {
+ $entity->restricted = true;
+ $entity->permissions()->delete();
+
+ $permissions = [];
+ foreach ($actions as $action) {
+ foreach ($roles as $role) {
+ $permissions[] = [
+ 'role_id' => $role->id,
+ 'action' => strtolower($action)
+ ];
+ }
+ }
+ $entity->permissions()->createMany($permissions);
+
+ $entity->save();
+ $entity->load('permissions');
+ $this->app[PermissionService::class]->buildJointPermissionsForEntity($entity);
+ $entity->load('jointPermissions');
+ }
+
+}
\ No newline at end of file
diff --git a/tests/TestCase.php b/tests/TestCase.php
index 5c37b6179..e0f160eed 100644
--- a/tests/TestCase.php
+++ b/tests/TestCase.php
@@ -1,21 +1,14 @@
<?php namespace Tests;
-use BookStack\Book;
-use BookStack\Chapter;
-use BookStack\Repos\EntityRepo;
-use BookStack\Role;
-use BookStack\Services\SettingService;
use Illuminate\Foundation\Testing\DatabaseTransactions;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
+use Illuminate\Foundation\Testing\TestResponse;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
use DatabaseTransactions;
-
- protected $admin;
- protected $editor;
-
+ use SharedTestHelpers;
/**
* The base URL to use while testing the application.
* @var string
@@ -23,100 +16,13 @@ abstract class TestCase extends BaseTestCase
protected $baseUrl = 'http://localhost';
/**
- * Set the current user context to be an admin.
- * @return $this
+ * Assert a permission error has occurred.
+ * @param TestResponse $response
*/
- public function asAdmin()
+ protected function assertPermissionError(TestResponse $response)
{
- return $this->actingAs($this->getAdmin());
- }
-
- /**
- * Get the current admin user.
- * @return mixed
- */
- public function getAdmin() {
- if($this->admin === null) {
- $adminRole = Role::getSystemRole('admin');
- $this->admin = $adminRole->users->first();
- }
- return $this->admin;
- }
-
- /**
- * Set the current user context to be an editor.
- * @return $this
- */
- public function asEditor()
- {
- return $this->actingAs($this->getEditor());
- }
-
-
- /**
- * Get a editor user.
- * @return mixed
- */
- public function getEditor() {
- if($this->editor === null) {
- $editorRole = Role::getRole('editor');
- $this->editor = $editorRole->users->first();
- }
- return $this->editor;
- }
-
- /**
- * Get an instance of a user with 'viewer' permissions
- * @param $attributes
- * @return mixed
- */
- protected function getViewer($attributes = [])
- {
- $user = \BookStack\Role::getRole('viewer')->users()->first();
- if (!empty($attributes)) $user->forceFill($attributes)->save();
- return $user;
- }
-
- /**
- * Create and return a new book.
- * @param array $input
- * @return Book
- */
- public function newBook($input = ['name' => 'test book', 'description' => 'My new test book']) {
- return $this->app[EntityRepo::class]->createFromInput('book', $input, false);
- }
-
- /**
- * Create and return a new test chapter
- * @param array $input
- * @param Book $book
- * @return Chapter
- */
- public function newChapter($input = ['name' => 'test chapter', 'description' => 'My new test chapter'], Book $book) {
- return $this->app[EntityRepo::class]->createFromInput('chapter', $input, $book);
- }
-
- /**
- * Create and return a new test page
- * @param array $input
- * @return Chapter
- */
- public function newPage($input = ['name' => 'test page', 'html' => 'My new test page']) {
- $book = Book::first();
- $entityRepo = $this->app[EntityRepo::class];
- $draftPage = $entityRepo->getDraftPage($book);
- return $entityRepo->publishPageDraft($draftPage, $input);
- }
-
- /**
- * Quickly sets an array of settings.
- * @param $settingsArray
- */
- protected function setSettings($settingsArray)
- {
- $settings = app(SettingService::class);
- foreach ($settingsArray as $key => $value) {
- $settings->put($key, $value);
- }
+ $response->assertRedirect('/');
+ $this->assertTrue(session()->has('error'));
+ session()->remove('error');
}
}
\ No newline at end of file