Bug 1224099 - Add two-factor authentication user guide

[loganrosen]

BMO’s two-factor authentication preferences still directed users to an outdated wiki page, leaving current TOTP, Duo, recovery-code, and API authentication behavior undocumented alongside the application.

This adds a comprehensive user guide to the maintained Sphinx documentation, illustrated with screenshots from a disposable local account. The preferences page now links to that guide from every MFA state, with Selenium coverage protecting those entry points. The documented behavior was checked against the current MFA implementation and focused tests.

https://bugzilla.mozilla.org/show_bug.cgi?id=1224099

[Copilot]

Pull request overview

Adds a maintained 2FA guide covering TOTP, Duo, recovery codes, API authentication, and troubleshooting.

Changes:

• Adds the illustrated 2FA documentation.
• Links MFA preference states to the new guide.
• Adds Selenium coverage for guide links.

Show a summary per file

File	Description
template/en/default/account/prefs/mfa.html.tmpl	Adds consistent guide links across MFA states.
qa/t/2_test_login_totp.t	Verifies guide links during the TOTP workflow.
docs/en/rst/using/two-factor-authentication.rst	Provides the comprehensive 2FA guide.
docs/en/rst/using/index.rst	Adds the guide to the documentation navigation.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 4/8 changed files
• Comments generated: 0
• Review effort level: Medium

[dklawren]

A few things worth fixing before this lands. The prose reads well overall — most of these are places where the documented steps don't match what the UI actually requires.

1. The screenshots publish live 2FA credentials

docs/en/images/mfa-totp-enrollment.png contains the enrollment QR code at full
resolution (195x195, sharp). It decodes with any phone or zbarimg to:

otpauth://totp/Bugzilla%40Mozilla:admin%40mozilla.bugs?secret=<...>&issuer=Bugzilla%40Mozilla

docs/en/images/mfa-recovery-codes.png likewise shows all ten nine-digit
recovery codes in legible cleartext.

These are dev-instance credentials for admin@mozilla.bugs, so the practical risk
is low — but the page's own warning a few lines down says:

The QR code and manual secret can generate verification codes for your account.
Do not save screenshots of them or share them with anyone.

Publishing a scannable secret in the doc that gives that advice is not a great
look, and readers who compare the two will draw the wrong conclusion about what's
safe to share. Please regenerate both images with the QR code and the codes
blurred or replaced with placeholder digits.

2. "Generate Recovery Codes" is missing the submit step

docs/en/rst/using/two-factor-authentication.rst:149-153 — following these steps
literally never generates anything:

#. Return to the :guilabel:`Two-Factor Authentication` preferences tab.
#. Click :guilabel:`Generate Printable Recovery Codes`.
#. Enter your current password and either a current TOTP code or an unused
   recovery code.
#. Print the codes and store them in a secure offline location.

Clicking Generate Printable Recovery Codes in the page only reveals the
password/code fields — js/account.js:147 relabels the form's #update submit
button with that same text. Nothing is sent until the user clicks the (now
identically-labelled) button at the bottom of the form, so
SaveMFA/SaveMFAupdate('recovery') never runs.

This is the worst one to get wrong: the user believes they have recovery codes,
and finds out otherwise when their phone is gone. Suggest adding a step between
3 and 4 — e.g. #. Click :guilabel:Generate Printable Recovery Codes again to submit the form. — and it may be worth calling out explicitly that the button
appears twice.

3. "If You Lose Your Device" is missing the password and Submit Changes

Same class of problem at lines 223-230:

#. Click :guilabel:`Disable Two-factor Authentication`.
#. Verify with another unused recovery code.

The #mfa-auth-container that this reveals
(template/en/default/account/prefs/mfa.html.tmpl:138-150) also requires the
current password, and nothing is applied until Submit Changes is clicked. A
user following this steps enters a code, sees nothing happen, and burns further
recovery codes retrying.

4. The manual-secret toggle is above the QR code, not below it

Line 78:

#. If you cannot scan the QR code, use the option below it to display the secret,

In template/en/default/mfa/totp/enroll.html.tmpl:50-54 the
<div id="toggle"> holding Show as text is emitted before <img id="qr">, so
it renders above the QR code — which the PR's own mfa-totp-enrollment.png
screenshot shows. The audience for this step is specifically people whose device
can't scan, so it's the one step where a wrong location really costs something.

Worth naming the control too, since the step gives no other cue:

#. If you cannot scan the QR code, click :guilabel:`Show as text` above it to
   display the secret, then choose manual entry in your TOTP application and
   enter that secret.

5. The new help link needs target="_blank"

template/en/default/account/prefs/mfa.html.tmpl:322-326:

[% BLOCK help_blurb %]
  <p>
    <b>Need help with 2FA?</b>
    Read the <a href="[% MFA_HOWTO FILTER html %]">two-factor authentication user guide</a>.
  </p>
[% END %]

Every other external link in this template uses
target="_blank" rel="noopener noreferrer" (lines 192, 193, 202, 251, 259, 280),
and here it matters more than usual: the [% INCLUDE help_blurb %] at line 179 is
outside the warning branches, so unlike the old markup it now renders in the
ordinary enable/select state too — and js/account.js hides only #mfa-select,
so the paragraph stays on screen next to a freshly generated QR code.

