From 6e03078de3860cc82e30b14ebbcd192a797fd75f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 9 Apr 2016 12:40:07 +0100 Subject: [PATCH 1/8] Started work towards adding role view permissions Work halted as re-write required. In reference to #92 --- app/Http/Controllers/BookController.php | 7 +-- app/Http/Controllers/ChapterController.php | 1 + app/Http/Controllers/PageController.php | 2 + ...9_100730_add_view_permissions_to_roles.php | 54 +++++++++++++++++++ resources/views/settings/roles/form.blade.php | 14 +++++ 5 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 3390b41c0..46636016f 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -1,13 +1,9 @@ -bookRepo->getBySlug($slug); + $this->checkOwnablePermission('book-view', $book); $bookChildren = $this->bookRepo->getChildren($book); Views::add($book); $this->setPageTitle($book->getShortName()); diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index 4641ddbdb..d1c6c1733 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -77,6 +77,7 @@ class ChapterController extends Controller { $book = $this->bookRepo->getBySlug($bookSlug); $chapter = $this->chapterRepo->getBySlug($chapterSlug, $book->id); + $this->checkOwnablePermission('chapter-view', $chapter); $sidebarTree = $this->bookRepo->getChildren($book); Views::add($chapter); $this->setPageTitle($chapter->getShortName()); diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index e250d8c85..30d6c2d76 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -127,6 +127,8 @@ class PageController extends Controller return redirect($page->getUrl()); } + $this->checkOwnablePermission('page-view', $page); + $sidebarTree = $this->bookRepo->getChildren($book); Views::add($page); $this->setPageTitle($page->getShortName()); diff --git a/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php b/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php new file mode 100644 index 000000000..dabd6a25e --- /dev/null +++ b/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php @@ -0,0 +1,54 @@ +name = strtolower($entity) . '-' . strtolower(str_replace(' ', '-', $op)); + $newPermission->display_name = $op . ' ' . $entity . 's'; + $newPermission->save(); + foreach ($currentRoles as $role) { + $role->attachPermission($newPermission); + } + } + } + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + // Delete the new view permissions + $entities = ['Book', 'Page', 'Chapter']; + $ops = ['View All', 'View Own']; + foreach ($entities as $entity) { + foreach ($ops as $op) { + $permissionName = strtolower($entity) . '-' . strtolower(str_replace(' ', '-', $op)); + $newPermission = \BookStack\Permission::where('name', '=', $permissionName)->first(); + foreach ($newPermission->roles as $role) { + $role->detachPermission($newPermission); + } + $newPermission->delete(); + } + } + } +} diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index ba57b4daa..cd81febb1 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -49,6 +49,7 @@ Create + View Edit Delete @@ -57,6 +58,10 @@ + + + + @@ -72,6 +77,10 @@ + + + + @@ -87,6 +96,10 @@ + + + + @@ -99,6 +112,7 @@ Images @include('settings/roles/checkbox', ['permission' => 'image-create-all']) + From ea287ebf86e2cafcab167eb048a04b6f80146d16 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 20 Apr 2016 21:37:57 +0100 Subject: [PATCH 2/8] Started creation of intermediate permission table --- app/EntityPermission.php | 28 +++++++ app/Services/RestrictionService.php | 79 ++++++++++++++++++- ...192649_create_entity_permissions_table.php | 34 ++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 app/EntityPermission.php create mode 100644 database/migrations/2016_04_20_192649_create_entity_permissions_table.php diff --git a/app/EntityPermission.php b/app/EntityPermission.php new file mode 100644 index 000000000..6b4ddd212 --- /dev/null +++ b/app/EntityPermission.php @@ -0,0 +1,28 @@ +belongsTo(Role::class); + } + + /** + * Get the entity this points to. + * @return \Illuminate\Database\Eloquent\Relations\MorphOne + */ + public function entity() + { + return $this->morphOne(Entity::class, 'entity'); + } +} diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 50cbe4a51..8d57b9edc 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -1,6 +1,13 @@ currentUser = auth()->user(); $this->userRoles = $this->currentUser ? $this->currentUser->roles->pluck('id') : []; $this->isAdmin = $this->currentUser ? $this->currentUser->hasRole('admin') : false; + + $this->entityPermission = $entityPermission; + $this->role = $role; + $this->permission = $permission; + $this->book = $book; + $this->chapter = $chapter; + $this->page = $page; + } + + + /** + * Re-generate all entity permission from scratch. + */ + public function buildEntityPermissions() + { + $this->entityPermission->truncate(); + + // Get all roles (Should be the most limited dimension) + $roles = $this->role->load('permissions')->all(); + + // Chunk through all books + $this->book->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + + // Chunk through all chapters + $this->chapter->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + + // Chunk through all pages + $this->page->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + } + + /** + * Create & Save entity permissions for many entities and permissions. + * @param Collection $entities + * @param Collection $roles + */ + protected function createManyEntityPermissions($entities, $roles) + { + $entityPermissions = []; + foreach ($entities as $entity) { + foreach ($roles as $role) { + $entityPermissions[] = $this->createEntityPermission($entity, $role); + } + } + $this->entityPermission->insert($entityPermissions); + } + + + protected function createEntityPermissionData(Entity $entity, Role $role) + { + // TODO - Check the permission values and return an EntityPermission } /** diff --git a/database/migrations/2016_04_20_192649_create_entity_permissions_table.php b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php new file mode 100644 index 000000000..359f25df9 --- /dev/null +++ b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php @@ -0,0 +1,34 @@ +increments('id'); + $table->integer('role_id'); + $table->string('entity_type'); + $table->integer('entity_id'); + $table->string('action'); + $table->boolean('has_permission')->default(false); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::drop('entity_permissions'); + } +} From ada7c83e96f35a6b484869d6d27939555e1942f7 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 23 Apr 2016 18:14:26 +0100 Subject: [PATCH 3/8] Continued with database work for permissions overhaul Added to the entity_permissions table with further required fields and indexes. Wrote the code for checking permissions. --- .../Commands/RegeneratePermissions.php | 51 ++++ app/Console/Kernel.php | 1 + app/Entity.php | 20 +- app/Services/RestrictionService.php | 286 +++++++----------- ...192649_create_entity_permissions_table.php | 9 + resources/views/settings/roles/form.blade.php | 1 + 6 files changed, 186 insertions(+), 182 deletions(-) create mode 100644 app/Console/Commands/RegeneratePermissions.php diff --git a/app/Console/Commands/RegeneratePermissions.php b/app/Console/Commands/RegeneratePermissions.php new file mode 100644 index 000000000..bd221c138 --- /dev/null +++ b/app/Console/Commands/RegeneratePermissions.php @@ -0,0 +1,51 @@ +restrictionService = $restrictionService; + parent::__construct(); + } + + /** + * Execute the console command. + * + * @return mixed + */ + public function handle() + { + $this->restrictionService->buildEntityPermissions(); + } +} diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index e3a71bd14..b725c9e21 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -15,6 +15,7 @@ class Kernel extends ConsoleKernel protected $commands = [ \BookStack\Console\Commands\Inspire::class, \BookStack\Console\Commands\ResetViews::class, + \BookStack\Console\Commands\RegeneratePermissions::class, ]; /** diff --git a/app/Entity.php b/app/Entity.php index 4f97c6bab..35badb461 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -73,6 +73,15 @@ abstract class Entity extends Ownable return $this->restrictions->where('role_id', $role_id)->where('action', $action)->count() > 0; } + /** + * Get the entity permissions this is connected to. + * @return \Illuminate\Database\Eloquent\Relations\MorphMany + */ + public function permissions() + { + return $this->morphMany(EntityPermission::class, 'entity'); + } + /** * Allows checking of the exact class, Used to check entity type. * Cleaner method for is_a. @@ -81,7 +90,16 @@ abstract class Entity extends Ownable */ public static function isA($type) { - return static::getClassName() === strtolower($type); + return static::getType() === strtolower($type); + } + + /** + * Get entity type. + * @return mixed + */ + public static function getType() + { + return strtolower(static::getClassName()); } /** diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 8d57b9edc..847db29fe 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -5,7 +5,6 @@ use BookStack\Chapter; use BookStack\Entity; use BookStack\EntityPermission; use BookStack\Page; -use BookStack\Permission; use BookStack\Role; use Illuminate\Database\Eloquent\Collection; @@ -23,18 +22,19 @@ class RestrictionService protected $entityPermission; protected $role; - protected $permission; + + protected $actions = ['view', 'create', 'update', 'delete']; /** * RestrictionService constructor. + * TODO - Handle events when roles or entities change. * @param EntityPermission $entityPermission * @param Book $book * @param Chapter $chapter * @param Page $page * @param Role $role - * @param Permission $permission */ - public function __construct(EntityPermission $entityPermission, Book $book, Chapter $chapter, Page $page, Role $role, Permission $permission) + public function __construct(EntityPermission $entityPermission, Book $book, Chapter $chapter, Page $page, Role $role) { $this->currentUser = auth()->user(); $this->userRoles = $this->currentUser ? $this->currentUser->roles->pluck('id') : []; @@ -42,13 +42,11 @@ class RestrictionService $this->entityPermission = $entityPermission; $this->role = $role; - $this->permission = $permission; $this->book = $book; $this->chapter = $chapter; $this->page = $page; } - /** * Re-generate all entity permission from scratch. */ @@ -65,12 +63,12 @@ class RestrictionService }); // Chunk through all chapters - $this->chapter->chunk(500, function ($books) use ($roles) { + $this->chapter->with('book')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); // Chunk through all pages - $this->page->chunk(500, function ($books) use ($roles) { + $this->page->with('book', 'chapter')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); } @@ -85,16 +83,69 @@ class RestrictionService $entityPermissions = []; foreach ($entities as $entity) { foreach ($roles as $role) { - $entityPermissions[] = $this->createEntityPermission($entity, $role); + foreach ($this->actions as $action) { + $entityPermissions[] = $this->createEntityPermissionData($entity, $role, $action); + } } } $this->entityPermission->insert($entityPermissions); } - protected function createEntityPermissionData(Entity $entity, Role $role) + protected function createEntityPermissionData(Entity $entity, Role $role, $action) { - // TODO - Check the permission values and return an EntityPermission + $permissionPrefix = $entity->getType() . '-' . $action; + $roleHasPermission = $role->hasPermission($permissionPrefix . '-all'); + $roleHasPermissionOwn = $role->hasPermission($permissionPrefix . '-own'); + + if ($entity->isA('book')) { + + if (!$entity->restricted) { + return $this->createEntityPermissionDataArray($entity, $role, $action, $roleHasPermission, $roleHasPermissionOwn); + } else { + $hasAccess = $entity->hasRestriction($role->id, $action); + return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); + } + + } elseif ($entity->isA('chapter')) { + + if (!$entity->restricted) { + $hasAccessToBook = $entity->book->hasRestriction($role->id, $action); + return $this->createEntityPermissionDataArray($entity, $role, $action, + ($roleHasPermission && $hasAccessToBook), ($roleHasPermissionOwn && $hasAccessToBook)); + } else { + $hasAccess = $entity->hasRestriction($role->id, $action); + return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); + } + + } elseif ($entity->isA('page')) { + + if (!$entity->restricted) { + $hasAccessToBook = $entity->book->hasRestriction($role->id, $action); + $hasAccessToChapter = $entity->chapter ? ($entity->chapter->hasRestriction($role->id, $action)) : true; + return $this->createEntityPermissionDataArray($entity, $role, $action, + ($roleHasPermission && $hasAccessToBook && $hasAccessToChapter), + ($roleHasPermissionOwn && $hasAccessToBook && $hasAccessToChapter)); + } else { + $hasAccess = $entity->hasRestriction($role->id, $action); + return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); + } + + } + } + + protected function createEntityPermissionDataArray(Entity $entity, Role $role, $action, $permissionAll, $permissionOwn) + { + $entityClass = get_class($entity); + return [ + 'role_id' => $role->id, + 'entity_id' => $entity->id, + 'entity_type' => $entityClass, + 'action' => $action, + 'has_permission' => $permissionAll, + 'has_permission_own' => $permissionOwn, + 'created_by' => $entity->created_by + ]; } /** @@ -157,86 +208,29 @@ class RestrictionService if ($this->isAdmin) return $query; $this->currentAction = $action; - return $this->pageRestrictionQuery($query); + return $this->entityRestrictionQuery($query); } /** - * The base query for restricting pages. + * The general query filter to remove all entities + * that the current user does not have access to. * @param $query * @return mixed */ - private function pageRestrictionQuery($query) + protected function entityRestrictionQuery($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); + return $query->where(function ($parentQuery) { + $parentQuery->whereHas('permissions', function ($permissionQuery) { + $permissionQuery->whereIn('role_id', $this->userRoles) + ->where('action', '=', $this->currentAction) + ->where(function ($query) { + $query->where('has_permission', '=', true) + ->orWhere(function ($query) { + $query->where('has_permission_own', '=', true) + ->where('created_by', '=', $this->currentUser->id); }); }); - }) - // 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 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'); - }); - }); - }) - // Page has accepted permissions - ->orWhereExists(function ($query) { - $this->checkRestrictionsQuery($query, 'pages', 'Page'); - }); + }); }); } @@ -250,43 +244,7 @@ class RestrictionService { 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'); - }); - }); + return $this->entityRestrictionQuery($query); } /** @@ -299,25 +257,7 @@ class RestrictionService { 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) - ->orWhere(function ($query) { - $query->where('restricted', '=', true)->whereExists(function ($query) { - $this->checkRestrictionsQuery($query, 'books', 'Book'); - }); - }); - }); + return $this->entityRestrictionQuery($query); } /** @@ -333,31 +273,23 @@ class RestrictionService 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); - }); + $query->whereExists(function ($permissionQuery) use (&$tableDetails) { + $permissionQuery->select('id')->from('entity_permissions') + ->whereRaw('entity_permissions.entity_id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) + ->whereRaw('entity_permissions.entity_type=' . $tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn']) + ->where('action', '=', $this->currentAction) + ->whereIn('role_id', $this->userRoles) + ->where(function ($query) { + $query->where('has_permission', '=', true)->orWhere(function ($query) { + $query->where('has_permission_own', '=', true) + ->where('created_by', '=', $this->currentUser->id); + }); }); - })->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); - }); - }); }); }); + } /** @@ -372,32 +304,24 @@ class RestrictionService if ($this->isAdmin) return $query; $this->currentAction = 'view'; $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn]; - return $query->where(function ($query) use (&$tableDetails) { + + return $query->where(function ($query) use ($tableDetails) { $query->where(function ($query) use (&$tableDetails) { - $query->whereExists(function ($query) use (&$tableDetails) { - $query->select('*')->from('pages')->whereRaw('pages.id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) + $query->whereExists(function ($permissionQuery) use (&$tableDetails) { + $permissionQuery->select('id')->from('entity_permissions') + ->whereRaw('entity_permissions.entity_id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) + ->where('entity_type', '=', 'Bookstack\\Page') + ->where('action', '=', $this->currentAction) + ->whereIn('role_id', $this->userRoles) ->where(function ($query) { - $this->pageRestrictionQuery($query); + $query->where('has_permission', '=', true)->orWhere(function ($query) { + $query->where('has_permission_own', '=', true) + ->where('created_by', '=', $this->currentUser->id); + }); }); - })->orWhere($tableDetails['entityIdColumn'], '=', 0); - }); + }); + })->orWhere($tableDetails['entityIdColumn'], '=', 0); }); } - /** - * 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/database/migrations/2016_04_20_192649_create_entity_permissions_table.php b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php index 359f25df9..6b273390b 100644 --- a/database/migrations/2016_04_20_192649_create_entity_permissions_table.php +++ b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php @@ -19,7 +19,16 @@ class CreateEntityPermissionsTable extends Migration $table->integer('entity_id'); $table->string('action'); $table->boolean('has_permission')->default(false); + $table->boolean('has_permission_own')->default(false); + $table->integer('created_by'); + $table->index(['entity_id', 'entity_type']); + $table->index('role_id'); + $table->index('action'); + $table->index('created_by'); }); + + $restrictionService = app(\BookStack\Services\RestrictionService::class); + $restrictionService->buildEntityPermissions(); } /** diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 0980d1b65..159470477 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -33,6 +33,7 @@ Create + View Edit Delete From a81a56706e8be77586631f3619ad84df36c8d84e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 24 Apr 2016 16:54:20 +0100 Subject: [PATCH 4/8] Rolled out new permissions system throughout application --- app/Entity.php | 15 +- app/Http/Controllers/BookController.php | 14 +- app/Http/Controllers/ChapterController.php | 9 +- app/Http/Controllers/PermissionController.php | 1 + app/Permission.php | 6 +- app/Repos/BookRepo.php | 47 +++- app/Repos/ChapterRepo.php | 15 +- app/Repos/EntityRepo.php | 1 + app/Repos/PageRepo.php | 2 + app/Repos/PermissionsRepo.php | 13 +- app/Role.php | 9 + app/Services/ActivityService.php | 12 +- app/Services/RestrictionService.php | 213 ++++++++++++++---- app/helpers.php | 14 +- ...9_100730_add_view_permissions_to_roles.php | 1 + tests/Permissions/RestrictionsTest.php | 4 + tests/Permissions/RolesTest.php | 12 +- tests/TestCase.php | 2 + 18 files changed, 295 insertions(+), 95 deletions(-) diff --git a/app/Entity.php b/app/Entity.php index 35badb461..c084a2870 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -70,7 +70,20 @@ abstract class Entity extends Ownable */ public function hasRestriction($role_id, $action) { - return $this->restrictions->where('role_id', $role_id)->where('action', $action)->count() > 0; + return $this->restrictions()->where('role_id', '=', $role_id) + ->where('action', '=', $action)->count() > 0; + } + + /** + * Check if this entity has live (active) restrictions in place. + * @param $role_id + * @param $action + * @return bool + */ + public function hasActiveRestriction($role_id, $action) + { + return $this->restricted && $this->restrictions() + ->where('role_id', '=', $role_id)->where('action', '=', $action)->count() > 0; } /** diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 498d6bb7f..356c7508f 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -71,11 +71,7 @@ class BookController extends Controller 'name' => 'required|string|max:255', 'description' => 'string|max:1000' ]); - $book = $this->bookRepo->newFromInput($request->all()); - $book->slug = $this->bookRepo->findSuitableSlug($book->name); - $book->created_by = Auth::user()->id; - $book->updated_by = Auth::user()->id; - $book->save(); + $book = $this->bookRepo->createFromInput($request->all()); Activity::add($book, 'book_create', $book->id); return redirect($book->getUrl()); } @@ -122,10 +118,7 @@ class BookController extends Controller 'name' => 'required|string|max:255', 'description' => 'string|max:1000' ]); - $book->fill($request->all()); - $book->slug = $this->bookRepo->findSuitableSlug($book->name, $book->id); - $book->updated_by = Auth::user()->id; - $book->save(); + $book = $this->bookRepo->updateFromInput($book, $request->all()); Activity::add($book, 'book_update', $book->id); return redirect($book->getUrl()); } @@ -210,6 +203,7 @@ class BookController extends Controller // Add activity for books foreach ($sortedBooks as $bookId) { $updatedBook = $this->bookRepo->getById($bookId); + $this->bookRepo->updateBookPermissions($updatedBook); Activity::add($updatedBook, 'book_sort', $updatedBook->id); } @@ -227,7 +221,7 @@ class BookController extends Controller $this->checkOwnablePermission('book-delete', $book); Activity::addMessage('book_delete', 0, $book->name); Activity::removeEntity($book); - $this->bookRepo->destroyBySlug($bookSlug); + $this->bookRepo->destroy($book); return redirect('/books'); } diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index d1c6c1733..d58be9ba0 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -57,12 +57,9 @@ class ChapterController extends Controller $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); - $chapter->created_by = auth()->user()->id; - $chapter->updated_by = auth()->user()->id; - $book->chapters()->save($chapter); + $input = $request->all(); + $input['priority'] = $this->bookRepo->getNewPriority($book); + $chapter = $this->chapterRepo->createFromInput($request->all(), $book); Activity::add($chapter, 'chapter_create', $book->id); return redirect($chapter->getUrl()); } diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php index c565bb20a..3f2eb7f99 100644 --- a/app/Http/Controllers/PermissionController.php +++ b/app/Http/Controllers/PermissionController.php @@ -2,6 +2,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Repos\PermissionsRepo; +use BookStack\Services\RestrictionService; use Illuminate\Http\Request; use BookStack\Http\Requests; diff --git a/app/Permission.php b/app/Permission.php index a146dcf63..e3f391562 100644 --- a/app/Permission.php +++ b/app/Permission.php @@ -1,6 +1,4 @@ -book->newInstance($input); + $book = $this->book->newInstance($input); + $book->slug = $this->findSuitableSlug($book->name); + $book->created_by = auth()->user()->id; + $book->updated_by = auth()->user()->id; + $book->save(); + $this->restrictionService->buildEntityPermissionsForEntity($book); + return $book; } /** - * Destroy a book identified by the given slug. - * @param $bookSlug + * Update the given book from user input. + * @param Book $book + * @param $input + * @return Book */ - public function destroyBySlug($bookSlug) + public function updateFromInput(Book $book, $input) + { + $book->fill($input); + $book->slug = $this->findSuitableSlug($book->name, $book->id); + $book->updated_by = auth()->user()->id; + $book->save(); + $this->restrictionService->buildEntityPermissionsForEntity($book); + return $book; + } + + /** + * Destroy the given book. + * @param Book $book + * @throws \Exception + */ + public function destroy(Book $book) { - $book = $this->getBySlug($bookSlug); foreach ($book->pages as $page) { $this->pageRepo->destroy($page); } @@ -146,9 +169,19 @@ class BookRepo extends EntityRepo } $book->views()->delete(); $book->restrictions()->delete(); + $this->restrictionService->deleteEntityPermissionsForEntity($book); $book->delete(); } + /** + * Alias method to update the book permissions in the RestrictionService. + * @param Book $book + */ + public function updateBookPermissions(Book $book) + { + $this->restrictionService->buildEntityPermissionsForEntity($book); + } + /** * Get the next child element priority. * @param Book $book diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index 530f550b1..84489c075 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -2,6 +2,7 @@ use Activity; +use BookStack\Book; use BookStack\Exceptions\NotFoundException; use Illuminate\Support\Str; use BookStack\Chapter; @@ -78,11 +79,18 @@ class ChapterRepo extends EntityRepo /** * Create a new chapter from request input. * @param $input - * @return $this + * @param Book $book + * @return Chapter */ - public function newFromInput($input) + public function createFromInput($input, Book $book) { - return $this->chapter->fill($input); + $chapter = $this->chapter->newInstance($input); + $chapter->slug = $this->findSuitableSlug($chapter->name, $book->id); + $chapter->created_by = auth()->user()->id; + $chapter->updated_by = auth()->user()->id; + $chapter = $book->chapters()->save($chapter); + $this->restrictionService->buildEntityPermissionsForEntity($chapter); + return $chapter; } /** @@ -100,6 +108,7 @@ class ChapterRepo extends EntityRepo Activity::removeEntity($chapter); $chapter->views()->delete(); $chapter->restrictions()->delete(); + $this->restrictionService->deleteEntityPermissionsForEntity($chapter); $chapter->delete(); } diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index cb3dd6674..6522e4e9c 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -151,6 +151,7 @@ class EntityRepo } } $entity->save(); + $this->restrictionService->buildEntityPermissionsForEntity($entity); } /** diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index ef470c01d..bfb0e70a7 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -168,6 +168,7 @@ class PageRepo extends EntityRepo if ($chapter) $page->chapter_id = $chapter->id; $book->pages()->save($page); + $this->restrictionService->buildEntityPermissionsForEntity($page); return $page; } @@ -583,6 +584,7 @@ class PageRepo extends EntityRepo $page->views()->delete(); $page->revisions()->delete(); $page->restrictions()->delete(); + $this->restrictionService->deleteEntityPermissionsForEntity($page); $page->delete(); } diff --git a/app/Repos/PermissionsRepo.php b/app/Repos/PermissionsRepo.php index 3c5efde23..ab265a45f 100644 --- a/app/Repos/PermissionsRepo.php +++ b/app/Repos/PermissionsRepo.php @@ -4,6 +4,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Permission; use BookStack\Role; +use BookStack\Services\RestrictionService; use Setting; class PermissionsRepo @@ -11,16 +12,19 @@ class PermissionsRepo protected $permission; protected $role; + protected $restrictionService; /** * PermissionsRepo constructor. - * @param $permission - * @param $role + * @param Permission $permission + * @param Role $role + * @param RestrictionService $restrictionService */ - public function __construct(Permission $permission, Role $role) + public function __construct(Permission $permission, Role $role, RestrictionService $restrictionService) { $this->permission = $permission; $this->role = $role; + $this->restrictionService = $restrictionService; } /** @@ -69,6 +73,7 @@ class PermissionsRepo $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : []; $this->assignRolePermissions($role, $permissions); + $this->restrictionService->buildEntityPermissionForRole($role); return $role; } @@ -91,6 +96,7 @@ class PermissionsRepo $role->fill($roleData); $role->save(); + $this->restrictionService->buildEntityPermissionForRole($role); } /** @@ -136,6 +142,7 @@ class PermissionsRepo } } + $this->restrictionService->deleteEntityPermissionsForRole($role); $role->delete(); } diff --git a/app/Role.php b/app/Role.php index 4e14db181..45d160cfe 100644 --- a/app/Role.php +++ b/app/Role.php @@ -17,6 +17,15 @@ class Role extends Model return $this->belongsToMany('BookStack\User'); } + /** + * Get all related entity permissions. + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function entityPermissions() + { + return $this->hasMany(EntityPermission::class); + } + /** * The permissions that belong to the role. */ diff --git a/app/Services/ActivityService.php b/app/Services/ActivityService.php index d0029b6c4..54e922667 100644 --- a/app/Services/ActivityService.php +++ b/app/Services/ActivityService.php @@ -105,8 +105,16 @@ class ActivityService */ public function entityActivity($entity, $count = 20, $page = 0) { - $activity = $entity->hasMany('BookStack\Activity')->orderBy('created_at', 'desc') - ->skip($count * $page)->take($count)->get(); + if ($entity->isA('book')) { + $query = $this->activity->where('book_id', '=', $entity->id); + } else { + $query = $this->activity->where('entity_type', '=', get_class($entity)) + ->where('entity_id', '=', $entity->id); + } + + $activity = $this->restrictionService + ->filterRestrictedEntityRelations($query, 'activities', 'entity_id', 'entity_type') + ->orderBy('created_at', 'desc')->skip($count * $page)->take($count)->get(); return $this->filterSimilar($activity); } diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 847db29fe..0050401bf 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -23,11 +23,14 @@ class RestrictionService protected $entityPermission; protected $role; + /** + * The actions that have permissions attached throughout the application. + * @var array + */ protected $actions = ['view', 'create', 'update', 'delete']; /** * RestrictionService constructor. - * TODO - Handle events when roles or entities change. * @param EntityPermission $entityPermission * @param Book $book * @param Chapter $chapter @@ -73,6 +76,92 @@ class RestrictionService }); } + /** + * Create the entity permissions for a particular entity. + * @param Entity $entity + */ + public function buildEntityPermissionsForEntity(Entity $entity) + { + $roles = $this->role->load('permissions')->all(); + $entities = collect([$entity]); + + if ($entity->isA('book')) { + $entities = $entities->merge($entity->chapters); + $entities = $entities->merge($entity->pages); + } elseif ($entity->isA('chapter')) { + $entities = $entities->merge($entity->pages); + } + + $this->deleteManyEntityPermissionsForEntities($entities); + $this->createManyEntityPermissions($entities, $roles); + } + + /** + * Build the entity permissions for a particular role. + * @param Role $role + */ + public function buildEntityPermissionForRole(Role $role) + { + $roles = collect([$role]); + + $this->deleteManyEntityPermissionsForRoles($roles); + + // Chunk through all books + $this->book->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + + // Chunk through all chapters + $this->chapter->with('book')->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + + // Chunk through all pages + $this->page->with('book', 'chapter')->chunk(500, function ($books) use ($roles) { + $this->createManyEntityPermissions($books, $roles); + }); + } + + /** + * Delete the entity permissions attached to a particular role. + * @param Role $role + */ + public function deleteEntityPermissionsForRole(Role $role) + { + $this->deleteManyEntityPermissionsForRoles([$role]); + } + + /** + * Delete all of the entity permissions for a list of entities. + * @param Role[] $roles + */ + protected function deleteManyEntityPermissionsForRoles($roles) + { + foreach ($roles as $role) { + $role->entityPermissions()->delete(); + } + } + + /** + * Delete the entity permissions for a particular entity. + * @param Entity $entity + */ + public function deleteEntityPermissionsForEntity(Entity $entity) + { + $this->deleteManyEntityPermissionsForEntities([$entity]); + } + + /** + * Delete all of the entity permissions for a list of entities. + * @param Entity[] $entities + */ + protected function deleteManyEntityPermissionsForEntities($entities) + { + foreach ($entities as $entity) { + $entity->permissions()->delete(); + } + } + /** * Create & Save entity permissions for many entities and permissions. * @param Collection $entities @@ -88,10 +177,18 @@ class RestrictionService } } } + \Log::info(collect($entityPermissions)->where('entity_id', 1)->where('entity_type', 'BookStack\\Page')->where('role_id', 2)->all()); $this->entityPermission->insert($entityPermissions); } - + /** + * Create entity permission data for an entity and role + * for a particular action. + * @param Entity $entity + * @param Role $role + * @param $action + * @return array + */ protected function createEntityPermissionData(Entity $entity, Role $role, $action) { $permissionPrefix = $entity->getType() . '-' . $action; @@ -103,29 +200,39 @@ class RestrictionService if (!$entity->restricted) { return $this->createEntityPermissionDataArray($entity, $role, $action, $roleHasPermission, $roleHasPermissionOwn); } else { - $hasAccess = $entity->hasRestriction($role->id, $action); + $hasAccess = $entity->hasActiveRestriction($role->id, $action); return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); } } elseif ($entity->isA('chapter')) { if (!$entity->restricted) { - $hasAccessToBook = $entity->book->hasRestriction($role->id, $action); + $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $action); + $hasPermissiveAccessToBook = !$entity->book->restricted; return $this->createEntityPermissionDataArray($entity, $role, $action, - ($roleHasPermission && $hasAccessToBook), ($roleHasPermissionOwn && $hasAccessToBook)); + ($hasExplicitAccessToBook || ($roleHasPermission && $hasPermissiveAccessToBook)), + ($hasExplicitAccessToBook || ($roleHasPermissionOwn && $hasPermissiveAccessToBook))); } else { - $hasAccess = $entity->hasRestriction($role->id, $action); + $hasAccess = $entity->hasActiveRestriction($role->id, $action); return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); } } elseif ($entity->isA('page')) { if (!$entity->restricted) { - $hasAccessToBook = $entity->book->hasRestriction($role->id, $action); - $hasAccessToChapter = $entity->chapter ? ($entity->chapter->hasRestriction($role->id, $action)) : true; + $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $action); + $hasPermissiveAccessToBook = !$entity->book->restricted; + $hasExplicitAccessToChapter = $entity->chapter && $entity->chapter->hasActiveRestriction($role->id, $action); + $hasPermissiveAccessToChapter = $entity->chapter && !$entity->chapter->restricted; + $acknowledgeChapter = ($entity->chapter && $entity->chapter->restricted); + + $hasExplicitAccessToParents = $acknowledgeChapter ? $hasExplicitAccessToChapter : $hasExplicitAccessToBook; + $hasPermissiveAccessToParents = $acknowledgeChapter ? $hasPermissiveAccessToChapter : $hasPermissiveAccessToBook; + return $this->createEntityPermissionDataArray($entity, $role, $action, - ($roleHasPermission && $hasAccessToBook && $hasAccessToChapter), - ($roleHasPermissionOwn && $hasAccessToBook && $hasAccessToChapter)); + ($hasExplicitAccessToParents || ($roleHasPermission && $hasPermissiveAccessToParents)), + ($hasExplicitAccessToParents || ($roleHasPermissionOwn && $hasPermissiveAccessToParents)) + ); } else { $hasAccess = $entity->hasRestriction($role->id, $action); return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); @@ -134,6 +241,16 @@ class RestrictionService } } + /** + * Create an array of data with the information of an entity permissions. + * Used to build data for bulk insertion. + * @param Entity $entity + * @param Role $role + * @param $action + * @param $permissionAll + * @param $permissionOwn + * @return array + */ protected function createEntityPermissionDataArray(Entity $entity, Role $role, $action, $permissionAll, $permissionOwn) { $entityClass = get_class($entity); @@ -151,22 +268,30 @@ class RestrictionService /** * Checks if an entity has a restriction set upon it. * @param Entity $entity - * @param $action + * @param $permission * @return bool */ - public function checkIfEntityRestricted(Entity $entity, $action) + public function checkEntityUserAccess(Entity $entity, $permission) { if ($this->isAdmin) return true; - $this->currentAction = $action; + $explodedPermission = explode('-', $permission); + $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; + + $nonEntityPermissions = ['restrictions']; + + // Handle non entity specific permissions + if (in_array($explodedPermission[0], $nonEntityPermissions)) { + $allPermission = $this->currentUser && $this->currentUser->can($permission . '-all'); + $ownPermission = $this->currentUser && $this->currentUser->can($permission . '-own'); + $this->currentAction = 'view'; + $isOwner = $this->currentUser && $this->currentUser->id === $entity->created_by; + return ($allPermission || ($isOwner && $ownPermission)); } - return false; + + $action = end($explodedPermission); + $this->currentAction = $action; + return $this->entityRestrictionQuery($baseQuery)->count() > 0; } /** @@ -188,29 +313,6 @@ class RestrictionService } } - /** - * Add restrictions for a page query - * @param $query - * @param string $action - * @return mixed - */ - public function enforcePageRestrictions($query, $action = 'view') - { - // Prevent drafts being visible to others. - $query = $query->where(function ($query) { - $query->where('draft', '=', false); - if ($this->currentUser) { - $query->orWhere(function ($query) { - $query->where('draft', '=', true)->where('created_by', '=', $this->currentUser->id); - }); - } - }); - - if ($this->isAdmin) return $query; - $this->currentAction = $action; - return $this->entityRestrictionQuery($query); - } - /** * The general query filter to remove all entities * that the current user does not have access to. @@ -234,6 +336,29 @@ class RestrictionService }); } + /** + * Add restrictions for a page query + * @param $query + * @param string $action + * @return mixed + */ + public function enforcePageRestrictions($query, $action = 'view') + { + // Prevent drafts being visible to others. + $query = $query->where(function ($query) { + $query->where('draft', '=', false); + if ($this->currentUser) { + $query->orWhere(function ($query) { + $query->where('draft', '=', true)->where('created_by', '=', $this->currentUser->id); + }); + } + }); + + if ($this->isAdmin) return $query; + $this->currentAction = $action; + return $this->entityRestrictionQuery($query); + } + /** * Add on permission restrictions to a chapter query. * @param $query @@ -316,7 +441,7 @@ class RestrictionService ->where(function ($query) { $query->where('has_permission', '=', true)->orWhere(function ($query) { $query->where('has_permission_own', '=', true) - ->where('created_by', '=', $this->currentUser->id); + ->where('created_by', '=', $this->currentUser ? $this->currentUser->id : 0); }); }); }); diff --git a/app/helpers.php b/app/helpers.php index eab8ca1c8..582e4e03a 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -45,20 +45,8 @@ function userCan($permission, \BookStack\Ownable $ownable = null) } // Check permission on ownable item - $permissionBaseName = strtolower($permission) . '-'; - $hasPermission = false; - if (auth()->user()->can($permissionBaseName . 'all')) $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; - - // Check restrictions on the entity $restrictionService = app('BookStack\Services\RestrictionService'); - $explodedPermission = explode('-', $permission); - $action = end($explodedPermission); - $hasAccess = $restrictionService->checkIfEntityRestricted($ownable, $action); - $restrictionsSet = $restrictionService->checkIfRestrictionsSet($ownable, $action); - return ($hasAccess && $restrictionsSet) || (!$restrictionsSet && $hasPermission); + return $restrictionService->checkEntityUserAccess($ownable, $permission); } /** diff --git a/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php b/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php index dabd6a25e..b97a3d09b 100644 --- a/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php +++ b/database/migrations/2016_04_09_100730_add_view_permissions_to_roles.php @@ -23,6 +23,7 @@ class AddViewPermissionsToRoles extends Migration $newPermission->name = strtolower($entity) . '-' . strtolower(str_replace(' ', '-', $op)); $newPermission->display_name = $op . ' ' . $entity . 's'; $newPermission->save(); + // Assign view permissions to all current roles foreach ($currentRoles as $role) { $role->attachPermission($newPermission); } diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 4ecf5fb20..0aa1389a6 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -4,12 +4,14 @@ class RestrictionsTest extends TestCase { protected $user; protected $viewer; + protected $restrictionService; public function setUp() { parent::setUp(); $this->user = $this->getNewUser(); $this->viewer = $this->getViewer(); + $this->restrictionService = $this->app[\BookStack\Services\RestrictionService::class]; } protected function getViewer() @@ -43,6 +45,8 @@ class RestrictionsTest extends TestCase } $entity->save(); $entity->load('restrictions'); + $this->restrictionService->buildEntityPermissionsForEntity($entity); + $entity->load('permissions'); } public function test_book_view_restriction() diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 8ecdb37a3..84169c90b 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -7,7 +7,15 @@ class RolesTest extends TestCase public function setUp() { parent::setUp(); - $this->user = $this->getNewBlankUser(); + $this->user = $this->getViewer(); + } + + protected function getViewer() + { + $role = \BookStack\Role::getRole('viewer'); + $viewer = $this->getNewBlankUser(); + $viewer->attachRole($role);; + return $viewer; } /** @@ -141,7 +149,7 @@ class RolesTest extends TestCase public function test_restrictions_manage_own_permission() { - $otherUsersPage = \BookStack\Page::take(1)->get()->first(); + $otherUsersPage = \BookStack\Page::first(); $content = $this->createEntityChainBelongingToUser($this->user); // Check can't restrict other's content $this->actingAs($this->user)->visit($otherUsersPage->getUrl()) diff --git a/tests/TestCase.php b/tests/TestCase.php index f46d73e04..1b6a69c62 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -65,6 +65,8 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase $page = factory(BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]); $book->chapters()->saveMany([$chapter]); $chapter->pages()->saveMany([$page]); + $restrictionService = $this->app[\BookStack\Services\RestrictionService::class]; + $restrictionService->buildEntityPermissionsForEntity($book); return [ 'book' => $book, 'chapter' => $chapter, From 9a31b83b2aaa2e5c9e4416cf378bcca74a397bef Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 26 Apr 2016 21:48:17 +0100 Subject: [PATCH 5/8] Worked around create permission quirks --- app/Http/Controllers/PageController.php | 2 +- app/Services/RestrictionService.php | 57 +++++++++++++++++-------- database/seeds/DummyContentSeeder.php | 5 ++- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index d2cb647b7..28247185f 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -69,7 +69,7 @@ class PageController extends Controller { $book = $this->bookRepo->getBySlug($bookSlug); $draft = $this->pageRepo->getById($pageId, true); - $this->checkOwnablePermission('page-create', $draft); + $this->checkOwnablePermission('page-create', $book); $this->setPageTitle('Edit Page Draft'); return view('pages/create', ['draft' => $draft, 'book' => $book]); diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 0050401bf..d3394fcd7 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -6,6 +6,7 @@ use BookStack\Entity; use BookStack\EntityPermission; use BookStack\Page; use BookStack\Role; +use BookStack\User; use Illuminate\Database\Eloquent\Collection; class RestrictionService @@ -23,12 +24,6 @@ class RestrictionService protected $entityPermission; protected $role; - /** - * The actions that have permissions attached throughout the application. - * @var array - */ - protected $actions = ['view', 'create', 'update', 'delete']; - /** * RestrictionService constructor. * @param EntityPermission $entityPermission @@ -40,6 +35,7 @@ class RestrictionService public function __construct(EntityPermission $entityPermission, Book $book, Chapter $chapter, Page $page, Role $role) { $this->currentUser = auth()->user(); + if ($this->currentUser === null) $this->currentUser = new User(['id' => 0]); $this->userRoles = $this->currentUser ? $this->currentUser->roles->pluck('id') : []; $this->isAdmin = $this->currentUser ? $this->currentUser->hasRole('admin') : false; @@ -172,15 +168,34 @@ class RestrictionService $entityPermissions = []; foreach ($entities as $entity) { foreach ($roles as $role) { - foreach ($this->actions as $action) { + foreach ($this->getActions($entity) as $action) { $entityPermissions[] = $this->createEntityPermissionData($entity, $role, $action); } } } - \Log::info(collect($entityPermissions)->where('entity_id', 1)->where('entity_type', 'BookStack\\Page')->where('role_id', 2)->all()); $this->entityPermission->insert($entityPermissions); } + + /** + * Get the actions related to an entity. + * @param $entity + * @return array + */ + protected function getActions($entity) + { + $baseActions = ['view', 'update', 'delete']; + + if ($entity->isA('chapter')) { + $baseActions[] = 'page-create'; + } else if ($entity->isA('book')) { + $baseActions[] = 'page-create'; + $baseActions[] = 'chapter-create'; + } + + return $baseActions; + } + /** * Create entity permission data for an entity and role * for a particular action. @@ -191,38 +206,40 @@ class RestrictionService */ protected function createEntityPermissionData(Entity $entity, Role $role, $action) { - $permissionPrefix = $entity->getType() . '-' . $action; + $permissionPrefix = (strpos($action, '-') === false ? ($entity->getType() . '-') : '') . $action; $roleHasPermission = $role->hasPermission($permissionPrefix . '-all'); $roleHasPermissionOwn = $role->hasPermission($permissionPrefix . '-own'); + $explodedAction = explode('-', $action); + $restrictionAction = end($explodedAction); if ($entity->isA('book')) { if (!$entity->restricted) { return $this->createEntityPermissionDataArray($entity, $role, $action, $roleHasPermission, $roleHasPermissionOwn); } else { - $hasAccess = $entity->hasActiveRestriction($role->id, $action); + $hasAccess = $entity->hasActiveRestriction($role->id, $restrictionAction); return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); } } elseif ($entity->isA('chapter')) { if (!$entity->restricted) { - $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $action); + $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction); $hasPermissiveAccessToBook = !$entity->book->restricted; return $this->createEntityPermissionDataArray($entity, $role, $action, ($hasExplicitAccessToBook || ($roleHasPermission && $hasPermissiveAccessToBook)), ($hasExplicitAccessToBook || ($roleHasPermissionOwn && $hasPermissiveAccessToBook))); } else { - $hasAccess = $entity->hasActiveRestriction($role->id, $action); + $hasAccess = $entity->hasActiveRestriction($role->id, $restrictionAction); return $this->createEntityPermissionDataArray($entity, $role, $action, $hasAccess, $hasAccess); } } elseif ($entity->isA('page')) { if (!$entity->restricted) { - $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $action); + $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction); $hasPermissiveAccessToBook = !$entity->book->restricted; - $hasExplicitAccessToChapter = $entity->chapter && $entity->chapter->hasActiveRestriction($role->id, $action); + $hasExplicitAccessToChapter = $entity->chapter && $entity->chapter->hasActiveRestriction($role->id, $restrictionAction); $hasPermissiveAccessToChapter = $entity->chapter && !$entity->chapter->restricted; $acknowledgeChapter = ($entity->chapter && $entity->chapter->restricted); @@ -277,6 +294,8 @@ class RestrictionService $explodedPermission = explode('-', $permission); $baseQuery = $entity->where('id', '=', $entity->id); + $action = end($explodedPermission); + $this->currentAction = $action; $nonEntityPermissions = ['restrictions']; @@ -289,8 +308,12 @@ class RestrictionService return ($allPermission || ($isOwner && $ownPermission)); } - $action = end($explodedPermission); - $this->currentAction = $action; + // Handle abnormal create permissions + if ($action === 'create') { + $this->currentAction = $permission; + } + + return $this->entityRestrictionQuery($baseQuery)->count() > 0; } @@ -441,7 +464,7 @@ class RestrictionService ->where(function ($query) { $query->where('has_permission', '=', true)->orWhere(function ($query) { $query->where('has_permission_own', '=', true) - ->where('created_by', '=', $this->currentUser ? $this->currentUser->id : 0); + ->where('created_by', '=', $this->currentUser->id); }); }); }); diff --git a/database/seeds/DummyContentSeeder.php b/database/seeds/DummyContentSeeder.php index 328971f26..f7ddd95c4 100644 --- a/database/seeds/DummyContentSeeder.php +++ b/database/seeds/DummyContentSeeder.php @@ -20,12 +20,15 @@ class DummyContentSeeder extends Seeder ->each(function($book) use ($user) { $chapters = factory(BookStack\Chapter::class, 5)->create(['created_by' => $user->id, 'updated_by' => $user->id]) ->each(function($chapter) use ($user, $book){ - $pages = factory(\BookStack\Page::class, 10)->make(['created_by' => $user->id, 'updated_by' => $user->id, 'book_id' => $book->id]); + $pages = factory(\BookStack\Page::class, 5)->make(['created_by' => $user->id, 'updated_by' => $user->id, 'book_id' => $book->id]); $chapter->pages()->saveMany($pages); }); $pages = factory(\BookStack\Page::class, 3)->make(['created_by' => $user->id, 'updated_by' => $user->id]); $book->chapters()->saveMany($chapters); $book->pages()->saveMany($pages); }); + + $restrictionService = app(\BookStack\Services\RestrictionService::class); + $restrictionService->buildEntityPermissions(); } } From 59367b34178739c3aedb4d21e863f3cc58401a2d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 30 Apr 2016 17:16:06 +0100 Subject: [PATCH 6/8] Improved permission regen performance by factor of 4 Worked around slower eloquent access to speed up permission generation. --- app/Activity.php | 2 -- app/EmailConfirmation.php | 2 -- app/Entity.php | 3 +-- app/EntityPermission.php | 6 +----- app/Model.php | 19 +++++++++++++++++++ app/Ownable.php | 1 - app/Page.php | 5 +---- app/PageRevision.php | 1 - app/Permission.php | 1 - app/Restriction.php | 5 +---- app/Role.php | 16 +++++++++------- app/Services/RestrictionService.php | 26 +++++++++++++------------- app/Setting.php | 6 +----- app/SocialAccount.php | 5 +---- app/User.php | 5 +---- app/View.php | 6 +----- 16 files changed, 49 insertions(+), 60 deletions(-) create mode 100644 app/Model.php diff --git a/app/Activity.php b/app/Activity.php index ac7c1d749..1fd00abea 100644 --- a/app/Activity.php +++ b/app/Activity.php @@ -2,8 +2,6 @@ namespace BookStack; -use Illuminate\Database\Eloquent\Model; - /** * @property string key * @property \User user diff --git a/app/EmailConfirmation.php b/app/EmailConfirmation.php index 46912e733..974cf201c 100644 --- a/app/EmailConfirmation.php +++ b/app/EmailConfirmation.php @@ -2,8 +2,6 @@ namespace BookStack; -use Illuminate\Database\Eloquent\Model; - class EmailConfirmation extends Model { protected $fillable = ['user_id', 'token']; diff --git a/app/Entity.php b/app/Entity.php index c084a2870..eb14780fe 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -82,8 +82,7 @@ abstract class Entity extends Ownable */ public function hasActiveRestriction($role_id, $action) { - return $this->restricted && $this->restrictions() - ->where('role_id', '=', $role_id)->where('action', '=', $action)->count() > 0; + return $this->getRawAttribute('restricted') && $this->hasRestriction($role_id, $action); } /** diff --git a/app/EntityPermission.php b/app/EntityPermission.php index 6b4ddd212..266930d2c 100644 --- a/app/EntityPermission.php +++ b/app/EntityPermission.php @@ -1,8 +1,4 @@ -permissions->pluck('name')->contains($permission); + $permissions = $this->getRelationValue('permissions'); + foreach ($permissions as $permission) { + if ($permission->getRawAttribute('name') === $permissionName) return true; + } + return false; } /** diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index d3394fcd7..40287bf77 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -54,21 +54,21 @@ class RestrictionService $this->entityPermission->truncate(); // Get all roles (Should be the most limited dimension) - $roles = $this->role->load('permissions')->all(); + $roles = $this->role->with('permissions')->get(); // Chunk through all books - $this->book->chunk(500, function ($books) use ($roles) { + $this->book->with('restrictions')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); // Chunk through all chapters - $this->chapter->with('book')->chunk(500, function ($books) use ($roles) { - $this->createManyEntityPermissions($books, $roles); + $this->chapter->with('book', 'restrictions')->chunk(500, function ($chapters) use ($roles) { + $this->createManyEntityPermissions($chapters, $roles); }); // Chunk through all pages - $this->page->with('book', 'chapter')->chunk(500, function ($books) use ($roles) { - $this->createManyEntityPermissions($books, $roles); + $this->page->with('book', 'chapter', 'restrictions')->chunk(500, function ($pages) use ($roles) { + $this->createManyEntityPermissions($pages, $roles); }); } @@ -78,7 +78,7 @@ class RestrictionService */ public function buildEntityPermissionsForEntity(Entity $entity) { - $roles = $this->role->load('permissions')->all(); + $roles = $this->role->with('permissions')->get(); $entities = collect([$entity]); if ($entity->isA('book')) { @@ -103,17 +103,17 @@ class RestrictionService $this->deleteManyEntityPermissionsForRoles($roles); // Chunk through all books - $this->book->chunk(500, function ($books) use ($roles) { + $this->book->with('restrictions')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); // Chunk through all chapters - $this->chapter->with('book')->chunk(500, function ($books) use ($roles) { + $this->chapter->with('book', 'restrictions')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); // Chunk through all pages - $this->page->with('book', 'chapter')->chunk(500, function ($books) use ($roles) { + $this->page->with('book', 'chapter', 'restrictions')->chunk(500, function ($books) use ($roles) { $this->createManyEntityPermissions($books, $roles); }); } @@ -272,13 +272,13 @@ class RestrictionService { $entityClass = get_class($entity); return [ - 'role_id' => $role->id, - 'entity_id' => $entity->id, + 'role_id' => $role->getRawAttribute('id'), + 'entity_id' => $entity->getRawAttribute('id'), 'entity_type' => $entityClass, 'action' => $action, 'has_permission' => $permissionAll, 'has_permission_own' => $permissionOwn, - 'created_by' => $entity->created_by + 'created_by' => $entity->getRawAttribute('created_by') ]; } diff --git a/app/Setting.php b/app/Setting.php index 05bd2c226..0af3652db 100644 --- a/app/Setting.php +++ b/app/Setting.php @@ -1,8 +1,4 @@ - Date: Sun, 1 May 2016 19:36:53 +0100 Subject: [PATCH 7/8] Added hidden public role to fit with new permissions system --- app/Http/Controllers/PermissionController.php | 2 + app/Http/Controllers/UserController.php | 6 ++- app/Repos/PermissionsRepo.php | 14 ++++-- app/Repos/UserRepo.php | 11 ++++- app/Role.php | 20 ++++++++ app/Services/RestrictionService.php | 35 ++++++++++--- app/helpers.php | 1 - ...192649_create_entity_permissions_table.php | 49 +++++++++++++++++++ resources/views/chapters/show.blade.php | 12 +++-- resources/views/settings/index.blade.php | 4 +- resources/views/settings/roles/form.blade.php | 11 +++-- resources/views/users/forms/ldap.blade.php | 2 +- .../views/users/forms/standard.blade.php | 2 +- tests/Permissions/RolesTest.php | 23 +++++++++ 14 files changed, 166 insertions(+), 26 deletions(-) diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php index 3f2eb7f99..22d0cfe0e 100644 --- a/app/Http/Controllers/PermissionController.php +++ b/app/Http/Controllers/PermissionController.php @@ -63,11 +63,13 @@ class PermissionController extends Controller * Show the form for editing a user role. * @param $id * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View + * @throws PermissionsException */ public function editRole($id) { $this->checkPermission('user-roles-manage'); $role = $this->permissionsRepo->getRoleById($id); + if ($role->hidden) throw new PermissionsException('This role cannot be edited'); return view('settings/roles/edit', ['role' => $role]); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index d59931640..6956b8d18 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -49,7 +49,8 @@ class UserController extends Controller { $this->checkPermission('users-manage'); $authMethod = config('auth.method'); - return view('users/create', ['authMethod' => $authMethod]); + $roles = $this->userRepo->getAssignableRoles(); + return view('users/create', ['authMethod' => $authMethod, 'roles' => $roles]); } /** @@ -117,7 +118,8 @@ class UserController extends Controller $user = $this->user->findOrFail($id); $activeSocialDrivers = $socialAuthService->getActiveDrivers(); $this->setPageTitle('User Profile'); - return view('users/edit', ['user' => $user, 'activeSocialDrivers' => $activeSocialDrivers, 'authMethod' => $authMethod]); + $roles = $this->userRepo->getAssignableRoles(); + return view('users/edit', ['user' => $user, 'activeSocialDrivers' => $activeSocialDrivers, 'authMethod' => $authMethod, 'roles' => $roles]); } /** diff --git a/app/Repos/PermissionsRepo.php b/app/Repos/PermissionsRepo.php index ab265a45f..8bdcc8382 100644 --- a/app/Repos/PermissionsRepo.php +++ b/app/Repos/PermissionsRepo.php @@ -14,6 +14,8 @@ class PermissionsRepo protected $role; protected $restrictionService; + protected $systemRoles = ['admin', 'public']; + /** * PermissionsRepo constructor. * @param Permission $permission @@ -33,7 +35,7 @@ class PermissionsRepo */ public function getAllRoles() { - return $this->role->all(); + return $this->role->where('hidden', '=', false)->get(); } /** @@ -43,7 +45,7 @@ class PermissionsRepo */ public function getAllRolesExcept(Role $role) { - return $this->role->where('id', '!=', $role->id)->get(); + return $this->role->where('id', '!=', $role->id)->where('hidden', '=', false)->get(); } /** @@ -82,10 +84,14 @@ class PermissionsRepo * Ensure Admin role always has all permissions. * @param $roleId * @param $roleData + * @throws PermissionsException */ public function updateRole($roleId, $roleData) { $role = $this->role->findOrFail($roleId); + + if ($role->hidden) throw new PermissionsException("Cannot update a hidden role"); + $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : []; $this->assignRolePermissions($role, $permissions); @@ -128,8 +134,8 @@ class PermissionsRepo $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'); + if ($role->system_name && in_array($role->system_name, $this->systemRoles)) { + throw new PermissionsException('This role is a system role and cannot be deleted'); } else if ($role->id == setting('registration-role')) { throw new PermissionsException('This role cannot be deleted while set as the default registration role.'); } diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 9b5c8d7e7..b4931bdff 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -168,6 +168,15 @@ class UserRepo ]; } + /** + * Get the roles in the system that are assignable to a user. + * @return mixed + */ + public function getAssignableRoles() + { + return $this->role->visible(); + } + /** * Get all the roles which can be given restricted access to * other entities in the system. @@ -175,7 +184,7 @@ class UserRepo */ public function getRestrictableRoles() { - return $this->role->where('name', '!=', 'admin')->get(); + return $this->role->where('hidden', '=', false)->where('system_name', '=', '')->get(); } } \ No newline at end of file diff --git a/app/Role.php b/app/Role.php index 3b930d113..b331e93e4 100644 --- a/app/Role.php +++ b/app/Role.php @@ -72,4 +72,24 @@ class Role extends Model { return static::where('name', '=', $roleName)->first(); } + + /** + * Get the role object for the specified system role. + * @param $roleName + * @return mixed + */ + public static function getSystemRole($roleName) + { + return static::where('system_name', '=', $roleName)->first(); + } + + /** + * GEt all visible roles + * @return mixed + */ + public static function visible() + { + return static::where('hidden', '=', false)->orderBy('name')->get(); + } + } diff --git a/app/Services/RestrictionService.php b/app/Services/RestrictionService.php index 40287bf77..ca5c6c9c1 100644 --- a/app/Services/RestrictionService.php +++ b/app/Services/RestrictionService.php @@ -35,9 +35,10 @@ class RestrictionService public function __construct(EntityPermission $entityPermission, Book $book, Chapter $chapter, Page $page, Role $role) { $this->currentUser = auth()->user(); - if ($this->currentUser === null) $this->currentUser = new User(['id' => 0]); - $this->userRoles = $this->currentUser ? $this->currentUser->roles->pluck('id') : []; - $this->isAdmin = $this->currentUser ? $this->currentUser->hasRole('admin') : false; + $userSet = $this->currentUser !== null; + $this->userRoles = false; + $this->isAdmin = $userSet ? $this->currentUser->hasRole('admin') : false; + if (!$userSet) $this->currentUser = new User(); $this->entityPermission = $entityPermission; $this->role = $role; @@ -46,6 +47,28 @@ class RestrictionService $this->page = $page; } + /** + * Get the roles for the current user; + * @return array|bool + */ + protected function getRoles() + { + if ($this->userRoles !== false) return $this->userRoles; + + $roles = []; + + if (auth()->guest()) { + $roles[] = $this->role->getSystemRole('public')->id; + return $roles; + } + + + foreach ($this->currentUser->roles as $role) { + $roles[] = $role->id; + } + return $roles; + } + /** * Re-generate all entity permission from scratch. */ @@ -346,7 +369,7 @@ class RestrictionService { return $query->where(function ($parentQuery) { $parentQuery->whereHas('permissions', function ($permissionQuery) { - $permissionQuery->whereIn('role_id', $this->userRoles) + $permissionQuery->whereIn('role_id', $this->getRoles()) ->where('action', '=', $this->currentAction) ->where(function ($query) { $query->where('has_permission', '=', true) @@ -428,7 +451,7 @@ class RestrictionService ->whereRaw('entity_permissions.entity_id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->whereRaw('entity_permissions.entity_type=' . $tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn']) ->where('action', '=', $this->currentAction) - ->whereIn('role_id', $this->userRoles) + ->whereIn('role_id', $this->getRoles()) ->where(function ($query) { $query->where('has_permission', '=', true)->orWhere(function ($query) { $query->where('has_permission_own', '=', true) @@ -460,7 +483,7 @@ class RestrictionService ->whereRaw('entity_permissions.entity_id=' . $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->where('entity_type', '=', 'Bookstack\\Page') ->where('action', '=', $this->currentAction) - ->whereIn('role_id', $this->userRoles) + ->whereIn('role_id', $this->getRoles()) ->where(function ($query) { $query->where('has_permission', '=', true)->orWhere(function ($query) { $query->where('has_permission_own', '=', true) diff --git a/app/helpers.php b/app/helpers.php index 582e4e03a..4fa2f2d4d 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -39,7 +39,6 @@ if (!function_exists('versioned_asset')) { */ function userCan($permission, \BookStack\Ownable $ownable = null) { - if (!auth()->check()) return false; if ($ownable === null) { return auth()->user() && auth()->user()->can($permission); } diff --git a/database/migrations/2016_04_20_192649_create_entity_permissions_table.php b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php index 6b273390b..0be507874 100644 --- a/database/migrations/2016_04_20_192649_create_entity_permissions_table.php +++ b/database/migrations/2016_04_20_192649_create_entity_permissions_table.php @@ -21,12 +21,53 @@ class CreateEntityPermissionsTable extends Migration $table->boolean('has_permission')->default(false); $table->boolean('has_permission_own')->default(false); $table->integer('created_by'); + // Create indexes $table->index(['entity_id', 'entity_type']); + $table->index('has_permission'); + $table->index('has_permission_own'); $table->index('role_id'); $table->index('action'); $table->index('created_by'); }); + Schema::table('roles', function (Blueprint $table) { + $table->string('system_name'); + $table->boolean('hidden')->default(false); + $table->index('hidden'); + $table->index('system_name'); + }); + + // Create the new public role + $publicRole = new \BookStack\Role(); + $publicRole->name = 'public'; + $publicRole->display_name = 'Public'; + $publicRole->description = 'The role given to public visitors if allowed'; + $publicRole->system_name = 'public'; + $publicRole->hidden = true; + // Ensure unique name + while (\BookStack\Role::getRole($publicRole->name) !== null) { + $publicRole->name = $publicRole->name . str_random(2); + } + $publicRole->save(); + + // Add new view permissions to public role + $entities = ['Book', 'Page', 'Chapter']; + $ops = ['View All', 'View Own']; + foreach ($entities as $entity) { + foreach ($ops as $op) { + $name = strtolower($entity) . '-' . strtolower(str_replace(' ', '-', $op)); + $permission = \BookStack\Permission::getByName($name); + // Assign view permissions to public + $publicRole->attachPermission($permission); + } + } + + // Update admin role with system name + $admin = \BookStack\Role::getRole('admin'); + $admin->system_name = 'admin'; + $admin->save(); + + // Generate the new entity permissions $restrictionService = app(\BookStack\Services\RestrictionService::class); $restrictionService->buildEntityPermissions(); } @@ -39,5 +80,13 @@ class CreateEntityPermissionsTable extends Migration public function down() { Schema::drop('entity_permissions'); + + // Delete the public role + $public = \BookStack\Role::getSystemRole('public'); + $public->delete(); + + Schema::table('roles', function (Blueprint $table) { + $table->dropColumn('system_name'); + }); } } diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index b6b2d5c97..0bb61cebc 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -49,9 +49,15 @@

