- 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.
try {
$user = $this->createNewFromLdapAndCreds($userDetails, $credentials);
} catch (UserRegistrationException $exception) {
- throw new LoginAttemptException($exception->message);
+ throw new LoginAttemptException($exception->getMessage());
}
}
}
// Create the user
- $newUser = $this->userRepo->registerNew($userData, $emailConfirmed);
+ $newUser = $this->userRepo->createWithoutActivity($userData, $emailConfirmed);
+ $newUser->attachDefaultRole();
// Assign social account if given
if ($socialAccount) {
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 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;
}
/**
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.
*
}
/**
- * 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, false);
+
+ if ($sendInvite) {
+ $this->inviteService->sendInvitation($user);
+ }
+
+ Activity::add(ActivityType::USER_CREATE, $user);
return $user;
}
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();
$code = $e->status;
}
+ if (method_exists($e, 'getStatus')) {
+ $code = $e->getStatus();
+ }
+
$responseData['error']['code'] = $code;
return new JsonResponse($responseData, $code, $headers);
{
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.
*
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;
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'],
*/
public function list()
{
- $this->checkPermission('users-manage');
-
$users = $this->userRepo->getApiUsersBuilder();
return $this->apiListingResponse($users, [
], [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)
{
- $this->checkPermission('users-manage');
-
$user = $this->userRepo->getById($id);
$this->singleFormatter($user);
/**
* Update an existing user in the system.
- * @throws \BookStack\Exceptions\UserUpdateException
+ * @throws UserUpdateException
*/
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'));
*/
public function delete(Request $request, string $id)
{
- $this->checkPermission('users-manage');
-
$user = $this->userRepo->getById($id);
$newOwnerId = $request->get('migrate_ownership_id', null);
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;
*/
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);
}
/**
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;
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;
}
}
/**
- * 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['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');
$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');
$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);
$order = 'asc';
}
- $user = $this->user->findOrFail($userId);
+ $user = $this->userRepo->getById($userId);
$sortKey = $listName . '_sort';
$orderKey = $listName . '_sort_order';
setting()->putUser($user, $sortKey, $sort);
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
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.
namespace Tests\Api;
+use BookStack\Actions\ActivityType;
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\Notification;
use Tests\TestCase;
class UsersApiTest extends TestCase
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()
{
- // 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()
{
- // 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_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();
$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();
projects / bookstack / commitdiff