One setup step remaining, two QR Code Scans required

Created on 6 October 2024, 6 months ago

Conversation on Slack in #contrib-tfa: https://drupal.slack.com/archives/C7SR7TWMS/p1728082318265809

I have TFA working with Google Auth on my D10 dev site, however when I scan the QR code and enter the verification code, it is successful:

Status message
Application code verified. One setup step remaining.
TFA setup complete.

The page is reloaded and focus is placed back in the verification code box, but won't accept the current TOTP code.

Clicking "Skip and Finish" takes me back to /user/{id}/security/tfa and shows that TFA is setup and working. I can then log out and back in, using TFA and it all works. So yes it works, but that UI won't work on prod. Every user would be super confused.

Disabling TFA and going through the process again, it turns out it wants me to scan the QR, enter the auth code, then scan the QR again and enter a second auth code, leaving me with two identically named Google Authenticator entries with unique codes.

That process gives me the expected:

Status message
TFA setup complete.

Summary: TFA is making me scan the QR code twice, add two Auth code accounts, to consider setup complete.

πŸ› Bug report
Status

Active

Version

1.8

Component

User interface

Created by

πŸ‡ΊπŸ‡ΈUnited States cmarcera

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @cmarcera
  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    Updated issue with reproduction steps.

    It appears this only occurs when setting up a plugin that is not the 'default' plugin type.

    Need to review code to determine what the original intended logic was before evaluating resolutions.

  • πŸ‡©πŸ‡ͺGermany lmoeni

    I came across this issue while using multiple plugin types. I can confirm that this only occurs for the plugin types that are not the default plugin type.
    The user does not get redirected to the overview but instead also has a "Skip and finish" button. When the user clicks on that button, the status message: "Setup of TFA Email one-time password (EOTP) canceled." appears, which suggests that the setup did not happen but it did.

    I removed most of the tfaNextSetupStep() function in a patch, which solved this problem for me. I could not find any further issues without the removed code.

  • πŸ‡΅πŸ‡ΉPortugal tmiguelv

    Hi!

    I found out that the form plugin was not updating when the form was rebuilding.
    I've created a little patch that updates the plugin before the form renders again.

  • πŸ‡¨πŸ‡¦Canada tame4tex

    This bug occurred for me while setting up the TFA TOTP validation plugin which I have set as the default plugin. I have no other validation plugins enabled but I do have the TFA Trusted Browser login plugin enabled.

    I can confirm the patch provided in #4 πŸ› One setup step remaining, two QR Code Scans required Active fixes the issue. Thanks for the fix @tmiguelv.

  • Status changed to Needs review about 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    D.O. has migrated away from patch files.

    In order for tests to run we require all submissions to be in the form of a Merge Request.

    More details may be found at: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... β†’

    Quick glancing:
    I suspect there will be a PHPCS warning on the comment in the patch file,
    I haven’t analyzed the logic flow.

  • πŸ‡©πŸ‡ͺGermany ammaletu Bonn, Germany

    Same here as for tame4tex. Drupal 10.3.13 and tfa module 1.10.0. I had the TOTP method set as default, with a second method enabled. When I tried to set up the TOTP for a user, the code scan was requested twice. This stopped with the patch.

  • πŸ‡©πŸ‡ͺGermany ammaletu Bonn, Germany

    I created an issue fork and committed the patch from #4 by tmiguelv. I fixed the comment to adhere to Drupal guidelines. For me, this works. but I also didn't analyze the logic flow in detail.

    I see there are some functional tests already. Why aren't they picking up this behavior? With the patch applied, the second step of the form was setting up the trusted browsers. `TfaTotpSetupPluginTest` probably doesn't use this feature, so it is not a multi-step form. I think, `testPluginSetup` should be copied to a `testPluginSetupMultiStep`. Or should we add a new test for the trusted browser feature?

  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    Quick glance, it appears the tests run with just a single plugin, they would not hit this condition. which is reasonable, the tests as written are plugin specific and should not be testing the rest of the general usage.

    Ideally yes this would be a dedicated test that performs a mulit-step setup to validate the multi step form. Ideally built off the test plugins and not the production plugins.

Production build 0.71.5 2024