Hold client records to the same boundary the rest of the library uses

The other half of the sweep. ClientsController and its API twin checked
`abort_unless($client->isClient(), 404)` and nothing else -- a type
check, not a boundary, which is the phrase #1701 used about the group
membership routes for exactly the same reason.

Measured before the fix, with a client-scoped role holding the client
permissions:

  GET    /clients            every client on the installation, name + email
  GET    /clients/{stranger} 200
  PATCH  /clients/{stranger} 302, name actually changed
  DELETE /clients/{stranger} 302, client gone

The tell was one route over. ClientFilesController::index already draws
this line with StaffLibraryScope::canAssignClient and calls it "the same
boundary StaffLibraryScope enforces everywhere else in the library". Its
neighbours in the same family did not.

So the predicate is not new here. What is new is StaffLibraryScope::clients(),
the listing half of canAssignClient, so a screen narrows by the rule its
own buttons are guarded with instead of restating it -- restating it is
how this went wrong, and how the last four of these went wrong.

Eight actions take it: edit, update, destroy and the two-factor reset on
both surfaces, plus both listings. Answering 404 rather than 403, since a
client outside the roster should not be distinguishable from one that is
not there -- matching the isClient() guard already above it.

Account requests stay installation-wide on purpose: a self-registered
client who has not been approved belongs to nobody yet, so there is no
roster to narrow by and narrowing would empty the screen.

The published API document is unchanged -- both routes already documented
the 404 that the type check produced.

CHANGELOG.md

@@ -226,6 +226,16 @@ a version is cut.
   (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in
   [#1705](https://github.com/projectsend/projectsend/pull/1705))
 
+- **A limited staff role no longer reaches every client record, or every file name on the
+  dashboard.** Two more places where holding a permission was treated as holding a boundary. The
+  clients screen listed every client on the installation by name and email, and a role limited to
+  its own assigned clients could open, rename, or delete any of them — the same through the API.
+  Separately, the dashboard's largest-files, expired-files and top-clients widgets named files and
+  clients from across the whole installation, which mattered more because the Client Manager role
+  that ships with ProjectSend holds the permission those widgets need. Both now use the same rule
+  the rest of the library already did. Installation-wide totals stay installation-wide: a count
+  carries no names. Nothing changes for an administrator or any unrestricted role.
+
 ## 2.1.0 — 18 August 2026
 
 Updating, mostly. ProjectSend now tells you when there is a new version, ends an update somewhere

app/Modules/Clients/Http/Controllers/Api/ClientsController.php

@@ -11,6 +11,7 @@ use App\Modules\Audit\Action;
 use App\Modules\Audit\ActivityLogger;
 use App\Modules\Clients\ClientCustomFieldType;
 use App\Modules\Clients\ClientStorageUsage;
+use App\Modules\Files\Access\StaffLibraryScope;
 use App\Modules\Clients\Http\Resources\Api\ClientResource;
 use App\Modules\Clients\Models\ClientCustomField;
 use App\Modules\Clients\Models\ClientCustomFieldValue;
@@ -54,6 +55,7 @@ class ClientsController extends Controller
         private readonly ClientStorageUsage $storageUsage,
         private readonly DeletedAccountContent $accountContent,
         private readonly AccountContentDeletion $accountDeletion,
+        private readonly StaffLibraryScope $scope,
     ) {}
 
     public function index(Request $request): AnonymousResourceCollection
@@ -63,7 +65,12 @@ class ClientsController extends Controller
             'status' => ['nullable', Rule::in(['active', 'inactive'])],
         ]);
 
-        $query = User::query()->where('type', UserType::Client);
+        // Narrowed the same way the web listing is, and by the same
+        // rule the object routes below are guarded with.
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        $query = $this->scope->clients($viewer);
 
         if (($filters['search'] ?? null) !== null) {
             $search = $filters['search'];
@@ -79,10 +86,19 @@ class ClientsController extends Controller
         return ClientResource::collection($this->polling->paginate($request, $query, 'users'));
     }
 
-    public function show(User $client): ClientResource
+    public function show(Request $request, User $client): ClientResource
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: the token's `edit_clients`
+        // says its owner manages clients, not that they manage *this*
+        // one. Mirrors the web controller, as every API twin here does.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         return $this->resourceFor($client);
     }
 
@@ -135,6 +151,15 @@ class ClientsController extends Controller
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: the token's `edit_clients`
+        // says its owner manages clients, not that they manage *this*
+        // one. Mirrors the web controller, as every API twin here does.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $validated = $request->validate([
             'name' => ['sometimes', 'string', 'max:255'],
             'email' => ['sometimes', 'string', 'lowercase', 'email', 'max:255', Rule::unique('users', 'email')->ignore($client->id)],
@@ -203,10 +228,19 @@ class ClientsController extends Controller
      * in the activity log against the caller. Answers 204 whether or not a
      * second factor was actually in force.
      */
