mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-18 11:30:55 +01:00
Allow installs to require multi-factor authentication for all users
Summary: Ref T5089. Adds a `security.require-multi-factor-auth` which forces all users to enroll in MFA before they can use their accounts. Test Plan: Config: {F159750} Roadblock: {F159748} After configuration: {F159749} - Required MFA, got roadblocked, added MFA, got unblocked. - Removed MFA, got blocked again. - Used `bin/auth strip` to strip MFA, got blocked. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T5089 Differential Revision: https://secure.phabricator.com/D9285
This commit is contained in:
parent
83112cc2e8
commit
99c72a32d0
11 changed files with 236 additions and 4 deletions
2
resources/sql/autopatches/20140524.auth.mfa.cache.sql
Normal file
2
resources/sql/autopatches/20140524.auth.mfa.cache.sql
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
ALTER TABLE {$NAMESPACE}_user.user
|
||||||
|
ADD isEnrolledInMultiFactor BOOL NOT NULL DEFAULT 0;
|
|
@ -1247,6 +1247,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorAuthManagementStripWorkflow' => 'applications/auth/management/PhabricatorAuthManagementStripWorkflow.php',
|
'PhabricatorAuthManagementStripWorkflow' => 'applications/auth/management/PhabricatorAuthManagementStripWorkflow.php',
|
||||||
'PhabricatorAuthManagementWorkflow' => 'applications/auth/management/PhabricatorAuthManagementWorkflow.php',
|
'PhabricatorAuthManagementWorkflow' => 'applications/auth/management/PhabricatorAuthManagementWorkflow.php',
|
||||||
'PhabricatorAuthNeedsApprovalController' => 'applications/auth/controller/PhabricatorAuthNeedsApprovalController.php',
|
'PhabricatorAuthNeedsApprovalController' => 'applications/auth/controller/PhabricatorAuthNeedsApprovalController.php',
|
||||||
|
'PhabricatorAuthNeedsMultiFactorController' => 'applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php',
|
||||||
'PhabricatorAuthNewController' => 'applications/auth/controller/config/PhabricatorAuthNewController.php',
|
'PhabricatorAuthNewController' => 'applications/auth/controller/config/PhabricatorAuthNewController.php',
|
||||||
'PhabricatorAuthOldOAuthRedirectController' => 'applications/auth/controller/PhabricatorAuthOldOAuthRedirectController.php',
|
'PhabricatorAuthOldOAuthRedirectController' => 'applications/auth/controller/PhabricatorAuthOldOAuthRedirectController.php',
|
||||||
'PhabricatorAuthOneTimeLoginController' => 'applications/auth/controller/PhabricatorAuthOneTimeLoginController.php',
|
'PhabricatorAuthOneTimeLoginController' => 'applications/auth/controller/PhabricatorAuthOneTimeLoginController.php',
|
||||||
|
@ -4013,6 +4014,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorAuthManagementStripWorkflow' => 'PhabricatorAuthManagementWorkflow',
|
'PhabricatorAuthManagementStripWorkflow' => 'PhabricatorAuthManagementWorkflow',
|
||||||
'PhabricatorAuthManagementWorkflow' => 'PhabricatorManagementWorkflow',
|
'PhabricatorAuthManagementWorkflow' => 'PhabricatorManagementWorkflow',
|
||||||
'PhabricatorAuthNeedsApprovalController' => 'PhabricatorAuthController',
|
'PhabricatorAuthNeedsApprovalController' => 'PhabricatorAuthController',
|
||||||
|
'PhabricatorAuthNeedsMultiFactorController' => 'PhabricatorAuthController',
|
||||||
'PhabricatorAuthNewController' => 'PhabricatorAuthProviderConfigController',
|
'PhabricatorAuthNewController' => 'PhabricatorAuthProviderConfigController',
|
||||||
'PhabricatorAuthOldOAuthRedirectController' => 'PhabricatorAuthController',
|
'PhabricatorAuthOldOAuthRedirectController' => 'PhabricatorAuthController',
|
||||||
'PhabricatorAuthOneTimeLoginController' => 'PhabricatorAuthController',
|
'PhabricatorAuthOneTimeLoginController' => 'PhabricatorAuthController',
|
||||||
|
|
|
@ -101,6 +101,8 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication {
|
||||||
=> 'PhabricatorAuthTerminateSessionController',
|
=> 'PhabricatorAuthTerminateSessionController',
|
||||||
'session/downgrade/'
|
'session/downgrade/'
|
||||||
=> 'PhabricatorAuthDowngradeSessionController',
|
=> 'PhabricatorAuthDowngradeSessionController',
|
||||||
|
'multifactor/'
|
||||||
|
=> 'PhabricatorAuthNeedsMultiFactorController',
|
||||||
),
|
),
|
||||||
|
|
||||||
'/oauth/(?P<provider>\w+)/login/'
|
'/oauth/(?P<provider>\w+)/login/'
|
||||||
|
|
|
@ -0,0 +1,92 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class PhabricatorAuthNeedsMultiFactorController
|
||||||
|
extends PhabricatorAuthController {
|
||||||
|
|
||||||
|
public function shouldRequireMultiFactorEnrollment() {
|
||||||
|
// Users need access to this controller in order to enroll in multi-factor
|
||||||
|
// auth.
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function processRequest() {
|
||||||
|
$request = $this->getRequest();
|
||||||
|
$viewer = $request->getUser();
|
||||||
|
|
||||||
|
$panel = id(new PhabricatorSettingsPanelMultiFactor())
|
||||||
|
->setUser($viewer)
|
||||||
|
->setViewer($viewer)
|
||||||
|
->setOverrideURI($this->getApplicationURI('/multifactor/'))
|
||||||
|
->processRequest($request);
|
||||||
|
|
||||||
|
if ($panel instanceof AphrontResponse) {
|
||||||
|
return $panel;
|
||||||
|
}
|
||||||
|
|
||||||
|
$crumbs = $this->buildApplicationCrumbs();
|
||||||
|
$crumbs->addTextCrumb(pht('Add Multi-Factor Auth'));
|
||||||
|
|
||||||
|
$viewer->updateMultiFactorEnrollment();
|
||||||
|
|
||||||
|
if (!$viewer->getIsEnrolledInMultiFactor()) {
|
||||||
|
$help = id(new AphrontErrorView())
|
||||||
|
->setTitle(pht('Add Multi-Factor Authentication To Your Account'))
|
||||||
|
->setSeverity(AphrontErrorView::SEVERITY_WARNING)
|
||||||
|
->setErrors(
|
||||||
|
array(
|
||||||
|
pht(
|
||||||
|
'Before you can use Phabricator, you need to add multi-factor '.
|
||||||
|
'authentication to your account.'),
|
||||||
|
pht(
|
||||||
|
'Multi-factor authentication helps secure your account by '.
|
||||||
|
'making it more difficult for attackers to gain access or '.
|
||||||
|
'take senstive actions.'),
|
||||||
|
pht(
|
||||||
|
'To learn more about multi-factor authentication, click the '.
|
||||||
|
'%s button below.',
|
||||||
|
phutil_tag('strong', array(), pht('Help'))),
|
||||||
|
pht(
|
||||||
|
'To add an authentication factor, click the %s button below.',
|
||||||
|
phutil_tag('strong', array(), pht('Add Authentication Factor'))),
|
||||||
|
pht(
|
||||||
|
'To continue, add at least one authentication factor to your '.
|
||||||
|
'account.'),
|
||||||
|
));
|
||||||
|
} else {
|
||||||
|
$help = id(new AphrontErrorView())
|
||||||
|
->setTitle(pht('Multi-Factor Authentication Configured'))
|
||||||
|
->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
|
||||||
|
->setErrors(
|
||||||
|
array(
|
||||||
|
pht(
|
||||||
|
'You have successfully configured multi-factor authentication '.
|
||||||
|
'for your account.'),
|
||||||
|
pht(
|
||||||
|
'You can make adjustments from the Settings panel later.'),
|
||||||
|
pht(
|
||||||
|
'When you are ready, %s.',
|
||||||
|
phutil_tag(
|
||||||
|
'strong',
|
||||||
|
array(),
|
||||||
|
phutil_tag(
|
||||||
|
'a',
|
||||||
|
array(
|
||||||
|
'href' => '/',
|
||||||
|
),
|
||||||
|
pht('continue to Phabricator')))),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->buildApplicationPage(
|
||||||
|
array(
|
||||||
|
$crumbs,
|
||||||
|
$help,
|
||||||
|
$panel,
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
'title' => pht('Add Multi-Factor Authentication'),
|
||||||
|
'device' => true,
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -153,7 +153,16 @@ final class PhabricatorAuthManagementStripWorkflow
|
||||||
$console->writeOut("%s\n", pht('Stripping authentication factors...'));
|
$console->writeOut("%s\n", pht('Stripping authentication factors...'));
|
||||||
|
|
||||||
foreach ($factors as $factor) {
|
foreach ($factors as $factor) {
|
||||||
|
$user = id(new PhabricatorPeopleQuery())
|
||||||
|
->setViewer($this->getViewer())
|
||||||
|
->withPHIDs(array($factor->getUserPHID()))
|
||||||
|
->executeOne();
|
||||||
|
|
||||||
$factor->delete();
|
$factor->delete();
|
||||||
|
|
||||||
|
if ($user) {
|
||||||
|
$user->updateMultiFactorEnrollment();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$console->writeOut("%s\n", pht('Done.'));
|
$console->writeOut("%s\n", pht('Done.'));
|
||||||
|
|
|
@ -32,6 +32,27 @@ abstract class PhabricatorController extends AphrontController {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldRequireMultiFactorEnrollment() {
|
||||||
|
if (!$this->shouldRequireLogin()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$this->shouldRequireEnabledUser()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($this->shouldAllowPartialSessions()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
$user = $this->getRequest()->getUser();
|
||||||
|
if (!$user->getIsStandardUser()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return PhabricatorEnv::getEnvConfig('security.require-multi-factor-auth');
|
||||||
|
}
|
||||||
|
|
||||||
public function willBeginExecution() {
|
public function willBeginExecution() {
|
||||||
$request = $this->getRequest();
|
$request = $this->getRequest();
|
||||||
|
|
||||||
|
@ -151,6 +172,21 @@ abstract class PhabricatorController extends AphrontController {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if the user needs to configure MFA.
|
||||||
|
$need_mfa = $this->shouldRequireMultiFactorEnrollment();
|
||||||
|
$have_mfa = $user->getIsEnrolledInMultiFactor();
|
||||||
|
if ($need_mfa && !$have_mfa) {
|
||||||
|
// Check if the cache is just out of date. Otherwise, roadblock the user
|
||||||
|
// and require MFA enrollment.
|
||||||
|
$user->updateMultiFactorEnrollment();
|
||||||
|
if (!$user->getIsEnrolledInMultiFactor()) {
|
||||||
|
$mfa_controller = new PhabricatorAuthNeedsMultiFactorController(
|
||||||
|
$request);
|
||||||
|
$this->setCurrentApplication($auth_application);
|
||||||
|
return $this->delegateToController($mfa_controller);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if ($this->shouldRequireLogin()) {
|
if ($this->shouldRequireLogin()) {
|
||||||
// This actually means we need either:
|
// This actually means we need either:
|
||||||
// - a valid user, or a public controller; and
|
// - a valid user, or a public controller; and
|
||||||
|
|
|
@ -57,6 +57,7 @@ final class PhabricatorAccessControlTestCase
|
||||||
$env->overrideEnvConfig('policy.allow-public', false);
|
$env->overrideEnvConfig('policy.allow-public', false);
|
||||||
$env->overrideEnvConfig('auth.require-email-verification', false);
|
$env->overrideEnvConfig('auth.require-email-verification', false);
|
||||||
$env->overrideEnvConfig('auth.email-domains', array());
|
$env->overrideEnvConfig('auth.email-domains', array());
|
||||||
|
$env->overrideEnvConfig('security.require-multi-factor-auth', false);
|
||||||
|
|
||||||
|
|
||||||
// Test standard defaults.
|
// Test standard defaults.
|
||||||
|
|
|
@ -82,6 +82,22 @@ final class PhabricatorSecurityConfigOptions
|
||||||
pht('Force HTTPS'),
|
pht('Force HTTPS'),
|
||||||
pht('Allow HTTP'),
|
pht('Allow HTTP'),
|
||||||
)),
|
)),
|
||||||
|
$this->newOption('security.require-multi-factor-auth', 'bool', false)
|
||||||
|
->setLocked(true)
|
||||||
|
->setSummary(
|
||||||
|
pht('Require all users to configure multi-factor authentication.'))
|
||||||
|
->setDescription(
|
||||||
|
pht(
|
||||||
|
'By default, Phabricator allows users to add multi-factor '.
|
||||||
|
'authentication to their accounts, but does not require it. '.
|
||||||
|
'By enabling this option, you can force all users to add '.
|
||||||
|
'at least one authentication factor before they can use their '.
|
||||||
|
'accounts.'))
|
||||||
|
->setBoolOptions(
|
||||||
|
array(
|
||||||
|
pht('Multi-Factor Required'),
|
||||||
|
pht('Multi-Factor Optional'),
|
||||||
|
)),
|
||||||
$this->newOption(
|
$this->newOption(
|
||||||
'phabricator.csrf-key',
|
'phabricator.csrf-key',
|
||||||
'string',
|
'string',
|
||||||
|
|
|
@ -1,5 +1,8 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @task factors Multi-Factor Authentication
|
||||||
|
*/
|
||||||
final class PhabricatorUser
|
final class PhabricatorUser
|
||||||
extends PhabricatorUserDAO
|
extends PhabricatorUserDAO
|
||||||
implements
|
implements
|
||||||
|
@ -32,6 +35,7 @@ final class PhabricatorUser
|
||||||
protected $isDisabled = 0;
|
protected $isDisabled = 0;
|
||||||
protected $isEmailVerified = 0;
|
protected $isEmailVerified = 0;
|
||||||
protected $isApproved = 0;
|
protected $isApproved = 0;
|
||||||
|
protected $isEnrolledInMultiFactor = 0;
|
||||||
|
|
||||||
protected $accountSecret;
|
protected $accountSecret;
|
||||||
|
|
||||||
|
@ -689,6 +693,59 @@ EOBODY;
|
||||||
$email->getUserPHID());
|
$email->getUserPHID());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* -( Multi-Factor Authentication )---------------------------------------- */
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Update the flag storing this user's enrollment in multi-factor auth.
|
||||||
|
*
|
||||||
|
* With certain settings, we need to check if a user has MFA on every page,
|
||||||
|
* so we cache MFA enrollment on the user object for performance. Calling this
|
||||||
|
* method synchronizes the cache by examining enrollment records. After
|
||||||
|
* updating the cache, use @{method:getIsEnrolledInMultiFactor} to check if
|
||||||
|
* the user is enrolled.
|
||||||
|
*
|
||||||
|
* This method should be called after any changes are made to a given user's
|
||||||
|
* multi-factor configuration.
|
||||||
|
*
|
||||||
|
* @return void
|
||||||
|
* @task factors
|
||||||
|
*/
|
||||||
|
public function updateMultiFactorEnrollment() {
|
||||||
|
$factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere(
|
||||||
|
'userPHID = %s',
|
||||||
|
$this->getPHID());
|
||||||
|
|
||||||
|
$enrolled = count($factors) ? 1 : 0;
|
||||||
|
if ($enrolled !== $this->isEnrolledInMultiFactor) {
|
||||||
|
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||||
|
queryfx(
|
||||||
|
$this->establishConnection('w'),
|
||||||
|
'UPDATE %T SET isEnrolledInMultiFactor = %d WHERE id = %d',
|
||||||
|
$this->getTableName(),
|
||||||
|
$enrolled,
|
||||||
|
$this->getID());
|
||||||
|
unset($unguarded);
|
||||||
|
|
||||||
|
$this->isEnrolledInMultiFactor = $enrolled;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if the user is enrolled in multi-factor authentication.
|
||||||
|
*
|
||||||
|
* Enrolled users have one or more multi-factor authentication sources
|
||||||
|
* attached to their account. For performance, this value is cached. You
|
||||||
|
* can use @{method:updateMultiFactorEnrollment} to update the cache.
|
||||||
|
*
|
||||||
|
* @return bool True if the user is enrolled.
|
||||||
|
* @task factors
|
||||||
|
*/
|
||||||
|
public function getIsEnrolledInMultiFactor() {
|
||||||
|
return $this->isEnrolledInMultiFactor;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( Omnipotence )-------------------------------------------------------- */
|
/* -( Omnipotence )-------------------------------------------------------- */
|
||||||
|
|
||||||
|
|
|
@ -12,13 +12,13 @@
|
||||||
* @task config Panel Configuration
|
* @task config Panel Configuration
|
||||||
* @task panel Panel Implementation
|
* @task panel Panel Implementation
|
||||||
* @task internal Internals
|
* @task internal Internals
|
||||||
*
|
|
||||||
* @group settings
|
|
||||||
*/
|
*/
|
||||||
abstract class PhabricatorSettingsPanel {
|
abstract class PhabricatorSettingsPanel {
|
||||||
|
|
||||||
private $user;
|
private $user;
|
||||||
private $viewer;
|
private $viewer;
|
||||||
|
private $overrideURI;
|
||||||
|
|
||||||
|
|
||||||
public function setUser(PhabricatorUser $user) {
|
public function setUser(PhabricatorUser $user) {
|
||||||
$this->user = $user;
|
$this->user = $user;
|
||||||
|
@ -38,6 +38,11 @@ abstract class PhabricatorSettingsPanel {
|
||||||
return $this->viewer;
|
return $this->viewer;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setOverrideURI($override_uri) {
|
||||||
|
$this->overrideURI = $override_uri;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( Panel Configuration )------------------------------------------------ */
|
/* -( Panel Configuration )------------------------------------------------ */
|
||||||
|
|
||||||
|
@ -148,11 +153,15 @@ abstract class PhabricatorSettingsPanel {
|
||||||
* @task panel
|
* @task panel
|
||||||
*/
|
*/
|
||||||
final public function getPanelURI($path = '') {
|
final public function getPanelURI($path = '') {
|
||||||
|
$path = ltrim($path, '/');
|
||||||
|
|
||||||
|
if ($this->overrideURI) {
|
||||||
|
return rtrim($this->overrideURI, '/').'/'.$path;
|
||||||
|
}
|
||||||
|
|
||||||
$key = $this->getPanelKey();
|
$key = $this->getPanelKey();
|
||||||
$key = phutil_escape_uri($key);
|
$key = phutil_escape_uri($key);
|
||||||
|
|
||||||
$path = ltrim($path, '/');
|
|
||||||
|
|
||||||
if ($this->getUser()->getPHID() != $this->getViewer()->getPHID()) {
|
if ($this->getUser()->getPHID() != $this->getViewer()->getPHID()) {
|
||||||
$user_id = $this->getUser()->getID();
|
$user_id = $this->getUser()->getID();
|
||||||
return "/settings/{$user_id}/panel/{$key}/{$path}";
|
return "/settings/{$user_id}/panel/{$key}/{$path}";
|
||||||
|
|
|
@ -196,6 +196,8 @@ final class PhabricatorSettingsPanelMultiFactor
|
||||||
PhabricatorUserLog::ACTION_MULTI_ADD);
|
PhabricatorUserLog::ACTION_MULTI_ADD);
|
||||||
$log->save();
|
$log->save();
|
||||||
|
|
||||||
|
$user->updateMultiFactorEnrollment();
|
||||||
|
|
||||||
return id(new AphrontRedirectResponse())
|
return id(new AphrontRedirectResponse())
|
||||||
->setURI($this->getPanelURI('?id='.$config->getID()));
|
->setURI($this->getPanelURI('?id='.$config->getID()));
|
||||||
}
|
}
|
||||||
|
@ -238,6 +240,8 @@ final class PhabricatorSettingsPanelMultiFactor
|
||||||
$factor->setFactorName($name);
|
$factor->setFactorName($name);
|
||||||
$factor->save();
|
$factor->save();
|
||||||
|
|
||||||
|
$user->updateMultiFactorEnrollment();
|
||||||
|
|
||||||
return id(new AphrontRedirectResponse())
|
return id(new AphrontRedirectResponse())
|
||||||
->setURI($this->getPanelURI('?id='.$factor->getID()));
|
->setURI($this->getPanelURI('?id='.$factor->getID()));
|
||||||
}
|
}
|
||||||
|
@ -293,6 +297,8 @@ final class PhabricatorSettingsPanelMultiFactor
|
||||||
PhabricatorUserLog::ACTION_MULTI_REMOVE);
|
PhabricatorUserLog::ACTION_MULTI_REMOVE);
|
||||||
$log->save();
|
$log->save();
|
||||||
|
|
||||||
|
$user->updateMultiFactorEnrollment();
|
||||||
|
|
||||||
return id(new AphrontRedirectResponse())
|
return id(new AphrontRedirectResponse())
|
||||||
->setURI($this->getPanelURI());
|
->setURI($this->getPanelURI());
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue