Page MenuHomePhabricator

Add multi-factor auth and TOTP support

Authored by epriestley on Apr 27 2014, 6:52 PM.



Ref T4398. This is still pretty rough and isn't exposed in the UI yet, but basically works. Some missing features / areas for improvement:

  • Rate limiting attempts (see TODO).
  • Marking tokens used after they're used once (see TODO), maybe. I can't think of ways an attacker could capture a token without also capturing a session, offhand.
  • Actually turning this on (see TODO).
  • This workflow is pretty wordy. It would be nice to calm it down a bit.
  • But also add more help/context to help users figure out what's going on here, I think it's not very obvious if you don't already know what "TOTP" is.
  • Add admin tool to strip auth factors off an account ("Help, I lost my phone and can't log in!").
  • Add admin tool to show users who don't have multi-factor auth? (so you can pester them)
  • Generate QR codes to make the transfer process easier (they're fairly complicated).
  • Make the "entering hi-sec" workflow actually check for auth factors and use them correctly.
  • Turn this on so users can use it.
  • Adding SMS as an option would be nice eventually.
  • Adding "password" as an option, maybe? TOTP feels fairly good to me.

I'll post a couple of screens...

Test Plan
  • Added TOTP token with Google Authenticator.
  • Added TOTP token with Authy.

Diff Detail

rP Phabricator
Lint Skipped
Unit Tests Skipped

Event Timeline

epriestley updated this revision to Diff 21056.Apr 27 2014, 6:52 PM
epriestley retitled this revision from to Add multi-factor auth and TOTP support.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley updated this revision to Diff 21057.Apr 27 2014, 6:54 PM
  • Fix a missing getPHID().

Main panel:

Choose a factor type (some day, SMS or "physical keyfob" or whatever else could go here);

Really wordy dialog which requires you type a lot of stuff into your phone right now:

New logs (the "unknown" stuff is fixed):

btrahan accepted this revision.Apr 28 2014, 4:22 PM
btrahan edited edge metadata.

V. nice.

This revision is now accepted and ready to land.Apr 28 2014, 4:22 PM
epriestley closed this revision.Apr 28 2014, 4:27 PM
epriestley updated this revision to Diff 21076.

Closed by commit rP17709bc1674e (authored by @epriestley).