From 55c18bc90041f15759f62eaa44e31e5f61359be1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2019 12:59:55 -0800 Subject: [PATCH] During first-time setup, create an administrator account with no authentication instead of weird, detached authentication Summary: Ref T6703. Currently, when you create an account on a new install, we prompt you to select a password. You can't actually use that password unless you set up a password provider, and that password can't be associated with a provider since a password provider won't exist yet. Instead, just don't ask for a password: create an account with a username and an email address only. Setup guidance points you toward Auth. If you lose the session, you can send yourself an email link (if email works yet) or `bin/auth recover` it. This isn't really much different than the pre-change behavior, since you can't use the password you set anyway until you configure password auth. This also makes fixing T9512 more important, which I'll do in a followup. I also plan to add slightly better guideposts toward Auth. Test Plan: Hit first-time setup, created an account. Reviewers: amckinley Reviewed By: amckinley Subscribers: revi Maniphest Tasks: T6703 Differential Revision: https://secure.phabricator.com/D20111 --- .../PhabricatorAuthRegisterController.php | 118 +++++++++--------- .../people/storage/PhabricatorUser.php | 2 +- .../people/storage/PhabricatorUserEmail.php | 5 +- ...figuring_accounts_and_registration.diviner | 32 +++-- 4 files changed, 89 insertions(+), 68 deletions(-) diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php index 9e1aef592c..0562a4242c 100644 --- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php +++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php @@ -21,7 +21,9 @@ final class PhabricatorAuthRegisterController list($account, $provider, $response) = $result; $is_default = false; } else if ($this->isFirstTimeSetup()) { - list($account, $provider, $response) = $this->loadSetupAccount(); + $account = null; + $provider = null; + $response = null; $is_default = true; $is_setup = true; } else { @@ -35,22 +37,24 @@ final class PhabricatorAuthRegisterController $invite = $this->loadInvite(); - if (!$provider->shouldAllowRegistration()) { - if ($invite) { - // If the user has an invite, we allow them to register with any - // provider, even a login-only provider. - } else { - // TODO: This is a routine error if you click "Login" on an external - // auth source which doesn't allow registration. The error should be - // more tailored. + if (!$is_setup) { + if (!$provider->shouldAllowRegistration()) { + if ($invite) { + // If the user has an invite, we allow them to register with any + // provider, even a login-only provider. + } else { + // TODO: This is a routine error if you click "Login" on an external + // auth source which doesn't allow registration. The error should be + // more tailored. - return $this->renderError( - pht( - 'The account you are attempting to register with uses an '. - 'authentication provider ("%s") which does not allow '. - 'registration. An administrator may have recently disabled '. - 'registration with this provider.', - $provider->getProviderName())); + return $this->renderError( + pht( + 'The account you are attempting to register with uses an '. + 'authentication provider ("%s") which does not allow '. + 'registration. An administrator may have recently disabled '. + 'registration with this provider.', + $provider->getProviderName())); + } } } @@ -58,14 +62,19 @@ final class PhabricatorAuthRegisterController $user = new PhabricatorUser(); - $default_username = $account->getUsername(); - $default_realname = $account->getRealName(); + if ($is_setup) { + $default_username = null; + $default_realname = null; + $default_email = null; + } else { + $default_username = $account->getUsername(); + $default_realname = $account->getRealName(); + $default_email = $account->getEmail(); + } $account_type = PhabricatorAuthPassword::PASSWORD_TYPE_ACCOUNT; $content_source = PhabricatorContentSource::newFromRequest($request); - $default_email = $account->getEmail(); - if ($invite) { $default_email = $invite->getEmailAddress(); } @@ -212,7 +221,11 @@ final class PhabricatorAuthRegisterController $can_edit_email = $profile->getCanEditEmail(); $can_edit_realname = $profile->getCanEditRealName(); - $must_set_password = $provider->shouldRequireRegistrationPassword(); + if ($is_setup) { + $must_set_password = false; + } else { + $must_set_password = $provider->shouldRequireRegistrationPassword(); + } $can_edit_anything = $profile->getCanEditAnything() || $must_set_password; $force_verify = $profile->getShouldVerifyEmail(); @@ -334,9 +347,11 @@ final class PhabricatorAuthRegisterController } if (!$errors) { - $image = $this->loadProfilePicture($account); - if ($image) { - $user->setProfileImagePHID($image->getPHID()); + if (!$is_setup) { + $image = $this->loadProfilePicture($account); + if ($image) { + $user->setProfileImagePHID($image->getPHID()); + } } try { @@ -346,17 +361,19 @@ final class PhabricatorAuthRegisterController $verify_email = true; } - if ($value_email === $default_email) { - if ($account->getEmailVerified()) { - $verify_email = true; - } + if (!$is_setup) { + if ($value_email === $default_email) { + if ($account->getEmailVerified()) { + $verify_email = true; + } - if ($provider->shouldTrustEmails()) { - $verify_email = true; - } + if ($provider->shouldTrustEmails()) { + $verify_email = true; + } - if ($invite) { - $verify_email = true; + if ($invite) { + $verify_email = true; + } } } @@ -438,9 +455,11 @@ final class PhabricatorAuthRegisterController $transaction_editor->applyTransactions($user, $xactions); } - $account->setUserPHID($user->getPHID()); - $provider->willRegisterAccount($account); - $account->save(); + if (!$is_setup) { + $account->setUserPHID($user->getPHID()); + $provider->willRegisterAccount($account); + $account->save(); + } $user->saveTransaction(); @@ -501,7 +520,6 @@ final class PhabricatorAuthRegisterController ->setAuthProvider($provider))); } - if ($can_edit_username) { $form->appendChild( id(new AphrontFormTextControl()) @@ -595,7 +613,7 @@ final class PhabricatorAuthRegisterController pht( 'Installation is complete. Register your administrator account '. 'below to log in. You will be able to configure options and add '. - 'other authentication mechanisms (like LDAP or OAuth) later on.')); + 'authentication mechanisms later on.')); } $object_box = id(new PHUIObjectBoxView()) @@ -612,11 +630,12 @@ final class PhabricatorAuthRegisterController $view = id(new PHUITwoColumnView()) ->setHeader($header) - ->setFooter(array( - $welcome_view, - $invite_header, - $object_box, - )); + ->setFooter( + array( + $welcome_view, + $invite_header, + $object_box, + )); return $this->newPage() ->setTitle($title) @@ -657,19 +676,6 @@ final class PhabricatorAuthRegisterController return array($account, $provider, $response); } - private function loadSetupAccount() { - $provider = new PhabricatorPasswordAuthProvider(); - $provider->attachProviderConfig( - id(new PhabricatorAuthProviderConfig()) - ->setShouldAllowRegistration(1) - ->setShouldAllowLogin(1) - ->setIsEnabled(true)); - - $account = $provider->getDefaultExternalAccount(); - $response = null; - return array($account, $provider, $response); - } - private function loadProfilePicture(PhabricatorExternalAccount $account) { $phid = $account->getProfileImagePHID(); if (!$phid) { diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 055df8b79e..c675878747 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -557,7 +557,7 @@ final class PhabricatorUser public static function describeValidUsername() { return pht( - 'Usernames must contain only numbers, letters, period, underscore and '. + 'Usernames must contain only numbers, letters, period, underscore, and '. 'hyphen, and can not end with a period. They must have no more than %d '. 'characters.', new PhutilNumber(self::MAXIMUM_USERNAME_LENGTH)); diff --git a/src/applications/people/storage/PhabricatorUserEmail.php b/src/applications/people/storage/PhabricatorUserEmail.php index 42946015de..572c7d6e8b 100644 --- a/src/applications/people/storage/PhabricatorUserEmail.php +++ b/src/applications/people/storage/PhabricatorUserEmail.php @@ -83,9 +83,8 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO { */ public static function describeValidAddresses() { return pht( - "Email addresses should be in the form '%s'. The maximum ". - "length of an email address is %s character(s).", - 'user@domain.com', + 'Email addresses should be in the form "user@domain.com". The maximum '. + 'length of an email address is %s characters.', new PhutilNumber(self::MAX_ADDRESS_LENGTH)); } diff --git a/src/docs/user/configuration/configuring_accounts_and_registration.diviner b/src/docs/user/configuration/configuring_accounts_and_registration.diviner index 05d11b11f3..a56d7377cb 100644 --- a/src/docs/user/configuration/configuring_accounts_and_registration.diviner +++ b/src/docs/user/configuration/configuring_accounts_and_registration.diviner @@ -3,7 +3,8 @@ Describes how to configure user access to Phabricator. -= Overview = +Overview +======== Phabricator supports a number of login systems. You can enable or disable these systems to configure who can register for and access your install, and how users @@ -28,24 +29,37 @@ After you add a provider, you can link it to existing accounts (for example, associate an existing Phabricator account with a GitHub OAuth account) or users can use it to register new accounts (assuming you enable these options). -= Recovering Inaccessible Accounts = + +Recovering Inaccessible Accounts +================================ If you accidentally lock yourself out of Phabricator (for example, by disabling -all authentication providers), you can use the `bin/auth` -script to recover access to an account. To recover access, run: +all authentication providers), you can normally use the "send a login link" +action from the login screen to email yourself a login link and regain access +to your account. - phabricator/ $ ./bin/auth recover +If that isn't working (perhaps because you haven't configured email yet), you +can use the `bin/auth` script to recover access to an account. To recover +access, run: + +``` +phabricator/ $ ./bin/auth recover +``` ...where `` is the account username you want to recover access to. This will generate a link which will log you in as the specified user. -= Managing Accounts with the Web Console = + +Managing Accounts with the Web Console +====================================== 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. -= Manually Creating New Accounts = + +Manually Creating New Accounts +============================== 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 @@ -60,7 +74,9 @@ the CLI. You can also use this script to make a user an administrator (if you accidentally remove your admin flag) or to create an administrative account. -= Next Steps = + +Next Steps +========== Continue by: