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', @@ -4013,6 +4014,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 @@ -101,6 +101,8 @@ => 'PhabricatorAuthTerminateSessionController', 'session/downgrade/' => 'PhabricatorAuthDowngradeSessionController', + 'multifactor/' + => 'PhabricatorAuthNeedsMultiFactorController', ), '/oauth/(?P\w+)/login/' 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,92 @@ +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 @@ 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()); }