A user who scans the QR, then clicks the guide link for the next instruction,
navigates the same tab away and loses the pending secret. Coming Back re-runs
user/mfa/totp/enroll, which issues a new secret, so the codes from the entry
they just scanned are rejected as "Invalid verification code" and they have to
delete the dead entry and start over.

6. qa/t/2_test_login_totp.t — third assertion isn't synchronised

The assertion at lines 71-75 follows click_ok('update') with no
title_is/wait_for_page_to_load, unlike the one at line 59-64. And
link=two-factor authentication user guide is rewritten by
Bugzilla::Test::Selenium::_fix_locator into
//a[normalize-space(text())="two-factor authentication user guide"], which
help_blurb renders identically in all three states.

So that check can resolve against the pre-submit page's matching anchor. If
someone later drops [% INCLUDE help_blurb %] from the mfa_recovery_token
branch (line 54), "MFA help is available with recovery codes" still passes and
the regression ships. Either add the page-load wait, or scope the locator to
something state-specific (e.g. an xpath relative to #mfa-recovery-frame).

Minor

• Missing anchors. docs/en/rst/style.rst says "Every heading should be
  preceded by an anchor, with a globally-unique name with no spaces." The
  top-level sections have them, but these seven don't: TOTP Code Is Rejected,
  Duo Prompt Does Not Load, No 2FA Method Is Available, Frequently Asked Questions, and the three FAQ entries. Without anchors nothing can :ref:
  them, and Sphinx's auto-generated slugs break silently on any rewording.
  preferences.rst, understanding.rst and finding.rst all follow the rule.

• Forced-enrollment state isn't covered. This PR adds the help link to the
  grace-period and hard-required screens (mfa.html.tmpl:34 and 164-173), which
  is where a confused user is most likely to click it — but the guide never
  explains why enrollment was forced, what the deadline means, or that automation
  accounts can file a bug to opt out of Duo (mfa.html.tmpl:205-207). A short
  section on that would serve the link's main audience.

• Drive-by, pre-existing: mfa.html.tmpl:256 has mismatched quoting —
  target="_blank rel="noopener noreferrer"" — so that link gets a literal
  _blank rel= target. Not from this PR, but you're in the file.

[loganrosen]

@dklawren Thanks for the detailed review. I addressed all of these in 562276b5b:

• Replaced the QR code and recovery codes with clearly labeled example values.
• Added the missing submit/password steps and corrected the Show as text location.
• Opened the guide safely in a new tab.
• Scoped the recovery-state Selenium assertion to the recovery iframe.
• Added the missing heading anchors and required-enrollment guidance.
• Removed the obsolete Windows Phone reference, flattened the remaining app list, and updated the “Other clients” link.

The template suite passes locally, and CI is running on the updated branch.

[Copilot]

Review details

Comments suppressed due to low confidence (1)

template/en/default/account/prefs/mfa.html.tmpl:42

• The no-password early-return branch now renders the help link, but the added Selenium assertions only cover the normal unenrolled, enabled, and recovery-code branches. Because this branch returns before the shared MFA container, a regression here would not be detected; add a case for an account whose cryptpassword is * and assert the guide URL.

  [% INCLUDE help_blurb %]

• Files reviewed: 4/8 changed files
• Comments generated: 0 new
• Review effort level: Medium

[dklawren]

Almost done. Just a few minor issues to resolve.

Issues

1. Hardcoded docs URL bypasses docs_urlbase (medium) — mfa.html.tmpl:9

Project convention is [% docs_urlbase FILTER html %]using/...html (25+ uses: search/form.html.tmpl:401, bug/tagging.html.tmpl:31). Precedent for hardcoding exists (account/prefs/apikey.html.tmpl:21), so not a blocker — but /en/latest/ pins to the dev build, not stable. Either use docs_urlbase or switch latest → stable.

2. Docs not built in CI (medium) — no sphinx job in .github/workflows/ci.yml, no .readthedocs.yaml in repo. Nothing validates the new RST: :ref:API key `` (target exists at using/preferences.rst:122), image paths, heading levels. Also .. figure:: is the first use in this repo (editing.rst:98 uses .. image::). Ask author to confirm a clean sphinx-build.

3. Test aborts instead of failing (low) — qa/t/2_test_login_totp.t:26

get_attribute dies when the locator misses, so the first assertion killing the file takes the whole TOTP suite with it. Author guarded the third check with is_element_present_ok('mfa-recovery-frame') — do the same for the first two.

4. URL duplicated in test (low) — $mfa_help_url hardcodes the same string as the template. Any URL change requires touching both. Consider asserting like(..., qr{two-factor-authentication.html$}).

5. Uncovered state (low) — [% INCLUDE help_blurb %] added to the no-password branch (cryptpassword == '*') has no test. Low value; noting for completeness.

6. Doc papers over a real UI bug (follow-up, not blocking)

The guide says: "BMO recovery codes cannot replace a Duo verification, even though the 2FA preferences page offers Duo users the recovery-code generator." Confirmed — the recovery button in mfa.html.tmpl:88-91 is unconditional, but Duo login goes through verify_prompt and never hits the recovery branch in MFA::verify_check. Documenting is right for now; file a follow-up to hide the button when user.mfa == 'Duo'.