diff --git a/app/Livewire/Auth/TwoFactorSetup.php b/app/Livewire/Auth/TwoFactorSetup.php index 4c6fbd0..c9eae30 100644 --- a/app/Livewire/Auth/TwoFactorSetup.php +++ b/app/Livewire/Auth/TwoFactorSetup.php @@ -54,9 +54,10 @@ class TwoFactorSetup extends Component // put (not flash): must survive the redirect AND the later openModal Livewire // request; the modal pulls it on mount to reveal the freshly generated codes once. session()->put('2fa.codes_fresh', true); - // One-time download grant — the download route requires + consumes it, so the - // fresh set can be downloaded once and a later direct hit 403s. - session()->put('2fa.download_grant', true); + // One-time download grant (a timestamp) — the download route requires + consumes + // it within a short window, so the fresh set can be downloaded once and a later + // direct hit 403s. + session()->put('2fa.download_grant', now()->timestamp); } return $this->redirect(route('settings'), navigate: true); diff --git a/app/Livewire/Files/Index.php b/app/Livewire/Files/Index.php index b299851..0e50e04 100644 --- a/app/Livewire/Files/Index.php +++ b/app/Livewire/Files/Index.php @@ -4,6 +4,8 @@ namespace App\Livewire\Files; use App\Livewire\Concerns\WithFleetContext; use App\Services\FleetService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Livewire\Attributes\Layout; use Livewire\Attributes\On; use Livewire\Component; @@ -170,6 +172,10 @@ class Index extends Component return; } + // Seal the FULL path + the server id so the token cannot be retargeted to + // another directory (client-held $path) or host (session-selected server). + $fullPath = rtrim($this->path, '/')."/{$name}"; + $this->dispatch('openModal', component: 'modals.confirm-action', arguments: [ @@ -178,24 +184,39 @@ class Index extends Component 'confirmLabel' => __('common.delete'), 'danger' => true, 'icon' => 'trash', - 'auditAction' => 'file.delete', - 'auditTarget' => rtrim($this->path, '/')."/{$name}", - 'event' => 'fileConfirmed', - 'params' => ['name' => $name], 'notify' => __('files.delete_notify', ['name' => $name]), + 'token' => ConfirmToken::issue( + 'fileConfirmed', + ['path' => $fullPath], + 'file.delete', + $fullPath, + $this->activeServer()?->id, + ), ], ); } /** Applies the confirmed deletion over SFTP, then reloads the directory. */ #[On('fileConfirmed')] - public function deleteEntry(string $name, FleetService $fleet): void + public function deleteEntry(string $confirmToken, FleetService $fleet): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'fileConfirmed'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } $active = $this->activeServer(); - if ($active && $active->credential_exists) { + if (! $active || ($payload['serverId'] ?? null) !== $active->id) { + return; // token sealed for a different server — refuse to retarget + } + + // Delete the SEALED path, never a client-recombined one. + $path = $payload['params']['path']; + + if ($active->credential_exists) { try { - $fleet->deleteFile($active, rtrim($this->path, '/').'/'.$name); + $fleet->deleteFile($active, $path); } catch (Throwable $e) { $this->dispatch('notify', message: __('files.delete_failed', ['error' => $e->getMessage()])); diff --git a/app/Livewire/Modals/ConfirmAction.php b/app/Livewire/Modals/ConfirmAction.php index 9f30f36..4aaf6ac 100644 --- a/app/Livewire/Modals/ConfirmAction.php +++ b/app/Livewire/Modals/ConfirmAction.php @@ -3,16 +3,26 @@ namespace App\Livewire\Modals; use App\Models\AuditEvent; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; +use Livewire\Attributes\Computed; +use Livewire\Attributes\Locked; use LivewireUI\Modal\ModalComponent; /** * Generic R5 confirmation dialog for destructive / stateful actions. * - * The opener passes copy + an audit descriptor + a follow-up event. On confirm - * we persist exactly one AuditEvent and re-dispatch the opener's event so the - * originating page can apply its own state change. The modal stays + * The opener mints a signed, single-use {@see ConfirmToken} that seals the follow-up + * event + its params + the audit descriptor, and passes ONLY that token (plus display + * copy) to the modal. On confirm we re-verify the token, persist exactly one + * AuditEvent from the SEALED descriptor (never from client input), and re-dispatch the + * sealed event so the originating page can apply its own state change — handing the + * same token on so the apply handler can burn it (single-use). The modal stays * domain-agnostic and is reused for every confirm in the app (R5). + * + * Routing lives only in the token: the modal exposes no client-mutable event/params/ + * audit fields, and the token itself is #[Locked] so it cannot be swapped after open. */ class ConfirmAction extends ModalComponent { @@ -26,59 +36,75 @@ class ConfirmAction extends ModalComponent public string $icon = ''; - /** Audit descriptor — one row written on confirm. */ - public string $auditAction = ''; - - public ?string $auditTarget = null; - - public ?int $serverId = null; - - /** Follow-up event re-dispatched to the page after a successful confirm. */ - public string $event = ''; - - /** @var array */ - public array $params = []; - /** Toast shown on confirm. Pass '' to defer the notification to the handler * (e.g. when the real outcome is only known after a remote command runs); * leave null to fall back to the generic default toast. */ public ?string $notify = null; + /** The sealed routing input — the ONLY thing that decides what runs/gets audited. */ + #[Locked] + public string $token = ''; + + public function mount( + string $token, + string $heading = '', + string $body = '', + string $confirmLabel = '', + bool $danger = false, + string $icon = '', + ?string $notify = null, + ): void { + $this->token = $token; + $this->body = $body; + $this->danger = $danger; + $this->icon = $icon; + + // Localized fallbacks for any copy the opener did not pass. + $this->heading = $heading !== '' ? $heading : __('modals.confirm_action.default_heading'); + $this->confirmLabel = $confirmLabel !== '' ? $confirmLabel : __('common.confirm'); + $this->notify = $notify ?? __('modals.confirm_action.default_notify'); + } + public static function modalMaxWidth(): string { return 'md'; } - /** Apply localized fallbacks for any copy the opener did not pass. */ - public function boot(): void + /** Audit-target line for the view — read from the signed token, not client state. */ + #[Computed] + public function targetLabel(): ?string { - if ($this->heading === '') { - $this->heading = __('modals.confirm_action.default_heading'); - } - if ($this->confirmLabel === '') { - $this->confirmLabel = __('common.confirm'); - } - if ($this->notify === null) { - $this->notify = __('modals.confirm_action.default_notify'); - } + return ConfirmToken::peek($this->token)['auditTarget'] ?? null; } public function confirm(): void { - if ($this->auditAction !== '') { + try { + // Flip the token pending → confirmed (single transition). This IS the + // confirmation: an apply handler may only consume a confirmed token. + $payload = ConfirmToken::confirm($this->token); + } catch (InvalidConfirmToken) { + // Forged / tampered / expired / already-confirmed — never audit or dispatch. + $this->dispatch('notify', message: __('modals.confirm_action.rejected')); + $this->closeModal(); + + return; + } + + if (($payload['auditAction'] ?? '') !== '') { AuditEvent::create([ 'user_id' => Auth::id(), - 'server_id' => $this->serverId, + 'server_id' => $payload['serverId'] ?? null, 'actor' => Auth::user()?->name ?? 'system', - 'action' => $this->auditAction, - 'target' => $this->auditTarget, + 'action' => $payload['auditAction'], + 'target' => $payload['auditTarget'] ?? null, 'ip' => request()->ip(), ]); } - if ($this->event !== '') { - $this->dispatch($this->event, ...$this->params); - } + // Re-dispatch the sealed event, handing the token on so the apply handler can + // burn it (single-use) and read the sealed params itself. + $this->dispatch($payload['event'], confirmToken: $this->token); // Empty notify = the handler will report the real outcome itself. if ($this->notify !== '') { diff --git a/app/Livewire/Modals/RecoveryCodes.php b/app/Livewire/Modals/RecoveryCodes.php index 3bb54ed..1462d1b 100644 --- a/app/Livewire/Modals/RecoveryCodes.php +++ b/app/Livewire/Modals/RecoveryCodes.php @@ -4,26 +4,21 @@ namespace App\Livewire\Modals; use App\Models\AuditEvent; use Illuminate\Support\Facades\Auth; -use Livewire\Attributes\Locked; use LivewireUI\Modal\ModalComponent; /** * Backup (recovery) codes, shown once in a modal. Opened on first-factor enrollment and * from Settings → Security ("Backup-Codes verwalten"). The download endpoint stays a route. + * + * Show-once is structural, not UX-level: the component holds NO codes and NO "revealed" + * state. Codes reach the browser only via a transient `reveal-codes` event fired from + * mount() (one-time `2fa.codes_fresh` flag) or regenerate(). Because mount() never runs on + * hydrate and dispatched events live in the one-time response effects (never the snapshot), + * a replayed/stale snapshot has nothing to re-render and fires no event — captured codes + * cannot be re-revealed by replaying a signed snapshot. */ class RecoveryCodes extends ModalComponent { - /** - * Codes are revealed ONLY when freshly generated in this flow — never re-viewable from - * "Backup-Codes verwalten". Set true by a one-time server flag on mount (first-factor - * enrollment) or by regenerate(). A client cannot tamper this into showing stored codes: - * render() only emits codes when $revealed, and $revealed is only set server-side. - * #[Locked] rejects any client-side update of this prop, so a crafted /livewire/update - * cannot flip it to re-view stored codes. - */ - #[Locked] - public bool $revealed = false; - public static function modalMaxWidth(): string { return 'lg'; @@ -32,9 +27,10 @@ class RecoveryCodes extends ModalComponent public function mount(): void { // One-time, server-set flag (put on first-factor enrollment). pull() reads + forgets, - // so a refresh of the manage view will no longer reveal the codes. + // so a refresh of the manage view will no longer reveal the codes. mount() does not + // run on hydrate, so a replayed snapshot never re-triggers this. if (session()->pull('2fa.codes_fresh', false)) { - $this->revealed = true; + $this->dispatchCodes(); } } @@ -51,20 +47,28 @@ class RecoveryCodes extends ModalComponent 'ip' => request()->ip(), ]); - // The freshly generated set may be shown once in this same modal instance. - $this->revealed = true; + // One-time download grant (a timestamp) for the freshly regenerated set, consumed by + // the download route within a short window. Without it a later direct hit 403s. + session()->put('2fa.download_grant', now()->timestamp); - // One-time download grant for the freshly regenerated set (consumed by the - // download route). Without it a later direct hit of the route 403s. - session()->put('2fa.download_grant', true); + // Reveal the freshly generated set once, via the transient event (never a property). + $this->dispatchCodes(); $this->dispatch('notify', message: __('auth.recovery_regenerated')); } public function render() { - return view('livewire.modals.recovery-codes', [ - 'codes' => $this->revealed ? Auth::user()->recoveryCodes() : [], - ]); + return view('livewire.modals.recovery-codes'); + } + + /** + * Deliver the current user's recovery codes to the browser via a one-time event. The + * codes never enter a Livewire property or the snapshot — Alpine holds them in JS memory + * only, so a replayed snapshot cannot re-render them. + */ + protected function dispatchCodes(): void + { + $this->dispatch('reveal-codes', codes: Auth::user()->recoveryCodes()); } } diff --git a/app/Livewire/Servers/Show.php b/app/Livewire/Servers/Show.php index 2f7788d..ea43646 100644 --- a/app/Livewire/Servers/Show.php +++ b/app/Livewire/Servers/Show.php @@ -8,6 +8,8 @@ use App\Services\Fail2banService; use App\Services\FirewallService; use App\Services\FleetService; use App\Services\HardeningService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use App\Support\Os\OsDetector; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Str; @@ -179,20 +181,32 @@ class Show extends Component 'confirmLabel' => __('common.remove'), 'danger' => true, 'icon' => 'trash', - 'auditAction' => 'ssh_key.remove', - 'auditTarget' => "{$comment} · {$this->server->name}", - 'serverId' => $this->server->id, - 'event' => 'keyRemoved', - 'params' => ['fingerprint' => $fingerprint], 'notify' => __('servers.notify_key_removed', ['name' => $comment]), + 'token' => ConfirmToken::issue( + 'keyRemoved', + ['fingerprint' => $fingerprint], + 'ssh_key.remove', + "{$comment} · {$this->server->name}", + $this->server->id, + ), ], ); } /** Applies the confirmed key removal over SSH, then reloads the list. */ #[On('keyRemoved')] - public function removeKey(string $fingerprint, FleetService $fleet): void + public function removeKey(string $confirmToken, FleetService $fleet): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'keyRemoved'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + if (($payload['serverId'] ?? null) !== $this->server->id) { + return; // token sealed for a different server — refuse to retarget + } + $fingerprint = $payload['params']['fingerprint']; + try { $fleet->removeAuthorizedKey($this->server, $fingerprint); } catch (Throwable $e) { @@ -279,20 +293,31 @@ class Show extends Component 'confirmLabel' => __('common.delete'), 'danger' => true, 'icon' => 'trash', - 'auditAction' => 'credential.delete', - 'auditTarget' => $this->server->name.' · '.$cred->username, - 'serverId' => $this->server->id, - 'event' => 'credentialDeleted', - 'params' => [], 'notify' => __('servers.notify_access_deleted'), + 'token' => ConfirmToken::issue( + 'credentialDeleted', + [], + 'credential.delete', + $this->server->name.' · '.$cred->username, + $this->server->id, + ), ], ); } /** Applies the confirmed credential deletion, then refreshes the row. */ #[On('credentialDeleted')] - public function deleteCredential(): void + public function deleteCredential(string $confirmToken): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'credentialDeleted'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + if (($payload['serverId'] ?? null) !== $this->server->id) { + return; // token sealed for a different server — refuse to retarget + } + $this->server->credential()->delete(); $this->server->refresh(); } @@ -332,17 +357,31 @@ class Show extends Component 'icon' => 'trash', // No premature audit/notify — the handler records the real outcome // only after the remote command returns (audit reflects success). - 'event' => 'firewallRuleDelete', - 'params' => ['spec' => $spec, 'label' => $label], 'notify' => '', + 'token' => ConfirmToken::issue( + 'firewallRuleDelete', + ['spec' => $spec, 'label' => $label], + serverId: $this->server->id, + ), ], ); } /** Applies the confirmed rule deletion over SSH, then audits + reloads on success. */ #[On('firewallRuleDelete')] - public function deleteFirewallRule(string $spec, string $label, FirewallService $firewall): void + public function deleteFirewallRule(string $confirmToken, FirewallService $firewall): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'firewallRuleDelete'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + if (($payload['serverId'] ?? null) !== $this->server->id) { + return; // token sealed for a different server — refuse to retarget + } + $spec = $payload['params']['spec']; + $label = $payload['params']['label']; + try { $res = $firewall->deleteRule($this->server, $spec); } catch (Throwable $e) { diff --git a/app/Livewire/Services/Index.php b/app/Livewire/Services/Index.php index d7a859c..7dfa9b9 100644 --- a/app/Livewire/Services/Index.php +++ b/app/Livewire/Services/Index.php @@ -4,6 +4,8 @@ namespace App\Livewire\Services; use App\Livewire\Concerns\WithFleetContext; use App\Services\FleetService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Str; use Livewire\Attributes\Layout; use Livewire\Attributes\On; @@ -122,23 +124,34 @@ class Index extends Component 'confirmLabel' => $label, 'danger' => $op === 'stop', 'icon' => $icon, - 'auditAction' => $action, - 'auditTarget' => "{$name} · {$this->server}", - 'event' => 'serviceConfirmed', - 'params' => ['op' => $op, 'name' => $name], 'notify' => __('services.confirm_notify', ['name' => $name, 'label' => $label]), + 'token' => ConfirmToken::issue( + 'serviceConfirmed', + ['op' => $op, 'name' => $name], + $action, + "{$name} · {$this->server}", + $this->activeServer()?->id, + ), ], ); } /** Performs the confirmed systemctl action over SSH, then reloads real state. */ #[On('serviceConfirmed')] - public function applyService(string $op, string $name, FleetService $fleet): void + public function applyService(string $confirmToken, FleetService $fleet): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'serviceConfirmed'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $op = $payload['params']['op']; + $name = $payload['params']['name']; + $active = $this->activeServer(); - if (! $active || ! $active->credential_exists) { - return; + if (! $active || ($payload['serverId'] ?? null) !== $active->id || ! $active->credential_exists) { + return; // wrong/absent server (retarget attempt) or no credential — no-op } try { diff --git a/app/Livewire/Settings/Security.php b/app/Livewire/Settings/Security.php index 9e4c52e..c90aa5d 100644 --- a/app/Livewire/Settings/Security.php +++ b/app/Livewire/Settings/Security.php @@ -2,6 +2,8 @@ namespace App\Livewire\Settings; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; use Livewire\Attributes\On; use Livewire\Component; @@ -18,17 +20,26 @@ class Security extends Component 'confirmLabel' => __('common.disable'), 'danger' => true, 'icon' => 'shield', - 'auditAction' => 'two_factor.disable', - 'auditTarget' => Auth::user()?->email, - 'event' => 'twoFactorDisabled', 'notify' => __('settings.disable_2fa_notify'), + 'token' => ConfirmToken::issue( + 'twoFactorDisabled', + [], + 'two_factor.disable', + Auth::user()?->email, + ), ], ); } #[On('twoFactorDisabled')] - public function disableTwoFactor(): void + public function disableTwoFactor(string $confirmToken): void { + try { + ConfirmToken::consume($confirmToken, 'twoFactorDisabled'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + // Remove the TOTP factor only — security keys are managed on their own card. If no // factor remains afterwards, the backup codes are dropped too. Auth::user()->forceFill([ diff --git a/app/Livewire/Settings/Sessions.php b/app/Livewire/Settings/Sessions.php index 1415da7..674be33 100644 --- a/app/Livewire/Settings/Sessions.php +++ b/app/Livewire/Settings/Sessions.php @@ -3,6 +3,8 @@ namespace App\Livewire\Settings; use App\Services\SessionService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; use Livewire\Attributes\On; use Livewire\Component; @@ -24,17 +26,26 @@ class Sessions extends Component 'confirmLabel' => __('sessions.logout_others'), 'danger' => true, 'icon' => 'logout', - 'auditAction' => 'session.logout_others', - 'auditTarget' => Auth::user()?->email, - 'event' => 'sessionsLogoutOthers', 'notify' => __('sessions.logout_others_notify'), + 'token' => ConfirmToken::issue( + 'sessionsLogoutOthers', + [], + 'session.logout_others', + Auth::user()?->email, + ), ], ); } #[On('sessionsLogoutOthers')] - public function logoutOthers(SessionService $sessions): void + public function logoutOthers(string $confirmToken, SessionService $sessions): void { + try { + ConfirmToken::consume($confirmToken, 'sessionsLogoutOthers'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $sessions->logoutOtherDevices(Auth::user()); // List re-renders on the next round trip; no explicit refresh needed. } @@ -54,19 +65,28 @@ class Sessions extends Component 'confirmLabel' => __('sessions.logout_all'), 'danger' => true, 'icon' => 'power', - 'auditAction' => 'session.logout_all', - 'auditTarget' => Auth::user()?->email, - 'event' => 'sessionsLogoutAll', // Defer the toast: this session is about to be destroyed and the // user redirected to login, so a toast would never be seen. 'notify' => '', + 'token' => ConfirmToken::issue( + 'sessionsLogoutAll', + [], + 'session.logout_all', + Auth::user()?->email, + ), ], ); } #[On('sessionsLogoutAll')] - public function logoutAll(SessionService $sessions) + public function logoutAll(string $confirmToken, SessionService $sessions) { + try { + ConfirmToken::consume($confirmToken, 'sessionsLogoutAll'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + // Truncates all sessions + rotates every remember_token. This logs the // operator out too, so push them to login (AuthenticateSession would // otherwise bounce the very next request). diff --git a/app/Livewire/Settings/Users.php b/app/Livewire/Settings/Users.php index 9d0b0a9..6186c19 100644 --- a/app/Livewire/Settings/Users.php +++ b/app/Livewire/Settings/Users.php @@ -5,6 +5,8 @@ namespace App\Livewire\Settings; use App\Models\AuditEvent; use App\Models\User; use App\Services\SessionService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Livewire\Attributes\On; @@ -46,18 +48,27 @@ class Users extends Component 'confirmLabel' => __('accounts.remove'), 'danger' => true, 'icon' => 'trash', - 'auditTarget' => $user->email, - 'event' => 'userRemoved', - 'params' => ['userId' => $userId], // Defer the toast: the apply handler reports the real outcome. 'notify' => '', + 'token' => ConfirmToken::issue( + 'userRemoved', + ['userId' => $userId], + auditTarget: $user->email, + ), ], ); } #[On('userRemoved')] - public function remove(int $userId, SessionService $sessions): void + public function remove(string $confirmToken, SessionService $sessions): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'userRemoved'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $userId = $payload['params']['userId']; + $self = Auth::id(); // Atomic: lock the user rows + recount inside the transaction so two concurrent @@ -111,17 +122,26 @@ class Users extends Component 'confirmLabel' => __('accounts.logout'), 'danger' => true, 'icon' => 'logout', - 'auditTarget' => $user->email, - 'event' => 'userLoggedOut', - 'params' => ['userId' => $userId], 'notify' => '', + 'token' => ConfirmToken::issue( + 'userLoggedOut', + ['userId' => $userId], + auditTarget: $user->email, + ), ], ); } #[On('userLoggedOut')] - public function logout(int $userId, SessionService $sessions): void + public function logout(string $confirmToken, SessionService $sessions): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'userLoggedOut'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $userId = $payload['params']['userId']; + $user = User::find($userId); if (! $user || $user->is(Auth::user())) { return; diff --git a/app/Livewire/Settings/WebauthnKeys.php b/app/Livewire/Settings/WebauthnKeys.php index 7d6cd46..a97b0b3 100644 --- a/app/Livewire/Settings/WebauthnKeys.php +++ b/app/Livewire/Settings/WebauthnKeys.php @@ -4,6 +4,8 @@ namespace App\Livewire\Settings; use App\Models\AuditEvent; use App\Services\WebauthnService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; use Livewire\Attributes\On; use Livewire\Attributes\Validate; @@ -40,9 +42,10 @@ class WebauthnKeys extends Component Auth::user()->replaceRecoveryCodes(); // Server-set one-time flag so the opening modal reveals the fresh set once. session()->put('2fa.codes_fresh', true); - // One-time download grant — the download route requires + consumes it, so the - // fresh set can be downloaded once and a later direct hit 403s. - session()->put('2fa.download_grant', true); + // One-time download grant (a timestamp) — the download route requires + consumes + // it within a short window, so the fresh set can be downloaded once and a later + // direct hit 403s. + session()->put('2fa.download_grant', now()->timestamp); $this->dispatch('openModal', component: 'modals.recovery-codes'); } @@ -73,18 +76,27 @@ class WebauthnKeys extends Component 'confirmLabel' => __('common.remove'), 'danger' => true, 'icon' => 'trash', - 'event' => 'webauthnKeyRemoved', - 'params' => ['id' => $id], 'notify' => '', + 'token' => ConfirmToken::issue( + 'webauthnKeyRemoved', + ['id' => $id], + ), ], ); } #[On('webauthnKeyRemoved')] - public function remove(int $id, WebauthnService $webauthn): void + public function remove(string $confirmToken, WebauthnService $webauthn): void { abort_unless($webauthn->available(), 404); + try { + $payload = ConfirmToken::consume($confirmToken, 'webauthnKeyRemoved'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $id = $payload['params']['id']; + $cred = Auth::user()->webauthnCredentials()->whereKey($id)->first(); if ($cred) { $name = $cred->name; diff --git a/app/Livewire/System/Index.php b/app/Livewire/System/Index.php index 62aa2e8..91e9a98 100644 --- a/app/Livewire/System/Index.php +++ b/app/Livewire/System/Index.php @@ -5,6 +5,8 @@ namespace App\Livewire\System; use App\Models\AuditEvent; use App\Models\Setting; use App\Services\DeploymentService; +use App\Support\Confirm\ConfirmToken; +use App\Support\Confirm\InvalidConfirmToken; use Illuminate\Support\Facades\Auth; use Livewire\Attributes\Layout; use Livewire\Attributes\On; @@ -101,19 +103,26 @@ class Index extends Component 'confirmLabel' => __('system.change_domain_confirm'), 'danger' => true, 'icon' => 'globe', - 'auditAction' => 'system.settings_updated', - 'auditTarget' => 'panel_domain='.($new !== '' ? $new : '(cleared)'), - 'event' => 'domainChanged', - 'params' => ['domain' => $new], 'notify' => '', // defer: we surface the restart notice instead of a toast + 'token' => ConfirmToken::issue( + 'domainChanged', + ['domain' => $new], + 'system.settings_updated', + 'panel_domain='.($new !== '' ? $new : '(cleared)'), + ), ], ); } #[On('domainChanged')] - public function applyDomain(string $domain, DeploymentService $deployment): void + public function applyDomain(string $confirmToken, DeploymentService $deployment): void { - $domain = strtolower(trim($domain)); + try { + $payload = ConfirmToken::consume($confirmToken, 'domainChanged'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $domain = strtolower(trim($payload['params']['domain'])); // Defensive re-validation (the value round-trips through the modal event). if ($domain !== '' && ! preg_match(DeploymentService::DOMAIN_REGEX, $domain)) { @@ -155,18 +164,27 @@ class Index extends Component 'confirmLabel' => __('system.change_channel_confirm'), 'danger' => false, 'icon' => 'git-branch', - 'auditAction' => 'system.settings_updated', - 'auditTarget' => 'release_channel='.$channel, - 'event' => 'channelChanged', - 'params' => ['channel' => $channel], 'notify' => __('system.change_channel_notify', ['channel' => $channel]), + 'token' => ConfirmToken::issue( + 'channelChanged', + ['channel' => $channel], + 'system.settings_updated', + 'release_channel='.$channel, + ), ], ); } #[On('channelChanged')] - public function applyChannel(string $channel): void + public function applyChannel(string $confirmToken): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'channelChanged'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $channel = $payload['params']['channel']; + if (! in_array($channel, self::CHANNELS, true)) { return; } @@ -192,16 +210,25 @@ class Index extends Component 'confirmLabel' => __('system.change_tls_confirm'), 'danger' => true, 'icon' => 'shield', - 'event' => 'tlsModeChanged', - 'params' => ['mode' => $mode], 'notify' => '', + 'token' => ConfirmToken::issue( + 'tlsModeChanged', + ['mode' => $mode], + ), ], ); } #[On('tlsModeChanged')] - public function applyTlsMode(string $mode, DeploymentService $deployment): void + public function applyTlsMode(string $confirmToken, DeploymentService $deployment): void { + try { + $payload = ConfirmToken::consume($confirmToken, 'tlsModeChanged'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — no-op + } + $mode = $payload['params']['mode']; + if (! in_array($mode, self::TLS_MODES, true)) { return; } diff --git a/app/Support/Confirm/ConfirmToken.php b/app/Support/Confirm/ConfirmToken.php new file mode 100644 index 0000000..5048e36 --- /dev/null +++ b/app/Support/Confirm/ConfirmToken.php @@ -0,0 +1,213 @@ + $params + * + * @throws InvalidArgumentException when $event is not allowlisted + */ + public static function issue( + string $event, + array $params = [], + string $auditAction = '', + ?string $auditTarget = null, + ?int $serverId = null, + ): string { + if (! in_array($event, self::ACTIONS, true)) { + throw new InvalidArgumentException("Unknown confirm action: {$event}"); + } + + $nonce = (string) Str::uuid(); + + Cache::put(self::CACHE_PREFIX.$nonce, self::STATE_PENDING, self::TTL); + + return Crypt::encryptString(json_encode([ + 'nonce' => $nonce, + 'event' => $event, + 'params' => $params, + 'auditAction' => $auditAction, + 'auditTarget' => $auditTarget, + 'serverId' => $serverId, + 'uid' => Auth::id(), + ], JSON_THROW_ON_ERROR)); + } + + /** + * The modal's confirm step: atomically flip the nonce `pending` → `confirmed`. + * Proves a human passed through the modal before any apply handler may run. + * + * @return array + * + * @throws InvalidConfirmToken + */ + public static function confirm(string $token): array + { + $payload = self::decode($token); + + self::transition($payload['nonce'], function (string $key) { + if (Cache::get($key) !== self::STATE_PENDING) { + return false; + } + Cache::put($key, self::STATE_CONFIRMED, self::TTL); + + return true; + }); + + return $payload; + } + + /** + * The apply handler's gate: a `confirmed` nonce is burned (single-use) and the + * sealed payload returned. $event is the handler's own action; it must match. + * + * @return array + * + * @throws InvalidConfirmToken + */ + public static function consume(string $token, string $event): array + { + $payload = self::decode($token); + + if ($payload['event'] !== $event) { + throw new InvalidConfirmToken('confirm token action mismatch'); + } + + self::transition($payload['nonce'], function (string $key) { + if (Cache::get($key) !== self::STATE_CONFIRMED) { + return false; + } + Cache::forget($key); + + return true; + }); + + return $payload; + } + + /** + * Signature-only decode for DISPLAY (e.g. the audit-target line). Never throws and + * does not touch the lifecycle state — display must not depend on it. + * + * @return array|null + */ + public static function peek(string $token): ?array + { + try { + return self::decode($token); + } catch (InvalidConfirmToken) { + return null; + } + } + + /** + * Run a state transition under a per-nonce lock so two concurrent requests cannot + * both pass the check (Cache::pull is get-then-forget, not atomic). The closure + * returns true on a successful transition; anything else (or a lost lock race) is + * an invalid token. + * + * @param Closure(string): bool $mutate + * + * @throws InvalidConfirmToken + */ + private static function transition(string $nonce, Closure $mutate): void + { + $key = self::CACHE_PREFIX.$nonce; + + $ok = Cache::lock($key.':lock', 10)->get(fn () => $mutate($key)); + + if ($ok !== true) { + throw new InvalidConfirmToken('confirm token expired, already used, or not confirmed'); + } + } + + /** + * Decrypt + integrity/allowlist/uid check. No lifecycle-state check here. + * + * @return array + * + * @throws InvalidConfirmToken + */ + private static function decode(string $token): array + { + try { + $payload = json_decode(Crypt::decryptString($token), true, 512, JSON_THROW_ON_ERROR); + } catch (DecryptException|JsonException) { + throw new InvalidConfirmToken('malformed confirm token'); + } + + if (! is_array($payload) + || ! is_string($payload['nonce'] ?? null) + || ! in_array($payload['event'] ?? null, self::ACTIONS, true) + || ($payload['uid'] ?? null) !== Auth::id()) { + throw new InvalidConfirmToken('invalid confirm token'); + } + + return $payload; + } +} diff --git a/app/Support/Confirm/InvalidConfirmToken.php b/app/Support/Confirm/InvalidConfirmToken.php new file mode 100644 index 0000000..af88f90 --- /dev/null +++ b/app/Support/Confirm/InvalidConfirmToken.php @@ -0,0 +1,13 @@ + **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove the snapshot-replay vector from the recovery-codes reveal so the plaintext codes and the "revealed" state never enter any persisted Livewire property or snapshot. + +**Architecture:** The `RecoveryCodes` modal keeps zero secret state. On a fresh reveal (one-time `2fa.codes_fresh` flag in `mount()`, or `regenerate()`) it dispatches a transient `reveal-codes` browser event carrying the codes; an Alpine `x-data` holds them in JS memory only and renders them with `x-for`. Because `mount()` never runs on hydrate and dispatched events live in the one-time response `effects.dispatches` (never the snapshot), a replayed/stale snapshot has nothing to re-render and fires no event. The download route gains a Redis-backed session lock (`->block(5, 5)`) to close the concurrent-download grant race. + +**Tech Stack:** Laravel 13, Livewire 3 (class-based, not Volt), Alpine.js, wire-elements/modal, Tailwind v4, Redis (cache/session locks). All tooling runs in-container (R8). + +**Spec:** `docs/superpowers/specs/2026-06-15-recovery-codes-airtight-reveal-design.md` + +--- + +## File Structure + +- `tests/Feature/RecoveryCodesModalTest.php` — rewritten test suite (the executable spec). +- `app/Livewire/Modals/RecoveryCodes.php` — remove `$revealed`, add `dispatchCodes()`, adjust `mount()`/`regenerate()`/`render()`. +- `resources/views/livewire/modals/recovery-codes.blade.php` — Alpine `x-data` + `x-for` + `x-show` panels. +- `routes/web.php` — `->block(5, 5)` on `two-factor.recovery.download` + comment. + +**Not touched (verified during design):** +- `resources/css/app.css` — `[x-cloak] { display: none }` already present (line ~107). No change. +- `app/Livewire/Auth/TwoFactorSetup.php`, `app/Livewire/Settings/WebauthnKeys.php`, `app/Livewire/Settings/Index.php` — still set `2fa.codes_fresh` / `2fa.download_grant` as today. No change. +- No i18n changes — no new visible strings. + +**Commit strategy:** The component, view, and route changes are coupled (the suite cannot go green until all three land), so there is a single feature commit after the suite passes (Task 5). This is intentional given the coupling. + +--- + +### Task 1: Rewrite the test suite (the failing spec) + +**Files:** +- Modify (replace whole file): `tests/Feature/RecoveryCodesModalTest.php` + +- [ ] **Step 1: Replace the test file with the new spec** + +Replace the entire contents of `tests/Feature/RecoveryCodesModalTest.php` with: + +```php +create(); + $codes = $user->replaceRecoveryCodes(); + + // Opened from "Backup-Codes verwalten" with no fresh-generation flag: never reveal. + // No reveal event fires, and the codes never appear in the server-rendered HTML. + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') + ->assertDontSee($codes[0]) + ->assertDontSee($codes[7]) + ->assertSee(__('auth.recovery_hidden_notice')); + } + + public function test_fresh_flag_dispatches_codes_once_and_keeps_them_out_of_the_snapshot(): void + { + $user = User::factory()->create(); + $codes = $user->replaceRecoveryCodes(); + session(['2fa.codes_fresh' => true]); + + // The fresh flag delivers the codes via a one-time transient event — never as a + // property and never in the server-rendered HTML, so they are absent from the snapshot. + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertDispatched('reveal-codes', codes: $codes) + ->assertDontSee($codes[0]) + ->assertDontSee($codes[7]); + + // pull() forgets the flag — it is single-use. + $this->assertFalse(session()->has('2fa.codes_fresh')); + } + + public function test_replayed_or_stale_snapshot_renders_no_codes(): void + { + $user = User::factory()->create(); + $codes = $user->replaceRecoveryCodes(); + session(['2fa.codes_fresh' => true]); + + // Legitimate one-time reveal: consumes the flag, fires the event once, renders no codes. + // A re-render of the SAME component (hydrate does not re-run mount()) renders no codes — + // this models replaying a captured snapshot to /livewire/update. + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertDispatched('reveal-codes', codes: $codes) + ->assertDontSee($codes[0]) + ->call('$refresh') + ->assertDontSee($codes[0]); + + // A fresh mount from the now-consumed flag (the state a replayed snapshot starts from) + // dispatches nothing and renders nothing — codes cannot be re-revealed. + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') + ->assertDontSee($codes[0]); + } + + public function test_regenerate_dispatches_the_new_codes_without_rendering_them(): void + { + $user = User::factory()->create(); + $old = $user->replaceRecoveryCodes(); + + // Manage view (no flag) → nothing dispatched → regenerate → the NEW set is dispatched once. + $component = Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') + ->call('regenerate'); + + $new = $user->fresh()->recoveryCodes(); + $this->assertNotEquals($old, $new); + + // New set delivered via the transient event; neither the old nor the new set is in the HTML. + $component->assertDispatched('reveal-codes', codes: $new) + ->assertDontSee($old[0]) + ->assertDontSee($new[0]); + + // Regenerate also grants exactly one download of the fresh set. + $this->assertTrue(session()->has('2fa.download_grant')); + } + + public function test_download_requires_a_fresh_grant(): void + { + $user = User::factory()->create(); + $user->replaceRecoveryCodes(); + + // Without a fresh-generation grant, a direct hit of the download route is forbidden, + // so stored codes can never be re-downloaded by an authed user at will. + $this->actingAs($user) + ->get(route('two-factor.recovery.download')) + ->assertForbidden(); + + // With the one-time grant (set when codes are freshly generated/regenerated) the + // download succeeds exactly once. + session(['2fa.download_grant' => true]); + $this->actingAs($user) + ->get(route('two-factor.recovery.download')) + ->assertOk() + ->assertDownload('clusev-recovery-codes.txt'); + + // The grant is single-use: pull() consumed it, so an immediate re-download 403s. + $this->actingAs($user) + ->get(route('two-factor.recovery.download')) + ->assertForbidden(); + } + + public function test_old_recovery_route_is_gone(): void + { + $this->assertFalse(Route::has('two-factor.recovery')); + $this->assertTrue(Route::has('two-factor.recovery.download')); + } +} +``` + +Notes for the implementer: +- The `CannotUpdateLockedPropertyException` import and the old `test_revealed_property_is_locked_against_client_tampering` test are intentionally gone — the `$revealed` prop no longer exists, so there is nothing to lock. +- `assertDispatched('reveal-codes', codes: $codes)` matches the named event param against the dispatched array. `replaceRecoveryCodes()` returns the same array `recoveryCodes()` later reads, so they compare equal. +- The authoritative replay proof is the second block of `test_replayed_or_stale_snapshot_renders_no_codes` (a brand-new mount with the flag already consumed). If your Livewire version accumulates dispatch effects across roundtrips on a single testable, only keep `assertDontSee` on the `$refresh` chain (already the case here) and rely on the fresh-mount block for `assertNotDispatched`. + +- [ ] **Step 2: Run the suite to verify it fails (red)** + +Run: `docker compose exec app php artisan test --filter=RecoveryCodesModalTest` +Expected: FAIL. The current component dispatches no `reveal-codes` event and renders the codes into the HTML when `revealed`, so `test_fresh_flag_...` (expects a dispatch + `assertDontSee`), `test_replayed_...`, `test_regenerate_...`, and `test_manage_view_...` (expects `assertNotDispatched`) fail. The download tests still pass. + +Do not commit yet — the suite is red until the implementation tasks land. + +--- + +### Task 2: Strip secret state from the component, dispatch codes transiently + +**Files:** +- Modify (replace whole file): `app/Livewire/Modals/RecoveryCodes.php` + +- [ ] **Step 1: Replace the component with the stateless version** + +Replace the entire contents of `app/Livewire/Modals/RecoveryCodes.php` with: + +```php +pull('2fa.codes_fresh', false)) { + $this->dispatchCodes(); + } + } + + /** Regenerate the current user's backup codes (invalidates the old set). */ + public function regenerate(): void + { + Auth::user()->replaceRecoveryCodes(); + + AuditEvent::create([ + 'user_id' => Auth::id(), + 'actor' => Auth::user()->name ?? 'system', + 'action' => 'two_factor.recovery_regenerate', + 'target' => Auth::user()->email, + 'ip' => request()->ip(), + ]); + + // One-time download grant for the freshly regenerated set (consumed by the + // download route). Without it a later direct hit of the route 403s. + session()->put('2fa.download_grant', true); + + // Reveal the freshly generated set once, via the transient event (never a property). + $this->dispatchCodes(); + + $this->dispatch('notify', message: __('auth.recovery_regenerated')); + } + + public function render() + { + return view('livewire.modals.recovery-codes'); + } + + /** + * Deliver the current user's recovery codes to the browser via a one-time event. The + * codes never enter a Livewire property or the snapshot — Alpine holds them in JS memory + * only, so a replayed snapshot cannot re-render them. + */ + protected function dispatchCodes(): void + { + $this->dispatch('reveal-codes', codes: Auth::user()->recoveryCodes()); + } +} +``` + +What changed vs. the old file: removed the `Livewire\Attributes\Locked` import and the `#[Locked] public bool $revealed` property; `mount()` now dispatches instead of setting `$revealed`; `regenerate()` drops `$this->revealed = true` and dispatches; `render()` no longer passes a `codes` array; added `dispatchCodes()`. + +--- + +### Task 3: Render codes client-side via Alpine (never server-rendered) + +**Files:** +- Modify (replace whole file): `resources/views/livewire/modals/recovery-codes.blade.php` + +- [ ] **Step 1: Replace the view with the Alpine version** + +Replace the entire contents of `resources/views/livewire/modals/recovery-codes.blade.php` with: + +```blade +
+
+ + + +
+

{{ __('auth.recovery_heading') }}

+

{{ __('auth.recovery_subtitle') }}

+
+
+ + {{-- Revealed panel: shown only once codes arrive via the transient `reveal-codes` event. + The codes live in Alpine JS memory (x-for) and are never in the server HTML/snapshot. --}} +
+
+ +

{{ __('auth.recovery_warning') }}

+
+ +
+ +
+ +
+ + {{ __('auth.recovery_download') }} + + {{ __('common.close') }} +
+
+ + {{-- Hidden notice: the default state (no codes in memory) — manage view, or any replayed + snapshot, which fires no event and therefore never populates `codes`. --}} +
+
+ +

{{ __('auth.recovery_hidden_notice') }}

+
+ +
+ + {{ __('auth.recovery_regenerate') }} + + {{ __('common.close') }} +
+
+
+``` + +Notes: +- `[x-cloak] { display: none }` already exists in `resources/css/app.css` — no CSS change needed. It hides the modal body until Alpine initializes, preventing a raw-template flash. +- Both panels exist in the server HTML; `x-show` toggles visibility client-side. The codes themselves come only from `x-for` over the JS `codes` array, so the server HTML/snapshot contains no codes. The `assertDontSee($codes[0])` assertions therefore hold in every state. + +--- + +### Task 4: Add the session lock to the download route + +**Files:** +- Modify: `routes/web.php` (the `two-factor.recovery.download` route, around line 57-70) + +- [ ] **Step 1: Append `->block(5, 5)` to the route** + +Find this line (end of the download route definition): + +```php + })->name('two-factor.recovery.download'); +``` + +Replace it with: + +```php + })->name('two-factor.recovery.download') + // Session blocking (Redis-backed atomic lock) serializes concurrent same-session + // requests so the one-time `2fa.download_grant` cannot be double-read by a racing + // request. True concurrency cannot be unit-tested; lock-capable stores (redis in + // prod, array/file in tests) keep the happy path green. + ->block(5, 5); +``` + +Leave the route body (the `abort_unless(session()->pull('2fa.download_grant', false), 403)` and the streamed download) unchanged. + +--- + +### Task 5: Green suite, browser verification, Codex review, commit + +**Files:** none (verification + commit) + +- [ ] **Step 1: Run the full recovery-codes suite (expect green)** + +Run: `docker compose exec app php artisan test --filter=RecoveryCodesModalTest` +Expected: PASS — all of `test_manage_view_...`, `test_fresh_flag_...`, `test_replayed_...`, `test_regenerate_...`, `test_download_requires_a_fresh_grant`, `test_old_recovery_route_is_gone`. + +If `test_download_requires_a_fresh_grant` errors on the lock (rather than failing an assertion), confirm the test cache store supports atomic locks (`array`/`file`/`redis` do). If your test env uses an unsupported store, set `CACHE_STORE=array` for the test run; do not remove `->block()`. + +- [ ] **Step 2: Run the broader suite to catch regressions** + +Run: `docker compose exec app php artisan test --filter=Recovery` +Expected: PASS (includes `UserRecoveryCodesTest`, `FirstFactorCodesTest`-style coverage that touches `codes_fresh`). Then run the full `docker compose exec app php artisan test` if time allows. + +- [ ] **Step 3: Browser verification (R12)** + +With the dev stack up (`docker compose up -d`), load both states and check HTTP 200 + zero console/network errors at 375 / 768 / 1280: +- Fresh reveal: trigger first-factor enrollment (or `regenerate` from the modal) → confirm the 8 codes render and the download button appears. +- Manage view: open the modal from Settings → Security ("Backup-Codes verwalten") with no fresh flag → confirm the hidden notice + regenerate button, no codes. +- In DevTools Network, open the `openModal` (Livewire `/livewire/update`) response for the fresh reveal and confirm the codes appear ONLY under `effects.dispatches` (the `reveal-codes` event) and are ABSENT from `serverMemo`/`snapshot` and the rendered `html`. +- Inspect the rendered DOM for leaked `@`/`{{ }}`/`$var`/`group.key` text (R17). + +- [ ] **Step 4: Codex review (R15)** + +Run `/codex:review` over the change. Fix and re-run until it reports no errors and no security issues. The task is not done until Codex passes. + +- [ ] **Step 5: Commit** + +```bash +cd /home/nexxo/clusev +git checkout -b feature/recovery-codes-airtight-reveal +git add app/Livewire/Modals/RecoveryCodes.php \ + resources/views/livewire/modals/recovery-codes.blade.php \ + routes/web.php \ + tests/Feature/RecoveryCodesModalTest.php \ + docs/superpowers/specs/2026-06-15-recovery-codes-airtight-reveal-design.md \ + docs/superpowers/plans/2026-06-15-recovery-codes-airtight-reveal.md +git commit -m "feat(2fa): make recovery-codes reveal airtight against snapshot replay + +Codes and reveal-state no longer enter any Livewire property/snapshot — they +are delivered via a transient reveal-codes event and rendered client-side by +Alpine, so a replayed/stale snapshot re-renders nothing. Add ->block(5,5) +session lock to the download route to close the grant race." +``` + +(Run `git status` first to confirm no stray secrets are staged. Do not push unless asked.) + +--- + +## Self-Review + +**Spec coverage:** +- Threat model / structural show-once → Task 2 (no `$revealed`, no codes prop; mount/regenerate dispatch only). ✓ +- Transient dispatch + Alpine render → Task 2 (`dispatchCodes`) + Task 3 (`x-data`/`x-for`/`x-show`). ✓ +- Download-race lock → Task 4 (`->block(5, 5)`). ✓ +- Replay-proof tests → Task 1 (`test_replayed_or_stale_snapshot_renders_no_codes`). ✓ +- Removed locked-prop test → Task 1 (dropped + import removed). ✓ +- No i18n / no CSS change → confirmed in File Structure (x-cloak already present). ✓ +- Verification R12/R15 → Task 5. ✓ + +**Placeholder scan:** No TBD/TODO; every code step shows full file or exact edit. ✓ + +**Type/name consistency:** `dispatchCodes()` defined in Task 2 and referenced only there; event name `reveal-codes` and detail key `codes` match across component dispatch (Task 2), Alpine listener `$event.detail.codes` (Task 3), and tests `assertDispatched('reveal-codes', codes: ...)` (Task 1). Route name `two-factor.recovery.download` unchanged. ✓ + +--- + +## Post-review additions (Codex review, 2026-06-15) + +Codex raised three findings; resolutions: + +- **F1 (Alpine reveal-state persists across morphs):** Not an exploitable vector — the + snapshot-replay threat needs the codes *in the snapshot*, and they are not. The persisting + state is client-side JS memory in the same tab that already saw the codes legitimately. No + code change; proven by the snapshot assertions below. +- **F2 (download grant had no expiry):** Implemented a 10-minute TTL. The grant is now stored + as `now()->timestamp` by all three setters (`Modals/RecoveryCodes::regenerate()`, + `Auth/TwoFactorSetup.php`, `Settings/WebauthnKeys.php`), and `routes/web.php` rejects any + non-integer or >600s-old grant: + `abort_unless(is_int($grantedAt) && now()->timestamp - $grantedAt <= 600, 403);` + New test `test_a_stale_download_grant_is_rejected` covers the stale, legacy-bool, and fresh + cases. The existing `test_download_requires_a_fresh_grant` now seeds `now()->timestamp`. +- **F3 (snapshot leak not directly tested):** Added explicit snapshot assertions via a + `assertSnapshotHidesCodes(array $codes, $component)` helper that loops over EVERY code and + asserts `assertStringNotContainsString($code, json_encode($component->snapshot))` in the + fresh-flag, replay, and regenerate tests, plus `assertNotDispatched('reveal-codes')` after + `$refresh` (hydration). +- **F4 (state-changing GET is CSRF-exposable — a cross-site request could burn the victim's + one-time grant):** Changed the download route `GET`→`POST` (CSRF-protected) and made the + view's download control a `
` + `@csrf`; download tests use `->post(...)`. + No code disclosure was possible (cross-origin response is unreadable); the fix prevents the + grant-consumption nuisance. + +Final suite: 7 tests / 61 assertions, all green. diff --git a/docs/superpowers/specs/2026-06-15-confirm-action-token-hardening-design.md b/docs/superpowers/specs/2026-06-15-confirm-action-token-hardening-design.md new file mode 100644 index 0000000..196125d --- /dev/null +++ b/docs/superpowers/specs/2026-06-15-confirm-action-token-hardening-design.md @@ -0,0 +1,331 @@ +# Confirm-Action token hardening — design + +**Date:** 2026-06-15 +**Scope:** `app/Livewire/Modals/ConfirmAction.php` + its 14 call sites/handlers across 8 components. +**Driver:** Codex v0.9.0 review (P2) on the generic R5 confirm modal. + +--- + +## 1. Problem + +`ConfirmAction` is the shared R5 confirm modal, reused by every confirm flow. It carries +client-mutable public properties — `event`, `params`, `auditAction`, `auditTarget`, `serverId` — +that it dispatches and audits on `confirm()`. An authenticated user (all accounts are equal +admins — no RBAC) can: + +- **(a) Bypass the modal** — dispatch a `#[On(...)]` apply handler directly (e.g. `keyRemoved`) + from the browser, skipping the human confirmation step entirely. +- **(b) Tamper with routing** — substitute `event` / `params` / `auditTarget` to retarget the + destructive action or forge a misleading audit row. + +This is **not** a privilege-boundary crossing: the apply handlers already re-validate the real +invariants (can't delete self/last user, channel/tls/domain are allowlisted and re-checked). The +fix is therefore scoped to **integrity / confirmation**, not authorization tiers: + +1. Replace arbitrary client-supplied `event` dispatch with a **server-side allowlist**. +2. Mark the modal's routing inputs **`#[Locked]`**. +3. Bind each confirm to a **signed, single-use token** issued server-side when the modal opens, so + a direct apply-handler call without a valid token is rejected and the audit descriptor cannot + be forged. + +**Out of scope (flagged as follow-up):** the sibling modals with their own client-mutable routing — +`HardeningAction` (mutable `serverId`/`action`/`enable`) and `Fail2banBan` (mutable `serverId`). +Same class of exposure, lower severity (each does its own work in `apply()`/`save()` and +re-validates the server). Tracked separately. + +--- + +## 2. The 14 ConfirmAction flows (inventory) + +| Event | Opener | Handler (component) | Params | Audit written in | +|---|---|---|---|---| +| `serviceConfirmed` | Services/Index | `applyService` (Services/Index) | op, name | confirm() | +| `userRemoved` | Settings/Users | `remove` (Settings/Users) | userId | handler (deferred) | +| `userLoggedOut` | Settings/Users | `logout` (Settings/Users) | userId | handler (deferred) | +| `domainChanged` | System/Index | `applyDomain` (System/Index) | domain | confirm() | +| `channelChanged` | System/Index | `applyChannel` (System/Index) | channel | confirm() | +| `tlsModeChanged` | System/Index | `applyTlsMode` (System/Index) | mode | handler (own audit) | +| `fileConfirmed` | Files/Index | `deleteEntry` (Files/Index) | name | confirm() | +| `keyRemoved` | Servers/Show | `removeKey` (Servers/Show) | fingerprint | confirm() | +| `credentialDeleted` | Servers/Show | `deleteCredential` (Servers/Show) | — | confirm() | +| `firewallRuleDelete` | Servers/Show | `deleteFirewallRule` (Servers/Show) | spec, label | handler (deferred) | +| `webauthnKeyRemoved` | Settings/WebauthnKeys | `remove` (Settings/WebauthnKeys) | id | handler (own audit) | +| `sessionsLogoutOthers` | Settings/Sessions | `logoutOthers` (Settings/Sessions) | — | confirm() | +| `sessionsLogoutAll` | Settings/Sessions | `logoutAll` (Settings/Sessions) | — | confirm() | +| `twoFactorDisabled` | Settings/Security | `disableTwoFactor` (Settings/Security) | — | confirm() | + +The split (audit in `confirm()` vs. in the handler) is the **existing** architecture and is +**preserved** — `confirm()` writes audit only when the opener supplied `auditAction`; deferred +flows audit in the handler after the remote op succeeds. + +--- + +## 3. New unit: `App\Support\Confirm\ConfirmToken` + +A small stateless service. Lives at `app/Support/Confirm/ConfirmToken.php`; companion exception +`app/Support/Confirm/InvalidConfirmToken.php` (`extends \RuntimeException`). + +### Responsibilities +- **Allowlist** of permitted confirm events (the 14 above) as a `public const ACTIONS`. +- **issue** — server-side token minting at opener time. +- **verify** — decode + validate without consuming (modal display + `confirm()`). +- **consume** — verify + single-use burn + event match (apply handlers — the security gate). +- **peek** — signature-only decode for display, never throws. + +### Token shape +Payload (JSON) → `Crypt::encryptString(...)`: + +``` +{ nonce, event, params, auditAction, auditTarget, serverId, uid } +``` + +- `Crypt` (APP_KEY, AES-256 + HMAC) gives **tamper-evidence** — a forged/edited token fails to + decrypt or fails the MAC. +- **Single-use** via cache: `issue` plants `confirm-token:{nonce}` (Redis, TTL 600s); + `consume` does `Cache::pull` (atomic get+delete). A replay finds nothing → rejected. +- `uid` is bound to `Auth::id()` and re-checked on decode → no cross-user replay. + +### API (signatures) +```php +public const ACTIONS = [ /* the 14 event names */ ]; + +public static function issue( + string $event, array $params = [], string $auditAction = '', + ?string $auditTarget = null, ?int $serverId = null, +): string; // throws \InvalidArgumentException if event ∉ ACTIONS + +public static function verify(string $token): array; // throws InvalidConfirmToken +public static function consume(string $token, string $event): array; // throws InvalidConfirmToken +public static function peek(string $token): ?array; // null on any failure +``` + +### Validation rules (decode) +- decrypts cleanly (catch `DecryptException`) and is valid JSON (catch `JsonException`); +- `event` ∈ `ACTIONS`; +- `uid` === `Auth::id()`. + +`verify` adds: nonce still present in cache. `consume` adds: `event` arg matches payload `event`, +and `Cache::pull` returns truthy (single-use). On any failure → `InvalidConfirmToken`. + +--- + +## 4. Modal rewrite: `ConfirmAction` + +- **Remove** public props `event`, `params`, `auditAction`, `auditTarget`, `serverId`. +- Add `#[Locked] public string $token = ''`. +- Keep display props `heading`, `body`, `confirmLabel`, `danger`, `icon`, `notify`. +- Replace `boot()` with `mount()`: assign `$token` + display copy, apply the localized fallbacks + (`default_heading`, `common.confirm`, `default_notify`) that `boot()` did. Setting `$token` in + `mount()` is the initial mount → allowed by `#[Locked]`; subsequent client round-trips (the + confirm click) cannot re-tamper it. +- Audit-target display line → `#[Computed] public function targetLabel(): ?string` using + `ConfirmToken::peek($this->token)['auditTarget'] ?? null`. Not client state → no tamper surface. + +```php +public function confirm(): void +{ + try { + $payload = ConfirmToken::verify($this->token); // non-consuming + } catch (InvalidConfirmToken) { + $this->dispatch('notify', message: __('modals.confirm_action.rejected')); + $this->closeModal(); + return; + } + + if (($payload['auditAction'] ?? '') !== '') { + AuditEvent::create([ + 'user_id' => Auth::id(), + 'server_id' => $payload['serverId'] ?? null, + 'actor' => Auth::user()?->name ?? 'system', + 'action' => $payload['auditAction'], + 'target' => $payload['auditTarget'] ?? null, + 'ip' => request()->ip(), + ]); + } + + $this->dispatch($payload['event'], confirmToken: $this->token); + + if ($this->notify !== '') { + $this->dispatch('notify', message: $this->notify); + } + + $this->closeModal(); +} +``` + +**Blade change** (`resources/views/livewire/modals/confirm-action.blade.php`): the audit-target +line switches from `@if ($auditTarget) ... {{ $auditTarget }}` to `@if ($this->targetLabel) ... +{{ $this->targetLabel }}`. No other markup changes. + +**Implementation risk to verify first:** confirm wire-elements/modal passes the `arguments` +through to `mount()` (it does — same path the other modals use). If a `#[Locked]` public prop set +from `arguments` ever threw, the `mount()` assignment avoids it; both paths covered. + +--- + +## 5. Openers (14, across 8 files) + +Replace the routing arguments (`event`, `params`, `auditAction`, `auditTarget`, `serverId`) with a +single `'token'`. Display copy is unchanged. Example (`Servers/Show::confirmRemoveKey`): + +```php +$this->dispatch('openModal', + component: 'modals.confirm-action', + arguments: [ + 'heading' => __('servers.confirm_remove_key_heading'), + 'body' => __('servers.confirm_remove_key_body', ['name' => $comment, 'server' => $this->server->name]), + 'confirmLabel' => __('common.remove'), + 'danger' => true, + 'icon' => 'trash', + 'notify' => __('servers.notify_key_removed', ['name' => $comment]), + 'token' => ConfirmToken::issue( + 'keyRemoved', + ['fingerprint' => $fingerprint], + 'ssh_key.remove', + "{$comment} · {$this->server->name}", + $this->server->id, + ), + ], +); +``` + +Openers that passed no `auditAction` (deferred flows) pass `''`; no `serverId` → `null`. + +--- + +## 6. Apply handlers (14) + +Replace domain params with `string $confirmToken`; consume + read params from the verified +payload. Injected services (by type) coexist with the token param (by name). Example: + +```php +#[On('keyRemoved')] +public function removeKey(string $confirmToken, FleetService $fleet): void +{ + try { + $payload = ConfirmToken::consume($confirmToken, 'keyRemoved'); + } catch (InvalidConfirmToken) { + return; // forged / replayed / direct-bypass attempt — silent no-op + } + $fingerprint = $payload['params']['fingerprint']; + // ... unchanged ... +} +``` + +Rules: +- Handlers read **only** `$payload['params']` — any client-passed value is ignored. +- Handlers that already re-validate (`applyChannel`, `applyTlsMode`, `applyDomain`, `remove`, + `logout`) keep those checks — defense in depth. +- Handlers stay **silent** on a rejected token (no toast) — no oracle for a prober. The + user-facing rejection notice is shown by the modal's `confirm()` only. + +--- + +## 7. Security model + +- **Single-use consume at the handler** — the destructive op is the gate. A direct `#[On]` call + without a valid, unburned token is a no-op. → Closes vector (a). +- **event / params / auditTarget come only from the signed token** — they can't be retargeted and + the audit descriptor can't be forged. → Closes vector (b). +- `confirm()` verifies non-consuming and writes audit from the verified payload; the handler burns + the token single-use. +- **Residual (documented, accepted):** a scripted `confirm()` replay *before* the handler round-trip + consumes the nonce could write duplicate audit rows — but they are **correctly attributed** to + the actor for an action they authorized. Negligible under the equal-admins model. After the + handler burns the nonce, all replays fail `verify`. + +--- + +## 8. Tests + +**Unit — `tests/Unit/Support/Confirm/ConfirmTokenTest.php`** (`actingAs` a user, `array` cache): +- `issue` → `verify`/`consume` round-trips and returns the payload. +- `issue` with an event ∉ `ACTIONS` throws `\InvalidArgumentException`. +- `consume` twice → second throws `InvalidConfirmToken` (single-use). +- `consume($token, 'otherEvent')` → throws (cross-action / event mismatch). +- garbage / edited token → `verify` & `consume` throw; `peek` returns null. +- token issued as user A, `Auth::id()` = user B → throws (uid binding). + +**Feature — `tests/Feature/Modals/ConfirmActionTest.php`** (`RefreshDatabase`): +- happy path: mount with a valid token → `call('confirm')` → asserts the event was dispatched with + `confirmToken` and (for an `auditAction` flow) exactly one `AuditEvent` row written from the + payload. +- forged token: `call('confirm')` → no `AuditEvent`, event not dispatched, `notify` = + `modals.confirm_action.rejected`. +- `->set('token', '...')` on the mounted modal throws (Locked). + +**Feature — handler bypass** (representative, no SSH): `tests/Feature/Settings/SecurityConfirmTest.php`: +- dispatch `twoFactorDisabled` with no / invalid `confirmToken` → 2FA stays enabled, no audit. +- with a valid issued token → 2FA disabled; a second dispatch with the same token → no-op + (single-use). + +All tests use `RefreshDatabase` + factories — **never** seed the dev DB (per project rule). + +--- + +## 9. Localization (R16) + +Add to `lang/de/modals.php` and `lang/en/modals.php` under `confirm_action`: +- `rejected` — de: `Bestätigung ungültig oder abgelaufen.` · en: `Confirmation invalid or expired.` + +(Keys identical in both files.) + +--- + +## 10. Verification (R8 / R12 / R15) + +1. Run the suite in-container: + `docker compose exec app php artisan test --filter='ConfirmToken|ConfirmAction|SecurityConfirm'` + then the full suite. +2. **R12 browser probe** every touched route — HTTP 200, zero console/network errors, loaded + state (not skeleton), at 375 / 768 / 1280: `servers/show`, `services`, `files`, `system`, + `settings/security`, `settings/sessions`, `settings/users`, `settings/webauthn-keys`. Open a + confirm modal on each and confirm one action end-to-end. +3. **R15** `/codex:review` over the diff → fix + re-run until 0 errors / 0 security findings. +4. `git status` for stray secrets → commit in sensible steps. + +--- + +## 11. File touch list + +**New:** `app/Support/Confirm/ConfirmToken.php`, `app/Support/Confirm/InvalidConfirmToken.php`, +3 test files. +**Edit:** `app/Livewire/Modals/ConfirmAction.php`, +`resources/views/livewire/modals/confirm-action.blade.php`, +`app/Livewire/Services/Index.php`, `app/Livewire/Settings/Users.php`, +`app/Livewire/Settings/Security.php`, `app/Livewire/Settings/Sessions.php`, +`app/Livewire/Settings/WebauthnKeys.php`, `app/Livewire/System/Index.php`, +`app/Livewire/Files/Index.php`, `app/Livewire/Servers/Show.php`, +`lang/de/modals.php`, `lang/en/modals.php`. + +--- + +## 12. Post-review revisions (Codex R15) + +The first Codex pass REJECTED the initial design (§3–§7). Three classes of finding were +valid and are now folded into the implementation: + +**A. Two-phase lifecycle (was: single nonce checked at confirm + consume).** +The original token was consumable the instant `issue()` returned, so an attacker who invoked +an opener could grab the token and call the apply handler directly — still skipping the modal +(and, for confirm-audited flows, skipping the audit). The nonce now carries state: +`issue()` → `pending`; `ConfirmToken::confirm($token)` (called only by the modal's `confirm()`) +flips `pending` → `confirmed`; `consume($token, $event)` requires `confirmed`, then deletes. +A never-confirmed token is rejected by the handler — the modal must run. This also removes the +§7 "duplicate-audit on replay" residual: `confirm()` transitions at most once. + +**B. Atomic transitions.** `Cache::pull` is get-then-forget (not atomic), so concurrent handler +requests could double-apply. Both `confirm()` and `consume()` now run their check-and-mutate +inside `Cache::lock($key.':lock', 10)->get(...)` (array + Redis both provide locks). + +**C. Server-scope binding.** Server-scoped actions seal the server id and the handler verifies it +against live state before acting — closing token retargeting to another host/path: +- `Servers/Show` `removeKey`/`deleteCredential`/`deleteFirewallRule`: reject unless + `$payload['serverId'] === $this->server->id` (firewall opener now seals `serverId`). +- `Files/Index` `deleteEntry`: opener seals the **full path** + active server id; handler rejects + on server mismatch and deletes the **sealed** path (never a client-recombined one). +- `Services/Index` `applyService`: opener seals the active server id; handler rejects on mismatch. + +Method shape became: `issue()`, `confirm()` (modal), `consume()` (handler), `peek()` (display). +`verify()` was removed. Added tests: `ConfirmServerScopeTest`, plus two-phase / unconfirmed-token +cases in the unit + Security/Modal feature tests. diff --git a/docs/superpowers/specs/2026-06-15-recovery-codes-airtight-reveal-design.md b/docs/superpowers/specs/2026-06-15-recovery-codes-airtight-reveal-design.md new file mode 100644 index 0000000..aeac014 --- /dev/null +++ b/docs/superpowers/specs/2026-06-15-recovery-codes-airtight-reveal-design.md @@ -0,0 +1,171 @@ +# Design — Airtight show-once recovery codes + +Date: 2026-06-15 +Status: Approved (brainstorming) — ready for implementation plan +Scope: low-priority hardening of the "show backup codes only once" feature + +## 1. Problem + +The recovery-codes reveal (`app/Livewire/Modals/RecoveryCodes.php` + view) gates display +behind a `#[Locked] public bool $revealed` prop, a one-time `2fa.codes_fresh` session flag, +and a `2fa.download_grant`-gated download route. + +A Codex review noted a residual, low-severity vector: `#[Locked]` blocks property-update +payloads but NOT Livewire snapshot hydration. A client that captured the `revealed=true` +snapshot during the legitimate one-time reveal could replay that signed snapshot to +re-render the (same) stored codes. + +Marginal exposure is ~zero — the replayer already saw those codes at the legitimate reveal, +same user, no privilege escalation — so it was accepted as an inherent Livewire-statelessness +limitation. This design removes the vector entirely for teams that want the show-once +guarantee to be structural rather than UX-level. + +## 2. Threat model + +Kill vector: a captured `revealed=true` snapshot replayed to `/livewire/update` re-renders +stored codes server-side. + +Fix principle: the plaintext codes and the reveal-state must NEVER enter any persisted +Livewire property or snapshot. + +- `mount()` does NOT run on hydrate — only on the initial component mount. So a replayed + snapshot never re-triggers mount-time logic. +- PHP-side dispatched events (`$this->dispatch(...)`) ride in the one-time response + `effects.dispatches`, NOT in the snapshot. A replayed snapshot carries no dispatch. + +Therefore: if codes are delivered only via a transient dispatched event fired from +`mount()`/`regenerate()`, and the component holds no codes/`revealed` state, a replayed or +stale snapshot has literally nothing to re-render and fires no event. + +## 3. Approach (selected) + +Transient dispatch → Alpine client-side render. Chosen over (a) a server-rendered design +gated by a single-use hydrate nonce (more moving parts, doesn't cleanly kill the same-window +race) and (b) a non-modal full-page reveal (breaks the wire-elements/modal UX across all four +open sites). Approach below has the smallest blast radius and is idiomatic Livewire 3 + Alpine. + +All four modal-open sites go through a Livewire `openModal` roundtrip (wire-elements/modal), +so `mount()` always runs inside a Livewire request and the dispatched event always rides back +in that response. Open sites confirmed: +- `app/Livewire/Auth/TwoFactorSetup.php` (first-factor enrollment → `put('2fa.codes_fresh')` + + flash `open_recovery_modal`, redirect to settings) +- `app/Livewire/Settings/Index.php` + `resources/views/livewire/settings/index.blade.php` + (`x-init="$dispatch('openModal', ...)"` on the post-enrollment auto-open) +- `app/Livewire/Settings/WebauthnKeys.php` (`$this->dispatch('openModal', ...)` after setting + the flags) +- `resources/views/livewire/settings/security.blade.php` (manage button — no fresh flag → hidden) + +## 4. Component — `app/Livewire/Modals/RecoveryCodes.php` + +- Delete `#[Locked] public bool $revealed`. No codes property. The component holds zero + secret state. +- `mount()`: `if (session()->pull('2fa.codes_fresh', false)) { $this->dispatchCodes(); }` +- `regenerate()`: `replaceRecoveryCodes()` + `AuditEvent::create([...])` (unchanged) + + `session()->put('2fa.download_grant', true)` + `$this->dispatchCodes()` + notify. Drop the + `$this->revealed = true` line. +- New `protected function dispatchCodes(): void`: + `$this->dispatch('reveal-codes', codes: Auth::user()->recoveryCodes());` +- `render()`: returns the view with NO codes payload (drop the `'codes' => ...` array). + +Codes leave the server only inside the transient `reveal-codes` event detail — same +visibility to the legitimate user, absent from the snapshot and from the server-rendered HTML. + +## 5. View — `resources/views/livewire/modals/recovery-codes.blade.php` + +- Root element: `x-data="{ codes: [] }" @reveal-codes.window="codes = $event.detail.codes"`, + plus `x-cloak` to suppress any raw-template flash before Alpine initializes. +- Codes grid: replace the Blade `@foreach ($codes as $c)` with + ``. +- Reveal panel (warning + grid + download button + close) wrapped in `x-show="codes.length"`. +- Hidden-notice panel (notice + regenerate button + close) wrapped in `x-show="!codes.length"`. +- Dependency: ensure `[x-cloak] { display: none }` exists in `resources/css/app.css`; add it if + missing. + +Known minor UX: on a fresh reveal there is a ~1-tick window where the hidden-notice panel can +flash before the `reveal-codes` event lands and flips `codes`. No secret is exposed by this; +accepted. + +## 6. Download route — `routes/web.php` + +Append `->block(5, 5)` to the `two-factor.recovery.download` route. Laravel session blocking +(Redis-backed atomic lock — Redis is in the stack) serializes concurrent same-session requests +so the `session()->pull('2fa.download_grant')` cannot be double-read by a racing request. The +`array`/`file` cache stores are also lock-capable, so the test suite is unaffected. Add a +one-line comment noting the lock closes the concurrent-download race (true concurrency cannot +be unit-tested). + +Grant TTL (added after Codex review — F2 defense-in-depth): the grant is stored as a Unix +timestamp instead of a bool, and the route rejects any grant that is not an integer or is older +than 10 minutes (600s): + +```php +$grantedAt = session()->pull('2fa.download_grant'); +abort_unless(is_int($grantedAt) && now()->timestamp - $grantedAt <= 600, 403); +``` + +This bounds an unused grant's replay window to 10 minutes instead of the whole authenticated +session. All three grant setters store `now()->timestamp` (see §8). + +CSRF (added after Codex review — F4): consuming the one-time grant is a state change, so the +route is `POST` (not `GET`) and carries Laravel's CSRF protection. The download control in the +view is a small `` with `@csrf` wrapping the button — behaves identically to +a download link, but a cross-site request can no longer burn the victim's grant. + +## 7. Tests — `tests/Feature/RecoveryCodesModalTest.php` + +- Remove `test_revealed_property_is_locked_against_client_tampering` (the `revealed` prop no + longer exists) and the now-unused `CannotUpdateLockedPropertyException` import. +- `manage view (no flag)`: `assertNotDispatched('reveal-codes')` + `assertDontSee($codes[0])` + + `assertSee(__('auth.recovery_hidden_notice'))`. +- `fresh flag reveals once`: `assertDispatched('reveal-codes', codes: $codes)` AND + `assertDontSee($codes[0])` (proves codes are absent from the server-rendered HTML/snapshot) + AND `assertFalse(session()->has('2fa.codes_fresh'))` (one-time). +- Replay proof: after the fresh mount, `->call('$refresh')` → + `assertNotDispatched('reveal-codes')` + `assertDontSee($codes[0])`; PLUS a second fresh + `Livewire::test(RecoveryCodes::class)` (the flag is already pulled) → same assertions. This + models a replayed/stale snapshot yielding zero codes (mount does not re-run on hydrate, and + the one-time flag is already consumed). +- `regenerate`: `assertDispatched('reveal-codes', codes: $new)` + `assertDontSee($old[0])` + + `assertDontSee($new[0])` (codes never in HTML) + `assertTrue(session()->has('2fa.download_grant'))`. +- Keep `test_download_requires_a_fresh_grant` and `test_old_recovery_route_is_gone` unchanged — + `->block()` leaves the happy path green. + +## 8. Grant setters + untouched + +- Grant setters (updated for the TTL — F2): `Auth/TwoFactorSetup.php`, + `Settings/WebauthnKeys.php`, and `Modals/RecoveryCodes::regenerate()` now store + `session()->put('2fa.download_grant', now()->timestamp)` instead of `true`. They still set + `2fa.codes_fresh` unchanged. +- `Settings/Index.php` is unchanged; the modal now consumes `codes_fresh` by dispatching the + codes instead of flipping a `revealed` bool. +- No i18n changes: no new visible strings (the heading, subtitle, warning, hidden-notice, and + button labels already exist under `auth.recovery_*` / `common.*`). The `x-for` renders raw + code strings, which are not translatable. R9 / R16 not applicable. + +## 9. Verification (R12 + R15) + +- Browser-load both states at 375 / 768 / 1280: + - fresh-enrollment reveal (codes render via Alpine), + - manage view (hidden notice, no codes). +- HTTP 200, zero console/network errors; for the lazy modal check the loaded state. +- Confirm in the openModal network response that the codes are ABSENT from the snapshot and the + server-rendered HTML, present only in `effects.dispatches`. Inspect the rendered DOM for + leaked `@`/`{{ }}`/`$var`/`group.key` text (R17). +- All tooling runs in-container (R8). Destructive/confirm UI stays wire-elements/modal (R5). +- Run `/codex:review` over the change; fix and re-run until it reports no errors and no security + issues (R15). The task is not done until Codex passes. + +## 10. Files touched + +1. `app/Livewire/Modals/RecoveryCodes.php` — remove prop, add `dispatchCodes()`, adjust + `mount()` / `regenerate()` / `render()`; grant stored as timestamp. +2. `resources/views/livewire/modals/recovery-codes.blade.php` — Alpine `x-data` + `x-for` + + `x-show` panels + `x-cloak`. +3. `routes/web.php` — download route changed `GET`→`POST` (CSRF) + `->block(5, 5)` + + timestamp-grant TTL (≤600s). +4. `tests/Feature/RecoveryCodesModalTest.php` — replace the locked-prop test; add dispatch + + replay + snapshot-inspection + regenerate + stale-grant assertions. +5. `app/Livewire/Auth/TwoFactorSetup.php`, `app/Livewire/Settings/WebauthnKeys.php` — grant + setters store `now()->timestamp` (TTL, F2). + +`resources/css/app.css` is NOT touched — `[x-cloak] { display: none }` is already present. diff --git a/lang/de/modals.php b/lang/de/modals.php index 0d95f1c..a4b533a 100644 --- a/lang/de/modals.php +++ b/lang/de/modals.php @@ -190,5 +190,6 @@ return [ 'confirm_action' => [ 'default_heading' => 'Aktion bestätigen', 'default_notify' => 'Aktion ausgeführt.', + 'rejected' => 'Bestätigung ungültig oder abgelaufen.', ], ]; diff --git a/lang/en/modals.php b/lang/en/modals.php index c681ca4..8317c42 100644 --- a/lang/en/modals.php +++ b/lang/en/modals.php @@ -189,5 +189,6 @@ return [ 'confirm_action' => [ 'default_heading' => 'Confirm action', 'default_notify' => 'Action completed.', + 'rejected' => 'Confirmation invalid or expired.', ], ]; diff --git a/resources/views/livewire/modals/confirm-action.blade.php b/resources/views/livewire/modals/confirm-action.blade.php index d6e5d21..9472f1b 100644 --- a/resources/views/livewire/modals/confirm-action.blade.php +++ b/resources/views/livewire/modals/confirm-action.blade.php @@ -10,8 +10,8 @@

