Page MenuHomePhorge

Explain consequences when adding second Multi-Factor Auth
AcceptedPublic

Authored by aklapper on Tue, May 20, 12:10.

Details

Summary

Warn users who already have MFA set up that adding a second Multi-Factor Auth will require entering both instead of being able to choose from one of them.
This is currently not clear. I was surprised by this, now I have another user also surprised.

Closes T16081

Test Plan
  1. As an admin, set up TOTP as an auth provider at http://phorge.localhost/auth/mfa/
  2. As a user, add a first TOTP auth factor at http://phorge.localhost/settings/panel/multifactor/
  3. As a user, try to add a second TOTP auth factor and see an additional sentence in the dialog

Diff Detail

Repository
rP Phorge
Branch
T16081secondMFA (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 2001
Build 2001: arc lint + arc unit

Event Timeline

Seems nice. Maybe we can generalize the text a bit to avoid "both" (since you can have multiple of them).

While testing I'm curious whenever it renders an empty <p></p> of if it's discarded. It seems it will generate an empty paragraph to me. So maybe we want to avoid to call that appendParagraph() (?)

Rephrase to not mentioned "second MFA"; Avoid empty HTML paragraph

Awesome. Optional bonus point, since this legacy feature thing is indeed veeeery confusing and very-Phorgi, maybe we may want to add a "Why" Phorge decided this, and highlight potential pitfalls.

Note: You already have an Auth Factor configured. Adding another factor will require you to always provide all Auth Factors, thus, dramatically increasing your corporate security (at the price of slowing you down even more).

lol

Hmm could you share why it's a "legacy feature"? I guess I'm clueless

(I just mean the feature comes from Phabricator, not from Phorge; sorry I always pick random words to express myself)

Maybe better to just expand dialog, so, less code to maintain (?)

$dialog = $this->newDialog()
  ->setTitle(pht('Choose Factor Type'))
  ->appendChild($menu)
  ->addCancelButton($cancel_uri);

if ($viewer->getIsEnrolledInMultiFactor()) {
  $dialog->appendParagraph(pht('Pi Pi Pi Bla Bli'));
}

return $dialog;

(Nice ping-pong lol)

Love this. Feel free to evaluate the addition of thus, dramatically increasing your corporate security (at the price of slowing you down even more). as stated here lol https://we.phorge.it/D26028#27452 or whatever small thing could help to better visualize this alien behavior that is very peculiar of Phorge (that is, as already said, not intuitive).

This revision is now accepted and ready to land.Tue, May 20, 12:48

I'm too stuck in Serious Business Mode to introduce another string to translate, I'm afraid :-/

mainframe98 added inline comments.
src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php
263–264

By using NOTE: in remarkup (and using appendRemarkup, we get fancy styling that is more eye-catching:

NOTE: You already have an Auth Factor configured. Adding another factor will require you to always provide all Auth Factors instead of selecting one of your Auth Factors.