From a54be85185225d6f18f94f041546dd663f8f0644 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 26 Feb 2016 23:44:02 +0000 Subject: [PATCH 1/9] Started work on exposing the role system as editable --- app/Http/Controllers/Controller.php | 1 + app/Http/Controllers/PermissionController.php | 49 ++++++++++++++ app/Http/routes.php | 5 ++ resources/views/settings/navbar.blade.php | 1 + resources/views/settings/roles/edit.blade.php | 64 +++++++++++++++++++ .../views/settings/roles/index.blade.php | 26 ++++++++ 6 files changed, 146 insertions(+) create mode 100644 app/Http/Controllers/PermissionController.php create mode 100644 resources/views/settings/roles/edit.blade.php create mode 100644 resources/views/settings/roles/index.blade.php diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index ab37a44a1..654fed538 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -81,6 +81,7 @@ abstract class Controller extends BaseController protected function checkPermission($permissionName) { if (!$this->currentUser || !$this->currentUser->can($permissionName)) { + dd($this->currentUser); $this->showPermissionError(); } diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php new file mode 100644 index 000000000..69e2619b6 --- /dev/null +++ b/app/Http/Controllers/PermissionController.php @@ -0,0 +1,49 @@ +role = $role; + parent::__construct(); + } + + /** + * Show a listing of the roles in the system. + */ + public function listRoles() + { + $this->checkPermission('settings-update'); + $roles = $this->role->all(); + return view('settings/roles/index', ['roles' => $roles]); + } + + /** + * Show the form for editing a user role. + * @param $id + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function editRole($id) + { + $this->checkPermission('settings-update'); + $role = $this->role->findOrFail($id); + return view('settings/roles/edit', ['role' => $role]); + } +} diff --git a/app/Http/routes.php b/app/Http/routes.php index 36cf2a19f..eea0a0337 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -87,6 +87,7 @@ Route::group(['middleware' => 'auth'], function () { Route::group(['prefix' => 'settings'], function() { Route::get('/', 'SettingController@index'); Route::post('/', 'SettingController@update'); + // Users Route::get('/users', 'UserController@index'); Route::get('/users/create', 'UserController@create'); @@ -95,6 +96,10 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/users/{id}', 'UserController@edit'); Route::put('/users/{id}', 'UserController@update'); Route::delete('/users/{id}', 'UserController@destroy'); + + // Roles + Route::get('/roles', 'PermissionController@listRoles'); + Route::get('/roles/{id}', 'PermissionController@editRole'); }); }); diff --git a/resources/views/settings/navbar.blade.php b/resources/views/settings/navbar.blade.php index 3afe59a8e..7c3186889 100644 --- a/resources/views/settings/navbar.blade.php +++ b/resources/views/settings/navbar.blade.php @@ -5,6 +5,7 @@
Settings Users + Roles
diff --git a/resources/views/settings/roles/edit.blade.php b/resources/views/settings/roles/edit.blade.php new file mode 100644 index 000000000..ae2d01538 --- /dev/null +++ b/resources/views/settings/roles/edit.blade.php @@ -0,0 +1,64 @@ +@extends('base') + +@section('content') + + @include('settings/navbar', ['selected' => 'roles']) + +
+

Edit Role {{ $role->display_name }}

+ +
+
+ +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
CreateEditDelete
Books
Chapters
Pages
Images
+
+
+
+ +
+ +
+ +
+ +
+
+ +
+ +
+
+ +@stop diff --git a/resources/views/settings/roles/index.blade.php b/resources/views/settings/roles/index.blade.php new file mode 100644 index 000000000..661d66f63 --- /dev/null +++ b/resources/views/settings/roles/index.blade.php @@ -0,0 +1,26 @@ +@extends('base') + +@section('content') + + @include('settings/navbar', ['selected' => 'roles']) + +
+ +

User Roles

+ + + + + + + @foreach($roles as $role) + + + + + + @endforeach +
Role NameUsers
{{ $role->display_name }}{{ $role->description }}{{ $role->users->count() }}
+
+ +@stop From 473261be35ab50e6c9bc5914c899a34cd6cccf57 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 27 Feb 2016 19:24:42 +0000 Subject: [PATCH 2/9] Finished initial implementation of custom role system --- app/Entity.php | 20 +-- app/Http/Controllers/BookController.php | 24 +-- app/Http/Controllers/ChapterController.php | 21 +-- app/Http/Controllers/Controller.php | 22 ++- app/Http/Controllers/ImageController.php | 8 +- app/Http/Controllers/PageController.php | 27 ++-- app/Http/Controllers/PermissionController.php | 149 +++++++++++++++++- app/Http/Controllers/SettingController.php | 4 +- app/Http/Controllers/UserController.php | 34 ++-- app/Http/routes.php | 5 + app/Image.php | 9 +- app/Ownable.php | 13 +- app/Permission.php | 10 ++ app/Repos/UserRepo.php | 22 +-- app/Role.php | 11 ++ app/User.php | 41 +++-- app/helpers.php | 20 +++ ...27_120329_update_permissions_and_roles.php | 97 ++++++++++++ resources/views/base.blade.php | 2 +- resources/views/books/index.blade.php | 2 +- resources/views/books/show.blade.php | 8 +- resources/views/chapters/show.blade.php | 6 +- .../views/form/role-checkboxes.blade.php | 14 ++ resources/views/pages/show.blade.php | 4 +- resources/views/settings/index.blade.php | 2 +- .../views/settings/roles/checkbox.blade.php | 3 + .../views/settings/roles/create.blade.php | 15 ++ .../views/settings/roles/delete.blade.php | 28 ++++ resources/views/settings/roles/edit.blade.php | 64 ++------ resources/views/settings/roles/form.blade.php | 84 ++++++++++ .../views/settings/roles/index.blade.php | 5 + resources/views/users/forms/ldap.blade.php | 8 +- .../views/users/forms/standard.blade.php | 4 +- resources/views/users/index.blade.php | 16 +- tests/Auth/AuthTest.php | 4 +- tests/RolesTest.php | 48 ++++++ tests/TestCase.php | 3 +- 37 files changed, 644 insertions(+), 213 deletions(-) create mode 100644 database/migrations/2016_02_27_120329_update_permissions_and_roles.php create mode 100644 resources/views/form/role-checkboxes.blade.php create mode 100644 resources/views/settings/roles/checkbox.blade.php create mode 100644 resources/views/settings/roles/create.blade.php create mode 100644 resources/views/settings/roles/delete.blade.php create mode 100644 resources/views/settings/roles/form.blade.php create mode 100644 tests/RolesTest.php diff --git a/app/Entity.php b/app/Entity.php index 42323628a..08aa14638 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -1,14 +1,9 @@ -checkPermission('book-create'); + $this->checkPermission('book-create-all'); $this->setPageTitle('Create New Book'); return view('books/create'); } @@ -68,9 +68,9 @@ class BookController extends Controller */ public function store(Request $request) { - $this->checkPermission('book-create'); + $this->checkPermission('book-create-all'); $this->validate($request, [ - 'name' => 'required|string|max:255', + 'name' => 'required|string|max:255', 'description' => 'string|max:1000' ]); $book = $this->bookRepo->newFromInput($request->all()); @@ -105,8 +105,8 @@ class BookController extends Controller */ public function edit($slug) { - $this->checkPermission('book-update'); $book = $this->bookRepo->getBySlug($slug); + $this->checkOwnablePermission('book-update', $book); $this->setPageTitle('Edit Book ' . $book->getShortName()); return view('books/edit', ['book' => $book, 'current' => $book]); } @@ -120,10 +120,10 @@ class BookController extends Controller */ public function update(Request $request, $slug) { - $this->checkPermission('book-update'); $book = $this->bookRepo->getBySlug($slug); + $this->checkOwnablePermission('book-update', $book); $this->validate($request, [ - 'name' => 'required|string|max:255', + 'name' => 'required|string|max:255', 'description' => 'string|max:1000' ]); $book->fill($request->all()); @@ -141,8 +141,8 @@ class BookController extends Controller */ public function showDelete($bookSlug) { - $this->checkPermission('book-delete'); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('book-delete', $book); $this->setPageTitle('Delete Book ' . $book->getShortName()); return view('books/delete', ['book' => $book, 'current' => $book]); } @@ -154,8 +154,8 @@ class BookController extends Controller */ public function sort($bookSlug) { - $this->checkPermission('book-update'); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('book-update', $book); $bookChildren = $this->bookRepo->getChildren($book); $books = $this->bookRepo->getAll(false); $this->setPageTitle('Sort Book ' . $book->getShortName()); @@ -184,8 +184,8 @@ class BookController extends Controller */ public function saveSort($bookSlug, Request $request) { - $this->checkPermission('book-update'); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('book-update', $book); // Return if no map sent if (!$request->has('sort-tree')) { @@ -229,8 +229,8 @@ class BookController extends Controller */ public function destroy($bookSlug) { - $this->checkPermission('book-delete'); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('book-delete', $book); Activity::addMessage('book_delete', 0, $book->name); Activity::removeEntity($book); $this->bookRepo->destroyBySlug($bookSlug); diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index fc13e8b58..3b4780f8d 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -1,13 +1,8 @@ -checkPermission('chapter-create'); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('chapter-create', $book); $this->setPageTitle('Create New Chapter'); return view('chapters/create', ['book' => $book, 'current' => $book]); } @@ -52,12 +46,13 @@ class ChapterController extends Controller */ public function store($bookSlug, Request $request) { - $this->checkPermission('chapter-create'); $this->validate($request, [ 'name' => 'required|string|max:255' ]); $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('chapter-create', $book); + $chapter = $this->chapterRepo->newFromInput($request->all()); $chapter->slug = $this->chapterRepo->findSuitableSlug($chapter->name, $book->id); $chapter->priority = $this->bookRepo->getNewPriority($book); @@ -92,9 +87,9 @@ class ChapterController extends Controller */ public function edit($bookSlug, $chapterSlug) { - $this->checkPermission('chapter-update'); $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('chapter-update', $chapter); $this->setPageTitle('Edit Chapter' . $chapter->getShortName()); return view('chapters/edit', ['book' => $book, 'chapter' => $chapter, 'current' => $chapter]); } @@ -108,9 +103,9 @@ class ChapterController extends Controller */ public function update(Request $request, $bookSlug, $chapterSlug) { - $this->checkPermission('chapter-update'); $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('chapter-update', $chapter); $chapter->fill($request->all()); $chapter->slug = $this->chapterRepo->findSuitableSlug($chapter->name, $book->id, $chapter->id); $chapter->updated_by = auth()->user()->id; @@ -127,9 +122,9 @@ class ChapterController extends Controller */ public function showDelete($bookSlug, $chapterSlug) { - $this->checkPermission('chapter-delete'); $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('chapter-delete', $chapter); $this->setPageTitle('Delete Chapter' . $chapter->getShortName()); return view('chapters/delete', ['book' => $book, 'chapter' => $chapter, 'current' => $chapter]); } @@ -142,9 +137,9 @@ class ChapterController extends Controller */ public function destroy($bookSlug, $chapterSlug) { - $this->checkPermission('chapter-delete'); $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('chapter-delete', $chapter); Activity::addMessage('chapter_delete', $book->id, $chapter->name); $this->chapterRepo->destroy($chapter); return redirect($book->getUrl()); diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 654fed538..fce479af0 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers; +use BookStack\Ownable; use HttpRequestException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Exception\HttpResponseException; @@ -61,7 +62,7 @@ abstract class Controller extends BaseController } /** - * On a permission error redirect to home and display + * On a permission error redirect to home and display. * the error as a notification. */ protected function showPermissionError() @@ -74,20 +75,31 @@ abstract class Controller extends BaseController /** * Checks for a permission. - * - * @param $permissionName + * @param string $permissionName * @return bool|\Illuminate\Http\RedirectResponse */ protected function checkPermission($permissionName) { if (!$this->currentUser || !$this->currentUser->can($permissionName)) { - dd($this->currentUser); $this->showPermissionError(); } - return true; } + /** + * Check the current user's permissions against an ownable item. + * @param $permission + * @param Ownable $ownable + * @return bool + */ + protected function checkOwnablePermission($permission, Ownable $ownable) + { + $permissionBaseName = strtolower($permission) . '-'; + if (userCan($permissionBaseName . 'all')) return true; + if (userCan($permissionBaseName . 'own') && $ownable->createdBy->id === $this->currentUser->id) return true; + $this->showPermissionError(); + } + /** * Check if a user has a permission or bypass if the callback is true. * @param $permissionName diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php index 3fff28d3b..48e89ee41 100644 --- a/app/Http/Controllers/ImageController.php +++ b/app/Http/Controllers/ImageController.php @@ -64,7 +64,7 @@ class ImageController extends Controller */ public function uploadByType($type, Request $request) { - $this->checkPermission('image-create'); + $this->checkPermission('image-create-all'); $this->validate($request, [ 'file' => 'image|mimes:jpeg,gif,png' ]); @@ -90,7 +90,7 @@ class ImageController extends Controller */ public function getThumbnail($id, $width, $height, $crop) { - $this->checkPermission('image-create'); + $this->checkPermission('image-create-all'); $image = $this->imageRepo->getById($id); $thumbnailUrl = $this->imageRepo->getThumbnail($image, $width, $height, $crop == 'false'); return response()->json(['url' => $thumbnailUrl]); @@ -104,11 +104,11 @@ class ImageController extends Controller */ public function update($imageId, Request $request) { - $this->checkPermission('image-update'); $this->validate($request, [ 'name' => 'required|min:2|string' ]); $image = $this->imageRepo->getById($imageId); + $this->checkOwnablePermission('image-update', $image); $image = $this->imageRepo->updateImageDetails($image, $request->all()); return response()->json($image); } @@ -123,8 +123,8 @@ class ImageController extends Controller */ public function destroy(PageRepo $pageRepo, Request $request, $id) { - $this->checkPermission('image-delete'); $image = $this->imageRepo->getById($id); + $this->checkOwnablePermission('image-delete', $image); // Check if this image is used on any pages $isForced = ($request->has('force') && ($request->get('force') === 'true') || $request->get('force') === true); diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index e78ae13e4..ac968159b 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -1,12 +1,8 @@ -checkPermission('page-create'); $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $chapterSlug ? $this->chapterRepo->getBySlug($chapterSlug, $book->id) : false; + $parent = $chapter ? $chapter : $book; + $this->checkOwnablePermission('page-create', $parent); $this->setPageTitle('Create New Page'); return view('pages/create', ['book' => $book, 'chapter' => $chapter]); } /** * Store a newly created page in storage. - * * @param Request $request * @param $bookSlug * @return Response */ public function store(Request $request, $bookSlug) { - $this->checkPermission('page-create'); $this->validate($request, [ 'name' => 'required|string|max:255' ]); @@ -72,6 +66,8 @@ class PageController extends Controller $input = $request->all(); $book = $this->bookRepo->getBySlug($bookSlug); $chapterId = ($request->has('chapter') && $this->chapterRepo->idExists($request->get('chapter'))) ? $request->get('chapter') : null; + $parent = $chapterId !== null ? $this->chapterRepo->getById($chapterId) : $book; + $this->checkOwnablePermission('page-create', $parent); $input['priority'] = $this->bookRepo->getNewPriority($book); $page = $this->pageRepo->saveNew($input, $book, $chapterId); @@ -84,7 +80,6 @@ class PageController extends Controller * Display the specified page. * If the page is not found via the slug the * revisions are searched for a match. - * * @param $bookSlug * @param $pageSlug * @return Response @@ -109,23 +104,21 @@ class PageController extends Controller /** * Show the form for editing the specified page. - * * @param $bookSlug * @param $pageSlug * @return Response */ public function edit($bookSlug, $pageSlug) { - $this->checkPermission('page-update'); $book = $this->bookRepo->getBySlug($bookSlug); $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('page-update', $page); $this->setPageTitle('Editing Page ' . $page->getShortName()); return view('pages/edit', ['page' => $page, 'book' => $book, 'current' => $page]); } /** * Update the specified page in storage. - * * @param Request $request * @param $bookSlug * @param $pageSlug @@ -133,12 +126,12 @@ class PageController extends Controller */ public function update(Request $request, $bookSlug, $pageSlug) { - $this->checkPermission('page-update'); $this->validate($request, [ 'name' => 'required|string|max:255' ]); $book = $this->bookRepo->getBySlug($bookSlug); $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('page-update', $page); $this->pageRepo->updatePage($page, $book->id, $request->all()); Activity::add($page, 'page_update', $book->id); return redirect($page->getUrl()); @@ -164,9 +157,9 @@ class PageController extends Controller */ public function showDelete($bookSlug, $pageSlug) { - $this->checkPermission('page-delete'); $book = $this->bookRepo->getBySlug($bookSlug); $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('page-delete', $page); $this->setPageTitle('Delete Page ' . $page->getShortName()); return view('pages/delete', ['book' => $book, 'page' => $page, 'current' => $page]); } @@ -181,9 +174,9 @@ class PageController extends Controller */ public function destroy($bookSlug, $pageSlug) { - $this->checkPermission('page-delete'); $book = $this->bookRepo->getBySlug($bookSlug); $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('page-delete', $page); Activity::addMessage('page_delete', $book->id, $page->name); $this->pageRepo->destroy($page); return redirect($book->getUrl()); @@ -229,9 +222,9 @@ class PageController extends Controller */ public function restoreRevision($bookSlug, $pageSlug, $revisionId) { - $this->checkPermission('page-update'); $book = $this->bookRepo->getBySlug($bookSlug); $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('page-update', $page); $page = $this->pageRepo->restoreRevision($page, $book, $revisionId); Activity::add($page, 'page_restore', $book->id); return redirect($page->getUrl()); diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php index 69e2619b6..8cc14fc7a 100644 --- a/app/Http/Controllers/PermissionController.php +++ b/app/Http/Controllers/PermissionController.php @@ -2,26 +2,27 @@ namespace BookStack\Http\Controllers; +use BookStack\Permission; use BookStack\Role; -use BookStack\User; use Illuminate\Http\Request; - use BookStack\Http\Requests; -use BookStack\Http\Controllers\Controller; class PermissionController extends Controller { protected $role; + protected $permission; /** * PermissionController constructor. - * @param $role - * @param $user + * @param Role $role + * @param Permission $permission + * @internal param $user */ - public function __construct(Role $role) + public function __construct(Role $role, Permission $permission) { $this->role = $role; + $this->permission = $permission; parent::__construct(); } @@ -30,11 +31,54 @@ class PermissionController extends Controller */ public function listRoles() { - $this->checkPermission('settings-update'); + $this->checkPermission('user-roles-manage'); $roles = $this->role->all(); return view('settings/roles/index', ['roles' => $roles]); } + /** + * Show the form to create a new role + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function createRole() + { + $this->checkPermission('user-roles-manage'); + return view('settings/roles/create'); + } + + /** + * Store a new role in the system. + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function storeRole(Request $request) + { + $this->checkPermission('user-roles-manage'); + $this->validate($request, [ + 'display_name' => 'required|min:3|max:200', + 'description' => 'max:250' + ]); + + $role = $this->role->newInstance($request->all()); + $role->name = str_replace(' ', '-', strtolower($request->get('display_name'))); + // Prevent duplicate names + while ($this->role->where('name', '=', $role->name)->count() > 0) { + $role->name .= strtolower(str_random(2)); + } + $role->save(); + + if ($request->has('permissions')) { + $permissionsNames = array_keys($request->get('permissions')); + $permissions = $this->permission->whereIn('name', $permissionsNames)->pluck('id')->toArray(); + $role->permissions()->sync($permissions); + } else { + $role->permissions()->sync([]); + } + + session()->flash('success', 'Role successfully created'); + return redirect('/settings/roles'); + } + /** * Show the form for editing a user role. * @param $id @@ -42,8 +86,97 @@ class PermissionController extends Controller */ public function editRole($id) { - $this->checkPermission('settings-update'); + $this->checkPermission('user-roles-manage'); $role = $this->role->findOrFail($id); return view('settings/roles/edit', ['role' => $role]); } + + /** + * Updates a user role. + * @param $id + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function updateRole($id, Request $request) + { + $this->checkPermission('user-roles-manage'); + $this->validate($request, [ + 'display_name' => 'required|min:3|max:200', + 'description' => 'max:250' + ]); + + $role = $this->role->findOrFail($id); + if ($request->has('permissions')) { + $permissionsNames = array_keys($request->get('permissions')); + $permissions = $this->permission->whereIn('name', $permissionsNames)->pluck('id')->toArray(); + $role->permissions()->sync($permissions); + } else { + $role->permissions()->sync([]); + } + + // Ensure admin account always has all permissions + if ($role->name === 'admin') { + $permissions = $this->permission->all()->pluck('id')->toArray(); + $role->permissions()->sync($permissions); + } + + $role->fill($request->all()); + $role->save(); + + session()->flash('success', 'Role successfully updated'); + return redirect('/settings/roles'); + } + + /** + * Show the view to delete a role. + * Offers the chance to migrate users. + * @param $id + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function showDeleteRole($id) + { + $this->checkPermission('user-roles-manage'); + $role = $this->role->findOrFail($id); + $roles = $this->role->where('id', '!=', $id)->get(); + $blankRole = $this->role->newInstance(['display_name' => 'Don\'t migrate users']); + $roles->prepend($blankRole); + return view('settings/roles/delete', ['role' => $role, 'roles' => $roles]); + } + + /** + * Delete a role from the system, + * Migrate from a previous role if set. + * @param $id + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function deleteRole($id, Request $request) + { + $this->checkPermission('user-roles-manage'); + $role = $this->role->findOrFail($id); + + // Prevent deleting admin role + if ($role->name === 'admin') { + session()->flash('error', 'The admin role cannot be deleted'); + return redirect()->back(); + } + + if ($role->id == \Setting::get('registration-role')) { + session()->flash('error', 'This role cannot be deleted while set as the default registration role.'); + return redirect()->back(); + } + + if ($request->has('migration_role_id')) { + $newRole = $this->role->find($request->get('migration_role_id')); + if ($newRole) { + $users = $role->users->pluck('id')->toArray(); + $newRole->users()->sync($users); + } + } + + $role->delete(); + + session()->flash('success', 'Role successfully deleted'); + return redirect('/settings/roles'); + } } diff --git a/app/Http/Controllers/SettingController.php b/app/Http/Controllers/SettingController.php index 1739e0b53..c43e6e399 100644 --- a/app/Http/Controllers/SettingController.php +++ b/app/Http/Controllers/SettingController.php @@ -17,7 +17,7 @@ class SettingController extends Controller */ public function index() { - $this->checkPermission('settings-update'); + $this->checkPermission('settings-manage'); $this->setPageTitle('Settings'); return view('settings/index'); } @@ -32,7 +32,7 @@ class SettingController extends Controller public function update(Request $request) { $this->preventAccessForDemoUsers(); - $this->checkPermission('settings-update'); + $this->checkPermission('settings-manage'); // Cycles through posted settings and update them foreach($request->all() as $name => $value) { diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 55ca5be19..1207c87f1 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -35,7 +35,7 @@ class UserController extends Controller */ public function index() { - $users = $this->user->all(); + $users = $this->userRepo->getAllUsers(); $this->setPageTitle('Users'); return view('users/index', ['users' => $users]); } @@ -46,7 +46,7 @@ class UserController extends Controller */ public function create() { - $this->checkPermission('user-create'); + $this->checkPermission('users-manage'); $authMethod = config('auth.method'); return view('users/create', ['authMethod' => $authMethod]); } @@ -58,11 +58,10 @@ class UserController extends Controller */ public function store(Request $request) { - $this->checkPermission('user-create'); + $this->checkPermission('users-manage'); $validationRules = [ 'name' => 'required', - 'email' => 'required|email|unique:users,email', - 'role' => 'required|exists:roles,id' + 'email' => 'required|email|unique:users,email' ]; $authMethod = config('auth.method'); @@ -84,7 +83,11 @@ class UserController extends Controller } $user->save(); - $user->attachRoleId($request->get('role')); + + if ($request->has('roles')) { + $roles = $request->get('roles'); + $user->roles()->sync($roles); + } // Get avatar from gravatar and save if (!config('services.disable_services')) { @@ -104,7 +107,7 @@ class UserController extends Controller */ public function edit($id, SocialAuthService $socialAuthService) { - $this->checkPermissionOr('user-update', function () use ($id) { + $this->checkPermissionOr('users-manage', function () use ($id) { return $this->currentUser->id == $id; }); @@ -125,7 +128,7 @@ class UserController extends Controller public function update(Request $request, $id) { $this->preventAccessForDemoUsers(); - $this->checkPermissionOr('user-update', function () use ($id) { + $this->checkPermissionOr('users-manage', function () use ($id) { return $this->currentUser->id == $id; }); @@ -133,8 +136,7 @@ class UserController extends Controller 'name' => 'min:2', 'email' => 'min:2|email|unique:users,email,' . $id, 'password' => 'min:5|required_with:password_confirm', - 'password-confirm' => 'same:password|required_with:password', - 'role' => 'exists:roles,id' + 'password-confirm' => 'same:password|required_with:password' ], [ 'password-confirm.required_with' => 'Password confirmation required' ]); @@ -143,8 +145,9 @@ class UserController extends Controller $user->fill($request->all()); // Role updates - if ($this->currentUser->can('user-update') && $request->has('role')) { - $user->attachRoleId($request->get('role')); + if (userCan('users-manage') && $request->has('roles')) { + $roles = $request->get('roles'); + $user->roles()->sync($roles); } // Password updates @@ -154,11 +157,12 @@ class UserController extends Controller } // External auth id updates - if ($this->currentUser->can('user-update') && $request->has('external_auth_id')) { + if ($this->currentUser->can('users-manage') && $request->has('external_auth_id')) { $user->external_auth_id = $request->get('external_auth_id'); } $user->save(); + session()->flash('success', 'User successfully updated'); return redirect('/settings/users'); } @@ -169,7 +173,7 @@ class UserController extends Controller */ public function delete($id) { - $this->checkPermissionOr('user-delete', function () use ($id) { + $this->checkPermissionOr('users-manage', function () use ($id) { return $this->currentUser->id == $id; }); @@ -186,7 +190,7 @@ class UserController extends Controller public function destroy($id) { $this->preventAccessForDemoUsers(); - $this->checkPermissionOr('user-delete', function () use ($id) { + $this->checkPermissionOr('users-manage', function () use ($id) { return $this->currentUser->id == $id; }); diff --git a/app/Http/routes.php b/app/Http/routes.php index eea0a0337..a1c737642 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -99,7 +99,12 @@ Route::group(['middleware' => 'auth'], function () { // Roles Route::get('/roles', 'PermissionController@listRoles'); + Route::get('/roles/new', 'PermissionController@createRole'); + Route::post('/roles/new', 'PermissionController@storeRole'); + Route::get('/roles/delete/{id}', 'PermissionController@showDeleteRole'); + Route::delete('/roles/delete/{id}', 'PermissionController@deleteRole'); Route::get('/roles/{id}', 'PermissionController@editRole'); + Route::put('/roles/{id}', 'PermissionController@updateRole'); }); }); diff --git a/app/Image.php b/app/Image.php index 3ac084d8f..ad23a077a 100644 --- a/app/Image.php +++ b/app/Image.php @@ -1,14 +1,9 @@ -belongsTo('BookStack\User', 'updated_by'); } + + /** + * Gets the class name. + * @return string + */ + public static function getClassName() + { + return strtolower(array_slice(explode('\\', static::class), -1, 1)[0]); + } + } \ No newline at end of file diff --git a/app/Permission.php b/app/Permission.php index 6859ed56e..794df01ab 100644 --- a/app/Permission.php +++ b/app/Permission.php @@ -13,4 +13,14 @@ class Permission extends Model { return $this->belongsToMany('BookStack\Permissions'); } + + /** + * Get the permission object by name. + * @param $roleName + * @return mixed + */ + public static function getByName($name) + { + return static::where('name', '=', $name)->first(); + } } diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 48541a51a..15813b3e1 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -42,6 +42,15 @@ class UserRepo return $this->user->findOrFail($id); } + /** + * Get all the users with their permissions. + * @return \Illuminate\Database\Eloquent\Builder|static + */ + public function getAllUsers() + { + return $this->user->with('roles', 'avatar')->orderBy('name', 'asc')->get(); + } + /** * Creates a new user and attaches a role to them. * @param array $data @@ -69,7 +78,7 @@ class UserRepo public function attachDefaultRole($user) { $roleId = Setting::get('registration-role'); - if ($roleId === false) $roleId = $this->role->getDefault()->id; + if ($roleId === false) $roleId = $this->role->first()->id; $user->attachRoleId($roleId); } @@ -80,15 +89,10 @@ class UserRepo */ public function isOnlyAdmin(User $user) { - if ($user->role->name != 'admin') { - return false; - } - - $adminRole = $this->role->where('name', '=', 'admin')->first(); - if (count($adminRole->users) > 1) { - return false; - } + if (!$user->roles->pluck('name')->contains('admin')) return false; + $adminRole = $this->role->getRole('admin'); + if ($adminRole->users->count() > 1) return false; return true; } diff --git a/app/Role.php b/app/Role.php index 3d93bf770..8d5ed7d66 100644 --- a/app/Role.php +++ b/app/Role.php @@ -6,6 +6,8 @@ use Illuminate\Database\Eloquent\Model; class Role extends Model { + + protected $fillable = ['display_name', 'description']; /** * Sets the default role name for newly registered users. * @var string @@ -28,6 +30,15 @@ class Role extends Model return $this->belongsToMany('BookStack\Permission'); } + /** + * Check if this role has a permission. + * @param $permission + */ + public function hasPermission($permission) + { + return $this->permissions->pluck('name')->contains($permission); + } + /** * Add a permission to this role. * @param Permission $permission diff --git a/app/User.php b/app/User.php index c55102078..b062aa78f 100644 --- a/app/User.php +++ b/app/User.php @@ -14,21 +14,18 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * The database table used by the model. - * * @var string */ protected $table = 'users'; /** * The attributes that are mass assignable. - * * @var array */ protected $fillable = ['name', 'email', 'image_id']; /** * The attributes excluded from the model's JSON form. - * * @var array */ protected $hidden = ['password', 'remember_token']; @@ -50,10 +47,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon ]); } - /** - * Permissions and roles - */ - /** * The roles that belong to the user. */ @@ -62,21 +55,29 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon return $this->belongsToMany('BookStack\Role'); } - public function getRoleAttribute() + /** + * Check if the user has a role. + * @param $role + * @return mixed + */ + public function hasRole($role) { - return $this->roles()->with('permissions')->first(); + return $this->roles->pluck('name')->contains($role); } /** - * Loads the user's permissions from their role. + * Get all permissions belonging to a the current user. + * @return \Illuminate\Database\Eloquent\Relations\HasManyThrough */ - private function loadPermissions() + public function permissions() { - if (isset($this->permissions)) return; + if(isset($this->permissions)) return $this->permissions; $this->load('roles.permissions'); - $permissions = $this->roles[0]->permissions; - $permissionsArray = $permissions->pluck('name')->all(); - $this->permissions = $permissionsArray; + $permissions = $this->roles->map(function($role) { + return $role->permissions; + })->flatten()->unique(); + $this->permissions = $permissions; + return $permissions; } /** @@ -86,11 +87,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function can($permissionName) { - if ($this->email == 'guest') { - return false; - } - $this->loadPermissions(); - return array_search($permissionName, $this->permissions) !== false; + if ($this->email === 'guest') return false; + return $this->permissions()->pluck('name')->contains($permissionName); } /** @@ -113,7 +111,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get the social account associated with this user. - * * @return \Illuminate\Database\Eloquent\Relations\HasMany */ public function socialAccounts() @@ -138,8 +135,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Returns the user's avatar, - * Uses Gravatar as the avatar service. - * * @param int $size * @return string */ diff --git a/app/helpers.php b/app/helpers.php index f25a8f765..db65407c7 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -27,4 +27,24 @@ if (! function_exists('versioned_asset')) { throw new InvalidArgumentException("File {$file} not defined in asset manifest."); } +} + +/** + * Check if the current user has a permission. + * If an ownable element is passed in the permissions are checked against + * that particular item. + * @param $permission + * @param \BookStack\Ownable $ownable + * @return mixed + */ +function userCan($permission, \BookStack\Ownable $ownable = null) +{ + if ($ownable === null) { + return auth()->user() && auth()->user()->can($permission); + } + + $permissionBaseName = strtolower($permission) . '-'; + if (userCan($permissionBaseName . 'all')) return true; + if (userCan($permissionBaseName . 'own') && $ownable->createdBy->id === auth()->user()->id) return true; + return false; } \ No newline at end of file diff --git a/database/migrations/2016_02_27_120329_update_permissions_and_roles.php b/database/migrations/2016_02_27_120329_update_permissions_and_roles.php new file mode 100644 index 000000000..9fb2e98f2 --- /dev/null +++ b/database/migrations/2016_02_27_120329_update_permissions_and_roles.php @@ -0,0 +1,97 @@ +each(function ($permission) { + $permission->delete(); + }); + + // Create & attach new admin permissions + $permissionsToCreate = [ + 'settings-manage' => 'Manage Settings', + 'users-manage' => 'Manage Users', + 'user-roles-manage' => 'Manage Roles & Permissions' + ]; + foreach ($permissionsToCreate as $name => $displayName) { + $newPermission = new \BookStack\Permission(); + $newPermission->name = $name; + $newPermission->display_name = $displayName; + $newPermission->save(); + $adminRole->attachPermission($newPermission); + } + + // Create & attach new entity permissions + $entities = ['Book', 'Page', 'Chapter', 'Image']; + $ops = ['Create All', 'Create Own', 'Update All', 'Update Own', 'Delete All', 'Delete Own']; + foreach ($entities as $entity) { + foreach ($ops as $op) { + $newPermission = new \BookStack\Permission(); + $newPermission->name = strtolower($entity) . '-' . strtolower(str_replace(' ', '-', $op)); + $newPermission->display_name = $op . ' ' . $entity . 's'; + $newPermission->save(); + $adminRole->attachPermission($newPermission); + if ($editorRole !== null) $editorRole->attachPermission($newPermission); + } + } + + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + // Get roles with permissions we need to change + $adminRole = \BookStack\Role::getRole('admin'); + + // Delete old permissions + $permissions = \BookStack\Permission::all(); + $permissions->each(function ($permission) { + $permission->delete(); + }); + + // Create default CRUD permissions and allocate to admins and editors + $entities = ['Book', 'Page', 'Chapter', 'Image']; + $ops = ['Create', 'Update', 'Delete']; + foreach ($entities as $entity) { + foreach ($ops as $op) { + $newPermission = new \BookStack\Permission(); + $newPermission->name = strtolower($entity) . '-' . strtolower($op); + $newPermission->display_name = $op . ' ' . $entity . 's'; + $newPermission->save(); + $adminRole->attachPermission($newPermission); + } + } + + // Create admin permissions + $entities = ['Settings', 'User']; + $ops = ['Create', 'Update', 'Delete']; + foreach ($entities as $entity) { + foreach ($ops as $op) { + $newPermission = new \BookStack\Permission(); + $newPermission->name = strtolower($entity) . '-' . strtolower($op); + $newPermission->display_name = $op . ' ' . $entity; + $newPermission->save(); + $adminRole->attachPermission($newPermission); + } + } + } +} diff --git a/resources/views/base.blade.php b/resources/views/base.blade.php index 0df49485e..e3cac3621 100644 --- a/resources/views/base.blade.php +++ b/resources/views/base.blade.php @@ -43,7 +43,7 @@
Books - @if(isset($currentUser) && $currentUser->can('settings-update')) + @if(isset($currentUser) && $currentUser->can('settings-manage')) Settings @endif @if(!isset($signedIn) || !$signedIn) diff --git a/resources/views/books/index.blade.php b/resources/views/books/index.blade.php index 9fa483735..d5d7cb139 100644 --- a/resources/views/books/index.blade.php +++ b/resources/views/books/index.blade.php @@ -8,7 +8,7 @@
- @if($currentUser->can('book-create')) + @if($currentUser->can('book-create-all')) Add new book @endif
diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index db89bed9e..12ebae0d6 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -7,17 +7,17 @@
- @if($currentUser->can('page-create')) + @if(userCan('page-create', $book)) New Page @endif - @if($currentUser->can('chapter-create')) + @if(userCan('chapter-create', $book)) New Chapter @endif - @if($currentUser->can('book-update')) + @if(userCan('book-update', $book)) Edit Sort @endif - @if($currentUser->can('book-delete')) + @if(userCan('book-delete', $book)) Delete @endif
diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index 7033093e5..18a115ea5 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -12,13 +12,13 @@
- @if($currentUser->can('chapter-create')) + @if(userCan('page-create', $chapter)) New Page @endif - @if($currentUser->can('chapter-update')) + @if(userCan('chapter-update', $chapter)) Edit @endif - @if($currentUser->can('chapter-delete')) + @if(userCan('chapter-delete', $chapter)) Delete @endif
diff --git a/resources/views/form/role-checkboxes.blade.php b/resources/views/form/role-checkboxes.blade.php new file mode 100644 index 000000000..df868ee98 --- /dev/null +++ b/resources/views/form/role-checkboxes.blade.php @@ -0,0 +1,14 @@ + +@foreach($roles as $role) + +@endforeach + +@if($errors->has($name)) +
{{ $errors->first($name) }}
+@endif \ No newline at end of file diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 004a240d0..52b9daee6 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -27,11 +27,11 @@
  • Plain Text File .txt
    • - @if($currentUser->can('page-update')) + @if(userCan('page-update', $page)) Revisions Edit @endif - @if($currentUser->can('page-delete')) + @if(userCan('page-delete', $page)) Delete @endif
    diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index b5b5d0a4f..4274378e6 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -54,7 +54,7 @@ hasPermission($permission)))) checked="checked" @endif + value="true"> \ No newline at end of file diff --git a/resources/views/settings/roles/create.blade.php b/resources/views/settings/roles/create.blade.php new file mode 100644 index 000000000..f7d39f454 --- /dev/null +++ b/resources/views/settings/roles/create.blade.php @@ -0,0 +1,15 @@ +@extends('base') + +@section('content') + + @include('settings/navbar', ['selected' => 'roles']) + +
    +

    Create New Role

    + +
    + @include('settings/roles/form') +
    +
    + +@stop diff --git a/resources/views/settings/roles/delete.blade.php b/resources/views/settings/roles/delete.blade.php new file mode 100644 index 000000000..5d1ffe259 --- /dev/null +++ b/resources/views/settings/roles/delete.blade.php @@ -0,0 +1,28 @@ +@extends('base') + +@section('content') + + @include('settings/navbar', ['selected' => 'roles']) + +
    +

    Delete Role

    +

    This will delete the role with the name '{{$role->display_name}}'.

    + +
    + {!! csrf_field() !!} + + + @if($role->users->count() > 0) +
    +

    This role has {{$role->users->count()}} users assigned to it. If you would like to migrate the users from this role select a new role below.

    + @include('form/role-select', ['options' => $roles, 'name' => 'migration_role_id']) +
    + @endif + +

    Are you sure you want to delete this role?

    + Cancel + +
    +
    + +@stop diff --git a/resources/views/settings/roles/edit.blade.php b/resources/views/settings/roles/edit.blade.php index ae2d01538..f98e6e0b3 100644 --- a/resources/views/settings/roles/edit.blade.php +++ b/resources/views/settings/roles/edit.blade.php @@ -5,59 +5,19 @@ @include('settings/navbar', ['selected' => 'roles'])
    -

    Edit Role {{ $role->display_name }}

    - -
    -
    - -
    - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
    CreateEditDelete
    Books
    Chapters
    Pages
    Images
    -
    -
    -
    - -
    - -
    - -
    - -
    -
    - +
    +
    +

    Edit Role {{ $role->display_name }}

    - +
    +

    + Delete Role +
    +
    + + + + @include('settings/roles/form', ['model' => $role])
    diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php new file mode 100644 index 000000000..b9c47053b --- /dev/null +++ b/resources/views/settings/roles/form.blade.php @@ -0,0 +1,84 @@ +{!! csrf_field() !!} + +
    + +
    +
    + + @include('form/text', ['name' => 'display_name']) +
    +
    + + @include('form/text', ['name' => 'description']) +
    +
    +
    + +
    + +
    + +
    +
    + +
    + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    CreateEditDelete
    Books@include('settings/roles/checkbox', ['permission' => 'book-create-all']) + + + + + +
    Chapters@include('settings/roles/checkbox', ['permission' => 'chapter-create-all']) + + + + + +
    Pages@include('settings/roles/checkbox', ['permission' => 'page-create-all']) + + + + + +
    Images@include('settings/roles/checkbox', ['permission' => 'image-create-all']) + + + + + +
    +
    + +
    + \ No newline at end of file diff --git a/resources/views/settings/roles/index.blade.php b/resources/views/settings/roles/index.blade.php index 661d66f63..601c6533e 100644 --- a/resources/views/settings/roles/index.blade.php +++ b/resources/views/settings/roles/index.blade.php @@ -7,6 +7,11 @@

    User Roles

    + +

    + Add new role +

    + diff --git a/resources/views/users/forms/ldap.blade.php b/resources/views/users/forms/ldap.blade.php index 4a6673dc2..47edb211b 100644 --- a/resources/views/users/forms/ldap.blade.php +++ b/resources/views/users/forms/ldap.blade.php @@ -3,21 +3,21 @@ @include('form.text', ['name' => 'name']) -@if($currentUser->can('user-update')) +@if(userCan('users-manage'))
    @include('form.text', ['name' => 'email'])
    @endif -@if($currentUser->can('user-update')) +@if(userCan('users-manage'))
    - @include('form.role-select', ['name' => 'role', 'options' => \BookStack\Role::all(), 'displayKey' => 'display_name']) + @include('form/role-checkboxes', ['name' => 'roles', 'roles' => \BookStack\Role::all()])
    @endif -@if($currentUser->can('user-update')) +@if(userCan('users-manage'))
    @include('form.text', ['name' => 'external_auth_id']) diff --git a/resources/views/users/forms/standard.blade.php b/resources/views/users/forms/standard.blade.php index c20e2955d..9bd70b43c 100644 --- a/resources/views/users/forms/standard.blade.php +++ b/resources/views/users/forms/standard.blade.php @@ -8,10 +8,10 @@ @include('form.text', ['name' => 'email'])
    -@if($currentUser->can('user-update')) +@if(userCan('users-manage'))
    - @include('form.role-select', ['name' => 'role', 'options' => \BookStack\Role::all(), 'displayKey' => 'display_name']) + @include('form/role-checkboxes', ['name' => 'roles', 'roles' => \BookStack\Role::all()])
    @endif diff --git a/resources/views/users/index.blade.php b/resources/views/users/index.blade.php index 6e5d10c5f..f06630714 100644 --- a/resources/views/users/index.blade.php +++ b/resources/views/users/index.blade.php @@ -8,7 +8,7 @@

    Users

    - @if($currentUser->can('user-create')) + @if(userCan('users-manage'))

    Add new user

    @@ -18,30 +18,32 @@
    - + @foreach($users as $user) - + @endforeach
    Role Name Name EmailUser TypeUser Roles
    {{$user->name}} - @if($currentUser->can('user-update') || $currentUser->id == $user->id) + @if(userCan('users-manage') || $currentUser->id == $user->id) @endif {{ $user->name }} - @if($currentUser->can('user-update') || $currentUser->id == $user->id) + @if(userCan('users-manage') || $currentUser->id == $user->id) @endif - @if($currentUser->can('user-update') || $currentUser->id == $user->id) + @if(userCan('users-manage') || $currentUser->id == $user->id) @endif {{ $user->email }} - @if($currentUser->can('user-update') || $currentUser->id == $user->id) + @if(userCan('users-manage') || $currentUser->id == $user->id) @endif {{ $user->role->display_name }} + {{ $user->roles->implode('display_name', ', ') }} +
    diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index 694022666..067840841 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -133,12 +133,12 @@ class AuthTest extends TestCase ->click('Add new user') ->type($user->name, '#name') ->type($user->email, '#email') - ->select(2, '#role') + ->check('roles[admin]') ->type($user->password, '#password') ->type($user->password, '#password-confirm') ->press('Save') - ->seeInDatabase('users', $user->toArray()) ->seePageIs('/settings/users') + ->seeInDatabase('users', $user->toArray()) ->see($user->name); } diff --git a/tests/RolesTest.php b/tests/RolesTest.php new file mode 100644 index 000000000..140290bea --- /dev/null +++ b/tests/RolesTest.php @@ -0,0 +1,48 @@ + 'test-role', + 'display_name' => 'Test Role', + 'description' => 'This is a role for testing' + ]); + } + + public function test_admin_can_see_settings() + { + $this->asAdmin()->visit('/settings')->see('Settings'); + } + + public function test_cannot_delete_admin_role() + { + $adminRole = \BookStack\Role::getRole('admin'); + $deletePageUrl = '/settings/roles/delete/' . $adminRole->id; + $this->asAdmin()->visit($deletePageUrl) + ->press('Confirm') + ->seePageIs($deletePageUrl) + ->see('cannot be deleted'); + } + + public function test_role_cannot_be_deleted_if_default() + { + $newRole = $this->createNewRole(); + $this->setSettings(['registration-role' => $newRole->id]); + + $deletePageUrl = '/settings/roles/delete/' . $newRole->id; + $this->asAdmin()->visit($deletePageUrl) + ->press('Confirm') + ->seePageIs($deletePageUrl) + ->see('cannot be deleted'); + } + +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 4b8578a43..ce4e93b12 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -32,7 +32,8 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase public function asAdmin() { if($this->admin === null) { - $this->admin = \BookStack\User::find(1); + $adminRole = \BookStack\Role::getRole('admin'); + $this->admin = $adminRole->users->first(); } return $this->actingAs($this->admin); } From a14b5c33fde770ce6b142f9e5535b4caffcdf121 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 27 Feb 2016 20:52:46 +0000 Subject: [PATCH 3/9] Added missing permission checkboxes and improved image AJAX permission responses --- app/Http/Controllers/Controller.php | 5 ++--- resources/assets/js/controllers.js | 21 ++++++++++++------- resources/lang/en/errors.php | 1 + resources/views/settings/roles/form.blade.php | 14 ++++++++++--- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index fce479af0..4182743a7 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -68,9 +68,8 @@ abstract class Controller extends BaseController protected function showPermissionError() { Session::flash('error', trans('errors.permission')); - throw new HttpResponseException( - redirect('/') - ); + $response = request()->wantsJson() ? response()->json(['error' => trans('errors.permissionJson')], 403) : redirect('/', 403); + throw new HttpResponseException($response); } /** diff --git a/resources/assets/js/controllers.js b/resources/assets/js/controllers.js index 76def6abd..1f7388859 100644 --- a/resources/assets/js/controllers.js +++ b/resources/assets/js/controllers.js @@ -118,6 +118,7 @@ module.exports = function (ngApp, events) { page++; }); } + $scope.fetchData = fetchData; /** @@ -130,12 +131,16 @@ module.exports = function (ngApp, events) { $http.put(url, this.selectedImage).then((response) => { events.emit('success', 'Image details updated'); }, (response) => { - var errors = response.data; - var message = ''; - Object.keys(errors).forEach((key) => { - message += errors[key].join('\n'); - }); - events.emit('error', message); + if (response.status === 422) { + var errors = response.data; + var message = ''; + Object.keys(errors).forEach((key) => { + message += errors[key].join('\n'); + }); + events.emit('error', message); + } else if (response.status === 403) { + events.emit('error', response.data.error); + } }); }; @@ -158,6 +163,8 @@ module.exports = function (ngApp, events) { // Pages failure if (response.status === 400) { $scope.dependantPages = response.data; + } else if (response.status === 403) { + events.emit('error', response.data.error); } }); }; @@ -167,7 +174,7 @@ module.exports = function (ngApp, events) { * @param stringDate * @returns {Date} */ - $scope.getDate = function(stringDate) { + $scope.getDate = function (stringDate) { return new Date(stringDate); }; diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 53785b684..b1a252bf3 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -8,4 +8,5 @@ return [ // Pages 'permission' => 'You do not have permission to access the requested page.', + 'permissionJson' => 'You do not have permission to perform the requested action.' ]; \ No newline at end of file diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index b9c47053b..3069896b9 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -31,7 +31,9 @@ Books - @include('settings/roles/checkbox', ['permission' => 'book-create-all']) + + + @@ -43,7 +45,10 @@ Chapters - @include('settings/roles/checkbox', ['permission' => 'chapter-create-all']) + + + + @@ -55,7 +60,10 @@ Pages - @include('settings/roles/checkbox', ['permission' => 'page-create-all']) + + + + From 201f7888062e6e59340b073f306c870776734e49 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 28 Feb 2016 10:49:41 +0000 Subject: [PATCH 4/9] Implemented database structure and inital interfaces for entity restrictions --- app/Entity.php | 20 ++++- app/Http/Controllers/BookController.php | 40 +++++++++- app/Http/Controllers/ChapterController.php | 45 +++++++++++- app/Http/Controllers/PageController.php | 47 +++++++++++- app/Http/routes.php | 6 ++ app/Repos/BookRepo.php | 23 ++++++ app/Repos/ChapterRepo.php | 23 ++++++ app/Repos/PageRepo.php | 23 ++++++ app/Repos/UserRepo.php | 10 +++ app/Restriction.php | 21 ++++++ ...27_120329_update_permissions_and_roles.php | 4 +- ...2_28_084200_add_entity_access_controls.php | 73 +++++++++++++++++++ resources/views/books/restrictions.blade.php | 10 +++ resources/views/books/show.blade.php | 3 + .../views/chapters/restrictions.blade.php | 10 +++ resources/views/chapters/show.blade.php | 3 + resources/views/form/checkbox.blade.php | 12 +++ .../views/form/restriction-checkbox.blade.php | 7 ++ .../views/form/restriction-form.blade.php | 28 +++++++ resources/views/pages/restrictions.blade.php | 10 +++ resources/views/pages/show.blade.php | 15 ++-- resources/views/settings/roles/form.blade.php | 27 +++++-- 22 files changed, 436 insertions(+), 24 deletions(-) create mode 100644 app/Restriction.php create mode 100644 database/migrations/2016_02_28_084200_add_entity_access_controls.php create mode 100644 resources/views/books/restrictions.blade.php create mode 100644 resources/views/chapters/restrictions.blade.php create mode 100644 resources/views/form/checkbox.blade.php create mode 100644 resources/views/form/restriction-checkbox.blade.php create mode 100644 resources/views/form/restriction-form.blade.php create mode 100644 resources/views/pages/restrictions.blade.php diff --git a/app/Entity.php b/app/Entity.php index 08aa14638..975009174 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -48,13 +48,31 @@ abstract class Entity extends Ownable /** * Get View objects for this entity. - * @return mixed */ public function views() { return $this->morphMany('BookStack\View', 'viewable'); } + /** + * Get this entities restrictions. + */ + public function restrictions() + { + return $this->morphMany('BookStack\Restriction', 'restrictable'); + } + + /** + * Check if this entity has a specific restriction set against it. + * @param $role_id + * @param $action + * @return bool + */ + public function hasRestriction($role_id, $action) + { + return $this->restrictions->where('role_id', $role_id)->where('action', $action)->count() > 0; + } + /** * Allows checking of the exact class, Used to check entity type. * Cleaner method for is_a. diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index c1320088d..5b2b510c9 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -3,6 +3,7 @@ namespace BookStack\Http\Controllers; use Activity; +use BookStack\Repos\UserRepo; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; @@ -19,18 +20,21 @@ class BookController extends Controller protected $bookRepo; protected $pageRepo; protected $chapterRepo; + protected $userRepo; /** * BookController constructor. * @param BookRepo $bookRepo * @param PageRepo $pageRepo * @param ChapterRepo $chapterRepo + * @param UserRepo $userRepo */ - public function __construct(BookRepo $bookRepo, PageRepo $pageRepo, ChapterRepo $chapterRepo) + public function __construct(BookRepo $bookRepo, PageRepo $pageRepo, ChapterRepo $chapterRepo, UserRepo $userRepo) { $this->bookRepo = $bookRepo; $this->pageRepo = $pageRepo; $this->chapterRepo = $chapterRepo; + $this->userRepo = $userRepo; parent::__construct(); } @@ -177,7 +181,6 @@ class BookController extends Controller /** * Saves an array of sort mapping to pages and chapters. - * * @param string $bookSlug * @param Request $request * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector @@ -223,7 +226,6 @@ class BookController extends Controller /** * Remove the specified book from storage. - * * @param $bookSlug * @return Response */ @@ -236,4 +238,36 @@ class BookController extends Controller $this->bookRepo->destroyBySlug($bookSlug); return redirect('/books'); } + + /** + * Show the Restrictions view. + * @param $bookSlug + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function showRestrict($bookSlug) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('restrictions-manage', $book); + $roles = $this->userRepo->getRestrictableRoles(); + return view('books/restrictions', [ + 'book' => $book, + 'roles' => $roles + ]); + } + + /** + * Set the restrictions for this book. + * @param $bookSlug + * @param $bookSlug + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function restrict($bookSlug, Request $request) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $this->checkOwnablePermission('restrictions-manage', $book); + $this->bookRepo->updateRestrictionsFromRequest($request, $book); + session()->flash('success', 'Page Restrictions Updated'); + return redirect($book->getUrl()); + } } diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index 3b4780f8d..2ad08c7da 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -1,6 +1,7 @@ bookRepo = $bookRepo; $this->chapterRepo = $chapterRepo; + $this->userRepo = $userRepo; parent::__construct(); } @@ -144,4 +148,39 @@ class ChapterController extends Controller $this->chapterRepo->destroy($chapter); return redirect($book->getUrl()); } + + /** + * Show the Restrictions view. + * @param $bookSlug + * @param $chapterSlug + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function showRestrict($bookSlug, $chapterSlug) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('restrictions-manage', $chapter); + $roles = $this->userRepo->getRestrictableRoles(); + return view('chapters/restrictions', [ + 'chapter' => $chapter, + 'roles' => $roles + ]); + } + + /** + * Set the restrictions for this chapter. + * @param $bookSlug + * @param $chapterSlug + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function restrict($bookSlug, $chapterSlug, Request $request) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('restrictions-manage', $chapter); + $this->chapterRepo->updateRestrictionsFromRequest($request, $chapter); + session()->flash('success', 'Page Restrictions Updated'); + return redirect($chapter->getUrl()); + } } diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index ac968159b..b469f51dd 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -1,6 +1,7 @@ pageRepo = $pageRepo; $this->bookRepo = $bookRepo; $this->chapterRepo = $chapterRepo; $this->exportService = $exportService; + $this->userRepo = $userRepo; parent::__construct(); } @@ -308,4 +312,39 @@ class PageController extends Controller ]); } + /** + * Show the Restrictions view. + * @param $bookSlug + * @param $pageSlug + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + */ + public function showRestrict($bookSlug, $pageSlug) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('restrictions-manage', $page); + $roles = $this->userRepo->getRestrictableRoles(); + return view('pages/restrictions', [ + 'page' => $page, + 'roles' => $roles + ]); + } + + /** + * Set the restrictions for this page. + * @param $bookSlug + * @param $pageSlug + * @param Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ + public function restrict($bookSlug, $pageSlug, Request $request) + { + $book = $this->bookRepo->getBySlug($bookSlug); + $page = $this->pageRepo->getBySlug($pageSlug, $book->id); + $this->checkOwnablePermission('restrictions-manage', $page); + $this->pageRepo->updateRestrictionsFromRequest($request, $page); + session()->flash('success', 'Page Restrictions Updated'); + return redirect($page->getUrl()); + } + } diff --git a/app/Http/routes.php b/app/Http/routes.php index a1c737642..81bbb16bc 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -19,6 +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('/{slug}/delete', 'BookController@showDelete'); Route::get('/{bookSlug}/sort', 'BookController@sort'); Route::put('/{bookSlug}/sort', 'BookController@saveSort'); @@ -32,6 +34,8 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/{bookSlug}/page/{pageSlug}/export/plaintext', 'PageController@exportPlainText'); Route::get('/{bookSlug}/page/{pageSlug}/edit', 'PageController@edit'); Route::get('/{bookSlug}/page/{pageSlug}/delete', 'PageController@showDelete'); + Route::get('/{bookSlug}/page/{pageSlug}/restrict', 'PageController@showRestrict'); + Route::put('/{bookSlug}/page/{pageSlug}/restrict', 'PageController@restrict'); Route::put('/{bookSlug}/page/{pageSlug}', 'PageController@update'); Route::delete('/{bookSlug}/page/{pageSlug}', 'PageController@destroy'); @@ -47,6 +51,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}/delete', 'ChapterController@showDelete'); Route::delete('/{bookSlug}/chapter/{chapterSlug}', 'ChapterController@destroy'); diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index d8a24c099..dbf95623e 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -238,4 +238,27 @@ class BookRepo return $books; } + /** + * Updates books restrictions from a request + * @param $request + * @param $book + */ + public function updateRestrictionsFromRequest($request, $book) + { + // TODO - extract into shared repo + $book->restricted = $request->has('restricted') && $request->get('restricted') === 'true'; + $book->restrictions()->delete(); + if ($request->has('restrictions')) { + foreach($request->get('restrictions') as $roleId => $restrictions) { + foreach ($restrictions as $action => $value) { + $book->restrictions()->create([ + 'role_id' => $roleId, + 'action' => strtolower($action) + ]); + } + } + } + $book->save(); + } + } \ No newline at end of file diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index bba6cbe7a..7e3560f2b 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -161,4 +161,27 @@ class ChapterRepo return $chapter; } + /** + * Updates pages restrictions from a request + * @param $request + * @param $chapter + */ + public function updateRestrictionsFromRequest($request, $chapter) + { + // TODO - extract into shared repo + $chapter->restricted = $request->has('restricted') && $request->get('restricted') === 'true'; + $chapter->restrictions()->delete(); + if ($request->has('restrictions')) { + foreach($request->get('restrictions') as $roleId => $restrictions) { + foreach ($restrictions as $action => $value) { + $chapter->restrictions()->create([ + 'role_id' => $roleId, + 'action' => strtolower($action) + ]); + } + } + } + $chapter->save(); + } + } \ No newline at end of file diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index f028a1fcc..1a98255ab 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -407,4 +407,27 @@ class PageRepo return $this->page->orderBy('updated_at', 'desc')->paginate($count); } + /** + * Updates pages restrictions from a request + * @param $request + * @param $page + */ + public function updateRestrictionsFromRequest($request, $page) + { + // TODO - extract into shared repo + $page->restricted = $request->has('restricted') && $request->get('restricted') === 'true'; + $page->restrictions()->delete(); + if ($request->has('restrictions')) { + foreach($request->get('restrictions') as $roleId => $restrictions) { + foreach ($restrictions as $action => $value) { + $page->restrictions()->create([ + 'role_id' => $roleId, + 'action' => strtolower($action) + ]); + } + } + } + $page->save(); + } + } diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 15813b3e1..01cf80d29 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -164,4 +164,14 @@ class UserRepo ]; } + /** + * Get all the roles which can be given restricted access to + * other entities in the system. + * @return mixed + */ + public function getRestrictableRoles() + { + return $this->role->where('name', '!=', 'admin')->get(); + } + } \ No newline at end of file diff --git a/app/Restriction.php b/app/Restriction.php new file mode 100644 index 000000000..58d117997 --- /dev/null +++ b/app/Restriction.php @@ -0,0 +1,21 @@ +morphTo(); + } +} diff --git a/database/migrations/2016_02_27_120329_update_permissions_and_roles.php b/database/migrations/2016_02_27_120329_update_permissions_and_roles.php index 9fb2e98f2..ea3735d9e 100644 --- a/database/migrations/2016_02_27_120329_update_permissions_and_roles.php +++ b/database/migrations/2016_02_27_120329_update_permissions_and_roles.php @@ -26,7 +26,9 @@ class UpdatePermissionsAndRoles extends Migration $permissionsToCreate = [ 'settings-manage' => 'Manage Settings', 'users-manage' => 'Manage Users', - 'user-roles-manage' => 'Manage Roles & Permissions' + 'user-roles-manage' => 'Manage Roles & Permissions', + 'restrictions-manage-all' => 'Manage All Entity Restrictions', + 'restrictions-manage-own' => 'Manage Entity Restrictions On Own Content' ]; foreach ($permissionsToCreate as $name => $displayName) { $newPermission = new \BookStack\Permission(); diff --git a/database/migrations/2016_02_28_084200_add_entity_access_controls.php b/database/migrations/2016_02_28_084200_add_entity_access_controls.php new file mode 100644 index 000000000..5df2353a2 --- /dev/null +++ b/database/migrations/2016_02_28_084200_add_entity_access_controls.php @@ -0,0 +1,73 @@ +integer('uploaded_to')->default(0); + $table->index('uploaded_to'); + }); + + Schema::table('books', function (Blueprint $table) { + $table->boolean('restricted')->default(false); + $table->index('restricted'); + }); + + Schema::table('chapters', function (Blueprint $table) { + $table->boolean('restricted')->default(false); + $table->index('restricted'); + }); + + Schema::table('pages', function (Blueprint $table) { + $table->boolean('restricted')->default(false); + $table->index('restricted'); + }); + + Schema::create('restrictions', function(Blueprint $table) { + $table->increments('id'); + $table->integer('restrictable_id'); + $table->string('restrictable_type'); + $table->integer('role_id'); + $table->string('action'); + $table->index('role_id'); + $table->index('action'); + $table->index(['restrictable_id', 'restrictable_type']); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('images', function (Blueprint $table) { + $table->dropColumn('uploaded_to'); + }); + + Schema::table('books', function (Blueprint $table) { + $table->dropColumn('restricted'); + }); + + Schema::table('chapters', function (Blueprint $table) { + $table->dropColumn('restricted'); + }); + + + Schema::table('pages', function (Blueprint $table) { + $table->dropColumn('restricted'); + }); + + Schema::drop('restrictions'); + } +} diff --git a/resources/views/books/restrictions.blade.php b/resources/views/books/restrictions.blade.php new file mode 100644 index 000000000..826f218ce --- /dev/null +++ b/resources/views/books/restrictions.blade.php @@ -0,0 +1,10 @@ +@extends('base') + +@section('content') + +
    +

    Book Restrictions

    + @include('form/restriction-form', ['model' => $book]) +
    + +@stop diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index 12ebae0d6..f8a22ada8 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -17,6 +17,9 @@ Edit Sort @endif + @if(userCan('restrictions-manage', $book)) + Restrict + @endif @if(userCan('book-delete', $book)) Delete @endif diff --git a/resources/views/chapters/restrictions.blade.php b/resources/views/chapters/restrictions.blade.php new file mode 100644 index 000000000..3b19b55c8 --- /dev/null +++ b/resources/views/chapters/restrictions.blade.php @@ -0,0 +1,10 @@ +@extends('base') + +@section('content') + +
    +

    Chapter Restrictions

    + @include('form/restriction-form', ['model' => $chapter]) +
    + +@stop diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index 18a115ea5..ac36a6b3e 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -18,6 +18,9 @@ @if(userCan('chapter-update', $chapter)) Edit @endif + @if(userCan('restrictions-manage', $chapter)) + Restrict + @endif @if(userCan('chapter-delete', $chapter)) Delete @endif diff --git a/resources/views/form/checkbox.blade.php b/resources/views/form/checkbox.blade.php new file mode 100644 index 000000000..255896906 --- /dev/null +++ b/resources/views/form/checkbox.blade.php @@ -0,0 +1,12 @@ + + + +@if($errors->has($name)) +
    {{ $errors->first($name) }}
    +@endif \ No newline at end of file diff --git a/resources/views/form/restriction-checkbox.blade.php b/resources/views/form/restriction-checkbox.blade.php new file mode 100644 index 000000000..a4449ccb8 --- /dev/null +++ b/resources/views/form/restriction-checkbox.blade.php @@ -0,0 +1,7 @@ + + \ No newline at end of file diff --git a/resources/views/form/restriction-form.blade.php b/resources/views/form/restriction-form.blade.php new file mode 100644 index 000000000..cb42497b4 --- /dev/null +++ b/resources/views/form/restriction-form.blade.php @@ -0,0 +1,28 @@ +
    + {!! csrf_field() !!} + + +
    + @include('form/checkbox', ['name' => 'restricted', 'label' => 'Restrict this page?']) +
    + + + + + + + @foreach($roles as $role) + + + + @if(!$model->isA('page')) + + @endif + + + + @endforeach +
    RoleisA('page')) colspan="3" @else colspan="4" @endif>Actions
    {{ $role->display_name }}@include('form/restriction-checkbox', ['name'=>'restrictions', 'label' => 'View', 'action' => 'view'])@include('form/restriction-checkbox', ['name'=>'restrictions', 'label' => 'Create', 'action' => 'create'])@include('form/restriction-checkbox', ['name'=>'restrictions', 'label' => 'Update', 'action' => 'update'])@include('form/restriction-checkbox', ['name'=>'restrictions', 'label' => 'Delete', 'action' => 'delete'])
    + + +
    \ No newline at end of file diff --git a/resources/views/pages/restrictions.blade.php b/resources/views/pages/restrictions.blade.php new file mode 100644 index 000000000..63ad1fade --- /dev/null +++ b/resources/views/pages/restrictions.blade.php @@ -0,0 +1,10 @@ +@extends('base') + +@section('content') + +
    +

    Page Restrictions

    + @include('form/restriction-form', ['model' => $page]) +
    + +@stop diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 52b9daee6..f84d1963d 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -22,17 +22,20 @@
    Export
     @if(userCan('page-update', $page)) - Revisions - Edit + Revisions + Edit + @endif + @if(userCan('restrictions-manage', $page)) + Restrict @endif @if(userCan('page-delete', $page)) - Delete + Delete @endif
    diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 3069896b9..0758f317a 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -12,13 +12,28 @@ @include('form/text', ['name' => 'description'])
    -
    - -
    - -
    - +
    +
    + +
    +
    + +
    +
    +
    +
    + +
    +
    + +
    +
    +
    +
    + +
    +
    From 7f5872372d0228f6ad6604cb433935b41c9ae94a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 28 Feb 2016 19:03:04 +0000 Subject: [PATCH 5/9] Added in restriction queries for most lists --- app/Activity.php | 6 +- app/Providers/CustomFacadeProvider.php | 11 +- app/Repos/BookRepo.php | 58 +++-- app/Repos/ChapterRepo.php | 27 +- app/Repos/EntityRepo.php | 24 +- app/Repos/PageRepo.php | 51 ++-- app/Services/ActivityService.php | 13 +- app/Services/RestrictionService.php | 230 ++++++++++++++++++ app/Services/ViewService.php | 32 ++- .../views/partials/activity-item.blade.php | 2 +- 10 files changed, 359 insertions(+), 95 deletions(-) create mode 100644 app/Services/RestrictionService.php diff --git a/app/Activity.php b/app/Activity.php index 34daa2760..a1fe608f0 100644 --- a/app/Activity.php +++ b/app/Activity.php @@ -19,11 +19,7 @@ class Activity extends Model */ public function entity() { - if ($this->entity_id) { - return $this->morphTo('entity')->first(); - } else { - return false; - } + return $this->morphTo('entity'); } /** diff --git a/app/Providers/CustomFacadeProvider.php b/app/Providers/CustomFacadeProvider.php index 1df14a076..9b290039c 100644 --- a/app/Providers/CustomFacadeProvider.php +++ b/app/Providers/CustomFacadeProvider.php @@ -28,11 +28,17 @@ class CustomFacadeProvider extends ServiceProvider public function register() { $this->app->bind('activity', function() { - return new ActivityService($this->app->make('BookStack\Activity')); + return new ActivityService( + $this->app->make('BookStack\Activity'), + $this->app->make('BookStack\Services\RestrictionService') + ); }); $this->app->bind('views', function() { - return new ViewService($this->app->make('BookStack\View')); + return new ViewService( + $this->app->make('BookStack\View'), + $this->app->make('BookStack\Services\RestrictionService') + ); }); $this->app->bind('setting', function() { @@ -41,6 +47,7 @@ class CustomFacadeProvider extends ServiceProvider $this->app->make('Illuminate\Contracts\Cache\Repository') ); }); + $this->app->bind('images', function() { return new ImageService( $this->app->make('Intervention\Image\ImageManager'), diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index dbf95623e..572030d43 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -1,6 +1,7 @@ book = $book; $this->pageRepo = $pageRepo; $this->chapterRepo = $chapterRepo; + $this->restrictionService = $restrictionService; + } + + /** + * Base query for getting books. + * Takes into account any restrictions. + * @return mixed + */ + private function bookQuery() + { + return $this->restrictionService->enforceBookRestrictions($this->book, 'view'); } /** @@ -32,7 +46,7 @@ class BookRepo */ public function getById($id) { - return $this->book->findOrFail($id); + return $this->bookQuery()->findOrFail($id); } /** @@ -42,7 +56,7 @@ class BookRepo */ public function getAll($count = 10) { - $bookQuery = $this->book->orderBy('name', 'asc'); + $bookQuery = $this->bookQuery()->orderBy('name', 'asc'); if (!$count) return $bookQuery->get(); return $bookQuery->take($count)->get(); } @@ -54,7 +68,8 @@ class BookRepo */ public function getAllPaginated($count = 10) { - return $this->book->orderBy('name', 'asc')->paginate($count); + return $this->bookQuery() + ->orderBy('name', 'asc')->paginate($count); } @@ -65,7 +80,7 @@ class BookRepo */ public function getLatest($count = 10) { - return $this->book->orderBy('created_at', 'desc')->take($count)->get(); + return $this->bookQuery()->orderBy('created_at', 'desc')->take($count)->get(); } /** @@ -76,6 +91,7 @@ class BookRepo */ public function getRecentlyViewed($count = 10, $page = 0) { + // TODO restrict return Views::getUserRecentlyViewed($count, $page, $this->book); } @@ -87,6 +103,7 @@ class BookRepo */ public function getPopular($count = 10, $page = 0) { + // TODO - Restrict return Views::getPopular($count, $page, $this->book); } @@ -97,7 +114,7 @@ class BookRepo */ public function getBySlug($slug) { - $book = $this->book->where('slug', '=', $slug)->first(); + $book = $this->bookQuery()->where('slug', '=', $slug)->first(); if ($book === null) abort(404); return $book; } @@ -109,7 +126,7 @@ class BookRepo */ public function exists($id) { - return $this->book->where('id', '=', $id)->exists(); + return $this->bookQuery()->where('id', '=', $id)->exists(); } /** @@ -119,17 +136,7 @@ class BookRepo */ public function newFromInput($input) { - return $this->book->fill($input); - } - - /** - * Count the amount of books that have a specific slug. - * @param $slug - * @return mixed - */ - public function countBySlug($slug) - { - return $this->book->where('slug', '=', $slug)->count(); + return $this->bookQuery()->fill($input); } /** @@ -202,8 +209,13 @@ class BookRepo */ public function getChildren(Book $book) { - $pages = $book->pages()->where('chapter_id', '=', 0)->get(); - $chapters = $book->chapters()->with('pages')->get(); + $pageQuery = $book->pages()->where('chapter_id', '=', 0); + $this->restrictionService->enforcePageRestrictions($pageQuery, 'view'); + $pages = $pageQuery->get(); + + $chapterQuery = $book->chapters()->with('pages'); + $this->restrictionService->enforceChapterRestrictions($chapterQuery, 'view'); + $chapters = $chapterQuery->get(); $children = $pages->merge($chapters); $bookSlug = $book->slug; $children->each(function ($child) use ($bookSlug) { @@ -227,7 +239,7 @@ class BookRepo public function getBySearch($term, $count = 20, $paginationAppends = []) { $terms = explode(' ', $term); - $books = $this->book->fullTextSearchQuery(['name', 'description'], $terms) + $books = $this->restrictionService->enforceBookRestrictions($this->book->fullTextSearchQuery(['name', 'description'], $terms)) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); foreach ($books as $book) { @@ -249,11 +261,11 @@ class BookRepo $book->restricted = $request->has('restricted') && $request->get('restricted') === 'true'; $book->restrictions()->delete(); if ($request->has('restrictions')) { - foreach($request->get('restrictions') as $roleId => $restrictions) { + foreach ($request->get('restrictions') as $roleId => $restrictions) { foreach ($restrictions as $action => $value) { $book->restrictions()->create([ 'role_id' => $roleId, - 'action' => strtolower($action) + 'action' => strtolower($action) ]); } } diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index 7e3560f2b..90f2f8c54 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -2,6 +2,7 @@ use Activity; +use BookStack\Services\RestrictionService; use Illuminate\Support\Str; use BookStack\Chapter; @@ -9,14 +10,26 @@ class ChapterRepo { protected $chapter; + protected $restrictionService; /** * ChapterRepo constructor. - * @param $chapter + * @param Chapter $chapter + * @param RestrictionService $restrictionService */ - public function __construct(Chapter $chapter) + public function __construct(Chapter $chapter, RestrictionService $restrictionService) { $this->chapter = $chapter; + $this->restrictionService = $restrictionService; + } + + /** + * Base query for getting chapters, Takes restrictions into account. + * @return mixed + */ + private function chapterQuery() + { + return $this->restrictionService->enforceChapterRestrictions($this->chapter, 'view'); } /** @@ -26,7 +39,7 @@ class ChapterRepo */ public function idExists($id) { - return $this->chapter->where('id', '=', $id)->count() > 0; + return $this->chapterQuery()->where('id', '=', $id)->count() > 0; } /** @@ -36,7 +49,7 @@ class ChapterRepo */ public function getById($id) { - return $this->chapter->findOrFail($id); + return $this->chapterQuery()->findOrFail($id); } /** @@ -45,7 +58,7 @@ class ChapterRepo */ public function getAll() { - return $this->chapter->all(); + return $this->chapterQuery()->all(); } /** @@ -56,7 +69,7 @@ class ChapterRepo */ public function getBySlug($slug, $bookId) { - $chapter = $this->chapter->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); + $chapter = $this->chapterQuery()->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); if ($chapter === null) abort(404); return $chapter; } @@ -132,7 +145,7 @@ class ChapterRepo public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { $terms = explode(' ', $term); - $chapters = $this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms) + $chapters = $this->restrictionService->enforceChapterRestrictions($this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms)) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); foreach ($chapters as $chapter) { diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index 28942d94a..46e1d98a5 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -4,6 +4,7 @@ use BookStack\Book; use BookStack\Chapter; use BookStack\Page; +use BookStack\Services\RestrictionService; class EntityRepo { @@ -11,18 +12,21 @@ class EntityRepo public $book; public $chapter; public $page; + private $restrictionService; /** * EntityService constructor. - * @param $book - * @param $chapter - * @param $page + * @param Book $book + * @param Chapter $chapter + * @param Page $page + * @param RestrictionService $restrictionService */ - public function __construct(Book $book, Chapter $chapter, Page $page) + public function __construct(Book $book, Chapter $chapter, Page $page, RestrictionService $restrictionService) { $this->book = $book; $this->chapter = $chapter; $this->page = $page; + $this->restrictionService = $restrictionService; } /** @@ -32,7 +36,8 @@ class EntityRepo */ public function getRecentlyCreatedBooks($count = 20, $page = 0) { - return $this->book->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); + return $this->restrictionService->enforceBookRestrictions($this->book) + ->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); } /** @@ -43,7 +48,8 @@ class EntityRepo */ public function getRecentlyUpdatedBooks($count = 20, $page = 0) { - return $this->book->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); + return $this->restrictionService->enforceBookRestrictions($this->book) + ->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); } /** @@ -53,7 +59,8 @@ class EntityRepo */ public function getRecentlyCreatedPages($count = 20, $page = 0) { - return $this->page->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); + return $this->restrictionService->enforcePageRestrictions($this->page) + ->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); } /** @@ -64,7 +71,8 @@ class EntityRepo */ public function getRecentlyUpdatedPages($count = 20, $page = 0) { - return $this->page->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); + return $this->restrictionService->enforcePageRestrictions($this->page) + ->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); } diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index 1a98255ab..c4cf00e7c 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -4,6 +4,7 @@ use Activity; use BookStack\Book; use BookStack\Chapter; +use BookStack\Services\RestrictionService; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; @@ -16,26 +17,28 @@ class PageRepo { protected $page; protected $pageRevision; + protected $restrictionService; /** * PageRepo constructor. - * @param Page $page + * @param Page $page * @param PageRevision $pageRevision + * @param RestrictionService $restrictionService */ - public function __construct(Page $page, PageRevision $pageRevision) + public function __construct(Page $page, PageRevision $pageRevision, RestrictionService $restrictionService) { $this->page = $page; $this->pageRevision = $pageRevision; + $this->restrictionService = $restrictionService; } /** - * Check if a page id exists. - * @param $id - * @return bool + * Base query for getting pages, Takes restrictions into account. + * @return mixed */ - public function idExists($id) + private function pageQuery() { - return $this->page->where('page_id', '=', $id)->count() > 0; + return $this->restrictionService->enforcePageRestrictions($this->page, 'view'); } /** @@ -45,16 +48,7 @@ class PageRepo */ public function getById($id) { - return $this->page->findOrFail($id); - } - - /** - * Get all pages. - * @return \Illuminate\Database\Eloquent\Collection|static[] - */ - public function getAll() - { - return $this->page->all(); + return $this->pageQuery()->findOrFail($id); } /** @@ -65,7 +59,7 @@ class PageRepo */ public function getBySlug($slug, $bookId) { - $page = $this->page->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); + $page = $this->pageQuery()->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); if ($page === null) throw new NotFoundHttpException('Page not found'); return $page; } @@ -81,6 +75,9 @@ class PageRepo public function findPageUsingOldSlug($pageSlug, $bookSlug) { $revision = $this->pageRevision->where('slug', '=', $pageSlug) + ->whereHas('page', function($query) { + $this->restrictionService->enforcePageRestrictions($query); + }) ->where('book_slug', '=', $bookSlug)->orderBy('created_at', 'desc') ->with('page')->first(); return $revision !== null ? $revision->page : null; @@ -97,16 +94,6 @@ class PageRepo return $page; } - /** - * Count the pages with a particular slug within a book. - * @param $slug - * @param $bookId - * @return mixed - */ - public function countBySlug($slug, $bookId) - { - return $this->page->where('slug', '=', $slug)->where('book_id', '=', $bookId)->count(); - } /** * Save a new page into the system. @@ -202,7 +189,7 @@ class PageRepo public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { $terms = explode(' ', $term); - $pages = $this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms) + $pages = $this->restrictionService->enforcePageRestrictions($this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms)) ->paginate($count)->appends($paginationAppends); // Add highlights to page text. @@ -240,7 +227,7 @@ class PageRepo */ public function searchForImage($imageString) { - $pages = $this->page->where('html', 'like', '%' . $imageString . '%')->get(); + $pages = $this->pageQuery()->where('html', 'like', '%' . $imageString . '%')->get(); foreach ($pages as $page) { $page->url = $page->getUrl(); $page->html = ''; @@ -395,7 +382,7 @@ class PageRepo */ public function getRecentlyCreatedPaginated($count = 20) { - return $this->page->orderBy('created_at', 'desc')->paginate($count); + return $this->pageQuery()->orderBy('created_at', 'desc')->paginate($count); } /** @@ -404,7 +391,7 @@ class PageRepo */ public function getRecentlyUpdatedPaginated($count = 20) { - return $this->page->orderBy('updated_at', 'desc')->paginate($count); + return $this->pageQuery()->orderBy('updated_at', 'desc')->paginate($count); } /** diff --git a/app/Services/ActivityService.php b/app/Services/ActivityService.php index a065ae01f..a57210b8d 100644 --- a/app/Services/ActivityService.php +++ b/app/Services/ActivityService.php @@ -9,14 +9,17 @@ class ActivityService { protected $activity; protected $user; + protected $restrictionService; /** * ActivityService constructor. - * @param $activity + * @param Activity $activity + * @param RestrictionService $restrictionService */ - public function __construct(Activity $activity) + public function __construct(Activity $activity, RestrictionService $restrictionService) { $this->activity = $activity; + $this->restrictionService = $restrictionService; $this->user = auth()->user(); } @@ -86,8 +89,10 @@ class ActivityService */ public function latest($count = 20, $page = 0) { - $activityList = $this->activity->orderBy('created_at', 'desc') - ->skip($count * $page)->take($count)->get(); + $activityList = $this->restrictionService + ->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type') + ->orderBy('created_at', 'desc')->skip($count * $page)->take($count)->get(); + return $this->filterSimilar($activityList); } diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php new file mode 100644 index 000000000..87dcbf0b3 --- /dev/null +++ b/app/Services/RestrictionService.php @@ -0,0 +1,230 @@ +userRoles = auth()->user()->roles->pluck('id'); + $this->isAdmin = auth()->user()->hasRole('admin'); + } + + /** + * Add restrictions for a page query + * @param $query + * @param string $action + * @return mixed + */ + public function enforcePageRestrictions($query, $action = 'view') + { + if ($this->isAdmin) return $query; + $this->currentAction = $action; + return $this->pageRestrictionQuery($query); + } + + /** + * The base query for restricting pages. + * @param $query + * @return mixed + */ + private function pageRestrictionQuery($query) + { + return $query->where(function ($parentWhereQuery) { + + $parentWhereQuery + // (Book & chapter & page) or (Book & page & NO CHAPTER) unrestricted + ->where(function ($query) { + $query->where(function ($query) { + $query->whereExists(function ($query) { + $query->select('*')->from('chapters') + ->whereRaw('chapters.id=pages.chapter_id') + ->where('restricted', '=', false); + })->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=pages.book_id') + ->where('restricted', '=', false); + })->where('restricted', '=', false); + })->orWhere(function ($query) { + $query->where('restricted', '=', false)->where('chapter_id', '=', 0) + ->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=pages.book_id') + ->where('restricted', '=', false); + }); + }); + }) + // Page unrestricted, Has no chapter & book has accepted restrictions + ->orWhere(function ($query) { + $query->where('restricted', '=', false) + ->whereExists(function ($query) { + $query->select('*')->from('chapters') + ->whereRaw('chapters.id=pages.chapter_id'); + }, 'and', true) + ->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=pages.book_id') + ->whereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'books', 'Book'); + }); + }); + }) + // Page unrestricted, Has a chapter with accepted permissions + ->orWhere(function ($query) { + $query->where('restricted', '=', false) + ->whereExists(function ($query) { + $query->select('*')->from('chapters') + ->whereRaw('chapters.id=pages.chapter_id') + ->whereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'chapters', 'Chapter'); + }); + }); + }) + // Page has accepted permissions + ->orWhereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'pages', 'Page'); + }); + }); + } + + /** + * Add on permission restrictions to a chapter query. + * @param $query + * @param string $action + * @return mixed + */ + public function enforceChapterRestrictions($query, $action = 'view') + { + if ($this->isAdmin) return $query; + $this->currentAction = $action; + return $this->chapterRestrictionQuery($query); + } + + /** + * The base query for restricting chapters. + * @param $query + * @return mixed + */ + private function chapterRestrictionQuery($query) + { + return $query->where(function ($parentWhereQuery) { + + $parentWhereQuery + // Book & chapter unrestricted + ->where(function ($query) { + $query->where('restricted', '=', false)->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=chapters.book_id') + ->where('restricted', '=', false); + }); + }) + // Chapter unrestricted & book has accepted restrictions + ->orWhere(function ($query) { + $query->where('restricted', '=', false) + ->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=chapters.book_id') + ->whereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'books', 'Book'); + }); + }); + }) + // Chapter has accepted permissions + ->orWhereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'chapters', 'Chapter'); + }); + }); + } + + /** + * Add restrictions to a book query. + * @param $query + * @param string $action + * @return mixed + */ + public function enforceBookRestrictions($query, $action = 'view') + { + if ($this->isAdmin) return $query; + $this->currentAction = $action; + return $this->bookRestrictionQuery($query); + } + + /** + * The base query for restricting books. + * @param $query + * @return mixed + */ + private function bookRestrictionQuery($query) + { + return $query->where(function ($parentWhereQuery) { + $parentWhereQuery + ->where('restricted', '=', false) + ->orWhereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'books', 'Book'); + }); + }); + } + + /** + * Filter items that have entities set a a polymorphic relation. + * @param $query + * @param string $tableName + * @param string $entityIdColumn + * @param string $entityTypeColumn + * @return mixed + */ + public function filterRestrictedEntityRelations($query, $tableName, $entityIdColumn, $entityTypeColumn) + { + if ($this->isAdmin) return $query; + $this->currentAction = 'view'; + $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn]; + return $query->where(function($query) use ($tableDetails) { + $query->where(function ($query) use (&$tableDetails) { + $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Page') + ->whereExists(function ($query) use (&$tableDetails) { + $query->select('*')->from('pages')->whereRaw('pages.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + ->where(function ($query) { + $this->pageRestrictionQuery($query); + }); + }); + })->orWhere(function ($query) use (&$tableDetails) { + $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Book')->whereExists(function ($query) use (&$tableDetails) { + $query->select('*')->from('books')->whereRaw('books.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + ->where(function ($query) { + $this->bookRestrictionQuery($query); + }); + }); + })->orWhere(function ($query) use (&$tableDetails) { + $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Chapter')->whereExists(function ($query) use (&$tableDetails) { + $query->select('*')->from('chapters')->whereRaw('chapters.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + ->where(function ($query) { + $this->chapterRestrictionQuery($query); + }); + }); + }); + }); + } + + /** + * The query to check the restrictions on an entity. + * @param $query + * @param $tableName + * @param $modelName + */ + private function checkRestrictionsQuery($query, $tableName, $modelName) + { + $query->select('*')->from('restrictions') + ->whereRaw('restrictions.restrictable_id=' . $tableName . '.id') + ->where('restrictions.restrictable_type', '=', 'BookStack\\' . $modelName) + ->where('restrictions.action', '=', $this->currentAction) + ->whereIn('restrictions.role_id', $this->userRoles); + } + + +} \ No newline at end of file diff --git a/app/Services/ViewService.php b/app/Services/ViewService.php index 5b800d939..75ffd21dc 100644 --- a/app/Services/ViewService.php +++ b/app/Services/ViewService.php @@ -9,15 +9,18 @@ class ViewService protected $view; protected $user; + protected $restrictionService; /** * ViewService constructor. - * @param $view + * @param View $view + * @param RestrictionService $restrictionService */ - public function __construct(View $view) + public function __construct(View $view, RestrictionService $restrictionService) { $this->view = $view; $this->user = auth()->user(); + $this->restrictionService = $restrictionService; } /** @@ -27,7 +30,7 @@ class ViewService */ public function add(Entity $entity) { - if($this->user === null) return 0; + if ($this->user === null) return 0; $view = $entity->views()->where('user_id', '=', $this->user->id)->first(); // Add view if model exists if ($view) { @@ -47,18 +50,19 @@ class ViewService /** * Get the entities with the most views. - * @param int $count - * @param int $page + * @param int $count + * @param int $page * @param bool|false $filterModel */ public function getPopular($count = 10, $page = 0, $filterModel = false) { $skipCount = $count * $page; - $query = $this->view->select('id', 'viewable_id', 'viewable_type', \DB::raw('SUM(views) as view_count')) + $query = $this->restrictionService->filterRestrictedEntityRelations($this->view, 'views', 'viewable_id', 'viewable_type') + ->select('id', 'viewable_id', 'viewable_type', \DB::raw('SUM(views) as view_count')) ->groupBy('viewable_id', 'viewable_type') ->orderBy('view_count', 'desc'); - if($filterModel) $query->where('viewable_type', '=', get_class($filterModel)); + if ($filterModel) $query->where('viewable_type', '=', get_class($filterModel)); $views = $query->with('viewable')->skip($skipCount)->take($count)->get(); $viewedEntities = $views->map(function ($item) { @@ -69,22 +73,24 @@ class ViewService /** * Get all recently viewed entities for the current user. - * @param int $count - * @param int $page + * @param int $count + * @param int $page * @param Entity|bool $filterModel * @return mixed */ public function getUserRecentlyViewed($count = 10, $page = 0, $filterModel = false) { - if($this->user === null) return collect(); + if ($this->user === null) return collect(); $skipCount = $count * $page; - $query = $this->view->where('user_id', '=', auth()->user()->id); + $query = $this->restrictionService + ->filterRestrictedEntityRelations($this->view, 'views', 'viewable_id', 'viewable_type'); - if ($filterModel) $query->where('viewable_type', '=', get_class($filterModel)); + if ($filterModel) $query = $query->where('viewable_type', '=', get_class($filterModel)); + $query = $query->where('user_id', '=', auth()->user()->id); $views = $query->with('viewable')->orderBy('updated_at', 'desc')->skip($skipCount)->take($count)->get(); $viewedEntities = $views->map(function ($item) { - return $item->viewable()->getResults(); + return $item->viewable; }); return $viewedEntities; } diff --git a/resources/views/partials/activity-item.blade.php b/resources/views/partials/activity-item.blade.php index f94fad5e1..00ca574dd 100644 --- a/resources/views/partials/activity-item.blade.php +++ b/resources/views/partials/activity-item.blade.php @@ -17,7 +17,7 @@ {{ $activity->getText() }} @if($activity->entity()) - {{ $activity->entity()->name }} + {{ $activity->entity->name }} @endif @if($activity->extra) "{{$activity->extra}}" @endif From 985d2f1c2cee6a1192df388c70154ccee9114795 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 29 Feb 2016 20:31:21 +0000 Subject: [PATCH 6/9] Tied entity restriction system into userCan checks --- app/Http/Controllers/Controller.php | 8 +++----- app/Services/RestrictionService.php | 30 +++++++++++++++++++++++------ app/helpers.php | 16 ++++++++++++--- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 4182743a7..f0cb47cd9 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -68,7 +68,7 @@ abstract class Controller extends BaseController protected function showPermissionError() { Session::flash('error', trans('errors.permission')); - $response = request()->wantsJson() ? response()->json(['error' => trans('errors.permissionJson')], 403) : redirect('/', 403); + $response = request()->wantsJson() ? response()->json(['error' => trans('errors.permissionJson')], 403) : redirect('/'); throw new HttpResponseException($response); } @@ -93,10 +93,8 @@ abstract class Controller extends BaseController */ protected function checkOwnablePermission($permission, Ownable $ownable) { - $permissionBaseName = strtolower($permission) . '-'; - if (userCan($permissionBaseName . 'all')) return true; - if (userCan($permissionBaseName . 'own') && $ownable->createdBy->id === $this->currentUser->id) return true; - $this->showPermissionError(); + if (userCan($permission, $ownable)) return true; + return $this->showPermissionError(); } /** diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 87dcbf0b3..0ef80b229 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -1,5 +1,7 @@ userRoles = auth()->user()->roles->pluck('id'); - $this->isAdmin = auth()->user()->hasRole('admin'); + $user = auth()->user(); + $this->userRoles = $user ? auth()->user()->roles->pluck('id') : false; + $this->isAdmin = $user ? auth()->user()->hasRole('admin') : false; + } + + public function checkIfEntityRestricted(Entity $entity, $action) + { + if ($this->isAdmin) return true; + $this->currentAction = $action; + $baseQuery = $entity->where('id', '=', $entity->id); + if ($entity->isA('page')) { + return $this->pageRestrictionQuery($baseQuery)->count() > 0; + } elseif ($entity->isA('chapter')) { + return $this->chapterRestrictionQuery($baseQuery)->count() > 0; + } elseif ($entity->isA('book')) { + return $this->bookRestrictionQuery($baseQuery)->count() > 0; + } + return false; } /** @@ -184,25 +202,25 @@ class RestrictionService if ($this->isAdmin) return $query; $this->currentAction = 'view'; $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn]; - return $query->where(function($query) use ($tableDetails) { + return $query->where(function ($query) use ($tableDetails) { $query->where(function ($query) use (&$tableDetails) { $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Page') ->whereExists(function ($query) use (&$tableDetails) { - $query->select('*')->from('pages')->whereRaw('pages.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + $query->select('*')->from('pages')->whereRaw('pages.id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->where(function ($query) { $this->pageRestrictionQuery($query); }); }); })->orWhere(function ($query) use (&$tableDetails) { $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Book')->whereExists(function ($query) use (&$tableDetails) { - $query->select('*')->from('books')->whereRaw('books.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + $query->select('*')->from('books')->whereRaw('books.id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->where(function ($query) { $this->bookRestrictionQuery($query); }); }); })->orWhere(function ($query) use (&$tableDetails) { $query->where($tableDetails['entityTypeColumn'], '=', 'BookStack\Chapter')->whereExists(function ($query) use (&$tableDetails) { - $query->select('*')->from('chapters')->whereRaw('chapters.id='.$tableDetails['tableName'].'.'.$tableDetails['entityIdColumn']) + $query->select('*')->from('chapters')->whereRaw('chapters.id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->where(function ($query) { $this->chapterRestrictionQuery($query); }); diff --git a/app/helpers.php b/app/helpers.php index db65407c7..8f080c5e1 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -43,8 +43,18 @@ function userCan($permission, \BookStack\Ownable $ownable = null) return auth()->user() && auth()->user()->can($permission); } + // Check permission on ownable item $permissionBaseName = strtolower($permission) . '-'; - if (userCan($permissionBaseName . 'all')) return true; - if (userCan($permissionBaseName . 'own') && $ownable->createdBy->id === auth()->user()->id) return true; - return false; + $hasPermission = false; + if (auth()->user()->can($permissionBaseName . 'all')) $hasPermission = true; + if (auth()->user()->can($permissionBaseName . 'own') && $ownable->createdBy->id === auth()->user()->id) $hasPermission = true; + + if(!$ownable instanceof \BookStack\Entity) return $hasPermission; + + // Check restrictions on the entitiy + $restrictionService = app('BookStack\Services\RestrictionService'); + $explodedPermission = explode('-', $permission); + $action = end($explodedPermission); + $hasAccess = $restrictionService->checkIfEntityRestricted($ownable, $action); + return $hasAccess && $hasPermission; } \ No newline at end of file From 8e274a5a84cc11c20c0d62f9ecc0b08cd9410305 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Mar 2016 22:35:01 +0000 Subject: [PATCH 7/9] Refactored some permission controls and increased testing for roles system --- app/Exceptions/PermissionsException.php | 6 + app/Http/Controllers/PermissionController.php | 89 +++-------- app/Repos/PermissionsRepo.php | 141 ++++++++++++++++++ app/Role.php | 14 -- app/User.php | 2 +- database/factories/ModelFactory.php | 8 + tests/RolesTest.php | 45 +++++- tests/TestCase.php | 4 +- 8 files changed, 216 insertions(+), 93 deletions(-) create mode 100644 app/Exceptions/PermissionsException.php create mode 100644 app/Repos/PermissionsRepo.php diff --git a/app/Exceptions/PermissionsException.php b/app/Exceptions/PermissionsException.php new file mode 100644 index 000000000..ae4c676d6 --- /dev/null +++ b/app/Exceptions/PermissionsException.php @@ -0,0 +1,6 @@ +role = $role; - $this->permission = $permission; + $this->permissionsRepo = $permissionsRepo; parent::__construct(); } @@ -32,7 +26,7 @@ class PermissionController extends Controller public function listRoles() { $this->checkPermission('user-roles-manage'); - $roles = $this->role->all(); + $roles = $this->permissionsRepo->getAllRoles(); return view('settings/roles/index', ['roles' => $roles]); } @@ -59,22 +53,7 @@ class PermissionController extends Controller 'description' => 'max:250' ]); - $role = $this->role->newInstance($request->all()); - $role->name = str_replace(' ', '-', strtolower($request->get('display_name'))); - // Prevent duplicate names - while ($this->role->where('name', '=', $role->name)->count() > 0) { - $role->name .= strtolower(str_random(2)); - } - $role->save(); - - if ($request->has('permissions')) { - $permissionsNames = array_keys($request->get('permissions')); - $permissions = $this->permission->whereIn('name', $permissionsNames)->pluck('id')->toArray(); - $role->permissions()->sync($permissions); - } else { - $role->permissions()->sync([]); - } - + $this->permissionsRepo->saveNewRole($request->all()); session()->flash('success', 'Role successfully created'); return redirect('/settings/roles'); } @@ -87,7 +66,7 @@ class PermissionController extends Controller public function editRole($id) { $this->checkPermission('user-roles-manage'); - $role = $this->role->findOrFail($id); + $role = $this->permissionsRepo->getRoleById($id); return view('settings/roles/edit', ['role' => $role]); } @@ -105,24 +84,7 @@ class PermissionController extends Controller 'description' => 'max:250' ]); - $role = $this->role->findOrFail($id); - if ($request->has('permissions')) { - $permissionsNames = array_keys($request->get('permissions')); - $permissions = $this->permission->whereIn('name', $permissionsNames)->pluck('id')->toArray(); - $role->permissions()->sync($permissions); - } else { - $role->permissions()->sync([]); - } - - // Ensure admin account always has all permissions - if ($role->name === 'admin') { - $permissions = $this->permission->all()->pluck('id')->toArray(); - $role->permissions()->sync($permissions); - } - - $role->fill($request->all()); - $role->save(); - + $this->permissionsRepo->updateRole($id, $request->all()); session()->flash('success', 'Role successfully updated'); return redirect('/settings/roles'); } @@ -136,9 +98,9 @@ class PermissionController extends Controller public function showDeleteRole($id) { $this->checkPermission('user-roles-manage'); - $role = $this->role->findOrFail($id); - $roles = $this->role->where('id', '!=', $id)->get(); - $blankRole = $this->role->newInstance(['display_name' => 'Don\'t migrate users']); + $role = $this->permissionsRepo->getRoleById($id); + $roles = $this->permissionsRepo->getAllRolesExcept($role); + $blankRole = $role->newInstance(['display_name' => 'Don\'t migrate users']); $roles->prepend($blankRole); return view('settings/roles/delete', ['role' => $role, 'roles' => $roles]); } @@ -153,29 +115,14 @@ class PermissionController extends Controller public function deleteRole($id, Request $request) { $this->checkPermission('user-roles-manage'); - $role = $this->role->findOrFail($id); - // Prevent deleting admin role - if ($role->name === 'admin') { - session()->flash('error', 'The admin role cannot be deleted'); + try { + $this->permissionsRepo->deleteRole($id, $request->get('migrate_role_id')); + } catch (PermissionsException $e) { + session()->flash('error', $e->getMessage()); return redirect()->back(); } - if ($role->id == \Setting::get('registration-role')) { - session()->flash('error', 'This role cannot be deleted while set as the default registration role.'); - return redirect()->back(); - } - - if ($request->has('migration_role_id')) { - $newRole = $this->role->find($request->get('migration_role_id')); - if ($newRole) { - $users = $role->users->pluck('id')->toArray(); - $newRole->users()->sync($users); - } - } - - $role->delete(); - session()->flash('success', 'Role successfully deleted'); return redirect('/settings/roles'); } diff --git a/app/Repos/PermissionsRepo.php b/app/Repos/PermissionsRepo.php new file mode 100644 index 000000000..c35f29d10 --- /dev/null +++ b/app/Repos/PermissionsRepo.php @@ -0,0 +1,141 @@ +permission = $permission; + $this->role = $role; + } + + /** + * Get all the user roles from the system. + * @return \Illuminate\Database\Eloquent\Collection|static[] + */ + public function getAllRoles() + { + return $this->role->all(); + } + + /** + * Get all the roles except for the provided one. + * @param Role $role + * @return mixed + */ + public function getAllRolesExcept(Role $role) + { + return $this->role->where('id', '!=', $role->id)->get(); + } + + /** + * Get a role via its ID. + * @param $id + * @return mixed + */ + public function getRoleById($id) + { + return $this->role->findOrFail($id); + } + + /** + * Save a new role into the system. + * @param array $roleData + * @return Role + */ + public function saveNewRole($roleData) + { + $role = $this->role->newInstance($roleData); + $role->name = str_replace(' ', '-', strtolower($roleData['display_name'])); + // Prevent duplicate names + while ($this->role->where('name', '=', $role->name)->count() > 0) { + $role->name .= strtolower(str_random(2)); + } + $role->save(); + + $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : []; + $this->assignRolePermissions($role, $permissions); + return $role; + } + + /** + * Updates an existing role. + * Ensure Admin role always has all permissions. + * @param $roleId + * @param $roleData + */ + public function updateRole($roleId, $roleData) + { + $role = $this->role->findOrFail($roleId); + $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : []; + $this->assignRolePermissions($role, $permissions); + + if ($role->name === 'admin') { + $permissions = $this->permission->all()->pluck('id')->toArray(); + $role->permissions()->sync($permissions); + } + + $role->fill($roleData); + $role->save(); + } + + /** + * Assign an list of permission names to an role. + * @param Role $role + * @param array $permissionNameArray + */ + public function assignRolePermissions(Role $role, $permissionNameArray = []) + { + $permissions = []; + if ($permissionNameArray && count($permissionNameArray) > 0) { + $permissions = $this->permission->whereIn('name', $permissionNameArray)->pluck('id')->toArray(); + } + $role->permissions()->sync($permissions); + } + + /** + * Delete a role from the system. + * Check it's not an admin role or set as default before deleting. + * If an migration Role ID is specified the users assign to the current role + * will be added to the role of the specified id. + * @param $roleId + * @param $migrateRoleId + * @throws PermissionsException + */ + public function deleteRole($roleId, $migrateRoleId) + { + $role = $this->role->findOrFail($roleId); + + // Prevent deleting admin role or default registration role. + if ($role->name === 'admin') { + throw new PermissionsException('The admin role cannot be deleted'); + } else if ($role->id == Setting::get('registration-role')) { + throw new PermissionsException('This role cannot be deleted while set as the default registration role.'); + } + + if ($migrateRoleId) { + $newRole = $this->role->find($migrateRoleId); + if ($newRole) { + $users = $role->users->pluck('id')->toArray(); + $newRole->users()->sync($users); + } + } + + $role->delete(); + } + +} \ No newline at end of file diff --git a/app/Role.php b/app/Role.php index 8d5ed7d66..270e4e0b8 100644 --- a/app/Role.php +++ b/app/Role.php @@ -8,11 +8,6 @@ class Role extends Model { protected $fillable = ['display_name', 'description']; - /** - * Sets the default role name for newly registered users. - * @var string - */ - protected static $default = 'viewer'; /** * The roles that belong to the role. @@ -48,15 +43,6 @@ class Role extends Model $this->permissions()->attach($permission->id); } - /** - * Get an instance of the default role. - * @return Role - */ - public static function getDefault() - { - return static::getRole(static::$default); - } - /** * Get the role object for the specified role. * @param $roleName diff --git a/app/User.php b/app/User.php index b062aa78f..2d14c6e6e 100644 --- a/app/User.php +++ b/app/User.php @@ -106,7 +106,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function attachRoleId($id) { - $this->roles()->sync([$id]); + $this->roles()->attach([$id]); } /** diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index e0f155087..2840356e8 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -17,6 +17,7 @@ $factory->define(BookStack\User::class, function ($faker) { 'email' => $faker->email, 'password' => str_random(10), 'remember_token' => str_random(10), + 'email_confirmed' => 1 ]; }); @@ -45,3 +46,10 @@ $factory->define(BookStack\Page::class, function ($faker) { 'text' => strip_tags($html) ]; }); + +$factory->define(BookStack\Role::class, function ($faker) { + return [ + 'display_name' => $faker->sentence(3), + 'description' => $faker->sentence(10) + ]; +}); \ No newline at end of file diff --git a/tests/RolesTest.php b/tests/RolesTest.php index 140290bea..7349c2968 100644 --- a/tests/RolesTest.php +++ b/tests/RolesTest.php @@ -9,13 +9,14 @@ class RolesTest extends TestCase parent::setUp(); } + /** + * Create a new basic role for testing purposes. + * @return static + */ protected function createNewRole() { - return \BookStack\Role::forceCreate([ - 'name' => 'test-role', - 'display_name' => 'Test Role', - 'description' => 'This is a role for testing' - ]); + $permissionRepo = app('BookStack\Repos\PermissionsRepo'); + return $permissionRepo->saveNewRole(factory(\BookStack\Role::class)->make()->toArray()); } public function test_admin_can_see_settings() @@ -45,4 +46,38 @@ class RolesTest extends TestCase ->see('cannot be deleted'); } + public function test_role_create_update_delete_flow() + { + $testRoleName = 'Test Role'; + $testRoleDesc = 'a little test description'; + $testRoleUpdateName = 'An Super Updated role'; + + // Creation + $this->asAdmin()->visit('/settings') + ->click('Roles') + ->seePageIs('/settings/roles') + ->click('Add new role') + ->type('Test Role', 'display_name') + ->type('A little test description', 'description') + ->press('Save Role') + ->seeInDatabase('roles', ['display_name' => $testRoleName, 'name' => 'test-role', 'description' => $testRoleDesc]) + ->seePageIs('/settings/roles'); + // Updating + $this->asAdmin()->visit('/settings/roles') + ->see($testRoleDesc) + ->click($testRoleName) + ->type($testRoleUpdateName, '#display_name') + ->press('Save Role') + ->seeInDatabase('roles', ['display_name' => $testRoleUpdateName, 'name' => 'test-role', 'description' => $testRoleDesc]) + ->seePageIs('/settings/roles'); + // Deleting + $this->asAdmin()->visit('/settings/roles') + ->click($testRoleUpdateName) + ->click('Delete Role') + ->see($testRoleUpdateName) + ->press('Confirm') + ->seePageIs('/settings/roles') + ->dontSee($testRoleUpdateName); + } + } diff --git a/tests/TestCase.php b/tests/TestCase.php index ce4e93b12..a521fd076 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -79,8 +79,8 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase protected function getNewUser($attributes = []) { $user = factory(\BookStack\User::class)->create($attributes); - $userRepo = app('BookStack\Repos\UserRepo'); - $userRepo->attachDefaultRole($user); + $role = \BookStack\Role::getRole('editor'); + $user->attachRole($role);; return $user; } From 268db6b1d0409766014ae9f1681ec1bf5bab7552 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 5 Mar 2016 12:09:09 +0000 Subject: [PATCH 8/9] Added a whole load of permission & role tests --- app/Activity.php | 2 +- app/Http/Controllers/UserController.php | 1 + app/Repos/BookRepo.php | 2 +- app/Repos/PermissionsRepo.php | 1 + app/User.php | 7 +- .../views/partials/activity-item.blade.php | 2 +- resources/views/settings/roles/form.blade.php | 2 +- tests/RolesTest.php | 431 +++++++++++++++++- tests/TestCase.php | 11 + 9 files changed, 450 insertions(+), 9 deletions(-) diff --git a/app/Activity.php b/app/Activity.php index a1fe608f0..ac7c1d749 100644 --- a/app/Activity.php +++ b/app/Activity.php @@ -15,10 +15,10 @@ class Activity extends Model /** * Get the entity for this activity. - * @return bool */ public function entity() { + if ($this->entity_type === '') $this->entity_type = null; return $this->morphTo('entity'); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 1207c87f1..9f6a4105f 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -35,6 +35,7 @@ class UserController extends Controller */ public function index() { + $this->checkPermission('users-manage'); $users = $this->userRepo->getAllUsers(); $this->setPageTitle('Users'); return view('users/index', ['users' => $users]); diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index 572030d43..73572f25e 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -136,7 +136,7 @@ class BookRepo */ public function newFromInput($input) { - return $this->bookQuery()->fill($input); + return $this->book->newInstance($input); } /** diff --git a/app/Repos/PermissionsRepo.php b/app/Repos/PermissionsRepo.php index c35f29d10..2d497b76a 100644 --- a/app/Repos/PermissionsRepo.php +++ b/app/Repos/PermissionsRepo.php @@ -101,6 +101,7 @@ class PermissionsRepo public function assignRolePermissions(Role $role, $permissionNameArray = []) { $permissions = []; + $permissionNameArray = array_values($permissionNameArray); if ($permissionNameArray && count($permissionNameArray) > 0) { $permissions = $this->permission->whereIn('name', $permissionNameArray)->pluck('id')->toArray(); } diff --git a/app/User.php b/app/User.php index 2d14c6e6e..e1b7c143b 100644 --- a/app/User.php +++ b/app/User.php @@ -67,11 +67,12 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get all permissions belonging to a the current user. + * @param bool $cache * @return \Illuminate\Database\Eloquent\Relations\HasManyThrough */ - public function permissions() + public function permissions($cache = true) { - if(isset($this->permissions)) return $this->permissions; + if(isset($this->permissions) && $cache) return $this->permissions; $this->load('roles.permissions'); $permissions = $this->roles->map(function($role) { return $role->permissions; @@ -106,7 +107,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function attachRoleId($id) { - $this->roles()->attach([$id]); + $this->roles()->attach($id); } /** diff --git a/resources/views/partials/activity-item.blade.php b/resources/views/partials/activity-item.blade.php index 00ca574dd..ff0d74586 100644 --- a/resources/views/partials/activity-item.blade.php +++ b/resources/views/partials/activity-item.blade.php @@ -16,7 +16,7 @@ {{ $activity->getText() }} - @if($activity->entity()) + @if($activity->entity) {{ $activity->entity->name }} @endif diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 0758f317a..ed0e3dd91 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -17,7 +17,7 @@
    - +
    diff --git a/tests/RolesTest.php b/tests/RolesTest.php index 7349c2968..baba208f1 100644 --- a/tests/RolesTest.php +++ b/tests/RolesTest.php @@ -7,16 +7,33 @@ class RolesTest extends TestCase public function setUp() { parent::setUp(); + $this->user = $this->getNewBlankUser(); + } + + /** + * Give the given user some permissions. + * @param \BookStack\User $user + * @param array $permissions + */ + protected function giveUserPermissions(\BookStack\User $user, $permissions = []) + { + $newRole = $this->createNewRole($permissions); + $user->attachRole($newRole); + $user->load('roles'); + $user->permissions(false); } /** * Create a new basic role for testing purposes. + * @param array $permissions * @return static */ - protected function createNewRole() + protected function createNewRole($permissions = []) { $permissionRepo = app('BookStack\Repos\PermissionsRepo'); - return $permissionRepo->saveNewRole(factory(\BookStack\Role::class)->make()->toArray()); + $roleData = factory(\BookStack\Role::class)->make()->toArray(); + $roleData['permissions'] = array_flip($permissions); + return $permissionRepo->saveNewRole($roleData); } public function test_admin_can_see_settings() @@ -80,4 +97,414 @@ class RolesTest extends TestCase ->dontSee($testRoleUpdateName); } + public function test_manage_user_permission() + { + $this->actingAs($this->user)->visit('/')->visit('/settings/users') + ->seePageIs('/'); + $this->giveUserPermissions($this->user, ['users-manage']); + $this->actingAs($this->user)->visit('/')->visit('/settings/users') + ->seePageIs('/settings/users'); + } + + public function test_user_roles_manage_permission() + { + $this->actingAs($this->user)->visit('/')->visit('/settings/roles') + ->seePageIs('/')->visit('/settings/roles/1')->seePageIs('/'); + $this->giveUserPermissions($this->user, ['user-roles-manage']); + $this->actingAs($this->user)->visit('/settings/roles') + ->seePageIs('/settings/roles')->click('Admin') + ->see('Edit Role'); + } + + public function test_settings_manage_permission() + { + $this->actingAs($this->user)->visit('/')->visit('/settings') + ->seePageIs('/'); + $this->giveUserPermissions($this->user, ['settings-manage']); + $this->actingAs($this->user)->visit('/')->visit('/settings') + ->seePageIs('/settings')->press('Save Settings')->see('Settings Saved'); + } + + public function test_restrictions_manage_all_permission() + { + $page = \BookStack\Page::take(1)->get()->first(); + $this->actingAs($this->user)->visit($page->getUrl()) + ->dontSee('Restrict') + ->visit($page->getUrl() . '/restrict') + ->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'); + } + + public function test_restrictions_manage_own_permission() + { + $otherUsersPage = \BookStack\Page::take(1)->get()->first(); + $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') + ->seePageIs('/'); + // Check can't restrict own content + $this->actingAs($this->user)->visit($content['page']->getUrl()) + ->dontSee('Restrict') + ->visit($content['page']->getUrl() . '/restrict') + ->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') + ->seePageIs('/'); + // Check can restrict own content + $this->actingAs($this->user)->visit($content['page']->getUrl()) + ->see('Restrict') + ->click('Restrict') + ->seePageIs($content['page']->getUrl() . '/restrict'); + } + + /** + * Check a standard entity access permission + * @param string $permission + * @param array $accessUrls Urls that are only accessible after having the permission + * @param array $visibles Check this text, In the buttons toolbar, is only visible with the permission + * @param null $callback + */ + private function checkAccessPermission($permission, $accessUrls = [], $visibles = []) + { + foreach ($accessUrls as $url) { + $this->actingAs($this->user)->visit('/')->visit($url) + ->seePageIs('/'); + } + foreach ($visibles as $url => $text) { + $this->actingAs($this->user)->visit('/')->visit($url) + ->dontSeeInElement('.action-buttons',$text); + } + + $this->giveUserPermissions($this->user, [$permission]); + + foreach ($accessUrls as $url) { + $this->actingAs($this->user)->visit('/')->visit($url) + ->seePageIs($url); + } + foreach ($visibles as $url => $text) { + $this->actingAs($this->user)->visit('/')->visit($url) + ->see($text); + } + } + + public function test_books_create_all_permissions() + { + $this->checkAccessPermission('book-create-all', [ + '/books/create' + ], [ + '/books' => 'Add new book' + ]); + + $this->visit('/books/create') + ->type('test book', 'name') + ->type('book desc', 'description') + ->press('Save Book') + ->seePageIs('/books/test-book'); + } + + public function test_books_edit_own_permission() + { + $otherBook = \BookStack\Book::take(1)->get()->first(); + $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; + $this->checkAccessPermission('book-update-own', [ + $ownBook->getUrl() . '/edit' + ], [ + $ownBook->getUrl() => 'Edit' + ]); + + $this->visit($otherBook->getUrl()) + ->dontSeeInElement('.action-buttons', 'Edit') + ->visit($otherBook->getUrl() . '/edit') + ->seePageIs('/'); + } + + public function test_books_edit_all_permission() + { + $otherBook = \BookStack\Book::take(1)->get()->first(); + $this->checkAccessPermission('book-update-all', [ + $otherBook->getUrl() . '/edit' + ], [ + $otherBook->getUrl() => 'Edit' + ]); + } + + public function test_books_delete_own_permission() + { + $this->giveUserPermissions($this->user, ['book-update-all']); + $otherBook = \BookStack\Book::take(1)->get()->first(); + $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; + $this->checkAccessPermission('book-delete-own', [ + $ownBook->getUrl() . '/delete' + ], [ + $ownBook->getUrl() => 'Delete' + ]); + + $this->visit($otherBook->getUrl()) + ->dontSeeInElement('.action-buttons', 'Delete') + ->visit($otherBook->getUrl() . '/delete') + ->seePageIs('/'); + $this->visit($ownBook->getUrl())->visit($ownBook->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs('/books') + ->dontSee($ownBook->name); + } + + public function test_books_delete_all_permission() + { + $this->giveUserPermissions($this->user, ['book-update-all']); + $otherBook = \BookStack\Book::take(1)->get()->first(); + $this->checkAccessPermission('book-delete-all', [ + $otherBook->getUrl() . '/delete' + ], [ + $otherBook->getUrl() => 'Delete' + ]); + + $this->visit($otherBook->getUrl())->visit($otherBook->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs('/books') + ->dontSee($otherBook->name); + } + + public function test_chapter_create_own_permissions() + { + $book = \BookStack\Book::take(1)->get()->first(); + $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; + $baseUrl = $ownBook->getUrl() . '/chapter'; + $this->checkAccessPermission('chapter-create-own', [ + $baseUrl . '/create' + ], [ + $ownBook->getUrl() => 'New Chapter' + ]); + + $this->visit($baseUrl . '/create') + ->type('test chapter', 'name') + ->type('chapter desc', 'description') + ->press('Save Chapter') + ->seePageIs($baseUrl . '/test-chapter'); + + $this->visit($book->getUrl()) + ->dontSeeInElement('.action-buttons', 'New Chapter') + ->visit($book->getUrl() . '/chapter/create') + ->seePageIs('/'); + } + + public function test_chapter_create_all_permissions() + { + $book = \BookStack\Book::take(1)->get()->first(); + $baseUrl = $book->getUrl() . '/chapter'; + $this->checkAccessPermission('chapter-create-all', [ + $baseUrl . '/create' + ], [ + $book->getUrl() => 'New Chapter' + ]); + + $this->visit($baseUrl . '/create') + ->type('test chapter', 'name') + ->type('chapter desc', 'description') + ->press('Save Chapter') + ->seePageIs($baseUrl . '/test-chapter'); + } + + public function test_chapter_edit_own_permission() + { + $otherChapter = \BookStack\Chapter::take(1)->get()->first(); + $ownChapter = $this->createEntityChainBelongingToUser($this->user)['chapter']; + $this->checkAccessPermission('chapter-update-own', [ + $ownChapter->getUrl() . '/edit' + ], [ + $ownChapter->getUrl() => 'Edit' + ]); + + $this->visit($otherChapter->getUrl()) + ->dontSeeInElement('.action-buttons', 'Edit') + ->visit($otherChapter->getUrl() . '/edit') + ->seePageIs('/'); + } + + public function test_chapter_edit_all_permission() + { + $otherChapter = \BookStack\Chapter::take(1)->get()->first(); + $this->checkAccessPermission('chapter-update-all', [ + $otherChapter->getUrl() . '/edit' + ], [ + $otherChapter->getUrl() => 'Edit' + ]); + } + + public function test_chapter_delete_own_permission() + { + $this->giveUserPermissions($this->user, ['chapter-update-all']); + $otherChapter = \BookStack\Chapter::take(1)->get()->first(); + $ownChapter = $this->createEntityChainBelongingToUser($this->user)['chapter']; + $this->checkAccessPermission('chapter-delete-own', [ + $ownChapter->getUrl() . '/delete' + ], [ + $ownChapter->getUrl() => 'Delete' + ]); + + $bookUrl = $ownChapter->book->getUrl(); + $this->visit($otherChapter->getUrl()) + ->dontSeeInElement('.action-buttons', 'Delete') + ->visit($otherChapter->getUrl() . '/delete') + ->seePageIs('/'); + $this->visit($ownChapter->getUrl())->visit($ownChapter->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs($bookUrl) + ->dontSeeInElement('.book-content', $ownChapter->name); + } + + public function test_chapter_delete_all_permission() + { + $this->giveUserPermissions($this->user, ['chapter-update-all']); + $otherChapter = \BookStack\Chapter::take(1)->get()->first(); + $this->checkAccessPermission('chapter-delete-all', [ + $otherChapter->getUrl() . '/delete' + ], [ + $otherChapter->getUrl() => 'Delete' + ]); + + $bookUrl = $otherChapter->book->getUrl(); + $this->visit($otherChapter->getUrl())->visit($otherChapter->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs($bookUrl) + ->dontSeeInElement('.book-content', $otherChapter->name); + } + + public function test_page_create_own_permissions() + { + $book = \BookStack\Book::take(1)->get()->first(); + $chapter = \BookStack\Chapter::take(1)->get()->first(); + + $entities = $this->createEntityChainBelongingToUser($this->user); + $ownBook = $entities['book']; + $ownChapter = $entities['chapter']; + + $baseUrl = $ownBook->getUrl() . '/page'; + + $this->checkAccessPermission('page-create-own', [ + $baseUrl . '/create', + $ownChapter->getUrl() . '/create-page' + ], [ + $ownBook->getUrl() => 'New Page', + $ownChapter->getUrl() => 'New Page' + ]); + + $this->visit($baseUrl . '/create') + ->type('test page', 'name') + ->type('page desc', 'html') + ->press('Save Page') + ->seePageIs($baseUrl . '/test-page'); + + $this->visit($book->getUrl()) + ->dontSeeInElement('.action-buttons', 'New Page') + ->visit($book->getUrl() . '/page/create') + ->seePageIs('/'); + $this->visit($chapter->getUrl()) + ->dontSeeInElement('.action-buttons', 'New Page') + ->visit($chapter->getUrl() . '/create-page') + ->seePageIs('/'); + } + + public function test_page_create_all_permissions() + { + $book = \BookStack\Book::take(1)->get()->first(); + $chapter = \BookStack\Chapter::take(1)->get()->first(); + $baseUrl = $book->getUrl() . '/page'; + $this->checkAccessPermission('page-create-all', [ + $baseUrl . '/create', + $chapter->getUrl() . '/create-page' + ], [ + $book->getUrl() => 'New Page', + $chapter->getUrl() => 'New Page' + ]); + + $this->visit($baseUrl . '/create') + ->type('test page', 'name') + ->type('page desc', 'html') + ->press('Save Page') + ->seePageIs($baseUrl . '/test-page'); + + $this->visit($chapter->getUrl() . '/create-page') + ->type('new test page', 'name') + ->type('page desc', 'html') + ->press('Save Page') + ->seePageIs($baseUrl . '/new-test-page'); + } + + public function test_page_edit_own_permission() + { + $otherPage = \BookStack\Page::take(1)->get()->first(); + $ownPage = $this->createEntityChainBelongingToUser($this->user)['page']; + $this->checkAccessPermission('page-update-own', [ + $ownPage->getUrl() . '/edit' + ], [ + $ownPage->getUrl() => 'Edit' + ]); + + $this->visit($otherPage->getUrl()) + ->dontSeeInElement('.action-buttons', 'Edit') + ->visit($otherPage->getUrl() . '/edit') + ->seePageIs('/'); + } + + public function test_page_edit_all_permission() + { + $otherPage = \BookStack\Page::take(1)->get()->first(); + $this->checkAccessPermission('page-update-all', [ + $otherPage->getUrl() . '/edit' + ], [ + $otherPage->getUrl() => 'Edit' + ]); + } + + public function test_page_delete_own_permission() + { + $this->giveUserPermissions($this->user, ['page-update-all']); + $otherPage = \BookStack\Page::take(1)->get()->first(); + $ownPage = $this->createEntityChainBelongingToUser($this->user)['page']; + $this->checkAccessPermission('page-delete-own', [ + $ownPage->getUrl() . '/delete' + ], [ + $ownPage->getUrl() => 'Delete' + ]); + + $bookUrl = $ownPage->book->getUrl(); + $this->visit($otherPage->getUrl()) + ->dontSeeInElement('.action-buttons', 'Delete') + ->visit($otherPage->getUrl() . '/delete') + ->seePageIs('/'); + $this->visit($ownPage->getUrl())->visit($ownPage->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs($bookUrl) + ->dontSeeInElement('.book-content', $ownPage->name); + } + + public function test_page_delete_all_permission() + { + $this->giveUserPermissions($this->user, ['page-update-all']); + $otherPage = \BookStack\Page::take(1)->get()->first(); + $this->checkAccessPermission('page-delete-all', [ + $otherPage->getUrl() . '/delete' + ], [ + $otherPage->getUrl() => 'Delete' + ]); + + $bookUrl = $otherPage->book->getUrl(); + $this->visit($otherPage->getUrl())->visit($otherPage->getUrl() . '/delete') + ->press('Confirm') + ->seePageIs($bookUrl) + ->dontSeeInElement('.book-content', $otherPage->name); + } + } diff --git a/tests/TestCase.php b/tests/TestCase.php index a521fd076..840fe0d08 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -84,6 +84,17 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase return $user; } + /** + * Quick way to create a new user without any permissions + * @param array $attributes + * @return mixed + */ + protected function getNewBlankUser($attributes = []) + { + $user = factory(\BookStack\User::class)->create($attributes); + return $user; + } + /** * Assert that a given string is seen inside an element. * From 8e6248f57f92d943a011c3219120d60ee4f2f00b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 5 Mar 2016 18:09:21 +0000 Subject: [PATCH 9/9] Added restriction tests and fixed any bugs in the process Also updated many styles within areas affected by the new permission and roles system. --- app/Exceptions/Handler.php | 3 +- app/Exceptions/NotFoundException.php | 14 + app/Http/Controllers/ChapterController.php | 9 +- app/Http/Controllers/PageController.php | 3 +- app/Repos/BookRepo.php | 13 +- app/Repos/ChapterRepo.php | 14 +- app/Repos/PageRepo.php | 5 +- app/Services/RestrictionService.php | 30 +- app/helpers.php | 9 +- database/seeds/DummyContentSeeder.php | 2 +- phpunit.xml | 1 + resources/assets/sass/_header.scss | 3 + resources/assets/sass/_lists.scss | 3 +- resources/views/books/index.blade.php | 4 +- resources/views/books/restrictions.blade.php | 13 + resources/views/books/show.blade.php | 32 +- .../views/chapters/restrictions.blade.php | 14 + resources/views/chapters/show.blade.php | 27 +- resources/views/errors/404.blade.php | 2 +- .../views/form/restriction-form.blade.php | 3 +- resources/views/pages/restrictions.blade.php | 21 + resources/views/pages/show.blade.php | 31 ++ resources/views/settings/roles/form.blade.php | 12 +- .../views/settings/roles/index.blade.php | 2 +- tests/RestrictionsTest.php | 407 ++++++++++++++++++ tests/TestCase.php | 35 ++ 26 files changed, 680 insertions(+), 32 deletions(-) create mode 100644 app/Exceptions/NotFoundException.php create mode 100644 tests/RestrictionsTest.php diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 73a316953..14d553ed0 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -56,7 +56,8 @@ class Handler extends ExceptionHandler // Which will include the basic message to point the user roughly to the cause. if (($e instanceof PrettyException || $e->getPrevious() instanceof PrettyException) && !config('app.debug')) { $message = ($e instanceof PrettyException) ? $e->getMessage() : $e->getPrevious()->getMessage(); - return response()->view('errors/500', ['message' => $message], 500); + $code = ($e->getCode() === 0) ? 500 : $e->getCode(); + return response()->view('errors/' . $code, ['message' => $message], $code); } return parent::render($request, $e); diff --git a/app/Exceptions/NotFoundException.php b/app/Exceptions/NotFoundException.php new file mode 100644 index 000000000..3c027dc44 --- /dev/null +++ b/app/Exceptions/NotFoundException.php @@ -0,0 +1,14 @@ +bookRepo->getChildren($book); Views::add($chapter); $this->setPageTitle($chapter->getShortName()); - return view('chapters/show', ['book' => $book, 'chapter' => $chapter, 'current' => $chapter, 'sidebarTree' => $sidebarTree]); + $pages = $this->chapterRepo->getChildren($chapter); + return view('chapters/show', [ + 'book' => $book, + 'chapter' => $chapter, + 'current' => $chapter, + 'sidebarTree' => $sidebarTree, + 'pages' => $pages + ]); } /** diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index b469f51dd..19e4744ea 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -1,6 +1,7 @@ pageRepo->getBySlug($pageSlug, $book->id); - } catch (NotFoundHttpException $e) { + } catch (NotFoundException $e) { $page = $this->pageRepo->findPageUsingOldSlug($pageSlug, $bookSlug); if ($page === null) abort(404); return redirect($page->getUrl()); diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index 73572f25e..4ae7cc062 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -1,6 +1,7 @@ bookQuery()->where('slug', '=', $slug)->first(); - if ($book === null) abort(404); + if ($book === null) throw new NotFoundException('Book not found'); return $book; } @@ -153,6 +155,7 @@ class BookRepo $this->chapterRepo->destroy($chapter); } $book->views()->delete(); + $book->restrictions()->delete(); $book->delete(); } @@ -210,11 +213,13 @@ class BookRepo public function getChildren(Book $book) { $pageQuery = $book->pages()->where('chapter_id', '=', 0); - $this->restrictionService->enforcePageRestrictions($pageQuery, 'view'); + $pageQuery = $this->restrictionService->enforcePageRestrictions($pageQuery, 'view'); $pages = $pageQuery->get(); - $chapterQuery = $book->chapters()->with('pages'); - $this->restrictionService->enforceChapterRestrictions($chapterQuery, 'view'); + $chapterQuery = $book->chapters()->with(['pages' => function($query) { + $this->restrictionService->enforcePageRestrictions($query, 'view'); + }]); + $chapterQuery = $this->restrictionService->enforceChapterRestrictions($chapterQuery, 'view'); $chapters = $chapterQuery->get(); $children = $pages->merge($chapters); $bookSlug = $book->slug; diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index 90f2f8c54..095596a60 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -2,6 +2,7 @@ use Activity; +use BookStack\Exceptions\NotFoundException; use BookStack\Services\RestrictionService; use Illuminate\Support\Str; use BookStack\Chapter; @@ -66,14 +67,24 @@ class ChapterRepo * @param $slug * @param $bookId * @return mixed + * @throws NotFoundException */ public function getBySlug($slug, $bookId) { $chapter = $this->chapterQuery()->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); - if ($chapter === null) abort(404); + if ($chapter === null) throw new NotFoundException('Chapter not found'); return $chapter; } + /** + * Get the child items for a chapter + * @param Chapter $chapter + */ + public function getChildren(Chapter $chapter) + { + return $this->restrictionService->enforcePageRestrictions($chapter->pages())->get(); + } + /** * Create a new chapter from request input. * @param $input @@ -98,6 +109,7 @@ class ChapterRepo } Activity::removeEntity($chapter); $chapter->views()->delete(); + $chapter->restrictions()->delete(); $chapter->delete(); } diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index c4cf00e7c..f3933af69 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -4,6 +4,7 @@ use Activity; use BookStack\Book; use BookStack\Chapter; +use BookStack\Exceptions\NotFoundException; use BookStack\Services\RestrictionService; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; @@ -56,11 +57,12 @@ class PageRepo * @param $slug * @param $bookId * @return mixed + * @throws NotFoundException */ public function getBySlug($slug, $bookId) { $page = $this->pageQuery()->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first(); - if ($page === null) throw new NotFoundHttpException('Page not found'); + if ($page === null) throw new NotFoundException('Page not found'); return $page; } @@ -373,6 +375,7 @@ class PageRepo Activity::removeEntity($page); $page->views()->delete(); $page->revisions()->delete(); + $page->restrictions()->delete(); $page->delete(); } diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 0ef80b229..f7838bf88 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -15,10 +15,16 @@ class RestrictionService public function __construct() { $user = auth()->user(); - $this->userRoles = $user ? auth()->user()->roles->pluck('id') : false; + $this->userRoles = $user ? auth()->user()->roles->pluck('id') : []; $this->isAdmin = $user ? auth()->user()->hasRole('admin') : false; } + /** + * Checks if an entity has a restriction set upon it. + * @param Entity $entity + * @param $action + * @return bool + */ public function checkIfEntityRestricted(Entity $entity, $action) { if ($this->isAdmin) return true; @@ -93,12 +99,28 @@ class RestrictionService }); }); }) + // Page unrestricted, Has an unrestricted chapter & book has accepted restrictions + ->orWhere(function ($query) { + $query->where('restricted', '=', false) + ->whereExists(function ($query) { + $query->select('*')->from('chapters') + ->whereRaw('chapters.id=pages.chapter_id')->where('restricted', '=', false); + }) + ->whereExists(function ($query) { + $query->select('*')->from('books') + ->whereRaw('books.id=pages.book_id') + ->whereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'books', 'Book'); + }); + }); + }) // Page unrestricted, Has a chapter with accepted permissions ->orWhere(function ($query) { $query->where('restricted', '=', false) ->whereExists(function ($query) { $query->select('*')->from('chapters') ->whereRaw('chapters.id=pages.chapter_id') + ->where('restricted', '=', true) ->whereExists(function ($query) { $this->checkRestrictionsQuery($query, 'chapters', 'Chapter'); }); @@ -183,8 +205,10 @@ class RestrictionService return $query->where(function ($parentWhereQuery) { $parentWhereQuery ->where('restricted', '=', false) - ->orWhereExists(function ($query) { - $this->checkRestrictionsQuery($query, 'books', 'Book'); + ->orWhere(function ($query) { + $query->where('restricted', '=', true)->whereExists(function ($query) { + $this->checkRestrictionsQuery($query, 'books', 'Book'); + }); }); }); } diff --git a/app/helpers.php b/app/helpers.php index 8f080c5e1..ead6b3008 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -1,10 +1,10 @@ check()) return false; if ($ownable === null) { return auth()->user() && auth()->user()->can($permission); } @@ -47,9 +48,9 @@ function userCan($permission, \BookStack\Ownable $ownable = null) $permissionBaseName = strtolower($permission) . '-'; $hasPermission = false; if (auth()->user()->can($permissionBaseName . 'all')) $hasPermission = true; - if (auth()->user()->can($permissionBaseName . 'own') && $ownable->createdBy->id === auth()->user()->id) $hasPermission = true; + if (auth()->user()->can($permissionBaseName . 'own') && $ownable->createdBy && $ownable->createdBy->id === auth()->user()->id) $hasPermission = true; - if(!$ownable instanceof \BookStack\Entity) return $hasPermission; + if (!$ownable instanceof \BookStack\Entity) return $hasPermission; // Check restrictions on the entitiy $restrictionService = app('BookStack\Services\RestrictionService'); diff --git a/database/seeds/DummyContentSeeder.php b/database/seeds/DummyContentSeeder.php index aa70eaa0a..328971f26 100644 --- a/database/seeds/DummyContentSeeder.php +++ b/database/seeds/DummyContentSeeder.php @@ -12,7 +12,7 @@ class DummyContentSeeder extends Seeder public function run() { $user = factory(BookStack\User::class, 1)->create(); - $role = \BookStack\Role::getDefault(); + $role = \BookStack\Role::getRole('editor'); $user->attachRole($role); diff --git a/phpunit.xml b/phpunit.xml index 762fc2da7..66196e8cf 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -21,6 +21,7 @@ + diff --git a/resources/assets/sass/_header.scss b/resources/assets/sass/_header.scss index 1edfc0037..87aa20046 100644 --- a/resources/assets/sass/_header.scss +++ b/resources/assets/sass/_header.scss @@ -87,6 +87,9 @@ header { padding-top: $-s; } } + .dropdown-container { + font-size: 0.9em; + } } form.search-box { diff --git a/resources/assets/sass/_lists.scss b/resources/assets/sass/_lists.scss index f0bd3b1ea..09707ebc4 100644 --- a/resources/assets/sass/_lists.scss +++ b/resources/assets/sass/_lists.scss @@ -95,13 +95,14 @@ // Sidebar list .book-tree { - padding: $-xl 0 0 0; + padding: $-l 0 0 0; position: relative; right: 0; top: 0; transition: ease-in-out 240ms; transition-property: right, border; border-left: 0px solid #FFF; + background-color: #FFF; &.fixed { position: fixed; top: 0; diff --git a/resources/views/books/index.blade.php b/resources/views/books/index.blade.php index d5d7cb139..7b5c92b5a 100644 --- a/resources/views/books/index.blade.php +++ b/resources/views/books/index.blade.php @@ -30,7 +30,9 @@ {!! $books->render() !!} @else

    No books have been created.

    - Create one now + @if(userCan('books-create-all')) + Create one now + @endif @endif
    diff --git a/resources/views/books/restrictions.blade.php b/resources/views/books/restrictions.blade.php index 826f218ce..60b126a7b 100644 --- a/resources/views/books/restrictions.blade.php +++ b/resources/views/books/restrictions.blade.php @@ -2,6 +2,19 @@ @section('content') +
    +
    +
    +
    +
    + {{ $book->getShortName() }} +
    +
    +
    +
    +
    + +

    Book Restrictions

    @include('form/restriction-form', ['model' => $book]) diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index f8a22ada8..cd32a406b 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -2,7 +2,7 @@ @section('content') -
    +
    @@ -15,13 +15,22 @@ @endif @if(userCan('book-update', $book)) Edit - Sort @endif - @if(userCan('restrictions-manage', $book)) - Restrict - @endif - @if(userCan('book-delete', $book)) - Delete + @if(userCan('book-update', $book) || userCan('restrictions-manage', $book) || userCan('book-delete', $book)) +
    + +
      + @if(userCan('book-update', $book)) +
    • Sort
      • + @endif + @if(userCan('restrictions-manage', $book)) +
    • Restrict
      • + @endif + @if(userCan('book-delete', $book)) +
    • Delete
      • + @endif +
    +
    @endif
    @@ -78,6 +87,15 @@
    + @if($book->restricted) +

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

    + @endif
    diff --git a/resources/views/chapters/restrictions.blade.php b/resources/views/chapters/restrictions.blade.php index 3b19b55c8..1f2f9c8fa 100644 --- a/resources/views/chapters/restrictions.blade.php +++ b/resources/views/chapters/restrictions.blade.php @@ -2,6 +2,20 @@ @section('content') +
    +
    +
    +
    +
    + {{ $chapter->book->getShortName() }} + » + {{$chapter->getShortName()}} +
    +
    +
    +
    +
    +

    Chapter Restrictions

    @include('form/restriction-form', ['model' => $chapter]) diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index ac36a6b3e..f053edc1c 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -37,10 +37,10 @@

    {{ $chapter->name }}

    {{ $chapter->description }}

    - @if(count($chapter->pages) > 0) + @if(count($pages) > 0)
    - @foreach($chapter->pages as $page) + @foreach($pages as $page) @include('pages/list-item', ['page' => $page])
    @endforeach @@ -63,6 +63,29 @@

    +
    + @if($book->restricted || $chapter->restricted) +
    + + @if($book->restricted) + @if(userCan('restrictions-manage', $book)) + Book Restricted + @else + Book Restricted + @endif +
    + @endif + + @if($chapter->restricted) + @if(userCan('restrictions-manage', $chapter)) + Chapter Restricted + @else + Chapter Restricted + @endif + @endif +
    + @endif + @include('pages/sidebar-tree-list', ['book' => $book, 'sidebarTree' => $sidebarTree])
    diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php index 191cf758f..a9294c7cf 100644 --- a/resources/views/errors/404.blade.php +++ b/resources/views/errors/404.blade.php @@ -4,7 +4,7 @@
    -

    Page Not Found

    +

    {{ $message or 'Page Not Found' }}

    Sorry, The page you were looking for could not be found.

    Return To Home
    diff --git a/resources/views/form/restriction-form.blade.php b/resources/views/form/restriction-form.blade.php index cb42497b4..d2fa23982 100644 --- a/resources/views/form/restriction-form.blade.php +++ b/resources/views/form/restriction-form.blade.php @@ -3,7 +3,7 @@
    - @include('form/checkbox', ['name' => 'restricted', 'label' => 'Restrict this page?']) + @include('form/checkbox', ['name' => 'restricted', 'label' => 'Restrict this ' . $model->getClassName()])
    @@ -24,5 +24,6 @@ @endforeach
    + Cancel
    \ No newline at end of file diff --git a/resources/views/pages/restrictions.blade.php b/resources/views/pages/restrictions.blade.php index 63ad1fade..d094abc71 100644 --- a/resources/views/pages/restrictions.blade.php +++ b/resources/views/pages/restrictions.blade.php @@ -2,6 +2,27 @@ @section('content') +
    +
    +
    +
    +
    + {{ $page->book->getShortName() }} + @if($page->hasChapter()) + » + + + {{$page->chapter->getShortName()}} + + @endif + » + {{ $page->getShortName() }} +
    +
    +
    +
    +
    +

    Page Restrictions

    @include('form/restriction-form', ['model' => $page]) diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index f84d1963d..286d44387 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -70,7 +70,38 @@
    +
    + @if($book->restricted || ($page->chapter && $page->chapter->restricted) || $page->restricted) +
    + @if($book->restricted) + @if(userCan('restrictions-manage', $book)) + Book restricted + @else + Book restricted + @endif +
    + @endif + + @if($page->chapter && $page->chapter->restricted) + @if(userCan('restrictions-manage', $page->chapter)) + Chapter restricted + @else + Chapter restricted + @endif +
    + @endif + + @if($page->restricted) + @if(userCan('restrictions-manage', $page)) + Page restricted + @else + Page restricted + @endif +
    + @endif +
    + @endif @include('pages/sidebar-tree-list', ['book' => $book, 'sidebarTree' => $sidebarTree])
    diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index ed0e3dd91..fafb9bed2 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -3,6 +3,7 @@
    +

    Role Details

    @include('form/text', ['name' => 'display_name']) @@ -11,7 +12,7 @@ @include('form/text', ['name' => 'description'])
    -
    +

    System Permissions

    @@ -33,10 +34,17 @@
    +
    + +

    Asset Permissions

    +

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

    @@ -104,4 +112,6 @@ + +Cancel \ No newline at end of file diff --git a/resources/views/settings/roles/index.blade.php b/resources/views/settings/roles/index.blade.php index 601c6533e..8f92a5eba 100644 --- a/resources/views/settings/roles/index.blade.php +++ b/resources/views/settings/roles/index.blade.php @@ -4,7 +4,7 @@ @include('settings/navbar', ['selected' => 'roles']) -
    +

    User Roles

    diff --git a/tests/RestrictionsTest.php b/tests/RestrictionsTest.php new file mode 100644 index 000000000..40b5a7647 --- /dev/null +++ b/tests/RestrictionsTest.php @@ -0,0 +1,407 @@ +user = $this->getNewUser(); + } + + /** + * Manually set some restrictions on an entity. + * @param \BookStack\Entity $entity + * @param $actions + */ + protected function setEntityRestrictions(\BookStack\Entity $entity, $actions) + { + $entity->restricted = true; + $entity->restrictions()->delete(); + $role = $this->user->roles->first(); + foreach ($actions as $action) { + $entity->restrictions()->create([ + 'role_id' => $role->id, + 'action' => strtolower($action) + ]); + } + $entity->save(); + $entity->load('restrictions'); + } + + public function test_book_view_restriction() + { + $book = \BookStack\Book::first(); + $bookPage = $book->pages->first(); + $bookChapter = $book->chapters->first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->user) + ->visit($bookUrl) + ->seePageIs($bookUrl); + + $this->setEntityRestrictions($book, []); + + $this->forceVisit($bookUrl) + ->see('Book not found'); + $this->forceVisit($bookPage->getUrl()) + ->see('Book not found'); + $this->forceVisit($bookChapter->getUrl()) + ->see('Book not found'); + + $this->setEntityRestrictions($book, ['view']); + + $this->visit($bookUrl) + ->see($book->name); + $this->visit($bookPage->getUrl()) + ->see($bookPage->name); + $this->visit($bookChapter->getUrl()) + ->see($bookChapter->name); + } + + public function test_book_create_restriction() + { + $book = \BookStack\Book::first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->user) + ->visit($bookUrl) + ->seeInElement('.action-buttons', 'New Page') + ->seeInElement('.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() + { + $book = \BookStack\Book::first(); + $bookPage = $book->pages->first(); + $bookChapter = $book->chapters->first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->user) + ->visit($bookUrl . '/edit') + ->see('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() + { + $book = \BookStack\Book::first(); + $bookPage = $book->pages->first(); + $bookChapter = $book->chapters->first(); + + $bookUrl = $book->getUrl(); + $this->actingAs($this->user) + ->visit($bookUrl . '/delete') + ->see('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'); + } + + public function test_chapter_view_restriction() + { + $chapter = \BookStack\Chapter::first(); + $chapterPage = $chapter->pages->first(); + + $chapterUrl = $chapter->getUrl(); + $this->actingAs($this->user) + ->visit($chapterUrl) + ->seePageIs($chapterUrl); + + $this->setEntityRestrictions($chapter, []); + + $this->forceVisit($chapterUrl) + ->see('Chapter not found'); + $this->forceVisit($chapterPage->getUrl()) + ->see('Page not found'); + + $this->setEntityRestrictions($chapter, ['view']); + + $this->visit($chapterUrl) + ->see($chapter->name); + $this->visit($chapterPage->getUrl()) + ->see($chapterPage->name); + } + + public function test_chapter_create_restriction() + { + $chapter = \BookStack\Chapter::first(); + + $chapterUrl = $chapter->getUrl(); + $this->actingAs($this->user) + ->visit($chapterUrl) + ->seeInElement('.action-buttons', 'New Page'); + + $this->setEntityRestrictions($chapter, ['view', 'delete', 'update']); + + $this->forceVisit($chapterUrl . '/create-page') + ->see('You do not have permission')->seePageIs('/'); + $this->visit($chapterUrl)->dontSeeInElement('.action-buttons', 'New Page'); + + $this->setEntityRestrictions($chapter, ['view', 'create']); + + + $this->visit($chapterUrl . '/create-page') + ->type('test page', 'name') + ->type('test content', 'html') + ->press('Save Page') + ->seePageIs($chapter->book->getUrl() . '/page/test-page'); + $this->visit($chapterUrl)->seeInElement('.action-buttons', 'New Page'); + } + + public function test_chapter_update_restriction() + { + $chapter = \BookStack\Chapter::first(); + $chapterPage = $chapter->pages->first(); + + $chapterUrl = $chapter->getUrl(); + $this->actingAs($this->user) + ->visit($chapterUrl . '/edit') + ->see('Edit Chapter'); + + $this->setEntityRestrictions($chapter, ['view', 'delete']); + + $this->forceVisit($chapterUrl . '/edit') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($chapterPage->getUrl() . '/edit') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($chapter, ['view', 'update']); + + $this->visit($chapterUrl . '/edit') + ->seePageIs($chapterUrl . '/edit')->see('Edit Chapter'); + $this->visit($chapterPage->getUrl() . '/edit') + ->seePageIs($chapterPage->getUrl() . '/edit'); + } + + public function test_chapter_delete_restriction() + { + $chapter = \BookStack\Chapter::first(); + $chapterPage = $chapter->pages->first(); + + $chapterUrl = $chapter->getUrl(); + $this->actingAs($this->user) + ->visit($chapterUrl . '/delete') + ->see('Delete Chapter'); + + $this->setEntityRestrictions($chapter, ['view', 'update']); + + $this->forceVisit($chapterUrl . '/delete') + ->see('You do not have permission')->seePageIs('/'); + $this->forceVisit($chapterPage->getUrl() . '/delete') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($chapter, ['view', 'delete']); + + $this->visit($chapterUrl . '/delete') + ->seePageIs($chapterUrl . '/delete')->see('Delete Chapter'); + $this->visit($chapterPage->getUrl() . '/delete') + ->seePageIs($chapterPage->getUrl() . '/delete')->see('Delete Page'); + } + + public function test_page_view_restriction() + { + $page = \BookStack\Page::first(); + + $pageUrl = $page->getUrl(); + $this->actingAs($this->user) + ->visit($pageUrl) + ->seePageIs($pageUrl); + + $this->setEntityRestrictions($page, ['update', 'delete']); + + $this->forceVisit($pageUrl) + ->see('Page not found'); + + $this->setEntityRestrictions($page, ['view']); + + $this->visit($pageUrl) + ->see($page->name); + } + + public function test_page_update_restriction() + { + $page = \BookStack\Chapter::first(); + + $pageUrl = $page->getUrl(); + $this->actingAs($this->user) + ->visit($pageUrl . '/edit') + ->seeInField('name', $page->name); + + $this->setEntityRestrictions($page, ['view', 'delete']); + + $this->forceVisit($pageUrl . '/edit') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($page, ['view', 'update']); + + $this->visit($pageUrl . '/edit') + ->seePageIs($pageUrl . '/edit')->seeInField('name', $page->name); + } + + public function test_page_delete_restriction() + { + $page = \BookStack\Page::first(); + + $pageUrl = $page->getUrl(); + $this->actingAs($this->user) + ->visit($pageUrl . '/delete') + ->see('Delete Page'); + + $this->setEntityRestrictions($page, ['view', 'update']); + + $this->forceVisit($pageUrl . '/delete') + ->see('You do not have permission')->seePageIs('/'); + + $this->setEntityRestrictions($page, ['view', 'delete']); + + $this->visit($pageUrl . '/delete') + ->seePageIs($pageUrl . '/delete')->see('Delete Page'); + } + + public function test_book_restriction_form() + { + $book = \BookStack\Book::first(); + $this->asAdmin()->visit($book->getUrl() . '/restrict') + ->see('Book Restrictions') + ->check('restricted') + ->check('restrictions[2][view]') + ->press('Save Restrictions') + ->seeInDatabase('books', ['id' => $book->id, 'restricted' => true]) + ->seeInDatabase('restrictions', [ + 'restrictable_id' => $book->id, + 'restrictable_type' => 'BookStack\Book', + 'role_id' => '2', + 'action' => 'view' + ]); + } + + public function test_chapter_restriction_form() + { + $chapter = \BookStack\Chapter::first(); + $this->asAdmin()->visit($chapter->getUrl() . '/restrict') + ->see('Chapter Restrictions') + ->check('restricted') + ->check('restrictions[2][update]') + ->press('Save Restrictions') + ->seeInDatabase('chapters', ['id' => $chapter->id, 'restricted' => true]) + ->seeInDatabase('restrictions', [ + 'restrictable_id' => $chapter->id, + 'restrictable_type' => 'BookStack\Chapter', + 'role_id' => '2', + 'action' => 'update' + ]); + } + + public function test_page_restriction_form() + { + $page = \BookStack\Page::first(); + $this->asAdmin()->visit($page->getUrl() . '/restrict') + ->see('Page Restrictions') + ->check('restricted') + ->check('restrictions[2][delete]') + ->press('Save Restrictions') + ->seeInDatabase('pages', ['id' => $page->id, 'restricted' => true]) + ->seeInDatabase('restrictions', [ + 'restrictable_id' => $page->id, + 'restrictable_type' => 'BookStack\Page', + 'role_id' => '2', + 'action' => 'delete' + ]); + } + + public function test_restricted_pages_not_visible_in_book_navigation_on_pages() + { + $chapter = \BookStack\Chapter::first(); + $page = $chapter->pages->first(); + $page2 = $chapter->pages[2]; + + $this->setEntityRestrictions($page, []); + + $this->actingAs($this->user) + ->visit($page2->getUrl()) + ->dontSeeInElement('.sidebar-page-list', $page->name); + } + + public function test_restricted_pages_not_visible_in_book_navigation_on_chapters() + { + $chapter = \BookStack\Chapter::first(); + $page = $chapter->pages->first(); + + $this->setEntityRestrictions($page, []); + + $this->actingAs($this->user) + ->visit($chapter->getUrl()) + ->dontSeeInElement('.sidebar-page-list', $page->name); + } + + public function test_restricted_pages_not_visible_on_chapter_pages() + { + $chapter = \BookStack\Chapter::first(); + $page = $chapter->pages->first(); + + $this->setEntityRestrictions($page, []); + + $this->actingAs($this->user) + ->visit($chapter->getUrl()) + ->dontSee($page->name); + } + +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 840fe0d08..567dc93ec 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -1,6 +1,7 @@ assertEquals( + $uri, $this->currentUri, "Did not land on expected page [{$uri}].\n" + ); + + return $this; + } + + /** + * Do a forced visit that does not error out on exception. + * @param string $uri + * @param array $parameters + * @param array $cookies + * @param array $files + * @return $this + */ + protected function forceVisit($uri, $parameters = [], $cookies = [], $files = []) + { + $method = 'GET'; + $uri = $this->prepareUrlForRequest($uri); + $this->call($method, $uri, $parameters, $cookies, $files); + $this->clearInputs()->followRedirects(); + $this->currentUri = $this->app->make('request')->fullUrl(); + $this->crawler = new Crawler($this->response->getContent(), $uri); + return $this; + } + /** * Click the text within the selected element. * @param $parentElement