don't require recovery codes when an otp code is provided

packages/panels/src/Auth/MultiFactor/App/Actions/DisableAppAuthenticationAction.php

@@ -43,7 +43,7 @@ class DisableAppAuthenticationAction
                         ->action(fn (Set $set) => $set('useRecoveryCode', true))
                         ->visible(fn (): bool => $isRecoverable && (! $get('useRecoveryCode'))))
                     ->validationAttribute(__('filament-panels::auth/multi-factor/app/actions/disable.modal.form.code.validation_attribute'))
-                    ->required(fn (Get $get): bool => (! $isRecoverable) || (! $get('useRecoveryCode')))
+                    ->required(fn (Get $get): bool => (! $isRecoverable) || (! $get('useRecoveryCode')) || blank($get('recoveryCode')))
                     ->rule(function () use ($appAuthentication): Closure {
                         return function (string $attribute, #[SensitiveParameter] mixed $value, Closure $fail) use ($appAuthentication): void {
                             $rateLimitingKey = 'filament-disable-app-authentication:' . Filament::auth()->id();
@@ -91,7 +91,6 @@ class DisableAppAuthenticationAction
                             $fail(__('filament-panels::auth/multi-factor/app/actions/disable.modal.form.recovery_code.messages.invalid'));
                         };
                     })
-                    ->required()
                     ->visible(fn (Get $get): bool => $isRecoverable && $get('useRecoveryCode'))
                     ->live(onBlur: true),
             ])

packages/panels/src/Auth/MultiFactor/App/AppAuthentication.php

@@ -348,7 +348,7 @@ class AppAuthentication implements MultiFactorAuthenticationProvider
                     ->action(fn (Set $set) => $set('useRecoveryCode', true))
                     ->visible(fn (): bool => $isRecoverable && (! $get('useRecoveryCode'))))
                 ->validationAttribute(__('filament-panels::auth/multi-factor/app/provider.login_form.code.validation_attribute'))
-                ->required(fn (Get $get): bool => (! $isRecoverable) || (! $get('useRecoveryCode')))
+                ->required(fn (Get $get): bool => (! $isRecoverable) || (! $get('useRecoveryCode')) || blank($get('recoveryCode')))
                 ->rule(function () use ($user): Closure {
                     return function (string $attribute, #[SensitiveParameter] $value, Closure $fail) use ($user): void {
                         if ($this->verifyCode($value, $this->getSecret($user), shouldPreventCodeReuse: true)) {
@@ -376,7 +376,6 @@ class AppAuthentication implements MultiFactorAuthenticationProvider
                         $fail(__('filament-panels::auth/multi-factor/app/provider.login_form.recovery_code.messages.invalid'));
                     };
                 })
-                ->required()
                 ->visible(fn (Get $get): bool => $isRecoverable && $get('useRecoveryCode'))
                 ->live(onBlur: true),
         ];

tests/src/Panels/Auth/MultiFactor/App/Actions/DisableAppAuthenticationActionTest.php

@@ -94,6 +94,28 @@ describe('disabling authentication', function (): void {
             ->toBeNull();
     });
 
