1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00

Add password authentication and registration to new registration

Summary:
Ref T1536. Ref T1930. Code is not reachable.

This provides password authentication and registration on the new provider/adapter framework.

I sort of cheated a little bit and don't really route any password logic through the adapter (instead, this provider uses an empty adapter and just sets the type/domain on it). I think the right way to do this //conceptually// is to treat username/passwords as an external black box which the adapter communicates with. However, this creates a lot of practical implementation and UX problems:

  - There would basically be two steps -- in the first one, you interact with the "password black box", which behaves like an OAuth provider. This produces some ExternalAccount associated with the username/password pair, then we go into normal registration.
  - In normal registration, we'd proceed normally.

This means:

  - The registration flow would be split into two parts, one where you select a username/password (interacting with the black box) and one where you actually register (interacting with the generic flow). This is unusual and probably confusing for users.
  - We would need to do a lot of re-hashing of passwords, since passwords currently depend on the username and user PHID, which won't exist yet during registration or the "black box" phase. This is a big mess I don't want to deal with.
  - We hit a weird condition where two users complete step 1 with the same username but don't complete step 2 yet. The box knows about two different copies of the username, with two different passwords. When we arrive at step 2 the second time we have a lot of bad choices about how to reoslve it, most of which create security problems. The most stragihtforward and "pure" way to resolve the issues is to put password-auth usernames in a separate space, but this would be incredibly confusuing to users (your login name might not be the same as your username, which is bizarre).
  - If we change this, we need to update all the other password-related code, which I don't want to bother with (at least for now).

Instead, let registration know about a "default" registration controller (which is always password, if enabled), and let it require a password. This gives us a much simpler (albeit slightly less pure) implementation:

  - All the fields are on one form.
  - Password adapter is just a shell.
  - Password provider does the heavy lifting.

We might make this more pure at some point, but I'm generally pretty satisfied with this.

This doesn't implement the brute-force CAPTCHA protection, that will be coming soon.

Test Plan: Registered with password only and logged in with a password. Hit various error conditions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T1536, T1930

Differential Revision: https://secure.phabricator.com/D6164
This commit is contained in:
epriestley 2013-06-16 10:15:49 -07:00
parent 104d3221d9
commit 1329b7b51e
7 changed files with 332 additions and 50 deletions

View file

