Merge pull request #4051 from BookStackApp/roles_api

User Roles API Endpoint

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

app/Auth/Permissions/RolePermission.php

@@ -8,6 +8,8 @@ use Illuminate\Database\Eloquent\Relations\BelongsToMany;
 
 /**
  * @property int $id
+ * @property string $name
+ * @property string $display_name
  */
 class RolePermission extends Model
 {

app/Auth/Role.php

@@ -27,10 +27,14 @@ 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'];
 
+    protected $casts = [
+        'mfa_enforced' => 'boolean',
+    ];
+
     /**
      * The roles that belong to the role.
      */

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

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

app/Http/Controllers/Api/RoleApiController.php

@@ -0,0 +1,136 @@
+<?php
+
+namespace BookStack\Http\Controllers\Api;
+
+use BookStack\Auth\Permissions\PermissionsRepo;
+use BookStack\Auth\Role;
+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', 'string', 'min:3', 'max:180'],
+            'description'   => ['string', 'max:180'],
+            'mfa_enforced'  => ['boolean'],
+            'external_auth_id' => ['string'],
+            'permissions'   => ['array'],
+            'permissions.*' => ['string'],
+        ],
+        'update' => [
+            'display_name'  => ['string', 'min:3', 'max:180'],
+            'description'   => ['string', '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.
+     * Permissions should be provided as an array of permission name strings.
+     * 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 role.
+     * Provides the permissions and a high-level list of the users assigned.
+     * 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.
+     * Permissions should be provided as an array of permission name strings.
+     * An empty "permissions" array would clear granted permissions.
+     * In many cases, where permissions are changed, you'll want to fetch the existing
+     * permissions and then modify before providing in your update request.
+     * 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 role from the system.
+     * 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()->orderBy('name', 'asc')->pluck('name'));
+        $role->makeVisible(['users', 'permissions']);
+    }
+}

app/Http/Controllers/Api/UserApiController.php

@@ -13,9 +13,9 @@ use Illuminate\Validation\Rules\Unique;
 
 class UserApiController extends ApiController
 {
-    protected $userRepo;
+    protected UserRepo $userRepo;
 
-    protected $fieldsToExpose = [
+    protected array $fieldsToExpose = [
         'email', 'created_at', 'updated_at', 'last_activity_at', 'external_auth_id',
     ];
 

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,21 @@ 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'));
+        $data['permissions'] = array_keys($data['permissions'] ?? []);
+        $data['mfa_enforced'] = ($data['mfa_enforced'] ?? 'false') === 'true';
+        $this->permissionsRepo->updateRole($id, $data);
 
         return redirect('/settings/roles');
     }
@@ -145,15 +151,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');
     }
 }

dev/api/requests/roles-create.json

@@ -0,0 +1,11 @@
+{
+  "display_name": "Book Maintainer",
+  "description": "People who maintain books",
+  "mfa_enforced": true,
+  "permissions": [
+    "book-view-all",
+    "book-update-all",
+    "book-delete-all",
+    "restrictions-manage-all"
+  ]
+}

dev/api/requests/roles-update.json

@@ -0,0 +1,14 @@
+{
+  "display_name": "Book & Shelf Maintainers",
+  "description": "All those who maintain books & shelves",
+  "mfa_enforced": false,
+  "permissions": [
+    "book-view-all",
+    "book-update-all",
+    "book-delete-all",
+    "bookshelf-view-all",
+    "bookshelf-update-all",
+    "bookshelf-delete-all",
+    "restrictions-manage-all"
+  ]
+}

dev/api/responses/roles-create.json

@@ -0,0 +1,15 @@
+{
+  "display_name": "Book Maintainer",
+  "description": "People who maintain books",
+  "mfa_enforced": true,
+  "updated_at": "2023-02-19T15:38:40.000000Z",
+  "created_at": "2023-02-19T15:38:40.000000Z",
+  "id": 26,
+  "permissions": [
+    "book-delete-all",
+    "book-update-all",
+    "book-view-all",
+    "restrictions-manage-all"
+  ],
+  "users": []
+}

dev/api/responses/roles-list.json

@@ -0,0 +1,41 @@
+{
+  "data": [
+    {
+      "id": 1,
+      "display_name": "Admin",
+      "description": "Administrator of the whole application",
+      "created_at": "2021-09-29T16:29:19.000000Z",
+      "updated_at": "2022-11-03T13:26:18.000000Z",
+      "system_name": "admin",
+      "external_auth_id": "wizards",
+      "mfa_enforced": true,
+      "users_count": 11,
+      "permissions_count": 54
+    },
+    {
+      "id": 2,
+      "display_name": "Editor",
+      "description": "User can edit Books, Chapters & Pages",
+      "created_at": "2021-09-29T16:29:19.000000Z",
+      "updated_at": "2022-12-01T02:32:57.000000Z",
+      "system_name": "",
+      "external_auth_id": "",
+      "mfa_enforced": false,
+      "users_count": 17,
+      "permissions_count": 49
+    },
+    {
+      "id": 3,
+      "display_name": "Public",
+      "description": "The role given to public visitors if allowed",
+      "created_at": "2021-09-29T16:29:19.000000Z",
+      "updated_at": "2022-09-02T12:32:12.000000Z",
+      "system_name": "public",
+      "external_auth_id": "",
+      "mfa_enforced": false,
+      "users_count": 1,
+      "permissions_count": 2
+    }
+  ],
+  "total": 3
+}

