From 097d9c9f3cb1941b372b44c9868c4c177a3b8f80 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 30 Mar 2016 20:15:44 +0100 Subject: [PATCH] Updated entity restrictions to allow permissions, Not just restrict Also changed wording from 'Restrictions' to 'Permissions' to keep things more familiar and to better reflect what they do. Referenced in issue #89. --- app/Http/routes.php | 12 +- app/Services/RestrictionService.php | 19 +++ app/helpers.php | 5 +- resources/views/books/restrictions.blade.php | 2 +- resources/views/books/show.blade.php | 6 +- .../views/chapters/restrictions.blade.php | 2 +- resources/views/chapters/show.blade.php | 10 +- .../views/form/restriction-form.blade.php | 9 +- resources/views/pages/restrictions.blade.php | 2 +- resources/views/pages/show.blade.php | 14 +- resources/views/settings/roles/form.blade.php | 6 +- resources/views/users/delete.blade.php | 2 +- tests/Permissions/RestrictionsTest.php | 132 ++++++++++++++++-- tests/Permissions/RolesTest.php | 28 ++-- tests/TestCase.php | 8 ++ 15 files changed, 201 insertions(+), 56 deletions(-) diff --git a/app/Http/routes.php b/app/Http/routes.php index 0be901231..eca37347c 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -19,8 +19,8 @@ Route::group(['middleware' => 'auth'], function () { Route::delete('/{id}', 'BookController@destroy'); Route::get('/{slug}/sort-item', 'BookController@getSortItem'); Route::get('/{slug}', 'BookController@show'); - Route::get('/{bookSlug}/restrict', 'BookController@showRestrict'); - Route::put('/{bookSlug}/restrict', 'BookController@restrict'); + Route::get('/{bookSlug}/permissions', 'BookController@showRestrict'); + Route::put('/{bookSlug}/permissions', 'BookController@restrict'); Route::get('/{slug}/delete', 'BookController@showDelete'); Route::get('/{bookSlug}/sort', 'BookController@sort'); Route::put('/{bookSlug}/sort', 'BookController@saveSort'); @@ -36,8 +36,8 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/{bookSlug}/page/{pageSlug}/edit', 'PageController@edit'); Route::get('/{bookSlug}/page/{pageSlug}/delete', 'PageController@showDelete'); Route::get('/{bookSlug}/draft/{pageId}/delete', 'PageController@showDeleteDraft'); - Route::get('/{bookSlug}/page/{pageSlug}/restrict', 'PageController@showRestrict'); - Route::put('/{bookSlug}/page/{pageSlug}/restrict', 'PageController@restrict'); + Route::get('/{bookSlug}/page/{pageSlug}/permissions', 'PageController@showRestrict'); + Route::put('/{bookSlug}/page/{pageSlug}/permissions', 'PageController@restrict'); Route::put('/{bookSlug}/page/{pageSlug}', 'PageController@update'); Route::delete('/{bookSlug}/page/{pageSlug}', 'PageController@destroy'); Route::delete('/{bookSlug}/draft/{pageId}', 'PageController@destroyDraft'); @@ -54,8 +54,8 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/{bookSlug}/chapter/{chapterSlug}', 'ChapterController@show'); Route::put('/{bookSlug}/chapter/{chapterSlug}', 'ChapterController@update'); Route::get('/{bookSlug}/chapter/{chapterSlug}/edit', 'ChapterController@edit'); - Route::get('/{bookSlug}/chapter/{chapterSlug}/restrict', 'ChapterController@showRestrict'); - Route::put('/{bookSlug}/chapter/{chapterSlug}/restrict', 'ChapterController@restrict'); + Route::get('/{bookSlug}/chapter/{chapterSlug}/permissions', 'ChapterController@showRestrict'); + Route::put('/{bookSlug}/chapter/{chapterSlug}/permissions', 'ChapterController@restrict'); Route::get('/{bookSlug}/chapter/{chapterSlug}/delete', 'ChapterController@showDelete'); Route::delete('/{bookSlug}/chapter/{chapterSlug}', 'ChapterController@destroy'); diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index d20724866..50cbe4a51 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -41,6 +41,25 @@ class RestrictionService return false; } + /** + * Check if an entity has restrictions set on itself or its + * parent tree. + * @param Entity $entity + * @param $action + * @return bool|mixed + */ + public function checkIfRestrictionsSet(Entity $entity, $action) + { + $this->currentAction = $action; + if ($entity->isA('page')) { + return $entity->restricted || ($entity->chapter && $entity->chapter->restricted) || $entity->book->restricted; + } elseif ($entity->isA('chapter')) { + return $entity->restricted || $entity->book->restricted; + } elseif ($entity->isA('book')) { + return $entity->restricted; + } + } + /** * Add restrictions for a page query * @param $query diff --git a/app/helpers.php b/app/helpers.php index f60e917c5..eab8ca1c8 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -52,12 +52,13 @@ function userCan($permission, \BookStack\Ownable $ownable = null) if (!$ownable instanceof \BookStack\Entity) return $hasPermission; - // Check restrictions on the entitiy + // Check restrictions on the entity $restrictionService = app('BookStack\Services\RestrictionService'); $explodedPermission = explode('-', $permission); $action = end($explodedPermission); $hasAccess = $restrictionService->checkIfEntityRestricted($ownable, $action); - return $hasAccess && $hasPermission; + $restrictionsSet = $restrictionService->checkIfRestrictionsSet($ownable, $action); + return ($hasAccess && $restrictionsSet) || (!$restrictionsSet && $hasPermission); } /** diff --git a/resources/views/books/restrictions.blade.php b/resources/views/books/restrictions.blade.php index 60b126a7b..7fdd3abef 100644 --- a/resources/views/books/restrictions.blade.php +++ b/resources/views/books/restrictions.blade.php @@ -16,7 +16,7 @@
-

