- Refactored some delete checks into repo.
- Added tests to cover.
- Moved some translations to align with activity/logging system.
namespace BookStack\Auth;
+use BookStack\Actions\ActivityType;
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;
*/
public function destroy(User $user, ?int $newOwnerId = null)
{
+ $this->ensureDeletable($user);
+
$user->socialAccounts()->delete();
$user->apiTokens()->delete();
$user->favourites()->delete();
$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());
+ }
}
/**
*/
protected $signature = 'bookstack:delete-users';
- protected $user;
-
protected $userRepo;
/**
*/
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();
}
$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();
- $users = $this->user->where('system_name', '=', null)->with('roles')->get();
+ $totalUsers = User::query()->count();
+ $users = User::query()->whereNull('system_name')->with('roles')->get();
foreach ($users as $user) {
if ($user->hasSystemRole('admin')) {
// don't delete users with "admin" role
use BookStack\Auth\User;
use BookStack\Auth\UserRepo;
use Closure;
+use Illuminate\Http\Request;
class UserApiController extends ApiController
{
],
'update' => [
],
+ 'delete' => [
+ 'migrate_ownership_id' => ['integer', 'exists:users,id'],
+ ],
];
public function __construct(UserRepo $userRepo)
return response()->json($singleUser);
}
+ /**
+ * 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)
+ {
+ $this->checkPermission('users-manage');
+
+ $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.
*/
$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');
}
--- /dev/null
+{
+ "migrate_ownership_id": 5
+}
\ No newline at end of file
'webhook_delete' => 'deleted webhook',
'webhook_delete_notification' => 'Webhook successfully deleted',
+ // Users
+ 'user_delete_notification' => 'User successfully removed',
+
// Other
'commented_on' => 'commented on',
'permissions_update' => 'updated permissions',
'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',
Route::delete('shelves/{id}', [BookshelfApiController::class, 'delete']);
Route::get('users', [UserApiController::class, 'list']);
-Route::get('users/{id}', [UserApiController::class, 'read']);
\ No newline at end of file
+Route::get('users/{id}', [UserApiController::class, 'read']);
+Route::delete('users/{id}', [UserApiController::class, 'delete']);
\ No newline at end of file
// TODO
}
+ 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
+ }
+
public function test_index_endpoint_returns_expected_shelf()
{
$this->actingAsApiAdmin();
],
]);
}
+
+ 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_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));
+ }
}
projects / bookstack / commitdiff