From bb20c136513c7cf16ed82abeed931e8483a206a6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 23 Jan 2019 06:49:55 -0800 Subject: [PATCH] Allow MFA factors to provide more guidance text on create workflows Summary: Depends on D20016. Ref T920. This does nothing interesting on its own since the TOTP provider has no guidance/warnings, but landing it separately helps to simplify an upcoming SMS diff. SMS will have these guidance messages: - "Administrator: you haven't configured any mailer which can send SMS, like Twilio." - "Administrator: SMS is weak." - "User: you haven't configured a contact number." Test Plan: {F6151283} {F6151284} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T920 Differential Revision: https://secure.phabricator.com/D20017 --- resources/celerity/map.php | 12 ++++----- ...icatorAuthFactorProviderEditController.php | 19 +++++++++++-- .../auth/factor/PhabricatorAuthFactor.php | 27 +++++++++++++++++++ .../storage/PhabricatorAuthFactorProvider.php | 8 ++++++ .../PhabricatorMultiFactorSettingsPanel.php | 27 +++++++++++++++++-- .../css/phui/object-item/phui-oi-big-ui.css | 5 ++++ 6 files changed, 88 insertions(+), 10 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 5764623ad5..651e95bf15 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'e94cc920', + 'core.pkg.css' => 'a66ea2e7', 'core.pkg.js' => '5c737607', 'differential.pkg.css' => 'b8df73d4', 'differential.pkg.js' => '67c9ea4c', @@ -127,7 +127,7 @@ return array( 'rsrc/css/phui/calendar/phui-calendar-list.css' => 'ccd7e4e2', 'rsrc/css/phui/calendar/phui-calendar-month.css' => 'cb758c42', 'rsrc/css/phui/calendar/phui-calendar.css' => 'f11073aa', - 'rsrc/css/phui/object-item/phui-oi-big-ui.css' => 'e5b1fb04', + 'rsrc/css/phui/object-item/phui-oi-big-ui.css' => '9e037c7a', 'rsrc/css/phui/object-item/phui-oi-color.css' => 'b517bfa0', 'rsrc/css/phui/object-item/phui-oi-drag-ui.css' => 'da15d3dc', 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e', @@ -832,7 +832,7 @@ return array( 'phui-lightbox-css' => '4ebf22da', 'phui-list-view-css' => '470b1adb', 'phui-object-box-css' => '9b58483d', - 'phui-oi-big-ui-css' => 'e5b1fb04', + 'phui-oi-big-ui-css' => '9e037c7a', 'phui-oi-color-css' => 'b517bfa0', 'phui-oi-drag-ui-css' => 'da15d3dc', 'phui-oi-flush-ui-css' => '490e2e2e', @@ -1710,6 +1710,9 @@ return array( 'javelin-uri', 'phabricator-textareautils', ), + '9e037c7a' => array( + 'phui-oi-list-view-css', + ), '9f081f05' => array( 'javelin-behavior', 'javelin-dom', @@ -2024,9 +2027,6 @@ return array( 'e562708c' => array( 'javelin-install', ), - 'e5b1fb04' => array( - 'phui-oi-list-view-css', - ), 'e5bdb730' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php index 0dde1b3c6f..a8d87e2ead 100644 --- a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php +++ b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php @@ -41,18 +41,33 @@ final class PhabricatorAuthFactorProviderEditController ->setBig(true) ->setFlush(true); + $factors = msortv($factors, 'newSortVector'); + foreach ($factors as $factor_key => $factor) { $factor_uri = id(new PhutilURI('/mfa/edit/')) ->setQueryParam('providerFactorKey', $factor_key); $factor_uri = $this->getApplicationURI($factor_uri); + $is_enabled = $factor->canCreateNewProvider(); + $item = id(new PHUIObjectItemView()) ->setHeader($factor->getFactorName()) - ->setHref($factor_uri) - ->setClickable(true) ->setImageIcon($factor->newIconView()) ->addAttribute($factor->getFactorCreateHelp()); + if ($is_enabled) { + $item + ->setHref($factor_uri) + ->setClickable(true); + } else { + $item->setDisabled(true); + } + + $create_description = $factor->getProviderCreateDescription(); + if ($create_description) { + $item->appendChild($create_description); + } + $menu->addItem($item); } diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php index f797ce3a15..c7fc9891af 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -45,6 +45,33 @@ abstract class PhabricatorAuthFactor extends Phobject { ->setIcon('fa-mobile'); } + public function canCreateNewProvider() { + return true; + } + + public function getProviderCreateDescription() { + return null; + } + + public function canCreateNewConfiguration(PhabricatorUser $user) { + return true; + } + + public function getConfigurationCreateDescription(PhabricatorUser $user) { + return null; + } + + public function getFactorOrder() { + return 1000; + } + + final public function newSortVector() { + return id(new PhutilSortVector()) + ->addInt($this->canCreateNewProvider() ? 0 : 1) + ->addInt($this->getFactorOrder()) + ->addString($this->getFactorName()); + } + protected function newChallenge( PhabricatorAuthFactorConfig $config, PhabricatorUser $viewer) { diff --git a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php index e61740b4d7..b1abb02ba5 100644 --- a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php @@ -101,6 +101,14 @@ final class PhabricatorAuthFactorProvider return $config; } + public function newSortVector() { + $factor = $this->getFactor(); + + return id(new PhutilSortVector()) + ->addInt($factor->getFactorOrder()) + ->addInt($this->getID()); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index d2e9c34247..60fca07ff9 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -169,6 +169,7 @@ final class PhabricatorMultiFactorSettingsPanel ->addCancelButton($cancel_uri); } $providers = mpull($providers, null, 'getPHID'); + $proivders = msortv($providers, 'newSortVector'); $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, @@ -180,6 +181,13 @@ final class PhabricatorMultiFactorSettingsPanel $selected_provider = null; } else { $selected_provider = $providers[$selected_phid]; + + // Only let the user continue creating a factor for a given provider if + // they actually pass the provider's checks. + $selected_factor = $selected_provider->getFactor(); + if (!$selected_factor->canCreateNewConfiguration($viewer)) { + $selected_provider = null; + } } if (!$selected_provider) { @@ -192,13 +200,28 @@ final class PhabricatorMultiFactorSettingsPanel $provider_uri = id(new PhutilURI($this->getPanelURI())) ->setQueryParam('providerPHID', $provider_phid); + $factor = $provider->getFactor(); + $is_enabled = $factor->canCreateNewConfiguration($viewer); + $item = id(new PHUIObjectItemView()) ->setHeader($provider->getDisplayName()) - ->setHref($provider_uri) - ->setClickable(true) ->setImageIcon($provider->newIconView()) ->addAttribute($provider->getDisplayDescription()); + if ($is_enabled) { + $item + ->setHref($provider_uri) + ->setClickable(true); + } else { + $item->setDisabled(true); + } + + $create_description = $factor->getConfigurationCreateDescription( + $viewer); + if ($create_description) { + $item->appendChild($create_description); + } + $menu->addItem($item); } diff --git a/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css b/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css index 6f60560a2e..a793c018c3 100644 --- a/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css +++ b/webroot/rsrc/css/phui/object-item/phui-oi-big-ui.css @@ -72,3 +72,8 @@ .device-desktop .phui-oi-linked-container a:hover { text-decoration: none; } + +/* Spacing for InfoView inside an object item list, like MFA setup. */ +.phui-oi .phui-info-view { + margin: 0 4px 4px; +}