1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01:00

Convert user MFA factors to point at configurable "MFA Providers", not raw "MFA Factors"

Summary:
Ref T13222. Users configure "Factor Configs", which say "I have an entry on my phone for TOTP secret key XYZ".

Currently, these point at raw implementations -- always "TOTP" in practice.

To support configuring available MFA types (like "no MFA") and adding MFA types that need some options set (like "Duo", which needs API keys), bind "Factor Configs" to a "Factor Provider" instead.

In the future, several "Factors" will be available (TOTP, SMS, Duo, Postal Mail, ...). Administrators configure zero or more "MFA Providers" they want to use (e.g., "Duo" + here's my API key). Then users can add configs for these providers (e.g., "here's my Duo account").

Upshot:

  - Factor: a PHP subclass, implements the technical details of a type of MFA factor (TOTP, SMS, Duo, etc).
  - FactorProvider: a storage object, owned by administrators, configuration of a Factor that says "this should be available on this install", plus provides API keys, a human-readable name, etc.
  - FactorConfig: a storage object, owned by a user, says "I have a factor for provider X on my phone/whatever with secret key Q / my duo account is X / my address is Y".

Couple of things not covered here:

  - Statuses for providers ("Disabled", "Deprecated") don't do anything yet, but you can't edit them anyway.
  - Some `bin/auth` tools need to be updated.
  - When no providers are configured, the MFA panel should probably vanish.
  - Documentation.

Test Plan:
  - Ran migration with providers, saw configs point at the first provider.
  - Ran migration without providers, saw a provider created and configs pointed at it.
  - Added/removed factors and providers. Passed MFA gates. Spot-checked database for general sanity.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D19975
This commit is contained in:
epriestley 2019-01-15 07:57:32 -08:00
parent c756bf3476
commit 0fcff78253
12 changed files with 364 additions and 111 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_auth.auth_factorconfig
ADD factorProviderPHID VARBINARY(64) NOT NULL;

View file

@ -0,0 +1,72 @@
<?php
// Previously, MFA factors for individual users were bound to raw factor types.
// The only factor type ever implemented in the upstream was "totp".
// Going forward, individual factors are bound to a provider instead. This
// allows factor types to have some configuration, like API keys for
// service-based MFA. It also allows installs to select which types of factors
// they want users to be able to set up.
// Migrate all existing TOTP factors to the first available TOTP provider,
// creating one if none exists. This migration is a little bit messy, but
// gives us a clean slate going forward with no "builtin" providers.
$table = new PhabricatorAuthFactorConfig();
$conn = $table->establishConnection('w');
$provider_table = new PhabricatorAuthFactorProvider();
$provider_phid = null;
$iterator = new LiskRawMigrationIterator($conn, $table->getTableName());
$totp_key = 'totp';
foreach ($iterator as $row) {
// This wasn't a TOTP factor, so skip it.
if ($row['factorKey'] !== $totp_key) {
continue;
}
// This factor already has an associated provider.
if (strlen($row['factorProviderPHID'])) {
continue;
}
// Find (or create) a suitable TOTP provider. Note that we can't "save()"
// an object or this migration will break if the object ever gets new
// columns; just INSERT the raw fields instead.
if ($provider_phid === null) {
$provider_row = queryfx_one(
$conn,
'SELECT phid FROM %R WHERE providerFactorKey = %s LIMIT 1',
$provider_table,
$totp_key);
if ($provider_row) {
$provider_phid = $provider_row['phid'];
} else {
$provider_phid = $provider_table->generatePHID();
queryfx(
$conn,
'INSERT INTO %R
(phid, providerFactorKey, name, status, properties,
dateCreated, dateModified)
VALUES (%s, %s, %s, %s, %s, %d, %d)',
$provider_table,
$provider_phid,
$totp_key,
'',
'active',
'{}',
PhabricatorTime::getNow(),
PhabricatorTime::getNow());
}
}
queryfx(
$conn,
'UPDATE %R SET factorProviderPHID = %s WHERE id = %d',
$table,
$provider_phid,
$row['id']);
}

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_auth.auth_factorconfig
DROP factorKey;

View file

@ -2207,6 +2207,7 @@ phutil_register_library_map(array(
'PhabricatorAuthEditController' => 'applications/auth/controller/config/PhabricatorAuthEditController.php',
'PhabricatorAuthFactor' => 'applications/auth/factor/PhabricatorAuthFactor.php',
'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php',
'PhabricatorAuthFactorConfigQuery' => 'applications/auth/query/PhabricatorAuthFactorConfigQuery.php',
'PhabricatorAuthFactorProvider' => 'applications/auth/storage/PhabricatorAuthFactorProvider.php',
'PhabricatorAuthFactorProviderController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderController.php',
'PhabricatorAuthFactorProviderEditController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php',
@ -7888,7 +7889,11 @@ phutil_register_library_map(array(
'PhabricatorAuthDowngradeSessionController' => 'PhabricatorAuthController',
'PhabricatorAuthEditController' => 'PhabricatorAuthProviderConfigController',
'PhabricatorAuthFactor' => 'Phobject',
'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO',
'PhabricatorAuthFactorConfig' => array(
'PhabricatorAuthDAO',
'PhabricatorPolicyInterface',
),
'PhabricatorAuthFactorConfigQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorAuthFactorProvider' => array(
'PhabricatorAuthDAO',
'PhabricatorApplicationTransactionInterface',

View file

@ -462,10 +462,14 @@ final class PhabricatorAuthSessionEngine extends Phobject {
return $token;
}
// Load the multi-factor auth sources attached to this account.
$factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere(
'userPHID = %s',
$viewer->getPHID());
// Load the multi-factor auth sources attached to this account. Note that
// we order factors from oldest to newest, which is not the default query
// ordering but makes the greatest sense in context.
$factors = id(new PhabricatorAuthFactorConfigQuery())
->setViewer($viewer)
->withUserPHIDs(array($viewer->getPHID()))
->setOrderVector(array('-id'))
->execute();
// If the account has no associated multi-factor auth, just issue a token
// without putting the session into high security mode. This is generally
@ -516,7 +520,8 @@ final class PhabricatorAuthSessionEngine extends Phobject {
foreach ($factors as $factor) {
$factor_phid = $factor->getPHID();
$issued_challenges = idx($challenge_map, $factor_phid, array());
$impl = $factor->requireImplementation();
$provider = $factor->getFactorProvider();
$impl = $provider->getFactor();
$new_challenges = $impl->getNewIssuedChallenges(
$factor,
@ -552,7 +557,9 @@ final class PhabricatorAuthSessionEngine extends Phobject {
// Limit factor verification rates to prevent brute force attacks.
$any_attempt = false;
foreach ($factors as $factor) {
$impl = $factor->requireImplementation();
$provider = $factor->getFactorProvider();
$impl = $provider->getFactor();
if ($impl->getRequestHasChallengeResponse($factor, $request)) {
$any_attempt = true;
break;
@ -577,7 +584,8 @@ final class PhabricatorAuthSessionEngine extends Phobject {
$issued_challenges = idx($challenge_map, $factor_phid, array());
$impl = $factor->requireImplementation();
$provider = $factor->getFactorProvider();
$impl = $provider->getFactor();
$validation_result = $impl->getResultFromChallengeResponse(
$factor,
@ -716,7 +724,10 @@ final class PhabricatorAuthSessionEngine extends Phobject {
foreach ($factors as $factor) {
$result = $validation_results[$factor->getPHID()];
$factor->requireImplementation()->renderValidateFactorForm(
$provider = $factor->getFactorProvider();
$impl = $provider->getFactor();
$impl->renderValidateFactorForm(
$factor,
$form,
$viewer,

View file

@ -7,6 +7,7 @@ abstract class PhabricatorAuthFactor extends Phobject {
abstract public function getFactorCreateHelp();
abstract public function getFactorDescription();
abstract public function processAddFactorForm(
PhabricatorAuthFactorProvider $provider,
AphrontFormView $form,
AphrontRequest $request,
PhabricatorUser $user);
@ -32,8 +33,7 @@ abstract class PhabricatorAuthFactor extends Phobject {
protected function newConfigForUser(PhabricatorUser $user) {
return id(new PhabricatorAuthFactorConfig())
->setUserPHID($user->getPHID())
->setFactorKey($this->getFactorKey());
->setUserPHID($user->getPHID());
}
protected function newResult() {

View file

@ -26,6 +26,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor {
}
public function processAddFactorForm(
PhabricatorAuthFactorProvider $provider,
AphrontFormView $form,
AphrontRequest $request,
PhabricatorUser $user) {

View file

@ -0,0 +1,88 @@
<?php
final class PhabricatorAuthFactorConfigQuery
extends PhabricatorCursorPagedPolicyAwareQuery {
private $ids;
private $phids;
private $userPHIDs;
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
}
public function withPHIDs(array $phids) {
$this->phids = $phids;
return $this;
}
public function withUserPHIDs(array $user_phids) {
$this->userPHIDs = $user_phids;
return $this;
}
public function newResultObject() {
return new PhabricatorAuthFactorConfig();
}
protected function loadPage() {
return $this->loadStandardPage($this->newResultObject());
}
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$where = parent::buildWhereClauseParts($conn);
if ($this->ids !== null) {
$where[] = qsprintf(
$conn,
'id IN (%Ld)',
$this->ids);
}
if ($this->phids !== null) {
$where[] = qsprintf(
$conn,
'phid IN (%Ls)',
$this->phids);
}
if ($this->userPHIDs !== null) {
$where[] = qsprintf(
$conn,
'userPHID IN (%Ls)',
$this->userPHIDs);
}
return $where;
}
protected function willFilterPage(array $configs) {
$provider_phids = mpull($configs, 'getFactorProviderPHID');
$providers = id(new PhabricatorAuthFactorProviderQuery())
->setViewer($this->getViewer())
->withPHIDs($provider_phids)
->execute();
$providers = mpull($providers, null, 'getPHID');
foreach ($configs as $key => $config) {
$provider = idx($providers, $config->getFactorProviderPHID());
if (!$provider) {
unset($configs[$key]);
$this->didRejectResult($config);
continue;
}
$config->attachFactorProvider($provider);
}
return $configs;
}
public function getQueryApplicationClass() {
return 'PhabricatorAuthApplication';
}
}

View file

@ -5,6 +5,8 @@ final class PhabricatorAuthFactorProviderQuery
private $ids;
private $phids;
private $statuses;
private $providerFactorKeys;
public function withIDs(array $ids) {
$this->ids = $ids;
@ -15,6 +17,17 @@ final class PhabricatorAuthFactorProviderQuery
$this->phids = $phids;
return $this;
}
public function withProviderFactorKeys(array $keys) {
$this->providerFactorKeys = $keys;
return $this;
}
public function withStatuses(array $statuses) {
$this->statuses = $statuses;
return $this;
}
public function newResultObject() {
return new PhabricatorAuthFactorProvider();
}
@ -40,6 +53,20 @@ final class PhabricatorAuthFactorProviderQuery
$this->phids);
}
if ($this->statuses !== null) {
$where[] = qsprintf(
$conn,
'status IN (%Ls)',
$this->statuses);
}
if ($this->providerFactorKeys !== null) {
$where[] = qsprintf(
$conn,
'providerFactorKey IN (%Ls)',
$this->providerFactorKeys);
}
return $where;
}

View file

@ -1,14 +1,17 @@
<?php
final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO {
final class PhabricatorAuthFactorConfig
extends PhabricatorAuthDAO
implements PhabricatorPolicyInterface {
protected $userPHID;
protected $factorKey;
protected $factorProviderPHID;
protected $factorName;
protected $factorSecret;
protected $properties = array();
private $sessionEngine;
private $factorProvider = self::ATTACHABLE;
protected function getConfiguration() {
return array(
@ -17,7 +20,6 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO {
),
self::CONFIG_AUX_PHID => true,
self::CONFIG_COLUMN_SCHEMA => array(
'factorKey' => 'text64',
'factorName' => 'text',
'factorSecret' => 'text',
),
@ -29,26 +31,18 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO {
) + parent::getConfiguration();
}
public function generatePHID() {
return PhabricatorPHID::generateNewPHID(
PhabricatorAuthAuthFactorPHIDType::TYPECONST);
public function getPHIDType() {
return PhabricatorAuthAuthFactorPHIDType::TYPECONST;
}
public function getImplementation() {
return idx(PhabricatorAuthFactor::getAllFactors(), $this->getFactorKey());
public function attachFactorProvider(
PhabricatorAuthFactorProvider $provider) {
$this->factorProvider = $provider;
return $this;
}
public function requireImplementation() {
$impl = $this->getImplementation();
if (!$impl) {
throw new Exception(
pht(
'Attempting to operate on multi-factor auth which has no '.
'corresponding implementation (factor key is "%s").',
$this->getFactorKey()));
}
return $impl;
public function getFactorProvider() {
return $this->assertAttached($this->factorProvider);
}
public function setSessionEngine(PhabricatorAuthSessionEngine $engine) {
@ -64,4 +58,23 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO {
return $this->sessionEngine;
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
);
}
public function getPolicy($capability) {
return $this->getUserPHID();
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
return false;
}
}

View file

@ -78,6 +78,29 @@ final class PhabricatorAuthFactorProvider
return $this->getFactor()->getFactorName();
}
public function newIconView() {
return $this->getFactor()->newIconView();
}
public function getDisplayDescription() {
return $this->getFactor()->getFactorDescription();
}
public function processAddFactorForm(
AphrontFormView $form,
AphrontRequest $request,
PhabricatorUser $user) {
$factor = $this->getFactor();
$config = $factor->processAddFactorForm($this, $form, $request, $user);
if ($config) {
$config->setFactorProviderPHID($this->getPHID());
}
return $config;
}
/* -( PhabricatorApplicationTransactionInterface )------------------------- */

View file

@ -16,7 +16,7 @@ final class PhabricatorMultiFactorSettingsPanel
}
public function processRequest(AphrontRequest $request) {
if ($request->getExists('new')) {
if ($request->getExists('new') || $request->getExists('providerPHID')) {
return $this->processNew($request);
}
@ -31,22 +31,18 @@ final class PhabricatorMultiFactorSettingsPanel
$user = $this->getUser();
$viewer = $request->getUser();
$factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere(
'userPHID = %s',
$user->getPHID());
$factors = id(new PhabricatorAuthFactorConfigQuery())
->setViewer($viewer)
->withUserPHIDs(array($user->getPHID()))
->setOrderVector(array('-id'))
->execute();
$rows = array();
$rowc = array();
$highlight_id = $request->getInt('id');
foreach ($factors as $factor) {
$impl = $factor->getImplementation();
if ($impl) {
$type = $impl->getFactorName();
} else {
$type = $factor->getFactorKey();
}
$provider = $factor->getFactorProvider();
if ($factor->getID() == $highlight_id) {
$rowc[] = 'highlighted';
@ -62,7 +58,7 @@ final class PhabricatorMultiFactorSettingsPanel
'sigil' => 'workflow',
),
$factor->getFactorName()),
$type,
$provider->getDisplayName(),
phabricator_datetime($factor->getDateCreated(), $viewer),
javelin_tag(
'a',
@ -128,88 +124,101 @@ final class PhabricatorMultiFactorSettingsPanel
$viewer = $request->getUser();
$user = $this->getUser();
$cancel_uri = $this->getPanelURI();
// 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();
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.'))
->addCancelButton($cancel_uri);
}
$providers = mpull($providers, null, 'getPHID');
$token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession(
$viewer,
$request,
$this->getPanelURI());
$cancel_uri);
$factors = PhabricatorAuthFactor::getAllFactors();
$selected_phid = $request->getStr('providerPHID');
if (empty($providers[$selected_phid])) {
$selected_provider = null;
} else {
$selected_provider = $providers[$selected_phid];
}
if (!$selected_provider) {
$menu = id(new PHUIObjectItemListView())
->setViewer($viewer)
->setBig(true)
->setFlush(true);
foreach ($providers as $provider_phid => $provider) {
$provider_uri = id(new PhutilURI($this->getPanelURI()))
->setQueryParam('providerPHID', $provider_phid);
$item = id(new PHUIObjectItemView())
->setHeader($provider->getDisplayName())
->setHref($provider_uri)
->setClickable(true)
->setImageIcon($provider->newIconView())
->addAttribute($provider->getDisplayDescription());
$menu->addItem($item);
}
return $this->newDialog()
->setTitle(pht('Choose Factor Type'))
->appendChild($menu)
->addCancelButton($cancel_uri);
}
$form = id(new AphrontFormView())
->setUser($viewer);
->setViewer($viewer);
$type = $request->getStr('type');
if (empty($factors[$type]) || !$request->isFormPost()) {
$factor = null;
} else {
$factor = $factors[$type];
$config = $selected_provider->processAddFactorForm(
$form,
$request,
$user);
if ($config) {
$config->save();
$log = PhabricatorUserLog::initializeNewLog(
$viewer,
$user->getPHID(),
PhabricatorUserLog::ACTION_MULTI_ADD);
$log->save();
$user->updateMultiFactorEnrollment();
// Terminate other sessions so they must log in and survive the
// multi-factor auth check.
id(new PhabricatorAuthSessionEngine())->terminateLoginSessions(
$user,
new PhutilOpaqueEnvelope(
$request->getCookie(PhabricatorCookies::COOKIE_SESSION)));
return id(new AphrontRedirectResponse())
->setURI($this->getPanelURI('?id='.$config->getID()));
}
$dialog = id(new AphrontDialogView())
->setUser($viewer)
->addHiddenInput('new', true);
if ($factor === null) {
$choice_control = id(new AphrontFormRadioButtonControl())
->setName('type')
->setValue(key($factors));
foreach ($factors as $available_factor) {
$choice_control->addButton(
$available_factor->getFactorKey(),
$available_factor->getFactorName(),
$available_factor->getFactorDescription());
}
$dialog->appendParagraph(
pht(
'Adding an additional authentication factor improves the security '.
'of your account. Choose the type of factor to add:'));
$form
->appendChild($choice_control);
} else {
$dialog->addHiddenInput('type', $type);
$config = $factor->processAddFactorForm(
$form,
$request,
$user);
if ($config) {
$config->save();
$log = PhabricatorUserLog::initializeNewLog(
$viewer,
$user->getPHID(),
PhabricatorUserLog::ACTION_MULTI_ADD);
$log->save();
$user->updateMultiFactorEnrollment();
// Terminate other sessions so they must log in and survive the
// multi-factor auth check.
id(new PhabricatorAuthSessionEngine())->terminateLoginSessions(
$user,
new PhutilOpaqueEnvelope(
$request->getCookie(PhabricatorCookies::COOKIE_SESSION)));
return id(new AphrontRedirectResponse())
->setURI($this->getPanelURI('?id='.$config->getID()));
}
}
$dialog
return $this->newDialog()
->addHiddenInput('providerPHID', $selected_provider->getPHID())
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle(pht('Add Authentication Factor'))
->appendChild($form->buildLayoutView())
->addSubmitButton(pht('Continue'))
->addCancelButton($this->getPanelURI());
return id(new AphrontDialogResponse())
->setDialog($dialog);
->addCancelButton($cancel_uri);
}
private function processEdit(AphrontRequest $request) {