boban
/
CluPilotCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Send an operator with an expired session to sign in, not a 500
tests / pest (push) Failing after 7m15s Details
tests / assets (push) Successful in 21s Details
tests / release (push) Has been skipped Details 

VpnConfigAccess::reveal() dereferenced Auth::guard('operator')->user()
unguarded, which is null when a modal's session has ended since it was
opened (a Livewire action reaches its component through /livewire/update
on its own, without the page's original route middleware). The same
unguarded pattern was in TwoFactorSetup, InstanceAdminAccess and Settings.

Add App\Livewire\Concerns\ResolvesOperator with currentOperator(), the one
place that resolves the operator and redirects to admin.login when there
isn't one, and use it everywhere the pattern occurred.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat/granted-plans
nexxo 2026-07-28 18:09:22 +02:00
parent 833f386e73
commit 164145463c
7 changed files with 180 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/Livewire/Admin/InstanceAdminAccess.php
View File

@ -2,10 +2,10 @@
namespace App\Livewire\Admin;
use App\Livewire\Concerns\ResolvesOperator;
use App\Models\Instance;
use App\Provisioning\Jobs\IssueInstanceAdminAccess;
use App\Services\Wireguard\ConfigHandoff;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Facades\RateLimiter;
use Illuminate\Support\Str;
@ -25,6 +25,8 @@ use LivewireUI\Modal\ModalComponent;
*/
class InstanceAdminAccess extends ModalComponent
{
use ResolvesOperator;
public string $uuid;
public string $subdomain = '';
@ -55,9 +57,11 @@ class InstanceAdminAccess extends ModalComponent
$this->authorize('instances.adminlogin');
$this->resetErrorBag('password');
$operator = Auth::guard('operator')->user();
if (! $operator = $this->currentOperator()) {
return;
}
$key = 'instance-admin:'.$operator?->id;
$key = 'instance-admin:'.$operator->id;
if (RateLimiter::tooManyAttempts($key, 5)) {
$this->addError('password', __('instances.too_many_attempts', ['seconds' => RateLimiter::availableIn($key)]));

54
app/Livewire/Admin/Settings.php
View File

@ -4,6 +4,7 @@ namespace App\Livewire\Admin;
use App\Http\Middleware\RestrictConsoleNetwork;
use App\Livewire\Concerns\ChangesOwnPassword;
use App\Livewire\Concerns\ResolvesOperator;
use App\Models\Customer;
use App\Models\Operator;
use App\Models\VpnPeer;
@ -34,6 +35,7 @@ use Symfony\Component\HttpFoundation\IpUtils;
class Settings extends Component
{
use ChangesOwnPassword;
use ResolvesOperator;
/** This page changes the signed-in OPERATOR's password, never a portal one. */
protected function passwordAccountGuard(): string
@ -65,8 +67,12 @@ class Settings extends Component
public function mount(): void
{
$this->name = Auth::guard('operator')->user()->name;
$this->email = Auth::guard('operator')->user()->email;
if (! $operator = $this->currentOperator()) {
return;
}
$this->name = $operator->name;
$this->email = $operator->email;
$this->requireTwoFactor = AppSettings::bool('console.require_2fa', false);
}
@ -89,10 +95,13 @@ class Settings extends Component
public function saveAccount(): void
{
$user = Auth::guard('operator')->user();
if (! $operator = $this->currentOperator()) {
return;
}
$data = $this->validate([
'name' => 'required|string|max:255',
'email' => 'required|email|max:255|unique:operators,email,'.$user->id,
'email' => 'required|email|max:255|unique:operators,email,'.$operator->id,
]);
// An operator email must never collide with a customer's — that would
@ -105,13 +114,18 @@ class Settings extends Component
return;
}
$user->update($data);
$operator->update($data);
$this->dispatch('notify', message: __('admin_settings.account_saved'));
}
public function inviteStaff(): void
{
$this->authorize('staff.manage');
if (! $operator = $this->currentOperator()) {
return;
}
$data = $this->validate([
'staffName' => 'required|string|max:255',
'staffEmail' => 'required|email|max:255|unique:operators,email',
@ -119,7 +133,7 @@ class Settings extends Component
]);
// Only an Owner may create another Owner.
if ($data['staffRole'] === 'Owner' && ! Auth::guard('operator')->user()->hasRole('Owner')) {
if ($data['staffRole'] === 'Owner' && ! $operator->hasRole('Owner')) {
$this->addError('staffRole', __('admin_settings.owner_only'));
return;
@ -135,12 +149,12 @@ class Settings extends Component
// password and surface it once to the Owner to share securely — the
// account is usable immediately (a proper invite link follows with mail).
$temp = Str::password(14);
$operator = Operator::create([
$newStaff = Operator::create([
'name' => $data['staffName'],
'email' => $data['staffEmail'],
'password' => Hash::make($temp),
]);
$operator->assignRole($data['staffRole']);
$newStaff->assignRole($data['staffRole']);
$this->invitedEmail = $data['staffEmail'];
$this->invitedPassword = $temp;
@ -156,7 +170,11 @@ class Settings extends Component
return;
}
$result = DB::transaction(function () use ($id, $role) {
if (! $operator = $this->currentOperator()) {
return;
}
$result = DB::transaction(function () use ($id, $role, $operator) {
// Serialize on the Owner role so the global owner count can't be
// raced to zero by concurrent demotions of different owners.
Role::query()->where('name', 'Owner')->lockForUpdate()->first();
@ -165,7 +183,7 @@ class Settings extends Component
if ($target === null) {
return 'gone';
}
if ($target->id === Auth::guard('operator')->id()) {
if ($target->id === $operator->id) {
return 'self';
}
// Only existing operators may be re-roled. The Customer check below
@ -176,7 +194,7 @@ class Settings extends Component
return 'not_staff';
}
// Granting or revoking Owner is Owner-only.
if (($role === 'Owner' || $target->hasRole('Owner')) && ! Auth::guard('operator')->user()->hasRole('Owner')) {
if (($role === 'Owner' || $target->hasRole('Owner')) && ! $operator->hasRole('Owner')) {
return 'owner_only';
}
// Never demote the last Owner.
@ -377,7 +395,11 @@ class Settings extends Component
{
$this->authorize('site.manage');
$accepted = app(UpdateChannel::class)->request(Auth::guard('operator')->user()->email);
if (! $operator = $this->currentOperator()) {
return;
}
$accepted = app(UpdateChannel::class)->request($operator->email);
$this->dispatch('notify', message: __($accepted
? 'admin_settings.update_requested'
@ -386,6 +408,8 @@ class Settings extends Component
public function render()
{
$operator = $this->currentOperator();
$staff = Operator::query()
->whereHas('roles', fn ($q) => $q->whereIn('name', Operator::OPERATOR_ROLES))
->with('roles')
@ -403,7 +427,7 @@ class Settings extends Component
'update' => app(UpdateChannel::class)->state(),
'updateLog' => app(UpdateChannel::class)->lastLog(),
'sitePublic' => AppSettings::bool('site.public', true),
'canManageSite' => Auth::guard('operator')->user()?->can('site.manage') ?? false,
'canManageSite' => $operator?->can('site.manage') ?? false,
// Shown so nobody has to guess why they still see the real site.
'viewerOnVpn' => IpUtils::checkIp(
(string) request()->ip(),
@ -417,8 +441,8 @@ class Settings extends Component
'viewerIp' => (string) request()->ip(),
'staff' => $staff,
'roles' => Operator::OPERATOR_ROLES,
'canManageStaff' => Auth::guard('operator')->user()->can('staff.manage'),
'isOwner' => Auth::guard('operator')->user()->hasRole('Owner'),
'canManageStaff' => $operator?->can('staff.manage') ?? false,
'isOwner' => $operator?->hasRole('Owner') ?? false,
]);
}
}

37
app/Livewire/Admin/TwoFactorSetup.php
View File

@ -3,7 +3,7 @@
namespace App\Livewire\Admin;
use App\Livewire\Concerns\ConfirmsPassword;
use Illuminate\Support\Facades\Auth;
use App\Livewire\Concerns\ResolvesOperator;
use Illuminate\Validation\ValidationException;
use Laravel\Fortify\Actions\ConfirmTwoFactorAuthentication;
use Laravel\Fortify\Actions\DisableTwoFactorAuthentication;
@ -38,6 +38,7 @@ use Livewire\Component;
class TwoFactorSetup extends Component
{
use ConfirmsPassword;
use ResolvesOperator;
/** Two-factor is confirmed against the operator's own record, not a portal one. */
protected function confirmationGuard(): string
@ -61,7 +62,11 @@ class TwoFactorSetup extends Component
{
$this->requireConfirmedPassword();
app(EnableTwoFactorAuthentication::class)(Auth::guard('operator')->user());
if (! $operator = $this->currentOperator()) {
return;
}
app(EnableTwoFactorAuthentication::class)($operator);
}
/** Accept a code from the app, which is what actually turns it on. */
@ -69,7 +74,9 @@ class TwoFactorSetup extends Component
{
$this->requireConfirmedPassword();
$operator = Auth::guard('operator')->user();
if (! $operator = $this->currentOperator()) {
return;
}
try {
app(ConfirmTwoFactorAuthentication::class)(
@ -94,7 +101,10 @@ class TwoFactorSetup extends Component
{
$this->requireConfirmedPassword();
$operator = Auth::guard('operator')->user();
if (! $operator = $this->currentOperator()) {
return;
}
app(GenerateNewRecoveryCodes::class)($operator);
$this->recoveryCodes = json_decode(decrypt($operator->refresh()->two_factor_recovery_codes), true);
}
@ -103,7 +113,11 @@ class TwoFactorSetup extends Component
{
$this->requireConfirmedPassword();
app(DisableTwoFactorAuthentication::class)(Auth::guard('operator')->user());
if (! $operator = $this->currentOperator()) {
return;
}
app(DisableTwoFactorAuthentication::class)($operator);
$this->recoveryCodes = null;
$this->dispatch('notify', message: __('two_factor_setup.off'));
}
@ -122,7 +136,18 @@ class TwoFactorSetup extends Component
public function render()
{
$operator = Auth::guard('operator')->user();
// Unlike the actions above, render() has no requireConfirmedPassword()
// gate in front of it — Livewire calls it on every update, including a
// bare property sync, so it is its own reachable path for a session
// that ended after the page loaded.
if (! $operator = $this->currentOperator()) {
return view('livewire.admin.two-factor-setup', [
'twoFactorPending' => false,
'twoFactorOn' => false,
'twoFactorQr' => null,
'passwordConfirmed' => false,
]);
}
return view('livewire.admin.two-factor-setup', [
// Never the secret itself — only whether it exists, and the SVG

10
app/Livewire/Admin/VpnConfigAccess.php
View File

@ -2,11 +2,11 @@
namespace App\Livewire\Admin;
use App\Livewire\Concerns\ResolvesOperator;
use App\Models\VpnPeer;
use App\Services\Wireguard\ConfigHandoff;
use App\Services\Wireguard\ConfigVault;
use App\Services\Wireguard\QrCode;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Facades\Hash;
@ -25,6 +25,8 @@ use LivewireUI\Modal\ModalComponent;
*/
class VpnConfigAccess extends ModalComponent
{
use ResolvesOperator;
public string $uuid;
public string $name = '';
@ -70,10 +72,12 @@ class VpnConfigAccess extends ModalComponent
$peer = VpnPeer::query()->where('uuid', $this->uuid)->firstOrFail();
$this->authorize('downloadConfig', $peer);
$operator = Auth::guard('operator')->user();
if (! $operator = $this->currentOperator()) {
return;
}
// Rate-limited per user, so a stolen session cannot grind the password.
$key = 'vpn-config:'.$operator?->id;
$key = 'vpn-config:'.$operator->id;
if (RateLimiter::tooManyAttempts($key, 5)) {
$this->addError('password', __('vpn.too_many_attempts', ['seconds' => RateLimiter::availableIn($key)]));

52
app/Livewire/Concerns/ResolvesOperator.php Normal file
View File

@ -0,0 +1,52 @@
<?php
namespace App\Livewire\Concerns;
use App\Models\Operator;
use Illuminate\Support\Facades\Auth;
/**
* The one place a console Livewire component resolves "who is signed in"
* and decides what happens when the answer is nobody.
*
* A Livewire action reaches its component through its own request to
* /livewire/update, separate from the request that first rendered the page —
* a modal especially, reachable without the page's own route middleware (see
* R20 in CLAUDE.md). An operator whose session has ended by the time an
* action runs still reaches this code, and Auth::guard('operator')->user()
* comes back null. Four components each dereferenced it unguarded
* ($operator->password, ->id, ->two_factor_recovery_codes, …), turning that
* into a 500 instead of a sign-in prompt.
*
* currentOperator() cannot throw its way out of the null case. Livewire only
* turns $this->redirect() into a browser navigation once the calling action
* returns NORMALLY: the redirect is queued as component state and picked up
* during Livewire's own dehydrate step (Livewire\Features\SupportRedirects),
* which runs strictly after the called method returns — an uncaught
* exception during the call skips it entirely and Livewire ships back a bare
* error response instead (confirmed by reading
* Livewire\Mechanisms\HandleComponents::update() and
* Livewire\Mechanisms\HandleRequests::handleUpdate(), which catches nothing
* but \TypeError). So this returns null and every caller checks for it:
*
* if (! $operator = $this->currentOperator()) {
* return;
* }
*
* That one-line guard is the only thing left for a caller to get wrong; the
* redirect target and the mechanism live here once, so the four cannot drift
* apart the way the original unguarded dereferences did.
*/
trait ResolvesOperator
{
protected function currentOperator(): ?Operator
{
$operator = Auth::guard('operator')->user();
if ($operator === null) {
$this->redirect(route('admin.login'));
}
return $operator;
}
}

16
tests/Feature/Admin/OperatorTwoFactorEnrollmentTest.php
View File

@ -1,6 +1,7 @@
<?php
use App\Livewire\Admin\TwoFactorSetup;
use Illuminate\Support\Facades\Auth;
use Livewire\Livewire;
use PragmaRX\Google2FA\Google2FA;
@ -154,3 +155,18 @@ it('lets an operator regenerate recovery codes and disable two-factor once confi
expect($op->refresh()->two_factor_confirmed_at)->toBeNull()
->and($op->two_factor_secret)->toBeNull();
});
it('sends the operator to sign in instead of a 500 when the session has ended', function () {
// render() has no requireConfirmedPassword() gate in front of it — unlike
// every action above, Livewire calls it on every update including a bare
// property sync, so it is directly reachable with no other guard in the
// way once the session that owned the page has ended.
$op = operator('Support');
$page = Livewire::actingAs($op, 'operator')->test(TwoFactorSetup::class);
// The page sat open long enough for the operator's own session to end.
Auth::guard('operator')->logout();
$page->call('$refresh')->assertRedirect(route('admin.login'));
});

28
tests/Feature/Admin/VpnTest.php
View File

@ -4,6 +4,7 @@ use App\Livewire\Admin\ConfirmDeleteVpnPeer;
use App\Livewire\Admin\Vpn;
use App\Livewire\Admin\VpnConfigAccess;
use App\Models\Host;
use App\Models\Operator;
use App\Models\VpnPeer;
use App\Provisioning\Jobs\ApplyVpnPeer;
use App\Provisioning\Jobs\SyncVpnPeers;
@ -12,6 +13,8 @@ use App\Services\Wireguard\Keypair;
use App\Services\Wireguard\LocalWireguardHub;
use App\Services\Wireguard\PeerSnapshot;
use App\Services\Wireguard\WireguardHub;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Facades\Queue;
use Livewire\Livewire;
@ -620,6 +623,31 @@ it('locks out password guessing', function () {
expect($modal->get('token'))->toBeNull();
});
it('sends the operator to sign in instead of a 500 when the session has ended', function () {
// reveal() re-checks $this->authorize('downloadConfig', $peer) on every
// call, which already refuses a guest (Gate auto-denies a policy whose
// first parameter is a non-nullable Operator). That is a separate,
// already-correct guard — not the one this test is for. A Gate::before
// with a NULLABLE $user isolates the one line this regression is about:
// Auth::guard('operator')->user() coming back null past that point,
// which used to be dereferenced unguarded ($operator->password).
Gate::before(fn (?Operator $user, string $ability) => $ability === 'downloadConfig' ? true : null);
vpnHub();
$support = operator('Support');
$peer = VpnPeer::factory()->ownedBy($support)->create([
'config_secret' => App\Services\Wireguard\ConfigVault::encrypt("[Interface]\nPrivateKey = XYZ\n"),
]);
$modal = Livewire::actingAs($support, 'operator')->test(VpnConfigAccess::class, ['uuid' => $peer->uuid]);
// The modal sat open long enough for the operator's own session to end.
Auth::guard('operator')->logout();
$modal->set('password', 'geheim-1234')->call('reveal')
->assertRedirect(route('admin.login'));
});
it('destroys the stored config when the access is revoked', function () {
vpnHub();
Queue::fake();