diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php index 40a7f6116..cf95f2854 100644 --- a/app/Auth/Permissions/PermissionApplicator.php +++ b/app/Auth/Permissions/PermissionApplicator.php @@ -11,37 +11,10 @@ use BookStack\Traits\HasCreatorAndUpdater; use BookStack\Traits\HasOwner; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\Builder as QueryBuilder; +use InvalidArgumentException; class PermissionApplicator { - /** - * @var ?array<int> - */ - protected $userRoles = null; - - /** - * @var ?User - */ - protected $currentUserModel = null; - - /** - * Get the roles for the current logged in user. - */ - protected function getCurrentUserRoles(): array - { - if (!is_null($this->userRoles)) { - return $this->userRoles; - } - - if (auth()->guest()) { - $this->userRoles = [Role::getSystemRole('public')->id]; - } else { - $this->userRoles = $this->currentUser()->roles->pluck('id')->values()->all(); - } - - return $this->userRoles; - } - /** * Checks if an entity has a restriction set upon it. * @@ -74,7 +47,6 @@ class PermissionApplicator // TODO - Use a non-query based check $hasAccess = $this->entityRestrictionQuery($baseQuery, $action)->count() > 0; - $this->clean(); return $hasAccess; } @@ -83,25 +55,23 @@ class PermissionApplicator * Checks if a user has the given permission for any items in the system. * Can be passed an entity instance to filter on a specific type. */ - public function checkUserHasPermissionOnAnything(string $permission, ?string $entityClass = null): bool + public function checkUserHasEntityPermissionOnAny(string $action, string $entityClass = ''): bool { - $userRoleIds = $this->currentUser()->roles()->select('id')->pluck('id')->toArray(); - $userId = $this->currentUser()->id; + if (strpos($action, '-') !== false) { + throw new InvalidArgumentException("Action should be a simple entity permission action, not a role permission"); + } - $permissionQuery = JointPermission::query() - ->where('action', '=', $permission) - ->whereIn('role_id', $userRoleIds) - ->where(function (Builder $query) use ($userId) { - $this->addJointHasPermissionCheck($query, $userId); - }); + $permissionQuery = EntityPermission::query() + ->where('action', '=', $action) + ->whereIn('role_id', $this->getCurrentUserRoleIds()); - if (!is_null($entityClass)) { - $entityInstance = app($entityClass); - $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass()); + if (!empty($entityClass)) { + /** @var Entity $entityInstance */ + $entityInstance = app()->make($entityClass); + $permissionQuery = $permissionQuery->where('restrictable_type', '=', $entityInstance->getMorphClass()); } $hasPermission = $permissionQuery->count() > 0; - $this->clean(); return $hasPermission; } @@ -114,7 +84,8 @@ class PermissionApplicator { $q = $query->where(function ($parentQuery) use ($action) { $parentQuery->whereHas('jointPermissions', function ($permissionQuery) use ($action) { - $permissionQuery->whereIn('role_id', $this->getCurrentUserRoles()) + $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds()) + // TODO - Delete line once only views ->where('action', '=', $action) ->where(function (Builder $query) { $this->addJointHasPermissionCheck($query, $this->currentUser()->id); @@ -122,23 +93,20 @@ class PermissionApplicator }); }); - $this->clean(); - return $q; } /** * Limited the given entity query so that the query will only - * return items that the user has permission for the given ability. + * return items that the user has view permission for. */ - public function restrictEntityQuery(Builder $query, string $ability = 'view'): Builder + public function restrictEntityQuery(Builder $query): Builder { - $this->clean(); - - return $query->where(function (Builder $parentQuery) use ($ability) { - $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) use ($ability) { - $permissionQuery->whereIn('role_id', $this->getCurrentUserRoles()) - ->where('action', '=', $ability) + return $query->where(function (Builder $parentQuery) { + $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) { + $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds()) + // TODO - Delete line once only views + ->where('action', '=', 'view') ->where(function (Builder $query) { $this->addJointHasPermissionCheck($query, $this->currentUser()->id); }); @@ -181,18 +149,18 @@ class PermissionApplicator * * @param Builder|QueryBuilder $query */ - public function filterRestrictedEntityRelations($query, string $tableName, string $entityIdColumn, string $entityTypeColumn, string $action = 'view') + public function filterRestrictedEntityRelations($query, string $tableName, string $entityIdColumn, string $entityTypeColumn) { $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn]; $pageMorphClass = (new Page())->getMorphClass(); - $q = $query->whereExists(function ($permissionQuery) use (&$tableDetails, $action) { + $q = $query->whereExists(function ($permissionQuery) use (&$tableDetails) { /** @var Builder $permissionQuery */ $permissionQuery->select(['role_id'])->from('joint_permissions') ->whereColumn('joint_permissions.entity_id', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn']) ->whereColumn('joint_permissions.entity_type', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn']) - ->where('joint_permissions.action', '=', $action) - ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoles()) + ->where('joint_permissions.action', '=', 'view') + ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds()) ->where(function (QueryBuilder $query) { $this->addJointHasPermissionCheck($query, $this->currentUser()->id); }); @@ -207,8 +175,6 @@ class PermissionApplicator }); }); - $this->clean(); - return $q; } @@ -228,7 +194,7 @@ class PermissionApplicator ->whereColumn('joint_permissions.entity_id', '=', $fullEntityIdColumn) ->where('joint_permissions.entity_type', '=', $morphClass) ->where('joint_permissions.action', '=', 'view') - ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoles()) + ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds()) ->where(function (QueryBuilder $query) { $this->addJointHasPermissionCheck($query, $this->currentUser()->id); }); @@ -251,8 +217,6 @@ class PermissionApplicator }); } - $this->clean(); - return $q; } @@ -273,21 +237,22 @@ class PermissionApplicator /** * Get the current user. */ - private function currentUser(): User + protected function currentUser(): User { - if (is_null($this->currentUserModel)) { - $this->currentUserModel = user(); - } - - return $this->currentUserModel; + return user(); } /** - * Clean the cached user elements. + * Get the roles for the current logged-in user. + * + * @return int[] */ - private function clean(): void + protected function getCurrentUserRoleIds(): array { - $this->currentUserModel = null; - $this->userRoles = null; + if (auth()->guest()) { + return [Role::getSystemRole('public')->id]; + } + + return $this->currentUser()->roles->pluck('id')->values()->all(); } } diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 84d31d82d..17f018a56 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -44,7 +44,6 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property Collection $tags * * @method static Entity|Builder visible() - * @method static Entity|Builder hasPermission(string $permission) * @method static Builder withLastView() * @method static Builder withViewCount() */ @@ -69,15 +68,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable */ public function scopeVisible(Builder $query): Builder { - return $this->scopeHasPermission($query, 'view'); - } - - /** - * Scope the query to those entities that the current user has the given permission for. - */ - public function scopeHasPermission(Builder $query, string $permission) - { - return app()->make(PermissionApplicator::class)->restrictEntityQuery($query, $permission); + return app()->make(PermissionApplicator::class)->restrictEntityQuery($query); } /** diff --git a/app/Entities/Queries/Popular.php b/app/Entities/Queries/Popular.php index 66006df1b..82786e3c6 100644 --- a/app/Entities/Queries/Popular.php +++ b/app/Entities/Queries/Popular.php @@ -10,7 +10,7 @@ class Popular extends EntityQuery public function run(int $count, int $page, array $filterModels = null) { $query = $this->permissionService() - ->filterRestrictedEntityRelations(View::query(), 'views', 'viewable_id', 'viewable_type', 'view') + ->filterRestrictedEntityRelations(View::query(), 'views', 'viewable_id', 'viewable_type') ->select('*', 'viewable_id', 'viewable_type', DB::raw('SUM(views) as view_count')) ->groupBy('viewable_id', 'viewable_type') ->orderBy('view_count', 'desc'); diff --git a/app/Entities/Queries/RecentlyViewed.php b/app/Entities/Queries/RecentlyViewed.php index 5a29ecd72..38d1f1be4 100644 --- a/app/Entities/Queries/RecentlyViewed.php +++ b/app/Entities/Queries/RecentlyViewed.php @@ -18,8 +18,7 @@ class RecentlyViewed extends EntityQuery View::query(), 'views', 'viewable_id', - 'viewable_type', - 'view' + 'viewable_type' ) ->orderBy('views.updated_at', 'desc') ->where('user_id', '=', user()->id); diff --git a/app/Entities/Queries/TopFavourites.php b/app/Entities/Queries/TopFavourites.php index 7522a894d..5d138679a 100644 --- a/app/Entities/Queries/TopFavourites.php +++ b/app/Entities/Queries/TopFavourites.php @@ -15,7 +15,7 @@ class TopFavourites extends EntityQuery } $query = $this->permissionService() - ->filterRestrictedEntityRelations(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type', 'view') + ->filterRestrictedEntityRelations(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type') ->select('favourites.*') ->leftJoin('views', function (JoinClause $join) { $join->on('favourites.favouritable_id', '=', 'views.viewable_id'); diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php index a294bf731..18adaa627 100644 --- a/app/Http/Controllers/BookshelfController.php +++ b/app/Http/Controllers/BookshelfController.php @@ -68,7 +68,7 @@ class BookshelfController extends Controller public function create() { $this->checkPermission('bookshelf-create-all'); - $books = Book::hasPermission('update')->get(); + $books = Book::visible()->get(); $this->setPageTitle(trans('entities.shelves_create')); return view('shelves.create', ['books' => $books]); @@ -139,7 +139,7 @@ class BookshelfController extends Controller $this->checkOwnablePermission('bookshelf-update', $shelf); $shelfBookIds = $shelf->books()->get(['id'])->pluck('id'); - $books = Book::hasPermission('update')->whereNotIn('id', $shelfBookIds)->get(); + $books = Book::visible()->whereNotIn('id', $shelfBookIds)->get(); $this->setPageTitle(trans('entities.shelves_edit_named', ['name' => $shelf->getShortName()])); diff --git a/app/helpers.php b/app/helpers.php index cfdf55445..191eddf4d 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -71,14 +71,14 @@ function userCan(string $permission, Model $ownable = null): bool } /** - * Check if the current user has the given permission - * on any item in the system. + * Check if the current user can perform the given action on any items in the system. + * Can be provided the class name of an entity to filter ability to that specific entity type. */ -function userCanOnAny(string $permission, string $entityClass = null): bool +function userCanOnAny(string $action, string $entityClass = ''): bool { $permissions = app(PermissionApplicator::class); - return $permissions->checkUserHasPermissionOnAnything($permission, $entityClass); + return $permissions->checkUserHasEntityPermissionOnAny($action, $entityClass); } /** diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index 3e015616a..8a86900fb 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -120,7 +120,7 @@ <span>{{ trans('common.edit') }}</span> </a> @endif - @if(userCanOnAny('chapter-create')) + @if(userCanOnAny('create', \BookStack\Entities\Models\Book::class) || userCan('chapter-create-all') || userCan('chapter-create-own')) <a href="{{ $chapter->getUrl('/copy') }}" class="icon-list-item"> <span>@icon('copy')</span> <span>{{ trans('common.copy') }}</span> diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 2a71c6021..f1aed730b 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -148,7 +148,7 @@ <span>{{ trans('common.edit') }}</span> </a> @endif - @if(userCanOnAny('page-create')) + @if(userCanOnAny('create', \BookStack\Entities\Models\Book::class) || userCanOnAny('create', \BookStack\Entities\Models\Chapter::class) || userCan('page-create-all') || userCan('page-create-own')) <a href="{{ $page->getUrl('/copy') }}" class="icon-list-item"> <span>@icon('copy')</span> <span>{{ trans('common.copy') }}</span>