luisgulo/BookStack
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Aded roles API controller methods

Browse source 
Altered & updated permissions repo, and existing connected
RoleController to suit.
Also extracts in-app success notifications to auto activity system.
Tweaked tests where required.
This commit is contained in:
Dan Brown 2023-02-18 18:36:34 +00:00
parent 55456a57d6
commit 723f108bd9
No known key found for this signature in database
GPG key ID: 46D9F943C24A2EF9
12 changed files with 238 additions and 72 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

68
app/Auth/Permissions/PermissionsRepo.php
View file

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

2
app/Auth/Role.php
View file

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

2
app/Auth/User.php
View file

@ -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',
];
/**

11
app/Http/Controllers/Api/ApiController.php
View file

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

133
app/Http/Controllers/Api/RoleApiController.php Normal file
View file

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

32
app/Http/Controllers/RoleController.php
View file

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

5
lang/en/activities.php
View file

@ -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',

3
lang/en/settings.php
View file

@ -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',

7
routes/api.php
View file

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

43
tests/Api/RolesApiTest.php
View file

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

2
tests/Helpers/UserRoleProvider.php
View file

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

2
tests/Permissions/RolesTest.php
View file

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