dev/api/responses/roles-read.json

@@ -0,0 +1,23 @@
+{
+  "id": 26,
+  "display_name": "Book Maintainer",
+  "description": "People who maintain books",
+  "created_at": "2023-02-19T15:38:40.000000Z",
+  "updated_at": "2023-02-19T15:38:40.000000Z",
+  "system_name": "",
+  "external_auth_id": "",
+  "mfa_enforced": true,
+  "permissions": [
+    "book-delete-all",
+    "book-update-all",
+    "book-view-all",
+    "restrictions-manage-all"
+  ],
+  "users": [
+    {
+      "id": 11,
+      "name": "Barry Scott",
+      "slug": "barry-scott"
+    }
+  ]
+}

dev/api/responses/roles-update.json

@@ -0,0 +1,26 @@
+{
+  "id": 26,
+  "display_name": "Book & Shelf Maintainers",
+  "description": "All those who maintain books & shelves",
+  "created_at": "2023-02-19T15:38:40.000000Z",
+  "updated_at": "2023-02-19T15:49:13.000000Z",
+  "system_name": "",
+  "external_auth_id": "",
+  "mfa_enforced": false,
+  "permissions": [
+    "book-delete-all",
+    "book-update-all",
+    "book-view-all",
+    "bookshelf-delete-all",
+    "bookshelf-update-all",
+    "bookshelf-view-all",
+    "restrictions-manage-all"
+  ],
+  "users": [
+    {
+      "id": 11,
+      "name": "Barry Scott",
+      "slug": "barry-scott"
+    }
+  ]
+}

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

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

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;
@@ -59,7 +60,7 @@ Route::delete('pages/{id}', [PageApiController::class, 'delete']);
 Route::get('pages/{id}/export/html', [PageExportApiController::class, 'exportHtml']);
 Route::get('pages/{id}/export/pdf', [PageExportApiController::class, 'exportPdf']);
 Route::get('pages/{id}/export/plaintext', [PageExportApiController::class, 'exportPlainText']);
-Route::get('pages/{id}/export/markdown', [PageExportApiController::class, 'exportMarkDown']);
+Route::get('pages/{id}/export/markdown', [PageExportApiController::class, 'exportMarkdown']);
 
 Route::get('search', [SearchApiController::class, 'all']);
 
@@ -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']);

tests/Api/RolesApiTest.php