-    public function destroyTwoFactor(User $client, TwoFactorAdministration $twoFactor): JsonResponse
+    public function destroyTwoFactor(Request $request, User $client, TwoFactorAdministration $twoFactor): JsonResponse
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: the token's `edit_clients`
+        // says its owner manages clients, not that they manage *this*
+        // one. Mirrors the web controller, as every API twin here does.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $twoFactor->reset($client);
 
         return response()->json(status: 204);
@@ -232,6 +266,15 @@ class ClientsController extends Controller
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: the token's `edit_clients`
+        // says its owner manages clients, not that they manage *this*
+        // one. Mirrors the web controller, as every API twin here does.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $validated = $this->accountDeletion->validate($request, $client);
 
         $name = $client->name;

app/Modules/Clients/Http/Controllers/ClientsController.php

@@ -10,6 +10,7 @@ use App\Modules\Audit\Action;
 use App\Modules\Audit\ActivityLogger;
 use App\Modules\Clients\ClientCustomFieldType;
 use App\Modules\Clients\ClientStorageUsage;
+use App\Modules\Files\Access\StaffLibraryScope;
 use App\Modules\Clients\Models\ClientCustomField;
 use App\Modules\Clients\Models\ClientCustomFieldValue;
 use App\Modules\Clients\Notifications\ClientAccountEditedNotification;
@@ -44,6 +45,7 @@ class ClientsController extends Controller
         private readonly ClientStorageUsage $storageUsage,
         private readonly DeletedAccountContent $accountContent,
         private readonly AccountContentDeletion $accountDeletion,
+        private readonly StaffLibraryScope $scope,
     ) {}
 
     public function index(Request $request): Response
@@ -58,8 +60,14 @@ class ClientsController extends Controller
             'status' => $validated['status'] ?? null,
         ];
 
-        $clients = User::query()
-            ->where('type', UserType::Client)
+        // Narrowed by the same rule the buttons on each row are guarded
+        // with. A client-scoped staff member is not shown the name and
+        // email of somebody they can reach nothing of — the same thing
+        // MembershipRequest::approvableBy does for its queue.
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        $clients = $this->scope->clients($viewer)
             ->when($filters['search'], fn (Builder $query, string $search) => $query->where(fn (Builder $q) => $q
                 ->where('name', 'like', "%{$search}%")
                 ->orWhere('email', 'like', "%{$search}%")))
@@ -133,10 +141,19 @@ class ClientsController extends Controller
         return redirect()->route('clients.edit', $client)->with('success', __('Client created.'));
     }
 
