DarkGhostHunter / Laraguard

"On-premises 2FA Authentication for all your users out-of-the-box
MIT License
266 stars 24 forks source link

Documentation v4 - not asking for 2FA code #68

Closed dragonfly4 closed 3 years ago

dragonfly4 commented 3 years ago

Dear

Upgrading v2 to v4 results in the login process skipping checks for users that have enabled 2FA. To simplify things I installed a clean Laravel v8.62 / PHP v8.0.11 to test it from scratch and the check / validation is not being executed.

'How it works' says no extra code is needed. https://github.com/DarkGhostHunter/Laraguard#how-this-works

It includes a custom view and a callback to handle the Two-Factor authentication itself during login attempts. Works without middleware or new guards, but you can go full manual if you want.

Contract & trait are added to my User model https://github.com/DarkGhostHunter/Laraguard#usage

Login part mentions overriding Login in LoginController. Is this needed for v4 please? https://github.com/DarkGhostHunter/Laraguard#logging-in

This forces everyone to use 2FA during login

To login, the user must issue a TOTP code along their credentials. Simply use attemptWhen() with Laraguard, which will automatically do the checks for you. By default, it checks for the 2fa_code input name, but you can issue your own.

FYI: In Laravel 8 the default User model is located in App\Models and the readme still references to namespace App

DarkGhostHunter commented 2 years ago

Came here to say v4 from v2 is not supported.

dragonfly4 commented 2 years ago

Digging deeper I saw I would have to rewrite a lot and modify my login process. So I have swapped Laravel/UI for Laravel/Fortify with built-in 2FA support. Your package has more features but let's hope Fortify gets a big update with Laravel v9.x

DarkGhostHunter commented 2 years ago

I was pretty much forced to do this package since there was no Fortify when I need it.

I’ll probably archive this package since Fortify exists with a cleaner implementation.

Italo Baeza C.

El 29-10-2021, a la(s) 04:39, dragonfly4 @.***> escribió:

 Digging deeper I saw I would have to rewrite a lot and modify my login process. So I have swapped Laravel/UI for Laravel/Fortify with built-in 2FA support. Your package has more features but let's hope Fortify gets a big update with Laravel v9.x

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

dragonfly4 commented 2 years ago

Correct but Fortify has some limitations and security risks.

I also had to extend some functions and write extra validation checks. I wouldn't switch yet if I was you.

DarkGhostHunter commented 2 years ago

Correct but Fortify has some limitations and security risks.

Like what?

dragonfly4 commented 2 years ago

Correct but Fortify has some limitations and security risks.

Like what?

https://github.com/laravel/fortify/issues/201

https://github.com/laravel/fortify/issues/322

https://github.com/laravel/fortify/issues/285 This doesn't work for now, need to investigate further.

++ you can re-use a TOPT so dangerous for MITM attack.

DarkGhostHunter commented 2 years ago

Thanks for the heads up. Seems weird that they decided to go for their own route considering this package already works like a charm (at least for me).