RecoveryCodeReplaced dispatched twice when two-factor.login receive a valid recovery code
Fortify Version
1.25.3
Laravel Version
10.48.27
PHP Version
8.1
Database Driver & Version
No response
Description
The TwoFactorAuthenticatedSessionController dispatches a RecoveryCodeReplaced event https://github.com/laravel/fortify/blob/17112764d9f6a81a002bddfc3e305e73f3d19b85/src/Http/Controllers/TwoFactorAuthenticatedSessionController.php#L57-L64
but User::replaceRecoveryCode already dispatches the exact same event https://github.com/laravel/fortify/blob/17112764d9f6a81a002bddfc3e305e73f3d19b85/src/TwoFactorAuthenticatable.php#L47-L57
I’m not sure where it would make the most sense to keep the dispatch, but I think it’s reasonable to avoid triggering it twice.
Steps To Reproduce
public function test_two_factor_challenge_with_recovery_code_dispatches_RecoveryCodeReplaced_twice()
{
Event::fake();
$user = UserWithTwoFactor::forceCreate([
'name' => 'Taylor Otwell',
'email' => '[email protected]',
'password' => bcrypt('secret'),
'two_factor_recovery_codes' => encrypt(json_encode(['invalid-code', 'valid-code'])),
]);
$response = $this->withSession([
'login.id' => $user->id,
'login.remember' => false,
])->withoutExceptionHandling()->post('/two-factor-challenge', [
'recovery_code' => 'valid-code',
]);
Event::assertDispatchedTimes(RecoveryCodeReplaced::class, 2);
}
@joke2k Can I try to resolve this one ?
Thank you for reporting this issue!
As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.
If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.
Thank you!
@developerluanramos sure you can consider that recently the dispatch was added to the Authenticable trait by this PR https://github.com/laravel/fortify/pull/564 and all other package events are raised within action classes.
Considering this I would remove revert that PR completely.
Same issue. Trying to listen for the event to notify my user that a recovery code was replaced. But, the email is being sent twice as the event is dispatched twice.
Unless I am completely missing something I would assume that the event should be raised once and via the controller not the trait.
But any solution to this would be appreciated.
TwoFactorAuthenticatedSessionController triggers the event after calling TwoFactorAuthenticatable::replaceRecoveryCode, which ALSO triggers the event.
Arguably, only the latter should be triggering the event, and not TwoFactorAuthenticatedSessionController.
On the other hand, @joke2k makes a good point about consistency, so perhaps reverting #564 would be the proper move.
In either case, there is now a PR to fix it one way, and an option to revert to fix it the other way. Please choose one. Thanks!