Summary: Depends on D20057. Currently, we show an "MFA" message on one of these and an "Error" message on the other, with different icons and colors. Use "MFA" for both, with the MFA icon / color.
Test Plan: Hit both varations, saw more consistency.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D20059
Summary:
Depends on D20026. Ref T13222. Ref T13231. The primary change here is that we'll no longer send you an SMS if you hit an MFA gate without CSRF tokens.
Then there's a lot of support for genralizing into Duo (and other push factors, potentially), I'll annotate things inline.
Test Plan: Implemented Duo, elsewhere.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13231, T13222
Differential Revision: https://secure.phabricator.com/D20028
Summary:
Depends on D19899. Ref T13222. When we prompt you for one-shot MFA, we currently give you a lot of misleading text about your session staying in "high security mode".
Differentiate between one-shot and session upgrade MFA, and give the user appropriate cues and explanatory text.
Test Plan:
- Hit one-shot MFA on an "mfa" task in Maniphest.
- Hit session upgrade MFA in Settings > Multi-Factor.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19900
Summary:
Ref T13222. See PHI873. Ref T9770.
Currently, we support only TOTP MFA. For some MFA (SMS and "push-to-app"-style MFA) we may need to keep track of MFA details (e.g., the code we SMS'd you). There isn't much support for that yet.
We also currently allow free reuse of TOTP responses across sessions and workflows. This hypothetically enables some "spyglass" attacks where you look at someone's phone and type the code in before they do. T9770 discusses this in more detail, but is focused on an attack window starting when the user submits the form. I claim the attack window opens when the TOTP code is shown on their phone, and the window between the code being shown and being submitted is //much// more interesting than the window after it is submitted.
To address both of these cases, start tracking MFA "Challenges". These are basically a record that we asked you to give us MFA credentials.
For TOTP, the challenge binds a particular timestep to a given session, so an attacker can't look at your phone and type the code into their browser before (or after) you do -- they have a different session. For now, this means that codes are reusable in the same session, but that will be refined in the future.
For SMS / push, the "Challenge" would store the code we sent you so we could validate it.
This is mostly a step on the way toward one-shot MFA, ad-hoc MFA in comment action stacks, and figuring out what's going on with Duo.
Test Plan:
- Passed MFA normally.
- Passed MFA normally, simultaneously, as two different users.
- With two different sessions for the same user:
- Opened MFA in A, opened MFA in B. B got a "wait".
- Submitted MFA in A.
- Clicked "Wait" a bunch in B.
- Submitted MFA in B when prompted.
- Passed MFA normally, then passed MFA normally again with the same code in the same session. (This change does not prevent code reuse.)
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13222, T9770
Differential Revision: https://secure.phabricator.com/D19886
Summary:
Ref T12855. PHP7 introduced "Throwables", which are sort of like super exceptions. Some errors that PHP raises at runtime have become Throwables instead of old-school errors now.
The major effect this has is blank pages during development under PHP7 for certain classes of errors: they skip all the nice "show a pretty error" handlers and
This isn't a compelete fix, but catches the most common classes of unexpected Throwable and sends them through the normal machinery. Principally, it shows a nice stack trace again instead of a blank page for a larger class of typos and minor mistakes.
Test Plan:
Before: blank page. After:
{F5007979}
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12855
Differential Revision: https://secure.phabricator.com/D18136
Summary:
Ref T1806. Ref T7173. Depends on D14047.
Currently, all exception handling is in this big messy clump in `AphrontDefaultApplicationConfiguration`.
Split it out into modular classes. This will let a future change add new classes in the Phacility cluster which intercept particular exceptions we care about and replaces the default, generic responses with more useful, tailored responses.
Test Plan:
{F777391}
- Hit a Conduit error (made a method throw).
- Hit an Ajax error (made comment preview throw).
- Hit a high security error (tried to edit TOTP).
- Hit a rate limiting error (added a bunch of email addresses).
- Hit a policy error (tried to look at something with no permission).
- Hit an arbitrary exception (made a randomc ontroller throw).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1806, T7173
Differential Revision: https://secure.phabricator.com/D14049
