From a81a56706e8be77586631f3619ad84df36c8d84e Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 24 Apr 2016 16:54:20 +0100
Subject: [PATCH] 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 @@
-<?php
-
-namespace BookStack;
+<?php namespace BookStack;
use Illuminate\Database\Eloquent\Model;
@@ -16,7 +14,7 @@ class Permission extends Model
/**
* Get the permission object by name.
- * @param $roleName
+ * @param $name
* @return mixed
*/
public static function getByName($name)
diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php
index 1a56843ae..864b1e240 100644
--- a/app/Repos/BookRepo.php
+++ b/app/Repos/BookRepo.php
@@ -1,5 +1,6 @@
<?php namespace BookStack\Repos;
+use Alpha\B;
use BookStack\Exceptions\NotFoundException;
use Illuminate\Support\Str;
use BookStack\Book;
@@ -123,21 +124,43 @@ class BookRepo extends EntityRepo
/**
* Get a new book instance from request input.
- * @param $input
+ * @param array $input
* @return Book
*/
- public function newFromInput($input)
+ public function createFromInput($input)
{
- return $this->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,