From 1834584e98b684e4892bcc5512797e7c447cc54a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 20 Jun 2013 11:18:48 -0700 Subject: [PATCH] Provide contextual help on auth provider configuration Summary: Ref T1536. - Move all the provider-specific help into contextual help in Auth. - This provides help much more contextually, and we can just tell the user the right values to use to configure things. - Rewrite account/registration help to reflect the newer state of the word. - Also clean up a few other loose ends. Test Plan: {F46937} Reviewers: chad, btrahan Reviewed By: chad CC: aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6247 --- conf/default.conf.php | 9 - .../PhabricatorApplicationAuth.php | 12 ++ .../PhabricatorEmailLoginController.php | 2 +- .../PhabricatorEmailTokenController.php | 2 +- .../config/PhabricatorAuthEditController.php | 6 + .../auth/provider/PhabricatorAuthProvider.php | 4 + .../PhabricatorAuthProviderOAuthDisqus.php | 18 ++ .../PhabricatorAuthProviderOAuthFacebook.php | 19 +++ .../PhabricatorAuthProviderOAuthGitHub.php | 21 +++ .../PhabricatorAuthProviderOAuthGoogle.php | 21 +++ .../PhabricatorAuthProviderPassword.php | 18 ++ .../base/controller/PhabricatorController.php | 2 +- ...PhabricatorAuthenticationConfigOptions.php | 13 -- ...catorExtendingPhabricatorConfigOptions.php | 6 - .../people/storage/PhabricatorUser.php | 2 +- .../PhabricatorSettingsPanelPassword.php | 2 +- .../configuration/configuration_guide.diviner | 7 +- ...figuring_accounts_and_registration.diviner | 161 +++++------------- 18 files changed, 167 insertions(+), 158 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index d061e79a5e..0ae150cef1 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -553,11 +553,6 @@ return array( // -- Auth ------------------------------------------------------------------ // - // Can users login with a username/password, or by following the link from - // a password reset email? You can disable this and configure one or more - // OAuth providers instead. - 'auth.password-auth-enabled' => true, - // Maximum number of simultaneous web sessions each user is permitted to have. // Setting this to "1" will prevent a user from logging in on more than one // browser at the same time. @@ -1032,10 +1027,6 @@ return array( 'aphront.default-application-configuration-class' => 'AphrontDefaultApplicationConfiguration', - 'controller.oauth-registration' => - 'PhabricatorOAuthDefaultRegistrationController', - - // Directory that phd (the Phabricator daemon control script) should use to // track running daemons. 'phd.pid-directory' => '/var/tmp/phd/pid', diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index 26c1bf3c2a..4b11931926 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -14,6 +14,18 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { return 'authentication'; } + public function getHelpURI() { + // NOTE: Although reasonable help exists for this in "Configuring Accounts + // and Registration", specifying a help URI here means we get the menu + // item in all the login/link interfaces, which is confusing and not + // helpful. + + // TODO: Special case this, or split the auth and auth administration + // applications? + + return null; + } + public function buildMainMenuItems( PhabricatorUser $user, PhabricatorController $controller = null) { diff --git a/src/applications/auth/controller/PhabricatorEmailLoginController.php b/src/applications/auth/controller/PhabricatorEmailLoginController.php index 3875de3c62..d1726e57f4 100644 --- a/src/applications/auth/controller/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/PhabricatorEmailLoginController.php @@ -10,7 +10,7 @@ final class PhabricatorEmailLoginController public function processRequest() { $request = $this->getRequest(); - if (!PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) { + if (!PhabricatorAuthProviderPassword::getPasswordProvider()) { return new Aphront400Response(); } diff --git a/src/applications/auth/controller/PhabricatorEmailTokenController.php b/src/applications/auth/controller/PhabricatorEmailTokenController.php index 2f8d51f061..e907a7dcb4 100644 --- a/src/applications/auth/controller/PhabricatorEmailTokenController.php +++ b/src/applications/auth/controller/PhabricatorEmailTokenController.php @@ -74,7 +74,7 @@ final class PhabricatorEmailTokenController unset($unguarded); $next = '/'; - if (!PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) { + if (!PhabricatorAuthProviderPassword::getPasswordProvider()) { $next = '/settings/panel/external/'; } else if (PhabricatorEnv::getEnvConfig('account.editable')) { $next = (string)id(new PhutilURI('/settings/panel/password/')) diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 1f50db00aa..edc9f529aa 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -224,6 +224,12 @@ final class PhabricatorAuthEditController ->addCancelButton($cancel_uri) ->setValue($button)); + $help = $provider->getConfigurationHelp(); + if ($help) { + $form->appendChild(id(new PHUIFormDividerControl())); + $form->appendRemarkupInstructions($help); + } + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( id(new PhabricatorCrumbView()) diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index 68b0912389..faa878d4a8 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -21,6 +21,10 @@ abstract class PhabricatorAuthProvider { return $this->providerConfig; } + public function getConfigurationHelp() { + return null; + } + public function getDefaultProviderConfig() { return id(new PhabricatorAuthProviderConfig()) ->setProviderClass(get_class($this)) diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php index acc039d832..4c374be64c 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php @@ -7,6 +7,24 @@ final class PhabricatorAuthProviderOAuthDisqus return pht('Disqus'); } + public function getConfigurationHelp() { + $login_uri = $this->getLoginURI(); + + return pht( + "To configure Disqus OAuth, create a new application here:". + "\n\n". + "http://disqus.com/api/applications/". + "\n\n". + "Create an application, then adjust these settings:". + "\n\n". + " - **Callback URL:** Set this to `%s`". + "\n\n". + "After creating an application, copy the **Public Key** and ". + "**Secret Key** to the fields above (the **Public Key** goes in ". + "**OAuth App ID**).", + $login_uri); + } + protected function newOAuthAdapter() { return new PhutilAuthAdapterOAuthDisqus(); } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php index eeb25ed5d6..6c3b91f286 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php @@ -9,6 +9,25 @@ final class PhabricatorAuthProviderOAuthFacebook return pht('Facebook'); } + public function getConfigurationHelp() { + $uri = new PhutilURI(PhabricatorEnv::getProductionURI('/')); + return pht( + 'To configure Facebook OAuth, create a new Facebook Application here:'. + "\n\n". + 'https://developers.facebook.com/apps'. + "\n\n". + 'You should use these settings in your application:'. + "\n\n". + " - **Site URL**: Set this to your full domain with protocol. For ". + " this Phabricator install, the correct value is: `%s`\n". + " - **Site Domain**: Set this to the full domain without a protocol. ". + " For this Phabricator install, the correct value is: `%s`\n\n". + "After creating your new application, copy the **App ID** and ". + "**App Secret** to the fields above.", + (string)$uri, + $uri->getDomain()); + } + public function getDefaultProviderConfig() { return parent::getDefaultProviderConfig() ->setProperty(self::KEY_REQUIRE_SECURE, 1); diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php index f56f202606..c87628f7ec 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php @@ -7,6 +7,27 @@ final class PhabricatorAuthProviderOAuthGitHub return pht('GitHub'); } + public function getConfigurationHelp() { + $uri = PhabricatorEnv::getProductionURI('/'); + $callback_uri = $this->getLoginURI(); + + return pht( + "To configure GitHub OAuth, create a new GitHub Application here:". + "\n\n". + "https://github.com/settings/applications/new". + "\n\n". + "You should use these settings in your application:". + "\n\n". + " - **URL:** Set this to your full domain with protocol. For this ". + " Phabricator install, the correct value is: `%s`\n". + " - **Callback URL**: Set this to: `%s`\n". + "\n\n". + "Once you've created an application, copy the **Client ID** and ". + "**Client Secret** into the fields above.", + $uri, + $callback_uri); + } + protected function newOAuthAdapter() { return new PhutilAuthAdapterOAuthGitHub(); } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php index a20aec5df2..bd10f13afd 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php @@ -7,6 +7,27 @@ final class PhabricatorAuthProviderOAuthGoogle return pht('Google'); } + public function getConfigurationHelp() { + $login_uri = $this->getLoginURI(); + + return pht( + "To configure Google OAuth, create a new 'API Project' here:". + "\n\n". + "https://code.google.com/apis/console/". + "\n\n". + "You don't need to enable any Services, just go to **API Access**, ". + "click **Create an OAuth 2.0 client ID...**, and configure these ". + "settings:". + "\n\n". + " - During initial setup click **More Options** (or after creating ". + " the client ID, click **Edit Settings...**), then add this to ". + " **Authorized Redirect URIs**: `%s`\n". + "\n\n". + "After completing configuration, copy the **Client ID** and ". + "**Client Secret** to the fields above.", + $login_uri); + } + protected function newOAuthAdapter() { return new PhutilAuthAdapterOAuthGoogle(); } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php index 3f315f676c..049c56d29e 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -9,6 +9,12 @@ final class PhabricatorAuthProviderPassword return pht('Username/Password'); } + public function getConfigurationHelp() { + return pht( + 'You can select a minimum password length by setting '. + '`account.minimum-password-length` in configuration.'); + } + public function getDescriptionForCreate() { return pht( 'Allow users to login or register using a username and password.'); @@ -227,4 +233,16 @@ final class PhabricatorAuthProviderPassword $account->setAccountID($account->getUserPHID()); } + public static function getPasswordProvider() { + $providers = self::getAllEnabledProviders(); + + foreach ($providers as $provider) { + if ($provider instanceof PhabricatorAuthProviderPassword) { + return $provider; + } + } + + return null; + } + } diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index bcfcce63fa..22df5631dc 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -101,7 +101,7 @@ abstract class PhabricatorController extends AphrontController { if ($this->shouldRequireLogin() && !$user->getPHID()) { $login_controller = new PhabricatorAuthStartController($request); - $login_controller->setCurrentApplication( + $this->setCurrentApplication( PhabricatorApplication::getByClass('PhabricatorApplicationAuth')); return $this->delegateToController($login_controller); } diff --git a/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php b/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php index d2f49d4886..9be99adff2 100644 --- a/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php +++ b/src/applications/config/option/PhabricatorAuthenticationConfigOptions.php @@ -13,19 +13,6 @@ final class PhabricatorAuthenticationConfigOptions public function getOptions() { return array( - $this->newOption( - 'auth.password-auth-enabled', 'bool', true) - ->setBoolOptions( - array( - pht("Allow password authentication"), - pht("Don't allow password authentication") - )) - ->setSummary(pht("Enables password-based authentication.")) - ->setDescription( - pht( - "Can users login with a username/password, or by following the ". - "link from a password reset email? You can disable this and ". - "configure one or more OAuth providers instead.")), $this->newOption('auth.sessions.web', 'int', 5) ->setSummary( pht("Number of web sessions a user can have simultaneously.")) diff --git a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php index 6f3cd66a7a..0b5b33b577 100644 --- a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php +++ b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php @@ -47,12 +47,6 @@ final class PhabricatorExtendingPhabricatorConfigOptions ->setBaseClass('AphrontApplicationConfiguration') // TODO: This could probably use some better documentation. ->setDescription(pht("Application configuration class.")), - $this->newOption( - 'controller.oauth-registration', - 'class', - 'PhabricatorOAuthDefaultRegistrationController') - ->setBaseClass('PhabricatorOAuthRegistrationController') - ->setDescription(pht("OAuth registration controller.")), ); } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 16016a3be5..324131194d 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -612,7 +612,7 @@ EOBODY; $new_username = $this->getUserName(); $password_instructions = null; - if (PhabricatorEnv::getEnvConfig('auth.password-auth-enabled')) { + if (PhabricatorAuthProviderPassword::getPasswordProvider()) { $uri = $this->getEmailLoginURI(); $password_instructions = << + +...where `` is the admin account username you want to recover access +to. This will give you a link which will log you in as the specified +administrative user. = Managing Accounts with the Web Console = @@ -38,114 +47,20 @@ To manage accounts from the web, login as an administrator account and go to ##/people/## or click "People" on the homepage. Provided you're an admin, you'll see options to create or edit accounts. -= Managing Accounts from the Command Line = += Manually Creating New Accounts = -You can use ##scripts/user/add_user.php## to batch create accounts. Run it -like: +There are two ways to manually create new accounts: via the web UI using +the "People" application (this is easiest), or via the CLI using the +`accountadmin` binary (this has a few more options). - $ ./add_user.php +To use the CLI script, run: -For example: + phabricator/ $ ./bin/accountadmin - $ ./add_user.php alincoln alincoln@logcabin.com 'Abraham Lincoln' tjefferson - -This will create a new ##alincoln## user and send them a "Welcome to -Phabricator" email from ##tjefferson## with instructions on how to log in and -set a password. - -= Configuring Facebook OAuth = - -You can configure Facebook OAuth to allow login, login and registration, or -nothing (the default). If registration is not allowed, users must have an -existing account in order to link a Facebook account to it, but can use -Facebook to login once the accounts are linked. - -To configure Facebook OAuth, create a new Facebook Application: - -https://developers.facebook.com/apps - -You should set these things in your application: - - - **Site URL**: Set this to your full domain with protocol, like - "##https://phabricator.example.com/##". - - **Site Domain**: Set this to the entire domain, like ##example.com##. You - might be able to get away with including the subdomain if you want to - scope more tightly. - -Once that is set up, edit your Phabricator configuration and set these keys: - - - **facebook.auth-enabled**: set this to ##true##. - - **facebook.application-id**: set to your Facebook application's ID. Make - sure you set this as a string. - - **facebook.application-secret**: set to your Facebook application's - secret key. - - **facebook.registration-enabled**: set this to ##true## to let users - register for your install with a Facebook account (this is a very open - setting) or ##false## to prevent users from registering with Facebook. - - **facebook.auth-permanent**: you can set this to prevent account unlinking. - It is unlikely you want to prevent it, but Facebook's internal install uses - this option since Facebook uses Facebook as its only auth mechanism. - -= Configuring GitHub OAuth = - -You can configure GitHub OAuth to allow login, login and registration, or -nothing (the default). - -To configure GitHub OAuth, create a new GitHub Application: - -https://github.com/settings/applications/new - -You should set these things in your application: - - - **URL**: Set this to the full domain with protocol, like - "##https://phabricator.example.com/##". - - **Callback URL**: Set this to your domain plus "##/oauth/github/login/##", - like "##https://phabricator.example.com/oauth/github/login/##". - -Once you've created an application, edit your Phabricator configuration and -set these keys: - - - **github.auth-enabled**: set this to ##true##. - - **github.application-id**: set this to your application/client ID. - - **github.application-secret**: set this to your application secret. - - **github.registration-enabled**: set to ##true## to let users register with - just GitHub credentials (this is a very open setting) or ##false## to - prevent users from registering. If set to ##false##, users may still link - existing accounts and use GitHub to login, they just can't create new - accounts. - - **github.auth-permanent**: set to ##true## to prevent unlinking Phabricator - accounts from GitHub accounts. - -= Configuring Google OAuth = - -You can configure Google OAuth to allow login, login and registration, or -nothing (the default). - -To configure Google OAuth, create a new Google "API Project": - -https://code.google.com/apis/console/ - -You don't need to enable any **Services**, just go to **API Access**, click -**"Create an OAuth 2.0 client ID..."**, and configure these settings: - - - Click **More Options** next to **Authorized Redirect APIs** and add the - full domain (with protocol) plus ##/oauth/google/login/## to the list. - For example, ##https://phabricator.example.com/oauth/google/login/## - - Click **Create Client ID**. - -Once you've created a client ID, edit your Phabricator configuration and set -these keys: - - - **google.auth-enabled**: set this to ##true##. - - **google.application-id**: set this to your Client ID (from above). - - **google.application-secret**: set this to your Client Secret (from above). - - **google.registration-enabled**: set this to ##true## to let users register - with just Google credentials (this is a very open setting) or ##false## to - prevent users from registering. If set to ##false##, users may still link - existing accounts and use Google to login, they jus can't create new - accounts. - - **google.auth-permanent**: set this to ##true## to prevent unlinking - Phabricator accounts from Google accounts. +Some options (like setting passwords and changing certain account flags) are +only available from the CLI. You can also use this script to make a user +an administrator (if you accidentally remove your admin flag) or create an +administrative account. = Next Steps =