From 776d3d99f4b7766d950bdb59d38e2530785a87a0 Mon Sep 17 00:00:00 2001 From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com> Date: Fri, 28 Aug 2026 03:28:19 +0200 Subject: [PATCH] Put a client on the roster of the scoped staff member who created them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client-scoped staff member with create_clients creates a client and loses it immediately. guardTarget() answers 404 for anything off their roster, and StaffLibraryScope::clients() leaves it out of their list -- so the record exists, is logged, is welcomed by email, and is invisible to the person who made it. store() redirects to the edit page, which is where they land: POST /clients → 302 → /clients/4 on_creator_roster → false GET /clients/4/edit → 404 clients listed → ["Mine"] the new client is not there Their own roster is where a client they created belongs, so it is attached there. An unscoped creator gains nothing: they see every client already, and a roster entry would change what assignedClients means for them. The API twin does the same, for the same reason -- a scoped token gets a 404 from every route that binds the client it just created. Three tests: the scoped creator can open and list the client, an unscoped creator gains no roster entry, and the API twin behaves like the web. The first and third go red without the fix. --- .../Controllers/Api/ClientsController.php | 14 +++++ .../Http/Controllers/ClientsController.php | 16 +++++- tests/Feature/Api/ClientsTest.php | 28 ++++++++++ .../Feature/Clients/ClientsManagementTest.php | 51 +++++++++++++++++++ 4 files changed, 108 insertions(+), 1 deletion(-) diff --git a/app/Modules/Clients/Http/Controllers/Api/ClientsController.php b/app/Modules/Clients/Http/Controllers/Api/ClientsController.php index e2a5df85..0a1430e2 100644 --- a/app/Modules/Clients/Http/Controllers/Api/ClientsController.php +++ b/app/Modules/Clients/Http/Controllers/Api/ClientsController.php @@ -153,6 +153,20 @@ class ClientsController extends Controller $this->activity->log(Action::UserCreated, subject: $client); + $creator = $request->user(); + assert($creator !== null); + + // A client-scoped creator would otherwise lose the client they just + // made. guardTarget() answers 404 for anything off their roster, so + // the record they created is not theirs to open, and + // StaffLibraryScope::clients() leaves it out of their list as well — + // the client exists, is welcomed by email, and is invisible to the + // person who made it. Their own roster is where a client they + // created belongs; an unscoped creator has no roster to add to. + if ($creator->isClientScoped()) { + $creator->assignedClients()->attach($client->id); + } + $this->saveCustomFieldValues($client, $validated['custom_field_values'] ?? []); if ($this->settings->get(Setting::EmailNotificationsEnabled) === true) { diff --git a/app/Modules/Clients/Http/Controllers/ClientsController.php b/app/Modules/Clients/Http/Controllers/ClientsController.php index 08743e22..9d412505 100644 --- a/app/Modules/Clients/Http/Controllers/ClientsController.php +++ b/app/Modules/Clients/Http/Controllers/ClientsController.php @@ -154,6 +154,20 @@ class ClientsController extends Controller $this->activity->log(Action::UserCreated, subject: $client); + $creator = $request->user(); + assert($creator !== null); + + // A client-scoped creator would otherwise lose the client they just + // made. guardTarget() answers 404 for anything off their roster, so + // the record they created is not theirs to open, and + // StaffLibraryScope::clients() leaves it out of their list as well — + // the client exists, is welcomed by email, and is invisible to the + // person who made it. Their own roster is where a client they + // created belongs; an unscoped creator has no roster to add to. + if ($creator->isClientScoped()) { + $creator->assignedClients()->attach($client->id); + } + $this->saveCustomFieldValues($client, $validated['custom_field_values'] ?? []); if ($this->settings->get(Setting::EmailNotificationsEnabled) === true) { @@ -166,7 +180,7 @@ class ClientsController extends Controller // Fall back to the create form: it shares this route's own gate, so // it is reachable by exactly whoever just created the record, and // the success toast shows there. - $target = $request->user()?->can('edit_clients') + $target = $creator->can('edit_clients') ? redirect()->route('clients.edit', $client) : redirect()->route('clients.create'); diff --git a/tests/Feature/Api/ClientsTest.php b/tests/Feature/Api/ClientsTest.php index 562882d1..e5fccf4b 100644 --- a/tests/Feature/Api/ClientsTest.php +++ b/tests/Feature/Api/ClientsTest.php @@ -9,6 +9,8 @@ use App\Modules\Clients\ClientCustomFieldType; use App\Modules\Clients\Models\ClientCustomField; use App\Modules\Clients\Models\ClientCustomFieldValue; use App\Modules\Files\Models\File; +use App\Modules\Identity\Models\Role; +use App\Modules\Identity\Models\RolePermission; use App\Modules\Identity\Permissions\Permission; use Illuminate\Support\Facades\Storage; @@ -99,6 +101,32 @@ test('a client can be created', function () { ->and(ActivityLog::query()->where('action', Action::UserCreated)->exists())->toBeTrue(); }); +test('a client-scoped token keeps the client it creates', function () { + // The API twin of the same rule: without the roster entry, the token's + // owner gets a 404 from every route that binds the client they just + // created, including show and update. + $role = Role::query()->create(['name' => 'Scoped creator', 'client_scoped' => true]); + RolePermission::query()->insert([ + ['role_id' => $role->id, 'permission' => Permission::ManageClients->value], + ['role_id' => $role->id, 'permission' => Permission::CreateClients->value], + ['role_id' => $role->id, 'permission' => Permission::EditClients->value], + ]); + $creator = User::factory()->create(['role_id' => $role->id]); + $token = $creator->createToken('t', [ + Permission::ManageClients->value, + Permission::CreateClients->value, + Permission::EditClients->value, + ])->plainTextToken; + + $id = $this->withToken($token)->postJson('/api/v1/clients', [ + 'name' => 'Brand New', + 'email' => 'api-brand-new@example.com', + 'password' => 'super-secret-password', + ])->assertCreated()->json('data.id'); + + $this->withToken($token)->getJson("/api/v1/clients/{$id}")->assertOk(); +}); + test('a required custom field is enforced on create', function () { ClientCustomField::query()->create([ 'name' => 'vat', 'label' => 'VAT number', 'type' => ClientCustomFieldType::Text, diff --git a/tests/Feature/Clients/ClientsManagementTest.php b/tests/Feature/Clients/ClientsManagementTest.php index 03f1c111..1b5f4a66 100644 --- a/tests/Feature/Clients/ClientsManagementTest.php +++ b/tests/Feature/Clients/ClientsManagementTest.php @@ -5,6 +5,9 @@ declare(strict_types=1); use App\Models\User; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; +use App\Modules\Identity\Models\Role; +use App\Modules\Identity\Models\RolePermission; +use App\Modules\Identity\Permissions\Permission; use App\Modules\Identity\Permissions\SystemRole; use App\Modules\Identity\UserType; use App\Modules\Platform\Capabilities\Edition; @@ -14,6 +17,54 @@ beforeEach(function () { $this->admin = User::factory()->create(); }); +/* +|-------------------------------------------------------------------------- +| A scoped creator keeps what they create +|-------------------------------------------------------------------------- +*/ + +test('a client-scoped creator can open the client they just made', function () { + // guardTarget() answers 404 for anything off the roster, and + // StaffLibraryScope::clients() leaves it out of the list -- so without + // the roster entry the record exists, is welcomed by email, and is + // invisible to the person who created it. + $role = Role::query()->create(['name' => 'Scoped creator', 'client_scoped' => true]); + RolePermission::query()->insert([ + ['role_id' => $role->id, 'permission' => Permission::ManageClients->value], + ['role_id' => $role->id, 'permission' => Permission::CreateClients->value], + ['role_id' => $role->id, 'permission' => Permission::EditClients->value], + ]); + $creator = User::factory()->create(['role_id' => $role->id]); + + $this->actingAs($creator)->post('/clients', [ + 'name' => 'Brand New', + 'email' => 'brand-new@example.com', + 'password' => 'super-secret-password', + 'password_confirmation' => 'super-secret-password', + ])->assertRedirect(); + + $client = User::query()->where('email', 'brand-new@example.com')->sole(); + + $this->actingAs($creator)->get("/clients/{$client->id}")->assertOk(); + + $props = $this->actingAs($creator)->get('/clients')->assertOk()->viewData('page')['props']; + expect(collect($props['clients'])->pluck('name')->all())->toContain('Brand New'); +}); + +test('an unscoped creator gains no roster entry from creating a client', function () { + // Nothing to add to: an unscoped staff member sees every client + // already, and a roster entry would change what assignedClients means + // for them. + $this->actingAs($this->admin)->post('/clients', [ + 'name' => 'Also New', + 'email' => 'also-new@example.com', + 'password' => 'super-secret-password', + 'password_confirmation' => 'super-secret-password', + ])->assertRedirect(); + + expect($this->admin->refresh()->assignedClients()->count())->toBe(0); +}); + test('the index lists clients only — staff never appear', function () { User::factory()->client()->create(['name' => 'A Client']); User::factory()->role(SystemRole::Uploader)->create(['name' => 'A Staffer']);