diff --git a/app/Auth/Permissions/PermissionsRepo.php b/app/Auth/Permissions/PermissionsRepo.php
index 6dcef7256..75354e79b 100644
--- a/app/Auth/Permissions/PermissionsRepo.php
+++ b/app/Auth/Permissions/PermissionsRepo.php
@@ -12,11 +12,8 @@ use Illuminate\Database\Eloquent\Collection;
class PermissionsRepo
{
protected JointPermissionBuilder $permissionBuilder;
- protected $systemRoles = ['admin', 'public'];
+ protected array $systemRoles = ['admin', 'public'];
- /**
- * PermissionsRepo constructor.
- */
public function __construct(JointPermissionBuilder $permissionBuilder)
{
$this->permissionBuilder = $permissionBuilder;
@@ -41,7 +38,7 @@ class PermissionsRepo
/**
* Get a role via its ID.
*/
- public function getRoleById($id): Role
+ public function getRoleById(int $id): Role
{
return Role::query()->findOrFail($id);
}
@@ -52,10 +49,10 @@ class PermissionsRepo
public function saveNewRole(array $roleData): Role
{
$role = new Role($roleData);
- $role->mfa_enforced = ($roleData['mfa_enforced'] ?? 'false') === 'true';
+ $role->mfa_enforced = boolval($roleData['mfa_enforced'] ?? false);
$role->save();
- $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : [];
+ $permissions = $roleData['permissions'] ?? [];
$this->assignRolePermissions($role, $permissions);
$this->permissionBuilder->rebuildForRole($role);
@@ -66,42 +63,45 @@ class PermissionsRepo
/**
* Updates an existing role.
- * Ensure Admin role always have core permissions.
+ * Ensures Admin system role always have core permissions.
*/
- public function updateRole($roleId, array $roleData)
+ public function updateRole($roleId, array $roleData): Role
{
$role = $this->getRoleById($roleId);
- $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : [];
+ if (isset($roleData['permissions'])) {
+ $this->assignRolePermissions($role, $roleData['permissions']);
+ }
+
+ $role->fill($roleData);
+ $role->save();
+ $this->permissionBuilder->rebuildForRole($role);
+
+ Activity::add(ActivityType::ROLE_UPDATE, $role);
+
+ return $role;
+ }
+
+ /**
+ * Assign a list of permission names to the given role.
+ */
+ protected function assignRolePermissions(Role $role, array $permissionNameArray = []): void
+ {
+ $permissions = [];
+ $permissionNameArray = array_values($permissionNameArray);
+
+ // Ensure the admin system role retains vital system permissions
if ($role->system_name === 'admin') {
- $permissions = array_merge($permissions, [
+ $permissionNameArray = array_unique(array_merge($permissionNameArray, [
'users-manage',
'user-roles-manage',
'restrictions-manage-all',
'restrictions-manage-own',
'settings-manage',
- ]);
+ ]));
}
- $this->assignRolePermissions($role, $permissions);
-
- $role->fill($roleData);
- $role->mfa_enforced = ($roleData['mfa_enforced'] ?? 'false') === 'true';
- $role->save();
- $this->permissionBuilder->rebuildForRole($role);
-
- Activity::add(ActivityType::ROLE_UPDATE, $role);
- }
-
- /**
- * Assign a list of permission names to a role.
- */
- protected function assignRolePermissions(Role $role, array $permissionNameArray = [])
- {
- $permissions = [];
- $permissionNameArray = array_values($permissionNameArray);
-
- if ($permissionNameArray) {
+ if (!empty($permissionNameArray)) {
$permissions = RolePermission::query()
->whereIn('name', $permissionNameArray)
->pluck('id')
@@ -114,13 +114,13 @@ class PermissionsRepo
/**
* Delete a role from the system.
* Check it's not an admin role or set as default before deleting.
- * If an migration Role ID is specified the users assign to the current role
+ * If a migration Role ID is specified the users assign to the current role
* will be added to the role of the specified id.
*
* @throws PermissionsException
* @throws Exception
*/
- public function deleteRole($roleId, $migrateRoleId)
+ public function deleteRole(int $roleId, int $migrateRoleId = 0): void
{
$role = $this->getRoleById($roleId);
@@ -131,7 +131,7 @@ class PermissionsRepo
throw new PermissionsException(trans('errors.role_registration_default_cannot_delete'));
}
- if ($migrateRoleId) {
+ if ($migrateRoleId !== 0) {
$newRole = Role::query()->find($migrateRoleId);
if ($newRole) {
$users = $role->users()->pluck('id')->toArray();
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index b293d1af2..0f43dd18c 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -27,7 +27,7 @@ class Role extends Model implements Loggable
{
use HasFactory;
- protected $fillable = ['display_name', 'description', 'external_auth_id'];
+ protected $fillable = ['display_name', 'description', 'external_auth_id', 'mfa_enforced'];
protected $hidden = ['pivot'];
diff --git a/app/Auth/User.php b/app/Auth/User.php
index cf9f20e52..90bb3d68e 100644
--- a/app/Auth/User.php
+++ b/app/Auth/User.php
@@ -72,7 +72,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
*/
protected $hidden = [
'password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email',
- 'created_at', 'updated_at', 'image_id', 'roles', 'avatar', 'user_id',
+ 'created_at', 'updated_at', 'image_id', 'roles', 'avatar', 'user_id', 'pivot',
];
/**
diff --git a/app/Http/Controllers/Api/ApiController.php b/app/Http/Controllers/Api/ApiController.php
index 9652654be..5c448e49f 100644
--- a/app/Http/Controllers/Api/ApiController.php
+++ b/app/Http/Controllers/Api/ApiController.php
@@ -32,10 +32,15 @@ abstract class ApiController extends Controller
*/
public function getValidationRules(): array
{
- if (method_exists($this, 'rules')) {
- return $this->rules();
- }
+ return $this->rules();
+ }
+ /**
+ * Get the validation rules for the actions in this controller.
+ * Defaults to a $rules property but can be a rules() method.
+ */
+ protected function rules(): array
+ {
return $this->rules;
}
}
diff --git a/app/Http/Controllers/Api/RoleApiController.php b/app/Http/Controllers/Api/RoleApiController.php
new file mode 100644
index 000000000..119279822
--- /dev/null
+++ b/app/Http/Controllers/Api/RoleApiController.php
@@ -0,0 +1,133 @@
+<?php
+
+namespace BookStack\Http\Controllers\Api;
+
+use BookStack\Auth\Permissions\PermissionsRepo;
+use BookStack\Auth\Role;
+use BookStack\Exceptions\UserUpdateException;
+use Illuminate\Http\Request;
+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',
+ ];
+
+ protected $rules = [
+ 'create' => [
+ 'display_name' => ['required', 'min:3', 'max:180'],
+ 'description' => ['max:180'],
+ 'mfa_enforced' => ['boolean'],
+ 'external_auth_id' => ['string'],
+ 'permissions' => ['array'],
+ 'permissions.*' => ['string'],
+ ],
+ 'update' => [
+ 'display_name' => ['min:3', 'max:180'],
+ 'description' => ['max:180'],
+ 'mfa_enforced' => ['boolean'],
+ 'external_auth_id' => ['string'],
+ 'permissions' => ['array'],
+ 'permissions.*' => ['string'],
+ ]
+ ];
+
+ public function __construct(PermissionsRepo $permissionsRepo)
+ {
+ $this->permissionsRepo = $permissionsRepo;
+
+ // Checks for all endpoints in this controller
+ $this->middleware(function ($request, $next) {
+ $this->checkPermission('user-roles-manage');
+
+ return $next($request);
+ });
+ }
+
+ /**
+ * Get a listing of roles in the system.
+ * Requires permission to manage roles.
+ */
+ public function list()
+ {
+ $roles = Role::query()->select(['*'])
+ ->withCount(['users', 'permissions']);
+
+ return $this->apiListingResponse($roles, [
+ ...$this->fieldsToExpose,
+ 'permissions_count',
+ 'users_count',
+ ]);
+ }
+
+ /**
+ * Create a new role in the system.
+ * Requires permission to manage roles.
+ */
+ public function create(Request $request)
+ {
+ $data = $this->validate($request, $this->rules()['create']);
+
+ $role = null;
+ DB::transaction(function () use ($data, &$role) {
+ $role = $this->permissionsRepo->saveNewRole($data);
+ });
+
+ $this->singleFormatter($role);
+
+ return response()->json($role);
+ }
+
+ /**
+ * View the details of a single user.
+ * Requires permission to manage roles.
+ */
+ public function read(string $id)
+ {
+ $user = $this->permissionsRepo->getRoleById($id);
+ $this->singleFormatter($user);
+
+ return response()->json($user);
+ }
+
+ /**
+ * Update an existing role in the system.
+ * Requires permission to manage roles.
+ */
+ public function update(Request $request, string $id)
+ {
+ $data = $this->validate($request, $this->rules()['update']);
+ $role = $this->permissionsRepo->updateRole($id, $data);
+
+ $this->singleFormatter($role);
+
+ return response()->json($role);
+ }
+
+ /**
+ * Delete a user from the system.
+ * Can optionally accept a user id via `migrate_ownership_id` to indicate
+ * who should be the new owner of their related content.
+ * Requires permission to manage roles.
+ */
+ public function delete(string $id)
+ {
+ $this->permissionsRepo->deleteRole(intval($id));
+
+ return response('', 204);
+ }
+
+ /**
+ * Format the given role model for single-result display.
+ */
+ protected function singleFormatter(Role $role)
+ {
+ $role->load('users:id,name,slug');
+ $role->unsetRelation('permissions');
+ $role->setAttribute('permissions', $role->permissions()->pluck('name'));
+ $role->makeVisible(['users', 'permissions']);
+ }
+}
diff --git a/app/Http/Controllers/RoleController.php b/app/Http/Controllers/RoleController.php
index a9be19e0c..2bf0a7b19 100644
--- a/app/Http/Controllers/RoleController.php
+++ b/app/Http/Controllers/RoleController.php
@@ -74,13 +74,17 @@ class RoleController extends Controller
public function store(Request $request)
{
$this->checkPermission('user-roles-manage');
- $this->validate($request, [
+ $data = $this->validate($request, [
'display_name' => ['required', 'min:3', 'max:180'],
'description' => ['max:180'],
+ 'external_auth_id' => ['string'],
+ 'permissions' => ['array'],
+ 'mfa_enforced' => ['string'],
]);
- $this->permissionsRepo->saveNewRole($request->all());
- $this->showSuccessNotification(trans('settings.role_create_success'));
+ $data['permissions'] = array_keys($data['permissions'] ?? []);
+ $data['mfa_enforced'] = ($data['mfa_enforced'] ?? 'false') === 'true';
+ $this->permissionsRepo->saveNewRole($data);
return redirect('/settings/roles');
}
@@ -100,19 +104,27 @@ class RoleController extends Controller
/**
* Updates a user role.
- *
- * @throws ValidationException
*/
public function update(Request $request, string $id)
{
$this->checkPermission('user-roles-manage');
- $this->validate($request, [
+ $data = $this->validate($request, [
'display_name' => ['required', 'min:3', 'max:180'],
'description' => ['max:180'],
+ 'external_auth_id' => ['string'],
+ 'permissions' => ['array'],
+ 'mfa_enforced' => ['string'],
]);
- $this->permissionsRepo->updateRole($id, $request->all());
- $this->showSuccessNotification(trans('settings.role_update_success'));
+ if (isset($data['permissions'])) {
+ $data['permissions'] = array_keys($data['permissions']);
+ }
+
+ if (isset($data['mfa_enforced'])) {
+ $data['mfa_enforced'] = $data['mfa_enforced'] === 'true';
+ }
+
+ $this->permissionsRepo->updateRole($id, $data);
return redirect('/settings/roles');
}
@@ -145,15 +157,13 @@ class RoleController extends Controller
$this->checkPermission('user-roles-manage');
try {
- $this->permissionsRepo->deleteRole($id, $request->get('migrate_role_id'));
+ $this->permissionsRepo->deleteRole($id, $request->get('migrate_role_id', 0));
} catch (PermissionsException $e) {
$this->showErrorNotification($e->getMessage());
return redirect()->back();
}
- $this->showSuccessNotification(trans('settings.role_delete_success'));
-
return redirect('/settings/roles');
}
}
diff --git a/lang/en/activities.php b/lang/en/activities.php
index f348bff1f..e89b8eab2 100644
--- a/lang/en/activities.php
+++ b/lang/en/activities.php
@@ -67,6 +67,11 @@ return [
'user_update_notification' => 'User successfully updated',
'user_delete_notification' => 'User successfully removed',
+ // Roles
+ 'role_create_notification' => 'Role successfully created',
+ 'role_update_notification' => 'Role successfully updated',
+ 'role_delete_notification' => 'Role successfully deleted',
+
// Other
'commented_on' => 'commented on',
'permissions_update' => 'updated permissions',
diff --git a/lang/en/settings.php b/lang/en/settings.php
index 6f4376d42..5a94a824a 100644
--- a/lang/en/settings.php
+++ b/lang/en/settings.php
@@ -143,13 +143,11 @@ return [
'roles_assigned_users' => 'Assigned Users',
'roles_permissions_provided' => 'Provided Permissions',
'role_create' => 'Create New Role',
- 'role_create_success' => 'Role successfully created',
'role_delete' => 'Delete Role',
'role_delete_confirm' => 'This will delete the role with the name \':roleName\'.',
'role_delete_users_assigned' => 'This role has :userCount users assigned to it. If you would like to migrate the users from this role select a new role below.',
'role_delete_no_migration' => "Don't migrate users",
'role_delete_sure' => 'Are you sure you want to delete this role?',
- 'role_delete_success' => 'Role successfully deleted',
'role_edit' => 'Edit Role',
'role_details' => 'Role Details',
'role_name' => 'Role Name',
@@ -175,7 +173,6 @@ return [
'role_own' => 'Own',
'role_controlled_by_asset' => 'Controlled by the asset they are uploaded to',
'role_save' => 'Save Role',
- 'role_update_success' => 'Role successfully updated',
'role_users' => 'Users in this role',
'role_users_none' => 'No users are currently assigned to this role',
diff --git a/routes/api.php b/routes/api.php
index d350fd86b..aa3f66b60 100644
--- a/routes/api.php
+++ b/routes/api.php
@@ -16,6 +16,7 @@ use BookStack\Http\Controllers\Api\ChapterExportApiController;
use BookStack\Http\Controllers\Api\PageApiController;
use BookStack\Http\Controllers\Api\PageExportApiController;
use BookStack\Http\Controllers\Api\RecycleBinApiController;
+use BookStack\Http\Controllers\Api\RoleApiController;
use BookStack\Http\Controllers\Api\SearchApiController;
use BookStack\Http\Controllers\Api\UserApiController;
use Illuminate\Support\Facades\Route;
@@ -75,6 +76,12 @@ Route::get('users/{id}', [UserApiController::class, 'read']);
Route::put('users/{id}', [UserApiController::class, 'update']);
Route::delete('users/{id}', [UserApiController::class, 'delete']);
+Route::get('roles', [RoleApiController::class, 'list']);
+Route::post('roles', [RoleApiController::class, 'create']);
+Route::get('roles/{id}', [RoleApiController::class, 'read']);
+Route::put('roles/{id}', [RoleApiController::class, 'update']);
+Route::delete('roles/{id}', [RoleApiController::class, 'delete']);
+
Route::get('recycle-bin', [RecycleBinApiController::class, 'list']);
Route::put('recycle-bin/{deletionId}', [RecycleBinApiController::class, 'restore']);
Route::delete('recycle-bin/{deletionId}', [RecycleBinApiController::class, 'destroy']);
diff --git a/tests/Api/RolesApiTest.php b/tests/Api/RolesApiTest.php
index 38026a40a..e231c167c 100644
--- a/tests/Api/RolesApiTest.php
+++ b/tests/Api/RolesApiTest.php
@@ -40,14 +40,15 @@ class RolesApiTest extends TestCase
$resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
$resp->assertJson(['data' => [
[
- 'id' => $firstRole->id,
- 'display_name' => $firstRole->display_name,
- 'description' => $firstRole->description,
- 'mfa_enforced' => $firstRole->mfa_enforced,
+ 'id' => $firstRole->id,
+ 'display_name' => $firstRole->display_name,
+ 'description' => $firstRole->description,
+ 'mfa_enforced' => $firstRole->mfa_enforced,
+ 'external_auth_id' => $firstRole->external_auth_id,
'permissions_count' => $firstRole->permissions()->count(),
- 'users_count' => $firstRole->users()->count(),
- 'created_at' => $firstRole->created_at->toJSON(),
- 'updated_at' => $firstRole->updated_at->toJSON(),
+ 'users_count' => $firstRole->users()->count(),
+ 'created_at' => $firstRole->created_at->toJSON(),
+ 'updated_at' => $firstRole->updated_at->toJSON(),
],
]]);
@@ -64,11 +65,12 @@ class RolesApiTest extends TestCase
'display_name' => 'My awesome role',
'description' => 'My great role description',
'mfa_enforced' => true,
+ 'external_auth_id' => 'auth_id',
'permissions' => [
'content-export',
- 'users-manage',
- 'page-view-own',
'page-view-all',
+ 'page-view-own',
+ 'users-manage',
]
]);
@@ -77,11 +79,12 @@ class RolesApiTest extends TestCase
'display_name' => 'My awesome role',
'description' => 'My great role description',
'mfa_enforced' => true,
+ 'external_auth_id' => 'auth_id',
'permissions' => [
'content-export',
- 'users-manage',
- 'page-view-own',
'page-view-all',
+ 'page-view-own',
+ 'users-manage',
]
]);
@@ -89,6 +92,7 @@ class RolesApiTest extends TestCase
'display_name' => 'My awesome role',
'description' => 'My great role description',
'mfa_enforced' => true,
+ 'external_auth_id' => 'auth_id',
]);
/** @var Role $role */
@@ -107,7 +111,7 @@ class RolesApiTest extends TestCase
'description' => 'My new role',
]);
$resp->assertStatus(422);
- $resp->assertJson($this->validationResponse(['display_name' => ['The display_name field is required.']]));
+ $resp->assertJson($this->validationResponse(['display_name' => ['The display name field is required.']]));
$resp = $this->postJson($this->baseEndpoint, [
'name' => 'My great role with a too long desc',
@@ -128,6 +132,7 @@ class RolesApiTest extends TestCase
'display_name' => $role->display_name,
'description' => $role->description,
'mfa_enforced' => $role->mfa_enforced,
+ 'external_auth_id' => $role->external_auth_id,
'permissions' => $role->permissions()->pluck('name')->toArray(),
'users' => $role->users()->get()->map(function (User $user) {
return [
@@ -147,11 +152,12 @@ class RolesApiTest extends TestCase
'display_name' => 'My updated role',
'description' => 'My great role description',
'mfa_enforced' => true,
+ 'external_auth_id' => 'updated_auth_id',
'permissions' => [
'content-export',
- 'users-manage',
- 'page-view-own',
'page-view-all',
+ 'page-view-own',
+ 'users-manage',
]
]);
@@ -161,16 +167,18 @@ class RolesApiTest extends TestCase
'display_name' => 'My updated role',
'description' => 'My great role description',
'mfa_enforced' => true,
+ 'external_auth_id' => 'updated_auth_id',
'permissions' => [
'content-export',
- 'users-manage',
- 'page-view-own',
'page-view-all',
+ 'page-view-own',
+ 'users-manage',
]
]);
$role->refresh();
$this->assertEquals(4, $role->permissions()->count());
+ $this->assertActivityExists(ActivityType::ROLE_UPDATE);
}
public function test_update_endpoint_does_not_remove_info_if_not_provided()
@@ -181,10 +189,11 @@ class RolesApiTest extends TestCase
$permissionCount = $role->permissions()->count();
$resp->assertStatus(200);
- $this->assertDatabaseHas('users', [
+ $this->assertDatabaseHas('roles', [
'id' => $role->id,
'display_name' => $role->display_name,
'description' => $role->description,
+ 'external_auth_id' => $role->external_auth_id,
]);
$role->refresh();
diff --git a/tests/Helpers/UserRoleProvider.php b/tests/Helpers/UserRoleProvider.php
index 355c1687c..a06112189 100644
--- a/tests/Helpers/UserRoleProvider.php
+++ b/tests/Helpers/UserRoleProvider.php
@@ -90,7 +90,7 @@ class UserRoleProvider
{
$permissionRepo = app(PermissionsRepo::class);
$roleData = Role::factory()->make()->toArray();
- $roleData['permissions'] = array_flip($rolePermissions);
+ $roleData['permissions'] = $rolePermissions;
return $permissionRepo->saveNewRole($roleData);
}
diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php
index 8bf700c07..d4d975dbd 100644
--- a/tests/Permissions/RolesTest.php
+++ b/tests/Permissions/RolesTest.php
@@ -869,7 +869,7 @@ class RolesTest extends TestCase
$this->asAdmin()->put('/settings/roles/' . $viewerRole->id, [
'display_name' => $viewerRole->display_name,
'description' => $viewerRole->description,
- 'permission' => [],
+ 'permissions' => [],
])->assertStatus(302);
$this->actingAs($viewer)->get($page->getUrl())->assertStatus(404);