From 5b1e7e51688fe0d33107a70c31bb39c811b7162d Mon Sep 17 00:00:00 2001 From: boban Date: Sun, 14 Jun 2026 21:40:15 +0200 Subject: [PATCH] fix(2fa): TwoFactorSetup back to auth layout; exception-safe User::verifyTotp at all call sites Co-Authored-By: Claude Opus 4.8 (1M context) --- app/Livewire/Auth/ForgotPassword.php | 5 +--- app/Livewire/Auth/TwoFactorChallenge.php | 4 +-- app/Livewire/Auth/TwoFactorSetup.php | 2 +- app/Models/User.php | 19 +++++++++++++ tests/Feature/VerifyTotpTest.php | 36 ++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 tests/Feature/VerifyTotpTest.php diff --git a/app/Livewire/Auth/ForgotPassword.php b/app/Livewire/Auth/ForgotPassword.php index 7b6422a..559491a 100644 --- a/app/Livewire/Auth/ForgotPassword.php +++ b/app/Livewire/Auth/ForgotPassword.php @@ -11,7 +11,6 @@ use Illuminate\Validation\Rules\Password; use Illuminate\Validation\ValidationException; use Livewire\Attributes\Layout; use Livewire\Component; -use PragmaRX\Google2FAQRCode\Google2FA; #[Layout('layouts.auth')] class ForgotPassword extends Component @@ -61,12 +60,10 @@ class ForgotPassword extends Component } $user = User::where('email', $this->email)->first(); - $clean = preg_replace('/\s+/', '', $this->code); $ok = $user && $user->hasTwoFactorEnabled() - && (($user->hasTotp() && (new Google2FA)->verifyKey($user->two_factor_secret, $clean)) - || $user->useRecoveryCode($this->code)); + && ($user->verifyTotp($this->code) || $user->useRecoveryCode($this->code)); if (! $ok) { RateLimiter::hit($key, 60); diff --git a/app/Livewire/Auth/TwoFactorChallenge.php b/app/Livewire/Auth/TwoFactorChallenge.php index e30e777..797cdd9 100644 --- a/app/Livewire/Auth/TwoFactorChallenge.php +++ b/app/Livewire/Auth/TwoFactorChallenge.php @@ -10,7 +10,6 @@ use Illuminate\Validation\ValidationException; use Livewire\Attributes\Layout; use Livewire\Attributes\Validate; use Livewire\Component; -use PragmaRX\Google2FAQRCode\Google2FA; #[Layout('layouts.auth')] class TwoFactorChallenge extends Component @@ -67,8 +66,7 @@ class TwoFactorChallenge extends Component // Accept the TOTP code only when the pending user actually has TOTP (otherwise a null // secret would break verifyKey); always allow a one-time backup (recovery) code. - $valid = ($user->hasTotp() && (new Google2FA)->verifyKey($user->two_factor_secret, preg_replace('/\s+/', '', $this->code))) - || $user->useRecoveryCode($this->code); + $valid = $user->verifyTotp($this->code) || $user->useRecoveryCode($this->code); if (! $valid) { RateLimiter::hit($key, 60); diff --git a/app/Livewire/Auth/TwoFactorSetup.php b/app/Livewire/Auth/TwoFactorSetup.php index d030f5f..71b3978 100644 --- a/app/Livewire/Auth/TwoFactorSetup.php +++ b/app/Livewire/Auth/TwoFactorSetup.php @@ -9,7 +9,7 @@ use Livewire\Attributes\Validate; use Livewire\Component; use PragmaRX\Google2FAQRCode\Google2FA; -#[Layout('layouts.app')] +#[Layout('layouts.auth')] class TwoFactorSetup extends Component { public string $secret = ''; diff --git a/app/Models/User.php b/app/Models/User.php index 74b4f44..e99e87b 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -11,6 +11,7 @@ use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; +use PragmaRX\Google2FAQRCode\Google2FA; #[Fillable(['name', 'email', 'password', 'must_change_password', 'locale'])] #[Hidden(['password', 'remember_token', 'two_factor_secret', 'two_factor_recovery_codes'])] @@ -88,6 +89,24 @@ class User extends Authenticatable return ! is_null($this->two_factor_confirmed_at) && ! is_null($this->two_factor_secret); } + /** + * Verify a TOTP code against the stored secret. Returns false (never throws) when the + * user has no TOTP or the stored secret is malformed — callers then fall back to a + * backup code. + */ + public function verifyTotp(string $code): bool + { + if (! $this->hasTotp()) { + return false; + } + + try { + return (new Google2FA)->verifyKey($this->two_factor_secret, preg_replace('/\s+/', '', $code) ?? ''); + } catch (\Throwable) { + return false; + } + } + /** 2FA is satisfied by EITHER factor — TOTP or a registered security key. */ public function hasTwoFactorEnabled(): bool { diff --git a/tests/Feature/VerifyTotpTest.php b/tests/Feature/VerifyTotpTest.php new file mode 100644 index 0000000..8d4ce7e --- /dev/null +++ b/tests/Feature/VerifyTotpTest.php @@ -0,0 +1,36 @@ +assertFalse(User::factory()->create()->verifyTotp('123456')); + } + + public function test_malformed_secret_returns_false_without_throwing(): void + { + // A non-null but invalid secret must not bubble a Google2FA exception. + $user = User::factory()->create(['two_factor_secret' => 'x', 'two_factor_confirmed_at' => now()]); + + $this->assertFalse($user->verifyTotp('123456')); + } + + public function test_valid_secret_accepts_the_current_code(): void + { + $g = new Google2FA; + $secret = $g->generateSecretKey(); + $user = User::factory()->create(['two_factor_secret' => $secret, 'two_factor_confirmed_at' => now()]); + + $this->assertTrue($user->verifyTotp($g->getCurrentOtp($secret))); + $this->assertFalse($user->verifyTotp('000000')); + } +}