Merge pull request #3238 from BookStackApp/users_api

User Management API

app/Api/ApiDocsGenerator.php

@@ -3,11 +3,13 @@
 namespace BookStack\Api;
 
 use BookStack\Http\Controllers\Api\ApiController;
+use Exception;
 use Illuminate\Contracts\Container\BindingResolutionException;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Str;
+use Illuminate\Validation\Rules\Password;
 use ReflectionClass;
 use ReflectionException;
 use ReflectionMethod;
@@ -100,11 +102,36 @@ class ApiDocsGenerator
             $this->controllerClasses[$className] = $class;
         }
 
-        $rules = $class->getValdationRules()[$methodName] ?? [];
+        $rules = collect($class->getValidationRules()[$methodName] ?? [])->map(function($validations) {
+            return array_map(function($validation) {
+                return $this->getValidationAsString($validation);
+            }, $validations);
+        })->toArray();
 
         return empty($rules) ? null : $rules;
     }
 
+    /**
+     * Convert the given validation message to a readable string.
+     */
+    protected function getValidationAsString($validation): string
+    {
+        if (is_string($validation)) {
+            return $validation;
+        }
+
+        if (is_object($validation) && method_exists($validation, '__toString')) {
+            return strval($validation);
+        }
+
+        if ($validation instanceof Password) {
+            return 'min:8';
+        }
+
+        $class = get_class($validation);
+        throw new Exception("Cannot provide string representation of rule for class: {$class}");
+    }
+
     /**
      * Parse out the description text from a class method comment.
      */

app/Api/ListingResponseBuilder.php

@@ -2,8 +2,10 @@
 
 namespace BookStack\Api;
 
+use BookStack\Model;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 
 class ListingResponseBuilder
@@ -12,6 +14,11 @@ class ListingResponseBuilder
     protected $request;
     protected $fields;
 
