Skip to content

Commit c8716df

Browse files
ssddanbrownssddanbrown
committed
Permissions: Removed unused role-perm columns, added permission enum
Updated main permission check methods to support our new enum.
1 parent 1ac7409 commit c8716df

10 files changed

Lines changed: 190 additions & 28 deletions

File tree

‎app/App/helpers.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use BookStack\App\AppVersion;
44
use BookStack\App\Model;
55
use BookStack\Facades\Theme;
6+
use BookStack\Permissions\Permission;
67
use BookStack\Permissions\PermissionApplicator;
78
use BookStack\Settings\SettingService;
89
use BookStack\Users\Models\User;
@@ -39,7 +40,7 @@ function user(): User
3940
* Check if the current user has a permission. If an ownable element
4041
* is passed in the jointPermissions are checked against that particular item.
4142
*/
42-
function userCan(string $permission, ?Model $ownable = null): bool
43+
function userCan(string|Permission $permission, ?Model $ownable = null): bool
4344
{
4445
if (is_null($ownable)) {
4546
return user()->can($permission);

‎app/Entities/Tools/PermissionsUpdater.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use BookStack\Entities\Models\Entity;
99
use BookStack\Facades\Activity;
1010
use BookStack\Permissions\Models\EntityPermission;
11+
use BookStack\Permissions\Permission;
1112
use BookStack\Users\Models\Role;
1213
use BookStack\Users\Models\User;
1314
use Illuminate\Http\Request;
@@ -93,8 +94,9 @@ protected function formatPermissionsFromRequestToEntityPermissions(array $permis
9394

9495
foreach ($permissions as $roleId => $info) {
9596
$entityPermissionData = ['role_id' => $roleId];
96-
foreach (EntityPermission::PERMISSIONS as $permission) {
97-
$entityPermissionData[$permission] = (($info[$permission] ?? false) === "true");
97+
foreach (Permission::genericForEntity() as $permission) {
98+
$permName = $permission->value;
99+
$entityPermissionData[$permName] = (($info[$permName] ?? false) === "true");
98100
}
99101
$formatted[] = $entityPermissionData;
100102
}
@@ -108,8 +110,9 @@ protected function formatPermissionsFromApiRequestToEntityPermissions(array $per
108110

109111
foreach ($permissions as $requestPermissionData) {
110112
$entityPermissionData = ['role_id' => $requestPermissionData['role_id']];
111-
foreach (EntityPermission::PERMISSIONS as $permission) {
112-
$entityPermissionData[$permission] = boolval($requestPermissionData[$permission] ?? false);
113+
foreach (Permission::genericForEntity() as $permission) {
114+
$permName = $permission->value;
115+
$entityPermissionData[$permName] = boolval($requestPermissionData[$permName] ?? false);
113116
}
114117
$formatted[] = $entityPermissionData;
115118
}

‎app/Permissions/Models/EntityPermission.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
*/
1919
class EntityPermission extends Model
2020
{
21-
public const PERMISSIONS = ['view', 'create', 'update', 'delete'];
22-
2321
protected $fillable = ['role_id', 'view', 'create', 'update', 'delete'];
2422
public $timestamps = false;
2523
protected $hidden = ['entity_id', 'entity_type', 'id'];

‎app/Permissions/Models/RolePermission.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
/**
1010
* @property int $id
1111
* @property string $name
12-
* @property string $display_name
1312
*/
1413
class RolePermission extends Model
1514
{

‎app/Permissions/Permission.php‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
<?php
2+
3+
namespace BookStack\Permissions;
4+
5+
/**
6+
* Enum to represent the permissions which may be used in checks.
7+
* These generally align with RolePermission names, although some are abstract or truncated as some checks
8+
* are performed across a range of different items which may be subject to inheritance and other complications.
9+
*
10+
* We use and still allow the string values in usage to allow for compatibility with scenarios where
11+
* users have customised their instance with additional permissions via the theme system.
12+
* This enum primarily exists for alignment within the codebase.
13+
*/
14+
enum Permission: string
15+
{
16+
// Generic Actions
17+
// Used for more abstract entity permission checks
18+
case View = 'view';
19+
case Create = 'create';
20+
case Update = 'update';
21+
case Delete = 'delete';
22+
23+
// System Permissions
24+
case AccessApi = 'access-api';
25+
case ContentExport = 'content-export';
26+
case ContentImport = 'content-import';
27+
case EditorChange = 'editor-change';
28+
case ReceiveNotifications = 'receive-notifications';
29+
case RestrictionsManageAll = 'restrictions-manage-all';
30+
case RestrictionsManageOwn = 'restrictions-manage-own';
31+
case SettingsManage = 'settings-manage';
32+
case TemplatesManage = 'templates-manage';
33+
case UserRolesManage = 'user-roles-manage';
34+
case UsersManage = 'users-manage';
35+
36+
// Entity permissions
37+
// Each has 'all' or 'own' in it's RolePermission, with the base non-suffix name being used
38+
// in actual checking logic, with the permission system handling the assessment of the underlying RolePermission.
39+
case AttachmentCreate = 'attachment-create';
40+
case AttachmentCreateAll = 'attachment-create-all';
41+
case AttachmentCreateOwn = 'attachment-create-own';
42+
43+
case AttachmentDelete = 'attachment-delete';
44+
case AttachmentDeleteAll = 'attachment-delete-all';
45+
case AttachmentDeleteOwn = 'attachment-delete-own';
46+
47+
case AttachmentUpdate = 'attachment-update';
48+
case AttachmentUpdateAll = 'attachment-update-all';
49+
case AttachmentUpdateOwn = 'attachment-update-own';
50+
51+
case BookCreate = 'book-create';
52+
case BookCreateAll = 'book-create-all';
53+
case BookCreateOwn = 'book-create-own';
54+
55+
case BookDelete = 'book-delete';
56+
case BookDeleteAll = 'book-delete-all';
57+
case BookDeleteOwn = 'book-delete-own';
58+
59+
case BookUpdate = 'book-update';
60+
case BookUpdateAll = 'book-update-all';
61+
case BookUpdateOwn = 'book-update-own';
62+
63+
case BookView = 'book-view';
64+
case BookViewAll = 'book-view-all';
65+
case BookViewOwn = 'book-view-own';
66+
67+
case BookshelfCreate = 'bookshelf-create';
68+
case BookshelfCreateAll = 'bookshelf-create-all';
69+
case BookshelfCreateOwn = 'bookshelf-create-own';
70+
71+
case BookshelfDelete = 'bookshelf-delete';
72+
case BookshelfDeleteAll = 'bookshelf-delete-all';
73+
case BookshelfDeleteOwn = 'bookshelf-delete-own';
74+
75+
case BookshelfUpdate = 'bookshelf-update';
76+
case BookshelfUpdateAll = 'bookshelf-update-all';
77+
case BookshelfUpdateOwn = 'bookshelf-update-own';
78+
79+
case BookshelfView = 'bookshelf-view';
80+
case BookshelfViewAll = 'bookshelf-view-all';
81+
case BookshelfViewOwn = 'bookshelf-view-own';
82+
83+
case ChapterCreate = 'chapter-create';
84+
case ChapterCreateAll = 'chapter-create-all';
85+
case ChapterCreateOwn = 'chapter-create-own';
86+
87+
case ChapterDelete = 'chapter-delete';
88+
case ChapterDeleteAll = 'chapter-delete-all';
89+
case ChapterDeleteOwn = 'chapter-delete-own';
90+
91+
case ChapterUpdate = 'chapter-update';
92+
case ChapterUpdateAll = 'chapter-update-all';
93+
case ChapterUpdateOwn = 'chapter-update-own';
94+
95+
case ChapterView = 'chapter-view';
96+
case ChapterViewAll = 'chapter-view-all';
97+
case ChapterViewOwn = 'chapter-view-own';
98+
99+
case CommentCreate = 'comment-create';
100+
case CommentCreateAll = 'comment-create-all';
101+
case CommentCreateOwn = 'comment-create-own';
102+
103+
case CommentDelete = 'comment-delete';
104+
case CommentDeleteAll = 'comment-delete-all';
105+
case CommentDeleteOwn = 'comment-delete-own';
106+
107+
case CommentUpdate = 'comment-update';
108+
case CommentUpdateAll = 'comment-update-all';
109+
case CommentUpdateOwn = 'comment-update-own';
110+
111+
case ImageCreate = 'image-create';
112+
case ImageCreateAll = 'image-create-all';
113+
case ImageCreateOwn = 'image-create-own';
114+
115+
case ImageDelete = 'image-delete';
116+
case ImageDeleteAll = 'image-delete-all';
117+
case ImageDeleteOwn = 'image-delete-own';
118+
119+
case ImageUpdate = 'image-update';
120+
case ImageUpdateAll = 'image-update-all';
121+
case ImageUpdateOwn = 'image-update-own';
122+
123+
case PageCreate = 'page-create';
124+
case PageCreateAll = 'page-create-all';
125+
case PageCreateOwn = 'page-create-own';
126+
127+
case PageDelete = 'page-delete';
128+
case PageDeleteAll = 'page-delete-all';
129+
case PageDeleteOwn = 'page-delete-own';
130+
131+
case PageUpdate = 'page-update';
132+
case PageUpdateAll = 'page-update-all';
133+
case PageUpdateOwn = 'page-update-own';
134+
135+
case PageView = 'page-view';
136+
case PageViewAll = 'page-view-all';
137+
case PageViewOwn = 'page-view-own';
138+
139+
/**
140+
* Get the generic permissions which may be queried for entities.
141+
*/
142+
public static function genericForEntity(): array
143+
{
144+
return [
145+
self::View,
146+
self::Create,
147+
self::Update,
148+
self::Delete,
149+
];
150+
}
151+
}

‎app/Permissions/PermissionApplicator.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@ public function __construct(
2424
/**
2525
* Checks if an entity has a restriction set upon it.
2626
*/
27-
public function checkOwnableUserAccess(Model&OwnableInterface $ownable, string $permission): bool
27+
public function checkOwnableUserAccess(Model&OwnableInterface $ownable, string|Permission $permission): bool
2828
{
29-
$explodedPermission = explode('-', $permission);
29+
$permissionName = is_string($permission) ? $permission : $permission->value;
30+
$explodedPermission = explode('-', $permissionName);
3031
$action = $explodedPermission[1] ?? $explodedPermission[0];
31-
$fullPermission = count($explodedPermission) > 1 ? $permission : $ownable->getMorphClass() . '-' . $permission;
32+
$fullPermission = count($explodedPermission) > 1 ? $permissionName : $ownable->getMorphClass() . '-' . $permissionName;
3233

3334
$user = $this->currentUser();
3435
$userRoleIds = $this->getCurrentUserRoleIds();
@@ -235,8 +236,13 @@ protected function getCurrentUserRoleIds(): array
235236
*/
236237
protected function ensureValidEntityAction(string $action): void
237238
{
238-
if (!in_array($action, EntityPermission::PERMISSIONS)) {
239-
throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
239+
$allowed = Permission::genericForEntity();
240+
foreach ($allowed as $permission) {
241+
if ($permission->value === $action) {
242+
return;
243+
}
240244
}
245+
246+
throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
241247
}
242248
}

‎app/Users/Models/Role.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public function jointPermissions(): HasMany
5757
*/
5858
public function permissions(): BelongsToMany
5959
{
60-
return $this->belongsToMany(RolePermission::class, 'permission_role', 'role_id', 'permission_id');
60+
return $this->belongsToMany(RolePermission::class, 'permission_role', 'role_id', 'permission_id')
61+
->select(['id', 'name']);
6162
}
6263

6364
/**

‎app/Users/Models/User.php‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use BookStack\App\Model;
1313
use BookStack\App\SluggableInterface;
1414
use BookStack\Entities\Tools\SlugGenerator;
15+
use BookStack\Permissions\Permission;
1516
use BookStack\Translation\LocaleDefinition;
1617
use BookStack\Translation\LocaleManager;
1718
use BookStack\Uploads\Image;
@@ -26,7 +27,6 @@
2627
use Illuminate\Database\Eloquent\Relations\BelongsTo;
2728
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
2829
use Illuminate\Database\Eloquent\Relations\HasMany;
29-
use Illuminate\Database\Eloquent\Relations\Relation;
3030
use Illuminate\Notifications\Notifiable;
3131
use Illuminate\Support\Collection;
3232

@@ -156,8 +156,9 @@ public function attachDefaultRole(): void
156156
/**
157157
* Check if the user has a particular permission.
158158
*/
159-
public function can(string $permissionName): bool
159+
public function can(string|Permission $permission): bool
160160
{
161+
$permissionName = is_string($permission) ? $permission : $permission->value;
161162
return $this->permissions()->contains($permissionName);
162163
}
163164

@@ -181,17 +182,17 @@ protected function permissions(): Collection
181182
}
182183

183184
/**
184-
* Clear any cached permissions on this instance.
185+
* Clear any cached permissions in this instance.
185186
*/
186-
public function clearPermissionCache()
187+
public function clearPermissionCache(): void
187188
{
188189
$this->permissions = null;
189190
}
190191

191192
/**
192193
* Attach a role to this user.
193194
*/
194-
public function attachRole(Role $role)
195+
public function attachRole(Role $role): void
195196
{
196197
$this->roles()->attach($role->id);
197198
$this->unsetRelation('roles');
@@ -207,15 +208,11 @@ public function socialAccounts(): HasMany
207208

208209
/**
209210
* Check if the user has a social account,
210-
* If a driver is passed it checks for that single account type.
211-
*
212-
* @param bool|string $socialDriver
213-
*
214-
* @return bool
211+
* If a driver is passed, it checks for that single account type.
215212
*/
216-
public function hasSocialAccount($socialDriver = false)
213+
public function hasSocialAccount(string $socialDriver = ''): bool
217214
{
218-
if ($socialDriver === false) {
215+
if (empty($socialDriver)) {
219216
return $this->socialAccounts()->count() > 0;
220217
}
221218

database/migrations/2025_09_02_111542_remove_comments_text_column.php renamed to database/migrations/2025_09_02_111542_remove_unused_columns.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ public function up(): void
1414
Schema::table('comments', function (Blueprint $table) {
1515
$table->dropColumn('text');
1616
});
17+
18+
Schema::table('role_permissions', function (Blueprint $table) {
19+
$table->dropColumn('display_name');
20+
$table->dropColumn('description');
21+
});
1722
}
1823

1924
/**

‎tests/Helpers/PermissionsProvider.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use BookStack\Entities\Models\Entity;
66
use BookStack\Permissions\Models\EntityPermission;
77
use BookStack\Permissions\Models\RolePermission;
8+
use BookStack\Permissions\Permission;
89
use BookStack\Settings\SettingService;
910
use BookStack\Users\Models\Role;
1011
use BookStack\Users\Models\User;
@@ -139,8 +140,8 @@ protected function addEntityPermissionEntries(Entity $entity, array $entityPermi
139140
protected function actionListToEntityPermissionData(array $actionList, int $roleId = 0): array
140141
{
141142
$permissionData = ['role_id' => $roleId];
142-
foreach (EntityPermission::PERMISSIONS as $possibleAction) {
143-
$permissionData[$possibleAction] = in_array($possibleAction, $actionList);
143+
foreach (Permission::genericForEntity() as $permission) {
144+
$permissionData[$permission->value] = in_array($permission->value, $actionList);
144145
}
145146

146147
return $permissionData;

0 commit comments

Comments
 (0)