+    it('can disable authentication with a one-time code after enabling the recovery code field', function (): void {
+        $appAuthentication = Arr::first(Filament::getCurrentOrDefaultPanel()->getMultiFactorAuthenticationProviders());
+
+        $user = auth()->user();
+
+        // Having enabled the recovery code field, the user can still change their mind and confirm with
+        // their one-time code, leaving the recovery code blank.
+        livewire(EditProfile::class)
+            ->mountAction(TestAction::make('disableAppAuthentication')
+                ->schemaComponent('app', schema: 'content'))
+            ->callAction(TestAction::make('useRecoveryCode')
+                ->schemaComponent('code'))
+            ->fillForm([
+                'code' => $appAuthentication->getCurrentCode($user),
+            ])
+            ->callMountedAction()
+            ->assertHasNoFormErrors();
+
+        expect(filled($user->getAppAuthenticationSecret()))
+            ->toBeFalse();
+    });
+
     it('will not disable authentication when an invalid code is used', function (): void {
         $appAuthentication = Arr::first(Filament::getCurrentOrDefaultPanel()->getMultiFactorAuthenticationProviders());
 
@@ -130,11 +152,11 @@ describe('disabling authentication', function (): void {
 });
 
 describe('validation', function (): void {
-    test('recovery codes are required when the recovery code field is enabled', function (): void {
+    test('a one-time code is still required when the recovery code field is enabled but left blank', function (): void {
         $user = auth()->user();
 
-        // Once the recovery code field is enabled it becomes the active factor, so a blank recovery
-        // code must fail on the recovery code field rather than falling back to the one-time code.
+        // Enabling the recovery code field does not force the user down the recovery path: with the
+        // recovery code left blank the one-time code is still required, so it can be used instead.
         livewire(EditProfile::class)
             ->mountAction(TestAction::make('disableAppAuthentication')
                 ->schemaComponent('app', schema: 'content'))
@@ -142,7 +164,7 @@ describe('validation', function (): void {
                 ->schemaComponent('code'))
             ->callMountedAction()
             ->assertHasFormErrors([
-                'recoveryCode' => 'required',
+                'code' => 'required',
             ]);
 
         expect(filled($user->getAppAuthenticationSecret()))

tests/src/Panels/Auth/MultiFactor/App/AppAuthenticationChallengeTest.php

@@ -124,6 +124,37 @@ describe('authentication flow', function (): void {
 
         $this->assertAuthenticatedAs($userToAuthenticate);
     });
+
+    it('will authenticate the user with a one-time code after enabling the recovery code field', function (): void {
+        $appAuthentication = Arr::first(Filament::getCurrentOrDefaultPanel()->getMultiFactorAuthenticationProviders());
+
+        $userToAuthenticate = User::factory()
+            ->hasAppAuthentication($appAuthentication->generateRecoveryCodes())
+            ->create();
+
+        livewire(Login::class)
+            ->fillForm([
+                'email' => $userToAuthenticate->email,
+                'password' => 'password',
+            ])
+            ->call('authenticate')
+            ->assertNotSet('userUndertakingMultiFactorAuthentication', null)
+            ->assertNoRedirect()
+            ->callAction(TestAction::make('useRecoveryCode')
+                ->schemaComponent("{$appAuthentication->getId()}.code", schema: 'multiFactorChallengeForm'))
+            // Having enabled the recovery code field, the user can still change their mind and authenticate
+            // with their one-time code, leaving the recovery code blank.
+            ->fillForm([
+                $appAuthentication->getId() => [
+                    'code' => $appAuthentication->getCurrentCode($userToAuthenticate),
+                ],
+            ], 'multiFactorChallengeForm')
+            ->call('authenticate')
+            ->assertHasNoErrors()
+            ->assertRedirect(Filament::getUrl());
+
+        $this->assertAuthenticatedAs($userToAuthenticate);
+    });
 });
 
 describe('failure cases', function (): void {
@@ -194,7 +225,7 @@ describe('failure cases', function (): void {
 });
 
 describe('validation', function (): void {
-    test('recovery codes are required when the recovery code field is enabled', function (): void {
+    test('a one-time code is still required when the recovery code field is enabled but left blank', function (): void {
         $appAuthentication = Arr::first(Filament::getCurrentOrDefaultPanel()->getMultiFactorAuthenticationProviders());
 
         $userToAuthenticate = User::factory()
@@ -211,18 +242,18 @@ describe('validation', function (): void {
             ->assertNoRedirect()
             ->callAction(TestAction::make('useRecoveryCode')
                 ->schemaComponent("{$appAuthentication->getId()}.code", schema: 'multiFactorChallengeForm'))
-            // Once the recovery code field is enabled it becomes the active factor, so a blank recovery
-            // code must fail on the recovery code field rather than falling back to the one-time code.
+            // Enabling the recovery code field does not force the user down the recovery path: with the
+            // recovery code left blank the one-time code is still required, so it can be used instead.
             ->call('authenticate')
             ->assertHasFormErrors([
-                "{$appAuthentication->getId()}.recoveryCode" => 'required',
+                "{$appAuthentication->getId()}.code" => 'required',
             ], 'multiFactorChallengeForm')
             ->assertNoRedirect();
 
         $this->assertGuest();
     });
 
-    test('recovery codes are required when the recovery code field is enabled directly through the form state', function (): void {
+    test('a one-time code is still required when the recovery code field is enabled directly through the form state but left blank', function (): void {
         $appAuthentication = Arr::first(Filament::getCurrentOrDefaultPanel()->getMultiFactorAuthenticationProviders());
 
         $userToAuthenticate = User::factory()
@@ -238,11 +269,11 @@ describe('validation', function (): void {
             ->assertNotSet('userUndertakingMultiFactorAuthentication', null)
             ->assertNoRedirect()
             // Enabling the recovery code field by writing directly to the state, rather than through the
-            // action, must still require a recovery code so that exactly one factor is always validated.
+            // action, must still leave the one-time code required while the recovery code is blank.
             ->set("data.multiFactor.{$appAuthentication->getId()}.useRecoveryCode", true)
             ->call('authenticate')
             ->assertHasFormErrors([
-                "{$appAuthentication->getId()}.recoveryCode" => 'required',
+                "{$appAuthentication->getId()}.code" => 'required',
             ], 'multiFactorChallengeForm')
             ->assertNoRedirect();