fix(2fa): narrow TOTP verify exception handling; guard enrollment against a tampered secret
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>feat/v1-foundation
parent
5b1e7e5168
commit
4f4b7225f5
|
|
@ -7,6 +7,7 @@ use Illuminate\Validation\ValidationException;
|
||||||
use Livewire\Attributes\Layout;
|
use Livewire\Attributes\Layout;
|
||||||
use Livewire\Attributes\Validate;
|
use Livewire\Attributes\Validate;
|
||||||
use Livewire\Component;
|
use Livewire\Component;
|
||||||
|
use PragmaRX\Google2FA\Exceptions\Google2FAException;
|
||||||
use PragmaRX\Google2FAQRCode\Google2FA;
|
use PragmaRX\Google2FAQRCode\Google2FA;
|
||||||
|
|
||||||
#[Layout('layouts.auth')]
|
#[Layout('layouts.auth')]
|
||||||
|
|
@ -30,7 +31,13 @@ class TwoFactorSetup extends Component
|
||||||
{
|
{
|
||||||
$this->validate();
|
$this->validate();
|
||||||
|
|
||||||
if (! (new Google2FA)->verifyKey($this->secret, preg_replace('/\s+/', '', $this->code))) {
|
try {
|
||||||
|
$matches = (new Google2FA)->verifyKey($this->secret, preg_replace('/\s+/', '', $this->code) ?? '');
|
||||||
|
} catch (Google2FAException $e) {
|
||||||
|
$matches = false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (! $matches) {
|
||||||
throw ValidationException::withMessages(['code' => __('auth.code_mismatch')]);
|
throw ValidationException::withMessages(['code' => __('auth.code_mismatch')]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ use Illuminate\Foundation\Auth\User as Authenticatable;
|
||||||
use Illuminate\Notifications\Notifiable;
|
use Illuminate\Notifications\Notifiable;
|
||||||
use Illuminate\Support\Facades\DB;
|
use Illuminate\Support\Facades\DB;
|
||||||
use Illuminate\Support\Str;
|
use Illuminate\Support\Str;
|
||||||
|
use PragmaRX\Google2FA\Exceptions\Google2FAException;
|
||||||
use PragmaRX\Google2FAQRCode\Google2FA;
|
use PragmaRX\Google2FAQRCode\Google2FA;
|
||||||
|
|
||||||
#[Fillable(['name', 'email', 'password', 'must_change_password', 'locale'])]
|
#[Fillable(['name', 'email', 'password', 'must_change_password', 'locale'])]
|
||||||
|
|
@ -102,7 +103,7 @@ class User extends Authenticatable
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return (new Google2FA)->verifyKey($this->two_factor_secret, preg_replace('/\s+/', '', $code) ?? '');
|
return (new Google2FA)->verifyKey($this->two_factor_secret, preg_replace('/\s+/', '', $code) ?? '');
|
||||||
} catch (\Throwable) {
|
} catch (Google2FAException) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -57,4 +57,17 @@ class FirstFactorCodesTest extends TestCase
|
||||||
|
|
||||||
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
$this->assertTrue($user->fresh()->hasRecoveryCodes());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_malformed_setup_secret_yields_validation_error_not_500(): void
|
||||||
|
{
|
||||||
|
$user = User::factory()->create();
|
||||||
|
|
||||||
|
Livewire::actingAs($user)->test(TwoFactorSetup::class)
|
||||||
|
->set('secret', 'x') // client-tampered, too short for Google2FA
|
||||||
|
->set('code', '123456')
|
||||||
|
->call('confirm')
|
||||||
|
->assertHasErrors('code');
|
||||||
|
|
||||||
|
$this->assertFalse($user->fresh()->hasTotp());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue