From 4ffdafc61481b1538bf268b8f19a4463d07da6b4 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 22:50:13 +0200 Subject: [PATCH] fix(vpn): losing the operator roles closes the owner doors by itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ownership alone kept download, block and delete reachable for someone whose roles were removed by any path other than revokeStaff() — a direct role edit, say. Being staff is now part of ownership, so revocation closes these doors without depending on whoever removed the role also remembering the tunnel. Co-Authored-By: Claude Opus 4.8 --- app/Policies/VpnPeerPolicy.php | 9 ++++++++- tests/Feature/Admin/VpnTest.php | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/Policies/VpnPeerPolicy.php b/app/Policies/VpnPeerPolicy.php index e8abfe9..649f8da 100644 --- a/app/Policies/VpnPeerPolicy.php +++ b/app/Policies/VpnPeerPolicy.php @@ -67,9 +67,16 @@ class VpnPeerPolicy && ! $peer->trashed(); } + /** + * isOperator() is part of ownership on purpose: roles can be taken away by + * other paths than revokeStaff() — a direct role edit, for instance. Losing + * the console must close these doors by itself, not rely on whoever removed + * the role also remembering the tunnel. + */ private function owns(User $user, VpnPeer $peer): bool { - return $peer->kind === VpnPeer::KIND_STAFF + return $user->isOperator() + && $peer->kind === VpnPeer::KIND_STAFF && $peer->user_id !== null && $peer->user_id === $user->id; } diff --git a/tests/Feature/Admin/VpnTest.php b/tests/Feature/Admin/VpnTest.php index 290b138..d805389 100644 --- a/tests/Feature/Admin/VpnTest.php +++ b/tests/Feature/Admin/VpnTest.php @@ -745,3 +745,24 @@ it('will not issue an access to someone who is no longer staff', function () { expect(VpnPeer::withTrashed()->count())->toBe(0); }); + +it('closes the owner doors as soon as the roles are gone', function () { + vpnHub(); + $support = operator('Support'); + $peer = VpnPeer::factory()->ownedBy($support)->create([ + 'config_secret' => App\Services\Wireguard\ConfigVault::encrypt("[Interface]\n"), + ]); + expect($support->can('downloadConfig', $peer))->toBeTrue(); + + // Roles removed directly, not through revokeStaff() — ownership alone must + // not keep the modals reachable. + $support->syncRoles([]); + app(Spatie\Permission\PermissionRegistrar::class)->forgetCachedPermissions(); + $support = $support->fresh(); + + expect($support->can('downloadConfig', $peer))->toBeFalse() + ->and($support->can('delete', $peer))->toBeFalse() + ->and($support->can('block', $peer))->toBeFalse(); + + Livewire::actingAs($support)->test(VpnConfigAccess::class, ['uuid' => $peer->uuid])->assertForbidden(); +});