@ -814,7 +814,7 @@ celerity_register_resource_map(array(
), ),
'aphront-form-view-css' => 'aphront-form-view-css' =>
array( array(
'uri' => '/res/4fe4c174/rsrc/css/aphront/form-view.css', 'uri' => '/res/a668dc36/rsrc/css/aphront/form-view.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -941,7 +941,7 @@ celerity_register_resource_map(array(
), ),
'conpherence-message-pane-css' => 'conpherence-message-pane-css' =>
array( array(
'uri' => '/res/a3cb0efc/rsrc/css/application/conpherence/message-pane.css', 'uri' => '/res/80cde760/rsrc/css/application/conpherence/message-pane.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -959,7 +959,7 @@ celerity_register_resource_map(array(
), ),
'conpherence-widget-pane-css' => 'conpherence-widget-pane-css' =>
array( array(
'uri' => '/res/d1db8220/rsrc/css/application/conpherence/widget-pane.css', 'uri' => '/res/ee9d5004/rsrc/css/application/conpherence/widget-pane.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -3601,7 +3601,7 @@ celerity_register_resource_map(array(
), ),
'phabricator-zindex-css' => 'phabricator-zindex-css' =>
array( array(
'uri' => '/res/f919f9e1/rsrc/css/core/z-index.css', 'uri' => '/res/99a9447b/rsrc/css/core/z-index.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -4048,7 +4048,7 @@ celerity_register_resource_map(array(
), array( ), array(
'packages' => 'packages' =>
array( array(
'0a101340' => 'ab82f2c0' =>
array( array(
'name' => 'core.pkg.css', 'name' => 'core.pkg.css',
'symbols' => 'symbols' =>
@ -4096,7 +4096,7 @@ celerity_register_resource_map(array(
40 => 'phabricator-property-list-view-css', 40 => 'phabricator-property-list-view-css',
41 => 'phabricator-tag-view-css', 41 => 'phabricator-tag-view-css',
), ),
'uri' => '/res/pkg/0a101340/core.pkg.css', 'uri' => '/res/pkg/ab82f2c0/core.pkg.css',
'type' => 'css', 'type' => 'css',
), ),
'f2ad0683' => 'f2ad0683' =>
@ -4290,16 +4290,16 @@ celerity_register_resource_map(array(
'reverse' => 'reverse' =>
array( array(
'aphront-attached-file-view-css' => 'a7ca34a9', 'aphront-attached-file-view-css' => 'a7ca34a9',
'aphront-dialog-view-css' => '0a101340', 'aphront-dialog-view-css' => 'ab82f2c0',
'aphront-error-view-css' => '0a101340', 'aphront-error-view-css' => 'ab82f2c0',
'aphront-form-view-css' => '0a101340', 'aphront-form-view-css' => 'ab82f2c0',
'aphront-list-filter-view-css' => '0a101340', 'aphront-list-filter-view-css' => 'ab82f2c0',
'aphront-pager-view-css' => '0a101340', 'aphront-pager-view-css' => 'ab82f2c0',
'aphront-panel-view-css' => '0a101340', 'aphront-panel-view-css' => 'ab82f2c0',
'aphront-table-view-css' => '0a101340', 'aphront-table-view-css' => 'ab82f2c0',
'aphront-tokenizer-control-css' => '0a101340', 'aphront-tokenizer-control-css' => 'ab82f2c0',
'aphront-tooltip-css' => '0a101340', 'aphront-tooltip-css' => 'ab82f2c0',
'aphront-typeahead-control-css' => '0a101340', 'aphront-typeahead-control-css' => 'ab82f2c0',
'differential-changeset-view-css' => 'dd27a69b', 'differential-changeset-view-css' => 'dd27a69b',
'differential-core-view-css' => 'dd27a69b', 'differential-core-view-css' => 'dd27a69b',
'differential-inline-comment-editor' => '9488bb69', 'differential-inline-comment-editor' => '9488bb69',
@ -4313,7 +4313,7 @@ celerity_register_resource_map(array(
'differential-table-of-contents-css' => 'dd27a69b', 'differential-table-of-contents-css' => 'dd27a69b',
'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-commit-view-css' => 'c8ce2d88',
'diffusion-icons-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88',
'global-drag-and-drop-css' => '0a101340', 'global-drag-and-drop-css' => 'ab82f2c0',
'inline-comment-summary-css' => 'dd27a69b', 'inline-comment-summary-css' => 'dd27a69b',
'javelin-aphlict' => 'f2ad0683', 'javelin-aphlict' => 'f2ad0683',
'javelin-behavior' => 'a9f14d76', 'javelin-behavior' => 'a9f14d76',
@ -4387,55 +4387,55 @@ celerity_register_resource_map(array(
'javelin-util' => 'a9f14d76', 'javelin-util' => 'a9f14d76',
'javelin-vector' => 'a9f14d76', 'javelin-vector' => 'a9f14d76',
'javelin-workflow' => 'a9f14d76', 'javelin-workflow' => 'a9f14d76',
'lightbox-attachment-css' => '0a101340', 'lightbox-attachment-css' => 'ab82f2c0',
'maniphest-task-summary-css' => 'a7ca34a9', 'maniphest-task-summary-css' => 'a7ca34a9',
'maniphest-transaction-detail-css' => 'a7ca34a9', 'maniphest-transaction-detail-css' => 'a7ca34a9',
'phabricator-action-list-view-css' => '0a101340', 'phabricator-action-list-view-css' => 'ab82f2c0',
'phabricator-application-launch-view-css' => '0a101340', 'phabricator-application-launch-view-css' => 'ab82f2c0',
'phabricator-busy' => 'f2ad0683', 'phabricator-busy' => 'f2ad0683',
'phabricator-content-source-view-css' => 'dd27a69b', 'phabricator-content-source-view-css' => 'dd27a69b',
'phabricator-core-css' => '0a101340', 'phabricator-core-css' => 'ab82f2c0',
'phabricator-crumbs-view-css' => '0a101340', 'phabricator-crumbs-view-css' => 'ab82f2c0',
'phabricator-drag-and-drop-file-upload' => '9488bb69', 'phabricator-drag-and-drop-file-upload' => '9488bb69',
'phabricator-dropdown-menu' => 'f2ad0683', 'phabricator-dropdown-menu' => 'f2ad0683',
'phabricator-file-upload' => 'f2ad0683', 'phabricator-file-upload' => 'f2ad0683',
'phabricator-filetree-view-css' => '0a101340', 'phabricator-filetree-view-css' => 'ab82f2c0',
'phabricator-flag-css' => '0a101340', 'phabricator-flag-css' => 'ab82f2c0',
'phabricator-form-view-css' => '0a101340', 'phabricator-form-view-css' => 'ab82f2c0',
'phabricator-header-view-css' => '0a101340', 'phabricator-header-view-css' => 'ab82f2c0',
'phabricator-hovercard' => 'f2ad0683', 'phabricator-hovercard' => 'f2ad0683',
'phabricator-jump-nav' => '0a101340', 'phabricator-jump-nav' => 'ab82f2c0',
'phabricator-keyboard-shortcut' => 'f2ad0683', 'phabricator-keyboard-shortcut' => 'f2ad0683',
'phabricator-keyboard-shortcut-manager' => 'f2ad0683', 'phabricator-keyboard-shortcut-manager' => 'f2ad0683',
'phabricator-main-menu-view' => '0a101340', 'phabricator-main-menu-view' => 'ab82f2c0',
'phabricator-menu-item' => 'f2ad0683', 'phabricator-menu-item' => 'f2ad0683',
'phabricator-nav-view-css' => '0a101340', 'phabricator-nav-view-css' => 'ab82f2c0',
'phabricator-notification' => 'f2ad0683', 'phabricator-notification' => 'f2ad0683',
'phabricator-notification-css' => '0a101340', 'phabricator-notification-css' => 'ab82f2c0',
'phabricator-notification-menu-css' => '0a101340', 'phabricator-notification-menu-css' => 'ab82f2c0',
'phabricator-object-item-list-view-css' => '0a101340', 'phabricator-object-item-list-view-css' => 'ab82f2c0',
'phabricator-object-selector-css' => 'dd27a69b', 'phabricator-object-selector-css' => 'dd27a69b',
'phabricator-phtize' => 'f2ad0683', 'phabricator-phtize' => 'f2ad0683',
'phabricator-prefab' => 'f2ad0683', 'phabricator-prefab' => 'f2ad0683',
'phabricator-project-tag-css' => 'a7ca34a9', 'phabricator-project-tag-css' => 'a7ca34a9',
'phabricator-property-list-view-css' => '0a101340', 'phabricator-property-list-view-css' => 'ab82f2c0',
'phabricator-remarkup-css' => '0a101340', 'phabricator-remarkup-css' => 'ab82f2c0',
'phabricator-shaped-request' => '9488bb69', 'phabricator-shaped-request' => '9488bb69',
'phabricator-side-menu-view-css' => '0a101340', 'phabricator-side-menu-view-css' => 'ab82f2c0',
'phabricator-standard-page-view' => '0a101340', 'phabricator-standard-page-view' => 'ab82f2c0',
'phabricator-tag-view-css' => '0a101340', 'phabricator-tag-view-css' => 'ab82f2c0',
'phabricator-textareautils' => 'f2ad0683', 'phabricator-textareautils' => 'f2ad0683',
'phabricator-tooltip' => 'f2ad0683', 'phabricator-tooltip' => 'f2ad0683',
'phabricator-transaction-view-css' => '0a101340', 'phabricator-transaction-view-css' => 'ab82f2c0',
'phabricator-zindex-css' => '0a101340', 'phabricator-zindex-css' => 'ab82f2c0',
'phui-button-css' => '0a101340', 'phui-button-css' => 'ab82f2c0',
'phui-form-css' => '0a101340', 'phui-form-css' => 'ab82f2c0',
'phui-icon-view-css' => '0a101340', 'phui-icon-view-css' => 'ab82f2c0',
'phui-spacing-css' => '0a101340', 'phui-spacing-css' => 'ab82f2c0',
'sprite-apps-large-css' => '0a101340', 'sprite-apps-large-css' => 'ab82f2c0',
'sprite-gradient-css' => '0a101340', 'sprite-gradient-css' => 'ab82f2c0',
'sprite-icons-css' => '0a101340', 'sprite-icons-css' => 'ab82f2c0',
'sprite-menu-css' => '0a101340', 'sprite-menu-css' => 'ab82f2c0',
'syntax-highlighting-css' => '0a101340', 'syntax-highlighting-css' => 'ab82f2c0',
), ),
)); ));

View file

@ -819,6 +819,7 @@ phutil_register_library_map(array(
'PhabricatorAuthProvider' => 'applications/auth/provider/PhabricatorAuthProvider.php', 'PhabricatorAuthProvider' => 'applications/auth/provider/PhabricatorAuthProvider.php',
'PhabricatorAuthProviderOAuth' => 'applications/auth/provider/PhabricatorAuthProviderOAuth.php', 'PhabricatorAuthProviderOAuth' => 'applications/auth/provider/PhabricatorAuthProviderOAuth.php',
'PhabricatorAuthProviderOAuthFacebook' => 'applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php', 'PhabricatorAuthProviderOAuthFacebook' => 'applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php',
'PhabricatorAuthProviderPassword' => 'applications/auth/provider/PhabricatorAuthProviderPassword.php',
'PhabricatorAuthRegisterController' => 'applications/auth/controller/PhabricatorAuthRegisterController.php', 'PhabricatorAuthRegisterController' => 'applications/auth/controller/PhabricatorAuthRegisterController.php',
'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php', 'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php',
'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php', 'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php',
@ -2678,6 +2679,7 @@ phutil_register_library_map(array(
'PhabricatorAuthLoginController' => 'PhabricatorAuthController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController',
'PhabricatorAuthProviderOAuth' => 'PhabricatorAuthProvider', 'PhabricatorAuthProviderOAuth' => 'PhabricatorAuthProvider',
'PhabricatorAuthProviderOAuthFacebook' => 'PhabricatorAuthProviderOAuth', 'PhabricatorAuthProviderOAuthFacebook' => 'PhabricatorAuthProviderOAuth',
'PhabricatorAuthProviderPassword' => 'PhabricatorAuthProvider',
'PhabricatorAuthRegisterController' => 'PhabricatorAuthController', 'PhabricatorAuthRegisterController' => 'PhabricatorAuthController',
'PhabricatorAuthStartController' => 'PhabricatorAuthController', 'PhabricatorAuthStartController' => 'PhabricatorAuthController',
'PhabricatorAuthValidateController' => 'PhabricatorAuthController', 'PhabricatorAuthValidateController' => 'PhabricatorAuthController',

View file

@ -37,7 +37,7 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication {
return array( return array(
'/auth/' => array( '/auth/' => array(
'login/(?P<pkey>[^/]+)/' => 'PhabricatorAuthLoginController', 'login/(?P<pkey>[^/]+)/' => 'PhabricatorAuthLoginController',
'register/(?P<akey>[^/]+)/' => 'PhabricatorAuthRegisterController', 'register/(?:(?P<akey>[^/]+)/)?' => 'PhabricatorAuthRegisterController',
'start/' => 'PhabricatorAuthStartController', 'start/' => 'PhabricatorAuthStartController',
'validate/' => 'PhabricatorAuthValidateController', 'validate/' => 'PhabricatorAuthValidateController',
), ),

View file

@ -30,6 +30,11 @@ final class PhabricatorAuthLoginController
return $response; return $response;
} }
if (!$account) {
throw new Exception(
"Auth provider failed to load an account from processLoginRequest()!");
}
if ($account->getUserPHID()) { if ($account->getUserPHID()) {
// The account is already attached to a Phabricator user, so this is // The account is already attached to a Phabricator user, so this is
// either a login or a bad account link request. // either a login or a bad account link request.
@ -170,6 +175,31 @@ final class PhabricatorAuthLoginController
)); ));
} }
public function buildProviderPageResponse(
PhabricatorAuthProvider $provider,
$content) {
$crumbs = $this->buildApplicationCrumbs();
$crumbs->addCrumb(
id(new PhabricatorCrumbView())
->setName(pht('Login'))
->setHref($this->getApplicationURI('start/')));
$crumbs->addCrumb(
id(new PhabricatorCrumbView())
->setName($provider->getProviderName()));
return $this->buildApplicationPage(
array(
$crumbs,
$content,
),
array(
'title' => pht('Login'),
'device' => true,
'dust' => true,
));
}
public function buildProviderErrorResponse( public function buildProviderErrorResponse(
PhabricatorAuthProvider $provider, PhabricatorAuthProvider $provider,
$message) { $message) {

View file

@ -74,6 +74,10 @@ abstract class PhabricatorAuthProvider {
return; return;
} }
protected function willRegisterAccount(PhabricatorExternalAccount $account) {
return;
}
protected function loadOrCreateAccount($account_id) { protected function loadOrCreateAccount($account_id) {
if (!strlen($account_id)) { if (!strlen($account_id)) {
throw new Exception( throw new Exception(
@ -107,7 +111,6 @@ abstract class PhabricatorAuthProvider {
->setAccountID($account_id); ->setAccountID($account_id);
} }
$account->setDisplayName('');
$account->setUsername($adapter->getAccountName()); $account->setUsername($adapter->getAccountName());
$account->setRealName($adapter->getAccountRealName()); $account->setRealName($adapter->getAccountRealName());
$account->setEmail($adapter->getAccountEmail()); $account->setEmail($adapter->getAccountEmail());
@ -147,4 +150,16 @@ abstract class PhabricatorAuthProvider {
return PhabricatorEnv::getURI($uri); return PhabricatorEnv::getURI($uri);
} }
public function isDefaultRegistrationProvider() {
return false;
}
public function shouldRequireRegistrationPassword() {
return false;
}
public function getDefaultExternalAccount() {
throw new Exception("Not implemented!");
}
} }

View file

@ -0,0 +1,231 @@
<?php
final class PhabricatorAuthProviderPassword
extends PhabricatorAuthProvider {
private $adapter;
public function getProviderName() {
return pht('Username/Password');
}
public function isEnabled() {
// TODO: Remove this once we switch to the new auth mechanism.
return false &&
PhabricatorEnv::getEnvConfig('auth.password-auth-enabled');
}
public function getAdapter() {
if (!$this->adapter) {
$adapter = new PhutilAuthAdapterEmpty();
$adapter->setAdapterType('password');
$adapter->setAdapterDomain('self');
$this->adapter = $adapter;
}
return $this->adapter;
}
public function shouldAllowLogin() {
return true;
}
public function shouldAllowRegistration() {
return true;
}
public function shouldAllowAccountLink() {
return false;
}
public function shouldAllowAccountUnlink() {
return false;
}
public function isDefaultRegistrationProvider() {
return true;
}
public function buildLoginForm(
PhabricatorAuthStartController $controller) {
$request = $controller->getRequest();
return $this->renderLoginForm($request);
}
private function renderLoginForm(
AphrontRequest $request,
$require_captcha = false,
$captcha_valid = false) {
$viewer = $request->getUser();
$submit = id(new AphrontFormSubmitControl())
->setValue(pht('Login'));
if ($this->shouldAllowRegistration()) {
$submit->addCancelButton(
'/auth/register/',
pht('Register New Account'));
}
$header = id(new PhabricatorHeaderView())
->setHeader(pht('Login to Phabricator'));
$v_user = nonempty(
$request->getStr('username'),
$request->getCookie('phusr'));
$e_user = null;
$e_pass = null;
$e_captcha = null;
$errors = array();
if ($require_captcha && !$captcha_valid) {
if (AphrontFormRecaptchaControl::hasCaptchaResponse($request)) {
$e_captcha = pht('Invalid');
$errors[] = pht('CAPTCHA was not entered correctly.');
} else {
$e_captcha = pht('Required');
$errors[] = pht('Too many login failures recently. You must '.
'submit a CAPTCHA with your login request.');
}
} else if ($request->isHTTPPost()) {
// NOTE: This is intentionally vague so as not to disclose whether a
// given username or email is registered.
$e_user = pht('Invalid');
$e_pass = pht('Invalid');
$errors[] = pht('Username or password are incorrect.');
}
$form = id(new AphrontFormView())
->setAction($this->getLoginURI())
->setUser($viewer)
->setFlexible(true)
->appendChild(
id(new AphrontFormTextControl())
->setLabel('Username/Email')
->setName('username')
->setValue($v_user)
->setError($e_user))
->appendChild(
id(new AphrontFormPasswordControl())
->setLabel('Password')
->setName('password')
->setError($e_pass)
->setCaption(
phutil_tag(
'a',
array(
'href' => '/login/email/',
),
pht('Forgot your password?'))));
if ($require_captcha) {
$form->appendChild(
id(new AphrontFormRecaptchaControl())
->setError($e_captcha));
}
$form
->appendChild($submit);
if ($errors) {
$errors = id(new AphrontErrorView())->setErrors($errors);
}
return array(
$errors,
$header,
$form,
);
}
public function processLoginRequest(
PhabricatorAuthLoginController $controller) {
$request = $controller->getRequest();
$viewer = $request->getUser();
$require_captcha = false;
$captcha_valid = false;
if (AphrontFormRecaptchaControl::isRecaptchaEnabled()) {
$failed_attempts = PhabricatorUserLog::loadRecentEventsFromThisIP(
PhabricatorUserLog::ACTION_LOGIN_FAILURE,
60 * 15);
if (count($failed_attempts) > 5) {
$require_captcha = true;
$captcha_valid = AphrontFormRecaptchaControl::processCaptcha($request);
}
}
$response = null;
$account = null;
$log_user = null;
if (!$require_captcha || $captcha_valid) {
$username_or_email = $request->getStr('username');
if (strlen($username_or_email)) {
$user = id(new PhabricatorUser())->loadOneWhere(
'username = %s',
$username_or_email);
if (!$user) {
$user = PhabricatorUser::loadOneWithEmailAddress($username_or_email);
}
}
if ($user) {
$envelope = new PhutilOpaqueEnvelope($request->getStr('password'));
if ($user->comparePassword($envelope)) {
$account = $this->loadOrCreateAccount($user->getPHID());
$log_user = $user;
}
}
}
if (!$account) {
$log = PhabricatorUserLog::newLog(
null,
$log_user,
PhabricatorUserLog::ACTION_LOGIN_FAILURE);
$log->save();
$request->clearCookie('phusr');
$request->clearCookie('phsid');
$response = $controller->buildProviderPageResponse(
$this,
$this->renderLoginForm(
$request,
$require_captcha,
$captcha_valid));
}
return array($account, $response);
}
public function shouldRequireRegistrationPassword() {
return true;
}
public function getDefaultExternalAccount() {
$adapter = $this->getAdapter();
return id(new PhabricatorExternalAccount())
->setAccountType($adapter->getAdapterType())
->setAccountDomain($adapter->getAdapterDomain());
}
protected function willSaveAccount(PhabricatorExternalAccount $account) {
parent::willSaveAccount($account);
$account->setUserPHID($account->getAccountID());
}
public function willRegisterAccount(PhabricatorExternalAccount $account) {
parent::willRegisterAccount($account);
$account->setAccountID($account->getUserPHID());
}
}

View file

@ -437,3 +437,7 @@ table.aphront-form-control-checkbox-layout th {
width: 85%; width: 85%;
margin: 15px auto; margin: 15px auto;
} }
.recaptcha_only_if_privacy {
display: none;
}