fix(2fa): bound the backup-code reveal flag with a 10-minute TTL
The 2fa.codes_fresh reveal flag is now a timestamp (set at enrollment) and RecoveryCodes::mount() only reveals within [now-600s, now], symmetric to the download-grant window. A reveal flag whose modal never opened can no longer re-reveal the codes later in the session. FirstFactorCodesTest asserts the flag is an int. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>feat/v1-foundation
parent
5ab665321f
commit
6290cec6ab
|
|
@ -53,7 +53,7 @@ class TwoFactorSetup extends Component
|
||||||
session()->flash('open_recovery_modal', true);
|
session()->flash('open_recovery_modal', true);
|
||||||
// put (not flash): must survive the redirect AND the later openModal Livewire
|
// 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.
|
// 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
|
// 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
|
// it within a short window, so the fresh set can be downloaded once and a later
|
||||||
// direct hit 403s.
|
// direct hit 403s.
|
||||||
|
|
|
||||||
|
|
@ -26,10 +26,14 @@ class RecoveryCodes extends ModalComponent
|
||||||
|
|
||||||
public function mount(): void
|
public function mount(): void
|
||||||
{
|
{
|
||||||
// One-time, server-set flag (put on first-factor enrollment). pull() reads + forgets,
|
// One-time, server-set flag (a timestamp put on first-factor enrollment). pull() reads
|
||||||
// so a refresh of the manage view will no longer reveal the codes. mount() does not
|
// + forgets, so a refresh of the manage view will no longer reveal the codes. mount()
|
||||||
// run on hydrate, so a replayed snapshot never re-triggers this.
|
// does not run on hydrate, so a replayed snapshot never re-triggers this. The flag is
|
||||||
if (session()->pull('2fa.codes_fresh', false)) {
|
// 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();
|
$this->dispatchCodes();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -41,7 +41,7 @@ class WebauthnKeys extends Component
|
||||||
if (! Auth::user()->hasRecoveryCodes()) {
|
if (! Auth::user()->hasRecoveryCodes()) {
|
||||||
Auth::user()->replaceRecoveryCodes();
|
Auth::user()->replaceRecoveryCodes();
|
||||||
// Server-set one-time flag so the opening modal reveals the fresh set once.
|
// 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
|
// 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
|
// it within a short window, so the fresh set can be downloaded once and a later
|
||||||
// direct hit 403s.
|
// direct hit 403s.
|
||||||
|
|
|
||||||
|
|
@ -454,4 +454,18 @@ Codex raised three findings; resolutions:
|
||||||
No code disclosure was possible (cross-origin response is unreadable); the fix prevents the
|
No code disclosure was possible (cross-origin response is unreadable); the fix prevents the
|
||||||
grant-consumption nuisance.
|
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.
|
||||||
|
|
|
||||||
|
|
@ -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`,
|
- Grant setters (updated for the TTL — F2): `Auth/TwoFactorSetup.php`,
|
||||||
`Settings/WebauthnKeys.php`, and `Modals/RecoveryCodes::regenerate()` now store
|
`Settings/WebauthnKeys.php`, and `Modals/RecoveryCodes::regenerate()` now store
|
||||||
`session()->put('2fa.download_grant', now()->timestamp)` instead of `true`. They still set
|
`session()->put('2fa.download_grant', now()->timestamp)` instead of `true`.
|
||||||
`2fa.codes_fresh` unchanged.
|
- 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
|
- `Settings/Index.php` is unchanged; the modal now consumes `codes_fresh` by dispatching the
|
||||||
codes instead of flipping a `revealed` bool.
|
codes instead of flipping a `revealed` bool.
|
||||||
- No i18n changes: no new visible strings (the heading, subtitle, warning, hidden-notice, and
|
- No i18n changes: no new visible strings (the heading, subtitle, warning, hidden-notice, and
|
||||||
|
|
|
||||||
|
|
@ -37,8 +37,9 @@ class FirstFactorCodesTest extends TestCase
|
||||||
|
|
||||||
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
||||||
$this->assertTrue(session('open_recovery_modal'));
|
$this->assertTrue(session('open_recovery_modal'));
|
||||||
// The modal that opens after the redirect reveals the fresh set once.
|
// The modal that opens after the redirect reveals the fresh set once. The flag is a
|
||||||
$this->assertTrue(session('2fa.codes_fresh'));
|
// 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
|
public function test_first_key_register_generates_codes_and_opens_modal(): void
|
||||||
|
|
@ -58,8 +59,8 @@ class FirstFactorCodesTest extends TestCase
|
||||||
->assertDispatched('openModal');
|
->assertDispatched('openModal');
|
||||||
|
|
||||||
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
||||||
// The opening modal reveals the fresh set once.
|
// The opening modal reveals the fresh set once. The flag is a TTL-bounded timestamp.
|
||||||
$this->assertTrue(session('2fa.codes_fresh'));
|
$this->assertIsInt(session('2fa.codes_fresh'));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_malformed_setup_secret_yields_validation_error_not_500(): void
|
public function test_malformed_setup_secret_yields_validation_error_not_500(): void
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue