Skip to content

Conversation

@pushpak1300
Copy link
Member

@pushpak1300 pushpak1300 commented Nov 18, 2025

This fixes a small papercut that happened when running the database seeder on a fresh installation. The seeder was creating a user with two factor enabled right away, which meant the user couldn’t log in.

@firebed
Copy link
Contributor

firebed commented Nov 18, 2025

Another perspective would be to remove the two_factor_* columns from the definition inside the UserFactory an rename the withoutTwoFactor to withTwoFactor and inverse the logic. This way the devs decide when to enable the 2fa functionality when writing tests and it's consistent with what happens when a user registers for the first time (the 2fa is disabled by default). Well, this is what I've been doing since the 2fa got integrated.

@pushpak1300
Copy link
Member Author

@firebed I had the same thought, but it ended up feeling inconsistent with the current factory model, especially with the existing unverified method:

@firebed
Copy link
Contributor

firebed commented Nov 18, 2025

I'm ok with this. Just expressing my opinion: The issue is that the user factory is creating users with two-factor authentication enabled by default. This means that any user created with User::factory() throughout the codebase (in tests, other seeders, or during development) will have 2FA enabled and won't be able to log in without additional configuration. Developers will need to remember to add ->withoutTwoFactor() everywhere they use the factory.
I believe this is exactly the issue you run into 🙂

@pushpak1300 pushpak1300 marked this pull request as draft November 18, 2025 15:28
@findrakecil
Copy link

I think I know why we get invalid payload error when login using test user. It is because the value of two_factor_secret and two_factor_recovery_codes is not encrypted (see the code to test 2FA in this commit)

It should be:

return [
    ...
    'two_factor_secret' => encrypt(Str::random(10)),
    'two_factor_recovery_codes' => encrypt(Str::random(10)),
    'two_factor_confirmed_at' => now(),
];

The problem is, we wouldn't know the secret to generate 2FA codes 😅

I tried to decrypt('two_factor_secret_value_from_database') using php artisan tinker and use the decrypted secret on authenticator app, but I get This secret key is not compatible with Google Authenticator. error when submit the generated code from the decrypted secret.

I agree with @firebed perspective on how it should be done in the first place and feel the same with @pushpak1300.

I think @pushpak1300 solution is the most effective one-line solution if we don't want to refactor the code too much 😂

@a-coding-a
Copy link

For anyone coming across this and only uses the User factory for non-prod, just update the 2fa secret and recovery code to some basic plain text values. Then use the recovery code to login.
Only if you wanna keep the 2fa behaviours anyway

database/factories/UserFactory.php

    public function definition(): array
    {
        return [
            ...
            'two_factor_secret' => encrypt('TESTSECRET'),
            'two_factor_recovery_codes' => encrypt(json_encode(['recovery-code-1'])),
            'two_factor_confirmed_at' => now(),
        ];
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants