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); }