Page MenuHomePhabricator
Create Task
Maniphest T6115

Allow multi-factor authentication to be a requirement for user subgroups, including administrators
Open, WishlistPublic
Actions

Description

On our installation, we'd like to enforce multi-factor authentication for all administrators only, while allowing users to optionally use TOTP codes where they see fit. I believe I discussed this with @epriestley at one point, and it would require a bit of work to do, but it'd be a nice addition.

The real reason we'd like to enforce this is because we'd like to add a secondary authentication factor, Duo Security, and then require administrators to all use Duo as their second factor for their logins.

Related Objects

Event Timeline

thoughtpolice created this task.Sep 17 2014, 3:16 PM
thoughtpolice raised the priority of this task from to Needs Triage.
thoughtpolice updated the task description. (Show Details)
thoughtpolice added a project: Phabricator.
thoughtpolice added a subscriber: thoughtpolice.
epriestley added a subscriber: epriestley.Sep 18 2014, 1:00 AM

Some previous discussion in T5089.

epriestley triaged this task as Wishlist priority.Sep 18 2014, 1:00 AM
epriestley edited projects, added Auth; removed Phabricator.
qgil added a subscriber: qgil.Sep 18 2014, 8:31 AM
epriestley mentioned this in T6549: Backup codes for multi-factor authentication.Feb 18 2015, 5:14 AM
eadler added a subscriber: eadler.Apr 28 2015, 3:49 AM
thoughtpolice added a comment.Aug 24 2015, 8:29 PM

Just as a note: I was thinking about this, and in fact, I cooked up a small (untested) patch for it to require Administrators to have MFA (see end of post) through a new setting.

However, thinking about it more, I think simply having a policy selection dialogue (like we do with 'View' and 'Edit' policies) is the right way to go. I think this covers all use cases, more along the lines of what was hinted at in T6549.

A policy selection allows you to immediately require all users, administrators, individuals or any subset of users to have MFA of some kind, which I think is overall the nicest. Per-user requirements can be totally subsumed by this I think, so this is the real win.

The reality is that with the advent of things like Spaces, I can definitely see myself requiring more granular MFA designs. For example, I may have a company where client data is all in one space and relatively safe, and clients may not be required to have MFA enabled just to file bugs with me or whatever in their Space. But, my team of employees may definitely all want MFA, and it's absolutely OK for some of them to not be Administrators - they still have important levels of access even without that which MFA can help protect. For example if an employee account is compromised some random way, you could just go and have Phabricator generate a new SSH key that you download from the UI, or even just uploading your own, giving them push access to any repositories (I believe SSH keys are under 'High Security' mode at the moment if you have MFA enabled). Being able to require MFA over a specific group of project users for example would fix this.

Anyway, this patch is untested, but if anyone is Brave and Bold I think this is all that's needed (probably not tho):

commit e7f6a9cb9f5c4281c031e3d1303d7596599be1fc
Author: Austin Seipp <aseipp@pobox.com>
Date:   Wed Aug 19 19:20:59 2015 -0500

    T6115: Add 'security.require-admin-multi-factor-auth'
    
    Signed-off-by: Austin Seipp <aseipp@pobox.com>

diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index 33c7dfc..730b03b 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -51,7 +51,12 @@ abstract class PhabricatorController extends AphrontController {
       return false;
     }
 
-    return PhabricatorEnv::getEnvConfig('security.require-multi-factor-auth');
+    if (PhabricatorEnv::getEnvConfig('security.require-multi-factor-auth')) {
+      return true;
+    }
+
+    return $user->getIsAdmin() &&
+      PhabricatorEnv::getEnvConfig('security.require-admin-multi-factor-auth');
   }
 
   public function shouldAllowLegallyNonCompliantUsers() {
diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
index b8ff3cc..8995a45 100644
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@@ -121,6 +121,25 @@ final class PhabricatorSecurityConfigOptions
             pht('Multi-Factor Optional'),
           )),
       $this->newOption(
+          'security.require-admin-multi-factor-auth', 'bool', false)
+        ->setLocked(true)
+        ->setSummary(
+          pht(
+            'Require Administrators to configure multi-factor authentication.'))
+        ->setDescription(
+          pht(
+            'By default, Phabricator allows any user to add multi-factor '.
+            'authentication to their accounts, but does not require it. '.
+            'By enabling this option, you can force administrators to add '.
+            'at least one authentication factor before they can use their '.
+            'accounts. Note that this is always overridden by the '.
+            '`security.require-multi-factor-auth` option.'))
+        ->setBoolOptions(
+          array(
+            pht('Multi-Factor Required'),
+            pht('Multi-Factor Optional'),
+          )),
+      $this->newOption(
         'phabricator.csrf-key',
         'string',
         '0b7ec0592e0a2829d8b71df2fa269b2c6172eca3')
greggrossmeier awarded a token.Sep 4 2015, 5:40 PM
greggrossmeier added a subscriber: greggrossmeier.
epriestley mentioned this in T10917: Build Glorious High-Security Notification/Audit Infrastructure.May 4 2016, 2:04 PM
thoughtpolice added a project: Haskell.org.May 13 2016, 6:08 PM
hskiba added a subscriber: hskiba.Sep 20 2017, 1:28 AM
epriestley mentioned this in T13222: 2018 Week 48-51 Bonus Content.Nov 26 2018, 5:07 PM
epriestley moved this task from Backlog to MFA on the Auth board.Dec 12 2018, 8:29 PM
epriestley mentioned this in D20031: Allow MFA providers to be deprecated or disabled.Jan 26 2019, 7:30 PM
epriestley renamed this task from Allow multi-factor authentication to be a requirement for administrators to Allow multi-factor authentication to be a requirement for user subgroups, including administrators.Feb 7 2019, 3:09 PM
Phabricator · Built by Phacility