From: Dan Brown Date: Mon, 8 Sep 2025 15:15:42 +0000 (+0100) Subject: Permissions: Added enum usage to controller helpers X-Git-Tag: v25.11~1^2~19^2~4 X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/5fc11d46d5115b96a2571fa371671c26900fab8f Permissions: Added enum usage to controller helpers Also fixed various missing types or spelling/formatting points. Added down action for role_permission table changes in migration. --- diff --git a/app/Http/Controller.php b/app/Http/Controller.php index 7f2134dd8..5d3be4951 100644 --- a/app/Http/Controller.php +++ b/app/Http/Controller.php @@ -6,6 +6,7 @@ use BookStack\Activity\Models\Loggable; use BookStack\App\Model; use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; +use BookStack\Permissions\Permission; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; use Illuminate\Http\JsonResponse; @@ -27,10 +28,9 @@ abstract class Controller extends BaseController } /** - * Stops the application and shows a permission error if - * the application is in demo mode. + * Stops the application and shows a permission error if the application is in demo mode. */ - protected function preventAccessInDemoMode() + protected function preventAccessInDemoMode(): void { if (config('app.env') === 'demo') { $this->showPermissionError(); @@ -40,14 +40,13 @@ abstract class Controller extends BaseController /** * Adds the page title into the view. */ - public function setPageTitle(string $title) + public function setPageTitle(string $title): void { view()->share('pageTitle', $title); } /** - * On a permission error redirect to home and display. - * the error as a notification. + * On a permission error redirect to home and display the error as a notification. * * @throws NotifyException */ @@ -61,7 +60,7 @@ abstract class Controller extends BaseController /** * Checks that the current user has the given permission otherwise throw an exception. */ - protected function checkPermission(string $permission): void + protected function checkPermission(string|Permission $permission): void { if (!user() || !user()->can($permission)) { $this->showPermissionError(); @@ -81,7 +80,7 @@ abstract class Controller extends BaseController /** * Check the current user's permissions against an ownable item otherwise throw an exception. */ - protected function checkOwnablePermission(string $permission, Model $ownable, string $redirectLocation = '/'): void + protected function checkOwnablePermission(string|Permission $permission, Model $ownable, string $redirectLocation = '/'): void { if (!userCan($permission, $ownable)) { $this->showPermissionError($redirectLocation); @@ -92,7 +91,7 @@ abstract class Controller extends BaseController * Check if a user has a permission or bypass the permission * check if the given callback resolves true. */ - protected function checkPermissionOr(string $permission, callable $callback): void + protected function checkPermissionOr(string|Permission $permission, callable $callback): void { if ($callback() !== true) { $this->checkPermission($permission); @@ -103,7 +102,7 @@ abstract class Controller extends BaseController * Check if the current user has a permission or bypass if the provided user * id matches the current user. */ - protected function checkPermissionOrCurrentUser(string $permission, int $userId): void + protected function checkPermissionOrCurrentUser(string|Permission $permission, int $userId): void { $this->checkPermissionOr($permission, function () use ($userId) { return $userId === user()->id; @@ -111,7 +110,7 @@ abstract class Controller extends BaseController } /** - * Send back a json error message. + * Send back a JSON error message. */ protected function jsonError(string $messageText = '', int $statusCode = 500): JsonResponse { @@ -127,7 +126,7 @@ abstract class Controller extends BaseController } /** - * Show a positive, successful notification to the user on next view load. + * Show a positive, successful notification to the user on the next view load. */ protected function showSuccessNotification(string $message): void { @@ -135,7 +134,7 @@ abstract class Controller extends BaseController } /** - * Show a warning notification to the user on next view load. + * Show a warning notification to the user on the next view load. */ protected function showWarningNotification(string $message): void { @@ -143,7 +142,7 @@ abstract class Controller extends BaseController } /** - * Show an error notification to the user on next view load. + * Show an error notification to the user on the next view load. */ protected function showErrorNotification(string $message): void { diff --git a/app/Users/Controllers/RoleApiController.php b/app/Users/Controllers/RoleApiController.php index 2f3638cd3..6d87c9e27 100644 --- a/app/Users/Controllers/RoleApiController.php +++ b/app/Users/Controllers/RoleApiController.php @@ -10,8 +10,6 @@ use Illuminate\Support\Facades\DB; class RoleApiController extends ApiController { - protected PermissionsRepo $permissionsRepo; - protected array $fieldsToExpose = [ 'display_name', 'description', 'mfa_enforced', 'external_auth_id', 'created_at', 'updated_at', ]; @@ -35,10 +33,9 @@ class RoleApiController extends ApiController ] ]; - public function __construct(PermissionsRepo $permissionsRepo) - { - $this->permissionsRepo = $permissionsRepo; - + public function __construct( + protected PermissionsRepo $permissionsRepo + ) { // Checks for all endpoints in this controller $this->middleware(function ($request, $next) { $this->checkPermission('user-roles-manage'); @@ -125,9 +122,9 @@ class RoleApiController extends ApiController } /** - * Format the given role model for single-result display. + * Format the given role model for a single-result display. */ - protected function singleFormatter(Role $role) + protected function singleFormatter(Role $role): void { $role->load('users:id,name,slug'); $role->unsetRelation('permissions'); diff --git a/database/migrations/2025_09_02_111542_remove_unused_columns.php b/database/migrations/2025_09_02_111542_remove_unused_columns.php index 1ec2dfdf9..3a36e5ad6 100644 --- a/database/migrations/2025_09_02_111542_remove_unused_columns.php +++ b/database/migrations/2025_09_02_111542_remove_unused_columns.php @@ -29,5 +29,10 @@ return new class extends Migration Schema::table('comments', function (Blueprint $table) { $table->longText('text')->nullable(); }); + + Schema::table('role_permissions', function (Blueprint $table) { + $table->string('display_name')->nullable(); + $table->string('description')->nullable(); + }); } };