From a54be85185225d6f18f94f041546dd663f8f0644 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 26 Feb 2016 23:44:02 +0000 Subject: [PATCH 01/16] 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 02/16] 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 03/16] 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 04/16] 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 05/16] 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 06/16] 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 07/16] 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 38d822e04c669d1929760983d829e9218aad38ed Mon Sep 17 00:00:00 2001 From: Nick Walke Date: Wed, 2 Mar 2016 20:44:00 -0600 Subject: [PATCH 08/16] Closes #69. Implemented and tested memcached. --- .env.example | 8 ++++++++ config/cache.php | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 6d706abdd..9d42d7487 100644 --- a/.env.example +++ b/.env.example @@ -12,6 +12,9 @@ DB_PASSWORD=database_user_password # Cache and session CACHE_DRIVER=file SESSION_DRIVER=file +# If using Memcached, comment the above and uncomment these +#CACHE_DRIVER=memcached +#SESSION_DRIVER=memcached QUEUE_DRIVER=sync # Storage @@ -54,3 +57,8 @@ MAIL_PORT=1025 MAIL_USERNAME=null MAIL_PASSWORD=null MAIL_ENCRYPTION=null + +# Memcached settings +#MEMCACHED_HOST=127.0.0.1 +# If using a UNIX socket path for the host, set the port to 0 +#MEMCACHED_PORT=11211 \ No newline at end of file diff --git a/config/cache.php b/config/cache.php index 379135b0e..b00a9989e 100644 --- a/config/cache.php +++ b/config/cache.php @@ -51,7 +51,9 @@ return [ 'driver' => 'memcached', 'servers' => [ [ - 'host' => '127.0.0.1', 'port' => 11211, 'weight' => 100, + 'host' => env('MEMCACHED_HOST', '127.0.0.1'), + 'port' => env('MEMCACHED_PORT', 11211), + 'weight' => 100, ], ], ], From 5681f4dd6963eb0b39333ba41ece512268534f42 Mon Sep 17 00:00:00 2001 From: Nick Walke Date: Wed, 2 Mar 2016 21:38:23 -0600 Subject: [PATCH 09/16] Closes #70. Added the ability to search by multi-word terms using double quotes. --- app/Repos/BookRepo.php | 9 ++++++++- app/Repos/ChapterRepo.php | 9 ++++++++- app/Repos/PageRepo.php | 9 ++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index d8a24c099..4b45a1a0b 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -226,7 +226,14 @@ class BookRepo */ public function getBySearch($term, $count = 20, $paginationAppends = []) { - $terms = explode(' ', $term); + preg_match_all('/"(.*?)"/', $term, $matches); + if (count($matches[1]) > 0) { + $terms = $matches[1]; + $term = trim(preg_replace('/"(.*?)"/', '', $term)); + } else { + $terms = []; + } + $terms = array_merge($terms, explode(' ', $term)); $books = $this->book->fullTextSearchQuery(['name', 'description'], $terms) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index bba6cbe7a..a3ed0a837 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -131,7 +131,14 @@ class ChapterRepo */ public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { - $terms = explode(' ', $term); + preg_match_all('/"(.*?)"/', $term, $matches); + if (count($matches[1]) > 0) { + $terms = $matches[1]; + $term = trim(preg_replace('/"(.*?)"/', '', $term)); + } else { + $terms = []; + } + $terms = array_merge($terms, explode(' ', $term)); $chapters = $this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index f028a1fcc..fa5f7dd01 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -201,7 +201,14 @@ class PageRepo */ public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { - $terms = explode(' ', $term); + preg_match_all('/"(.*?)"/', $term, $matches); + if (count($matches[1]) > 0) { + $terms = $matches[1]; + $term = trim(preg_replace('/"(.*?)"/', '', $term)); + } else { + $terms = []; + } + $terms = array_merge($terms, explode(' ', $term)); $pages = $this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms) ->paginate($count)->appends($paginationAppends); From 33bf20cfc8e0903da0df4f17481d2a47a9a3ddb5 Mon Sep 17 00:00:00 2001 From: Nick Walke Date: Fri, 4 Mar 2016 02:33:57 -0600 Subject: [PATCH 10/16] Found the source of the issue, not sure how to fix --- app/Entity.php | 13 ++++++++----- app/Repos/PageRepo.php | 4 +++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/Entity.php b/app/Entity.php index 42323628a..122aa2aa2 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -100,10 +100,14 @@ abstract class Entity extends Model */ public static function fullTextSearchQuery($fieldsToSearch, $terms, $wheres = []) { - $termString = ''; - foreach ($terms as $term) { - $termString .= htmlentities($term) . '* '; + foreach ($terms as $key => $term) { + $term = htmlentities($term); + if (preg_match('/\s/', $term)) { + $term = '"' . $term . '"'; + } + $terms[$key] = $term . '*'; } + $termString = "'" . implode(' ', $terms) . "'"; $fields = implode(',', $fieldsToSearch); $termStringEscaped = \DB::connection()->getPdo()->quote($termString); $search = static::addSelect(\DB::raw('*, MATCH(name) AGAINST('.$termStringEscaped.' IN BOOLEAN MODE) AS title_relevance')); @@ -113,9 +117,8 @@ abstract class Entity extends Model foreach ($wheres as $whereTerm) { $search->where($whereTerm[0], $whereTerm[1], $whereTerm[2]); } - // Load in relations - if (static::isA('page')) { + if (static::isA('page')) { $search = $search->with('book', 'chapter', 'createdBy', 'updatedBy'); } else if (static::isA('chapter')) { $search = $search->with('book'); diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index fa5f7dd01..bfaff79ae 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -208,7 +208,9 @@ class PageRepo } else { $terms = []; } - $terms = array_merge($terms, explode(' ', $term)); + if (!empty($term)) { + $terms = array_merge($terms, explode(' ', $term)); + } $pages = $this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms) ->paginate($count)->appends($paginationAppends); From bc2b310638d7769e3e0f6c168e0c0618d5f245ec Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 4 Mar 2016 20:49:35 +0000 Subject: [PATCH 11/16] Updated fulltext search with custom escaped query Changed pdo quoted query to use custom escaping and added like searches when quoted terms are used. --- app/Entity.php | 29 ++++++++++++++++++++++------- app/Repos/BookRepo.php | 4 +++- app/Repos/ChapterRepo.php | 4 +++- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/Entity.php b/app/Entity.php index 122aa2aa2..4e85e252e 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -87,8 +87,8 @@ abstract class Entity extends Model */ public function getShortName($length = 25) { - if(strlen($this->name) <= $length) return $this->name; - return substr($this->name, 0, $length-3) . '...'; + if (strlen($this->name) <= $length) return $this->name; + return substr($this->name, 0, $length - 3) . '...'; } /** @@ -100,19 +100,34 @@ abstract class Entity extends Model */ public static function fullTextSearchQuery($fieldsToSearch, $terms, $wheres = []) { + $exactTerms = []; foreach ($terms as $key => $term) { - $term = htmlentities($term); + $term = htmlentities($term, ENT_QUOTES); + $term = preg_replace('/[+\-><\(\)~*\"@]+/', ' ', $term); if (preg_match('/\s/', $term)) { + $exactTerms[] = '%' . $term . '%'; $term = '"' . $term . '"'; + } else { + $term = '' . $term . '*'; } - $terms[$key] = $term . '*'; + if ($term !== '*') $terms[$key] = $term; } - $termString = "'" . implode(' ', $terms) . "'"; + $termString = implode(' ', $terms); $fields = implode(',', $fieldsToSearch); - $termStringEscaped = \DB::connection()->getPdo()->quote($termString); - $search = static::addSelect(\DB::raw('*, MATCH(name) AGAINST('.$termStringEscaped.' IN BOOLEAN MODE) AS title_relevance')); + $search = static::selectRaw('*, MATCH(name) AGAINST(? IN BOOLEAN MODE) AS title_relevance', [$termString]); $search = $search->whereRaw('MATCH(' . $fields . ') AGAINST(? IN BOOLEAN MODE)', [$termString]); + // Ensure at least one exact term matches if in search + if (count($exactTerms) > 0) { + $search = $search->where(function($query) use ($exactTerms, $fieldsToSearch) { + foreach ($exactTerms as $exactTerm) { + foreach ($fieldsToSearch as $field) { + $query->orWhere($field, 'like', $exactTerm); + } + } + }); + } + // Add additional where terms foreach ($wheres as $whereTerm) { $search->where($whereTerm[0], $whereTerm[1], $whereTerm[2]); diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index 4b45a1a0b..afd802337 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -233,7 +233,9 @@ class BookRepo } else { $terms = []; } - $terms = array_merge($terms, explode(' ', $term)); + if (!empty($term)) { + $terms = array_merge($terms, explode(' ', $term)); + } $books = $this->book->fullTextSearchQuery(['name', 'description'], $terms) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index a3ed0a837..542cdd532 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -138,7 +138,9 @@ class ChapterRepo } else { $terms = []; } - $terms = array_merge($terms, explode(' ', $term)); + if (!empty($term)) { + $terms = array_merge($terms, explode(' ', $term)); + } $chapters = $this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); From 479dd80a8c0a971e5fc5db8679f3677205e44109 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 5 Mar 2016 09:47:24 +0000 Subject: [PATCH 12/16] Made memcached config allow mulitple servers --- .env.example | 13 +++++++------ config/cache.php | 21 ++++++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 9d42d7487..5661cda22 100644 --- a/.env.example +++ b/.env.example @@ -17,6 +17,12 @@ SESSION_DRIVER=file #SESSION_DRIVER=memcached QUEUE_DRIVER=sync +# Memcached settings +# If using a UNIX socket path for the host, set the port to 0 +# This follows the following format: HOST:PORT:WEIGHT +# For multiple servers separate with a comma +MEMCACHED_SERVERS=127.0.0.1:11211:100 + # Storage STORAGE_TYPE=local # Amazon S3 Config @@ -56,9 +62,4 @@ MAIL_HOST=localhost MAIL_PORT=1025 MAIL_USERNAME=null MAIL_PASSWORD=null -MAIL_ENCRYPTION=null - -# Memcached settings -#MEMCACHED_HOST=127.0.0.1 -# If using a UNIX socket path for the host, set the port to 0 -#MEMCACHED_PORT=11211 \ No newline at end of file +MAIL_ENCRYPTION=null \ No newline at end of file diff --git a/config/cache.php b/config/cache.php index b00a9989e..076a0299f 100644 --- a/config/cache.php +++ b/config/cache.php @@ -1,5 +1,18 @@ $memcachedServer) { + $memcachedServerDetails = explode(':', $memcachedServer); + $components = count($memcachedServerDetails); + if ($components < 2) $memcachedServerDetails[] = '11211'; + if ($components < 3) $memcachedServerDetails[] = '100'; + $memcachedServers[$index] = array_combine($memcachedServerKeys, $memcachedServerDetails); + } +} + return [ /* @@ -49,13 +62,7 @@ return [ 'memcached' => [ 'driver' => 'memcached', - 'servers' => [ - [ - 'host' => env('MEMCACHED_HOST', '127.0.0.1'), - 'port' => env('MEMCACHED_PORT', 11211), - 'weight' => 100, - ], - ], + 'servers' => env('CACHE_DRIVER') === 'memcached' ? $memcachedServers : [], ], 'redis' => [ From 268db6b1d0409766014ae9f1681ec1bf5bab7552 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 5 Mar 2016 12:09:09 +0000 Subject: [PATCH 13/16] 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 14/16] 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 From 76eaf64f945c822824fdc650893cabed8b3ec1d2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 5 Mar 2016 19:00:26 +0000 Subject: [PATCH 15/16] Fixed errors that occured when merging & refactored entity repositories Also deleted the git '.orig' files that got added in last merge. --- app/Repos/BookRepo.php | 50 +--- app/Repos/BookRepo.php.orig | 295 --------------------- app/Repos/ChapterRepo.php | 54 +--- app/Repos/ChapterRepo.php.orig | 226 ---------------- app/Repos/EntityRepo.php | 83 ++++-- app/Repos/PageRepo.php | 51 +--- app/Repos/PageRepo.php.orig | 437 ------------------------------- app/Services/ActivityService.php | 1 - 8 files changed, 80 insertions(+), 1117 deletions(-) delete mode 100644 app/Repos/BookRepo.php.orig delete mode 100644 app/Repos/ChapterRepo.php.orig delete mode 100644 app/Repos/PageRepo.php.orig diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index 816db4cf0..2ec9a4c25 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -1,31 +1,25 @@ book = $book; $this->pageRepo = $pageRepo; $this->chapterRepo = $chapterRepo; - $this->restrictionService = $restrictionService; + parent::__construct(); } /** @@ -90,7 +84,6 @@ class BookRepo */ public function getRecentlyViewed($count = 10, $page = 0) { - // TODO restrict return Views::getUserRecentlyViewed($count, $page, $this->book); } @@ -102,7 +95,6 @@ class BookRepo */ public function getPopular($count = 10, $page = 0) { - // TODO - Restrict return Views::getPopular($count, $page, $this->book); } @@ -241,16 +233,7 @@ class BookRepo */ public function getBySearch($term, $count = 20, $paginationAppends = []) { - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } + $terms = $this->prepareSearchTerms($term); $books = $this->restrictionService->enforceBookRestrictions($this->book->fullTextSearchQuery(['name', 'description'], $terms)) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); @@ -262,27 +245,4 @@ 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/BookRepo.php.orig b/app/Repos/BookRepo.php.orig deleted file mode 100644 index 4b9709fe6..000000000 --- a/app/Repos/BookRepo.php.orig +++ /dev/null @@ -1,295 +0,0 @@ -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'); - } - - /** - * Get the book that has the given id. - * @param $id - * @return mixed - */ - public function getById($id) - { - return $this->bookQuery()->findOrFail($id); - } - - /** - * Get all books, Limited by count. - * @param int $count - * @return mixed - */ - public function getAll($count = 10) - { - $bookQuery = $this->bookQuery()->orderBy('name', 'asc'); - if (!$count) return $bookQuery->get(); - return $bookQuery->take($count)->get(); - } - - /** - * Get all books paginated. - * @param int $count - * @return mixed - */ - public function getAllPaginated($count = 10) - { - return $this->bookQuery() - ->orderBy('name', 'asc')->paginate($count); - } - - - /** - * Get the latest books. - * @param int $count - * @return mixed - */ - public function getLatest($count = 10) - { - return $this->bookQuery()->orderBy('created_at', 'desc')->take($count)->get(); - } - - /** - * Gets the most recently viewed for a user. - * @param int $count - * @param int $page - * @return mixed - */ - public function getRecentlyViewed($count = 10, $page = 0) - { - // TODO restrict - return Views::getUserRecentlyViewed($count, $page, $this->book); - } - - /** - * Gets the most viewed books. - * @param int $count - * @param int $page - * @return mixed - */ - public function getPopular($count = 10, $page = 0) - { - // TODO - Restrict - return Views::getPopular($count, $page, $this->book); - } - - /** - * Get a book by slug - * @param $slug - * @return mixed - * @throws NotFoundException - */ - public function getBySlug($slug) - { - $book = $this->bookQuery()->where('slug', '=', $slug)->first(); - if ($book === null) throw new NotFoundException('Book not found'); - return $book; - } - - /** - * Checks if a book exists. - * @param $id - * @return bool - */ - public function exists($id) - { - return $this->bookQuery()->where('id', '=', $id)->exists(); - } - - /** - * Get a new book instance from request input. - * @param $input - * @return Book - */ - public function newFromInput($input) - { - return $this->book->newInstance($input); - } - - /** - * Destroy a book identified by the given slug. - * @param $bookSlug - */ - public function destroyBySlug($bookSlug) - { - $book = $this->getBySlug($bookSlug); - foreach ($book->pages as $page) { - $this->pageRepo->destroy($page); - } - foreach ($book->chapters as $chapter) { - $this->chapterRepo->destroy($chapter); - } - $book->views()->delete(); - $book->restrictions()->delete(); - $book->delete(); - } - - /** - * Get the next child element priority. - * @param Book $book - * @return int - */ - public function getNewPriority($book) - { - $lastElem = $this->getChildren($book)->pop(); - return $lastElem ? $lastElem->priority + 1 : 0; - } - - /** - * @param string $slug - * @param bool|false $currentId - * @return bool - */ - public function doesSlugExist($slug, $currentId = false) - { - $query = $this->book->where('slug', '=', $slug); - if ($currentId) { - $query = $query->where('id', '!=', $currentId); - } - return $query->count() > 0; - } - - /** - * Provides a suitable slug for the given book name. - * Ensures the returned slug is unique in the system. - * @param string $name - * @param bool|false $currentId - * @return string - */ - public function findSuitableSlug($name, $currentId = false) - { - $originalSlug = Str::slug($name); - $slug = $originalSlug; - $count = 2; - while ($this->doesSlugExist($slug, $currentId)) { - $slug = $originalSlug . '-' . $count; - $count++; - } - return $slug; - } - - /** - * Get all child objects of a book. - * Returns a sorted collection of Pages and Chapters. - * Loads the bookslug onto child elements to prevent access database access for getting the slug. - * @param Book $book - * @return mixed - */ - public function getChildren(Book $book) - { - $pageQuery = $book->pages()->where('chapter_id', '=', 0); - $pageQuery = $this->restrictionService->enforcePageRestrictions($pageQuery, 'view'); - $pages = $pageQuery->get(); - - $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; - $children->each(function ($child) use ($bookSlug) { - $child->setAttribute('bookSlug', $bookSlug); - if ($child->isA('chapter')) { - $child->pages->each(function ($page) use ($bookSlug) { - $page->setAttribute('bookSlug', $bookSlug); - }); - } - }); - return $children->sortBy('priority'); - } - - /** - * Get books by search term. - * @param $term - * @param int $count - * @param array $paginationAppends - * @return mixed - */ - public function getBySearch($term, $count = 20, $paginationAppends = []) - { -<<<<<<< HEAD - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } - $books = $this->book->fullTextSearchQuery(['name', 'description'], $terms) -======= - $terms = explode(' ', $term); - $books = $this->restrictionService->enforceBookRestrictions($this->book->fullTextSearchQuery(['name', 'description'], $terms)) ->>>>>>> custom_role_system - ->paginate($count)->appends($paginationAppends); - $words = join('|', explode(' ', preg_quote(trim($term), '/'))); - foreach ($books as $book) { - //highlight - $result = preg_replace('#' . $words . '#iu', "\$0", $book->getExcerpt(100)); - $book->searchSnippet = $result; - } - 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 6868bbf89..5d1d6437f 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -3,27 +3,11 @@ use Activity; use BookStack\Exceptions\NotFoundException; -use BookStack\Services\RestrictionService; use Illuminate\Support\Str; use BookStack\Chapter; -class ChapterRepo +class ChapterRepo extends EntityRepo { - - protected $chapter; - protected $restrictionService; - - /** - * ChapterRepo constructor. - * @param Chapter $chapter - * @param RestrictionService $restrictionService - */ - public function __construct(Chapter $chapter, RestrictionService $restrictionService) - { - $this->chapter = $chapter; - $this->restrictionService = $restrictionService; - } - /** * Base query for getting chapters, Takes restrictions into account. * @return mixed @@ -148,7 +132,7 @@ class ChapterRepo /** * Get chapters by the given search term. - * @param $term + * @param string $term * @param array $whereTerms * @param int $count * @param array $paginationAppends @@ -156,16 +140,7 @@ class ChapterRepo */ public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } + $terms = $this->prepareSearchTerms($term); $chapters = $this->restrictionService->enforceChapterRestrictions($this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms)) ->paginate($count)->appends($paginationAppends); $words = join('|', explode(' ', preg_quote(trim($term), '/'))); @@ -195,27 +170,4 @@ 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/ChapterRepo.php.orig b/app/Repos/ChapterRepo.php.orig deleted file mode 100644 index be4a4e6b5..000000000 --- a/app/Repos/ChapterRepo.php.orig +++ /dev/null @@ -1,226 +0,0 @@ -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'); - } - - /** - * Check if an id exists. - * @param $id - * @return bool - */ - public function idExists($id) - { - return $this->chapterQuery()->where('id', '=', $id)->count() > 0; - } - - /** - * Get a chapter by a specific id. - * @param $id - * @return mixed - */ - public function getById($id) - { - return $this->chapterQuery()->findOrFail($id); - } - - /** - * Get all chapters. - * @return \Illuminate\Database\Eloquent\Collection|static[] - */ - public function getAll() - { - return $this->chapterQuery()->all(); - } - - /** - * Get a chapter that has the given slug within the given book. - * @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) 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 - * @return $this - */ - public function newFromInput($input) - { - return $this->chapter->fill($input); - } - - /** - * Destroy a chapter and its relations by providing its slug. - * @param Chapter $chapter - */ - public function destroy(Chapter $chapter) - { - if (count($chapter->pages) > 0) { - foreach ($chapter->pages as $page) { - $page->chapter_id = 0; - $page->save(); - } - } - Activity::removeEntity($chapter); - $chapter->views()->delete(); - $chapter->restrictions()->delete(); - $chapter->delete(); - } - - /** - * Check if a chapter's slug exists. - * @param $slug - * @param $bookId - * @param bool|false $currentId - * @return bool - */ - public function doesSlugExist($slug, $bookId, $currentId = false) - { - $query = $this->chapter->where('slug', '=', $slug)->where('book_id', '=', $bookId); - if ($currentId) { - $query = $query->where('id', '!=', $currentId); - } - return $query->count() > 0; - } - - /** - * Finds a suitable slug for the provided name. - * Checks database to prevent duplicate slugs. - * @param $name - * @param $bookId - * @param bool|false $currentId - * @return string - */ - public function findSuitableSlug($name, $bookId, $currentId = false) - { - $slug = Str::slug($name); - while ($this->doesSlugExist($slug, $bookId, $currentId)) { - $slug .= '-' . substr(md5(rand(1, 500)), 0, 3); - } - return $slug; - } - - /** - * Get chapters by the given search term. - * @param $term - * @param array $whereTerms - * @param int $count - * @param array $paginationAppends - * @return mixed - */ - public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) - { -<<<<<<< HEAD - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } - $chapters = $this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms) -======= - $terms = explode(' ', $term); - $chapters = $this->restrictionService->enforceChapterRestrictions($this->chapter->fullTextSearchQuery(['name', 'description'], $terms, $whereTerms)) ->>>>>>> custom_role_system - ->paginate($count)->appends($paginationAppends); - $words = join('|', explode(' ', preg_quote(trim($term), '/'))); - foreach ($chapters as $chapter) { - //highlight - $result = preg_replace('#' . $words . '#iu', "\$0", $chapter->getExcerpt(100)); - $chapter->searchSnippet = $result; - } - return $chapters; - } - - /** - * Changes the book relation of this chapter. - * @param $bookId - * @param Chapter $chapter - * @return Chapter - */ - public function changeBook($bookId, Chapter $chapter) - { - $chapter->book_id = $bookId; - foreach ($chapter->activity as $activity) { - $activity->book_id = $bookId; - $activity->save(); - } - $chapter->slug = $this->findSuitableSlug($chapter->name, $bookId, $chapter->id); - $chapter->save(); - 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/EntityRepo.php b/app/Repos/EntityRepo.php index 46e1d98a5..9c5184e2f 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -1,32 +1,43 @@ book = $book; - $this->chapter = $chapter; - $this->page = $page; - $this->restrictionService = $restrictionService; + $this->book = app(Book::class); + $this->chapter = app(Chapter::class); + $this->page = app(Page::class); + $this->restrictionService = app(RestrictionService::class); } /** @@ -37,7 +48,7 @@ class EntityRepo public function getRecentlyCreatedBooks($count = 20, $page = 0) { return $this->restrictionService->enforceBookRestrictions($this->book) - ->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); + ->orderBy('created_at', 'desc')->skip($page * $count)->take($count)->get(); } /** @@ -49,7 +60,7 @@ class EntityRepo public function getRecentlyUpdatedBooks($count = 20, $page = 0) { return $this->restrictionService->enforceBookRestrictions($this->book) - ->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); + ->orderBy('updated_at', 'desc')->skip($page * $count)->take($count)->get(); } /** @@ -60,7 +71,7 @@ class EntityRepo public function getRecentlyCreatedPages($count = 20, $page = 0) { return $this->restrictionService->enforcePageRestrictions($this->page) - ->orderBy('created_at', 'desc')->skip($page*$count)->take($count)->get(); + ->orderBy('created_at', 'desc')->skip($page * $count)->take($count)->get(); } /** @@ -72,7 +83,49 @@ class EntityRepo public function getRecentlyUpdatedPages($count = 20, $page = 0) { return $this->restrictionService->enforcePageRestrictions($this->page) - ->orderBy('updated_at', 'desc')->skip($page*$count)->take($count)->get(); + ->orderBy('updated_at', 'desc')->skip($page * $count)->take($count)->get(); + } + + /** + * Updates entity restrictions from a request + * @param $request + * @param Entity $entity + */ + public function updateRestrictionsFromRequest($request, Entity $entity) + { + $entity->restricted = $request->has('restricted') && $request->get('restricted') === 'true'; + $entity->restrictions()->delete(); + if ($request->has('restrictions')) { + foreach ($request->get('restrictions') as $roleId => $restrictions) { + foreach ($restrictions as $action => $value) { + $entity->restrictions()->create([ + 'role_id' => $roleId, + 'action' => strtolower($action) + ]); + } + } + } + $entity->save(); + } + + /** + * Prepare a string of search terms by turning + * it into an array of terms. + * Keeps quoted terms together. + * @param $termString + * @return array + */ + protected function prepareSearchTerms($termString) + { + preg_match_all('/"(.*?)"/', $termString, $matches); + if (count($matches[1]) > 0) { + $terms = $matches[1]; + $termString = trim(preg_replace('/"(.*?)"/', '', $termString)); + } else { + $terms = []; + } + if (!empty($termString)) $terms = array_merge($terms, explode(' ', $termString)); + return $terms; } diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index 3d675183e..4784ad407 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -3,34 +3,23 @@ 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; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; use BookStack\Page; use BookStack\PageRevision; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; -class PageRepo +class PageRepo extends EntityRepo { - protected $page; protected $pageRevision; - protected $restrictionService; /** * PageRepo constructor. - * @param Page $page * @param PageRevision $pageRevision - * @param RestrictionService $restrictionService */ - public function __construct(Page $page, PageRevision $pageRevision, RestrictionService $restrictionService) + public function __construct(PageRevision $pageRevision) { - $this->page = $page; $this->pageRevision = $pageRevision; - $this->restrictionService = $restrictionService; + parent::__construct(); } /** @@ -200,16 +189,7 @@ class PageRepo */ public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) { - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } + $terms = $this->prepareSearchTerms($term); $pages = $this->restrictionService->enforcePageRestrictions($this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms)) ->paginate($count)->appends($paginationAppends); @@ -416,27 +396,4 @@ class PageRepo return $this->pageQuery()->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/PageRepo.php.orig b/app/Repos/PageRepo.php.orig deleted file mode 100644 index c1e02b501..000000000 --- a/app/Repos/PageRepo.php.orig +++ /dev/null @@ -1,437 +0,0 @@ -page = $page; - $this->pageRevision = $pageRevision; - $this->restrictionService = $restrictionService; - } - - /** - * Base query for getting pages, Takes restrictions into account. - * @return mixed - */ - private function pageQuery() - { - return $this->restrictionService->enforcePageRestrictions($this->page, 'view'); - } - - /** - * Get a page via a specific ID. - * @param $id - * @return mixed - */ - public function getById($id) - { - return $this->pageQuery()->findOrFail($id); - } - - /** - * Get a page identified by the given slug. - * @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 NotFoundException('Page not found'); - return $page; - } - - /** - * Search through page revisions and retrieve - * the last page in the current book that - * has a slug equal to the one given. - * @param $pageSlug - * @param $bookSlug - * @return null | Page - */ - 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; - } - - /** - * Get a new Page instance from the given input. - * @param $input - * @return Page - */ - public function newFromInput($input) - { - $page = $this->page->fill($input); - return $page; - } - - - /** - * Save a new page into the system. - * Input validation must be done beforehand. - * @param array $input - * @param Book $book - * @param int $chapterId - * @return Page - */ - public function saveNew(array $input, Book $book, $chapterId = null) - { - $page = $this->newFromInput($input); - $page->slug = $this->findSuitableSlug($page->name, $book->id); - - if ($chapterId) $page->chapter_id = $chapterId; - - $page->html = $this->formatHtml($input['html']); - $page->text = strip_tags($page->html); - $page->created_by = auth()->user()->id; - $page->updated_by = auth()->user()->id; - - $book->pages()->save($page); - return $page; - } - - /** - * Formats a page's html to be tagged correctly - * within the system. - * @param string $htmlText - * @return string - */ - protected function formatHtml($htmlText) - { - if($htmlText == '') return $htmlText; - libxml_use_internal_errors(true); - $doc = new \DOMDocument(); - $doc->loadHTML(mb_convert_encoding($htmlText, 'HTML-ENTITIES', 'UTF-8')); - - $container = $doc->documentElement; - $body = $container->childNodes->item(0); - $childNodes = $body->childNodes; - - // Ensure no duplicate ids are used - $idArray = []; - - foreach ($childNodes as $index => $childNode) { - /** @var \DOMElement $childNode */ - if (get_class($childNode) !== 'DOMElement') continue; - - // Overwrite id if not a BookStack custom id - if ($childNode->hasAttribute('id')) { - $id = $childNode->getAttribute('id'); - if (strpos($id, 'bkmrk') === 0 && array_search($id, $idArray) === false) { - $idArray[] = $id; - continue; - }; - } - - // Create an unique id for the element - // Uses the content as a basis to ensure output is the same every time - // the same content is passed through. - $contentId = 'bkmrk-' . substr(strtolower(preg_replace('/\s+/', '-', trim($childNode->nodeValue))), 0, 20); - $newId = urlencode($contentId); - $loopIndex = 0; - while (in_array($newId, $idArray)) { - $newId = urlencode($contentId . '-' . $loopIndex); - $loopIndex++; - } - - $childNode->setAttribute('id', $newId); - $idArray[] = $newId; - } - - // Generate inner html as a string - $html = ''; - foreach ($childNodes as $childNode) { - $html .= $doc->saveHTML($childNode); - } - - return $html; - } - - - /** - * Gets pages by a search term. - * Highlights page content for showing in results. - * @param string $term - * @param array $whereTerms - * @param int $count - * @param array $paginationAppends - * @return mixed - */ - public function getBySearch($term, $whereTerms = [], $count = 20, $paginationAppends = []) - { -<<<<<<< HEAD - preg_match_all('/"(.*?)"/', $term, $matches); - if (count($matches[1]) > 0) { - $terms = $matches[1]; - $term = trim(preg_replace('/"(.*?)"/', '', $term)); - } else { - $terms = []; - } - if (!empty($term)) { - $terms = array_merge($terms, explode(' ', $term)); - } - $pages = $this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms) -======= - $terms = explode(' ', $term); - $pages = $this->restrictionService->enforcePageRestrictions($this->page->fullTextSearchQuery(['name', 'text'], $terms, $whereTerms)) ->>>>>>> custom_role_system - ->paginate($count)->appends($paginationAppends); - - // Add highlights to page text. - $words = join('|', explode(' ', preg_quote(trim($term), '/'))); - //lookahead/behind assertions ensures cut between words - $s = '\s\x00-/:-@\[-`{-~'; //character set for start/end of words - - foreach ($pages as $page) { - preg_match_all('#(?<=[' . $s . ']).{1,30}((' . $words . ').{1,30})+(?=[' . $s . '])#uis', $page->text, $matches, PREG_SET_ORDER); - //delimiter between occurrences - $results = []; - foreach ($matches as $line) { - $results[] = htmlspecialchars($line[0], 0, 'UTF-8'); - } - $matchLimit = 6; - if (count($results) > $matchLimit) { - $results = array_slice($results, 0, $matchLimit); - } - $result = join('... ', $results); - - //highlight - $result = preg_replace('#' . $words . '#iu', "\$0", $result); - if (strlen($result) < 5) { - $result = $page->getExcerpt(80); - } - $page->searchSnippet = $result; - } - return $pages; - } - - /** - * Search for image usage. - * @param $imageString - * @return mixed - */ - public function searchForImage($imageString) - { - $pages = $this->pageQuery()->where('html', 'like', '%' . $imageString . '%')->get(); - foreach ($pages as $page) { - $page->url = $page->getUrl(); - $page->html = ''; - $page->text = ''; - } - return count($pages) > 0 ? $pages : false; - } - - /** - * Updates a page with any fillable data and saves it into the database. - * @param Page $page - * @param int $book_id - * @param string $input - * @return Page - */ - public function updatePage(Page $page, $book_id, $input) - { - // Save a revision before updating - if ($page->html !== $input['html'] || $page->name !== $input['name']) { - $this->saveRevision($page); - } - - // Prevent slug being updated if no name change - if ($page->name !== $input['name']) { - $page->slug = $this->findSuitableSlug($input['name'], $book_id, $page->id); - } - - // Update with new details - $page->fill($input); - $page->html = $this->formatHtml($input['html']); - $page->text = strip_tags($page->html); - $page->updated_by = auth()->user()->id; - $page->save(); - return $page; - } - - /** - * Restores a revision's content back into a page. - * @param Page $page - * @param Book $book - * @param int $revisionId - * @return Page - */ - public function restoreRevision(Page $page, Book $book, $revisionId) - { - $this->saveRevision($page); - $revision = $this->getRevisionById($revisionId); - $page->fill($revision->toArray()); - $page->slug = $this->findSuitableSlug($page->name, $book->id, $page->id); - $page->text = strip_tags($page->html); - $page->updated_by = auth()->user()->id; - $page->save(); - return $page; - } - - /** - * Saves a page revision into the system. - * @param Page $page - * @return $this - */ - public function saveRevision(Page $page) - { - $revision = $this->pageRevision->fill($page->toArray()); - $revision->page_id = $page->id; - $revision->slug = $page->slug; - $revision->book_slug = $page->book->slug; - $revision->created_by = auth()->user()->id; - $revision->created_at = $page->updated_at; - $revision->save(); - // Clear old revisions - if ($this->pageRevision->where('page_id', '=', $page->id)->count() > 50) { - $this->pageRevision->where('page_id', '=', $page->id) - ->orderBy('created_at', 'desc')->skip(50)->take(5)->delete(); - } - return $revision; - } - - /** - * Gets a single revision via it's id. - * @param $id - * @return mixed - */ - public function getRevisionById($id) - { - return $this->pageRevision->findOrFail($id); - } - - /** - * Checks if a slug exists within a book already. - * @param $slug - * @param $bookId - * @param bool|false $currentId - * @return bool - */ - public function doesSlugExist($slug, $bookId, $currentId = false) - { - $query = $this->page->where('slug', '=', $slug)->where('book_id', '=', $bookId); - if ($currentId) $query = $query->where('id', '!=', $currentId); - return $query->count() > 0; - } - - /** - * Changes the related book for the specified page. - * Changes the book id of any relations to the page that store the book id. - * @param int $bookId - * @param Page $page - * @return Page - */ - public function changeBook($bookId, Page $page) - { - $page->book_id = $bookId; - foreach ($page->activity as $activity) { - $activity->book_id = $bookId; - $activity->save(); - } - $page->slug = $this->findSuitableSlug($page->name, $bookId, $page->id); - $page->save(); - return $page; - } - - /** - * Gets a suitable slug for the resource - * @param $name - * @param $bookId - * @param bool|false $currentId - * @return string - */ - public function findSuitableSlug($name, $bookId, $currentId = false) - { - $slug = Str::slug($name); - while ($this->doesSlugExist($slug, $bookId, $currentId)) { - $slug .= '-' . substr(md5(rand(1, 500)), 0, 3); - } - return $slug; - } - - /** - * Destroy a given page along with its dependencies. - * @param $page - */ - public function destroy($page) - { - Activity::removeEntity($page); - $page->views()->delete(); - $page->revisions()->delete(); - $page->restrictions()->delete(); - $page->delete(); - } - - /** - * Get the latest pages added to the system. - * @param $count - */ - public function getRecentlyCreatedPaginated($count = 20) - { - return $this->pageQuery()->orderBy('created_at', 'desc')->paginate($count); - } - - /** - * Get the latest pages added to the system. - * @param $count - */ - public function getRecentlyUpdatedPaginated($count = 20) - { - return $this->pageQuery()->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/Services/ActivityService.php b/app/Services/ActivityService.php index a57210b8d..118bd6d9c 100644 --- a/app/Services/ActivityService.php +++ b/app/Services/ActivityService.php @@ -1,6 +1,5 @@ Date: Sat, 5 Mar 2016 22:54:53 +0000 Subject: [PATCH 16/16] Fixed incorrect recents pages on homescreen Fixed the bug causing the recently updated pages to be exaclty the same as the recently create pages. Also added in tests to prevent regression. --- app/Http/Controllers/HomeController.php | 1 - resources/views/home.blade.php | 8 ++++++-- tests/EntityTest.php | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php index e20c89e06..489396df6 100644 --- a/app/Http/Controllers/HomeController.php +++ b/app/Http/Controllers/HomeController.php @@ -24,7 +24,6 @@ class HomeController extends Controller /** * Display the homepage. - * * @return Response */ public function index() diff --git a/resources/views/home.blade.php b/resources/views/home.blade.php index 8aaae1e1b..f840be965 100644 --- a/resources/views/home.blade.php +++ b/resources/views/home.blade.php @@ -33,10 +33,14 @@

    Recently Created Pages

    - @include('partials/entity-list', ['entities' => $recentlyCreatedPages, 'style' => 'compact']) +
    + @include('partials/entity-list', ['entities' => $recentlyCreatedPages, 'style' => 'compact']) +

    Recently Updated Pages

    - @include('partials/entity-list', ['entities' => $recentlyCreatedPages, 'style' => 'compact']) +
    + @include('partials/entity-list', ['entities' => $recentlyUpdatedPages, 'style' => 'compact']) +
    diff --git a/tests/EntityTest.php b/tests/EntityTest.php index 2936fc047..30858f8d9 100644 --- a/tests/EntityTest.php +++ b/tests/EntityTest.php @@ -225,4 +225,22 @@ class EntityTest extends TestCase ->seePageIs($newPageUrl); } + public function test_recently_updated_pages_on_home() + { + $page = \BookStack\Page::orderBy('updated_at', 'asc')->first(); + $this->asAdmin()->visit('/') + ->dontSeeInElement('#recently-updated-pages', $page->name); + $this->visit($page->getUrl() . '/edit') + ->press('Save Page') + ->visit('/') + ->seeInElement('#recently-updated-pages', $page->name); + } + + public function test_recently_created_pages_on_home() + { + $entityChain = $this->createEntityChainBelongingToUser($this->getNewUser()); + $this->asAdmin()->visit('/') + ->seeInElement('#recently-created-pages', $entityChain['page']->name); + } + }