From 1b193ba4ea37fe0a68af5c4c9d514f224f61effb Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Sat, 25 Jul 2026 21:49:09 +0200
Subject: [PATCH] fix(vpn): join the host pipeline's allocation lock;
 re-authorize on every poll
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two more from Codex:

- Addresses come from one subnet but live in two tables (hosts.wg_ip and
  vpn_peers.allowed_ip), so a concurrent host onboarding and console creation
  could each see the same address as free and both insert — neither unique
  index sees the other. Creation now waits on Cache::lock('wireguard:allocate'),
  the same lock ConfigureWireguard already holds while reserving a host address.

- mount() runs once, so revoking vpn.manage left an open tab polling every five
  seconds and still receiving fresh peer state. Authorization now also runs on
  hydration, which is what the poll goes through.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 app/Livewire/Admin/Vpn.php      | 45 +++++++++++++++++----------------
 tests/Feature/Admin/VpnTest.php | 33 ++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/app/Livewire/Admin/Vpn.php b/app/Livewire/Admin/Vpn.php
index de245d2..749bbc2 100644
--- a/app/Livewire/Admin/Vpn.php
+++ b/app/Livewire/Admin/Vpn.php
@@ -7,7 +7,6 @@ use App\Provisioning\Jobs\ApplyVpnPeer;
 use App\Provisioning\Jobs\SyncVpnPeers;
 use App\Services\Wireguard\Keypair;
 use App\Services\Wireguard\WireguardHub;
-use Illuminate\Database\QueryException;
 use Illuminate\Support\Facades\Cache;
 use Livewire\Attributes\Layout;
 use Livewire\Attributes\On;
@@ -36,6 +35,17 @@ class Vpn extends Component
         $this->authorize('vpn.manage');
     }
 
+    /**
+     * mount() runs once; every later request — including the five-second poll —
+     * only hydrates. Without this, revoking vpn.manage would not take effect
+     * until the operator happened to reload the page, and the open tab would
+     * keep serving fresh peer state.
+     */
+    public function hydrate(): void
+    {
+        $this->authorize('vpn.manage');
+    }
+
     public function create(): void
     {
         $this->authorize('vpn.manage');
@@ -67,27 +77,18 @@ class Vpn extends Component
 
         $hub = app(WireguardHub::class);
 
-        // allocateIp reads the addresses already handed out; two operators
-        // creating at the same moment can still pick the same one, so the unique
-        // index is the real guard and we retry once against it.
-        $peer = null;
-        foreach (range(1, 3) as $attempt) {
-            try {
-                $peer = VpnPeer::create([
-                    'name' => trim($this->name),
-                    'public_key' => $publicKey,
-                    'allowed_ip' => $hub->allocateIp(),
-                    'enabled' => true,
-                    'present' => false,
-                    'created_by' => auth()->id(),
-                ]);
-                break;
-            } catch (QueryException $e) {
-                if ($attempt === 3) {
-                    throw $e;
-                }
-            }
-        }
+        // Same lock the host pipeline holds while it reserves an address
+        // (ConfigureWireguard). Addresses come from one subnet but live in two
+        // tables, so neither unique index can catch the other's insert — the
+        // shared lock is what keeps a host and an access off the same tunnel IP.
+        $peer = Cache::lock('wireguard:allocate', 30)->block(10, fn () => VpnPeer::create([
+            'name' => trim($this->name),
+            'public_key' => $publicKey,
+            'allowed_ip' => $hub->allocateIp(),
+            'enabled' => true,
+            'present' => false,
+            'created_by' => auth()->id(),
+        ]));
 
         ApplyVpnPeer::dispatch($peer->public_key, $peer->allowed_ip, true);
 
diff --git a/tests/Feature/Admin/VpnTest.php b/tests/Feature/Admin/VpnTest.php
index 3f97348..ae2fade 100644
--- a/tests/Feature/Admin/VpnTest.php
+++ b/tests/Feature/Admin/VpnTest.php
@@ -273,3 +273,36 @@ it('keeps a revoked address and key reserved until the hub confirms', function (
         ->call('create')
         ->assertHasErrors('publicKey');
 });
+
+it('stops an open page from polling once the capability is revoked', function () {
+    vpnHub();
+    $user = operator('Owner');
+    $component = Livewire::actingAs($user)->test(Vpn::class);
+
+    $user->syncRoles(['Support']);
+    app(Spatie\Permission\PermissionRegistrar::class)->forgetCachedPermissions();
+
+    // The five-second poll must not keep serving peer state to a demoted account.
+    $component->call('refreshPeers')->assertForbidden();
+});
+
+it('holds the same allocation lock the host pipeline uses', function () {
+    vpnHub();
+
+    // Onboarding (ConfigureWireguard) holds this exact lock while it reserves a
+    // host address. Addresses come from one subnet but live in two tables, so
+    // no unique index can catch the other side's insert — waiting is the only
+    // thing that keeps a host and an access off the same tunnel IP.
+    // Short TTL so the test measures the wait instead of sitting out a timeout.
+    expect(Illuminate\Support\Facades\Cache::lock('wireguard:allocate', 1)->get())->toBeTrue();
+
+    $started = microtime(true);
+    Livewire::actingAs(operator('Owner'))->test(Vpn::class)
+        ->set('name', 'parallel')
+        ->call('create')
+        ->assertHasNoErrors();
+    $elapsed = microtime(true) - $started;
+
+    expect(VpnPeer::query()->count())->toBe(1)
+        ->and($elapsed)->toBeGreaterThan(0.8);   // it waited for the lock to expire
+});