Merge pull request #1709 from denkfabrik-li/fix/seat-cap-approval-doors

Two doors onto the client seat cap did not ask it. Both update()
methods -- the edit screen and PATCH /api/v1/clients/{id} -- clear
account_requested when a pending client is activated, under a comment
saying that counts as approval, and approval is the moment a seat is
spent. So a managed installation sitting at its cap kept taking clients
on for as long as registrations arrived, and self-registration is open
to strangers, so the supply of pending rows is not the operator's to
control.

Verified rather than taken on trust: the two new door tests were run
against the unguarded controllers and fail there, and every place in
app/ that clears the flag was enumerated to check no third door was
missed. There is none -- the other six already ask, and a conversion
refuses a pending account outright rather than approving it sideways.

The guard sits inside the approval branch, so an installation at its cap
can still rename a client it already holds. That is pinned by a test of
its own.

Conflicted with tonight's seat work in SeatAllowanceTest, which had
added an import beside the one this adds. Resolved by keeping both;
suite green at 2065 and PHPStan clean after resolution.

Reported and fixed by @denkfabrik-li.

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

@@ -191,7 +191,11 @@ class ClientsController extends Controller
             $client->storage_quota_mb = $validated['storage_quota_mb'] ?? 0;
         }
 
+        // Approval, and so the moment the seat is spent — same rule the
+        // web edit screen and approve() answer to. Inside the branch, so a
+        // capped installation can still edit a client it already holds.
         if (($validated['active'] ?? false) && $client->account_requested) {
+            $this->seats->guardClient('active');
             $client->account_requested = false;
         }
 

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

@@ -246,8 +246,13 @@ class ClientsController extends Controller
         ]);
 
         // Activating a pending account through the edit screen counts as
-        // approval and clears the request flag.
+        // approval and clears the request flag — which is the moment a
+        // seat is spent, so the cap is asked here for the same reason
+        // AccountRequestsController::approve() asks it one screen over.
+        // Inside the branch, not above it: an installation at its cap must
+        // still be able to rename a client it already has.
         if ($client->account_requested && $validated['active']) {
+            $this->seats->guardClient('active');
             $client->account_requested = false;
         }
 

tests/Feature/Platform/SeatAllowanceTest.php

@@ -10,6 +10,7 @@ use App\Modules\Platform\Settings\Setting;
 use App\Modules\Platform\Settings\Settings;
 use Illuminate\Validation\ValidationException;
 use Inertia\Testing\AssertableInertia;
+use Laravel\Sanctum\Sanctum;
 
 /**
  * A cap is only a cap if every door asks.
@@ -192,6 +193,53 @@ test('door: approving an account request', function () {
     expect($pending->refresh()->account_requested)->toBeTrue();
 });
 
+test('door: approving a pending client through the edit screen', function () {
+    seatLimits(clients: 0);
+
+    $pending = User::factory()->client()->create(['account_requested' => true, 'active' => false]);
+
+    $this->actingAs($this->admin)->patch("/clients/{$pending->id}", [
+        'name' => $pending->name,
+        'email' => $pending->email,
+        'active' => true,
+    ])->assertSessionHasErrors('active');
+
+    // Nothing is written: the guard throws before save(), so a refused
+    // approval does not leave the name or the flag half-applied.
+    expect($pending->refresh()->account_requested)->toBeTrue()
+        ->and($pending->active)->toBeFalse();
+});
+
+test('door: approving a pending client through the API', function () {
+    seatLimits(clients: 0);
+
+    $pending = User::factory()->client()->create(['account_requested' => true, 'active' => false]);
+
+    Sanctum::actingAs($this->admin, ['*']);
+
+    $this->patchJson("/api/v1/clients/{$pending->id}", ['active' => true])
+        ->assertStatus(422)
+        ->assertJsonValidationErrors('active');
+
+    expect($pending->refresh()->account_requested)->toBeTrue();
+});
+
+test('the cap does not block editing a client the installation already holds', function () {
+    // The guard sits inside the approval branch. Above it, an installation
+    // sitting at its cap could not rename anybody.
+    seatLimits(clients: 0);
+
+    $client = User::factory()->client()->create(['account_requested' => false, 'active' => true]);
+
+    $this->actingAs($this->admin)->patch("/clients/{$client->id}", [
+        'name' => 'Renamed Ltd',
+        'email' => $client->email,
+        'active' => true,
+    ])->assertSessionHasNoErrors();
+
+    expect($client->refresh()->name)->toBe('Renamed Ltd');
+});
+
 test('door: demoting a staff account to client', function () {
     seatLimits(clients: 0);