Confirming 2FA lacks error message and refreshes page/state incorrectly (Inertia stack)

ManuelLeiner commented 2 years ago

Jetstream Version: 2.7.1
Jetstream Stack: Inertia
Uses Teams: no
Laravel Version: 9.6.0
PHP Version: 8.1.2
Database Driver & Version: mysql

Description:

This is an Inertia stack only problem. The Livewire stack works without any problems.

When enabling 2FA and providing a wrong or empty confirmation code, the user will not see any error message. The UI partially refreshes, e.g. buttons change but texts stay the same. I am happy to provide a PR as soon as I find the time. I do not know how trivial the solution actually is. I provided a solution for one part and some sources for the other part of the problem, see below.

https://user-images.githubusercontent.com/8204574/161546265-028acf02-fa74-4e5d-bc68-3c77a62c6fc4.mov

Steps To Reproduce:

install a new application (docs)
install Laravel Jetstream (docs)
install Inertia stack (docs)
finalize installation (docs)
make sure the fortify 2FA-feature is enabled with 'confirm' => true, (default)
register a new user
open profile settings
click button to enable 2FA, $user->two_factor_secret is set in database
enter wrong or empty confirmation code
page refreshes, no error message is displayed, user in database gets reset, e.g. two_factor_secret is null again

Missing error message:

When provided a wrong/empty code, Fortify throws the validation error message into an error bag. Adding an additional option and provide the error bag name in the Vue component fixes that and the error can be retrieved as intended.

confirmationForm.post("/user/confirmed-two-factor-authentication", {
    errorBag: "confirmTwoFactorAuthentication",
    preserveScroll: true,
    preserveState: true,
    onSuccess: () => {
      confirming.value = false;
      qrCode.value = null;
      setupKey.value = null;
    },
  });

...

<JetInputError :message="confirmationForm.errors.code" class="mt-2" />

Refreshing page / state / user:

When starting the confirmation process, $user->two_factor_secret is set in the database. There is a computed property to check if 2FA is enabled. This computed property relies on the user's two_factor_secret, see here. After providing a wrong/empty conformation code, this code will be executed and the user gets reset in the database. The computed property then changes its value from true to false and partially refreshes the UI.

driesvints commented 2 years ago

@ps-sean do you have any ideas here?

ps-sean commented 2 years ago

As @ManuelLeiner mentioned, setting the error bag fixes the issue with the error not displaying. I manged to get the box to reopen by adding onError as below:

 confirmationForm.post('/user/confirmed-two-factor-authentication', {
        errorBag: 'confirmTwoFactorAuthentication',
        preserveScroll: true,
        preserveState: true,
        onSuccess: () => {
            confirming.value = false;
            qrCode.value = null;
            setupKey.value = null;
        },
        onError: () => {
            enableTwoFactorAuthentication();
        }
    });

However, this flickers when the page reloads. I don't know enough about inertia to know why that happens unfortunately.

ManuelLeiner commented 2 years ago

However, this flickers when the page reloads. I don't know enough about inertia to know why that happens unfortunately.

It flickers because the function to enable 2fa initiates another roundtrip (POST) to the backend, including 1) new qr code, 2) new setup key, 3) changing the secret and recovery keys in the database. Then the user actually has to scan the new qr code.

ps-sean commented 2 years ago

This seems to work better: const stillEnabling = ref(false);

const confirmTwoFactorAuthentication = () => {
    confirmationForm.post('/user/confirmed-two-factor-authentication', {
        errorBag: 'confirmTwoFactorAuthentication',
        preserveScroll: true,
        preserveState: true,
        onSuccess: () => {
            confirming.value = false;
            qrCode.value = null;
            setupKey.value = null;
            stillEnabling.value = false;
        },
        onError: () => {
            stillEnabling.value = true;
        }
    });
};

<div v-if="twoFactorEnabled || stillEnabling">
                <div v-if="qrCode">
                    <div class="mt-4 max-w-xl text-sm text-gray-600">
                        <p v-if="confirming" class="font-semibold">
                            To finish enabling two factor authentication, scan the following QR code using your phone's authenticator application or enter the setup key and provide the generated OTP code.
                        </p>

                        <p v-else>
                            Two factor authentication is now enabled. Scan the following QR code using your phone's authenticator application or enter the setup key.
                        </p>
                    </div>

                    <div class="mt-4" v-html="qrCode" />

                    <div class="mt-4 max-w-xl text-sm text-gray-600">
                        <p class="font-semibold">
                            Setup Key: <span v-html="setupKey"></span>
                        </p>
                    </div>

                    <div v-if="confirming" class="mt-4">
                        <JetLabel for="code" value="Code" />

                        <JetInput
                            id="code"
                            v-model="confirmationForm.code"
                            type="text"
                            name="code"
                            class="block mt-1 w-1/2"
                            inputmode="numeric"
                            autofocus
                            autocomplete="one-time-code"
                            @keyup.enter="confirmTwoFactorAuthentication"
                        />

                        <JetInputError :message="confirmationForm.errors.code" class="mt-2" />
                    </div>
                </div>

                <div v-if="recoveryCodes.length > 0 && ! confirming">
                    <div class="mt-4 max-w-xl text-sm text-gray-600">
                        <p class="font-semibold">
                            Store these recovery codes in a secure password manager. They can be used to recover access to your account if your two factor authentication device is lost.
                        </p>
                    </div>

                    <div class="grid gap-1 max-w-xl mt-4 px-4 py-4 font-mono text-sm bg-gray-100 rounded-lg">
                        <div v-for="code in recoveryCodes" :key="code">
                            {{ code }}
                        </div>
                    </div>
                </div>
            </div>

ManuelLeiner commented 2 years ago

Could you have a look at the user in the database? I think that the user at this point has already been reset in database. What I mean by that is, that the two_factor_secret and two_factor_recovery_code are set to null again. Therefore, stillEnabling is not a solution.

What happens is:

Inertia sends aPOST /user/confirmed-two-factor-authentication to confirm the code
A validation exception will be thrown
The Laravel exception handler converts the ValidationException to a redirect response, redirecting to the previous, i.e. profile, url
Then Inertia follows the redirect GET /profile (see screenshots)
Getting the user resets the two_factor columns in the database

The problem lies within UserProfileController@show where we are redirected to. There the user model gets reset in database. We have to prevent that reset, I guess, which I tried in the linked PR. The moment we try to work around in the Vue component is too late from a model in database perspective :)

laravel / jetstream