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 7f2134dd87abe8d7eedd8cc9d03038ba54d55260..5d3be4951ca70b597e5159c4f3dc13e3e2afdaf6 100644 (file)
--- 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\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;
 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();
     {
         if (config('app.env') === 'demo') {
             $this->showPermissionError();


@@ -40,14 +40,13 @@ abstract class Controller extends BaseController
     /**
      * Adds the page title into the view.
      */
     /**
      * Adds the page title into the view.
      */

-    public function setPageTitle(string $title)
+    public function setPageTitle(string $title): void

     {
         view()->share('pageTitle', $title);
     }
 
     /**
     {
         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
      */
      *
      * @throws NotifyException
      */


@@ -61,7 +60,7 @@ abstract class Controller extends BaseController
     /**
      * Checks that the current user has the given permission otherwise throw an exception.
      */
     /**
      * 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();
     {
         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.
      */
     /**
      * 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);
     {
         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.
      */
      * 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);
     {
         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.
      */
      * 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;
     {
         $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
     {
      */
     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
     {
      */
     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
     {
      */
     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
     {
      */
     protected function showErrorNotification(string $message): void
     {

diff --git a/app/Users/Controllers/RoleApiController.php b/app/Users/Controllers/RoleApiController.php
index 2f3638cd3e2777731cf19a551a46f42dc2855aa1..6d87c9e275e71f7c6d393540bcbffac7a612fe92 100644 (file)
--- 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
 {
 
 class RoleApiController extends ApiController
 {

-    protected PermissionsRepo $permissionsRepo;
-

     protected array $fieldsToExpose = [
         'display_name', 'description', 'mfa_enforced', 'external_auth_id', 'created_at', 'updated_at',
     ];
     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');
         // 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');
     {
         $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 1ec2dfdf991de4118a0004e1835d631f2f47fff3..3a36e5ad603fa67786283cb0055666a6a574fc27 100644 (file)
--- 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('comments', function (Blueprint $table) {
             $table->longText('text')->nullable();
         });

+
+        Schema::table('role_permissions', function (Blueprint $table) {
+            $table->string('display_name')->nullable();
+            $table->string('description')->nullable();
+        });

     }
 };
     }
 };