Skip to content

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

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

Closed
ManuelLeiner opened this issue Apr 4, 2022 · 5 comments · Fixed by #1030
Closed
Labels

Comments

@ManuelLeiner
Copy link
Contributor

  • 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.

Screen.Recording.2022-04-04.at.14.35.06.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.

@ManuelLeiner ManuelLeiner changed the title Confirm 2FA lacks error message and refreshes page/state incorrectly (Inertia stack) Confirming 2FA lacks error message and refreshes page/state incorrectly (Inertia stack) Apr 4, 2022
@driesvints
Copy link
Member

@ps-sean do you have any ideas here?

@ps-sean
Copy link
Contributor

ps-sean commented Apr 5, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

ps-sean commented Apr 5, 2022

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
Copy link
Contributor Author

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

image

image

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants