Also, just so I don't forget:

• Edit forms should get "workflow" if we're going to prompt you when you submit them.

• Also, comment interactions should get a warning when you'll be required to MFA on submit, especially when you don't have MFA on your account.

I expect to make TOTP configuration a concrete object, then pursue Yubikey/U2F (T8787) and Duo (PHI912).

• U2F seems premature today given uneven browser support (see T8787 for more detailed discussion).
• Duo seems like it's actually a very good technical solution today, it's just unfortunate that it's a Cisco "Contact Sales" outfit now. This will entangle with T13229 somehow.

I maaaaaybe want to take a look at SMS just to figure out where it's at, too, although I think there's a mountain of evidence at this point that SMS MFA is very weak. I'm more interested in SMS for push notifications than MFA.

Given that U2F seems premature, I'm slightly more interested in SMS to get a second MFA integration upstream to better prove that the implementation is general enough to support push-style MFA factor types. Still, not sure where this is going to go.

D19935 prepares for multiple MFA provider types.

Before implementing an external Duo provider, I'd like to implement at least a second MFA provider in the upstream so Duo is less of a one-off. SMS (T920) seems like the most obvious choice since it's generally similar to Duo (both are API-based push factors).

Modern SMS implementation triggers some refactoring in mailers, and mailers also have some legacy code on the chopping block after the cluster.mailers change about a year ago. Some configuration simplification here is generally triggering nearby issues like T7477, and some backlogged issues like T12921 probably make sense to resolve here because of testing overlap.

(I'm just going to stick with this "Bonus Content" until we wrap MFA, however long that ends up taking, since there's a bunch of errata collected here.)

One of the changes here involves removing the default "To:" placeholder and replacing it with a voided placeholder instead. Adjacently, we should probably prevent applications from claiming RFC2142 email addresses, including abuse@ and hostmaster@. See also https://hackerone.com/reports/397792. Some addresses like support@ and security@ are fine, but addresses which might conceivably be used by CAs to validate domain ownership (ssladmin@) or MTAs / mailing lists (majordomo@) to do whatever inscrutable things MTAs do should likely be reserved and rejected.

One issue here is that existing MFA factors now require a concrete Factor object to associate with. To preserve existing behavior, we'd need to create a new one automatically on all installs. This inevitably turns into a little bit of a mess where we have some builtinKey factors and some organic factors. Nothing terrible, but not especially clean.

However, since "not supporting MFA" is a legitimate configuration state, I think a cleaner approach is probably to just migrate:

• If no user has TOTP MFA, bail.
• If no TOTP provider exists, create one. Otherwise, pick the first one.
• Point unattached TOTP factors (NULL value for factorProviderPHID) at whichever one we created/picked.
• We're done forever.
• Document how to do MFA config.

This is significantly simpler. The cost is that to enable TOTP MFA for the first time on a new install, you now need to go click a button somewhere, but that's probably good as we move forward toward broader MFA support since "just use TOTP" is no longer a completely unambiguous choice.

since there's a bunch of errata collected here

Here's a rollup:

One small leftover from D19792: it would be nice to show the bit-strength of TOTP secrets in the UI.

Tangential thought: at some point, the "Has MFA" filters in People should perhaps move to somewhere in Auth, with a more detailed UI for reviewing and managing MFA across user accounts.

Finally, T13138 notes a couple of MFA workflow/consistency improvements which could be revisited.

One adjacent change is T7667 (bin/auth lock / unlock).

Another change I'd like to make is to restore the "login page instructions" configuration. This was once a insecure blob of arbitrary HTML which we rightly got rid of, but we could let you put a blob of remarkup there easily enough. For secure, that would let us clarify that registration on this install is not open.

From rP1d0b99e1f8348ca222498551423154a0c37878fb, I'd like to add a hard-cap on password attempts per time period per remote address regardless of whether CAPTCHAs are configured or not. If you can crack CAPTCHAs or an install doesn't configure them (very reasonable given that configuring them lets Google run arbitrary Javascript on your install), we currently fail fairly wide open.

Existing flows should get an MFA workflow key where practical.

Where it makes sense, "High Security" workflows (that keep you in "sudo" mode) should also be reduced to "MFA" (one-shot) workflows. A good example of a workflow which can reasonably be reduced like this is the "Rename User" workflow. Another good examples is the Passphrase "Reveal Credential" workflow.

Edit forms should get "workflow" if we're going to prompt you when you submit them.

Also, comment interactions should get a warning when you'll be required to MFA on submit, especially when you don't have MFA on your account.

When no providers are configured, the MFA panel should probably vanish.

The new MFA stuff should be documented.

Statuses for providers ("Disabled", "Deprecated") don't do anything yet, but you can't edit them anyway.

And the remaining non-errata stuff is:

• SMS MFA.
• Duo MFA.

...which hopefully we're now in striking distance of.

SMS MFA

There are two pieces here before the actual MFA piece:

1. Sending SMS via MetaMTAMail.
2. Storing phone numbers for users.

