fix(2fa): close two re-view holes in backup-codes show-once

Stored recovery codes were re-viewable two ways after their one-time
reveal. Both paths are now closed so codes are only viewable/downloadable
right after a fresh generation (enrollment or regenerate).

HOLE 1 — RecoveryCodes::$revealed was a mutable public Livewire prop, so
a crafted /livewire/update could flip it to true and make render() emit
stored codes. Annotated with #[Locked]; Livewire now rejects any
client-side write (set still happens server-side in mount()/regenerate()).

HOLE 2 — two-factor.recovery.download was auth-gated only, so any authed
user could GET it anytime to re-download stored codes. Added a one-time
session grant (2fa.download_grant) put alongside the existing
2fa.codes_fresh wherever codes are freshly generated (TwoFactorSetup,
WebauthnKeys, RecoveryCodes::regenerate); the route now
abort_unless(pull(grant), 403) before streaming — one download per fresh
generation, later direct hits 403.

Tests: lock rejection + download-grant lifecycle (forbidden → granted ok
→ consumed forbidden). Full suite 137 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat/v1-foundation
boban 2026-06-15 06:21:45 +02:00
parent dd891cd11e
commit 9a6c09e488
5 changed files with 60 additions and 0 deletions

View File

@ -54,6 +54,9 @@ 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);
}
return $this->redirect(route('settings'), navigate: true);

View File

@ -4,6 +4,7 @@ namespace App\Livewire\Modals;
use App\Models\AuditEvent;
use Illuminate\Support\Facades\Auth;
use Livewire\Attributes\Locked;
use LivewireUI\Modal\ModalComponent;
/**
@ -17,7 +18,10 @@ class RecoveryCodes extends ModalComponent
* "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
@ -50,6 +54,10 @@ class RecoveryCodes extends ModalComponent
// The freshly generated set may be shown once in this same modal instance.
$this->revealed = true;
// 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);
$this->dispatch('notify', message: __('auth.recovery_regenerated'));
}

View File

@ -40,6 +40,9 @@ 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);
$this->dispatch('openModal', component: 'modals.recovery-codes');
}

View File

@ -55,6 +55,11 @@ Route::middleware('auth')->group(function () {
// 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);
$codes = AuthFacade::user()->recoveryCodes();
return response()->streamDownload(

View File

@ -6,6 +6,7 @@ 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;
@ -61,6 +62,46 @@ class RecoveryCodesModalTest extends TestCase
->assertSee($user->fresh()->recoveryCodes()[0]);
}
public function test_revealed_property_is_locked_against_client_tampering(): 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)
->assertDontSee($codes[0]);
$this->expectException(CannotUpdateLockedPropertyException::class);
$component->set('revealed', true);
}
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'));