{{ $heading }}

{{ $body }}

- @if ($auditTarget) -

{{ $auditTarget }}

+ @if ($this->targetLabel) +

{{ $this->targetLabel }}

@endif
diff --git a/resources/views/livewire/modals/recovery-codes.blade.php b/resources/views/livewire/modals/recovery-codes.blade.php index aa7d9d7..04bc574 100644 --- a/resources/views/livewire/modals/recovery-codes.blade.php +++ b/resources/views/livewire/modals/recovery-codes.blade.php @@ -1,4 +1,4 @@ -
+
@@ -9,25 +9,36 @@
- @if ($revealed) + {{-- Revealed panel: shown only once codes arrive via the transient `reveal-codes` event. + The codes live in Alpine JS memory (x-for) and are never in the server HTML/snapshot. --}} +

{{ __('auth.recovery_warning') }}

- @foreach ($codes as $c) - {{ $c }} - @endforeach +
- - {{ __('auth.recovery_download') }} - + {{-- POST + @csrf: consuming the one-time grant is a state change, so a cross-site + request cannot burn the grant. Behaves identically to a download link. --}} + + @csrf + + {{ __('auth.recovery_download') }} + + {{ __('common.close') }}
- @else +
+ + {{-- Hidden notice: the default state (no codes in memory) — manage view, or any replayed + snapshot, which fires no event and therefore never populates `codes`. --}} +