@@ -0,0 +1,236 @@
+<?php
+
+namespace Tests\Api;
+
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Role;
+use BookStack\Auth\User;
+use Tests\TestCase;
+
+class RolesApiTest extends TestCase
+{
+    use TestsApi;
+
+    protected string $baseEndpoint = '/api/roles';
+
+    protected array $endpointMap = [
+        ['get', '/api/roles'],
+        ['post', '/api/roles'],
+        ['get', '/api/roles/1'],
+        ['put', '/api/roles/1'],
+        ['delete', '/api/roles/1'],
+    ];
+
+    public function test_user_roles_manage_permission_needed_for_all_endpoints()
+    {
+        $this->actingAsApiEditor();
+        foreach ($this->endpointMap as [$method, $uri]) {
+            $resp = $this->json($method, $uri);
+            $resp->assertStatus(403);
+            $resp->assertJson($this->permissionErrorResponse());
+        }
+    }
+
+    public function test_index_endpoint_returns_expected_role_and_count()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Role $firstRole */
+        $firstRole = Role::query()->orderBy('id', 'asc')->first();
+
+        $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,
+                '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(),
+            ],
+        ]]);
+
+        $resp->assertJson(['total' => Role::query()->count()]);
+    }
+
+    public function test_create_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Role $role */
+        $role = Role::query()->first();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'display_name' => 'My awesome role',
+            'description'  => 'My great role description',
+            'mfa_enforced' => true,
+            'external_auth_id' => 'auth_id',
+            'permissions'  => [
+                'content-export',
+                'page-view-all',
+                'page-view-own',
+                'users-manage',
+            ]
+        ]);
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'display_name' => 'My awesome role',
+            'description'  => 'My great role description',
+            'mfa_enforced' => true,
+            'external_auth_id' => 'auth_id',
+            'permissions'  => [
+                'content-export',
+                'page-view-all',
+                'page-view-own',
+                'users-manage',
+            ]
+        ]);
+
+        $this->assertDatabaseHas('roles', [
+            'display_name' => 'My awesome role',
+            'description'  => 'My great role description',
+            'mfa_enforced' => true,
+            'external_auth_id' => 'auth_id',
+        ]);
+
+        /** @var Role $role */
+        $role = Role::query()->where('display_name', '=', 'My awesome role')->first();
+        $this->assertActivityExists(ActivityType::ROLE_CREATE, null, $role->logDescriptor());
+        $this->assertEquals(4, $role->permissions()->count());
+    }
+
+    public function test_create_name_and_description_validation()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $existingUser */
+        $existingUser = User::query()->first();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'description' => 'My new role',
+        ]);
+        $resp->assertStatus(422);
+        $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',
+            'description' => str_repeat('My great desc', 20),
+        ]);
+        $resp->assertStatus(422);
+        $resp->assertJson($this->validationResponse(['description' => ['The description may not be greater than 180 characters.']]));
+    }
+
+    public function test_read_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        $role = $this->users->editor()->roles()->first();
+        $resp = $this->getJson($this->baseEndpoint . "/{$role->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'display_name' => $role->display_name,
+            'description'  => $role->description,
+            'mfa_enforced' => $role->mfa_enforced,
+            'external_auth_id' => $role->external_auth_id,
+            'permissions'  => $role->permissions()->orderBy('name', 'asc')->pluck('name')->toArray(),
+            'users' => $role->users()->get()->map(function (User $user) {
+                return [
+                    'id' => $user->id,
+                    'name' => $user->name,
+                    'slug' => $user->slug,
+                ];
+            })->toArray(),
+        ]);
+    }
+
+    public function test_update_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        $role = $this->users->editor()->roles()->first();
+        $resp = $this->putJson($this->baseEndpoint . "/{$role->id}", [
+            'display_name' => 'My updated role',
+            'description'  => 'My great role description',
+            'mfa_enforced' => true,
+            'external_auth_id' => 'updated_auth_id',
+            'permissions'  => [
+                'content-export',
+                'page-view-all',
+                'page-view-own',
+                'users-manage',
+            ]
+        ]);
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id'           => $role->id,
+            'display_name' => 'My updated role',
+            'description'  => 'My great role description',
+            'mfa_enforced' => true,
+            'external_auth_id' => 'updated_auth_id',
+            'permissions'  => [
+                'content-export',
+                '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()
+    {
+        $this->actingAsApiAdmin();
+        $role = $this->users->editor()->roles()->first();
+        $resp = $this->putJson($this->baseEndpoint . "/{$role->id}", []);
+        $permissionCount = $role->permissions()->count();
+
+        $resp->assertStatus(200);
+        $this->assertDatabaseHas('roles', [
+            'id'           => $role->id,
+            'display_name' => $role->display_name,
+            'description'  => $role->description,
+            'external_auth_id' => $role->external_auth_id,
+        ]);
+
+        $role->refresh();
+        $this->assertEquals($permissionCount, $role->permissions()->count());
+    }
+
+    public function test_delete_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        $role = $this->users->editor()->roles()->first();
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$role->id}");
+
+        $resp->assertStatus(204);
+        $this->assertActivityExists(ActivityType::ROLE_DELETE, null, $role->logDescriptor());
+    }
+
+    public function test_delete_endpoint_fails_deleting_system_role()
+    {
+        $this->actingAsApiAdmin();
+        $adminRole = Role::getSystemRole('admin');
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$adminRole->id}");
+
+        $resp->assertStatus(500);
+        $resp->assertJson($this->errorResponse('This role is a system role and cannot be deleted', 500));
+    }
+
+    public function test_delete_endpoint_fails_deleting_default_registration_role()
+    {
+        $this->actingAsApiAdmin();
+        $role = $this->users->attachNewRole($this->users->editor());
+        $this->setSettings(['registration-role' => $role->id]);
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$role->id}");
+
+        $resp->assertStatus(500);
+        $resp->assertJson($this->errorResponse('This role cannot be deleted while set as the default registration role', 500));
+    }
+}

tests/Api/UsersApiTest.php

@@ -15,9 +15,9 @@ class UsersApiTest extends TestCase
 {
     use TestsApi;
 
-    protected $baseEndpoint = '/api/users';
+    protected string $baseEndpoint = '/api/users';
 
-    protected $endpointMap = [
+    protected array $endpointMap = [
         ['get', '/api/users'],
         ['post', '/api/users'],
         ['get', '/api/users/1'],
@@ -47,7 +47,7 @@ class UsersApiTest extends TestCase
         }
     }
 
-    public function test_index_endpoint_returns_expected_shelf()
+    public function test_index_endpoint_returns_expected_user()
     {
         $this->actingAsApiAdmin();
         /** @var User $firstUser */

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

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