diff --git a/app/Auth/Permissions/EntityPermission.php b/app/Auth/Permissions/EntityPermission.php index 131771a38..8af5f480a 100644 --- a/app/Auth/Permissions/EntityPermission.php +++ b/app/Auth/Permissions/EntityPermission.php @@ -3,18 +3,29 @@ namespace BookStack\Auth\Permissions; use BookStack\Model; +use Illuminate\Database\Eloquent\Relations\MorphTo; +/** + * @property int $id + * @property int $role_id + * @property int $entity_id + * @property string $entity_type + * @property boolean $view + * @property boolean $create + * @property boolean $update + * @property boolean $delete + */ class EntityPermission extends Model { - protected $fillable = ['role_id', 'action']; + public const PERMISSIONS = ['view', 'create', 'update', 'delete']; + + protected $fillable = ['role_id', 'view', 'create', 'update', 'delete']; public $timestamps = false; /** - * Get all this restriction's attached entity. - * - * @return \Illuminate\Database\Eloquent\Relations\MorphTo + * Get this restriction's attached entity. */ - public function restrictable() + public function restrictable(): MorphTo { return $this->morphTo('restrictable'); } diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php index f377eef5c..01a623109 100644 --- a/app/Auth/Permissions/JointPermissionBuilder.php +++ b/app/Auth/Permissions/JointPermissionBuilder.php @@ -250,10 +250,13 @@ class JointPermissionBuilder $permissions = $this->getEntityPermissionsForEntities($entities); // Create a mapping of explicit entity permissions + // TODO - Handle new format, Now getting all defined entity permissions + // from the above call, Need to handle entries with none, and the 'Other Roles' (role_id=0) + // fallback option. $permissionMap = []; foreach ($permissions as $permission) { - $key = $permission->restrictable_type . ':' . $permission->restrictable_id . ':' . $permission->role_id; - $isRestricted = $entityRestrictedMap[$permission->restrictable_type . ':' . $permission->restrictable_id]; + $key = $permission->entity_type . ':' . $permission->entity_id . ':' . $permission->role_id; + $isRestricted = $entityRestrictedMap[$permission->entity_type . ':' . $permission->entity_id]; $permissionMap[$key] = $isRestricted; } @@ -319,11 +322,10 @@ class JointPermissionBuilder { $idsByType = $this->entitiesToTypeIdMap($entities); $permissionFetch = EntityPermission::query() - ->where('action', '=', 'view') ->where(function (Builder $query) use ($idsByType) { foreach ($idsByType as $type => $ids) { $query->orWhere(function (Builder $query) use ($type, $ids) { - $query->where('restrictable_type', '=', $type)->whereIn('restrictable_id', $ids); + $query->where('entity_type', '=', $type)->whereIn('entity_id', $ids); }); } }); diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php index d840ccd16..6ddb152a0 100644 --- a/app/Auth/Permissions/PermissionApplicator.php +++ b/app/Auth/Permissions/PermissionApplicator.php @@ -59,6 +59,8 @@ class PermissionApplicator */ protected function hasEntityPermission(Entity $entity, array $userRoleIds, string $action): ?bool { + $this->ensureValidEntityAction($action); + $adminRoleId = Role::getSystemRole('admin')->id; if (in_array($adminRoleId, $userRoleIds)) { return true; @@ -81,7 +83,7 @@ class PermissionApplicator if ($currentEntity->restricted) { return $currentEntity->permissions() ->whereIn('role_id', $userRoleIds) - ->where('action', '=', $action) + ->where($action, '=', true) ->count() > 0; } } @@ -95,18 +97,16 @@ class PermissionApplicator */ public function checkUserHasEntityPermissionOnAny(string $action, string $entityClass = ''): bool { - if (strpos($action, '-') !== false) { - throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission'); - } + $this->ensureValidEntityAction($action); $permissionQuery = EntityPermission::query() - ->where('action', '=', $action) + ->where($action, '=', true) ->whereIn('role_id', $this->getCurrentUserRoleIds()); if (!empty($entityClass)) { /** @var Entity $entityInstance */ $entityInstance = app()->make($entityClass); - $permissionQuery = $permissionQuery->where('restrictable_type', '=', $entityInstance->getMorphClass()); + $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass()); } $hasPermission = $permissionQuery->count() > 0; @@ -255,4 +255,16 @@ class PermissionApplicator return $this->currentUser()->roles->pluck('id')->values()->all(); } + + /** + * Ensure the given action is a valid and expected entity action. + * Throws an exception if invalid otherwise does nothing. + * @throws InvalidArgumentException + */ + protected function ensureValidEntityAction(string $action): void + { + if (!in_array($action, EntityPermission::PERMISSIONS)) { + throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission'); + } + } } diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 3528eaf2b..a5254875d 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -176,7 +176,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable */ public function permissions(): MorphMany { - return $this->morphMany(EntityPermission::class, 'restrictable'); + return $this->morphMany(EntityPermission::class, 'entity'); } /** @@ -186,7 +186,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable { return $this->permissions() ->where('role_id', '=', $role_id) - ->where('action', '=', $action) + ->where($action, '=', true) ->count() > 0; } diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 1f144b1a8..556ded017 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -139,7 +139,7 @@ class BookshelfRepo */ public function copyDownPermissions(Bookshelf $shelf, $checkUserPermissions = true): int { - $shelfPermissions = $shelf->permissions()->get(['role_id', 'action'])->toArray(); + $shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray(); $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']); $updatedBookCount = 0; diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index 86f392e61..3662b7db3 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -123,7 +123,7 @@ class Cloner public function copyEntityPermissions(Entity $sourceEntity, Entity $targetEntity): void { $targetEntity->restricted = $sourceEntity->restricted; - $permissions = $sourceEntity->permissions()->get(['role_id', 'action'])->toArray(); + $permissions = $sourceEntity->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray(); $targetEntity->permissions()->delete(); $targetEntity->permissions()->createMany($permissions); $targetEntity->rebuildPermissions(); diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php index c771ee4b6..a547cd0a8 100644 --- a/app/Entities/Tools/PermissionsUpdater.php +++ b/app/Entities/Tools/PermissionsUpdater.php @@ -3,6 +3,7 @@ namespace BookStack\Entities\Tools; use BookStack\Actions\ActivityType; +use BookStack\Auth\Permissions\EntityPermission; use BookStack\Auth\User; use BookStack\Entities\Models\Entity; use BookStack\Facades\Activity; @@ -16,11 +17,9 @@ class PermissionsUpdater */ public function updateFromPermissionsForm(Entity $entity, Request $request) { - $restricted = $request->get('restricted') === 'true'; - $permissions = $request->get('restrictions', null); + $permissions = $request->get('permissions', null); $ownerId = $request->get('owned_by', null); - $entity->restricted = $restricted; $entity->permissions()->delete(); if (!is_null($permissions)) { @@ -52,18 +51,20 @@ class PermissionsUpdater } /** - * Format permissions provided from a permission form to be - * EntityPermission data. + * Format permissions provided from a permission form to be EntityPermission data. */ - protected function formatPermissionsFromRequestToEntityPermissions(array $permissions): Collection + protected function formatPermissionsFromRequestToEntityPermissions(array $permissions): array { - return collect($permissions)->flatMap(function ($restrictions, $roleId) { - return collect($restrictions)->keys()->map(function ($action) use ($roleId) { - return [ - 'role_id' => $roleId, - 'action' => strtolower($action), - ]; - }); - }); + $formatted = []; + + foreach ($permissions as $roleId => $info) { + $entityPermissionData = ['role_id' => $roleId]; + foreach (EntityPermission::PERMISSIONS as $permission) { + $entityPermissionData[$permission] = (($info[$permission] ?? false) === "true"); + } + $formatted[] = $entityPermissionData; + } + + return $formatted; } } diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php index f8c1dc1c7..ce8beaec3 100644 --- a/resources/views/form/entity-permissions-row.blade.php +++ b/resources/views/form/entity-permissions-row.blade.php @@ -28,19 +28,20 @@ @endif
+
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting]) + @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting])
@if(!$model instanceof \BookStack\Entities\Models\Page) - @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting]) + @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting]) @endif
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting]) + @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting])
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting]) + @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting])
\ No newline at end of file diff --git a/tests/Helpers/EntityProvider.php b/tests/Helpers/EntityProvider.php index 05925909e..70678a6a5 100644 --- a/tests/Helpers/EntityProvider.php +++ b/tests/Helpers/EntityProvider.php @@ -2,6 +2,7 @@ namespace Tests\Helpers; +use BookStack\Auth\Permissions\EntityPermission; use BookStack\Auth\Role; use BookStack\Auth\User; use BookStack\Entities\Models\Book; @@ -207,13 +208,12 @@ class EntityProvider $entity->permissions()->delete(); $permissions = []; - foreach ($actions as $action) { - foreach ($roles as $role) { - $permissions[] = [ - 'role_id' => $role->id, - 'action' => strtolower($action), - ]; + foreach ($roles as $role) { + $permission = ['role_id' => $role->id]; + foreach (EntityPermission::PERMISSIONS as $possibleAction) { + $permission[$possibleAction] = in_array($possibleAction, $actions); } + $permissions[] = $permission; } $entity->permissions()->createMany($permissions);