Allow MFA providers to be deprecated or disabled

Summary: Ref T13222. Providers can now be deprecated (existing factors still work, but users can't add new factors for the provider) or disabled (factors stop working, also can't add new ones).

Test Plan:
  - Enabled, deprecated, and disabled some providers.
  - Viewed provider detail, provider list.
  - Viewed MFA settings list.
  - Verified that I'm prompted for enabled + deprecated only at gates.
  - Tried to disable final provider, got an error.
  - Hit the MFA setup gate by enabling "Require MFA" with no providers, got a more useful message.
  - Immediately forced a user to the "MFA Setup Gate" by disabling their only active provider with another provider enabled ("We no longer support TOTP, you HAVE to finish Duo enrollment to continue starting Monday.").

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D20031

src/__phutil_library_map__.php

@@ -2234,6 +2234,8 @@ phutil_register_library_map(array(
     'PhabricatorAuthFactorProviderListController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php',
     'PhabricatorAuthFactorProviderNameTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderNameTransaction.php',
     'PhabricatorAuthFactorProviderQuery' => 'applications/auth/query/PhabricatorAuthFactorProviderQuery.php',
+    'PhabricatorAuthFactorProviderStatus' => 'applications/auth/constants/PhabricatorAuthFactorProviderStatus.php',
+    'PhabricatorAuthFactorProviderStatusTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php',
     'PhabricatorAuthFactorProviderTransaction' => 'applications/auth/storage/PhabricatorAuthFactorProviderTransaction.php',
     'PhabricatorAuthFactorProviderTransactionQuery' => 'applications/auth/query/PhabricatorAuthFactorProviderTransactionQuery.php',
     'PhabricatorAuthFactorProviderTransactionType' => 'applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php',
@@ -7961,6 +7963,8 @@ phutil_register_library_map(array(
     'PhabricatorAuthFactorProviderListController' => 'PhabricatorAuthProviderController',
     'PhabricatorAuthFactorProviderNameTransaction' => 'PhabricatorAuthFactorProviderTransactionType',
     'PhabricatorAuthFactorProviderQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorAuthFactorProviderStatus' => 'Phobject',
+    'PhabricatorAuthFactorProviderStatusTransaction' => 'PhabricatorAuthFactorProviderTransactionType',
     'PhabricatorAuthFactorProviderTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorAuthFactorProviderTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorAuthFactorProviderTransactionType' => 'PhabricatorModularTransactionType',

src/applications/auth/constants/PhabricatorAuthFactorProviderStatus.php

@@ -0,0 +1,103 @@
+<?php
+
+final class PhabricatorAuthFactorProviderStatus
+  extends Phobject {
+
+  private $key;
+  private $spec = array();
+
+  const STATUS_ACTIVE = 'active';
+  const STATUS_DEPRECATED = 'deprecated';
+  const STATUS_DISABLED = 'disabled';
+
+  public static function newForStatus($status) {
+    $result = new self();
+
+    $result->key = $status;
+    $result->spec = self::newSpecification($status);
+
+    return $result;
+  }
+
+  public function getName() {
+    return idx($this->spec, 'name', $this->key);
+  }
+
+  public function getStatusHeaderIcon() {
+    return idx($this->spec, 'header.icon');
+  }
+
+  public function getStatusHeaderColor() {
+    return idx($this->spec, 'header.color');
+  }
+
+  public function isActive() {
+    return ($this->key === self::STATUS_ACTIVE);
+  }
+
+  public function getListIcon() {
+    return idx($this->spec, 'list.icon');
+  }
+
+  public function getListColor() {
+    return idx($this->spec, 'list.color');
+  }
+
+  public function getFactorIcon() {
+    return idx($this->spec, 'factor.icon');
+  }
+
+  public function getFactorColor() {
+    return idx($this->spec, 'factor.color');
+  }
+
+  public function getOrder() {
+    return idx($this->spec, 'order', 0);
+  }
+
+  public static function getMap() {
+    $specs = self::newSpecifications();
+    return ipull($specs, 'name');
+  }
+
+  private static function newSpecification($key) {
+    $specs = self::newSpecifications();
+    return idx($specs, $key, array());
+  }
+
+  private static function newSpecifications() {
+    return array(
+      self::STATUS_ACTIVE => array(
+        'name' => pht('Active'),
+        'header.icon' => 'fa-check',
+        'header.color' => null,
+        'list.icon' => null,
+        'list.color' => null,
+        'factor.icon' => 'fa-check',
+        'factor.color' => 'green',
+        'order' => 1,
+      ),
+      self::STATUS_DEPRECATED => array(
+        'name' => pht('Deprecated'),
+        'header.icon' => 'fa-ban',
+        'header.color' => 'indigo',
+        'list.icon' => 'fa-ban',
+        'list.color' => 'indigo',
+        'factor.icon' => 'fa-ban',
+        'factor.color' => 'indigo',
+        'order' => 2,
+      ),
+      self::STATUS_DISABLED => array(
+        'name' => pht('Disabled'),
+        'header.icon' => 'fa-times',
+        'header.color' => 'red',
+        'list.icon' => 'fa-times',
+        'list.color' => 'red',
+        'factor.icon' => 'fa-times',
+        'factor.color' => 'grey',
+        'order' => 3,
+      ),
+    );
+  }
+
+}

src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php

@@ -197,6 +197,8 @@ final class PhabricatorAuthNeedsMultiFactorController
         ->addCancelButton('/', pht('Continue'));
     }
 
+    $views = array();
+
     $messages = array();
 
     $messages[] = pht(
@@ -210,7 +212,39 @@ final class PhabricatorAuthNeedsMultiFactorController
       ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
       ->setErrors($messages);
 
-    return $view;
+    $views[] = $view;
+
+
+    $providers = id(new PhabricatorAuthFactorProviderQuery())
+      ->setViewer($viewer)
+      ->withStatuses(
+        array(
+          PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE,
+        ))
+      ->execute();
+    if (!$providers) {
+      $messages = array();
+
+      $required_key = 'security.require-multi-factor-auth';
+
+      $messages[] = pht(
+        'This install has the configuration option "%s" enabled, but does '.
+        'not have any active multifactor providers configured. This means '.
+        'you are required to add MFA, but are also prevented from doing so. '.
+        'An administrator must disable "%s" or enable an MFA provider to '.
+        'allow you to continue.',
+        $required_key,
+        $required_key);
+
+      $view = id(new PHUIInfoView())
+        ->setTitle(pht('Multi-Factor Authentication is Misconfigured'))
+        ->setSeverity(PHUIInfoView::SEVERITY_ERROR)
+        ->setErrors($messages);
+
+      $views[] = $view;
+    }
+
+    return $views;
   }
 
 }

src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php

@@ -20,6 +20,16 @@ final class PhabricatorAuthFactorProviderListController
         ->setHeader($provider->getDisplayName())
         ->setHref($provider->getURI());
 
+      $status = $provider->newStatus();
+
+      $icon = $status->getListIcon();
+      $color = $status->getListColor();
+      if ($icon !== null) {
+        $item->setStatusIcon("{$icon} {$color}", $status->getName());
+      }
+
+      $item->setDisabled(!$status->isActive());
+
       $list->addItem($item);
     }
 

src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php

@@ -58,6 +58,15 @@ final class PhabricatorAuthFactorProviderViewController
       ->setHeader($provider->getDisplayName())
       ->setPolicyObject($provider);
 
+    $status = $provider->newStatus();
+
+    $header_icon = $status->getStatusHeaderIcon();
+    $header_color = $status->getStatusHeaderColor();
+    $header_name = $status->getName();
+    if ($header_icon !== null) {
+      $view->setStatus($header_icon, $header_color, $header_name);
+    }
+
     return $view;
   }
 

