Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to login after setting up the 2fa #18

Closed
Jacotheron opened this issue Jun 21, 2022 · 6 comments · Fixed by #20
Closed

Unable to login after setting up the 2fa #18

Jacotheron opened this issue Jun 21, 2022 · 6 comments · Fixed by #20

Comments

@Jacotheron
Copy link

Most probably I have done something wrong while setting everything up (this project is very early development still).

So for this project, all users are required to enable 2fa; so on my authenticated routes, I have added the 2fa.enabled middleware. While I am using Laravel Breeze, for now I am mostly using your original views (though I created a 'confirm', 'prepare' and 'recovery' view to also use during the setup of 2fa for a user). I am also using the Facade to handle the login (as per the readme, set in the LoginRequest).

I further copied the "ConfirmtwoFactorCodeController" over to my other controllers, and modified it to also handle the setup of the 2fa for a user, using the following routes:

Route::get('2fa-notice', [\App\Http\Controllers\Frontend\ConfirmTwoFactorCodeController::class, 'notifyTwoFactor'])->name('2fa.notice');
Route::get('2fa-prepare', [\App\Http\Controllers\Frontend\ConfirmTwoFactorCodeController::class, 'prepareTwoFactor'])->name('2fa.prepare');
Route::post('2fa-prepare', [\App\Http\Controllers\Frontend\ConfirmTwoFactorCodeController::class, 'confirmTwoFactor'])->name('2fa.confirm');
Route::get('2fa-recovery', [\App\Http\Controllers\Frontend\ConfirmTwoFactorCodeController::class, 'showRecoveryCodes'])->name('2fa.recovery');

So the setup part is working, and the app code, verifies correctly.

The issue is happening when I attempt to log in. I provide the email and password, and then see the screen asking for the code (weird thing here is it immediately shows the field as having an error, saying the code is either wrong or expired - this is prior to me providing any code). Then when I provide the correct code, it tells me the login was unsuccessful (basically that both the email and password fields are required).

Any idea how I can solve this issue.

@DarkGhostHunter
Copy link
Contributor

DarkGhostHunter commented Jun 22, 2022

The issue is happening when I attempt to log in. I provide the email and password, and then see the screen asking for the code (weird thing here is it immediately shows the field as having an error, saying the code is either wrong or expired - this is prior to me providing any code).

This is because it detects that, at first try, the credentials don't have the 2fa_code key, which is mandatory for 2FA to work. I fixed that in the dev version, where the warning doesn't show if the key is not present. If it's present, but empty or invalid, the error will show.

Then when I provide the correct code, it tells me the login was unsuccessful (basically that both the email and password fields are required).

Because you have $request->validate([ ... ]); before the helper, where you're requiring both email and password.

This is not clear in the docs, so I will update the docs to reflect that these should be mandatory if the user is not adding a 2FA Code already.

@DarkGhostHunter DarkGhostHunter linked a pull request Jun 22, 2022 that will close this issue
@DarkGhostHunter
Copy link
Contributor

Fixed in next patch version.

@Jacotheron
Copy link
Author

Thank you for the quick response (I am now able to login). For anyone wondering how to implement it with Breeze, here is what I did:

In LoginRequest, the 'rules' method

      if($this->isNotFilled('2fa_code')) {
            return [
                'email' => ['required', 'string', 'email'],
                'password' => ['required', 'string'],
            ];
        }

        return [
            '2fa_code' => ['required']
        ];

This is simply due to using the FormRequest to validate, even before hitting the controller.

@tklie
Copy link
Contributor

tklie commented Jun 22, 2022

@DarkGhostHunter I'm currently working on a project using Fortify and am facing a similar issue, where I enter my credentials on the login page, this then redirects me to the F2A view, and after entering the code I am redirected back to the login view with the error messages that the email and password fields are required.

As far as I can tell, this is because the F2A view sends a POST request to the login route on submit (just like the original login request), but it only submits the 2fa_code entered. In the old version of Laraguard, this view received the credentials as hidden input fields so they could be included in the final login request. How does this work now?

@DarkGhostHunter
Copy link
Contributor

@DarkGhostHunter I'm currently working on a project using Fortify and am facing a similar issue, where I enter my credentials on the login page, this then redirects me to the F2A view, and after entering the code I am redirected back to the login view with the error messages that the email and password fields are required.

As far as I can tell, this is because the F2A view sends a POST request to the login route on submit (just like the original login request), but it only submits the 2fa_code entered. In the old version of Laraguard, this view received the credentials as hidden input fields so they could be included in the final login request. How does this work now?

#18 (comment)

@DarkGhostHunter
Copy link
Contributor

Oh! You're using Fortify. Use their 2FA system instead.

Otherwise, you're bound to change the authentication pipeline by overriding the AttemptToAuthenticate action and LoginRequest Request.

Yeah, Fortify is good but too unflexible to mix their 2FA with my 2FA.

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 a pull request may close this issue.

3 participants