Added user-create API endpoint

- Required extracting logic into repo.
- Changed some existing creation paths to standardise behaviour.
- Added test to cover new endpoint.
- Added extra test for user delete to test migration.
- Changed how permission errors are thrown to ensure the right status
  code can be reported when handled in API.

diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php
index 078487224b20b5fa3b547cf536d8c3fb7aa69c22..5a902af7655aa021b37b9faa1425047f2a258005 100644 (file)
--- a/app/Auth/Access/Guards/LdapSessionGuard.php
+++ b/app/Auth/Access/Guards/LdapSessionGuard.php
@@ -84,7 +84,7 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
             try {
                 $user = $this->createNewFromLdapAndCreds($userDetails, $credentials);
             } catch (UserRegistrationException $exception) {
             try {
                 $user = $this->createNewFromLdapAndCreds($userDetails, $credentials);
             } catch (UserRegistrationException $exception) {

-                throw new LoginAttemptException($exception->message);
+                throw new LoginAttemptException($exception->getMessage());

             }
         }
 
             }
         }
 

diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php
index dcdb68bd5cd725530ab31cf69c4a6cc382ea39b6..6fcb404ee83bc5023753c8747b1439a398847b5a 100644 (file)
--- a/app/Auth/Access/RegistrationService.php
+++ b/app/Auth/Access/RegistrationService.php
@@ -96,7 +96,8 @@ class RegistrationService
         }
 
         // Create the user
         }
 
         // Create the user