src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php

@@ -95,6 +95,8 @@ final class PhabricatorAuthFactorProviderEditEngine
   protected function buildCustomEditFields($object) {
     $factor_name = $object->getFactor()->getFactorName();
 
+    $status_map = PhabricatorAuthFactorProviderStatus::getMap();
+
     return array(
       id(new PhabricatorStaticEditField())
         ->setKey('displayType')
@@ -109,6 +111,14 @@ final class PhabricatorAuthFactorProviderEditEngine
         ->setDescription(pht('Display name for the MFA provider.'))
         ->setValue($object->getName())
         ->setPlaceholder($factor_name),
+      id(new PhabricatorSelectEditField())
+        ->setKey('status')
+        ->setTransactionType(
+          PhabricatorAuthFactorProviderStatusTransaction::TRANSACTIONTYPE)
+        ->setLabel(pht('Status'))
+        ->setDescription(pht('Status of the MFA provider.'))
+        ->setValue($object->getStatus())
+        ->setOptions($status_map),
     );
   }
 

src/applications/auth/engine/PhabricatorAuthSessionEngine.php

@@ -473,9 +473,20 @@ final class PhabricatorAuthSessionEngine extends Phobject {
     $factors = id(new PhabricatorAuthFactorConfigQuery())
       ->setViewer($viewer)
       ->withUserPHIDs(array($viewer->getPHID()))
-      ->setOrderVector(array('-id'))
+      ->withFactorProviderStatuses(
+        array(
+          PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE,
+          PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED,
+        ))
       ->execute();
 
+    // Sort factors in the same order that they appear in on the Settings
+    // panel. This means that administrators changing provider statuses may
+    // change the order of prompts for users, but the alternative is that the
+    // Settings panel order disagrees with the prompt order, which seems more
+    // disruptive.
+    $factors = msort($factors, 'newSortVector');
+
     // If the account has no associated multi-factor auth, just issue a token
     // without putting the session into high security mode. This is generally
     // easier for users. A minor but desirable side effect is that when a user

src/applications/auth/factor/PhabricatorAuthFactor.php

@@ -54,11 +54,15 @@ abstract class PhabricatorAuthFactor extends Phobject {
     return null;
   }
 
-  public function canCreateNewConfiguration(PhabricatorUser $user) {
+  public function canCreateNewConfiguration(
+    PhabricatorAuthFactorProvider $provider,
+    PhabricatorUser $user) {
     return true;
   }
 
-  public function getConfigurationCreateDescription(PhabricatorUser $user) {
+  public function getConfigurationCreateDescription(
+    PhabricatorAuthFactorProvider $provider,
+    PhabricatorUser $user) {
     return null;
   }
 

src/applications/auth/factor/PhabricatorSMSAuthFactor.php

@@ -67,7 +67,10 @@ final class PhabricatorSMSAuthFactor
     return $messages;
   }
 
-  public function canCreateNewConfiguration(PhabricatorUser $user) {
+  public function canCreateNewConfiguration(
+    PhabricatorAuthFactorProvider $provider,
+    PhabricatorUser $user) {
+
     if (!$this->loadUserContactNumber($user)) {
       return false;
     }
@@ -75,7 +78,9 @@ final class PhabricatorSMSAuthFactor
     return true;
   }
 
-  public function getConfigurationCreateDescription(PhabricatorUser $user) {
+  public function getConfigurationCreateDescription(
+    PhabricatorAuthFactorProvider $provider,
+    PhabricatorUser $user) {
 
     $messages = array();
 

src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php

@@ -7,6 +7,7 @@ final class PhabricatorAuthFactorConfigQuery
   private $phids;
   private $userPHIDs;
   private $factorProviderPHIDs;
+  private $factorProviderStatuses;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -28,6 +29,11 @@ final class PhabricatorAuthFactorConfigQuery
     return $this;
   }
 
+  public function withFactorProviderStatuses(array $statuses) {
+    $this->factorProviderStatuses = $statuses;
+    return $this;
+  }
+
   public function newResultObject() {
     return new PhabricatorAuthFactorConfig();
   }
@@ -42,34 +48,54 @@ final class PhabricatorAuthFactorConfigQuery
     if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
-        'id IN (%Ld)',
+        'config.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
-        'phid IN (%Ls)',
+        'config.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->userPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'userPHID IN (%Ls)',
+        'config.userPHID IN (%Ls)',
         $this->userPHIDs);
     }
 
     if ($this->factorProviderPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'factorProviderPHID IN (%Ls)',
+        'config.factorProviderPHID IN (%Ls)',
         $this->factorProviderPHIDs);
     }
 
+    if ($this->factorProviderStatuses !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'provider.status IN (%Ls)',
+        $this->factorProviderStatuses);
+    }
+
     return $where;
   }
 
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
+
+    if ($this->factorProviderStatuses !== null) {
+      $joins[] = qsprintf(
+        $conn,
+        'JOIN %R provider ON config.factorProviderPHID = provider.phid',
+        new PhabricatorAuthFactorProvider());
+    }
+
+    return $joins;
+  }
+
   protected function willFilterPage(array $configs) {
     $provider_phids = mpull($configs, 'getFactorProviderPHID');
 
@@ -94,6 +120,10 @@ final class PhabricatorAuthFactorConfigQuery
     return $configs;
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'config';
+  }
+
   public function getQueryApplicationClass() {
     return 'PhabricatorAuthApplication';
   }

