Adding Two Factor Auth Feature #84

tnylea · 2025-04-17T19:00:46Z

This PR adds Two Factor Authentication to this starter kit.

On the user settings page, a new tab called Two-Factor Auth will be added (we can rename this tab if desired).

When the user clicks that tab, they will see the following page which notifies them that 2FA is currently Disabled.

Clicking the Enable button will open a modal with a QR code and an alpha-numeric code. The user can enter this into their 2FA application (Google Authenticator).

Next, after clicking Continue, they’ll be prompted to enter the authentication code from their authenticator app.

If the user enters the correct code the modal will close and they will see the following screen.

This shows the user that 2FA is now Enabled and provides them with Recovery codes. Now, when the user logs in to their account, they will see an additional authentication page where they are prompted for their 2FA code.

The user may also click the Use Recovery Codes button, and they will be presented with an input to enter one of their recovery codes. A recovery code can only be used once. After the user enters the recovery code, it will no longer be valid. If the user adds all the codes to the input, the logic will strip out the first code and verify it.

The 2FA challenge page utilizes the auth layout, so it will look good on any layout the developer chooses.

Other Starter Kit 2FA Feature PR's

React: github.com/laravel/react-starter-kit/pull/101
Vue: github.com/laravel/vue-starter-kit/pull/120

app/Actions/TwoFactorAuth/GenerateQrCodeAndSecretKey.php

database/migrations/0001_01_01_000003_add_two_factor_auth_columns.php

lucasjose501 · 2025-04-17T23:22:12Z

I love this implementation, I did something similar in my project, and while I was thinking about security, I think there are two things to consider:

Make the database secret and recovery codes encrypted, someone with access to the database would be able to bypass the login with that information.
Ask for password confirmation before enabling/disabling, and showing recovery codes.

I'm not an expert in security, this is just something that crossed my mind.

tnylea · 2025-04-22T14:19:08Z

otp maxlength not working

This has been fixed in the latest commit. Thanks for letting me know 👏

jsmit99 · 2025-04-23T08:49:23Z

@tnylea great work, would love to see it getting in there.

Btw (might be offtopic a little) but does anyone has any suggestion on what best way to implement this in existing project (based on laravel livewire starter kit) is?

lokomilo · 2025-04-23T11:22:52Z

app/Actions/TwoFactorAuth/GenerateQrCodeAndSecretKey.php

+    public string $companyName;
+
+    /**
+     * Generate new recovery codes for the user.


Suggested change

* Generate new recovery codes for the user.

* Generate a QR code image and secret key for the user.

lucasjose501 · 2025-04-23T17:32:29Z

@tnylea great work, would love to see it getting in there.

Btw (might be offtopic a little) but does anyone has any suggestion on what best way to implement this in existing project (based on laravel livewire starter kit) is?

The best way is probably waiting for the PR to merge, so everything is reviewed and ready to use, and then look at the changed files and copy/merge into your project by hand.

joetannenbaum

Haven't reviewed the inner workings of the code just yet, but have some nits before I dive in (see comments).

First, love the screen visuals, design part looks great.

Some other notes (all notes refer to the images below them):

This looks like a button, is there another way to style this to make it look more like a status or indicator?

This looks great, couple of things:

The lock icon should be closed? Unless open in standard in this case?
Would love a “copy” and/or “download” button
We’re mixing zinc (body bg color) with stone and the grays feel off, maybe we lock in on zinc? Not a designer but it’s triggering my spidey sense.

Punched in some random numbers on the 2FA screen after login to see what the error looks like and got this (also happens on correct code entry afterwards).

joetannenbaum · 2025-04-24T15:53:12Z

app/Actions/TwoFactorAuth/CompleteTwoFactorAuthentication.php

+    public function __invoke($user): void
+    {
+        // Get the remember preference from the session (default to false if not set)
+        $remember = Session::get('login.remember', false);


Any reason to not use Session::pull since you're doing it below anyway?

joetannenbaum · 2025-04-24T15:53:40Z

app/Actions/TwoFactorAuth/GenerateNewRecoveryCodes.php

+     * @param  mixed  $user
+     * @return void
+     */
+    public function __invoke($user): Collection


We're not using this argument, remove?

joetannenbaum · 2025-04-24T15:53:51Z

app/Actions/TwoFactorAuth/DisableTwoFactorAuthentication.php

+
+namespace App\Actions\TwoFactorAuth;
+
+use App\Models\User;


Unused import

joetannenbaum · 2025-04-24T15:54:13Z

app/Actions/TwoFactorAuth/GenerateQrCodeAndSecretKey.php

+use BaconQrCode\Renderer\ImageRenderer;
+use BaconQrCode\Renderer\RendererStyle\RendererStyle;
+use BaconQrCode\Writer;
+use App\Models\User;


Unused import

resources/css/app.css

joetannenbaum · 2025-04-24T16:05:46Z

resources/views/components/input-otp.blade.php

+                }
+            }
+        }
+        let that = this;


If you use an arrow function for the setTimeout, don't you avoid having to set this variable?

joetannenbaum · 2025-04-24T16:06:22Z

resources/views/components/input-otp.blade.php

+            let focusLastInput = (paste.length <= this.total_digits) ? paste.length : this.total_digits;
+            this.$refs['input' + focusLastInput].focus();
+            if (paste.length >= this.total_digits) {
+                let that = this;


See arrow function comment above

joetannenbaum · 2025-04-24T16:07:29Z

resources/views/livewire/auth/login.blade.php

@@ -29,20 +32,56 @@ public function login(): void

        $this->ensureIsNotRateLimited();

-        if (! Auth::attempt(['email' => $this->email, 'password' => $this->password], $this->remember)) {
+        // Get the user model from auth config in a single line
+        $userAttemptingLogin = User::where('email', $this->email)->first();


What if the user isn't coming from User model? Can we tap into the auth user resolver?

joetannenbaum · 2025-04-24T16:08:42Z

resources/views/livewire/auth/two-factor-challenge.blade.php

+            RateLimiter::clear($this->throttleKey($user));
+            
+            // Redirect to the intended page
+            return $this->redirectIntended(default: route('dashboard', absolute: false), navigate: true);


If you're returning here, no need for an else statement below

So, the ProcessRecoveryCode::class will return false if the entered recovery code does not match. If it returns false, we need to display a message to the user, which occurs in the else condition.

However, feel free to elaborate on your thinking; perhaps there's something I'm missing.

Thanks!

joetannenbaum · 2025-04-24T16:10:46Z

tests/Feature/Auth/TwoFactorAuthTest.php

+use App\Actions\TwoFactorAuth\GenerateQrCodeAndSecretKey;
+use App\Actions\TwoFactorAuth\GenerateNewRecoveryCodes;
+use Illuminate\Support\Facades\Session;
+use Illuminate\Support\Facades\Hash;


Unused import

valorin · 2025-04-28T05:34:46Z

My reply to the discussion about not using Auth::attempt() is hidden (at least for me) under multiple collapsed sections, so I'm duplicating it here so it's not missed:

I'm not saying you need to use Auth::attempt() here - it makes sense to use Auth::login() given the situation. However running Auth::login() on it's own isn't enough.

During a successful login Auth::attempt() fires the Attempting event, and triggers password rehashing. Both of these are missing and will not occur when a user has 2FA enabled. The Attempting event is an important event for security logging/monitoring, as it gives visibility on login attempts, and rehashing is important to ensure user password hashes are kept updated.

Also, I'm pretty sure the new logic will also fail to trigger Failed when a login is failed, which is also an incredibly important event for security logging/monitoring.

My suggestion is to either replicate those events and rehashing within the starter kit logic, or create a methods on the Guard that will fire the events and perform rehashing, but also support the 2-step process needed here. I can see it being useful for others, so something in the framework could be useful. Also would avoid duplicating across the different starter kits.

As for Fortify not using Auth::attempt(), it's probably wrong too. I haven't audited it's code, so don't know the mechanics. All I can tell you is that this specific PR is missing functionality.

resources/views/livewire/auth/two-factor-challenge.blade.php

valorin · 2025-04-30T05:52:35Z

resources/views/livewire/auth/two-factor-challenge.blade.php

+     */
+    protected function throttleKey(User $user): string
+    {
+        return Str::transliterate($user->id . '|2fa|' . request()->ip());


I suggest dropping the IP and limiting purely based on User ID. Otherwise, it would be possible for an attacker to rotate their IP to bypass the limit.

iamsandakelum · 2025-04-30T13:37:35Z

There's a critical error when attempting a two factor verification, on rate limitation code.

It's the "ensureIsNotRateLimited" function at \resources\views\livewire\auth\two-factor-challenge.blade.php

Fix would be this:

protected function ensureIsNotRateLimited(): void
{
    $user = app(GetTwoFactorAuthenticatableUser::class)();

    if (! RateLimiter::tooManyAttempts($this->throttleKey($user), 5)) {
        return;
    }

    event(new Lockout(request()));

    $seconds = RateLimiter::availableIn($this->throttleKey($user));

    throw ValidationException::withMessages([
        'auth_code' => __('auth.throttle', [
            'seconds' => $seconds,
            'minutes' => ceil($seconds / 60),
        ]),
    ]);
}

Fixes #84 (review)

iamsandakelum · 2025-04-30T13:40:39Z

I think it would be wise to confirm the 2FA code once again before allowing to disable the 2FA. At the moment, just a button click disables it off.

tnylea · 2025-04-30T18:03:24Z

Thanks for the review, @joetannenbaum; I've implemented all the recommendations, as well as the security recommendations from @valorin (I appreciate it!).

I've also made a few additional security updates, pointed out in this comment: laravel/vue-starter-kit#120 (comment), from the Vue PR.

@iamsandakelum, thank you for pointing out the issue above. I've gone ahead and fixed it.

tnylea added 13 commits April 4, 2025 17:31

Adding files for 2FA

aed4c2d

Adding some finishing touches to 2fa

3265500

removing middleware file to rename

ebcd24a

Adding correct file

f541ff6

Adding updates to support dark mode in custom designs

dc4bdae

Adding a few more updates to recovery code section

74c255f

Re-organization

43397be

updating user model with user model from config/auth

36f5121

fixing the login and auth middleware

e346f42

cleaning up some of the tests

07ab1be

refactoring the challenge auth page with new Actions

00eafe4

clean and refactor

fd98bc2

Remove decryption of 2FA secret when verifying authentication code

3623e9e

tnylea temporarily deployed to Testing April 17, 2025 19:00 — with GitHub Actions Inactive

Jubeki reviewed Apr 17, 2025

View reviewed changes

app/Actions/TwoFactorAuth/GenerateQrCodeAndSecretKey.php Outdated Show resolved Hide resolved

Adding a few more updates

329f6f7

tnylea temporarily deployed to Testing April 17, 2025 19:08 — with GitHub Actions Inactive

rendering the QR code using SVG as opposed to png with Imagick

6347ecc

tnylea temporarily deployed to Testing April 17, 2025 19:41 — with GitHub Actions Inactive

tnylea marked this pull request as ready for review April 17, 2025 19:43

reverting the style back to simple auth layout

33ae2b1

tnylea temporarily deployed to Testing April 17, 2025 22:17 — with GitHub Actions Inactive

lucasjose501 reviewed Apr 17, 2025

View reviewed changes

database/migrations/0001_01_01_000003_add_two_factor_auth_columns.php Show resolved Hide resolved

updating syntax in the migration

5f5645a

tnylea temporarily deployed to Testing April 18, 2025 14:47 — with GitHub Actions Inactive

tnylea temporarily deployed to Testing April 22, 2025 14:18 — with GitHub Actions Inactive

Adding security fixes to 2fa functionality

039933f

tnylea temporarily deployed to Testing April 22, 2025 14:33 — with GitHub Actions Inactive

Adding throttling and security fixes to the 2fa challenge

87f4219

tnylea temporarily deployed to Testing April 22, 2025 14:42 — with GitHub Actions Inactive

keeping things consistent with React version

d693b5b

tnylea temporarily deployed to Testing April 22, 2025 14:54 — with GitHub Actions Inactive

tnylea mentioned this pull request Apr 22, 2025

Adding 2fa feature for Vue Starter laravel/vue-starter-kit#120

Open

lokomilo reviewed Apr 23, 2025

View reviewed changes

duncanmcclean mentioned this pull request Apr 24, 2025

[6.x] Two-Factor Authentication statamic/cms#11664

Merged

wip

43506e1

joetannenbaum temporarily deployed to Testing April 24, 2025 16:13 — with GitHub Actions Inactive

joetannenbaum reviewed Apr 24, 2025

View reviewed changes

valorin reviewed Apr 28, 2025

View reviewed changes

resources/views/livewire/auth/two-factor-challenge.blade.php Show resolved Hide resolved

valorin reviewed Apr 30, 2025

View reviewed changes

Added better security practices, cleaned code and fixed a few bugs

f0b8a5a

tnylea temporarily deployed to Testing April 30, 2025 17:51 — with GitHub Actions Inactive

tnylea requested a review from joetannenbaum April 30, 2025 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Two Factor Auth Feature #84

Adding Two Factor Auth Feature #84

tnylea commented Apr 17, 2025 •

edited

Loading

lucasjose501 commented Apr 17, 2025

tnylea commented Apr 22, 2025

jsmit99 commented Apr 23, 2025

lokomilo Apr 23, 2025

lucasjose501 commented Apr 23, 2025

joetannenbaum left a comment

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

joetannenbaum Apr 24, 2025

tnylea Apr 30, 2025

joetannenbaum Apr 24, 2025

valorin commented Apr 28, 2025

valorin Apr 30, 2025

iamsandakelum commented Apr 30, 2025 •

edited

Loading

iamsandakelum commented Apr 30, 2025

tnylea commented Apr 30, 2025

	* Generate new recovery codes for the user.
	* Generate a QR code image and secret key for the user.

Adding Two Factor Auth Feature #84

Are you sure you want to change the base?

Adding Two Factor Auth Feature #84

Conversation

tnylea commented Apr 17, 2025 • edited Loading

lucasjose501 commented Apr 17, 2025

tnylea commented Apr 22, 2025

jsmit99 commented Apr 23, 2025

Choose a reason for hiding this comment

lucasjose501 commented Apr 23, 2025

joetannenbaum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valorin commented Apr 28, 2025

Choose a reason for hiding this comment

iamsandakelum commented Apr 30, 2025 • edited Loading

iamsandakelum commented Apr 30, 2025

tnylea commented Apr 30, 2025

tnylea commented Apr 17, 2025 •

edited

Loading

iamsandakelum commented Apr 30, 2025 •

edited

Loading