+    /**
+     * @var array<callable>
+     */
+    protected $resultModifiers = [];
+
     protected $filterOperators = [
         'eq'   => '=',
         'ne'   => '!=',
@@ -24,6 +31,7 @@ class ListingResponseBuilder
 
     /**
      * ListingResponseBuilder constructor.
+     * The given fields will be forced visible within the model results.
      */
     public function __construct(Builder $query, Request $request, array $fields)
     {
@@ -35,12 +43,16 @@ class ListingResponseBuilder
     /**
      * Get the response from this builder.
      */
-    public function toResponse()
+    public function toResponse(): JsonResponse
     {
         $filteredQuery = $this->filterQuery($this->query);
 
         $total = $filteredQuery->count();
-        $data = $this->fetchData($filteredQuery);
+        $data = $this->fetchData($filteredQuery)->each(function($model) {
+            foreach ($this->resultModifiers as $modifier) {
+                $modifier($model);
+            }
+        });
 
         return response()->json([
             'data'  => $data,
@@ -49,7 +61,16 @@ class ListingResponseBuilder
     }
 
     /**
-     * Fetch the data to return in the response.
+     * Add a callback to modify each element of the results
+     * @param (callable(Model)) $modifier
+     */
+    public function modifyResults($modifier): void
+    {
+        $this->resultModifiers[] = $modifier;
+    }
+
+    /**
+     * Fetch the data to return within the response.
      */
     protected function fetchData(Builder $query): Collection
     {

app/Auth/Access/Guards/LdapSessionGuard.php

@@ -84,7 +84,7 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
             try {
                 $user = $this->createNewFromLdapAndCreds($userDetails, $credentials);
             } catch (UserRegistrationException $exception) {
-                throw new LoginAttemptException($exception->message);
+                throw new LoginAttemptException($exception->getMessage());
             }
         }
 

app/Auth/Access/RegistrationService.php

@@ -96,7 +96,8 @@ class RegistrationService
         }
 
         // Create the user
-        $newUser = $this->userRepo->registerNew($userData, $emailConfirmed);
+        $newUser = $this->userRepo->createWithoutActivity($userData, $emailConfirmed);
+        $newUser->attachDefaultRole();
 
         // Assign social account if given
         if ($socialAccount) {

app/Auth/Role.php

@@ -28,6 +28,8 @@ class Role extends Model implements Loggable
 
     protected $fillable = ['display_name', 'description', 'external_auth_id'];
 
+    protected $hidden = ['pivot'];
+
     /**
      * 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',
+        'created_at', 'updated_at', 'image_id', 'roles', 'avatar', 'user_id',
     ];
 
     /**

app/Auth/UserRepo.php

@@ -2,30 +2,37 @@
 
 namespace BookStack\Auth;
 
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\UserInviteService;
 use BookStack\Entities\EntityProvider;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Exceptions\NotFoundException;
+use BookStack\Exceptions\NotifyException;
 use BookStack\Exceptions\UserUpdateException;
+use BookStack\Facades\Activity;
 use BookStack\Uploads\UserAvatars;
 use Exception;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Pagination\LengthAwarePaginator;
 use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Str;
 
 class UserRepo
 {
     protected $userAvatar;
+    protected $inviteService;
 
     /**
      * UserRepo constructor.
      */
-    public function __construct(UserAvatars $userAvatar)
+    public function __construct(UserAvatars $userAvatar, UserInviteService $inviteService)
     {
         $this->userAvatar = $userAvatar;
+        $this->inviteService = $inviteService;
     }
 
     /**
@@ -53,11 +60,13 @@ class UserRepo
     }
 
     /**
-     * Get all the users with their permissions.
+     * Get all users as Builder for API
      */
-    public function getAllUsers(): Collection
+    public function getApiUsersBuilder(): Builder
     {
-        return User::query()->with('roles', 'avatar')->orderBy('name', 'asc')->get();
+        return User::query()->select(['*'])
+            ->scopes('withLastActivityAt')
+            ->with(['avatar']);
     }
 
     /**
@@ -87,18 +96,6 @@ class UserRepo
         return $query->paginate($count);
     }
 
-    /**
-     * Creates a new user and attaches a role to them.
-     */
-    public function registerNew(array $data, bool $emailConfirmed = false): User
-    {
-        $user = $this->create($data, $emailConfirmed);
-        $user->attachDefaultRole();
-        $this->downloadAndAssignUserAvatar($user);
-
-        return $user;
-    }
-
     /**
      * Assign a user to a system-level role.
      *
@@ -161,23 +158,85 @@ class UserRepo
     }
 
     /**
-     * Create a new basic instance of user.
+     * Create a new basic instance of user with the given pre-validated data.
+     * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
      */
-    public function create(array $data, bool $emailConfirmed = false): User
+    public function createWithoutActivity(array $data, bool $emailConfirmed = false): User
     {
-        $details = [
-            'name'             => $data['name'],
-            'email'            => $data['email'],
-            'password'         => bcrypt($data['password']),
-            'email_confirmed'  => $emailConfirmed,
-            'external_auth_id' => $data['external_auth_id'] ?? '',
-        ];
-
         $user = new User();
-        $user->forceFill($details);
+        $user->name = $data['name'];
+        $user->email = $data['email'];
+        $user->password = bcrypt(empty($data['password']) ? Str::random(32) : $data['password']);
+        $user->email_confirmed = $emailConfirmed;
+        $user->external_auth_id = $data['external_auth_id'] ?? '';
+
         $user->refreshSlug();
         $user->save();
 
+        if (!empty($data['language'])) {
+            setting()->putUser($user, 'language', $data['language']);
+        }
+
+        if (isset($data['roles'])) {
+            $this->setUserRoles($user, $data['roles']);
+        }
+
+        $this->downloadAndAssignUserAvatar($user);
+
+        return $user;
+    }
+
+    /**
+     * As per "createWithoutActivity" but records a "create" activity.
+     * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
+     */
+    public function create(array $data, bool $sendInvite = false): User
+    {
+        $user = $this->createWithoutActivity($data, true);
+
+        if ($sendInvite) {
+            $this->inviteService->sendInvitation($user);
+        }
+
+        Activity::add(ActivityType::USER_CREATE, $user);
+        return $user;
+    }
+
+    /**
+     * Update the given user with the given data.
+     * @param array{name: ?string, email: ?string, external_auth_id: ?string, password: ?string, roles: ?array<int>, language: ?string} $data
+     * @throws UserUpdateException
+     */
+    public function update(User $user, array $data, bool $manageUsersAllowed): User
+    {
+        if (!empty($data['name'])) {
+            $user->name = $data['name'];
+            $user->refreshSlug();
+        }
+
+        if (!empty($data['email']) && $manageUsersAllowed) {
+            $user->email = $data['email'];
+        }
+
+        if (!empty($data['external_auth_id']) && $manageUsersAllowed) {
+            $user->external_auth_id = $data['external_auth_id'];
+        }
+
+        if (isset($data['roles']) && $manageUsersAllowed) {
+            $this->setUserRoles($user, $data['roles']);
+        }
+
+        if (!empty($data['password'])) {
+            $user->password = bcrypt($data['password']);
+        }
+
+        if (!empty($data['language'])) {
+            setting()->putUser($user, 'language', $data['language']);
+        }
+
+        $user->save();
+        Activity::add(ActivityType::USER_UPDATE, $user);
+
         return $user;
     }
 
@@ -188,6 +247,8 @@ class UserRepo
      */
     public function destroy(User $user, ?int $newOwnerId = null)
     {
+        $this->ensureDeletable($user);
+
         $user->socialAccounts()->delete();
         $user->apiTokens()->delete();
         $user->favourites()->delete();
@@ -203,6 +264,22 @@ class UserRepo
                 $this->migrateOwnership($user, $newOwner);
             }
         }
+
+        Activity::add(ActivityType::USER_DELETE, $user);
+    }
+
+    /**
+     * @throws NotifyException
+     */
+    protected function ensureDeletable(User $user): void
+    {
+        if ($this->isOnlyAdmin($user)) {
+            throw new NotifyException(trans('errors.users_cannot_delete_only_admin'), $user->getEditUrl());
+        }
+
+        if ($user->system_name === 'public') {
+            throw new NotifyException(trans('errors.users_cannot_delete_guest'), $user->getEditUrl());
+        }
     }
 
     /**

app/Console/Commands/CreateAdmin.php

@@ -84,9 +84,8 @@ class CreateAdmin extends Command
             return SymfonyCommand::FAILURE;
         }
 
-        $user = $this->userRepo->create($validator->validated());
+        $user = $this->userRepo->createWithoutActivity($validator->validated());
         $this->userRepo->attachSystemRole($user, 'admin');
-        $this->userRepo->downloadAndAssignUserAvatar($user);
         $user->email_confirmed = true;
         $user->save();
 

app/Console/Commands/DeleteUsers.php

@@ -15,8 +15,6 @@ class DeleteUsers extends Command
      */
     protected $signature = 'bookstack:delete-users';
 
-    protected $user;
-
     protected $userRepo;
 
     /**
@@ -26,9 +24,8 @@ class DeleteUsers extends Command
      */
     protected $description = 'Delete users that are not "admin" or system users';
 
-    public function __construct(User $user, UserRepo $userRepo)
+    public function __construct(UserRepo $userRepo)
     {
-        $this->user = $user;
         $this->userRepo = $userRepo;
         parent::__construct();
     }
@@ -38,8 +35,8 @@ class DeleteUsers extends Command
         $confirm = $this->ask('This will delete all users from the system that are not "admin" or system users. Are you sure you want to continue? (Type "yes" to continue)');
         $numDeleted = 0;
         if (strtolower(trim($confirm)) === 'yes') {
-            $totalUsers = $this->user->count();
+            $totalUsers = User::query()->count();
-            $users = $this->user->where('system_name', '=', null)->with('roles')->get();
+            $users = User::query()->whereNull('system_name')->with('roles')->get();
             foreach ($users as $user) {
                 if ($user->hasSystemRole('admin')) {
                     // don't delete users with "admin" role

app/Exceptions/Handler.php

@@ -101,6 +101,10 @@ class Handler extends ExceptionHandler
             $code = $e->status;
         }
 
+        if (method_exists($e, 'getStatus')) {
+            $code = $e->getStatus();
+        }
+
         $responseData['error']['code'] = $code;
 
         return new JsonResponse($responseData, $code, $headers);

app/Exceptions/NotifyException.php

@@ -9,17 +9,27 @@ class NotifyException extends Exception implements Responsable
 {
     public $message;
     public $redirectLocation;
+    protected $status;
 
     /**
      * NotifyException constructor.
      */
-    public function __construct(string $message, string $redirectLocation = '/')
+    public function __construct(string $message, string $redirectLocation = '/', int $status = 500)
     {
         $this->message = $message;
         $this->redirectLocation = $redirectLocation;
+        $this->status = $status;
         parent::__construct();
     }
 
+    /**
+     * Get the desired status code for this exception.
+     */
+    public function getStatus(): int
+    {
+        return $this->status;
+    }
+
     /**
      * Send the response for this type of exception.
      *
@@ -29,6 +39,11 @@ class NotifyException extends Exception implements Responsable
     {
         $message = $this->getMessage();
 
+        // Front-end JSON handling. API-side handling managed via handler.
+        if ($request->wantsJson()) {
+            return response()->json(['error' => $message], 403);
+        }
+
         if (!empty($message)) {
             session()->flash('error', $message);
         }

app/Http/Controllers/Api/ApiController.php

@@ -15,10 +15,14 @@ abstract class ApiController extends Controller
      * Provide a paginated listing JSON response in a standard format
      * taking into account any pagination parameters passed by the user.
      */
-    protected function apiListingResponse(Builder $query, array $fields): JsonResponse
+    protected function apiListingResponse(Builder $query, array $fields, array $modifiers = []): JsonResponse
     {
         $listing = new ListingResponseBuilder($query, request(), $fields);
 
+        foreach ($modifiers as $modifier) {
+            $listing->modifyResults($modifier);
+        }
+
         return $listing->toResponse();
     }
 
@@ -26,7 +30,7 @@ abstract class ApiController extends Controller
      * Get the validation rules for this controller.
      * Defaults to a $rules property but can be a rules() method.
      */
-    public function getValdationRules(): array
+    public function getValidationRules(): array
     {
         if (method_exists($this, 'rules')) {
             return $this->rules();

app/Http/Controllers/Api/UserApiController.php

@@ -0,0 +1,164 @@
+<?php
+
+namespace BookStack\Http\Controllers\Api;
+
+use BookStack\Auth\User;
+use BookStack\Auth\UserRepo;
+use BookStack\Exceptions\UserUpdateException;
+use Closure;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Validation\Rules\Password;
+use Illuminate\Validation\Rules\Unique;
+
+class UserApiController extends ApiController
+{
+    protected $userRepo;
+
+    protected $fieldsToExpose = [
+        'email', 'created_at', 'updated_at', 'last_activity_at', 'external_auth_id'
+    ];
+
+    public function __construct(UserRepo $userRepo)
+    {
+        $this->userRepo = $userRepo;
+
+        // Checks for all endpoints in this controller
+        $this->middleware(function ($request, $next) {
+            $this->checkPermission('users-manage');
+            $this->preventAccessInDemoMode();
+            return $next($request);
+        });
+    }
+
+    protected function rules(int $userId = null): array
+    {
+        return [
+            'create' => [
+                'name' => ['required', 'min:2'],
+                'email' => [
+                    'required', 'min:2', 'email', new Unique('users', 'email')
+                ],
+                'external_auth_id' => ['string'],
+                'language' => ['string'],
+                'password' => [Password::default()],
+                'roles' => ['array'],
+                'roles.*' => ['integer'],
+                'send_invite' => ['boolean'],
+            ],
+            'update' => [
+                'name' => ['min:2'],
+                'email' => [
+                    'min:2',
+                    'email',
+                    (new Unique('users', 'email'))->ignore($userId ?? null)
+                ],
+                'external_auth_id' => ['string'],
+                'language' => ['string'],
+                'password' => [Password::default()],
+                'roles' => ['array'],
+                'roles.*' => ['integer'],
+            ],
+            'delete' => [
+                'migrate_ownership_id' => ['integer', 'exists:users,id'],
+            ],
+        ];
+    }
+
+    /**
+     * Get a listing of users in the system.
+     * Requires permission to manage users.
+     */
+    public function list()
+    {
+        $users = $this->userRepo->getApiUsersBuilder();
+
+        return $this->apiListingResponse($users, [
+            'id', 'name', 'slug', 'email', 'external_auth_id',
+            'created_at', 'updated_at', 'last_activity_at',
+        ], [Closure::fromCallable([$this, 'listFormatter'])]);
+    }
+
+    /**
+     * Create a new user in the system.
+     * Requires permission to manage users.
+     */
+    public function create(Request $request)
+    {
+        $data = $this->validate($request, $this->rules()['create']);
+        $sendInvite = ($data['send_invite'] ?? false) === true;
+
+        $user = null;
+        DB::transaction(function () use ($data, $sendInvite, &$user) {
+            $user = $this->userRepo->create($data, $sendInvite);
+        });
+
+        $this->singleFormatter($user);
+
+        return response()->json($user);
+    }
+
+    /**
+     * View the details of a single user.
+     * Requires permission to manage users.
+     */
+    public function read(string $id)
+    {
+        $user = $this->userRepo->getById($id);
+        $this->singleFormatter($user);
+
+        return response()->json($user);
+    }
+
+    /**
+     * Update an existing user in the system.
+     * Requires permission to manage users.
+     * @throws UserUpdateException
+     */
+    public function update(Request $request, string $id)
+    {
+        $data = $this->validate($request, $this->rules($id)['update']);
+        $user = $this->userRepo->getById($id);
+        $this->userRepo->update($user, $data, userCan('users-manage'));
+        $this->singleFormatter($user);
+
+        return response()->json($user);
+    }
+
+    /**
+     * 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 users.
+     */
+    public function delete(Request $request, string $id)
+    {
+        $user = $this->userRepo->getById($id);
+        $newOwnerId = $request->get('migrate_ownership_id', null);
+
+        $this->userRepo->destroy($user, $newOwnerId);
+
+        return response('', 204);
+    }
+
+    /**
+     * Format the given user model for single-result display.
+     */
+    protected function singleFormatter(User $user)
+    {
+        $this->listFormatter($user);
+        $user->load('roles:id,display_name');
+        $user->makeVisible(['roles']);
+    }
+
+    /**
+     * Format the given user model for a listing multi-result display.
+     */
+    protected function listFormatter(User $user)
+    {
+        $user->makeVisible($this->fieldsToExpose);
+        $user->setAttribute('profile_url', $user->getProfileUrl());
+        $user->setAttribute('edit_url', $user->getEditUrl());
+        $user->setAttribute('avatar_url', $user->getAvatar());
+    }
+}

app/Http/Controllers/Controller.php

@@ -2,13 +2,13 @@
 
 namespace BookStack\Http\Controllers;
 
+use BookStack\Exceptions\NotifyException;
 use BookStack\Facades\Activity;
 use BookStack\Interfaces\Loggable;
 use BookStack\Model;
 use BookStack\Util\WebSafeMimeSniffer;
 use Illuminate\Foundation\Bus\DispatchesJobs;
 use Illuminate\Foundation\Validation\ValidatesRequests;
-use Illuminate\Http\Exceptions\HttpResponseException;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Response;
 use Illuminate\Routing\Controller as BaseController;
@@ -53,14 +53,8 @@ abstract class Controller extends BaseController
      */
     protected function showPermissionError()
     {
-        if (request()->wantsJson()) {
+        $message = request()->wantsJson() ? trans('errors.permissionJson') : trans('errors.permission');
-            $response = response()->json(['error' => trans('errors.permissionJson')], 403);
+        throw new NotifyException($message, '/', 403);
-        } else {
-            $response = redirect('/');
-            $this->showErrorNotification(trans('errors.permission'));
-        }
-
-        throw new HttpResponseException($response);
     }
 
     /**

app/Http/Controllers/UserController.php

@@ -2,9 +2,7 @@
 
 namespace BookStack\Http\Controllers;
 
-use BookStack\Actions\ActivityType;
 use BookStack\Auth\Access\SocialAuthService;
-use BookStack\Auth\Access\UserInviteService;
 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\ImageUploadException;
@@ -13,25 +11,20 @@ use BookStack\Uploads\ImageRepo;
 use Exception;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\DB;
-use Illuminate\Support\Str;
 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\ValidationException;
 
 class UserController extends Controller
 {
-    protected $user;
     protected $userRepo;
-    protected $inviteService;
     protected $imageRepo;
 
     /**
      * UserController constructor.
      */
-    public function __construct(User $user, UserRepo $userRepo, UserInviteService $inviteService, ImageRepo $imageRepo)
+    public function __construct(UserRepo $userRepo, ImageRepo $imageRepo)
     {
-        $this->user = $user;
         $this->userRepo = $userRepo;
-        $this->inviteService = $inviteService;
         $this->imageRepo = $imageRepo;
     }
 
@@ -68,63 +61,34 @@ class UserController extends Controller
     }
 
     /**
-     * Store a newly created user in storage.
+     * Store a new user in storage.
      *
-     * @throws UserUpdateException
      * @throws ValidationException
      */
     public function store(Request $request)
     {
         $this->checkPermission('users-manage');
-        $validationRules = [
-            'name'    => ['required'],
-            'email'   => ['required', 'email', 'unique:users,email'],
-            'setting' => ['array'],
-        ];
 
         $authMethod = config('auth.method');
         $sendInvite = ($request->get('send_invite', 'false') === 'true');
+        $externalAuth = $authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'oidc';
+        $passwordRequired = ($authMethod === 'standard' && !$sendInvite);
 
-        if ($authMethod === 'standard' && !$sendInvite) {
+        $validationRules = [
-            $validationRules['password'] = ['required', Password::default()];
+            'name'    => ['required'],
-            $validationRules['password-confirm'] = ['required', 'same:password'];
+            'email'   => ['required', 'email', 'unique:users,email'],
-        } elseif ($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') {
+            'language' => ['string'],
-            $validationRules['external_auth_id'] = ['required'];
+            'roles'            => ['array'],
-        }
+            'roles.*'          => ['integer'],
-        $this->validate($request, $validationRules);
+            'password' => $passwordRequired ? ['required', Password::default()] : null,
+            'password-confirm' => $passwordRequired ? ['required', 'same:password'] : null,
+            'external_auth_id' => $externalAuth ? ['required'] : null,
+        ];
 
-        $user = $this->user->fill($request->all());
+        $validated = $this->validate($request, array_filter($validationRules));
 
-        if ($authMethod === 'standard') {
+        DB::transaction(function () use ($validated, $sendInvite) {
-            $user->password = bcrypt($request->get('password', Str::random(32)));
+            $this->userRepo->create($validated, $sendInvite);
-        } elseif ($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') {
-            $user->external_auth_id = $request->get('external_auth_id');
-        }
-
-        $user->refreshSlug();
-
-        DB::transaction(function () use ($user, $sendInvite, $request) {
-            $user->save();
-
-            // Save user-specific settings
-            if ($request->filled('setting')) {
-                foreach ($request->get('setting') as $key => $value) {
-                    setting()->putUser($user, $key, $value);
-                }
-            }
-
-            if ($sendInvite) {
-                $this->inviteService->sendInvitation($user);
-            }
-
-            if ($request->filled('roles')) {
-                $roles = $request->get('roles');
-                $this->userRepo->setUserRoles($user, $roles);
-            }
-
-            $this->userRepo->downloadAndAssignUserAvatar($user);
-
-            $this->logActivity(ActivityType::USER_CREATE, $user);
         });
 
         return redirect('/settings/users');
@@ -138,7 +102,7 @@ class UserController extends Controller
         $this->checkPermissionOrCurrentUser('users-manage', $id);
 
         /** @var User $user */
-        $user = $this->user->newQuery()->with(['apiTokens', 'mfaValues'])->findOrFail($id);
+        $user = User::query()->with(['apiTokens', 'mfaValues'])->findOrFail($id);
 
         $authMethod = ($user->system_name) ? 'system' : config('auth.method');
 
@@ -168,51 +132,20 @@ class UserController extends Controller
         $this->preventAccessInDemoMode();
         $this->checkPermissionOrCurrentUser('users-manage', $id);
 
-        $this->validate($request, [
+        $validated = $this->validate($request, [
             'name'             => ['min:2'],
             'email'            => ['min:2', 'email', 'unique:users,email,' . $id],
             'password'         => ['required_with:password_confirm', Password::default()],
             'password-confirm' => ['same:password', 'required_with:password'],
-            'setting'          => ['array'],
+            'language'         => ['string'],
+            'roles'            => ['array'],
+            'roles.*'          => ['integer'],
+            'external_auth_id' => ['string'],
             'profile_image'    => array_merge(['nullable'], $this->getImageValidationRules()),
         ]);
 
         $user = $this->userRepo->getById($id);
-        $user->fill($request->except(['email']));
+        $this->userRepo->update($user, $validated, userCan('users-manage'));
-
-        // Email updates
-        if (userCan('users-manage') && $request->filled('email')) {
-            $user->email = $request->get('email');
-        }
-
-        // Refresh the slug if the user's name has changed
-        if ($user->isDirty('name')) {
-            $user->refreshSlug();
-        }
-
-        // Role updates
-        if (userCan('users-manage') && $request->filled('roles')) {
-            $roles = $request->get('roles');
-            $this->userRepo->setUserRoles($user, $roles);
-        }
-
-        // Password updates
-        if ($request->filled('password')) {
-            $password = $request->get('password');
-            $user->password = bcrypt($password);
-        }
-
-        // External auth id updates
-        if (user()->can('users-manage') && $request->filled('external_auth_id')) {
-            $user->external_auth_id = $request->get('external_auth_id');
-        }
-
-        // Save user-specific settings
-        if ($request->filled('setting')) {
-            foreach ($request->get('setting') as $key => $value) {
-                setting()->putUser($user, $key, $value);
-            }
-        }
 
         // Save profile image if in request
         if ($request->hasFile('profile_image')) {
@@ -220,6 +153,7 @@ class UserController extends Controller
             $this->imageRepo->destroyImage($user->avatar);
             $image = $this->imageRepo->saveNew($imageUpload, 'user', $user->id);
             $user->image_id = $image->id;
+            $user->save();
         }
 
         // Delete the profile image if reset option is in request
@@ -227,11 +161,7 @@ class UserController extends Controller
             $this->imageRepo->destroyImage($user->avatar);
         }
 
-        $user->save();
+        $redirectUrl = userCan('users-manage') ? '/settings/users' : "/settings/users/{$user->id}";
-        $this->showSuccessNotification(trans('settings.users_edit_success'));
-        $this->logActivity(ActivityType::USER_UPDATE, $user);
-
-        $redirectUrl = userCan('users-manage') ? '/settings/users' : ('/settings/users/' . $user->id);
 
         return redirect($redirectUrl);
     }
@@ -262,21 +192,7 @@ class UserController extends Controller
         $user = $this->userRepo->getById($id);
         $newOwnerId = $request->get('new_owner_id', null);
 
-        if ($this->userRepo->isOnlyAdmin($user)) {
-            $this->showErrorNotification(trans('errors.users_cannot_delete_only_admin'));
-
-            return redirect($user->getEditUrl());
-        }
-
-        if ($user->system_name === 'public') {
-            $this->showErrorNotification(trans('errors.users_cannot_delete_guest'));
-
-            return redirect($user->getEditUrl());
-        }
-
         $this->userRepo->destroy($user, $newOwnerId);
-        $this->showSuccessNotification(trans('settings.users_delete_success'));
-        $this->logActivity(ActivityType::USER_DELETE, $user);
 
         return redirect('/settings/users');
     }
@@ -361,7 +277,7 @@ class UserController extends Controller
 
         $newState = $request->get('expand', 'false');
 
-        $user = $this->user->findOrFail($id);
+        $user = $this->userRepo->getById($id);
         setting()->putUser($user, 'section_expansion#' . $key, $newState);
 
         return response('', 204);
@@ -384,7 +300,7 @@ class UserController extends Controller
             $order = 'asc';
         }
 
-        $user = $this->user->findOrFail($userId);
+        $user = $this->userRepo->getById($userId);
         $sortKey = $listName . '_sort';
         $orderKey = $listName . '_sort_order';
         setting()->putUser($user, $sortKey, $sort);

app/Providers/AuthServiceProvider.php

@@ -23,6 +23,7 @@ class AuthServiceProvider extends ServiceProvider
     public function boot()
     {
         // Password Configuration
+        // Changes here must be reflected in ApiDocsGenerate@getValidationAsString.
         Password::defaults(function () {
             return Password::min(8);
         });

dev/api/requests/users-create.json

@@ -0,0 +1,7 @@
+{
+  "name": "Dan Brown",
+  "email": "dannyb@example.com",
+  "roles": [1],
+  "language": "fr",
+  "send_invite": true
+}

dev/api/requests/users-delete.json

@@ -0,0 +1,3 @@
+{
+  "migrate_ownership_id": 5
+}

dev/api/requests/users-update.json

@@ -0,0 +1,7 @@
+{
+  "name": "Dan Spaggleforth",
+  "email": "dspaggles@example.com",
+  "roles": [2],
+  "language": "de",
+  "password": "hunter2000"
+}

dev/api/responses/users-create.json

@@ -0,0 +1,19 @@
+{
+  "id": 1,
+  "name": "Dan Brown",
+  "email": "dannyb@example.com",
+  "created_at": "2022-02-03T16:27:55.000000Z",
+  "updated_at": "2022-02-03T16:27:55.000000Z",
+  "external_auth_id": "abc123456",
+  "slug": "dan-brown",
+  "last_activity_at": "2022-02-03T16:27:55.000000Z",
+  "profile_url": "https://docs.example.com/user/dan-brown",
+  "edit_url": "https://docs.example.com/settings/users/1",
+  "avatar_url": "https://docs.example.com/uploads/images/user/2021-10/thumbs-50-50/profile-2021.jpg",
+  "roles": [
+    {
+      "id": 1,
+      "display_name": "Admin"
+    }
+  ]
+}

dev/api/responses/users-list.json

@@ -0,0 +1,33 @@
+{
+  "data": [
+    {
+      "id": 1,
+      "name": "Dan Brown",
+      "email": "dannyb@example.com",
+      "created_at": "2022-02-03T16:27:55.000000Z",
+      "updated_at": "2022-02-03T16:27:55.000000Z",
+      "external_auth_id": "abc123456",
+      "slug": "dan-brown",
+      "user_id": 1,
+      "last_activity_at": "2022-02-03T16:27:55.000000Z",
+      "profile_url": "https://docs.example.com/user/dan-brown",
+      "edit_url": "https://docs.example.com/settings/users/1",
+      "avatar_url": "https://docs.example.com/uploads/images/user/2021-10/thumbs-50-50/profile-2021.jpg"
+    },
+    {
+      "id": 2,
+      "name": "Benny",
+      "email": "benny@example.com",
+      "created_at": "2022-01-31T20:39:24.000000Z",
+      "updated_at": "2021-11-18T17:10:58.000000Z",
+      "external_auth_id": "",
+      "slug": "benny",
+      "user_id": 2,
+      "last_activity_at": "2022-01-31T20:39:24.000000Z",
+      "profile_url": "https://docs.example.com/user/benny",
+      "edit_url": "https://docs.example.com/settings/users/2",
+      "avatar_url": "https://docs.example.com/uploads/images/user/2021-11/thumbs-50-50/guest.jpg"
+    }
+  ],
+  "total": 28
+}

dev/api/responses/users-read.json

@@ -0,0 +1,19 @@
+{
+  "id": 1,
+  "name": "Dan Brown",
+  "email": "dannyb@example.com",
+  "created_at": "2022-02-03T16:27:55.000000Z",
+  "updated_at": "2022-02-03T16:27:55.000000Z",
+  "external_auth_id": "abc123456",
+  "slug": "dan-brown",
+  "last_activity_at": "2022-02-03T16:27:55.000000Z",
+  "profile_url": "https://docs.example.com/user/dan-brown",
+  "edit_url": "https://docs.example.com/settings/users/1",
+  "avatar_url": "https://docs.example.com/uploads/images/user/2021-10/thumbs-50-50/profile-2021.jpg",
+  "roles": [
+    {
+      "id": 1,
+      "display_name": "Admin"
+    }
+  ]
+}

dev/api/responses/users-update.json

@@ -0,0 +1,19 @@
+{
+  "id": 1,
+  "name": "Dan Spaggleforth",
+  "email": "dspaggles@example.com",
+  "created_at": "2022-02-03T16:27:55.000000Z",
+  "updated_at": "2022-02-03T16:27:55.000000Z",
+  "external_auth_id": "abc123456",
+  "slug": "dan-spaggleforth",
+  "last_activity_at": "2022-02-03T16:27:55.000000Z",
+  "profile_url": "https://docs.example.com/user/dan-spaggleforth",
+  "edit_url": "https://docs.example.com/settings/users/1",
+  "avatar_url": "https://docs.example.com/uploads/images/user/2021-10/thumbs-50-50/profile-2021.jpg",
+  "roles": [
+    {
+      "id": 2,
+      "display_name": "Editors"
+    }
+  ]
+}

resources/lang/en/activities.php

@@ -59,6 +59,10 @@ return [
     'webhook_delete' => 'deleted webhook',
     'webhook_delete_notification' => 'Webhook successfully deleted',
 
+    // Users
+    'user_update_notification' => 'User successfully updated',
+    'user_delete_notification' => 'User successfully removed',
+
     // Other
     'commented_on'                => 'commented on',
     'permissions_update'          => 'updated permissions',

resources/lang/en/settings.php

@@ -188,10 +188,8 @@ return [
     'users_migrate_ownership' => 'Migrate Ownership',
     'users_migrate_ownership_desc' => 'Select a user here if you want another user to become the owner of all items currently owned by this user.',
     'users_none_selected' => 'No user selected',
-    'users_delete_success' => 'User successfully removed',
     'users_edit' => 'Edit User',
     'users_edit_profile' => 'Edit Profile',
-    'users_edit_success' => 'User successfully updated',
     'users_avatar' => 'User Avatar',
     'users_avatar_desc' => 'Select an image to represent this user. This should be approx 256px square.',
     'users_preferred_language' => 'Preferred Language',

resources/views/users/parts/language-option-row.blade.php

@@ -9,7 +9,7 @@ $value - Currently selected lanuage value
         </p>
     </div>
     <div>
-        <select name="setting[language]" id="user-language">
+        <select name="language" id="user-language">
             @foreach(trans('settings.language_select') as $lang => $label)
                 <option @if($value === $lang) selected @endif value="{{ $lang }}">{{ $label }}</option>
             @endforeach

routes/api.php

@@ -10,6 +10,7 @@ use BookStack\Http\Controllers\Api\ChapterExportApiController;
 use BookStack\Http\Controllers\Api\PageApiController;
 use BookStack\Http\Controllers\Api\PageExportApiController;
 use BookStack\Http\Controllers\Api\SearchApiController;
+use BookStack\Http\Controllers\Api\UserApiController;
 use Illuminate\Support\Facades\Route;
 
 /**
@@ -65,3 +66,9 @@ Route::post('shelves', [BookshelfApiController::class, 'create']);
 Route::get('shelves/{id}', [BookshelfApiController::class, 'read']);
 Route::put('shelves/{id}', [BookshelfApiController::class, 'update']);
 Route::delete('shelves/{id}', [BookshelfApiController::class, 'delete']);
+
+Route::get('users', [UserApiController::class, 'list']);
+Route::post('users', [UserApiController::class, 'create']);
+Route::get('users/{id}', [UserApiController::class, 'read']);
+Route::put('users/{id}', [UserApiController::class, 'update']);
+Route::delete('users/{id}', [UserApiController::class, 'delete']);

tests/Api/TestsApi.php

@@ -35,6 +35,14 @@ trait TestsApi
         return ['error' => ['code' => $code, 'message' => $message]];
     }
 
+    /**
+     * Get the structure that matches a permission error response.
+     */
+    protected function permissionErrorResponse(): array
+    {
+        return $this->errorResponse('You do not have permission to perform the requested action.', 403);
+    }
+
     /**
      * Format the given (field_name => ["messages"]) array
      * into a standard validation response format.

tests/Api/UsersApiTest.php

@@ -0,0 +1,286 @@
+<?php
+
+namespace Tests\Api;
+
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Role;
+use BookStack\Auth\User;
+use BookStack\Entities\Models\Entity;
+use BookStack\Notifications\UserInvite;
+use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Notification;
+use Tests\TestCase;
+
+class UsersApiTest extends TestCase
+{
+    use TestsApi;
+
+    protected $baseEndpoint = '/api/users';
+
+    protected $endpointMap = [
+        ['get', '/api/users'],
+        ['post', '/api/users'],
+        ['get', '/api/users/1'],
+        ['put', '/api/users/1'],
+        ['delete', '/api/users/1'],
+    ];
+
+    public function test_users_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_no_endpoints_accessible_in_demo_mode()
+    {
+        config()->set('app.env', 'demo');
+        $this->actingAsApiAdmin();
+
+        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_shelf()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $firstUser */
+        $firstUser = User::query()->orderBy('id', 'asc')->first();
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJson(['data' => [
+            [
+                'id'   => $firstUser->id,
+                'name' => $firstUser->name,
+                'slug' => $firstUser->slug,
+                'email' => $firstUser->email,
+                'profile_url' => $firstUser->getProfileUrl(),
+                'edit_url' => $firstUser->getEditUrl(),
+                'avatar_url' => $firstUser->getAvatar(),
+            ],
+        ]]);
+    }
+
+    public function test_create_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Role $role */
+        $role = Role::query()->first();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'name' => 'Benny Boris',
+            'email' => 'bboris@example.com',
+            'password' => 'mysuperpass',
+            'language' => 'it',
+            'roles' => [$role->id],
+            'send_invite' => false,
+        ]);
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'name' => 'Benny Boris',
+            'email' => 'bboris@example.com',
+            'external_auth_id' => '',
+            'roles' => [
+                [
+                    'id' => $role->id,
+                    'display_name' => $role->display_name,
+                ]
+            ],
+        ]);
+        $this->assertDatabaseHas('users', ['email' => 'bboris@example.com']);
+
+        /** @var User $user */
+        $user = User::query()->where('email', '=', 'bboris@example.com')->first();
+        $this->assertActivityExists(ActivityType::USER_CREATE, null, $user->logDescriptor());
+        $this->assertEquals(1, $user->roles()->count());
+        $this->assertEquals('it', setting()->getUser($user, 'language'));
+    }
+
+    public function test_create_with_send_invite()
+    {
+        $this->actingAsApiAdmin();
+        Notification::fake();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'name' => 'Benny Boris',
+            'email' => 'bboris@example.com',
+            'send_invite' => true,
+        ]);
+
+        $resp->assertStatus(200);
+        /** @var User $user */
+        $user = User::query()->where('email', '=', 'bboris@example.com')->first();
+        Notification::assertSentTo($user, UserInvite::class);
+    }
+
+    public function test_create_name_and_email_validation()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $existingUser */
+        $existingUser = User::query()->first();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'email' => 'bboris@example.com',
+        ]);
+        $resp->assertStatus(422);
+        $resp->assertJson($this->validationResponse(['name' => ['The name field is required.']]));
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'name' => 'Benny Boris',
+        ]);
+        $resp->assertStatus(422);
+        $resp->assertJson($this->validationResponse(['email' => ['The email field is required.']]));
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'email' => $existingUser->email,
+            'name' => 'Benny Boris',
+        ]);
+        $resp->assertStatus(422);
+        $resp->assertJson($this->validationResponse(['email' => ['The email has already been taken.']]));
+    }
+
+    public function test_read_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = User::query()->first();
+        /** @var Role $userRole */
+        $userRole = $user->roles()->first();
+
+        $resp = $this->getJson($this->baseEndpoint . "/{$user->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id'         => $user->id,
+            'slug'       => $user->slug,
+            'email'      => $user->email,
+            'external_auth_id' => $user->external_auth_id,
+            'roles' => [
+                [
+                    'id' => $userRole->id,
+                    'display_name' => $userRole->display_name,
+                ]
+            ],
+        ]);
+    }
+
+    public function test_update_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = $this->getAdmin();
+        $roles = Role::query()->pluck('id');
+        $resp = $this->putJson($this->baseEndpoint . "/{$user->id}", [
+            'name' => 'My updated user',
+            'email' => 'barrytest@example.com',
+            'roles' => $roles,
+            'external_auth_id' => 'btest',
+            'password' => 'barrytester',
+            'language' => 'fr',
+        ]);
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id' => $user->id,
+            'name' => 'My updated user',
+            'email' => 'barrytest@example.com',
+            'external_auth_id' => 'btest',
+        ]);
+        $user->refresh();
+        $this->assertEquals('fr', setting()->getUser($user, 'language'));
+        $this->assertEquals(count($roles), $user->roles()->count());
+        $this->assertNotEquals('barrytester', $user->password);
+        $this->assertTrue(Hash::check('barrytester', $user->password));
+    }
+
+    public function test_update_endpoint_does_not_remove_info_if_not_provided()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = $this->getAdmin();
+        $roleCount = $user->roles()->count();
+        $resp = $this->putJson($this->baseEndpoint . "/{$user->id}", []);
+
+        $resp->assertStatus(200);
+        $this->assertDatabaseHas('users', [
+            'id' => $user->id,
+            'name' => $user->name,
+            'email' => $user->email,
+            'password' => $user->password,
+        ]);
+        $this->assertEquals($roleCount, $user->roles()->count());
+    }
+
+    public function test_delete_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = User::query()->where('id', '!=', $this->getAdmin()->id)
+            ->whereNull('system_name')
+            ->first();
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$user->id}");
+
+        $resp->assertStatus(204);
+        $this->assertActivityExists('user_delete', null, $user->logDescriptor());
+    }
+
+    public function test_delete_endpoint_with_ownership_migration_user()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = User::query()->where('id', '!=', $this->getAdmin()->id)
+            ->whereNull('system_name')
+            ->first();
+        $entityChain = $this->createEntityChainBelongingToUser($user);
+        /** @var User $newOwner */
+        $newOwner = User::query()->where('id', '!=', $user->id)->first();
+
+        /** @var Entity $entity */
+        foreach ($entityChain as $entity) {
+            $this->assertEquals($user->id, $entity->owned_by);
+        }
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$user->id}", [
+            'migrate_ownership_id' => $newOwner->id,
+        ]);
+
+        $resp->assertStatus(204);
+        /** @var Entity $entity */
+        foreach ($entityChain as $entity) {
+            $this->assertEquals($newOwner->id, $entity->refresh()->owned_by);
+        }
+    }
+
+    public function test_delete_endpoint_fails_deleting_only_admin()
+    {
+        $this->actingAsApiAdmin();
+        $adminRole = Role::getSystemRole('admin');
+        $adminToDelete = $adminRole->users()->first();
+        $adminRole->users()->where('id', '!=', $adminToDelete->id)->delete();
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$adminToDelete->id}");
+
+        $resp->assertStatus(500);
+        $resp->assertJson($this->errorResponse('You cannot delete the only admin', 500));
+    }
+
+    public function test_delete_endpoint_fails_deleting_public_user()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $publicUser */
+        $publicUser = User::query()->where('system_name', '=', 'public')->first();
+
+        $resp = $this->deleteJson($this->baseEndpoint . "/{$publicUser->id}");
+
+        $resp->assertStatus(500);
+        $resp->assertJson($this->errorResponse('You cannot delete the guest user', 500));
+    }
+}

tests/Auth/UserInviteTest.php

@@ -45,9 +45,7 @@ class UserInviteTest extends TestCase
             'name'        => 'Barry',
             'email'       => $email,
             'send_invite' => 'true',
-            'setting'     => [
             'language'    => 'de',
-            ],
         ]);
         $resp->assertRedirect('/settings/users');
 

tests/User/UserManagementTest.php

@@ -189,7 +189,7 @@ class UserManagementTest extends TestCase
         foreach ($langs as $lang) {
             config()->set('app.locale', $lang);
             $resp = $this->asAdmin()->get('/settings/users/create');
-            $resp->assertElementExists('select[name="setting[language]"] option[value="' . $lang . '"][selected]');
+            $resp->assertElementExists('select[name="language"] option[value="' . $lang . '"][selected]');
         }
     }