Book Restrictions

+

Book Permissions

@include('form/restriction-form', ['model' => $book])
diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index cd32a406b..5f8067bfb 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -24,7 +24,7 @@
  • Sort
    • @endif @if(userCan('restrictions-manage', $book)) -
  • Restrict
    • +
  • Permissions
    • @endif @if(userCan('book-delete', $book))
  • Delete
    • @@ -90,9 +90,9 @@ @if($book->restricted)

    @if(userCan('restrictions-manage', $book)) - Book Restricted + Book Permissions Active @else - Book Restricted + Book Permissions Active @endif

    @endif diff --git a/resources/views/chapters/restrictions.blade.php b/resources/views/chapters/restrictions.blade.php index 1f2f9c8fa..c25c0755d 100644 --- a/resources/views/chapters/restrictions.blade.php +++ b/resources/views/chapters/restrictions.blade.php @@ -17,7 +17,7 @@
    -

    Chapter Restrictions

    +

    Chapter Permissions

    @include('form/restriction-form', ['model' => $chapter])
    diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index dc20d144e..b6b2d5c97 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -19,7 +19,7 @@ Edit @endif @if(userCan('restrictions-manage', $chapter)) - Restrict + Permissions @endif @if(userCan('chapter-delete', $chapter)) Delete @@ -69,18 +69,18 @@ @if($book->restricted) @if(userCan('restrictions-manage', $book)) - Book Restricted + Book Permissions Active @else - Book Restricted + Book Permissions Active @endif
    @endif @if($chapter->restricted) @if(userCan('restrictions-manage', $chapter)) - Chapter Restricted + Chapter Permissions Active @else - Chapter Restricted + Chapter Permissions Active @endif @endif diff --git a/resources/views/form/restriction-form.blade.php b/resources/views/form/restriction-form.blade.php index d2fa23982..f61a535e7 100644 --- a/resources/views/form/restriction-form.blade.php +++ b/resources/views/form/restriction-form.blade.php @@ -1,11 +1,14 @@ -
    + {!! csrf_field() !!} +

    Once enabled, These permissions will take priority over any set role permissions.

    +
    - @include('form/checkbox', ['name' => 'restricted', 'label' => 'Restrict this ' . $model->getClassName()]) + @include('form/checkbox', ['name' => 'restricted', 'label' => 'Enable custom permissions'])
    + @@ -25,5 +28,5 @@
    Role
    Cancel - +
    \ No newline at end of file diff --git a/resources/views/pages/restrictions.blade.php b/resources/views/pages/restrictions.blade.php index d094abc71..09eb8a65b 100644 --- a/resources/views/pages/restrictions.blade.php +++ b/resources/views/pages/restrictions.blade.php @@ -24,7 +24,7 @@
    -

    Page Restrictions

    +

    Page Permissions

    @include('form/restriction-form', ['model' => $page])
    diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 286d44387..8640a34db 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -32,7 +32,7 @@ Edit @endif @if(userCan('restrictions-manage', $page)) - Restrict + Permissions @endif @if(userCan('page-delete', $page)) Delete @@ -76,27 +76,27 @@ @if($book->restricted) @if(userCan('restrictions-manage', $book)) - Book restricted + Book Permissions Active @else - Book restricted + Book Permissions Active @endif
    @endif @if($page->chapter && $page->chapter->restricted) @if(userCan('restrictions-manage', $page->chapter)) - Chapter restricted + Chapter Permissions Active @else - Chapter restricted + Chapter Permissions Active @endif
    @endif @if($page->restricted) @if(userCan('restrictions-manage', $page)) - Page restricted + Page Permissions Active @else - Page restricted + Page Permissions Active @endif
    @endif diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index fafb9bed2..ba57b4daa 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -24,10 +24,10 @@
    - +
    - +
    @@ -43,7 +43,7 @@

    Asset Permissions

    These permissions control default access to the assets within the system.
    - Restrictions on Books, Chapters and Pages will override these permissions. + Permissions on Books, Chapters and Pages will override these permissions.

    diff --git a/resources/views/users/delete.blade.php b/resources/views/users/delete.blade.php index 282ae242b..af247509d 100644 --- a/resources/views/users/delete.blade.php +++ b/resources/views/users/delete.blade.php @@ -10,7 +10,7 @@ {!! csrf_field() !!} - Cancel + Cancel diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 40b5a7647..4ecf5fb20 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -3,11 +3,21 @@ class RestrictionsTest extends TestCase { protected $user; + protected $viewer; public function setUp() { parent::setUp(); $this->user = $this->getNewUser(); + $this->viewer = $this->getViewer(); + } + + protected function getViewer() + { + $role = \BookStack\Role::getRole('viewer'); + $viewer = $this->getNewBlankUser(); + $viewer->attachRole($role);; + return $viewer; } /** @@ -20,11 +30,16 @@ class RestrictionsTest extends TestCase $entity->restricted = true; $entity->restrictions()->delete(); $role = $this->user->roles->first(); + $viewerRole = $this->viewer->roles->first(); foreach ($actions as $action) { $entity->restrictions()->create([ 'role_id' => $role->id, 'action' => strtolower($action) ]); + $entity->restrictions()->create([ + 'role_id' => $viewerRole->id, + 'action' => strtolower($action) + ]); } $entity->save(); $entity->load('restrictions'); @@ -65,6 +80,10 @@ class RestrictionsTest extends TestCase $book = \BookStack\Book::first(); $bookUrl = $book->getUrl(); + $this->actingAs($this->viewer) + ->visit($bookUrl) + ->dontSeeInElement('.action-buttons', 'New Page') + ->dontSeeInElement('.action-buttons', 'New Chapter'); $this->actingAs($this->user) ->visit($bookUrl) ->seeInElement('.action-buttons', 'New Page') @@ -319,11 +338,11 @@ class RestrictionsTest extends TestCase public function test_book_restriction_form() { $book = \BookStack\Book::first(); - $this->asAdmin()->visit($book->getUrl() . '/restrict') - ->see('Book Restrictions') + $this->asAdmin()->visit($book->getUrl() . '/permissions') + ->see('Book Permissions') ->check('restricted') ->check('restrictions[2][view]') - ->press('Save Restrictions') + ->press('Save Permissions') ->seeInDatabase('books', ['id' => $book->id, 'restricted' => true]) ->seeInDatabase('restrictions', [ 'restrictable_id' => $book->id, @@ -336,11 +355,11 @@ class RestrictionsTest extends TestCase public function test_chapter_restriction_form() { $chapter = \BookStack\Chapter::first(); - $this->asAdmin()->visit($chapter->getUrl() . '/restrict') - ->see('Chapter Restrictions') + $this->asAdmin()->visit($chapter->getUrl() . '/permissions') + ->see('Chapter Permissions') ->check('restricted') ->check('restrictions[2][update]') - ->press('Save Restrictions') + ->press('Save Permissions') ->seeInDatabase('chapters', ['id' => $chapter->id, 'restricted' => true]) ->seeInDatabase('restrictions', [ 'restrictable_id' => $chapter->id, @@ -353,11 +372,11 @@ class RestrictionsTest extends TestCase public function test_page_restriction_form() { $page = \BookStack\Page::first(); - $this->asAdmin()->visit($page->getUrl() . '/restrict') - ->see('Page Restrictions') + $this->asAdmin()->visit($page->getUrl() . '/permissions') + ->see('Page Permissions') ->check('restricted') ->check('restrictions[2][delete]') - ->press('Save Restrictions') + ->press('Save Permissions') ->seeInDatabase('pages', ['id' => $page->id, 'restricted' => true]) ->seeInDatabase('restrictions', [ 'restrictable_id' => $page->id, @@ -404,4 +423,99 @@ class RestrictionsTest extends TestCase ->dontSee($page->name); } + public function test_book_create_restriction_override() + { + $book = \BookStack\Book::first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->viewer) + ->visit($bookUrl) + ->dontSeeInElement('.action-buttons', 'New Page') + ->dontSeeInElement('.action-buttons', 'New Chapter'); + + $this->setEntityRestrictions($book, ['view', 'delete', 'update']); + + $this->forceVisit($bookUrl . '/chapter/create') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($bookUrl . '/page/create') + ->see('You do not have permission')->seePageIs('/'); + $this->visit($bookUrl)->dontSeeInElement('.action-buttons', 'New Page') + ->dontSeeInElement('.action-buttons', 'New Chapter'); + + $this->setEntityRestrictions($book, ['view', 'create']); + + $this->visit($bookUrl . '/chapter/create') + ->type('test chapter', 'name') + ->type('test description for chapter', 'description') + ->press('Save Chapter') + ->seePageIs($bookUrl . '/chapter/test-chapter'); + $this->visit($bookUrl . '/page/create') + ->type('test page', 'name') + ->type('test content', 'html') + ->press('Save Page') + ->seePageIs($bookUrl . '/page/test-page'); + $this->visit($bookUrl)->seeInElement('.action-buttons', 'New Page') + ->seeInElement('.action-buttons', 'New Chapter'); + } + + public function test_book_update_restriction_override() + { + $book = \BookStack\Book::first(); + $bookPage = $book->pages->first(); + $bookChapter = $book->chapters->first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->viewer) + ->visit($bookUrl . '/edit') + ->dontSee('Edit Book'); + + $this->setEntityRestrictions($book, ['view', 'delete']); + + $this->forceVisit($bookUrl . '/edit') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($bookPage->getUrl() . '/edit') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($bookChapter->getUrl() . '/edit') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($book, ['view', 'update']); + + $this->visit($bookUrl . '/edit') + ->seePageIs($bookUrl . '/edit'); + $this->visit($bookPage->getUrl() . '/edit') + ->seePageIs($bookPage->getUrl() . '/edit'); + $this->visit($bookChapter->getUrl() . '/edit') + ->see('Edit Chapter'); + } + + public function test_book_delete_restriction_override() + { + $book = \BookStack\Book::first(); + $bookPage = $book->pages->first(); + $bookChapter = $book->chapters->first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->viewer) + ->visit($bookUrl . '/delete') + ->dontSee('Delete Book'); + + $this->setEntityRestrictions($book, ['view', 'update']); + + $this->forceVisit($bookUrl . '/delete') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($bookPage->getUrl() . '/delete') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($bookChapter->getUrl() . '/delete') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($book, ['view', 'delete']); + + $this->visit($bookUrl . '/delete') + ->seePageIs($bookUrl . '/delete')->see('Delete Book'); + $this->visit($bookPage->getUrl() . '/delete') + ->seePageIs($bookPage->getUrl() . '/delete')->see('Delete Page'); + $this->visit($bookChapter->getUrl() . '/delete') + ->see('Delete Chapter'); + } + } diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 9c312626f..8ecdb37a3 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -129,14 +129,14 @@ class RolesTest extends TestCase { $page = \BookStack\Page::take(1)->get()->first(); $this->actingAs($this->user)->visit($page->getUrl()) - ->dontSee('Restrict') - ->visit($page->getUrl() . '/restrict') + ->dontSee('Permissions') + ->visit($page->getUrl() . '/permissions') ->seePageIs('/'); $this->giveUserPermissions($this->user, ['restrictions-manage-all']); $this->actingAs($this->user)->visit($page->getUrl()) - ->see('Restrict') - ->click('Restrict') - ->see('Page Restrictions')->seePageIs($page->getUrl() . '/restrict'); + ->see('Permissions') + ->click('Permissions') + ->see('Page Permissions')->seePageIs($page->getUrl() . '/permissions'); } public function test_restrictions_manage_own_permission() @@ -145,27 +145,27 @@ class RolesTest extends TestCase $content = $this->createEntityChainBelongingToUser($this->user); // Check can't restrict other's content $this->actingAs($this->user)->visit($otherUsersPage->getUrl()) - ->dontSee('Restrict') - ->visit($otherUsersPage->getUrl() . '/restrict') + ->dontSee('Permissions') + ->visit($otherUsersPage->getUrl() . '/permissions') ->seePageIs('/'); // Check can't restrict own content $this->actingAs($this->user)->visit($content['page']->getUrl()) - ->dontSee('Restrict') - ->visit($content['page']->getUrl() . '/restrict') + ->dontSee('Permissions') + ->visit($content['page']->getUrl() . '/permissions') ->seePageIs('/'); $this->giveUserPermissions($this->user, ['restrictions-manage-own']); // Check can't restrict other's content $this->actingAs($this->user)->visit($otherUsersPage->getUrl()) - ->dontSee('Restrict') - ->visit($otherUsersPage->getUrl() . '/restrict') + ->dontSee('Permissions') + ->visit($otherUsersPage->getUrl() . '/permissions') ->seePageIs('/'); // Check can restrict own content $this->actingAs($this->user)->visit($content['page']->getUrl()) - ->see('Restrict') - ->click('Restrict') - ->seePageIs($content['page']->getUrl() . '/restrict'); + ->see('Permissions') + ->click('Permissions') + ->seePageIs($content['page']->getUrl() . '/permissions'); } /** diff --git a/tests/TestCase.php b/tests/TestCase.php index 567dc93ec..d3b41831e 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -170,4 +170,12 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase $this->visit($link->link()->getUri()); return $this; } + + protected function actingAsUsers($usersArray, $callback) + { + foreach ($usersArray as $user) { + $this->actingAs($user); + $callback($user); + } + } }