diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index 6d48f1240..84002b7f7 100644
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -63,13 +63,16 @@ class UserRepo
/**
* Get all the users with their permissions in a paginated format.
+ * Note: Due to the use of email search this should only be used when
+ * user is assumed to be trusted. (Admin users).
+ * Email search can be abused to extract email addresses.
*/
public function getAllUsersPaginatedAndSorted(int $count, array $sortData): LengthAwarePaginator
{
$sort = $sortData['sort'];
$query = User::query()->select(['*'])
- ->withLastActivityAt()
+ ->scopes(['withLastActivityAt'])
->with(['roles', 'avatar'])
->withCount('mfaValues')
->orderBy($sort, $sortData['order']);
diff --git a/app/Http/Controllers/UserSearchController.php b/app/Http/Controllers/UserSearchController.php
index f7ed9e57a..1a37ca491 100644
--- a/app/Http/Controllers/UserSearchController.php
+++ b/app/Http/Controllers/UserSearchController.php
@@ -3,7 +3,6 @@
namespace BookStack\Http\Controllers;
use BookStack\Auth\User;
-use Illuminate\Database\Eloquent\Builder;
use Illuminate\Http\Request;
class UserSearchController extends Controller
@@ -14,19 +13,27 @@ class UserSearchController extends Controller
*/
public function forSelect(Request $request)
{
+ $hasPermission = signedInUser() && (
+ userCan('users-manage')
+ || userCan('restrictions-manage-own')
+ || userCan('restrictions-manage-all')
+ );
+
+ if (!$hasPermission) {
+ $this->showPermissionError();
+ }
+
$search = $request->get('search', '');
- $query = User::query()->orderBy('name', 'desc')
+ $query = User::query()
+ ->orderBy('name', 'asc')
->take(20);
if (!empty($search)) {
- $query->where(function (Builder $query) use ($search) {
- $query->where('email', 'like', '%' . $search . '%')
- ->orWhere('name', 'like', '%' . $search . '%');
- });
+ $query->where('name', 'like', '%' . $search . '%');
}
- $users = $query->get();
-
- return view('form.user-select-list', compact('users'));
+ return view('form.user-select-list', [
+ 'users' => $query->get(),
+ ]);
}
}
diff --git a/resources/views/users/delete.blade.php b/resources/views/users/delete.blade.php
index aea4d7954..490e9d6c5 100644
--- a/resources/views/users/delete.blade.php
+++ b/resources/views/users/delete.blade.php
@@ -12,17 +12,19 @@
{{ trans('settings.users_delete_warning', ['userName' => $user->name]) }}
-
+ @if(userCan('users-manage'))
+
-
-
-
-
{{ trans('settings.users_migrate_ownership_desc') }}
+
+
+
+
{{ trans('settings.users_migrate_ownership_desc') }}
+
+
+ @include('form.user-select', ['name' => 'new_owner_id', 'user' => null, 'compact' => false])
+
-
- @include('form.user-select', ['name' => 'new_owner_id', 'user' => null, 'compact' => false])
-
-
+ @endif
diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php
index 5a36b85df..94970df4f 100644
--- a/tests/User/UserManagementTest.php
+++ b/tests/User/UserManagementTest.php
@@ -130,6 +130,21 @@ class UserManagementTest extends TestCase
$resp->assertSee('new_owner_id');
}
+ public function test_migrate_option_hidden_if_user_cannot_manage_users()
+ {
+ $editor = $this->getEditor();
+
+ $resp = $this->asEditor()->get("settings/users/{$editor->id}/delete");
+ $resp->assertDontSee('Migrate Ownership');
+ $resp->assertDontSee('new_owner_id');
+
+ $this->giveUserPermissions($editor, ['users-manage']);
+
+ $resp = $this->asEditor()->get("settings/users/{$editor->id}/delete");
+ $resp->assertSee('Migrate Ownership');
+ $resp->assertSee('new_owner_id');
+ }
+
public function test_delete_with_new_owner_id_changes_ownership()
{
$page = Page::query()->first();
diff --git a/tests/User/UserSearchTest.php b/tests/User/UserSearchTest.php
new file mode 100644
index 000000000..1bf3f2d29
--- /dev/null
+++ b/tests/User/UserSearchTest.php
@@ -0,0 +1,68 @@
+getViewer();
+ $admin = $this->getAdmin();
+ $resp = $this->actingAs($admin)->get('/search/users/select?search=' . urlencode($viewer->name));
+
+ $resp->assertOk();
+ $resp->assertSee($viewer->name);
+ $resp->assertDontSee($admin->name);
+ }
+
+ public function test_select_search_shows_first_by_name_without_search()
+ {
+ /** @var User $firstUser */
+ $firstUser = User::query()->orderBy('name', 'desc')->first();
+ $resp = $this->asAdmin()->get('/search/users/select');
+
+ $resp->assertOk();
+ $resp->assertSee($firstUser->name);
+ }
+
+ public function test_select_search_does_not_match_by_email()
+ {
+ $viewer = $this->getViewer();
+ $editor = $this->getEditor();
+ $resp = $this->actingAs($editor)->get('/search/users/select?search=' . urlencode($viewer->email));
+
+ $resp->assertDontSee($viewer->name);
+ }
+
+ public function test_select_requires_right_permission()
+ {
+ $permissions = ['users-manage', 'restrictions-manage-own', 'restrictions-manage-all'];
+ $user = $this->getViewer();
+
+ foreach ($permissions as $permission) {
+ $resp = $this->actingAs($user)->get('/search/users/select?search=a');
+ $this->assertPermissionError($resp);
+
+ $this->giveUserPermissions($user, [$permission]);
+ $resp = $this->actingAs($user)->get('/search/users/select?search=a');
+ $resp->assertOk();
+ $user->roles()->delete();
+ $user->clearPermissionCache();
+ }
+ }
+
+ public function test_select_requires_logged_in_user()
+ {
+ $this->setSettings(['app-public' => true]);
+ $defaultUser = User::getDefault();
+ $this->giveUserPermissions($defaultUser, ['users-manage']);
+
+ $resp = $this->get('/search/users/select?search=a');
+ $this->assertPermissionError($resp);
+ }
+
+}
\ No newline at end of file