Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13993179
D9285.id22049.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
15 KB
Referenced Files
None
Subscribers
None
D9285.id22049.diff
View Options
diff --git a/resources/sql/autopatches/20140524.auth.mfa.cache.sql b/resources/sql/autopatches/20140524.auth.mfa.cache.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140524.auth.mfa.cache.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_user.user
+ ADD isEnrolledInMultiFactor BOOL NOT NULL DEFAULT 0;
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1247,6 +1247,7 @@
'PhabricatorAuthManagementStripWorkflow' => 'applications/auth/management/PhabricatorAuthManagementStripWorkflow.php',
'PhabricatorAuthManagementWorkflow' => 'applications/auth/management/PhabricatorAuthManagementWorkflow.php',
'PhabricatorAuthNeedsApprovalController' => 'applications/auth/controller/PhabricatorAuthNeedsApprovalController.php',
+ 'PhabricatorAuthNeedsMultiFactorController' => 'applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php',
'PhabricatorAuthNewController' => 'applications/auth/controller/config/PhabricatorAuthNewController.php',
'PhabricatorAuthOldOAuthRedirectController' => 'applications/auth/controller/PhabricatorAuthOldOAuthRedirectController.php',
'PhabricatorAuthOneTimeLoginController' => 'applications/auth/controller/PhabricatorAuthOneTimeLoginController.php',
@@ -4014,6 +4015,7 @@
'PhabricatorAuthManagementStripWorkflow' => 'PhabricatorAuthManagementWorkflow',
'PhabricatorAuthManagementWorkflow' => 'PhabricatorManagementWorkflow',
'PhabricatorAuthNeedsApprovalController' => 'PhabricatorAuthController',
+ 'PhabricatorAuthNeedsMultiFactorController' => 'PhabricatorAuthController',
'PhabricatorAuthNewController' => 'PhabricatorAuthProviderConfigController',
'PhabricatorAuthOldOAuthRedirectController' => 'PhabricatorAuthController',
'PhabricatorAuthOneTimeLoginController' => 'PhabricatorAuthController',
diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php
--- a/src/applications/auth/application/PhabricatorApplicationAuth.php
+++ b/src/applications/auth/application/PhabricatorApplicationAuth.php
@@ -93,6 +93,8 @@
=> 'PhabricatorAuthTerminateSessionController',
'session/downgrade/'
=> 'PhabricatorAuthDowngradeSessionController',
+ 'multifactor/(?P<key>[^/]+)?'
+ => 'PhabricatorAuthNeedsMultiFactorController',
),
'/oauth/(?P<provider>\w+)/login/'
diff --git a/src/applications/auth/controller/PhabricatorAuthFinishController.php b/src/applications/auth/controller/PhabricatorAuthFinishController.php
--- a/src/applications/auth/controller/PhabricatorAuthFinishController.php
+++ b/src/applications/auth/controller/PhabricatorAuthFinishController.php
@@ -63,6 +63,7 @@
// Upgrade the partial session to a full session.
$engine->upgradePartialSession($viewer);
+
// TODO: It might be nice to add options like "bind this session to my IP"
// here, even for accounts without multi-factor auth attached to them.
diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
@@ -0,0 +1,98 @@
+<?php
+
+final class PhabricatorAuthNeedsMultiFactorController
+ extends PhabricatorAuthController {
+
+ private $key;
+
+ public function shouldRequireMultiFactorEnrollment() {
+ // Users need access to this controller in order to enroll in multi-factor
+ // auth.
+ return false;
+ }
+
+ public function willProcessRequest(array $data) {
+ $this->key = idx($data, 'key');
+ }
+
+ 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,
+ ));
+ }
+
+}
diff --git a/src/applications/auth/management/PhabricatorAuthManagementStripWorkflow.php b/src/applications/auth/management/PhabricatorAuthManagementStripWorkflow.php
--- a/src/applications/auth/management/PhabricatorAuthManagementStripWorkflow.php
+++ b/src/applications/auth/management/PhabricatorAuthManagementStripWorkflow.php
@@ -153,7 +153,16 @@
$console->writeOut("%s\n", pht('Stripping authentication factors...'));
foreach ($factors as $factor) {
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($factor->getUserPHID()))
+ ->executeOne();
+
$factor->delete();
+
+ if ($user) {
+ $user->updateMultiFactorEnrollment();
+ }
}
$console->writeOut("%s\n", pht('Done.'));
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -32,6 +32,27 @@
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() {
$request = $this->getRequest();
@@ -151,6 +172,21 @@
}
}
+ // 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()) {
// This actually means we need either:
// - a valid user, or a public controller; and
diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
--- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
+++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
@@ -57,6 +57,7 @@
$env->overrideEnvConfig('policy.allow-public', false);
$env->overrideEnvConfig('auth.require-email-verification', false);
$env->overrideEnvConfig('auth.email-domains', array());
+ $env->overrideEnvConfig('security.require-multi-factor-auth', false);
// Test standard defaults.
diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@@ -82,6 +82,22 @@
pht('Force HTTPS'),
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(
'phabricator.csrf-key',
'string',
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -1,5 +1,8 @@
<?php
+/**
+ * @task factors Multi-Factor Authentication
+ */
final class PhabricatorUser
extends PhabricatorUserDAO
implements
@@ -44,6 +47,7 @@
private $alternateCSRFString = self::ATTACHABLE;
private $session = self::ATTACHABLE;
+ private $isEnrolledInMultiFactor;
protected function readField($field) {
switch ($field) {
@@ -689,6 +693,59 @@
$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 )-------------------------------------------------------- */
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanel.php b/src/applications/settings/panel/PhabricatorSettingsPanel.php
--- a/src/applications/settings/panel/PhabricatorSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanel.php
@@ -12,13 +12,13 @@
* @task config Panel Configuration
* @task panel Panel Implementation
* @task internal Internals
- *
- * @group settings
*/
abstract class PhabricatorSettingsPanel {
private $user;
private $viewer;
+ private $overrideURI;
+
public function setUser(PhabricatorUser $user) {
$this->user = $user;
@@ -38,6 +38,11 @@
return $this->viewer;
}
+ public function setOverrideURI($override_uri) {
+ $this->overrideURI = $override_uri;
+ return $this;
+ }
+
/* -( Panel Configuration )------------------------------------------------ */
@@ -148,11 +153,15 @@
* @task panel
*/
final public function getPanelURI($path = '') {
+ $path = ltrim($path, '/');
+
+ if ($this->overrideURI) {
+ return rtrim($this->overrideURI, '/').'/'.$path;
+ }
+
$key = $this->getPanelKey();
$key = phutil_escape_uri($key);
- $path = ltrim($path, '/');
-
if ($this->getUser()->getPHID() != $this->getViewer()->getPHID()) {
$user_id = $this->getUser()->getID();
return "/settings/{$user_id}/panel/{$key}/{$path}";
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php b/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php
--- a/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanelMultiFactor.php
@@ -196,6 +196,8 @@
PhabricatorUserLog::ACTION_MULTI_ADD);
$log->save();
+ $user->updateMultiFactorEnrollment();
+
return id(new AphrontRedirectResponse())
->setURI($this->getPanelURI('?id='.$config->getID()));
}
@@ -238,6 +240,8 @@
$factor->setFactorName($name);
$factor->save();
+ $user->updateMultiFactorEnrollment();
+
return id(new AphrontRedirectResponse())
->setURI($this->getPanelURI('?id='.$factor->getID()));
}
@@ -293,6 +297,8 @@
PhabricatorUserLog::ACTION_MULTI_REMOVE);
$log->save();
+ $user->updateMultiFactorEnrollment();
+
return id(new AphrontRedirectResponse())
->setURI($this->getPanelURI());
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Oct 23, 9:12 PM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6745137
Default Alt Text
D9285.id22049.diff (15 KB)
Attached To
Mode
D9285: Allow installs to require multi-factor authentication for all users
Attached
Detach File
Event Timeline
Log In to Comment