src/applications/auth/storage/PhabricatorAuthFactorConfig.php

@@ -80,6 +80,12 @@ final class PhabricatorAuthFactorConfig
     return $this;
   }
 
+  public function newSortVector() {
+    return id(new PhutilSortVector())
+      ->addInt($this->getFactorProvider()->newStatus()->getOrder())
+      ->addInt($this->getID());
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 

src/applications/auth/storage/PhabricatorAuthFactorProvider.php

@@ -14,15 +14,11 @@ final class PhabricatorAuthFactorProvider
 
   private $factor = self::ATTACHABLE;
 
-  const STATUS_ACTIVE = 'active';
-  const STATUS_DEPRECATED = 'deprecated';
-  const STATUS_DISABLED = 'disabled';
-
   public static function initializeNewProvider(PhabricatorAuthFactor $factor) {
     return id(new self())
       ->setProviderFactorKey($factor->getFactorKey())
       ->attachFactor($factor)
-      ->setStatus(self::STATUS_ACTIVE);
+      ->setStatus(PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE);
   }
 
   protected function getConfiguration() {
@@ -117,6 +113,20 @@ final class PhabricatorAuthFactorProvider
     return $this->getFactor()->getEnrollButtonText($this, $user);
   }
 
+  public function newStatus() {
+    $status_key = $this->getStatus();
+    return PhabricatorAuthFactorProviderStatus::newForStatus($status_key);
+  }
+
+  public function canCreateNewConfiguration(PhabricatorUser $user) {
+    return $this->getFactor()->canCreateNewConfiguration($this, $user);
+  }
+
+  public function getConfigurationCreateDescription(PhabricatorUser $user) {
+    return $this->getFactor()->getConfigurationCreateDescription($this, $user);
+  }
+
+
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
 
 

src/applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php

@@ -0,0 +1,103 @@
+<?php
+
+final class PhabricatorAuthFactorProviderStatusTransaction
+  extends PhabricatorAuthFactorProviderTransactionType {
+
+  const TRANSACTIONTYPE = 'status';
+
+  public function generateOldValue($object) {
+    return $object->getStatus();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setStatus($value);
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $old_display = PhabricatorAuthFactorProviderStatus::newForStatus($old)
+      ->getName();
+    $new_display = PhabricatorAuthFactorProviderStatus::newForStatus($new)
+      ->getName();
+
+    return pht(
+      '%s changed the status of this provider from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderValue($old_display),
+      $this->renderValue($new_display));
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+    $actor = $this->getActor();
+
+    $map = PhabricatorAuthFactorProviderStatus::getMap();
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+
+      if (!isset($map[$new_value])) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Status "%s" is invalid. Valid statuses are: %s.',
+            $new_value,
+            implode(', ', array_keys($map))),
+          $xaction);
+        continue;
+      }
+
+      $require_key = 'security.require-multi-factor-auth';
+      $require_mfa = PhabricatorEnv::getEnvConfig($require_key);
+
+      if ($require_mfa) {
+        $status_active = PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE;
+        if ($new_value !== $status_active) {
+          $active_providers = id(new PhabricatorAuthFactorProviderQuery())
+            ->setViewer($actor)
+            ->withStatuses(
+              array(
+                $status_active,
+              ))
+            ->execute();
+          $active_providers = mpull($active_providers, null, 'getID');
+          unset($active_providers[$object->getID()]);
+
+          if (!$active_providers) {
+            $errors[] = $this->newInvalidError(
+              pht(
+                'You can not deprecate or disable the last active MFA '.
+                'provider while "%s" is enabled, because new users would '.
+                'be unable to enroll in MFA. Disable the MFA requirement '.
+                'in Config, or create or enable another MFA provider first.',
+                $require_key));
+            continue;
+          }
+        }
+      }
+    }
+
+    return $errors;
+  }
+
+  public function didCommitTransaction($object, $value) {
+    $status = PhabricatorAuthFactorProviderStatus::newForStatus($value);
+
+    // If a provider has undergone a status change, reset the MFA enrollment
+    // cache for all users. This may immediately force a lot of users to redo
+    // MFA enrollment.
+
+    // We could be more surgical about this: we only really need to affect
+    // users who had a factor under the provider, and only really need to
+    // do anything if a provider was disabled. This is just a little simpler.
+
+    $table = new PhabricatorUser();
+    $conn = $table->establishConnection('w');
+
+    queryfx(
+      $conn,
+      'UPDATE %R SET isEnrolledInMultiFactor = 0',
+      $table);
+  }
+
+}