No pages are currently in this chapter.

- Create a new page -   -or-    - Sort the current book + @if(userCan('page-create', $chapter)) + Create a new page + @endif + @if(userCan('page-create', $chapter) && userCan('book-update', $book)) +   -or-    + @endif + @if(userCan('book-update', $book)) + Sort the current book + @endif

@endif diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index 7e38154d5..4697d3467 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -66,8 +66,8 @@
hasPermission($permission)))) checked="checked" @endif + @if(old('permissions'.$permission, false)|| (!old('display_name', false) && (isset($role) && $role->hasPermission($permission)))) checked="checked" @endif value="true"> \ 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 770123cbd..6181acaea 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -18,7 +18,7 @@ - +
diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 0aa1389a6..75d83cbfc 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -11,7 +11,7 @@ class RestrictionsTest extends TestCase parent::setUp(); $this->user = $this->getNewUser(); $this->viewer = $this->getViewer(); - $this->restrictionService = $this->app[\BookStack\Services\RestrictionService::class]; + $this->restrictionService = $this->app[\BookStack\Services\PermissionService::class]; } protected function getViewer() @@ -23,30 +23,30 @@ class RestrictionsTest extends TestCase } /** - * Manually set some restrictions on an entity. + * Manually set some permissions on an entity. * @param \BookStack\Entity $entity * @param $actions */ protected function setEntityRestrictions(\BookStack\Entity $entity, $actions) { $entity->restricted = true; - $entity->restrictions()->delete(); + $entity->permissions()->delete(); $role = $this->user->roles->first(); $viewerRole = $this->viewer->roles->first(); foreach ($actions as $action) { - $entity->restrictions()->create([ + $entity->permissions()->create([ 'role_id' => $role->id, 'action' => strtolower($action) ]); - $entity->restrictions()->create([ + $entity->permissions()->create([ 'role_id' => $viewerRole->id, 'action' => strtolower($action) ]); } $entity->save(); - $entity->load('restrictions'); - $this->restrictionService->buildEntityPermissionsForEntity($entity); $entity->load('permissions'); + $this->restrictionService->buildJointPermissionsForEntity($entity); + $entity->load('jointPermissions'); } public function test_book_view_restriction() @@ -348,7 +348,7 @@ class RestrictionsTest extends TestCase ->check('restrictions[2][view]') ->press('Save Permissions') ->seeInDatabase('books', ['id' => $book->id, 'restricted' => true]) - ->seeInDatabase('restrictions', [ + ->seeInDatabase('entity_permissions', [ 'restrictable_id' => $book->id, 'restrictable_type' => 'BookStack\Book', 'role_id' => '2', @@ -365,7 +365,7 @@ class RestrictionsTest extends TestCase ->check('restrictions[2][update]') ->press('Save Permissions') ->seeInDatabase('chapters', ['id' => $chapter->id, 'restricted' => true]) - ->seeInDatabase('restrictions', [ + ->seeInDatabase('entity_permissions', [ 'restrictable_id' => $chapter->id, 'restrictable_type' => 'BookStack\Chapter', 'role_id' => '2', @@ -382,7 +382,7 @@ class RestrictionsTest extends TestCase ->check('restrictions[2][delete]') ->press('Save Permissions') ->seeInDatabase('pages', ['id' => $page->id, 'restricted' => true]) - ->seeInDatabase('restrictions', [ + ->seeInDatabase('entity_permissions', [ 'restrictable_id' => $page->id, 'restrictable_type' => 'BookStack\Page', 'role_id' => '2', diff --git a/tests/TestCase.php b/tests/TestCase.php index 1b6a69c62..5d0545b66 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -65,8 +65,8 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase $page = factory(BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]); $book->chapters()->saveMany([$chapter]); $chapter->pages()->saveMany([$page]); - $restrictionService = $this->app[\BookStack\Services\RestrictionService::class]; - $restrictionService->buildEntityPermissionsForEntity($book); + $restrictionService = $this->app[\BookStack\Services\PermissionService::class]; + $restrictionService->buildJointPermissionsForEntity($book); return [ 'book' => $book, 'chapter' => $chapter,