From 5dccc14bbf46b0cfaabf1d595c5f446c6e0302c1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Sep 2015 10:34:39 -0700 Subject: [PATCH] Modularize generation of supplemental login messages Summary: Ref T9346. This mostly allows us to give users additional advice based on which instance they are trying to log in to in the Phacility cluster. It's also slightly more flexible than `auth.login-message` was, and maybe we'll add some more hooks here eventually. This feels like it's a sidegrade in complexity rather than really an improvement, but not too terrible. Test Plan: - Wrote the custom handler in T9346 to replicate old config functionality. - Wrote a smart handler for Phacility that can provide context-sensitive messages based on which OAuth client you're trying to use. See new message box at top (implementation in next diff): {F780375} Reviewers: chad Reviewed By: chad Maniphest Tasks: T9346 Differential Revision: https://secure.phabricator.com/D14057 --- src/__phutil_library_map__.php | 2 ++ .../PhabricatorAuthStartController.php | 20 +++++++++-- .../handler/PhabricatorAuthLoginHandler.php | 36 +++++++++++++++++++ .../PhabricatorExtraConfigSetupCheck.php | 4 +++ ...PhabricatorAuthenticationConfigOptions.php | 8 ----- 5 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/applications/auth/handler/PhabricatorAuthLoginHandler.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index aba0b8a1c5..f01d82e40f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1613,6 +1613,7 @@ phutil_register_library_map(array( 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', 'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php', 'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php', + 'PhabricatorAuthLoginHandler' => 'applications/auth/handler/PhabricatorAuthLoginHandler.php', 'PhabricatorAuthManagementCachePKCS8Workflow' => 'applications/auth/management/PhabricatorAuthManagementCachePKCS8Workflow.php', 'PhabricatorAuthManagementLDAPWorkflow' => 'applications/auth/management/PhabricatorAuthManagementLDAPWorkflow.php', 'PhabricatorAuthManagementListFactorsWorkflow' => 'applications/auth/management/PhabricatorAuthManagementListFactorsWorkflow.php', @@ -5458,6 +5459,7 @@ phutil_register_library_map(array( 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController', + 'PhabricatorAuthLoginHandler' => 'Phobject', 'PhabricatorAuthManagementCachePKCS8Workflow' => 'PhabricatorAuthManagementWorkflow', 'PhabricatorAuthManagementLDAPWorkflow' => 'PhabricatorAuthManagementWorkflow', 'PhabricatorAuthManagementListFactorsWorkflow' => 'PhabricatorAuthManagementWorkflow', diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 01578446a5..ff7c18091f 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -163,8 +163,22 @@ final class PhabricatorAuthStartController $button_columns); } - $login_message = PhabricatorEnv::getEnvConfig('auth.login-message'); - $login_message = phutil_safe_html($login_message); + $handlers = PhabricatorAuthLoginHandler::getAllHandlers(); + + $delegating_controller = $this->getDelegatingController(); + + $header = array(); + foreach ($handlers as $handler) { + $handler = clone $handler; + + $handler->setRequest($request); + + if ($delegating_controller) { + $handler->setDelegatingController($delegating_controller); + } + + $header[] = $handler->getAuthLoginHeaderContent(); + } $invite_message = null; if ($invite) { @@ -178,7 +192,7 @@ final class PhabricatorAuthStartController return $this->buildApplicationPage( array( $crumbs, - $login_message, + $header, $invite_message, $out, ), diff --git a/src/applications/auth/handler/PhabricatorAuthLoginHandler.php b/src/applications/auth/handler/PhabricatorAuthLoginHandler.php new file mode 100644 index 0000000000..eabbf91843 --- /dev/null +++ b/src/applications/auth/handler/PhabricatorAuthLoginHandler.php @@ -0,0 +1,36 @@ +delegatingController = $controller; + return $this; + } + + final public function getDelegatingController() { + return $this->delegatingController; + } + + final public function setRequest(AphrontRequest $request) { + $this->request = $request; + return $this; + } + + final public function getRequest() { + return $this->request; + } + + final public static function getAllHandlers() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->execute(); + } +} diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index a482f4d073..b73f145033 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -276,6 +276,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'Impersonating users over the API is no longer supported.'), 'feed.public' => pht('The framable public feed is no longer supported.'), + + 'auth.login-message' => pht( + 'This configuration option has been replaced with a modular '. + 'handler. See T9346.'), ); return $ancient_config; diff --git a/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php b/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php index a965d16f42..8092f52ff8 100644 --- a/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php +++ b/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php @@ -73,14 +73,6 @@ final class PhabricatorAuthenticationConfigOptions ->addExample( "yourcompany.com\nmail.yourcompany.com", pht('Valid Setting')), - $this->newOption('auth.login-message', 'string', null) - ->setLocked(true) - ->setSummary(pht('A block of HTML displayed on the login screen.')) - ->setDescription( - pht( - "You can provide an arbitrary block of HTML here, which will ". - "appear on the login screen. Normally, you'd use this to provide ". - "login or registration instructions to users.")), $this->newOption('account.editable', 'bool', true) ->setBoolOptions( array(