src/applications/people/storage/PhabricatorUser.php

@@ -908,9 +908,15 @@ final class PhabricatorUser
    * @task factors
    */
   public function updateMultiFactorEnrollment() {
-    $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere(
-      'userPHID = %s',
-      $this->getPHID());
+    $factors = id(new PhabricatorAuthFactorConfigQuery())
+      ->setViewer($this)
+      ->withUserPHIDs(array($this->getPHID()))
+      ->withFactorProviderStatuses(
+        array(
+          PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE,
+          PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED,
+        ))
+      ->execute();
 
     $enrolled = count($factors) ? 1 : 0;
     if ($enrolled !== $this->isEnrolledInMultiFactor) {

src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php

@@ -53,8 +53,8 @@ final class PhabricatorMultiFactorSettingsPanel
     $factors = id(new PhabricatorAuthFactorConfigQuery())
       ->setViewer($viewer)
       ->withUserPHIDs(array($user->getPHID()))
-      ->setOrderVector(array('-id'))
       ->execute();
+    $factors = msort($factors, 'newSortVector');
 
     $rows = array();
     $rowc = array();
@@ -69,7 +69,16 @@ final class PhabricatorMultiFactorSettingsPanel
         $rowc[] = null;
       }
 
+      $status = $provider->newStatus();
+      $status_icon = $status->getFactorIcon();
+      $status_color = $status->getFactorColor();
+
+      $icon = id(new PHUIIconView())
+        ->setIcon("{$status_icon} {$status_color}")
+        ->setTooltip(pht('Provider: %s', $status->getName()));
+
       $rows[] = array(
+        $icon,
         javelin_tag(
           'a',
           array(
@@ -95,21 +104,24 @@ final class PhabricatorMultiFactorSettingsPanel
       pht("You haven't added any authentication factors to your account yet."));
     $table->setHeaders(
       array(
+        null,
         pht('Name'),
         pht('Type'),
         pht('Created'),
-        '',
+        null,
       ));
     $table->setColumnClasses(
       array(
+        null,
         'wide pri',
-        '',
+        null,
         'right',
         'action',
       ));
     $table->setRowClasses($rowc);
     $table->setDeviceVisibility(
       array(
+        true,
         true,
         false,
         false,
@@ -129,12 +141,15 @@ final class PhabricatorMultiFactorSettingsPanel
       $add_color = PHUIButtonView::GREY;
     }
 
+    $can_add = (bool)$this->loadActiveMFAProviders();
+
     $buttons[] = id(new PHUIButtonView())
       ->setTag('a')
       ->setIcon('fa-plus')
       ->setText(pht('Add Auth Factor'))
       ->setHref($this->getPanelURI('?new=true'))
       ->setWorkflow(true)
+      ->setDisabled(!$can_add)
       ->setColor($add_color);
 
     $buttons[] = id(new PHUIButtonView())
@@ -155,21 +170,18 @@ final class PhabricatorMultiFactorSettingsPanel
 
     // Check that we have providers before we send the user through the MFA
     // gate, so you don't authenticate and then immediately get roadblocked.
-    $providers = id(new PhabricatorAuthFactorProviderQuery())
-      ->setViewer($viewer)
-      ->withStatuses(array(PhabricatorAuthFactorProvider::STATUS_ACTIVE))
-      ->execute();
+    $providers = $this->loadActiveMFAProviders();
+
     if (!$providers) {
       return $this->newDialog()
         ->setTitle(pht('No MFA Providers'))
         ->appendParagraph(
           pht(
-            'There are no active MFA providers. At least one active provider '.
-            'must be available to add new MFA factors.'))
+            'This install does not have any active MFA providers configured. '.
+            'At least one provider must be configured and active before you '.
+            'can add new MFA factors.'))
         ->addCancelButton($cancel_uri);
     }
-    $providers = mpull($providers, null, 'getPHID');
-    $proivders = msortv($providers, 'newSortVector');
 
     $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession(
       $viewer,
@@ -184,8 +196,7 @@ final class PhabricatorMultiFactorSettingsPanel
 
       // Only let the user continue creating a factor for a given provider if
       // they actually pass the provider's checks.
-      $selected_factor = $selected_provider->getFactor();
-      if (!$selected_factor->canCreateNewConfiguration($viewer)) {
+      if (!$selected_provider->canCreateNewConfiguration($viewer)) {
         $selected_provider = null;
       }
     }
@@ -200,8 +211,7 @@ final class PhabricatorMultiFactorSettingsPanel
         $provider_uri = id(new PhutilURI($this->getPanelURI()))
           ->setQueryParam('providerPHID', $provider_phid);
 
-        $factor = $provider->getFactor();
-        $is_enabled = $factor->canCreateNewConfiguration($viewer);
+        $is_enabled = $provider->canCreateNewConfiguration($viewer);
 
         $item = id(new PHUIObjectItemView())
           ->setHeader($provider->getDisplayName())
@@ -216,7 +226,7 @@ final class PhabricatorMultiFactorSettingsPanel
           $item->setDisabled(true);
         }
 
-        $create_description = $factor->getConfigurationCreateDescription(
+        $create_description = $provider->getConfigurationCreateDescription(
           $viewer);
         if ($create_description) {
           $item->appendChild($create_description);
@@ -424,5 +434,22 @@ final class PhabricatorMultiFactorSettingsPanel
       ->setDialog($dialog);
   }
 
+  private function loadActiveMFAProviders() {
+    $viewer = $this->getViewer();
+
+    $providers = id(new PhabricatorAuthFactorProviderQuery())
+      ->setViewer($viewer)
+      ->withStatuses(
+        array(
+          PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE,
+        ))
+      ->execute();
+
+    $providers = mpull($providers, null, 'getPHID');
+    $providers = msortv($providers, 'newSortVector');
+
+    return $providers;
+  }
+
 
 }