(1) is likely straightforward.

For (2), one option is just to store the phone number on the factor: you click New MFA → SMS and enter a phone number. However, I think we're likely to outgrow this almost immediately given the other cases for SMS in T920, the likely need to verify/block/track-blocklist-state of numbers eventually (T13115), the difficulty of updating phone numbers, desire to prevent duplicate phone numbers, and so on.

So I'm leaning toward Settings → Contact Numbers or similar. For now this is probably just basic phone without too many fancy features, and New MFA → SMS says "Add a phone contact number first.". This could grow into APNS/Whatsapp/GCM/whatever eventually, and the pathway is pretty straightforward (no complicated migrations, etc).

Duo MFA

When we get there, I'm just going to put this one on the factor. Alternative is to make it an auth provider, but I think "login with Duo" isn't really much of a thing and since I'm not planning to bring the actual Duo magic sauce upstream I don't want to complicate things too much.

The fix for T12921: Link to referenced object in transaction emails (in D19968) is a bit buggy -- the links aren't generating as absolute links with a protocol and domain name. This is likely an easy fix, although I don't expect to get to it tonight.

I'm also not in love with linking "epriestley did stuff..." in every transaction. That seems pretty low value, especially when the actor is an application. I'm inclined to see if I can find an easy way to squelch that.

T13227 also needs to happen this week.

From D19975, two considerations:

• The "MFA Required" flow (login to a new account with no MFA, when require-mfa is configured) needs to be carefully tested to make sure it works properly, particularly when only one SMS provider is enabled (users must be able to set contact numbers). This may motivate reworking the "MFA Required" screen into more of a "subset of Settings" screen.
• We should have better documentation and/or tooling around session resets after MFA changes. Today, users who have existing MFA configurations will not be automatically required to update them after MFA provider changes (their existing sessions remain valid). You can bin/auth revoke --type session to force this, but we should provide clearer guidance around it and/or a "downgrade sessions to partial" tool.

The need for more UI support on the "MFA Required" flow is making me consider expanding scope here into knocking "Settings" back out to a full-width UI. I think that's not terribly hard. I'm maybe going to start there so it can get more testing as I go through everything else, and I can give up if it's harder than I think.

So I don't forget about it: the default settings/ UI is now incorrectly showing Conpherence settings, not date and time settings. Clicking anything works properly.

Also, I think we still have an issue with rate limiting where:

• You type some stuff in.
• The challenge has expired, and you get hit by "Wait Patiently".
• You still get points against you for rate limiting.

Currently, I think attackers can CSRF the MFA enroll endpoint (/mfa/new/?provider=xyz). This could send you an SMS, soon. This is mostly harmless but potentially a little annoying with push factors like SMS/Duo. I'll get a CSRF gate on it.

Also, managing contact numbers needs to require MFA or evading SMS MFA is trivial. 🐈

managing contact numbers

This also means that we need to prevent you from disabling or editing your primary contact number if you have SMS MFA configured: "You have SMS MFA configured, so you can not change your primary contact number. Remove the SMS MFA factor to edit your contact number."

This is a bit roundabout, but is actually the right flow with "MFA Required" I think -- it temporarily puts you back in MFA jail until you sort things out.

This will probably roll forward into a "next iteration", but it would be nice-to-have to show users the numbers we expect to text them from, so they can whitelist/anticipate the messages.

This is straightforward with Twilio and presumably also reasonable with SNS, although it might get a little tricky if an install has 9 providers configured in some complex series of fallbacks.

Rolling this mess up again:

• T13227 needs to happen.
• The actual SMS prompt needs CSRF.
• Read-Only / Disable for providers should be editable.
• MFA settings panel state for no read-only / no providers.
• Guidance on "Required" gate for read-only / no providers.
• "Kick everyone down to partial session" docs / tooling.
• MFAEngine usability:
  • When MFAEngine will require MFA, warn users before submitting comments.
  • When MFAEngine will require MFA, warn users before submitting EditEngine forms and put workflow on them.
• Existing Flows:
  • Reduce "require MFA session" to "require MFA token" where possible.
  • Put workflow keys on existing flows.
  • T13138 notes flow inconsistency on existing checkpoints.
• bin/auth lock/unlock
• Maybe move "who has MFA?" tools from the user directory to Auth.
• Show TOTP secret bit strength.
• Documentation.
• Some other stuff on the Auth workboard although this is mostly splash damage.
• Maybe guidance to users about "from" numbers, although this looks nontrivial for SNS.
• Maybe customizable enroll guidance.

I think the only non-cleanup sort of thing left is that the Duo provider needs to have editable fields, while the upstream (TOTP, MFA) providers do not. This probably means Duo integration ends up being three files (factor itself, plus two transaction types for "credential" and "hostname"), but that seems alright, and could be cheated through with a generic transaction implementation for the moment.

"Kick everyone down to partial session" docs / tooling.

This turned out to be mostly trivial since PhabricatorUser has a cache we can safely invalidate without too much effort.