From c4244aa177b8aecf52f57fce697baa1d5cd0bd73 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Jan 2019 11:04:49 -0800 Subject: [PATCH] Allow users to access some settings at the "Add MFA" account setup roadblock Summary: Depends on D20006. Ref T13222. Currently, the "MFA Is Required" gate doesn't let you do anything else, but you'll need to be able to access "Contact Numbers" if an install provides SMS MFA. Tweak this UI to give users limited access to settings, so they can set up contact numbers and change their language. (This is a little bit fiddly, and I'm doing it early on partly so it can get more testing as these changes move forward.) Test Plan: {F6146136} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D20008 --- .../PhabricatorAuthApplication.php | 6 +- ...bricatorAuthNeedsMultiFactorController.php | 225 +++++++++++++----- ...PhabricatorAuthContactNumberController.php | 15 ++ ...PhabricatorContactNumbersSettingsPanel.php | 4 + .../PhabricatorLanguageSettingsPanel.php | 4 + .../PhabricatorMultiFactorSettingsPanel.php | 25 +- .../panel/PhabricatorSettingsPanel.php | 11 + 7 files changed, 228 insertions(+), 62 deletions(-) diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index 20547d8ca3..4c0003e806 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -72,8 +72,10 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { => 'PhabricatorAuthRevokeTokenController', 'session/downgrade/' => 'PhabricatorAuthDowngradeSessionController', - 'multifactor/' - => 'PhabricatorAuthNeedsMultiFactorController', + 'enroll/' => array( + '(?:(?P[^/]+)/)?(?:(?Psaved)/)?' + => 'PhabricatorAuthNeedsMultiFactorController', + ), 'sshkey/' => array( $this->getQueryRoutePattern('for/(?P[^/]+)/') => 'PhabricatorAuthSSHKeyListController', diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php index 27e03485ca..92328b2000 100644 --- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php +++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php @@ -30,80 +30,187 @@ final class PhabricatorAuthNeedsMultiFactorController return new Aphront400Response(); } - $panel = id(new PhabricatorMultiFactorSettingsPanel()) - ->setUser($viewer) - ->setViewer($viewer) - ->setOverrideURI($this->getApplicationURI('/multifactor/')) - ->processRequest($request); + $panels = $this->loadPanels(); - if ($panel instanceof AphrontResponse) { - return $panel; + $multifactor_key = id(new PhabricatorMultiFactorSettingsPanel()) + ->getPanelKey(); + + $panel_key = $request->getURIData('pageKey'); + if (!strlen($panel_key)) { + $panel_key = $multifactor_key; } - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Add Multi-Factor Auth')); + if (!isset($panels[$panel_key])) { + return new Aphront404Response(); + } + + $nav = $this->newNavigation(); + $nav->selectFilter($panel_key); + + $panel = $panels[$panel_key]; $viewer->updateMultiFactorEnrollment(); - if (!$viewer->getIsEnrolledInMultiFactor()) { - $help = id(new PHUIInfoView()) - ->setTitle(pht('Add Multi-Factor Authentication To Your Account')) - ->setSeverity(PHUIInfoView::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 sensitive 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.'), - )); + if ($panel_key === $multifactor_key) { + $header_text = pht('Add Multi-Factor Auth'); + $help = $this->newGuidance(); + $panel->setIsEnrollment(true); } else { - $help = id(new PHUIInfoView()) - ->setTitle(pht('Multi-Factor Authentication Configured')) - ->setSeverity(PHUIInfoView::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')))), - )); + $header_text = $panel->getPanelName(); + $help = null; } - $view = array( - $help, - $panel, - ); + $response = $panel + ->setController($this) + ->setNavigation($nav) + ->processRequest($request); + + if (($response instanceof AphrontResponse) || + ($response instanceof AphrontResponseProducerInterface)) { + return $response; + } + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Add Multi-Factor Auth')) + ->setBorder(true); + + $header = id(new PHUIHeaderView()) + ->setHeader($header_text); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter( + array( + $help, + $response, + )); return $this->newPage() ->setTitle(pht('Add Multi-Factor Authentication')) ->setCrumbs($crumbs) + ->setNavigation($nav) ->appendChild($view); } + private function loadPanels() { + $viewer = $this->getViewer(); + $preferences = PhabricatorUserPreferences::loadUserPreferences($viewer); + + $panels = PhabricatorSettingsPanel::getAllDisplayPanels(); + $base_uri = $this->newEnrollBaseURI(); + + $result = array(); + foreach ($panels as $key => $panel) { + $panel + ->setPreferences($preferences) + ->setViewer($viewer) + ->setUser($viewer) + ->setOverrideURI(urisprintf('%s%s/', $base_uri, $key)); + + if (!$panel->isEnabled()) { + continue; + } + + if (!$panel->isUserPanel()) { + continue; + } + + if (!$panel->isMultiFactorEnrollmentPanel()) { + continue; + } + + if (!empty($result[$key])) { + throw new Exception(pht( + "Two settings panels share the same panel key ('%s'): %s, %s.", + $key, + get_class($panel), + get_class($result[$key]))); + } + + $result[$key] = $panel; + } + + return $result; + } + + + private function newNavigation() { + $viewer = $this->getViewer(); + + $enroll_uri = $this->newEnrollBaseURI(); + + $nav = id(new AphrontSideNavFilterView()) + ->setBaseURI(new PhutilURI($enroll_uri)); + + $multifactor_key = id(new PhabricatorMultiFactorSettingsPanel()) + ->getPanelKey(); + + $nav->addFilter( + $multifactor_key, + pht('Enroll in MFA'), + null, + 'fa-exclamation-triangle blue'); + + $panels = $this->loadPanels(); + + if ($panels) { + $nav->addLabel(pht('Settings')); + } + + foreach ($panels as $panel_key => $panel) { + if ($panel_key === $multifactor_key) { + continue; + } + + $nav->addFilter( + $panel->getPanelKey(), + $panel->getPanelName(), + null, + $panel->getPanelMenuIcon()); + } + + return $nav; + } + + private function newEnrollBaseURI() { + return $this->getApplicationURI('enroll/'); + } + + private function newGuidance() { + $viewer = $this->getViewer(); + + if ($viewer->getIsEnrolledInMultiFactor()) { + $guidance = pht( + '{icon check, color="green"} **Setup Complete!**'. + "\n\n". + 'You have successfully configured multi-factor authentication '. + 'for your account.'. + "\n\n". + 'You can make adjustments from the [[ /settings/ | Settings ]] panel '. + 'later.'); + + return $this->newDialog() + ->setTitle(pht('Multi-Factor Authentication Setup Complete')) + ->setWidth(AphrontDialogView::WIDTH_FULL) + ->appendChild(new PHUIRemarkupView($viewer, $guidance)) + ->addCancelButton('/', pht('Continue')); + } + + $messages = array(); + + $messages[] = pht( + 'Before you can use Phabricator, you need to add multi-factor '. + 'authentication to your account. Multi-factor authentication helps '. + 'secure your account by making it more difficult for attackers to '. + 'gain access or take sensitive actions.'); + + $view = id(new PHUIInfoView()) + ->setTitle(pht('Add Multi-Factor Authentication To Your Account')) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors($messages); + + return $view; + } + } diff --git a/src/applications/auth/controller/contact/PhabricatorAuthContactNumberController.php b/src/applications/auth/controller/contact/PhabricatorAuthContactNumberController.php index a713f48a3b..3ae923fbbc 100644 --- a/src/applications/auth/controller/contact/PhabricatorAuthContactNumberController.php +++ b/src/applications/auth/controller/contact/PhabricatorAuthContactNumberController.php @@ -3,6 +3,21 @@ abstract class PhabricatorAuthContactNumberController extends PhabricatorAuthController { + // Users may need to access these controllers to enroll in SMS MFA during + // account setup. + + public function shouldRequireMultiFactorEnrollment() { + return false; + } + + public function shouldRequireEnabledUser() { + return false; + } + + public function shouldRequireEmailVerification() { + return false; + } + protected function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); diff --git a/src/applications/settings/panel/PhabricatorContactNumbersSettingsPanel.php b/src/applications/settings/panel/PhabricatorContactNumbersSettingsPanel.php index 7834bff593..3e4ed8880e 100644 --- a/src/applications/settings/panel/PhabricatorContactNumbersSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorContactNumbersSettingsPanel.php @@ -19,6 +19,10 @@ final class PhabricatorContactNumbersSettingsPanel return PhabricatorSettingsAuthenticationPanelGroup::PANELGROUPKEY; } + public function isMultiFactorEnrollmentPanel() { + return true; + } + public function processRequest(AphrontRequest $request) { $user = $this->getUser(); $viewer = $request->getUser(); diff --git a/src/applications/settings/panel/PhabricatorLanguageSettingsPanel.php b/src/applications/settings/panel/PhabricatorLanguageSettingsPanel.php index 65a0be4e79..39bd5deac9 100644 --- a/src/applications/settings/panel/PhabricatorLanguageSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorLanguageSettingsPanel.php @@ -25,4 +25,8 @@ final class PhabricatorLanguageSettingsPanel return true; } + public function isMultiFactorEnrollmentPanel() { + return true; + } + } diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index 93a95254a9..d2e9c34247 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -3,6 +3,8 @@ final class PhabricatorMultiFactorSettingsPanel extends PhabricatorSettingsPanel { + private $isEnrollment; + public function getPanelKey() { return 'multifactor'; } @@ -19,6 +21,19 @@ final class PhabricatorMultiFactorSettingsPanel return PhabricatorSettingsAuthenticationPanelGroup::PANELGROUPKEY; } + public function isMultiFactorEnrollmentPanel() { + return true; + } + + public function setIsEnrollment($is_enrollment) { + $this->isEnrollment = $is_enrollment; + return $this; + } + + public function getIsEnrollment() { + return $this->isEnrollment; + } + public function processRequest(AphrontRequest $request) { if ($request->getExists('new') || $request->getExists('providerPHID')) { return $this->processNew($request); @@ -106,13 +121,21 @@ final class PhabricatorMultiFactorSettingsPanel $buttons = array(); + // If we're enrolling a new account in MFA, provide a small visual hint + // that this is the button they want to click. + if ($this->getIsEnrollment()) { + $add_color = PHUIButtonView::BLUE; + } else { + $add_color = PHUIButtonView::GREY; + } + $buttons[] = id(new PHUIButtonView()) ->setTag('a') ->setIcon('fa-plus') ->setText(pht('Add Auth Factor')) ->setHref($this->getPanelURI('?new=true')) ->setWorkflow(true) - ->setColor(PHUIButtonView::GREY); + ->setColor($add_color); $buttons[] = id(new PHUIButtonView()) ->setTag('a') diff --git a/src/applications/settings/panel/PhabricatorSettingsPanel.php b/src/applications/settings/panel/PhabricatorSettingsPanel.php index 8250418812..e2efd92093 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanel.php @@ -198,6 +198,17 @@ abstract class PhabricatorSettingsPanel extends Phobject { return false; } + /** + * Return true if this panel should be available when enrolling in MFA on + * a new account with MFA requiredd. + * + * @return bool True to allow configuration during MFA enrollment. + * @task config + */ + public function isMultiFactorEnrollmentPanel() { + return false; + } + /* -( Panel Implementation )----------------------------------------------- */