fix(vpn): join the host pipeline's allocation lock; re-authorize on every poll

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>
feat/portal-design
nexxo 2026-07-25 21:49:09 +02:00
parent 300ea60f19
commit 1b193ba4ea
2 changed files with 56 additions and 22 deletions

View File

@ -7,7 +7,6 @@ use App\Provisioning\Jobs\ApplyVpnPeer;
use App\Provisioning\Jobs\SyncVpnPeers; use App\Provisioning\Jobs\SyncVpnPeers;
use App\Services\Wireguard\Keypair; use App\Services\Wireguard\Keypair;
use App\Services\Wireguard\WireguardHub; use App\Services\Wireguard\WireguardHub;
use Illuminate\Database\QueryException;
use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Cache;
use Livewire\Attributes\Layout; use Livewire\Attributes\Layout;
use Livewire\Attributes\On; use Livewire\Attributes\On;
@ -36,6 +35,17 @@ class Vpn extends Component
$this->authorize('vpn.manage'); $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 public function create(): void
{ {
$this->authorize('vpn.manage'); $this->authorize('vpn.manage');
@ -67,27 +77,18 @@ class Vpn extends Component
$hub = app(WireguardHub::class); $hub = app(WireguardHub::class);
// allocateIp reads the addresses already handed out; two operators // Same lock the host pipeline holds while it reserves an address
// creating at the same moment can still pick the same one, so the unique // (ConfigureWireguard). Addresses come from one subnet but live in two
// index is the real guard and we retry once against it. // tables, so neither unique index can catch the other's insert — the
$peer = null; // shared lock is what keeps a host and an access off the same tunnel IP.
foreach (range(1, 3) as $attempt) { $peer = Cache::lock('wireguard:allocate', 30)->block(10, fn () => VpnPeer::create([
try {
$peer = VpnPeer::create([
'name' => trim($this->name), 'name' => trim($this->name),
'public_key' => $publicKey, 'public_key' => $publicKey,
'allowed_ip' => $hub->allocateIp(), 'allowed_ip' => $hub->allocateIp(),
'enabled' => true, 'enabled' => true,
'present' => false, 'present' => false,
'created_by' => auth()->id(), 'created_by' => auth()->id(),
]); ]));
break;
} catch (QueryException $e) {
if ($attempt === 3) {
throw $e;
}
}
}
ApplyVpnPeer::dispatch($peer->public_key, $peer->allowed_ip, true); ApplyVpnPeer::dispatch($peer->public_key, $peer->allowed_ip, true);

View File

@ -273,3 +273,36 @@ it('keeps a revoked address and key reserved until the hub confirms', function (
->call('create') ->call('create')
->assertHasErrors('publicKey'); ->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
});