From 371d517261b1ed8a9209454f41e4439102fdefb9 Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Sat, 25 Jul 2026 22:36:11 +0200
Subject: [PATCH] fix(vpn): dispatch the revocation removal after the
 transaction commits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A worker could pick the job up mid-transaction, still see the peer as active,
leave it on the hub — and never be asked again, because the committed state is
only a soft delete. A revoked colleague would keep their tunnel until some
later reconciliation noticed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 app/Livewire/Admin/Settings.php | 14 ++++++++++++--
 tests/Feature/Admin/VpnTest.php | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/app/Livewire/Admin/Settings.php b/app/Livewire/Admin/Settings.php
index 432d3c3..e9d2cfd 100644
--- a/app/Livewire/Admin/Settings.php
+++ b/app/Livewire/Admin/Settings.php
@@ -155,7 +155,9 @@ class Settings extends Component
     {
         $this->authorize('staff.manage');
 
-        $result = DB::transaction(function () use ($id) {
+        $revokedPeers = [];
+
+        $result = DB::transaction(function () use ($id, &$revokedPeers) {
             Role::query()->where('name', 'Owner')->lockForUpdate()->first();
 
             $target = User::query()->whereKey($id)->lockForUpdate()->first();
@@ -177,12 +179,20 @@ class Settings extends Component
             foreach (VpnPeer::query()->where('user_id', $target->id)->get() as $peer) {
                 $peer->purgeSecret();
                 $peer->delete();
-                ApplyVpnPeer::dispatch($peer->public_key, null, false);
+                $revokedPeers[] = $peer->public_key;
             }
 
             return 'revoked';
         });
 
+        // Dispatched only once the rows are committed: a worker picking the job
+        // up mid-transaction would still see the peer as active, leave it on the
+        // hub, and never be asked again — a revoked colleague would keep their
+        // tunnel until the next reconciliation happened to notice.
+        foreach ($revokedPeers as $publicKey) {
+            ApplyVpnPeer::dispatch($publicKey, null, false);
+        }
+
         $this->flash($result);
     }
 
diff --git a/tests/Feature/Admin/VpnTest.php b/tests/Feature/Admin/VpnTest.php
index 4966c16..2374fe3 100644
--- a/tests/Feature/Admin/VpnTest.php
+++ b/tests/Feature/Admin/VpnTest.php
@@ -640,3 +640,18 @@ it('takes the tunnel away when a staff member is revoked', function () {
         ->and(VpnPeer::withTrashed()->first()->config_secret)->toBeNull();
     Queue::assertPushed(ApplyVpnPeer::class, fn ($job) => $job->publicKey === $peer->public_key && ! $job->enabled);
 });
+
+it('removes a revoked colleague from the hub only after the rows are committed', function () {
+    $hub = vpnHub();
+    $support = operator('Support');
+    $peer = VpnPeer::factory()->ownedBy($support)->create();
+    $hub->addPeer($peer->public_key, $peer->allowed_ip);
+
+    // Sync queue: the job runs the moment it is dispatched, so if that happened
+    // inside the transaction it would still see a live peer and leave it up.
+    Livewire::actingAs(operator('Owner'))->test(App\Livewire\Admin\Settings::class)
+        ->call('revokeStaff', $support->id);
+
+    expect($hub->peerIps())->toBe([])
+        ->and(VpnPeer::withTrashed()->count())->toBe(0);   // tombstone cleared by the removal
+});