-    public function edit(User $client): Response
+    public function edit(Request $request, User $client): Response
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: `edit_clients` says this staff
+        // member manages clients, not that they manage *this* one. The
+        // same rule ClientFilesController::index applies one route over.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         return Inertia::render('clients/edit', [
             'client' => [
                 'id' => $client->id,
@@ -162,6 +179,15 @@ class ClientsController extends Controller
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: `edit_clients` says this staff
+        // member manages clients, not that they manage *this* one. The
+        // same rule ClientFilesController::index applies one route over.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $validated = $request->validate(array_merge([
             'name' => ['required', 'string', 'max:255'],
             'email' => ['required', 'string', 'lowercase', 'email', 'max:255', Rule::unique('users', 'email')->ignore($client->id)],
@@ -219,10 +245,19 @@ class ClientsController extends Controller
      * Remove this account's second factor, for the client who has lost
      * their authenticator and their recovery codes.
      */
-    public function destroyTwoFactor(User $client, TwoFactorAdministration $twoFactor): RedirectResponse
+    public function destroyTwoFactor(Request $request, User $client, TwoFactorAdministration $twoFactor): RedirectResponse
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: `edit_clients` says this staff
+        // member manages clients, not that they manage *this* one. The
+        // same rule ClientFilesController::index applies one route over.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $twoFactor->reset($client);
 
         return back()->with('success', __('Two-factor authentication removed.'));
@@ -232,6 +267,15 @@ class ClientsController extends Controller
     {
         abort_unless($client->isClient(), 404);
 
+        $viewer = $request->user();
+        assert($viewer !== null);
+
+        // A permission is not a boundary: `edit_clients` says this staff
+        // member manages clients, not that they manage *this* one. The
+        // same rule ClientFilesController::index applies one route over.
+        abort_unless($this->scope->canAssignClient($viewer, $client), 404);
+
+
         $validated = $this->accountDeletion->validate($request, $client);
 
         $name = $client->name;

app/Modules/Files/Access/StaffLibraryScope.php

@@ -10,6 +10,7 @@ use App\Modules\Files\Models\FileAssignment;
 use App\Modules\Files\Models\Folder;
 use App\Modules\Files\Models\FolderAssignment;
 use App\Modules\Groups\Models\Group;
+use App\Modules\Identity\UserType;
 use Illuminate\Database\Eloquent\Builder;
 
 /**
@@ -176,6 +177,25 @@ class StaffLibraryScope
         return $ids === null || in_array($client->id, $ids, true);
     }
 
+    /**
+     * Every client this staff member may act on, as a query.
+     *
+     * The listing half of canAssignClient(), so a screen narrows by the
+     * same rule its buttons are guarded with rather than restating it —
+     * which is how ClientsController came to list every client on the
+     * installation, name and email, to a viewer who could reach nothing
+     * of theirs. An unscoped user gets the whole roster, unchanged.
+     *
+     * @return Builder<User>
+     */
+    public function clients(User $user): Builder
+    {
+        $query = User::query()->where('type', UserType::Client);
+        $ids = $this->assignableClientIds($user);
+
+        return $ids === null ? $query : $query->whereIn('id', $ids);
+    }
+
     public function canAssignGroup(User $user, Group $group): bool
     {
         $ids = $this->assignableGroupIds($user);

tests/Feature/Clients/ClientLibraryScopeTest.php

@@ -0,0 +1,92 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Models\User;
+use App\Modules\Identity\Models\Role;
+use App\Modules\Identity\Models\RolePermission;
+use App\Modules\Identity\Permissions\Permission;
+use Inertia\Testing\AssertableInertia;
+use Laravel\Sanctum\Sanctum;
+
+/**
+ * `edit_clients` says a staff member manages clients. It does not say
+ * they manage *this* one. ClientFilesController::index drew that line
+ * with StaffLibraryScope::canAssignClient; its neighbours in the same
+ * family checked only that the record was a client at all, which is a
+ * type check, not a boundary.
+ */
+beforeEach(function () {
+    $this->admin = User::factory()->create();
+
+    $role = Role::query()->create(['name' => 'Scoped manager', 'client_scoped' => true]);
+    foreach ([Permission::ManageClients, Permission::EditClients, Permission::DeleteClients, Permission::Upload] as $permission) {
+        RolePermission::query()->create(['role_id' => $role->id, 'permission' => $permission->value]);
+    }
+
+    $this->manager = User::factory()->create(['role_id' => $role->id]);
+    $this->mine = User::factory()->client()->create(['name' => 'Mine']);
+    $this->manager->assignedClients()->sync([$this->mine->id]);
+
+    $this->stranger = User::factory()->client()->create(['name' => 'Stranger', 'email' => 'stranger@example.test']);
+});
+
+test('the client list names only the clients this manager holds', function () {
+    $this->actingAs($this->manager)->get('/clients')->assertInertia(
+        fn (AssertableInertia $page) => $page
+            ->has('clients', 1)
+            ->where('clients.0.name', 'Mine'),
+    );
+});
+
+test('a stranger client cannot be opened, changed, or deleted', function () {
+    $this->actingAs($this->manager)->get("/clients/{$this->stranger->id}")->assertNotFound();
+
+    $this->actingAs($this->manager)->patch("/clients/{$this->stranger->id}", [
+        'name' => 'Renamed By Somebody Else',
+        'email' => $this->stranger->email,
+        'active' => true,
+    ])->assertNotFound();
+
+    // password.confirm sits in front of this one, so the session has to
+    // say it was confirmed or the redirect answers before the guard does.
+    $this->actingAs($this->manager)
+        ->withSession(['auth.password_confirmed_at' => time()])
+        ->delete("/clients/{$this->stranger->id}/two-factor")
+        ->assertNotFound();
+    $this->actingAs($this->manager)->delete("/clients/{$this->stranger->id}")->assertNotFound();
+
+    expect($this->stranger->fresh()->name)->toBe('Stranger');
+});
+
+test('the API answers the same way', function () {
+    Sanctum::actingAs($this->manager, ['manage_clients', 'edit_clients', 'delete_clients']);
+
+    $this->getJson('/api/v1/clients')->assertOk()->assertJsonCount(1, 'data');
+    $this->getJson("/api/v1/clients/{$this->stranger->id}")->assertNotFound();
+    $this->patchJson("/api/v1/clients/{$this->stranger->id}", ['name' => 'Nope'])->assertNotFound();
+    $this->deleteJson("/api/v1/clients/{$this->stranger->id}/two-factor")->assertNotFound();
+    $this->deleteJson("/api/v1/clients/{$this->stranger->id}")->assertNotFound();
+
+    expect(User::query()->whereKey($this->stranger->id)->exists())->toBeTrue();
+});
+
+test('a client this manager does hold stays fully manageable', function () {
+    $this->actingAs($this->manager)->get("/clients/{$this->mine->id}")->assertOk();
+
+    $this->actingAs($this->manager)->patch("/clients/{$this->mine->id}", [
+        'name' => 'Mine, Renamed',
+        'email' => $this->mine->email,
+        'active' => true,
+    ])->assertRedirect();
+
+    expect($this->mine->fresh()->name)->toBe('Mine, Renamed');
+});
+
+test('an unscoped administrator reaches every client exactly as before', function () {
+    $this->actingAs($this->admin)->get('/clients')->assertInertia(
+        fn (AssertableInertia $page) => $page->has('clients', 2),
+    );
+
+    $this->actingAs($this->admin)->get("/clients/{$this->stranger->id}")->assertOk();
+});