From 633aa5288c58d507d8227b1007e3f1dca7cb1e4f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Oct 2019 18:03:11 -0700 Subject: [PATCH] Persist login instructions onto flow-specific login pages (username/password and LDAP) Summary: Fixes T13433. Currently, "Login Screen Instructions" in "Auth" are shown only on the main login screen. If you enter a bad password or bad LDAP credential set and move to the flow-specific login failure screen (for example, "invalid password"), the instructions vanish. Instead, persist them. There are reasonable cases where this is highly useful and the cases which spring to mind where this is possibly misleading are fairly easy to fix by making the instructions more specific. Test Plan: - Configured login instructions in "Auth". - Viewed main login screen, saw instructions. - Entered a bad username/password and a bad LDAP credential set, got kicked to workflow sub-pages and still saw instructions (previously: no instructions). - Grepped for other callers to `buildProviderPageResponse()` to look for anything weird, came up empty. Maniphest Tasks: T13433 Differential Revision: https://secure.phabricator.com/D20863 --- resources/celerity/map.php | 6 ++--- .../controller/PhabricatorAuthController.php | 22 +++++++++++++++++++ .../PhabricatorAuthLoginController.php | 12 +++++++--- .../PhabricatorAuthStartController.php | 21 ------------------ webroot/rsrc/css/application/auth/auth.css | 2 +- 5 files changed, 35 insertions(+), 28 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 682f75a16b..0e53737aaf 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' => '88366522', + 'core.pkg.css' => '686ae87c', 'core.pkg.js' => '6e5c894f', 'differential.pkg.css' => '607c84be', 'differential.pkg.js' => 'a0212a0b', @@ -36,7 +36,7 @@ return array( 'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2', 'rsrc/css/aphront/typeahead.css' => '8779483d', 'rsrc/css/application/almanac/almanac.css' => '2e050f4f', - 'rsrc/css/application/auth/auth.css' => 'add92fd8', + 'rsrc/css/application/auth/auth.css' => 'c2f23d74', 'rsrc/css/application/base/main-menu-view.css' => '17b71bbc', 'rsrc/css/application/base/notification-menu.css' => '4df1ee30', 'rsrc/css/application/base/phui-theme.css' => '35883b37', @@ -540,7 +540,7 @@ return array( 'aphront-tooltip-css' => 'e3f2412f', 'aphront-typeahead-control-css' => '8779483d', 'application-search-view-css' => '0f7c06d8', - 'auth-css' => 'add92fd8', + 'auth-css' => 'c2f23d74', 'bulk-job-css' => '73af99f5', 'conduit-api-css' => 'ce2cfc41', 'config-options-css' => '16c920ae', diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php index cda56d34b1..505620b3f3 100644 --- a/src/applications/auth/controller/PhabricatorAuthController.php +++ b/src/applications/auth/controller/PhabricatorAuthController.php @@ -286,4 +286,26 @@ abstract class PhabricatorAuthController extends PhabricatorController { ->appendChild($invite_list); } + + final protected function newCustomStartMessage() { + $viewer = $this->getViewer(); + + $text = PhabricatorAuthMessage::loadMessageText( + $viewer, + PhabricatorAuthLoginMessageType::MESSAGEKEY); + + if (!strlen($text)) { + return null; + } + + $remarkup_view = new PHUIRemarkupView($viewer, $text); + + return phutil_tag( + 'div', + array( + 'class' => 'auth-custom-message', + ), + $remarkup_view); + } + } diff --git a/src/applications/auth/controller/PhabricatorAuthLoginController.php b/src/applications/auth/controller/PhabricatorAuthLoginController.php index e7dabd9340..b46cf36d49 100644 --- a/src/applications/auth/controller/PhabricatorAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthLoginController.php @@ -238,18 +238,24 @@ final class PhabricatorAuthLoginController $content) { $crumbs = $this->buildApplicationCrumbs(); + $viewer = $this->getViewer(); - if ($this->getRequest()->getUser()->isLoggedIn()) { + if ($viewer->isLoggedIn()) { $crumbs->addTextCrumb(pht('Link Account'), $provider->getSettingsURI()); } else { - $crumbs->addTextCrumb(pht('Log In'), $this->getApplicationURI('start/')); + $crumbs->addTextCrumb(pht('Login'), $this->getApplicationURI('start/')); + + $content = array( + $this->newCustomStartMessage(), + $content, + ); } $crumbs->addTextCrumb($provider->getProviderName()); $crumbs->setBorder(true); return $this->newPage() - ->setTitle(pht('Log In')) + ->setTitle(pht('Login')) ->setCrumbs($crumbs) ->appendChild($content); } diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 72cbbea5a8..7e9b17feff 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -298,27 +298,6 @@ final class PhabricatorAuthStartController ->setURI($auto_uri); } - private function newCustomStartMessage() { - $viewer = $this->getViewer(); - - $text = PhabricatorAuthMessage::loadMessageText( - $viewer, - PhabricatorAuthLoginMessageType::MESSAGEKEY); - - if (!strlen($text)) { - return null; - } - - $remarkup_view = new PHUIRemarkupView($viewer, $text); - - return phutil_tag( - 'div', - array( - 'class' => 'auth-custom-message', - ), - $remarkup_view); - } - private function newEmailLoginView(array $configs) { assert_instances_of($configs, 'PhabricatorAuthProviderConfig'); diff --git a/webroot/rsrc/css/application/auth/auth.css b/webroot/rsrc/css/application/auth/auth.css index 687aaf2bb4..28b18b85c5 100644 --- a/webroot/rsrc/css/application/auth/auth.css +++ b/webroot/rsrc/css/application/auth/auth.css @@ -57,7 +57,7 @@ } .auth-custom-message { - margin: 32px auto 64px; + margin: 32px auto 48px; max-width: 548px; background: #fff; padding: 16px;