From 6e03078de3860cc82e30b14ebbcd192a797fd75f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 9 Apr 2016 12:40:07 +0100 Subject: [PATCH 01/34] 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 02/34] 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 03/34] 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 04/34] 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 05/34] 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 06/34] 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 07/34] 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, From 3385ec3f7859dfe949807dd4c48c07c05b64e355 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 10:35:42 +0100 Subject: [PATCH 09/34] Updated database config to be codeship compatible --- config/database.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/database.php b/config/database.php index e4d6880c7..832852dc2 100644 --- a/config/database.php +++ b/config/database.php @@ -84,8 +84,8 @@ return [ 'driver' => 'mysql', 'host' => 'localhost', 'database' => 'bookstack-test', - 'username' => 'bookstack-test', - 'password' => 'bookstack-test', + 'username' => env('MYSQL_USER', 'bookstack-test'), + 'password' => env('MYSQL_PASSWORD', 'bookstack-test'), 'charset' => 'utf8', 'collation' => 'utf8_unicode_ci', 'prefix' => '', From 5c1015d6fc9231a37b8155c601befb2c9ae1104d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 11:26:47 +0100 Subject: [PATCH 10/34] Updated social testing compatibility --- app/Services/SocialAuthService.php | 3 --- phpunit.xml | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/Services/SocialAuthService.php b/app/Services/SocialAuthService.php index df213609a..ba3479349 100644 --- a/app/Services/SocialAuthService.php +++ b/app/Services/SocialAuthService.php @@ -1,14 +1,11 @@ + From 3a1cda580250ff120f29a051b4a3c61178c6f267 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 11:38:07 +0100 Subject: [PATCH 11/34] Updated ldap so extension not required in testing --- app/Services/Ldap.php | 11 +++++++++++ app/Services/LdapService.php | 2 +- tests/Auth/LdapTest.php | 6 +++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/Services/Ldap.php b/app/Services/Ldap.php index cfefbb4b6..196e46a2f 100644 --- a/app/Services/Ldap.php +++ b/app/Services/Ldap.php @@ -33,6 +33,17 @@ class Ldap return ldap_set_option($ldapConnection, $option, $value); } + /** + * Set the version number for the given ldap connection. + * @param $ldapConnection + * @param $version + * @return bool + */ + public function setVersion($ldapConnection, $version) + { + return $this->setOption($ldapConnection, LDAP_OPT_PROTOCOL_VERSION, $version); + } + /** * Search LDAP tree using the provided filter. * @param resource $ldapConnection diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index 3d89e1e44..b7f101ad2 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -122,7 +122,7 @@ class LdapService // Set any required options if ($this->config['version']) { - $this->ldap->setOption($ldapConnection, LDAP_OPT_PROTOCOL_VERSION, $this->config['version']); + $this->ldap->setVersion($ldapConnection, $this->config['version']); } $this->ldapConnection = $ldapConnection; diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index b52b6ffe1..76fbc662a 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -22,7 +22,7 @@ class LdapTest extends \TestCase public function test_login() { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setOption')->once(); + $this->mockLdap->shouldReceive('setVersion')->once(); $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) ->with($this->resourceId, config('services.ldap.base_dn'), Mockery::type('string'), Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ @@ -49,7 +49,7 @@ class LdapTest extends \TestCase public function test_login_works_when_no_uid_provided_by_ldap_server() { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setOption')->once(); + $this->mockLdap->shouldReceive('setVersion')->once(); $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn'); $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) ->with($this->resourceId, config('services.ldap.base_dn'), Mockery::type('string'), Mockery::type('array')) @@ -73,7 +73,7 @@ class LdapTest extends \TestCase public function test_initial_incorrect_details() { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setOption')->once(); + $this->mockLdap->shouldReceive('setVersion')->once(); $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) ->with($this->resourceId, config('services.ldap.base_dn'), Mockery::type('string'), Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ From 37337b8b1d14c9a0ee5fc6e7eb49035cf9a426aa Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 12:02:23 +0100 Subject: [PATCH 12/34] Added codeship CI badge --- readme.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readme.md b/readme.md index 067983e84..11c99c905 100644 --- a/readme.md +++ b/readme.md @@ -1,5 +1,7 @@ # BookStack +![CI Status](https://codeship.com/projects/395dbfb0-f274-0133-7932-6ec9db8dac2c/status?branch=master) + A platform for storing and organising information and documentation. General information and documentation for BookStack can be found at https://www.bookstackapp.com/. * [Installation Instructions](https://www.bookstackapp.com/docs/admin/installation) From 0d5da2d9d4767f618dc60ff272ac9cc3f240ad2c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 12:26:09 +0100 Subject: [PATCH 13/34] Added travis CI file --- .travis.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000000000..7163e93d5 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,22 @@ +language: php + +php: + - 7.0 + + +addons: + mariadb: '10.1' + +before_script: + - mysql -e 'create database `bookstack-test`;' + - composer config -g github-oauth.github.com $GITHUB_ACCESS_TOKEN + - phpenv config-rm xdebug.ini + - composer self-update + - composer install --prefer-dist --no-interaction + - npm install + - ./node_modules/.bin/gulp + - php artisan migrate --force -n --database=mysql_testing + - php artisan db:seed --force -n --class=DummyContentSeeder --database=mysql_testing + +script: + - vendor/bin/phpunit \ No newline at end of file From 223a6b546cd35e4363e06bfaa21b0af326b52334 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 12:37:58 +0100 Subject: [PATCH 14/34] Added dependancy caching and travis badge --- .travis.yml | 4 ++++ readme.md | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index 7163e93d5..3569fe478 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,10 @@ language: php php: - 7.0 +cache: + directories: + - node_modules + - vendor addons: mariadb: '10.1' diff --git a/readme.md b/readme.md index 067983e84..8a20d52d9 100644 --- a/readme.md +++ b/readme.md @@ -1,5 +1,7 @@ # BookStack +[![Build Status](https://travis-ci.org/ssddanbrown/BookStack.svg)](https://travis-ci.org/ssddanbrown/BookStack) + A platform for storing and organising information and documentation. General information and documentation for BookStack can be found at https://www.bookstackapp.com/. * [Installation Instructions](https://www.bookstackapp.com/docs/admin/installation) From a40af08018018a23816b1c1fc33fd912929565f1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 12:50:17 +0100 Subject: [PATCH 15/34] Changed mariadb version --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3569fe478..8a09a926c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ cache: - vendor addons: - mariadb: '10.1' + mariadb: '10.0' before_script: - mysql -e 'create database `bookstack-test`;' From 6f9d2939e701990bbbb387e7a1159b6a3194b627 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 2 May 2016 13:14:35 +0100 Subject: [PATCH 16/34] Updated travisCI to help fix node error --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 8a09a926c..8c3e1757d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,9 @@ cache: addons: mariadb: '10.0' +before_install: + - npm install -g npm@latest + before_script: - mysql -e 'create database `bookstack-test`;' - composer config -g github-oauth.github.com $GITHUB_ACCESS_TOKEN From 5080b4996e32faed8d62f2625e2d310b432202e6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 6 May 2016 20:33:08 +0100 Subject: [PATCH 17/34] Started base work on attribute system --- app/Attribute.php | 19 ++++++++++ app/Entity.php | 25 ++++++++++++ app/Http/Controllers/AttributeController.php | 32 ++++++++++++++++ app/Http/routes.php | 7 +++- app/Repos/AttributeRepo.php | 32 ++++++++++++++++ app/Services/PermissionService.php | 19 +++++++--- ...6_05_06_185215_create_attributes_table.php | 38 +++++++++++++++++++ 7 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 app/Attribute.php create mode 100644 app/Http/Controllers/AttributeController.php create mode 100644 app/Repos/AttributeRepo.php create mode 100644 database/migrations/2016_05_06_185215_create_attributes_table.php diff --git a/app/Attribute.php b/app/Attribute.php new file mode 100644 index 000000000..62dc62be7 --- /dev/null +++ b/app/Attribute.php @@ -0,0 +1,19 @@ +morphTo('entity'); + } +} \ No newline at end of file diff --git a/app/Entity.php b/app/Entity.php index a0b25eba7..abf3e834e 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -54,6 +54,15 @@ abstract class Entity extends Ownable return $this->morphMany(View::class, 'viewable'); } + /** + * Get the Attribute models that have been user assigned to this entity. + * @return \Illuminate\Database\Eloquent\Relations\MorphMany + */ + public function attributes() + { + return $this->morphMany(Attribute::class, 'entity'); + } + /** * Get this entities restrictions. */ @@ -114,6 +123,22 @@ abstract class Entity extends Ownable return strtolower(static::getClassName()); } + /** + * Get an instance of an entity of the given type. + * @param $type + * @return Entity + */ + public static function getEntityInstance($type) + { + $types = ['Page', 'Book', 'Chapter']; + $className = str_replace([' ', '-', '_'], '', ucwords($type)); + if (!in_array($className, $types)) { + return null; + } + + return app('BookStack\\' . $className); + } + /** * Gets a limited-length version of the entities name. * @param int $length diff --git a/app/Http/Controllers/AttributeController.php b/app/Http/Controllers/AttributeController.php new file mode 100644 index 000000000..09523af47 --- /dev/null +++ b/app/Http/Controllers/AttributeController.php @@ -0,0 +1,32 @@ +attributeRepo = $attributeRepo; + } + + + /** + * Get all the Attributes for a particular entity + * @param $entityType + * @param $entityId + */ + public function getForEntity($entityType, $entityId) + { + + } +} diff --git a/app/Http/routes.php b/app/Http/routes.php index 9565b7576..8b7ec3bc2 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -80,11 +80,16 @@ Route::group(['middleware' => 'auth'], function () { Route::delete('/{imageId}', 'ImageController@destroy'); }); - // Ajax routes + // AJAX routes Route::put('/ajax/page/{id}/save-draft', 'PageController@saveDraft'); Route::get('/ajax/page/{id}', 'PageController@getPageAjax'); Route::delete('/ajax/page/{id}', 'PageController@ajaxDestroy'); + // Attribute routes (AJAX) + Route::group(['prefix' => 'ajax/attributes'], function() { + Route::get('/get/{entityType}/{entityId}', 'AttributeController@getForEntity'); + }); + // Links Route::get('/link/{id}', 'PageController@redirectFromLink'); diff --git a/app/Repos/AttributeRepo.php b/app/Repos/AttributeRepo.php new file mode 100644 index 000000000..d5cf5b0fd --- /dev/null +++ b/app/Repos/AttributeRepo.php @@ -0,0 +1,32 @@ +attribute = $attr; + $this->entity = $ent; + $this->permissionService = $ps; + } + + +} \ No newline at end of file diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php index 2d5ee97a5..17c1b1285 100644 --- a/app/Services/PermissionService.php +++ b/app/Services/PermissionService.php @@ -400,9 +400,7 @@ class PermissionService } }); - if ($this->isAdmin) return $query; - $this->currentAction = $action; - return $this->entityRestrictionQuery($query); + return $this->enforceEntityRestrictions($query, $action); } /** @@ -413,9 +411,7 @@ class PermissionService */ public function enforceChapterRestrictions($query, $action = 'view') { - if ($this->isAdmin) return $query; - $this->currentAction = $action; - return $this->entityRestrictionQuery($query); + return $this->enforceEntityRestrictions($query, $action); } /** @@ -425,6 +421,17 @@ class PermissionService * @return mixed */ public function enforceBookRestrictions($query, $action = 'view') + { + $this->enforceEntityRestrictions($query, $action); + } + + /** + * Add restrictions for a generic entity + * @param $query + * @param string $action + * @return mixed + */ + public function enforceEntityRestrictions($query, $action = 'view') { if ($this->isAdmin) return $query; $this->currentAction = $action; diff --git a/database/migrations/2016_05_06_185215_create_attributes_table.php b/database/migrations/2016_05_06_185215_create_attributes_table.php new file mode 100644 index 000000000..df3e55421 --- /dev/null +++ b/database/migrations/2016_05_06_185215_create_attributes_table.php @@ -0,0 +1,38 @@ +increments('id'); + $table->integer('entity_id'); + $table->string('entity_type', 100); + $table->string('name'); + $table->string('value'); + $table->timestamps(); + + $table->index('name'); + $table->index('value'); + $table->index(['entity_id', 'entity_type']); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::drop('attributes'); + } +} From c99653f0f2799a8adb73ab0ebc761c649debfb44 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 6 May 2016 20:44:07 +0100 Subject: [PATCH 18/34] Fixed bad refactor in the permission service --- app/Services/PermissionService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php index 17c1b1285..218cb30a5 100644 --- a/app/Services/PermissionService.php +++ b/app/Services/PermissionService.php @@ -422,7 +422,7 @@ class PermissionService */ public function enforceBookRestrictions($query, $action = 'view') { - $this->enforceEntityRestrictions($query, $action); + return $this->enforceEntityRestrictions($query, $action); } /** From fcfb9470c96c9bff054dbc28a7dea1d7b87ccb91 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 7 May 2016 14:29:43 +0100 Subject: [PATCH 19/34] Added further attribute endpoints and added tests --- app/Entity.php | 10 +- app/Http/Controllers/AttributeController.php | 41 ++++++- app/Http/Controllers/Controller.php | 11 ++ app/Http/routes.php | 2 + app/Repos/AttributeRepo.php | 71 ++++++++++++ app/Repos/PageRepo.php | 1 + database/factories/ModelFactory.php | 7 ++ tests/Auth/AuthTest.php | 2 +- tests/Entity/AttributeTests.php | 115 +++++++++++++++++++ tests/Entity/EntityTest.php | 14 +-- tests/Entity/PageDraftTest.php | 6 +- tests/Permissions/RestrictionsTest.php | 2 +- tests/TestCase.php | 21 +++- tests/UserProfileTest.php | 6 +- 14 files changed, 282 insertions(+), 27 deletions(-) create mode 100644 tests/Entity/AttributeTests.php diff --git a/app/Entity.php b/app/Entity.php index abf3e834e..79ff9ff38 100644 --- a/app/Entity.php +++ b/app/Entity.php @@ -1,7 +1,7 @@ orderBy('title_relevance', 'desc'); } - - /** - * Get the url for this item. - * @return string - */ - abstract public function getUrl(); - + } diff --git a/app/Http/Controllers/AttributeController.php b/app/Http/Controllers/AttributeController.php index 09523af47..d7282696a 100644 --- a/app/Http/Controllers/AttributeController.php +++ b/app/Http/Controllers/AttributeController.php @@ -2,7 +2,6 @@ use BookStack\Repos\AttributeRepo; use Illuminate\Http\Request; - use BookStack\Http\Requests; class AttributeController extends Controller @@ -19,7 +18,6 @@ class AttributeController extends Controller $this->attributeRepo = $attributeRepo; } - /** * Get all the Attributes for a particular entity * @param $entityType @@ -27,6 +25,43 @@ class AttributeController extends Controller */ public function getForEntity($entityType, $entityId) { - + $attributes = $this->attributeRepo->getForEntity($entityType, $entityId); + return response()->json($attributes); } + + /** + * Update the attributes for a particular entity. + * @param $entityType + * @param $entityId + * @param Request $request + * @return mixed + */ + public function updateForEntity($entityType, $entityId, Request $request) + { + + $this->validate($request, [ + 'attributes.*.name' => 'required|min:3|max:250', + 'attributes.*.value' => 'max:250' + ]); + + $entity = $this->attributeRepo->getEntity($entityType, $entityId, 'update'); + if ($entity === null) return $this->jsonError("Entity not found", 404); + + $inputAttributes = $request->input('attributes'); + $attributes = $this->attributeRepo->saveAttributesToEntity($entity, $inputAttributes); + return response()->json($attributes); + } + + /** + * Get attribute name suggestions from a given search term. + * @param Request $request + */ + public function getNameSuggestions(Request $request) + { + $searchTerm = $request->get('search'); + $suggestions = $this->attributeRepo->getNameSuggestions($searchTerm); + return response()->json($suggestions); + } + + } diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index f0cb47cd9..26eeb3002 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -110,4 +110,15 @@ abstract class Controller extends BaseController return true; } + /** + * Send back a json error message. + * @param string $messageText + * @param int $statusCode + * @return mixed + */ + protected function jsonError($messageText = "", $statusCode = 500) + { + return response()->json(['message' => $messageText], $statusCode); + } + } diff --git a/app/Http/routes.php b/app/Http/routes.php index 8b7ec3bc2..7c6911b2e 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -88,6 +88,8 @@ Route::group(['middleware' => 'auth'], function () { // Attribute routes (AJAX) Route::group(['prefix' => 'ajax/attributes'], function() { Route::get('/get/{entityType}/{entityId}', 'AttributeController@getForEntity'); + Route::get('/suggest', 'AttributeController@getNameSuggestions'); + Route::post('/update/{entityType}/{entityId}', 'AttributeController@updateForEntity'); }); // Links diff --git a/app/Repos/AttributeRepo.php b/app/Repos/AttributeRepo.php index d5cf5b0fd..7318b253b 100644 --- a/app/Repos/AttributeRepo.php +++ b/app/Repos/AttributeRepo.php @@ -28,5 +28,76 @@ class AttributeRepo $this->permissionService = $ps; } + /** + * Get an entity instance of its particular type. + * @param $entityType + * @param $entityId + * @param string $action + */ + public function getEntity($entityType, $entityId, $action = 'view') + { + $entityInstance = $this->entity->getEntityInstance($entityType); + $searchQuery = $entityInstance->where('id', '=', $entityId)->with('attributes'); + $searchQuery = $this->permissionService->enforceEntityRestrictions($searchQuery, $action); + return $searchQuery->first(); + } + + /** + * Get all attributes for a particular entity. + * @param string $entityType + * @param int $entityId + * @return mixed + */ + public function getForEntity($entityType, $entityId) + { + $entity = $this->getEntity($entityType, $entityId); + if ($entity === null) return collect(); + + return $entity->attributes; + } + + /** + * Get attribute name suggestions from scanning existing attribute names. + * @param $searchTerm + * @return array + */ + public function getNameSuggestions($searchTerm) + { + if ($searchTerm === '') return []; + $query = $this->attribute->where('name', 'LIKE', $searchTerm . '%')->groupBy('name')->orderBy('name', 'desc'); + $query = $this->permissionService->filterRestrictedEntityRelations($query, 'attributes', 'entity_id', 'entity_type'); + return $query->get(['name'])->pluck('name'); + } + + /** + * Save an array of attributes to an entity + * @param Entity $entity + * @param array $attributes + * @return array|\Illuminate\Database\Eloquent\Collection + */ + public function saveAttributesToEntity(Entity $entity, $attributes = []) + { + $entity->attributes()->delete(); + $newAttributes = []; + foreach ($attributes as $attribute) { + $newAttributes[] = $this->newInstanceFromInput($attribute); + } + + return $entity->attributes()->saveMany($newAttributes); + } + + /** + * Create a new Attribute instance from user input. + * @param $input + * @return static + */ + protected function newInstanceFromInput($input) + { + $name = trim($input['name']); + $value = isset($input['value']) ? trim($input['value']) : ''; + // Any other modification or cleanup required can go here + $values = ['name' => $name, 'value' => $value]; + return $this->attribute->newInstance($values); + } } \ No newline at end of file diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php index 549ec98a7..ef50b7181 100644 --- a/app/Repos/PageRepo.php +++ b/app/Repos/PageRepo.php @@ -582,6 +582,7 @@ class PageRepo extends EntityRepo { Activity::removeEntity($page); $page->views()->delete(); + $page->attributes()->delete(); $page->revisions()->delete(); $page->permissions()->delete(); $this->permissionService->deleteJointPermissionsForEntity($page); diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index 2840356e8..a1a6a92a0 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -52,4 +52,11 @@ $factory->define(BookStack\Role::class, function ($faker) { 'display_name' => $faker->sentence(3), 'description' => $faker->sentence(10) ]; +}); + +$factory->define(BookStack\Attribute::class, function ($faker) { + return [ + 'name' => $faker->city, + 'value' => $faker->sentence(3) + ]; }); \ No newline at end of file diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index 067840841..306771ed5 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -181,7 +181,7 @@ class AuthTest extends TestCase public function test_user_deletion() { $userDetails = factory(\BookStack\User::class)->make(); - $user = $this->getNewUser($userDetails->toArray()); + $user = $this->getEditor($userDetails->toArray()); $this->asAdmin() ->visit('/settings/users/' . $user->id) diff --git a/tests/Entity/AttributeTests.php b/tests/Entity/AttributeTests.php new file mode 100644 index 000000000..11b66b9e7 --- /dev/null +++ b/tests/Entity/AttributeTests.php @@ -0,0 +1,115 @@ +defaultAttrCount)->make(); + } + + $page->attributes()->saveMany($attributes); + return $page; + } + + public function test_get_page_attributes() + { + $page = $this->getPageWithAttributes(); + + // Add some other attributes to check they don't interfere + factory(Attribute::class, $this->defaultAttrCount)->create(); + + $this->asAdmin()->get("/ajax/attributes/get/page/" . $page->id) + ->shouldReturnJson(); + + $json = json_decode($this->response->getContent()); + $this->assertTrue(count($json) === $this->defaultAttrCount, "Returned JSON item count is not as expected"); + } + + public function test_attribute_name_suggestions() + { + // Create some attributes with similar names to test with + $attrs = collect(); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'country'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'color'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'city'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'county'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'planet'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'plans'])); + $page = $this->getPageWithAttributes($attrs); + + $this->asAdmin()->get('/ajax/attributes/suggest?search=dog')->seeJsonEquals([]); + $this->get('/ajax/attributes/suggest?search=co')->seeJsonEquals(['color', 'country', 'county']); + $this->get('/ajax/attributes/suggest?search=cou')->seeJsonEquals(['country', 'county']); + $this->get('/ajax/attributes/suggest?search=pla')->seeJsonEquals(['planet', 'plans']); + } + + public function test_entity_permissions_effect_attribute_suggestions() + { + $permissionService = $this->app->make(PermissionService::class); + + // Create some attributes with similar names to test with and save to a page + $attrs = collect(); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'country'])); + $attrs = $attrs->merge(factory(Attribute::class, 5)->make(['name' => 'color'])); + $page = $this->getPageWithAttributes($attrs); + + $this->asAdmin()->get('/ajax/attributes/suggest?search=co')->seeJsonEquals(['color', 'country']); + $this->asEditor()->get('/ajax/attributes/suggest?search=co')->seeJsonEquals(['color', 'country']); + + // Set restricted permission the page + $page->restricted = true; + $page->save(); + $permissionService->buildJointPermissionsForEntity($page); + + $this->asAdmin()->get('/ajax/attributes/suggest?search=co')->seeJsonEquals(['color', 'country']); + $this->asEditor()->get('/ajax/attributes/suggest?search=co')->seeJsonEquals([]); + } + + public function test_entity_attribute_updating() + { + $page = $this->getPageWithAttributes(); + + $testJsonData = [ + ['name' => 'color', 'value' => 'red'], + ['name' => 'color', 'value' => ' blue '], + ['name' => 'city', 'value' => 'London '], + ['name' => 'country', 'value' => ' England'], + ]; + $testResponseJsonData = [ + ['name' => 'color', 'value' => 'red'], + ['name' => 'color', 'value' => 'blue'], + ['name' => 'city', 'value' => 'London'], + ['name' => 'country', 'value' => 'England'], + ]; + + $this->asAdmin()->json("POST", "/ajax/attributes/update/page/" . $page->id, ['attributes' => $testJsonData]); + $this->asAdmin()->get("/ajax/attributes/get/page/" . $page->id); + $jsonData = json_decode($this->response->getContent()); + // Check counts + $this->assertTrue(count($jsonData) === count($testJsonData), "The received attribute count is incorrect"); + // Check data is correct + $testDataCorrect = true; + foreach ($jsonData as $data) { + $testItem = ['name' => $data->name, 'value' => $data->value]; + if (!in_array($testItem, $testResponseJsonData)) $testDataCorrect = false; + } + $testMessage = "Expected data was not found in the response.\nExpected Data: %s\nRecieved Data: %s"; + $this->assertTrue($testDataCorrect, sprintf($testMessage, json_encode($testResponseJsonData), json_encode($jsonData))); + } + +} diff --git a/tests/Entity/EntityTest.php b/tests/Entity/EntityTest.php index eebb0dc36..3bf6a3f2a 100644 --- a/tests/Entity/EntityTest.php +++ b/tests/Entity/EntityTest.php @@ -161,8 +161,8 @@ class EntityTest extends TestCase public function test_entities_viewable_after_creator_deletion() { // Create required assets and revisions - $creator = $this->getNewUser(); - $updater = $this->getNewUser(); + $creator = $this->getEditor(); + $updater = $this->getEditor(); $entities = $this->createEntityChainBelongingToUser($creator, $updater); $this->actingAs($creator); app('BookStack\Repos\UserRepo')->destroy($creator); @@ -174,8 +174,8 @@ class EntityTest extends TestCase public function test_entities_viewable_after_updater_deletion() { // Create required assets and revisions - $creator = $this->getNewUser(); - $updater = $this->getNewUser(); + $creator = $this->getEditor(); + $updater = $this->getEditor(); $entities = $this->createEntityChainBelongingToUser($creator, $updater); $this->actingAs($updater); app('BookStack\Repos\UserRepo')->destroy($updater); @@ -198,7 +198,7 @@ class EntityTest extends TestCase public function test_recently_created_pages_view() { - $user = $this->getNewUser(); + $user = $this->getEditor(); $content = $this->createEntityChainBelongingToUser($user); $this->asAdmin()->visit('/pages/recently-created') @@ -207,7 +207,7 @@ class EntityTest extends TestCase public function test_recently_updated_pages_view() { - $user = $this->getNewUser(); + $user = $this->getEditor(); $content = $this->createEntityChainBelongingToUser($user); $this->asAdmin()->visit('/pages/recently-updated') @@ -241,7 +241,7 @@ class EntityTest extends TestCase public function test_recently_created_pages_on_home() { - $entityChain = $this->createEntityChainBelongingToUser($this->getNewUser()); + $entityChain = $this->createEntityChainBelongingToUser($this->getEditor()); $this->asAdmin()->visit('/') ->seeInElement('#recently-created-pages', $entityChain['page']->name); } diff --git a/tests/Entity/PageDraftTest.php b/tests/Entity/PageDraftTest.php index 2c9a28814..108b7459f 100644 --- a/tests/Entity/PageDraftTest.php +++ b/tests/Entity/PageDraftTest.php @@ -32,7 +32,7 @@ class PageDraftTest extends TestCase ->dontSeeInField('html', $addedContent); $newContent = $this->page->html . $addedContent; - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->pageRepo->saveUpdateDraft($this->page, ['html' => $newContent]); $this->actingAs($newUser)->visit($this->page->getUrl() . '/edit') ->dontSeeInField('html', $newContent); @@ -54,7 +54,7 @@ class PageDraftTest extends TestCase ->dontSeeInField('html', $addedContent); $newContent = $this->page->html . $addedContent; - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->pageRepo->saveUpdateDraft($this->page, ['html' => $newContent]); $this->actingAs($newUser) @@ -79,7 +79,7 @@ class PageDraftTest extends TestCase { $book = \BookStack\Book::first(); $chapter = $book->chapters->first(); - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->actingAs($newUser)->visit('/') ->visit($book->getUrl() . '/page/create') diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 75d83cbfc..d3830cff7 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -9,7 +9,7 @@ class RestrictionsTest extends TestCase public function setUp() { parent::setUp(); - $this->user = $this->getNewUser(); + $this->user = $this->getEditor(); $this->viewer = $this->getViewer(); $this->restrictionService = $this->app[\BookStack\Services\PermissionService::class]; } diff --git a/tests/TestCase.php b/tests/TestCase.php index 5d0545b66..4c2893f4e 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -14,7 +14,10 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase * @var string */ protected $baseUrl = 'http://localhost'; + + // Local user instances private $admin; + private $editor; /** * Creates the application. @@ -30,6 +33,10 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase return $app; } + /** + * Set the current user context to be an admin. + * @return $this + */ public function asAdmin() { if($this->admin === null) { @@ -39,6 +46,18 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase return $this->actingAs($this->admin); } + /** + * Set the current editor context to be an editor. + * @return $this + */ + public function asEditor() + { + if($this->editor === null) { + $this->editor = $this->getEditor(); + } + return $this->actingAs($this->editor); + } + /** * Quickly sets an array of settings. * @param $settingsArray @@ -79,7 +98,7 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase * @param array $attributes * @return mixed */ - protected function getNewUser($attributes = []) + protected function getEditor($attributes = []) { $user = factory(\BookStack\User::class)->create($attributes); $role = \BookStack\Role::getRole('editor'); diff --git a/tests/UserProfileTest.php b/tests/UserProfileTest.php index 170e7eed1..40ae004e9 100644 --- a/tests/UserProfileTest.php +++ b/tests/UserProfileTest.php @@ -33,7 +33,7 @@ class UserProfileTest extends TestCase public function test_profile_page_shows_created_content_counts() { - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->asAdmin()->visit('/user/' . $newUser->id) ->see($newUser->name) @@ -52,7 +52,7 @@ class UserProfileTest extends TestCase public function test_profile_page_shows_recent_activity() { - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); Activity::add($entities['book'], 'book_update', $entities['book']->id); @@ -66,7 +66,7 @@ class UserProfileTest extends TestCase public function test_clicking_user_name_in_activity_leads_to_profile_page() { - $newUser = $this->getNewUser(); + $newUser = $this->getEditor(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); Activity::add($entities['book'], 'book_update', $entities['book']->id); From 1fa079b46626b5cbb3d1b055542e4a98b4b0ca0a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 12 May 2016 23:12:05 +0100 Subject: [PATCH 20/34] Started the page attributes interface --- app/Attribute.php | 2 +- app/Http/Controllers/AttributeController.php | 11 +-- app/Http/Controllers/PageController.php | 2 +- app/Repos/AttributeRepo.php | 1 + ...6_05_06_185215_create_attributes_table.php | 2 + resources/assets/js/controllers.js | 94 ++++++++++++++++++- resources/assets/sass/styles.scss | 14 +++ resources/views/pages/create.blade.php | 17 ---- resources/views/pages/edit.blade.php | 17 +++- resources/views/pages/form.blade.php | 1 + tests/Entity/AttributeTests.php | 25 +++-- 11 files changed, 152 insertions(+), 34 deletions(-) delete mode 100644 resources/views/pages/create.blade.php diff --git a/app/Attribute.php b/app/Attribute.php index 62dc62be7..3fe201a42 100644 --- a/app/Attribute.php +++ b/app/Attribute.php @@ -6,7 +6,7 @@ */ class Attribute extends Model { - protected $fillable = ['name', 'value']; + protected $fillable = ['name', 'value', 'order']; /** * Get the entity that this attribute belongs to diff --git a/app/Http/Controllers/AttributeController.php b/app/Http/Controllers/AttributeController.php index d7282696a..6e6913722 100644 --- a/app/Http/Controllers/AttributeController.php +++ b/app/Http/Controllers/AttributeController.php @@ -38,18 +38,15 @@ class AttributeController extends Controller */ public function updateForEntity($entityType, $entityId, Request $request) { - - $this->validate($request, [ - 'attributes.*.name' => 'required|min:3|max:250', - 'attributes.*.value' => 'max:250' - ]); - $entity = $this->attributeRepo->getEntity($entityType, $entityId, 'update'); if ($entity === null) return $this->jsonError("Entity not found", 404); $inputAttributes = $request->input('attributes'); $attributes = $this->attributeRepo->saveAttributesToEntity($entity, $inputAttributes); - return response()->json($attributes); + return response()->json([ + 'attributes' => $attributes, + 'message' => 'Attributes successfully updated' + ]); } /** diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 19e5632a4..da9273743 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -72,7 +72,7 @@ class PageController extends Controller $this->checkOwnablePermission('page-create', $book); $this->setPageTitle('Edit Page Draft'); - return view('pages/create', ['draft' => $draft, 'book' => $book]); + return view('pages/edit', ['page' => $draft, 'book' => $book, 'isDraft' => true]); } /** diff --git a/app/Repos/AttributeRepo.php b/app/Repos/AttributeRepo.php index 7318b253b..fb742b2d8 100644 --- a/app/Repos/AttributeRepo.php +++ b/app/Repos/AttributeRepo.php @@ -80,6 +80,7 @@ class AttributeRepo $entity->attributes()->delete(); $newAttributes = []; foreach ($attributes as $attribute) { + if (trim($attribute['name']) === '') continue; $newAttributes[] = $this->newInstanceFromInput($attribute); } diff --git a/database/migrations/2016_05_06_185215_create_attributes_table.php b/database/migrations/2016_05_06_185215_create_attributes_table.php index df3e55421..b6412037c 100644 --- a/database/migrations/2016_05_06_185215_create_attributes_table.php +++ b/database/migrations/2016_05_06_185215_create_attributes_table.php @@ -18,10 +18,12 @@ class CreateAttributesTable extends Migration $table->string('entity_type', 100); $table->string('name'); $table->string('value'); + $table->integer('order'); $table->timestamps(); $table->index('name'); $table->index('value'); + $table->index('order'); $table->index(['entity_id', 'entity_type']); }); } diff --git a/resources/assets/js/controllers.js b/resources/assets/js/controllers.js index 8b3d952be..e73efaac9 100644 --- a/resources/assets/js/controllers.js +++ b/resources/assets/js/controllers.js @@ -400,4 +400,96 @@ module.exports = function (ngApp, events) { }]); -}; \ No newline at end of file + ngApp.controller('PageAttributeController', ['$scope', '$http', '$attrs', + function ($scope, $http, $attrs) { + + const pageId = Number($attrs.pageId); + $scope.attributes = []; + + /** + * Push an empty attribute to the end of the scope attributes. + */ + function addEmptyAttribute() { + $scope.attributes.push({ + name: '', + value: '' + }); + } + + /** + * Get all attributes for the current book and add into scope. + */ + function getAttributes() { + $http.get('/ajax/attributes/get/page/' + pageId).then((responseData) => { + $scope.attributes = responseData.data; + addEmptyAttribute(); + }); + } + getAttributes(); + + /** + * Set the order property on all attributes. + */ + function setAttributeOrder() { + for (let i = 0; i < $scope.attributes.length; i++) { + $scope.attributes[i].order = i; + } + } + + /** + * When an attribute changes check if another empty editable + * field needs to be added onto the end. + * @param attribute + */ + $scope.attributeChange = function(attribute) { + let cPos = $scope.attributes.indexOf(attribute); + if (cPos !== $scope.attributes.length-1) return; + + if (attribute.name !== '' || attribute.value !== '') { + addEmptyAttribute(); + } + }; + + /** + * When an attribute field loses focus check the attribute to see if its + * empty and therefore could be removed from the list. + * @param attribute + */ + $scope.attributeBlur = function(attribute) { + let isLast = $scope.attributes.length - 1 === $scope.attributes.indexOf(attribute); + if (attribute.name === '' && attribute.value === '' && !isLast) { + let cPos = $scope.attributes.indexOf(attribute); + $scope.attributes.splice(cPos, 1); + } + }; + + $scope.saveAttributes = function() { + setAttributeOrder(); + let postData = {attributes: $scope.attributes}; + $http.post('/ajax/attributes/update/page/' + pageId, postData).then((responseData) => { + $scope.attributes = responseData.data.attributes; + addEmptyAttribute(); + events.emit('success', responseData.data.message); + }) + }; + + }]); + +}; + + + + + + + + + + + + + + + + + diff --git a/resources/assets/sass/styles.scss b/resources/assets/sass/styles.scss index d8453b9ed..4b6b1cc46 100644 --- a/resources/assets/sass/styles.scss +++ b/resources/assets/sass/styles.scss @@ -201,4 +201,18 @@ $btt-size: 40px; background-color: $negative; color: #EEE; } +} + +// Attribute form +.floating-toolbox { + background-color: #FFF; + border: 1px solid #BBB; + border-radius: 3px; + padding: $-l; + position: fixed; + right: $-xl*2; + top: 100px; + z-index: 99; + height: 800px; + overflow-y: scroll; } \ No newline at end of file diff --git a/resources/views/pages/create.blade.php b/resources/views/pages/create.blade.php deleted file mode 100644 index 2c6403e48..000000000 --- a/resources/views/pages/create.blade.php +++ /dev/null @@ -1,17 +0,0 @@ -@extends('base') - -@section('head') - -@stop - -@section('body-class', 'flexbox') - -@section('content') - -
-
- @include('pages/form', ['model' => $draft]) -
-
- @include('partials/image-manager', ['imageType' => 'gallery', 'uploaded_to' => $draft->id]) -@stop \ No newline at end of file diff --git a/resources/views/pages/edit.blade.php b/resources/views/pages/edit.blade.php index 0ad06fc53..a536ad23e 100644 --- a/resources/views/pages/edit.blade.php +++ b/resources/views/pages/edit.blade.php @@ -10,9 +10,24 @@
- + @if(!isset($isDraft)) + + @endif @include('pages/form', ['model' => $page])
+ +
+
+ + + + + +
+ +
+
+
@include('partials/image-manager', ['imageType' => 'gallery', 'uploaded_to' => $page->id]) diff --git a/resources/views/pages/form.blade.php b/resources/views/pages/form.blade.php index 7ce9dbfe5..aa05a9014 100644 --- a/resources/views/pages/form.blade.php +++ b/resources/views/pages/form.blade.php @@ -41,6 +41,7 @@ @include('form/text', ['name' => 'name', 'placeholder' => 'Page Title']) +
@if(setting('app-editor') === 'wysiwyg') +

Registration Settings

+
From 9d3f329bc9b2c3b1f11f7710bb07bfcb56fdac4c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 22 May 2016 14:56:26 +0100 Subject: [PATCH 34/34] Fixed missing column drop on migration rollback --- .../2016_04_20_192649_create_joint_permissions_table.php | 1 + 1 file changed, 1 insertion(+) diff --git a/database/migrations/2016_04_20_192649_create_joint_permissions_table.php b/database/migrations/2016_04_20_192649_create_joint_permissions_table.php index db941f9de..4c1b43c4e 100644 --- a/database/migrations/2016_04_20_192649_create_joint_permissions_table.php +++ b/database/migrations/2016_04_20_192649_create_joint_permissions_table.php @@ -97,6 +97,7 @@ class CreateJointPermissionsTable extends Migration Schema::table('roles', function (Blueprint $table) { $table->dropColumn('system_name'); + $table->dropColumn('hidden'); }); } }