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.
This commit is contained in:
Dan Brown
2025-09-08 16:15:42 +01:00
parent c8716df284
commit 5fc11d46d5
3 changed files with 23 additions and 22 deletions

View File

@@ -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
{

View File

@@ -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');

View File

@@ -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();
});
}
};