diff --git a/app/Livewire/Auth/TwoFactorSetup.php b/app/Livewire/Auth/TwoFactorSetup.php index c9eae30..a698dff 100644 --- a/app/Livewire/Auth/TwoFactorSetup.php +++ b/app/Livewire/Auth/TwoFactorSetup.php @@ -53,7 +53,7 @@ class TwoFactorSetup extends Component session()->flash('open_recovery_modal', true); // 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); + session()->put('2fa.codes_fresh', now()->timestamp); // 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. diff --git a/app/Livewire/Modals/RecoveryCodes.php b/app/Livewire/Modals/RecoveryCodes.php index 1462d1b..551195b 100644 --- a/app/Livewire/Modals/RecoveryCodes.php +++ b/app/Livewire/Modals/RecoveryCodes.php @@ -26,10 +26,14 @@ 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. mount() does not - // run on hydrate, so a replayed snapshot never re-triggers this. - if (session()->pull('2fa.codes_fresh', false)) { + // One-time, server-set flag (a timestamp put on first-factor enrollment). pull() reads + // + forgets, 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. The flag is + // valid only within [now-600s, now]: a stale flag (enrollment whose modal never opened) + // or a non-timestamp value reveals nothing. + $freshAt = session()->pull('2fa.codes_fresh'); + $age = is_int($freshAt) ? now()->timestamp - $freshAt : null; + if ($age !== null && $age >= 0 && $age <= 600) { $this->dispatchCodes(); } } diff --git a/app/Livewire/Settings/WebauthnKeys.php b/app/Livewire/Settings/WebauthnKeys.php index a97b0b3..1b77182 100644 --- a/app/Livewire/Settings/WebauthnKeys.php +++ b/app/Livewire/Settings/WebauthnKeys.php @@ -41,7 +41,7 @@ class WebauthnKeys extends Component if (! Auth::user()->hasRecoveryCodes()) { Auth::user()->replaceRecoveryCodes(); // Server-set one-time flag so the opening modal reveals the fresh set once. - session()->put('2fa.codes_fresh', true); + session()->put('2fa.codes_fresh', now()->timestamp); // 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. diff --git a/docs/superpowers/plans/2026-06-15-recovery-codes-airtight-reveal.md b/docs/superpowers/plans/2026-06-15-recovery-codes-airtight-reveal.md index 2199f64..6dd4b68 100644 --- a/docs/superpowers/plans/2026-06-15-recovery-codes-airtight-reveal.md +++ b/docs/superpowers/plans/2026-06-15-recovery-codes-airtight-reveal.md @@ -454,4 +454,18 @@ Codex raised three findings; resolutions: 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. +- **F5 (`2fa.codes_fresh` reveal flag had no expiry — symmetric to F2):** The flag is now a + timestamp set by `Auth/TwoFactorSetup.php` and `Settings/WebauthnKeys.php`, and + `RecoveryCodes::mount()` reveals only within a 10-minute window. New test + `test_a_stale_codes_fresh_flag_reveals_nothing`. `tests/Feature/FirstFactorCodesTest.php` + updated to assert the flag is an int (`assertIsInt`) rather than `true`. +- **F6 (timestamp comparison accepted a future value):** Both checks (download route and + `mount()`) now validate `$age >= 0 && $age <= 600`, rejecting future-dated timestamps. New + case added to `test_a_stale_download_grant_is_rejected`. + +Touched beyond the original plan: `app/Livewire/Auth/TwoFactorSetup.php`, +`app/Livewire/Settings/WebauthnKeys.php` (both flag types → timestamps), and +`tests/Feature/FirstFactorCodesTest.php` (flag type assertion). + +Final suite: `RecoveryCodesModalTest` 8 tests / 65 assertions, all green; recovery/2FA +regression set green. 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 index aeac014..1c4e7b3 100644 --- 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 @@ -134,8 +134,13 @@ a download link, but a cross-site request can no longer burn the victim's grant. - 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. + `session()->put('2fa.download_grant', now()->timestamp)` instead of `true`. +- Reveal-flag setters (updated for symmetry — F5): `Auth/TwoFactorSetup.php` and + `Settings/WebauthnKeys.php` store `2fa.codes_fresh` as `now()->timestamp` too, and + `RecoveryCodes::mount()` only reveals when that flag is within `[now-600s, now]` — a stale + flag (enrollment whose modal never opened) reveals nothing. +- Both timestamp checks (route grant + `mount()` flag) validate the window as + `$age !== null && $age >= 0 && $age <= 600`, which also rejects a future-dated value (F6). - `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 diff --git a/tests/Feature/FirstFactorCodesTest.php b/tests/Feature/FirstFactorCodesTest.php index b7e2f6e..5041b7f 100644 --- a/tests/Feature/FirstFactorCodesTest.php +++ b/tests/Feature/FirstFactorCodesTest.php @@ -37,8 +37,9 @@ class FirstFactorCodesTest extends TestCase $this->assertTrue($user->fresh()->hasRecoveryCodes()); $this->assertTrue(session('open_recovery_modal')); - // The modal that opens after the redirect reveals the fresh set once. - $this->assertTrue(session('2fa.codes_fresh')); + // The modal that opens after the redirect reveals the fresh set once. The flag is a + // timestamp (TTL-bounded) — the modal pulls it within its 10-minute window. + $this->assertIsInt(session('2fa.codes_fresh')); } public function test_first_key_register_generates_codes_and_opens_modal(): void @@ -58,8 +59,8 @@ class FirstFactorCodesTest extends TestCase ->assertDispatched('openModal'); $this->assertTrue($user->fresh()->hasRecoveryCodes()); - // The opening modal reveals the fresh set once. - $this->assertTrue(session('2fa.codes_fresh')); + // The opening modal reveals the fresh set once. The flag is a TTL-bounded timestamp. + $this->assertIsInt(session('2fa.codes_fresh')); } public function test_malformed_setup_secret_yields_validation_error_not_500(): void