-        $newUser = $this->userRepo->registerNew($userData, $emailConfirmed);
+        $newUser = $this->userRepo->createWithoutActivity($userData, $emailConfirmed);
+        $newUser->attachDefaultRole();

 
         // Assign social account if given
         if ($socialAccount) {
 
         // Assign social account if given
         if ($socialAccount) {

diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index cb0c0d2fad2113ab4272ad69abec4d9f3c4f6981..c87fda4c89b5322ddc18164f4d00edf6aff012a2 100644 (file)
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -3,6 +3,7 @@
 namespace BookStack\Auth;
 
 use BookStack\Actions\ActivityType;
 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\EntityProvider;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;


@@ -18,17 +19,20 @@ use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Pagination\LengthAwarePaginator;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Pagination\LengthAwarePaginator;
 use Illuminate\Support\Facades\Log;

+use Illuminate\Support\Str;

 
 class UserRepo
 {
     protected $userAvatar;
 
 class UserRepo
 {
     protected $userAvatar;

+    protected $inviteService;

 
     /**
      * UserRepo constructor.
      */
 
     /**
      * UserRepo constructor.
      */

-    public function __construct(UserAvatars $userAvatar)
+    public function __construct(UserAvatars $userAvatar, UserInviteService $inviteService)

     {
         $this->userAvatar = $userAvatar;
     {
         $this->userAvatar = $userAvatar;

+        $this->inviteService = $inviteService;

     }
 
     /**
     }
 
     /**


@@ -92,18 +96,6 @@ class UserRepo
         return $query->paginate($count);
     }
 
         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.
      *
     /**
      * Assign a user to a system-level role.
      *


@@ -166,23 +158,47 @@ 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 = 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();
 
         $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, false);
+
+        if ($sendInvite) {
+            $this->inviteService->sendInvitation($user);
+        }
+
+        Activity::add(ActivityType::USER_CREATE, $user);

         return $user;
     }
 
         return $user;
     }
 

diff --git a/app/Console/Commands/CreateAdmin.php b/app/Console/Commands/CreateAdmin.php
index c3faef79c306a3ed518e3fadb38cb930d4a9878d..c571d383ea92f1866e4632283718f3b3071a15e3 100644 (file)
--- a/app/Console/Commands/CreateAdmin.php
+++ b/app/Console/Commands/CreateAdmin.php
@@ -84,9 +84,8 @@ class CreateAdmin extends Command
             return SymfonyCommand::FAILURE;
         }
 
             return SymfonyCommand::FAILURE;
         }
 

-        $user = $this->userRepo->create($validator->validated());
+        $user = $this->userRepo->createWithoutActivity($validator->validated());

         $this->userRepo->attachSystemRole($user, 'admin');
         $this->userRepo->attachSystemRole($user, 'admin');

-        $this->userRepo->downloadAndAssignUserAvatar($user);

         $user->email_confirmed = true;
         $user->save();
 
         $user->email_confirmed = true;
         $user->save();
 

diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php
index 7ec502525091f487b3a822df9d983a963bf5fde2..317b011d87de92f3369d7b1b3a993efacdd1c864 100644 (file)
--- a/app/Exceptions/Handler.php
+++ b/app/Exceptions/Handler.php
@@ -101,6 +101,10 @@ class Handler extends ExceptionHandler
             $code = $e->status;
         }
 
             $code = $e->status;
         }
 

+        if (method_exists($e, 'getStatus')) {
+            $code = $e->getStatus();
+        }
+

         $responseData['error']['code'] = $code;
 
         return new JsonResponse($responseData, $code, $headers);
         $responseData['error']['code'] = $code;
 
         return new JsonResponse($responseData, $code, $headers);

diff --git a/app/Exceptions/NotifyException.php b/app/Exceptions/NotifyException.php
index 8e748a21dc77ed17301610d6f6e1f877de4ac53e..e09247208fc7d432e84932c97c3252e3bda8f124 100644 (file)
--- a/app/Exceptions/NotifyException.php
+++ b/app/Exceptions/NotifyException.php
@@ -9,17 +9,27 @@ class NotifyException extends Exception implements Responsable
 {
     public $message;
     public $redirectLocation;
 {
     public $message;
     public $redirectLocation;

+    protected $status;

 
     /**
      * NotifyException constructor.
      */
 
     /**
      * 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->message = $message;
         $this->redirectLocation = $redirectLocation;

+        $this->status = $status;

         parent::__construct();
     }
 
         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.
      *
     /**
      * Send the response for this type of exception.
      *

diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php
index 88350e0ea1078ac6f6659d471e4e811653af93d4..cd97dead1ffc0e042ab0ed199496a6ba21227346 100644 (file)
--- a/app/Http/Controllers/Api/UserApiController.php
+++ b/app/Http/Controllers/Api/UserApiController.php
@@ -4,8 +4,10 @@ namespace BookStack\Http\Controllers\Api;
 
 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 
 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;

+use BookStack\Exceptions\UserUpdateException;

 use Closure;
 use Illuminate\Http\Request;
 use Closure;
 use Illuminate\Http\Request;

+use Illuminate\Support\Facades\DB;

 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\Rules\Unique;
 
 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\Rules\Unique;
 


@@ -20,12 +22,29 @@ class UserApiController extends ApiController
     public function __construct(UserRepo $userRepo)
     {
         $this->userRepo = $userRepo;
     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' => [
     }
 
     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'],
             ],
             'update' => [
                 'name' => ['min:2'],


@@ -52,8 +71,6 @@ class UserApiController extends ApiController
      */
     public function list()
     {
      */
     public function list()
     {

-        $this->checkPermission('users-manage');
-

         $users = $this->userRepo->getApiUsersBuilder();
 
         return $this->apiListingResponse($users, [
         $users = $this->userRepo->getApiUsersBuilder();
 
         return $this->apiListingResponse($users, [


@@ -62,14 +79,30 @@ class UserApiController extends ApiController
         ], [Closure::fromCallable([$this, 'listFormatter'])]);
     }
 
         ], [Closure::fromCallable([$this, 'listFormatter'])]);
     }
 

+    /**
+     * Create a new user in the system.
+     */
+    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)
     {
     /**
      * View the details of a single user.
      * Requires permission to manage users.
      */
     public function read(string $id)
     {

-        $this->checkPermission('users-manage');
-

         $user = $this->userRepo->getById($id);
         $this->singleFormatter($user);
 
         $user = $this->userRepo->getById($id);
         $this->singleFormatter($user);
 


@@ -78,12 +111,10 @@ class UserApiController extends ApiController
 
     /**
      * Update an existing user in the system.
 
     /**
      * Update an existing user in the system.

-     * @throws \BookStack\Exceptions\UserUpdateException
+     * @throws UserUpdateException

      */
     public function update(Request $request, string $id)
     {
      */
     public function update(Request $request, string $id)
     {

-        $this->checkPermission('users-manage');
-

         $data = $this->validate($request, $this->rules($id)['update']);
         $user = $this->userRepo->getById($id);
         $this->userRepo->update($user, $data, userCan('users-manage'));
         $data = $this->validate($request, $this->rules($id)['update']);
         $user = $this->userRepo->getById($id);
         $this->userRepo->update($user, $data, userCan('users-manage'));


@@ -100,8 +131,6 @@ class UserApiController extends ApiController
      */
     public function delete(Request $request, string $id)
     {
      */
     public function delete(Request $request, string $id)
     {

-        $this->checkPermission('users-manage');
-

         $user = $this->userRepo->getById($id);
         $newOwnerId = $request->get('migrate_ownership_id', null);
 
         $user = $this->userRepo->getById($id);
         $newOwnerId = $request->get('migrate_ownership_id', null);
 

diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php
index 2c4c2df1e384e2b2b433062d82d82aa959c3e1c8..13a86f6f7c78672ef99cdab9dedb566d1b148cec 100644 (file)
--- a/app/Http/Controllers/Controller.php
+++ b/app/Http/Controllers/Controller.php
@@ -2,13 +2,13 @@
 
 namespace BookStack\Http\Controllers;
 
 
 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 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;
 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()
     {
      */
     protected function showPermissionError()
     {

-        if (request()->wantsJson()) {
-            $response = response()->json(['error' => trans('errors.permissionJson')], 403);
-        } else {
-            $response = redirect('/');
-            $this->showErrorNotification(trans('errors.permission'));
-        }
-
-        throw new HttpResponseException($response);
+        $message = request()->wantsJson() ? trans('errors.permissionJson') : trans('errors.permission');
+        throw new NotifyException($message, '/', 403);

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php
index 9e702a1d74bf5f14fc860a0d895c8d830ec83dc7..46e858d9b6276eeb5c73805803738da9476e5dff 100644 (file)
--- a/app/Http/Controllers/UserController.php
+++ b/app/Http/Controllers/UserController.php
@@ -2,9 +2,7 @@
 
 namespace BookStack\Http\Controllers;
 
 
 namespace BookStack\Http\Controllers;
 

-use BookStack\Actions\ActivityType;

 use BookStack\Auth\Access\SocialAuthService;
 use BookStack\Auth\Access\SocialAuthService;

-use BookStack\Auth\Access\UserInviteService;

 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\ImageUploadException;
 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 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
 {
 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\ValidationException;
 
 class UserController extends Controller
 {

-    protected $user;

     protected $userRepo;
     protected $userRepo;

-    protected $inviteService;

     protected $imageRepo;
 
     /**
      * UserController constructor.
      */
     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->userRepo = $userRepo;

-        $this->inviteService = $inviteService;

         $this->imageRepo = $imageRepo;
     }
 
         $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');
      * @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');
 
         $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['password'] = ['required', Password::default()];
-            $validationRules['password-confirm'] = ['required', 'same:password'];
-        } elseif ($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') {
-            $validationRules['external_auth_id'] = ['required'];
-        }
-        $this->validate($request, $validationRules);
-
-        $user = $this->user->fill($request->all());
-
-        if ($authMethod === 'standard') {
-            $user->password = bcrypt($request->get('password', Str::random(32)));
-        } 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);
-            }
+        $validationRules = [
+            'name'    => ['required'],
+            'email'   => ['required', 'email', 'unique:users,email'],
+            'language' => ['string'],
+            'roles'            => ['array'],
+            'roles.*'          => ['integer'],
+            'password' => $passwordRequired ? ['required', Password::default()] : null,
+            'password-confirm' => $passwordRequired ? ['required', 'same:password'] : null,
+            'external_auth_id' => $externalAuth ? ['required'] : null,
+        ];

 
 

-            $this->userRepo->downloadAndAssignUserAvatar($user);
+        $validated = $this->validate($request, array_filter($validationRules));

 
 

-            $this->logActivity(ActivityType::USER_CREATE, $user);
+        DB::transaction(function () use ($validated, $sendInvite) {
+            $this->userRepo->create($validated, $sendInvite);

         });
 
         return redirect('/settings/users');
         });
 
         return redirect('/settings/users');


@@ -138,7 +102,7 @@ class UserController extends Controller
         $this->checkPermissionOrCurrentUser('users-manage', $id);
 
         /** @var User $user */
         $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');
 
 
         $authMethod = ($user->system_name) ? 'system' : config('auth.method');
 


@@ -312,7 +276,7 @@ class UserController extends Controller
 
         $newState = $request->get('expand', 'false');
 
 
         $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);
         setting()->putUser($user, 'section_expansion#' . $key, $newState);
 
         return response('', 204);


@@ -335,7 +299,7 @@ class UserController extends Controller
             $order = 'asc';
         }
 
             $order = 'asc';
         }
 

-        $user = $this->user->findOrFail($userId);
+        $user = $this->userRepo->getById($userId);

         $sortKey = $listName . '_sort';
         $orderKey = $listName . '_sort_order';
         setting()->putUser($user, $sortKey, $sort);
         $sortKey = $listName . '_sort';
         $orderKey = $listName . '_sort_order';
         setting()->putUser($user, $sortKey, $sort);

diff --git a/routes/api.php b/routes/api.php
index 0325d7c2af568515d0942ff9b85b01a83a92dfc7..c7b8887b650ae761218fe815b74b212b33fedf5b 100644 (file)
--- a/routes/api.php
+++ b/routes/api.php
@@ -68,6 +68,7 @@ Route::put('shelves/{id}', [BookshelfApiController::class, 'update']);
 Route::delete('shelves/{id}', [BookshelfApiController::class, 'delete']);
 
 Route::get('users', [UserApiController::class, 'list']);
 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']);
\ No newline at end of file
 Route::get('users/{id}', [UserApiController::class, 'read']);
 Route::put('users/{id}', [UserApiController::class, 'update']);
 Route::delete('users/{id}', [UserApiController::class, 'delete']);
\ No newline at end of file

diff --git a/tests/Api/TestsApi.php b/tests/Api/TestsApi.php
index 97ca82ea71c914bf5597408f8dd5fc5aec2087b2..0cdd93741272c608b38233e8f75365ab5bc740cb 100644 (file)
--- a/tests/Api/TestsApi.php
+++ b/tests/Api/TestsApi.php
@@ -35,6 +35,14 @@ trait TestsApi
         return ['error' => ['code' => $code, 'message' => $message]];
     }
 
         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.
     /**
      * Format the given (field_name => ["messages"]) array
      * into a standard validation response format.

diff --git a/tests/Api/UsersApiTest.php b/tests/Api/UsersApiTest.php
index 19b7b0adcad20356d0e43edd397784dc4de932fa..e1bcb02d57b1bb701e6c590e9a1baf224cd62a9e 100644 (file)
--- a/tests/Api/UsersApiTest.php
+++ b/tests/Api/UsersApiTest.php
@@ -2,10 +2,13 @@
 
 namespace Tests\Api;
 
 
 namespace Tests\Api;
 

+use BookStack\Actions\ActivityType;

 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;

-use Illuminate\Support\Facades\Auth;
+use BookStack\Entities\Models\Entity;
+use BookStack\Notifications\UserInvite;

 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Hash;

+use Illuminate\Support\Facades\Notification;

 use Tests\TestCase;
 
 class UsersApiTest extends TestCase
 use Tests\TestCase;
 
 class UsersApiTest extends TestCase


@@ -14,17 +17,34 @@ class UsersApiTest extends TestCase
 
     protected $baseEndpoint = '/api/users';
 
 
     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()
     {
     public function test_users_manage_permission_needed_for_all_endpoints()
     {

-        // TODO
+        $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()
     {
     }
 
     public function test_no_endpoints_accessible_in_demo_mode()
     {

-        // TODO
-        // $this->preventAccessInDemoMode();
-        // Can't use directly in constructor as blocks access to docs
-        // Maybe via route middleware
+        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()
     }
 
     public function test_index_endpoint_returns_expected_shelf()


@@ -47,6 +67,85 @@ class UsersApiTest extends TestCase
         ]]);
     }
 
         ]]);
     }
 

+    public function test_create_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Role $role */
+        $role = Role::query()->first();
+
+        $resp = $this->postJson($this->baseEndpoint, [
+            'name' => 'Benny Boris',
+            'email' => '[email protected]',
+            'password' => 'mysuperpass',
+            'language' => 'it',
+            'roles' => [$role->id],
+            'send_invite' => false,
+        ]);
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'name' => 'Benny Boris',
+            'email' => '[email protected]',
+            'external_auth_id' => '',
+            'roles' => [
+                [
+                    'id' => $role->id,
+                    'display_name' => $role->display_name,
+                ]
+            ],
+        ]);
+        $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+
+        /** @var User $user */
+        $user = User::query()->where('email', '=', '[email protected]')->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' => '[email protected]',
+            'send_invite' => true,
+        ]);
+
+        $resp->assertStatus(200);
+        /** @var User $user */
+        $user = User::query()->where('email', '=', '[email protected]')->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' => '[email protected]',
+        ]);
+        $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();
     public function test_read_endpoint()
     {
         $this->actingAsApiAdmin();


@@ -133,6 +232,33 @@ class UsersApiTest extends TestCase
         $this->assertActivityExists('user_delete', null, $user->logDescriptor());
     }
 
         $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();
     public function test_delete_endpoint_fails_deleting_only_admin()
     {
         $this->actingAsApiAdmin();