RecoveryCodeReplaced dispatched twice when two-factor.login receive a valid recovery code

[joke2k]

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

fortify/src/Http/Controllers/TwoFactorAuthenticatedSessionController.php

Lines 57 to 64 in 1711276

	public function store(TwoFactorLoginRequest $request)
	{
	$user = $request->challengedUser();
	if ($code = $request->validRecoveryCode()) {
	$user->replaceRecoveryCode($code);
	event(new RecoveryCodeReplaced($user, $code));

but User::replaceRecoveryCode already dispatches the exact same event

fortify/src/TwoFactorAuthenticatable.php

Lines 47 to 57 in 1711276

	public function replaceRecoveryCode($code)
	{
	$this->forceFill([
	'two_factor_recovery_codes' => encrypt(str_replace(
	$code,
	RecoveryCode::generate(),
	decrypt($this->two_factor_recovery_codes)
	)),
	])->save();
	RecoveryCodeReplaced::dispatch($this, $code);

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' => 'taylor@laravel.com',
            '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);
    }

[developerluanramos]

@joke2k Can I try to resolve this one ?

[github-actions]

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!

[joke2k]

@developerluanramos sure you can consider that recently the dispatch was added to the Authenticable trait by this PR #564
and all other package events are raised within action classes.

Considering this I would remove revert that PR completely.

[aman-evoenergyio]

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.

[coolAlias]

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!

[joke2k]

thanks @coolAlias