diff --git a/app/Modules/Clients/Http/Controllers/Api/ClientsController.php b/app/Modules/Clients/Http/Controllers/Api/ClientsController.php index fa4aaa78..f00e4570 100644 --- a/app/Modules/Clients/Http/Controllers/Api/ClientsController.php +++ b/app/Modules/Clients/Http/Controllers/Api/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) { diff --git a/app/Modules/Clients/Http/Controllers/ClientsController.php b/app/Modules/Clients/Http/Controllers/ClientsController.php index a4b1fe48..df7f9565 100644 --- a/app/Modules/Clients/Http/Controllers/ClientsController.php +++ b/app/Modules/Clients/Http/Controllers/ClientsController.php @@ -159,6 +159,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) { @@ -171,7 +185,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 4895c1d4..36505b1a 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; @@ -169,6 +171,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']);