{{ __('auth.recovery_hidden_notice') }}

@@ -39,5 +50,5 @@ {{ __('common.close') }}
- @endif +
diff --git a/routes/web.php b/routes/web.php index 093fda7..56e9815 100644 --- a/routes/web.php +++ b/routes/web.php @@ -54,11 +54,17 @@ Route::middleware('auth')->group(function () { Route::get('/two-factor-setup', Auth\TwoFactorSetup::class)->name('two-factor.setup'); // 2FA backup-codes download — the codes themselves are shown in a modal (Modals\RecoveryCodes). - Route::get('/two-factor/recovery-codes/download', function () { - // One download per fresh generation: a one-time grant is put alongside the codes - // when they are freshly generated/regenerated. pull() consumes it, so re-downloading - // stored codes by hitting this route directly 403s. - abort_unless(session()->pull('2fa.download_grant', false), 403); + // POST (not GET): consuming the one-time grant is a state change, so it must carry a CSRF + // token — a cross-site GET cannot burn the victim's grant. + Route::post('/two-factor/recovery-codes/download', function () { + // One download per fresh generation, and only briefly. The grant is a timestamp put + // alongside the codes when they are freshly generated/regenerated. pull() consumes it + // (single-use, so re-downloading stored codes by hitting this route directly 403s), + // and a grant older than 10 minutes — or any non-timestamp value — is rejected, so an + // unused grant cannot linger and be replayed later in the session. ->block() below + // serializes racing reads of the grant. + $grantedAt = session()->pull('2fa.download_grant'); + abort_unless(is_int($grantedAt) && now()->timestamp - $grantedAt <= 600, 403); $codes = AuthFacade::user()->recoveryCodes(); @@ -67,7 +73,12 @@ Route::middleware('auth')->group(function () { 'clusev-recovery-codes.txt', ['Content-Type' => 'text/plain; charset=UTF-8'], ); - })->name('two-factor.recovery.download'); + })->name('two-factor.recovery.download') + // Session blocking (Redis-backed atomic lock) serializes concurrent same-session + // requests so the one-time `2fa.download_grant` cannot be double-read by a racing + // request. True concurrency cannot be unit-tested; lock-capable stores (redis in + // prod, array/file in tests) keep the happy path green. + ->block(5, 5); Route::post('/logout', function () { AuthFacade::logout(); diff --git a/tests/Feature/ConfirmServerScopeTest.php b/tests/Feature/ConfirmServerScopeTest.php new file mode 100644 index 0000000..e81662a --- /dev/null +++ b/tests/Feature/ConfirmServerScopeTest.php @@ -0,0 +1,149 @@ + $name, 'ip' => $ip, 'ssh_port' => 22, 'status' => 'online']); + } + + private function confirmed(string $event, array $params, ?int $serverId): string + { + $token = ConfirmToken::issue($event, $params, '', null, $serverId); + ConfirmToken::confirm($token); // the modal confirmation step + + return $token; + } + + public function test_key_removal_token_for_another_server_is_rejected(): void + { + $this->actingAs(User::factory()->create()); + $serverA = $this->server('a', '10.0.0.1'); + $serverB = $this->server('b', '10.0.0.2'); + + $fleet = Mockery::mock(FleetService::class); + $fleet->shouldReceive('removeAuthorizedKey')->never(); + app()->instance(FleetService::class, $fleet); + + $token = $this->confirmed('keyRemoved', ['fingerprint' => 'ab:cd'], $serverA->id); + + // Handler is bound to server B; the token was sealed for A. + Livewire::test(Show::class, ['server' => $serverB])->call('removeKey', $token); + + $this->assertTrue(true); // Mockery verifies removeAuthorizedKey was never called + } + + public function test_key_removal_token_for_this_server_proceeds(): void + { + $this->actingAs(User::factory()->create()); + $server = $this->server('a', '10.0.0.1'); + + $fleet = Mockery::mock(FleetService::class); + $fleet->shouldReceive('removeAuthorizedKey')->once() + ->with(Mockery::on(fn ($s) => $s->id === $server->id), 'ab:cd'); + $fleet->shouldReceive('sshKeys')->andReturn([]); // reloadKeys() after success + app()->instance(FleetService::class, $fleet); + + $token = $this->confirmed('keyRemoved', ['fingerprint' => 'ab:cd'], $server->id); + + Livewire::test(Show::class, ['server' => $server]) + ->call('removeKey', $token) + ->assertSet('sshKeys', []); // reloadKeys() ran → success path reached + } + + public function test_file_delete_token_for_another_server_is_rejected(): void + { + $this->actingAs(User::factory()->create()); + $serverA = $this->server('a', '10.0.0.1'); + $serverB = $this->server('b', '10.0.0.2'); + session(['active_server_id' => $serverB->id]); + + $fleet = Mockery::mock(FleetService::class); + $fleet->shouldReceive('files')->andReturn([]); // any dir listing during mount/load + $fleet->shouldReceive('deleteFile')->never(); + app()->instance(FleetService::class, $fleet); + + $token = $this->confirmed('fileConfirmed', ['path' => '/etc/passwd'], $serverA->id); + + Livewire::test(Files::class)->call('deleteEntry', $token); + + $this->assertTrue(true); // Mockery verifies deleteFile was never called + } + + public function test_credential_deletion_token_for_another_server_is_rejected(): void + { + $this->actingAs(User::factory()->create()); + $serverA = $this->server('a', '10.0.0.1'); + $serverB = $this->server('b', '10.0.0.2'); + $serverB->credential()->create(['username' => 'root', 'auth_type' => 'password', 'secret' => 'x']); + + $token = $this->confirmed('credentialDeleted', [], $serverA->id); + + Livewire::test(Show::class, ['server' => $serverB])->call('deleteCredential', $token); + + $this->assertTrue($serverB->credential()->exists(), "B's credential must survive a server-A token"); + } + + public function test_firewall_rule_deletion_token_for_another_server_is_rejected(): void + { + $this->actingAs(User::factory()->create()); + $serverA = $this->server('a', '10.0.0.1'); + $serverB = $this->server('b', '10.0.0.2'); + + $firewall = Mockery::mock(FirewallService::class); + $firewall->shouldReceive('deleteRule')->never(); + app()->instance(FirewallService::class, $firewall); + + $token = $this->confirmed('firewallRuleDelete', ['spec' => '22/tcp', 'label' => 'SSH'], $serverA->id); + + Livewire::test(Show::class, ['server' => $serverB])->call('deleteFirewallRule', $token); + + $this->assertTrue(true); // Mockery verifies deleteRule was never called + } + + public function test_service_action_token_for_another_server_is_rejected(): void + { + $this->actingAs(User::factory()->create()); + $serverA = $this->server('a', '10.0.0.1'); + $serverB = $this->server('b', '10.0.0.2'); + session(['active_server_id' => $serverB->id]); + + $fleet = Mockery::mock(FleetService::class); + $fleet->shouldReceive('serviceAction')->never(); + app()->instance(FleetService::class, $fleet); + + $token = $this->confirmed('serviceConfirmed', ['op' => 'stop', 'name' => 'nginx.service'], $serverA->id); + + Livewire::test(Services::class)->call('applyService', $token); + + $this->assertTrue(true); // Mockery verifies serviceAction was never called + } +} diff --git a/tests/Feature/Modals/ConfirmActionTest.php b/tests/Feature/Modals/ConfirmActionTest.php new file mode 100644 index 0000000..2b5fbc4 --- /dev/null +++ b/tests/Feature/Modals/ConfirmActionTest.php @@ -0,0 +1,106 @@ +create(); + $this->actingAs($user); + + return $user; + } + + public function test_confirm_with_a_valid_token_audits_from_the_payload_and_dispatches_the_event(): void + { + $this->login(); + $server = Server::create(['name' => 'box', 'ip' => '10.0.0.9', 'ssh_port' => 22, 'status' => 'online']); + $token = ConfirmToken::issue('keyRemoved', ['fingerprint' => 'ab:cd'], 'ssh_key.remove', 'box · key', $server->id); + + Livewire::test(ConfirmAction::class, ['token' => $token]) + ->call('confirm') + ->assertDispatched('keyRemoved', confirmToken: $token); + + $this->assertSame(1, AuditEvent::where('action', 'ssh_key.remove')->count()); + $this->assertDatabaseHas('audit_events', [ + 'action' => 'ssh_key.remove', + 'target' => 'box · key', + 'server_id' => $server->id, + ]); + } + + public function test_confirm_on_a_deferred_flow_dispatches_without_writing_audit(): void + { + $this->login(); + // No auditAction → the handler audits after the remote op; confirm() must not. + $token = ConfirmToken::issue('firewallRuleDelete', ['spec' => '22/tcp', 'label' => 'SSH']); + + Livewire::test(ConfirmAction::class, ['token' => $token]) + ->call('confirm') + ->assertDispatched('firewallRuleDelete', confirmToken: $token); + + $this->assertSame(0, AuditEvent::count()); + } + + public function test_confirm_with_a_forged_token_is_rejected_no_audit_no_dispatch(): void + { + $this->login(); + + Livewire::test(ConfirmAction::class, ['token' => 'forged-token']) + ->call('confirm') + ->assertDispatched('notify', message: __('modals.confirm_action.rejected')) + ->assertNotDispatched('keyRemoved'); + + $this->assertSame(0, AuditEvent::count()); + } + + public function test_confirm_cannot_be_replayed_after_the_handler_burned_the_token(): void + { + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled', [], 'two_factor.disable', 'a@b.c'); + + // Simulate the full lifecycle: modal confirmed it, handler burned it. + ConfirmToken::confirm($token); + ConfirmToken::consume($token, 'twoFactorDisabled'); + + Livewire::test(ConfirmAction::class, ['token' => $token]) + ->call('confirm') + ->assertDispatched('notify', message: __('modals.confirm_action.rejected')) + ->assertNotDispatched('twoFactorDisabled'); + + $this->assertSame(0, AuditEvent::count()); + } + + public function test_the_routing_token_is_locked_against_client_tampering(): void + { + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled'); + + $this->expectException(CannotUpdateLockedPropertyException::class); + + Livewire::test(ConfirmAction::class, ['token' => $token]) + ->set('token', ConfirmToken::issue('userRemoved', ['userId' => 1])); + } + + public function test_the_display_target_comes_from_the_signed_token(): void + { + $this->login(); + $token = ConfirmToken::issue('keyRemoved', [], 'ssh_key.remove', 'webserver · deploy@host', 7); + + Livewire::test(ConfirmAction::class, ['token' => $token]) + ->assertSee('webserver · deploy@host'); + } +} diff --git a/tests/Feature/MultiUserTest.php b/tests/Feature/MultiUserTest.php index 66a21a2..c011c32 100644 --- a/tests/Feature/MultiUserTest.php +++ b/tests/Feature/MultiUserTest.php @@ -6,6 +6,7 @@ use App\Livewire\Modals\CreateUser; use App\Livewire\Settings\Users; use App\Models\AuditEvent; use App\Models\User; +use App\Support\Confirm\ConfirmToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; @@ -105,8 +106,11 @@ class MultiUserTest extends TestCase ->call('confirmRemove', $admin->id) ->assertNotDispatched('openModal'); + $this->actingAs($admin); + $token = ConfirmToken::issue('userRemoved', ['userId' => $admin->id]); + ConfirmToken::confirm($token); Livewire::actingAs($admin)->test(Users::class) - ->call('remove', $admin->id); + ->call('remove', $token); $this->assertDatabaseHas('users', ['id' => $admin->id]); $this->assertSame(0, AuditEvent::where('action', 'user.delete')->count()); @@ -122,8 +126,11 @@ class MultiUserTest extends TestCase ->call('confirmRemove', $admin->id) ->assertNotDispatched('openModal'); + $this->actingAs($admin); + $token = ConfirmToken::issue('userRemoved', ['userId' => $admin->id]); + ConfirmToken::confirm($token); Livewire::actingAs($admin)->test(Users::class) - ->call('remove', $admin->id); + ->call('remove', $token); $this->assertDatabaseHas('users', ['id' => $admin->id]); } @@ -146,8 +153,11 @@ class MultiUserTest extends TestCase $victim = User::factory()->create(['email' => 'victim@example.com', 'remember_token' => 'victim-token']); $this->seedSession($victim, 'victim-sess'); + $this->actingAs($admin); + $token = ConfirmToken::issue('userRemoved', ['userId' => $victim->id]); + ConfirmToken::confirm($token); Livewire::actingAs($admin)->test(Users::class) - ->call('remove', $victim->id); + ->call('remove', $token); // User gone. $this->assertDatabaseMissing('users', ['id' => $victim->id]); @@ -169,8 +179,11 @@ class MultiUserTest extends TestCase $target = User::factory()->create(['email' => 'target@example.com', 'remember_token' => 'target-token']); $this->seedSession($target, 'target-sess'); + $this->actingAs($admin); + $token = ConfirmToken::issue('userLoggedOut', ['userId' => $target->id]); + ConfirmToken::confirm($token); Livewire::actingAs($admin)->test(Users::class) - ->call('logout', $target->id); + ->call('logout', $token); // Target still exists but is signed out everywhere + token rotated. $this->assertDatabaseHas('users', ['id' => $target->id]); @@ -193,8 +206,11 @@ class MultiUserTest extends TestCase ->call('confirmLogout', $admin->id) ->assertNotDispatched('openModal'); + $this->actingAs($admin); + $token = ConfirmToken::issue('userLoggedOut', ['userId' => $admin->id]); + ConfirmToken::confirm($token); Livewire::actingAs($admin)->test(Users::class) - ->call('logout', $admin->id); + ->call('logout', $token); $this->assertSame(0, AuditEvent::where('action', 'user.logout')->count()); } diff --git a/tests/Feature/RecoveryCodesModalTest.php b/tests/Feature/RecoveryCodesModalTest.php index 4fa037d..aafc07e 100644 --- a/tests/Feature/RecoveryCodesModalTest.php +++ b/tests/Feature/RecoveryCodesModalTest.php @@ -6,7 +6,6 @@ use App\Livewire\Modals\RecoveryCodes; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Route; -use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException; use Livewire\Livewire; use Tests\TestCase; @@ -14,67 +13,109 @@ class RecoveryCodesModalTest extends TestCase { use RefreshDatabase; - public function test_manage_view_without_fresh_flag_hides_the_codes(): void + public function test_manage_view_without_fresh_flag_dispatches_nothing_and_hides_codes(): void { $user = User::factory()->create(); $codes = $user->replaceRecoveryCodes(); // Opened from "Backup-Codes verwalten" with no fresh-generation flag: never reveal. + // No reveal event fires, and the codes never appear in the server-rendered HTML. Livewire::actingAs($user)->test(RecoveryCodes::class) - ->assertSet('revealed', false) + ->assertNotDispatched('reveal-codes') ->assertDontSee($codes[0]) ->assertDontSee($codes[7]) ->assertSee(__('auth.recovery_hidden_notice')); } - public function test_fresh_flag_reveals_the_codes_once(): void + public function test_fresh_flag_dispatches_codes_once_and_keeps_them_out_of_the_snapshot(): void { $user = User::factory()->create(); $codes = $user->replaceRecoveryCodes(); - session(['2fa.codes_fresh' => true]); + session(['2fa.codes_fresh' => now()->timestamp]); - Livewire::actingAs($user)->test(RecoveryCodes::class) - ->assertSet('revealed', true) - ->assertSee($codes[0]) - ->assertSee($codes[7]); + // The fresh flag delivers the codes via a one-time transient event — never as a + // property and never in the server-rendered HTML, so they are absent from the snapshot. + $component = Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertDispatched('reveal-codes', codes: $codes) + ->assertDontSee($codes[0]) + ->assertDontSee($codes[7]); + + // Prove the codes are absent from the serialized snapshot itself — the signed + // wire:snapshot blob a client could capture and replay — not just from visible HTML. + $this->assertSnapshotHidesCodes($codes, $component); // pull() forgets the flag — it is single-use. $this->assertFalse(session()->has('2fa.codes_fresh')); } - public function test_regenerate_from_manage_view_reveals_the_new_codes(): void - { - $user = User::factory()->create(); - $old = $user->replaceRecoveryCodes(); - - // Manage view (no flag) → not revealed → regenerate → the NEW set is revealed once. - Livewire::actingAs($user)->test(RecoveryCodes::class) - ->assertSet('revealed', false) - ->call('regenerate') - ->assertSet('revealed', true) - ->assertDontSee($old[0]); - - $new = $user->fresh()->recoveryCodes(); - $this->assertNotEquals($old, $new); - - Livewire::actingAs($user)->test(RecoveryCodes::class) - ->call('regenerate') - ->assertSee($user->fresh()->recoveryCodes()[0]); - } - - public function test_revealed_property_is_locked_against_client_tampering(): void + public function test_a_stale_codes_fresh_flag_reveals_nothing(): void { $user = User::factory()->create(); $codes = $user->replaceRecoveryCodes(); - // Manage view (no fresh flag): codes stay hidden. A crafted client update that flips - // the #[Locked] $revealed prop must be rejected — Livewire throws on a locked set. - $component = Livewire::actingAs($user)->test(RecoveryCodes::class) - ->assertSet('revealed', false) + // A reveal flag older than the 10-minute window (e.g. enrollment whose modal never + // opened) must not reveal the codes when the modal is later mounted. + session(['2fa.codes_fresh' => now()->subMinutes(11)->timestamp]); + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') ->assertDontSee($codes[0]); - $this->expectException(CannotUpdateLockedPropertyException::class); - $component->set('revealed', true); + // The stale flag is still consumed (pulled), so it cannot trigger a later reveal. + $this->assertFalse(session()->has('2fa.codes_fresh')); + } + + public function test_replayed_or_stale_snapshot_renders_no_codes(): void + { + $user = User::factory()->create(); + $codes = $user->replaceRecoveryCodes(); + session(['2fa.codes_fresh' => now()->timestamp]); + + // Legitimate one-time reveal: consumes the flag, fires the event once, renders no codes. + $component = Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertDispatched('reveal-codes', codes: $codes) + ->assertDontSee($codes[0]); + + // The captured snapshot carries no codes — there is nothing in it to replay. + $this->assertSnapshotHidesCodes($codes, $component); + + // Replaying that snapshot = a later roundtrip that hydrates the same state. mount() + // does not re-run on hydrate, so no reveal-codes event fires and no codes appear; the + // re-serialized snapshot stays clean too. + $component->call('$refresh') + ->assertNotDispatched('reveal-codes') + ->assertDontSee($codes[0]); + $this->assertSnapshotHidesCodes($codes, $component); + + // A fresh mount from the now-consumed flag (the state a replayed snapshot starts from) + // dispatches nothing and renders nothing — codes cannot be re-revealed. + Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') + ->assertDontSee($codes[0]); + } + + public function test_regenerate_dispatches_the_new_codes_without_rendering_them(): void + { + $user = User::factory()->create(); + $old = $user->replaceRecoveryCodes(); + + // Manage view (no flag) → nothing dispatched → regenerate → the NEW set is dispatched once. + $component = Livewire::actingAs($user)->test(RecoveryCodes::class) + ->assertNotDispatched('reveal-codes') + ->call('regenerate'); + + $new = $user->fresh()->recoveryCodes(); + $this->assertNotEquals($old, $new); + + // New set delivered via the transient event; neither the old nor the new set is in the HTML. + $component->assertDispatched('reveal-codes', codes: $new) + ->assertDontSee($old[0]) + ->assertDontSee($new[0]); + + // The regenerated set is absent from the serialized snapshot too. + $this->assertSnapshotHidesCodes($new, $component); + + // Regenerate also grants exactly one download of the fresh set. + $this->assertTrue(session()->has('2fa.download_grant')); } public function test_download_requires_a_fresh_grant(): void @@ -85,26 +126,73 @@ class RecoveryCodesModalTest extends TestCase // Without a fresh-generation grant, a direct hit of the download route is forbidden, // so stored codes can never be re-downloaded by an authed user at will. $this->actingAs($user) - ->get(route('two-factor.recovery.download')) + ->post(route('two-factor.recovery.download')) ->assertForbidden(); - // With the one-time grant (set when codes are freshly generated/regenerated) the - // download succeeds exactly once. - session(['2fa.download_grant' => true]); + // With a fresh one-time grant (a timestamp set when codes are freshly generated/ + // regenerated) the download succeeds exactly once. + session(['2fa.download_grant' => now()->timestamp]); $this->actingAs($user) - ->get(route('two-factor.recovery.download')) + ->post(route('two-factor.recovery.download')) ->assertOk() ->assertDownload('clusev-recovery-codes.txt'); // The grant is single-use: pull() consumed it, so an immediate re-download 403s. $this->actingAs($user) - ->get(route('two-factor.recovery.download')) + ->post(route('two-factor.recovery.download')) ->assertForbidden(); } + public function test_a_stale_download_grant_is_rejected(): void + { + $user = User::factory()->create(); + $user->replaceRecoveryCodes(); + + // A grant older than the 10-minute window is refused, so an unused grant cannot + // linger and be replayed later in the authenticated session. + session(['2fa.download_grant' => now()->subMinutes(11)->timestamp]); + $this->actingAs($user) + ->post(route('two-factor.recovery.download')) + ->assertForbidden(); + + // A non-timestamp (legacy bool) grant is likewise refused. + session(['2fa.download_grant' => true]); + $this->actingAs($user) + ->post(route('two-factor.recovery.download')) + ->assertForbidden(); + + // A future-dated grant is refused too (only [now-600s, now] is valid). + session(['2fa.download_grant' => now()->addDay()->timestamp]); + $this->actingAs($user) + ->post(route('two-factor.recovery.download')) + ->assertForbidden(); + + // A grant within the window still works. + session(['2fa.download_grant' => now()->timestamp]); + $this->actingAs($user) + ->post(route('two-factor.recovery.download')) + ->assertOk(); + } + public function test_old_recovery_route_is_gone(): void { $this->assertFalse(Route::has('two-factor.recovery')); $this->assertTrue(Route::has('two-factor.recovery.download')); } + + /** + * Assert that NONE of the given recovery codes appear anywhere in the component's + * serialized snapshot — the signed wire:snapshot blob a client could capture and replay. + * + * @param array $codes + * @param \Livewire\Features\SupportTesting\Testable $component + */ + private function assertSnapshotHidesCodes(array $codes, $component): void + { + $snapshot = json_encode($component->snapshot); + + foreach ($codes as $code) { + $this->assertStringNotContainsString($code, $snapshot); + } + } } diff --git a/tests/Feature/SessionManagementTest.php b/tests/Feature/SessionManagementTest.php index 192b6be..b1daf56 100644 --- a/tests/Feature/SessionManagementTest.php +++ b/tests/Feature/SessionManagementTest.php @@ -7,6 +7,7 @@ use App\Livewire\Settings\Sessions; use App\Models\AuditEvent; use App\Models\User; use App\Services\SessionService; +use App\Support\Confirm\ConfirmToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Session; @@ -128,9 +129,12 @@ class SessionManagementTest extends TestCase ->call('confirmLogoutOthers') ->assertDispatched('openModal'); - // The confirm modal re-dispatches the apply event; simulate that. + // The confirm modal re-dispatches the apply event with the signed token; simulate that. + $this->actingAs($user); + $token = ConfirmToken::issue('sessionsLogoutOthers'); + ConfirmToken::confirm($token); Livewire::actingAs($user)->test(Sessions::class) - ->call('logoutOthers'); + ->call('logoutOthers', $token); $this->assertDatabaseHas('sessions', ['id' => $current]); $this->assertDatabaseMissing('sessions', ['id' => 'sess-other']); @@ -145,8 +149,11 @@ class SessionManagementTest extends TestCase ->call('confirmLogoutAll') ->assertDispatched('openModal'); + $this->actingAs($user); + $token = ConfirmToken::issue('sessionsLogoutAll'); + ConfirmToken::confirm($token); Livewire::actingAs($user)->test(Sessions::class) - ->call('logoutAll') + ->call('logoutAll', $token) ->assertRedirect(route('login')); $this->assertSame(0, DB::table('sessions')->count()); @@ -155,15 +162,14 @@ class SessionManagementTest extends TestCase public function test_confirm_modal_writes_audit_row_on_confirm(): void { - // The generic confirm modal persists the audit row from the descriptor the - // Sessions component passes (mirrors Settings\Security). Exercise it directly. + // The generic confirm modal persists the audit row from the SEALED descriptor + // in the signed token (mirrors Settings\Security). Exercise it directly. $user = User::factory()->create(); + $this->actingAs($user); - Livewire::actingAs($user)->test(ConfirmAction::class, [ - 'auditAction' => 'session.logout_others', - 'auditTarget' => $user->email, - 'event' => 'sessionsLogoutOthers', - ])->call('confirm'); + $token = ConfirmToken::issue('sessionsLogoutOthers', [], 'session.logout_others', $user->email); + + Livewire::actingAs($user)->test(ConfirmAction::class, ['token' => $token])->call('confirm'); $this->assertDatabaseHas('audit_events', [ 'action' => 'session.logout_others', diff --git a/tests/Feature/Settings/SecurityConfirmTest.php b/tests/Feature/Settings/SecurityConfirmTest.php new file mode 100644 index 0000000..2590bba --- /dev/null +++ b/tests/Feature/Settings/SecurityConfirmTest.php @@ -0,0 +1,78 @@ +create([ + 'two_factor_secret' => 'S', + 'two_factor_confirmed_at' => now(), + ]); + } + + public function test_direct_handler_call_with_a_garbage_token_does_not_disable_2fa(): void + { + $user = $this->userWithTotp(); + + Livewire::actingAs($user)->test(Security::class)->call('disableTwoFactor', 'forged-token'); + + $this->assertTrue($user->fresh()->hasTotp(), 'forged token must not disable 2FA'); + } + + public function test_an_unconfirmed_token_is_rejected_by_the_handler(): void + { + // Issuing a token (the opener) is not enough — the modal's confirm() must run + // first. This closes the "skip the confirmation modal" bypass. + $user = $this->userWithTotp(); + $this->actingAs($user); + $token = ConfirmToken::issue('twoFactorDisabled', [], 'two_factor.disable', $user->email); + + Livewire::actingAs($user->fresh())->test(Security::class)->call('disableTwoFactor', $token); + + $this->assertTrue($user->fresh()->hasTotp(), 'a never-confirmed token must not apply'); + } + + public function test_a_confirmed_token_disables_2fa(): void + { + $user = $this->userWithTotp(); + $this->actingAs($user); + $token = ConfirmToken::issue('twoFactorDisabled', [], 'two_factor.disable', $user->email); + ConfirmToken::confirm($token); // the modal confirmation step + + Livewire::actingAs($user->fresh())->test(Security::class)->call('disableTwoFactor', $token); + + $this->assertFalse($user->fresh()->hasTotp(), 'a confirmed token must disable 2FA'); + } + + public function test_the_handler_burns_the_token_so_it_cannot_be_replayed(): void + { + $user = $this->userWithTotp(); + $this->actingAs($user); + $token = ConfirmToken::issue('twoFactorDisabled', [], 'two_factor.disable', $user->email); + ConfirmToken::confirm($token); + + Livewire::actingAs($user->fresh())->test(Security::class)->call('disableTwoFactor', $token); + $this->assertFalse($user->fresh()->hasTotp()); + + // Burned: the same token can no longer be confirmed or consumed. + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::confirm($token); + } +} diff --git a/tests/Feature/SettingsFactorManagementTest.php b/tests/Feature/SettingsFactorManagementTest.php index 7974fc9..2c901ef 100644 --- a/tests/Feature/SettingsFactorManagementTest.php +++ b/tests/Feature/SettingsFactorManagementTest.php @@ -7,6 +7,7 @@ use App\Livewire\Settings\WebauthnKeys; use App\Models\User; use App\Models\WebauthnCredential; use App\Services\WebauthnService; +use App\Support\Confirm\ConfirmToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Livewire\Livewire; use Mockery; @@ -27,7 +28,10 @@ class SettingsFactorManagementTest extends TestCase $user = User::factory()->create(['two_factor_secret' => 'S', 'two_factor_confirmed_at' => now()]); $user->replaceRecoveryCodes(); - Livewire::actingAs($user)->test(Security::class)->call('disableTwoFactor'); + $this->actingAs($user); + $token = ConfirmToken::issue('twoFactorDisabled'); + ConfirmToken::confirm($token); + Livewire::actingAs($user)->test(Security::class)->call('disableTwoFactor', $token); $fresh = $user->fresh(); $this->assertFalse($fresh->hasTotp()); @@ -40,7 +44,10 @@ class SettingsFactorManagementTest extends TestCase WebauthnCredential::create(['user_id' => $user->id, 'name' => 'k', 'credential_id' => 'cid', 'public_key' => '{}', 'sign_count' => 0]); $user->replaceRecoveryCodes(); - Livewire::actingAs($user->fresh())->test(Security::class)->call('disableTwoFactor'); + $this->actingAs($user); + $token = ConfirmToken::issue('twoFactorDisabled'); + ConfirmToken::confirm($token); + Livewire::actingAs($user->fresh())->test(Security::class)->call('disableTwoFactor', $token); $fresh = $user->fresh(); $this->assertFalse($fresh->hasTotp()); @@ -73,7 +80,10 @@ class SettingsFactorManagementTest extends TestCase $svc->shouldReceive('available')->andReturnTrue(); app()->instance(WebauthnService::class, $svc); - Livewire::actingAs($user->fresh())->test(WebauthnKeys::class)->call('remove', $cred->id, $svc); + $this->actingAs($user); + $token = ConfirmToken::issue('webauthnKeyRemoved', ['id' => $cred->id]); + ConfirmToken::confirm($token); + Livewire::actingAs($user->fresh())->test(WebauthnKeys::class)->call('remove', $token, $svc); $this->assertFalse($user->fresh()->hasRecoveryCodes()); } diff --git a/tests/Feature/TlsModeToggleTest.php b/tests/Feature/TlsModeToggleTest.php index aa88867..07feed6 100644 --- a/tests/Feature/TlsModeToggleTest.php +++ b/tests/Feature/TlsModeToggleTest.php @@ -5,6 +5,7 @@ namespace Tests\Feature; use App\Livewire\System\Index; use App\Models\User; use App\Services\DeploymentService; +use App\Support\Confirm\ConfirmToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Livewire\Livewire; use Tests\TestCase; @@ -17,8 +18,10 @@ class TlsModeToggleTest extends TestCase { $this->actingAs(User::factory()->create(['must_change_password' => false])); + $token = ConfirmToken::issue('tlsModeChanged', ['mode' => 'external']); + ConfirmToken::confirm($token); Livewire::test(Index::class) - ->call('applyTlsMode', 'external', app(DeploymentService::class)) + ->call('applyTlsMode', $token, app(DeploymentService::class)) ->assertSet('tlsMode', 'external') ->assertSet('restartPending', true); @@ -29,8 +32,10 @@ class TlsModeToggleTest extends TestCase { $this->actingAs(User::factory()->create(['must_change_password' => false])); + $token = ConfirmToken::issue('tlsModeChanged', ['mode' => 'garbage']); + ConfirmToken::confirm($token); Livewire::test(Index::class) - ->call('applyTlsMode', 'garbage', app(DeploymentService::class)) + ->call('applyTlsMode', $token, app(DeploymentService::class)) ->assertSet('tlsMode', 'caddy'); $this->assertFalse(app(DeploymentService::class)->externalTls()); @@ -40,8 +45,10 @@ class TlsModeToggleTest extends TestCase { $this->actingAs(User::factory()->create(['must_change_password' => false])); + $token = ConfirmToken::issue('tlsModeChanged', ['mode' => 'external']); + ConfirmToken::confirm($token); Livewire::test(Index::class) - ->call('applyTlsMode', 'external', app(DeploymentService::class)); + ->call('applyTlsMode', $token, app(DeploymentService::class)); $this->assertDatabaseHas('audit_events', [ 'action' => 'system.settings_updated', diff --git a/tests/Feature/WebauthnKeysTest.php b/tests/Feature/WebauthnKeysTest.php index db2579c..eb4572b 100644 --- a/tests/Feature/WebauthnKeysTest.php +++ b/tests/Feature/WebauthnKeysTest.php @@ -6,6 +6,7 @@ use App\Livewire\Settings\WebauthnKeys; use App\Models\User; use App\Models\WebauthnCredential; use App\Services\WebauthnService; +use App\Support\Confirm\ConfirmToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Livewire\Livewire; use Tests\TestCase; @@ -51,7 +52,9 @@ class WebauthnKeysTest extends TestCase $cred = WebauthnCredential::create(['user_id' => $user->id, 'name' => 'k', 'credential_id' => 'cid', 'public_key' => '{}', 'sign_count' => 0]); $this->mock(WebauthnService::class)->shouldReceive('available')->andReturn(true); - Livewire::test(WebauthnKeys::class)->call('remove', $cred->id); + $token = ConfirmToken::issue('webauthnKeyRemoved', ['id' => $cred->id]); + ConfirmToken::confirm($token); + Livewire::test(WebauthnKeys::class)->call('remove', $token); $this->assertDatabaseMissing('webauthn_credentials', ['id' => $cred->id]); } diff --git a/tests/Unit/Support/Confirm/ConfirmTokenTest.php b/tests/Unit/Support/Confirm/ConfirmTokenTest.php new file mode 100644 index 0000000..e2f935e --- /dev/null +++ b/tests/Unit/Support/Confirm/ConfirmTokenTest.php @@ -0,0 +1,133 @@ +create(); + $this->actingAs($user); + + return $user; + } + + public function test_issue_confirm_consume_is_the_happy_path(): void + { + $this->login(); + $token = ConfirmToken::issue('keyRemoved', ['fingerprint' => 'ab:cd'], 'ssh_key.remove', 'box · key', 7); + + // The modal confirms (pending → confirmed)… + $this->assertSame('keyRemoved', ConfirmToken::confirm($token)['event']); + + // …then the handler consumes (confirmed → burned). + $payload = ConfirmToken::consume($token, 'keyRemoved'); + $this->assertSame(['fingerprint' => 'ab:cd'], $payload['params']); + $this->assertSame('ssh_key.remove', $payload['auditAction']); + $this->assertSame('box · key', $payload['auditTarget']); + $this->assertSame(7, $payload['serverId']); + } + + public function test_a_freshly_issued_token_cannot_be_consumed_without_confirmation(): void + { + // The modal-bypass guard: issuing a token (opener) does NOT authorize apply. + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled'); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::consume($token, 'twoFactorDisabled'); + } + + public function test_issue_rejects_an_event_outside_the_allowlist(): void + { + $this->login(); + + $this->expectException(\InvalidArgumentException::class); + ConfirmToken::issue('evilEvent', ['userId' => 1]); + } + + public function test_a_token_can_be_confirmed_only_once(): void + { + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled'); + + ConfirmToken::confirm($token); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::confirm($token); + } + + public function test_a_confirmed_token_is_consumed_only_once(): void + { + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled'); + ConfirmToken::confirm($token); + + ConfirmToken::consume($token, 'twoFactorDisabled'); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::consume($token, 'twoFactorDisabled'); + } + + public function test_consume_rejects_a_token_minted_for_a_different_action(): void + { + $this->login(); + $token = ConfirmToken::issue('fileConfirmed', ['name' => 'x']); + ConfirmToken::confirm($token); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::consume($token, 'keyRemoved'); + } + + public function test_confirm_rejects_a_garbage_token(): void + { + $this->login(); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::confirm('not-a-real-token'); + } + + public function test_consume_rejects_a_garbage_token(): void + { + $this->login(); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::consume('not-a-real-token', 'keyRemoved'); + } + + public function test_peek_returns_null_on_a_garbage_token_and_never_throws(): void + { + $this->login(); + + $this->assertNull(ConfirmToken::peek('not-a-real-token')); + } + + public function test_peek_exposes_the_audit_target_for_display(): void + { + $this->login(); + $token = ConfirmToken::issue('keyRemoved', [], 'ssh_key.remove', 'box · key', 7); + + $this->assertSame('box · key', ConfirmToken::peek($token)['auditTarget']); + } + + public function test_a_token_is_bound_to_the_issuing_user(): void + { + $this->login(); + $token = ConfirmToken::issue('twoFactorDisabled'); + + // A different operator must not be able to confirm Alice's token. + Auth::login(User::factory()->create()); + + $this->expectException(InvalidConfirmToken::class); + ConfirmToken::confirm($token); + } +}