Added users-delete API endpoint

- Refactored some delete checks into repo.
- Added tests to cover.
- Moved some translations to align with activity/logging system.

app/Auth/UserRepo.php

@@ -2,13 +2,16 @@
 
 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;
@@ -189,6 +192,8 @@ class UserRepo
      */
     public function destroy(User $user, ?int $newOwnerId = null)
     {
+        $this->ensureDeletable($user);
+
         $user->socialAccounts()->delete();
         $user->apiTokens()->delete();
         $user->favourites()->delete();
@@ -204,6 +209,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/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();
-            $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

app/Http/Controllers/Api/UserApiController.php

@@ -5,6 +5,7 @@ namespace BookStack\Http\Controllers\Api;
 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use Closure;
+use Illuminate\Http\Request;
 
 class UserApiController extends ApiController
 {
@@ -19,6 +20,9 @@ class UserApiController extends ApiController
         ],
         'update' => [
         ],
+        'delete' => [
+            'migrate_ownership_id' => ['integer', 'exists:users,id'],
+        ],
     ];
 
     public function __construct(UserRepo $userRepo)
@@ -56,6 +60,24 @@ class UserApiController extends ApiController
         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.
      */

app/Http/Controllers/UserController.php

@@ -262,21 +262,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');
     }

dev/api/requests/users-delete.json

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

resources/lang/en/activities.php

@@ -59,6 +59,9 @@ return [
     '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',

resources/lang/en/settings.php

@@ -188,7 +188,6 @@ 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',

routes/api.php

@@ -68,4 +68,5 @@ Route::put('shelves/{id}', [BookshelfApiController::class, 'update']);
 Route::delete('shelves/{id}', [BookshelfApiController::class, 'delete']);
 
 Route::get('users', [UserApiController::class, 'list']);
-Route::get('users/{id}', [UserApiController::class, 'read']);
+Route::get('users/{id}', [UserApiController::class, 'read']);
+Route::delete('users/{id}', [UserApiController::class, 'delete']);

tests/Api/UsersApiTest.php

@@ -17,6 +17,14 @@ class UsersApiTest extends TestCase
         // 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();
@@ -61,4 +69,43 @@ class UsersApiTest extends TestCase
             ],
         ]);
     }
+
+    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));
+    }
 }