New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
<?php
|
|
|
|
|
|
|
|
final class PhabricatorAuthRegisterController
|
|
|
|
extends PhabricatorAuthController {
|
|
|
|
|
|
|
|
private $accountKey;
|
|
|
|
|
|
|
|
public function shouldRequireLogin() {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function willProcessRequest(array $data) {
|
|
|
|
$this->accountKey = idx($data, 'akey');
|
|
|
|
}
|
|
|
|
|
|
|
|
public function processRequest() {
|
|
|
|
$request = $this->getRequest();
|
|
|
|
|
|
|
|
if ($request->getUser()->isLoggedIn()) {
|
|
|
|
return $this->renderError(pht('You are already logged in.'));
|
|
|
|
}
|
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
$is_setup = false;
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
if (strlen($this->accountKey)) {
|
2013-06-17 15:12:45 +02:00
|
|
|
$result = $this->loadAccountForRegistrationOrLinking($this->accountKey);
|
|
|
|
list($account, $provider, $response) = $result;
|
2013-06-17 21:14:25 +02:00
|
|
|
$is_default = false;
|
2013-06-20 01:28:48 +02:00
|
|
|
} else if ($this->isFirstTimeSetup()) {
|
|
|
|
list($account, $provider, $response) = $this->loadSetupAccount();
|
|
|
|
$is_default = true;
|
|
|
|
$is_setup = true;
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
} else {
|
2013-06-17 15:12:45 +02:00
|
|
|
list($account, $provider, $response) = $this->loadDefaultAccount();
|
2013-06-17 21:14:25 +02:00
|
|
|
$is_default = true;
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($response) {
|
|
|
|
return $response;
|
|
|
|
}
|
|
|
|
|
2013-06-17 15:12:45 +02:00
|
|
|
if (!$provider->shouldAllowRegistration()) {
|
|
|
|
|
|
|
|
// 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()));
|
|
|
|
}
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
$user = new PhabricatorUser();
|
|
|
|
|
|
|
|
$default_username = $account->getUsername();
|
|
|
|
$default_realname = $account->getRealName();
|
|
|
|
$default_email = $account->getEmail();
|
|
|
|
if ($default_email) {
|
2013-11-12 23:36:49 +01:00
|
|
|
// If the account source provided an email, but it's not allowed by
|
|
|
|
// the configuration, roadblock the user. Previously, we let the user
|
|
|
|
// pick a valid email address instead, but this does not align well with
|
|
|
|
// user expectation and it's not clear the cases it enables are valuable.
|
|
|
|
// See discussion in T3472.
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
if (!PhabricatorUserEmail::isAllowedAddress($default_email)) {
|
2013-11-12 23:36:49 +01:00
|
|
|
return $this->renderError(
|
|
|
|
array(
|
|
|
|
pht(
|
|
|
|
'The account you are attempting to register with has an invalid '.
|
|
|
|
'email address (%s). This Phabricator install only allows '.
|
|
|
|
'registration with specific email addresses:',
|
|
|
|
$default_email),
|
|
|
|
phutil_tag('br'),
|
|
|
|
phutil_tag('br'),
|
|
|
|
PhabricatorUserEmail::describeAllowedAddresses(),
|
|
|
|
));
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
// If the account source provided an email, but another account already
|
|
|
|
// has that email, just pretend we didn't get an email.
|
|
|
|
|
|
|
|
// TODO: See T3340.
|
2013-11-12 23:36:49 +01:00
|
|
|
// TODO: See T3472.
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
if ($default_email) {
|
|
|
|
$same_email = id(new PhabricatorUserEmail())->loadOneWhere(
|
|
|
|
'address = %s',
|
|
|
|
$default_email);
|
|
|
|
if ($same_email) {
|
|
|
|
$default_email = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$profile = id(new PhabricatorRegistrationProfile())
|
|
|
|
->setDefaultUsername($default_username)
|
|
|
|
->setDefaultEmail($default_email)
|
|
|
|
->setDefaultRealName($default_realname)
|
|
|
|
->setCanEditUsername(true)
|
|
|
|
->setCanEditEmail(($default_email === null))
|
|
|
|
->setCanEditRealName(true)
|
|
|
|
->setShouldVerifyEmail(false);
|
|
|
|
|
|
|
|
$event_type = PhabricatorEventType::TYPE_AUTH_WILLREGISTERUSER;
|
|
|
|
$event_data = array(
|
|
|
|
'account' => $account,
|
|
|
|
'profile' => $profile,
|
|
|
|
);
|
|
|
|
|
|
|
|
$event = id(new PhabricatorEvent($event_type, $event_data))
|
|
|
|
->setUser($user);
|
|
|
|
PhutilEventEngine::dispatchEvent($event);
|
|
|
|
|
|
|
|
$default_username = $profile->getDefaultUsername();
|
|
|
|
$default_email = $profile->getDefaultEmail();
|
|
|
|
$default_realname = $profile->getDefaultRealName();
|
|
|
|
|
|
|
|
$can_edit_username = $profile->getCanEditUsername();
|
|
|
|
$can_edit_email = $profile->getCanEditEmail();
|
|
|
|
$can_edit_realname = $profile->getCanEditRealName();
|
|
|
|
|
2013-06-17 15:12:45 +02:00
|
|
|
$must_set_password = $provider->shouldRequireRegistrationPassword();
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
$can_edit_anything = $profile->getCanEditAnything() || $must_set_password;
|
|
|
|
$force_verify = $profile->getShouldVerifyEmail();
|
|
|
|
|
|
|
|
$value_username = $default_username;
|
|
|
|
$value_realname = $default_realname;
|
|
|
|
$value_email = $default_email;
|
|
|
|
$value_password = null;
|
|
|
|
|
|
|
|
$errors = array();
|
|
|
|
|
|
|
|
$e_username = strlen($value_username) ? null : true;
|
|
|
|
$e_realname = strlen($value_realname) ? null : true;
|
|
|
|
$e_email = strlen($value_email) ? null : true;
|
|
|
|
$e_password = true;
|
2013-09-20 23:54:57 +02:00
|
|
|
$e_captcha = true;
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
$min_len = PhabricatorEnv::getEnvConfig('account.minimum-password-length');
|
|
|
|
$min_len = (int)$min_len;
|
|
|
|
|
|
|
|
if ($request->isFormPost() || !$can_edit_anything) {
|
|
|
|
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
|
|
|
|
2013-09-20 23:54:57 +02:00
|
|
|
if ($must_set_password) {
|
|
|
|
$e_captcha = pht('Again');
|
|
|
|
|
|
|
|
$captcha_ok = AphrontFormRecaptchaControl::processCaptcha($request);
|
|
|
|
if (!$captcha_ok) {
|
|
|
|
$errors[] = pht("Captcha response is incorrect, try again.");
|
|
|
|
$e_captcha = pht('Invalid');
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
if ($can_edit_username) {
|
|
|
|
$value_username = $request->getStr('username');
|
|
|
|
if (!strlen($value_username)) {
|
|
|
|
$e_username = pht('Required');
|
|
|
|
$errors[] = pht('Username is required.');
|
|
|
|
} else if (!PhabricatorUser::validateUsername($value_username)) {
|
|
|
|
$e_username = pht('Invalid');
|
|
|
|
$errors[] = PhabricatorUser::describeValidUsername();
|
|
|
|
} else {
|
|
|
|
$e_username = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($must_set_password) {
|
|
|
|
$value_password = $request->getStr('password');
|
|
|
|
$value_confirm = $request->getStr('confirm');
|
|
|
|
if (!strlen($value_password)) {
|
|
|
|
$e_password = pht('Required');
|
|
|
|
$errors[] = pht('You must choose a password.');
|
|
|
|
} else if ($value_password !== $value_confirm) {
|
|
|
|
$e_password = pht('No Match');
|
|
|
|
$errors[] = pht('Password and confirmation must match.');
|
|
|
|
} else if (strlen($value_password) < $min_len) {
|
|
|
|
$e_password = pht('Too Short');
|
|
|
|
$errors[] = pht(
|
|
|
|
'Password is too short (must be at least %d characters long).',
|
|
|
|
$min_len);
|
2014-01-23 23:01:18 +01:00
|
|
|
} else if (
|
|
|
|
PhabricatorCommonPasswords::isCommonPassword($value_password)) {
|
|
|
|
|
|
|
|
$e_password = pht('Very Weak');
|
|
|
|
$errors[] = pht(
|
|
|
|
'Password is pathologically weak. This password is one of the '.
|
|
|
|
'most common passwords in use, and is extremely easy for '.
|
|
|
|
'attackers to guess. You must choose a stronger password.');
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
} else {
|
|
|
|
$e_password = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($can_edit_email) {
|
|
|
|
$value_email = $request->getStr('email');
|
|
|
|
if (!strlen($value_email)) {
|
|
|
|
$e_email = pht('Required');
|
|
|
|
$errors[] = pht('Email is required.');
|
|
|
|
} else if (!PhabricatorUserEmail::isAllowedAddress($value_email)) {
|
|
|
|
$e_email = pht('Invalid');
|
|
|
|
$errors[] = PhabricatorUserEmail::describeAllowedAddresses();
|
|
|
|
} else {
|
|
|
|
$e_email = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($can_edit_realname) {
|
|
|
|
$value_realname = $request->getStr('realName');
|
|
|
|
if (!strlen($value_realname)) {
|
|
|
|
$e_realname = pht('Required');
|
|
|
|
$errors[] = pht('Real name is required.');
|
|
|
|
} else {
|
|
|
|
$e_realname = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$errors) {
|
|
|
|
$image = $this->loadProfilePicture($account);
|
|
|
|
if ($image) {
|
|
|
|
$user->setProfileImagePHID($image->getPHID());
|
|
|
|
}
|
|
|
|
|
|
|
|
try {
|
|
|
|
if ($force_verify) {
|
|
|
|
$verify_email = true;
|
|
|
|
} else {
|
|
|
|
$verify_email =
|
|
|
|
($account->getEmailVerified()) &&
|
|
|
|
($value_email === $default_email);
|
|
|
|
}
|
|
|
|
|
|
|
|
$email_obj = id(new PhabricatorUserEmail())
|
|
|
|
->setAddress($value_email)
|
|
|
|
->setIsVerified((int)$verify_email);
|
|
|
|
|
|
|
|
$user->setUsername($value_username);
|
|
|
|
$user->setRealname($value_realname);
|
|
|
|
|
2013-11-13 20:24:56 +01:00
|
|
|
if ($is_setup) {
|
|
|
|
$must_approve = false;
|
|
|
|
} else {
|
|
|
|
$must_approve = PhabricatorEnv::getEnvConfig(
|
|
|
|
'auth.require-approval');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($must_approve) {
|
|
|
|
$user->setIsApproved(0);
|
|
|
|
} else {
|
|
|
|
$user->setIsApproved(1);
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
$user->openTransaction();
|
|
|
|
|
|
|
|
$editor = id(new PhabricatorUserEditor())
|
|
|
|
->setActor($user);
|
|
|
|
|
|
|
|
$editor->createNewUser($user, $email_obj);
|
|
|
|
if ($must_set_password) {
|
|
|
|
$envelope = new PhutilOpaqueEnvelope($value_password);
|
|
|
|
$editor->changePassword($user, $envelope);
|
|
|
|
}
|
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
if ($is_setup) {
|
|
|
|
$editor->makeAdminUser($user, true);
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
$account->setUserPHID($user->getPHID());
|
2013-06-17 15:12:45 +02:00
|
|
|
$provider->willRegisterAccount($account);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
$account->save();
|
|
|
|
|
|
|
|
$user->saveTransaction();
|
|
|
|
|
|
|
|
if (!$email_obj->getIsVerified()) {
|
|
|
|
$email_obj->sendVerificationEmail($user);
|
|
|
|
}
|
|
|
|
|
2013-11-13 20:24:56 +01:00
|
|
|
if ($must_approve) {
|
|
|
|
$this->sendWaitingForApprovalEmail($user);
|
|
|
|
}
|
|
|
|
|
2013-06-17 01:35:36 +02:00
|
|
|
return $this->loginUser($user);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
} catch (AphrontQueryDuplicateKeyException $exception) {
|
|
|
|
$same_username = id(new PhabricatorUser())->loadOneWhere(
|
|
|
|
'userName = %s',
|
|
|
|
$user->getUserName());
|
|
|
|
|
|
|
|
$same_email = id(new PhabricatorUserEmail())->loadOneWhere(
|
|
|
|
'address = %s',
|
|
|
|
$value_email);
|
|
|
|
|
|
|
|
if ($same_username) {
|
|
|
|
$e_username = pht('Duplicate');
|
|
|
|
$errors[] = pht('Another user already has that username.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($same_email) {
|
|
|
|
// TODO: See T3340.
|
|
|
|
$e_email = pht('Duplicate');
|
|
|
|
$errors[] = pht('Another user already has that email.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$same_username && !$same_email) {
|
|
|
|
throw $exception;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
unset($unguarded);
|
|
|
|
}
|
|
|
|
|
|
|
|
$form = id(new AphrontFormView())
|
2013-06-17 21:14:25 +02:00
|
|
|
->setUser($request->getUser());
|
|
|
|
|
|
|
|
if (!$is_default) {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormMarkupControl())
|
|
|
|
->setLabel(pht('External Account'))
|
|
|
|
->setValue(
|
|
|
|
id(new PhabricatorAuthAccountView())
|
|
|
|
->setUser($request->getUser())
|
|
|
|
->setExternalAccount($account)
|
|
|
|
->setAuthProvider($provider)));
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2013-11-13 20:25:43 +01:00
|
|
|
if ($can_edit_username) {
|
|
|
|
$form->appendChild(
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
id(new AphrontFormTextControl())
|
2013-06-17 21:14:25 +02:00
|
|
|
->setLabel(pht('Phabricator Username'))
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
->setName('username')
|
|
|
|
->setValue($value_username)
|
|
|
|
->setError($e_username));
|
2013-11-13 20:25:43 +01:00
|
|
|
} else {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormMarkupControl())
|
|
|
|
->setLabel(pht('Phabricator Username'))
|
|
|
|
->setValue($value_username)
|
|
|
|
->setError($e_username));
|
|
|
|
}
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
if ($must_set_password) {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormPasswordControl())
|
|
|
|
->setLabel(pht('Password'))
|
|
|
|
->setName('password')
|
|
|
|
->setError($e_password)
|
|
|
|
->setCaption(
|
|
|
|
$min_len
|
|
|
|
? pht('Minimum length of %d characters.', $min_len)
|
|
|
|
: null));
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormPasswordControl())
|
|
|
|
->setLabel(pht('Confirm Password'))
|
|
|
|
->setName('confirm')
|
|
|
|
->setError($e_password));
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($can_edit_email) {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormTextControl())
|
|
|
|
->setLabel(pht('Email'))
|
|
|
|
->setName('email')
|
|
|
|
->setValue($value_email)
|
|
|
|
->setCaption(PhabricatorUserEmail::describeAllowedAddresses())
|
|
|
|
->setError($e_email));
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($can_edit_realname) {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormTextControl())
|
|
|
|
->setLabel(pht('Real Name'))
|
|
|
|
->setName('realName')
|
|
|
|
->setValue($value_realname)
|
|
|
|
->setError($e_realname));
|
|
|
|
}
|
|
|
|
|
2013-09-20 23:54:57 +02:00
|
|
|
if ($must_set_password) {
|
|
|
|
$form->appendChild(
|
|
|
|
id(new AphrontFormRecaptchaControl())
|
2013-10-05 00:13:05 +02:00
|
|
|
->setLabel(pht('Captcha'))
|
2013-09-20 23:54:57 +02:00
|
|
|
->setError($e_captcha));
|
|
|
|
}
|
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
$submit = id(new AphrontFormSubmitControl());
|
|
|
|
|
|
|
|
if ($is_setup) {
|
|
|
|
$submit
|
|
|
|
->setValue(pht('Create Admin Account'));
|
|
|
|
} else {
|
|
|
|
$submit
|
2013-06-17 21:14:25 +02:00
|
|
|
->addCancelButton($this->getApplicationURI('start/'))
|
2013-06-20 01:28:48 +02:00
|
|
|
->setValue(pht('Register Phabricator Account'));
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
$form->appendChild($submit);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
|
|
|
$crumbs = $this->buildApplicationCrumbs();
|
2013-06-20 01:28:48 +02:00
|
|
|
|
|
|
|
if ($is_setup) {
|
2013-12-19 02:47:34 +01:00
|
|
|
$crumbs->addTextCrumb(pht('Setup Admin Account'));
|
2013-06-20 01:28:48 +02:00
|
|
|
$title = pht('Welcome to Phabricator');
|
|
|
|
} else {
|
2013-12-19 02:47:34 +01:00
|
|
|
$crumbs->addTextCrumb(pht('Register'));
|
|
|
|
$crumbs->addTextCrumb($provider->getProviderName());
|
2013-06-20 01:28:48 +02:00
|
|
|
$title = pht('Phabricator Registration');
|
|
|
|
}
|
|
|
|
|
|
|
|
$welcome_view = null;
|
|
|
|
if ($is_setup) {
|
|
|
|
$welcome_view = id(new AphrontErrorView())
|
|
|
|
->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
|
|
|
|
->setTitle(pht('Welcome to Phabricator'))
|
|
|
|
->appendChild(
|
|
|
|
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.'));
|
|
|
|
}
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
|
2013-10-05 00:13:05 +02:00
|
|
|
$object_box = id(new PHUIObjectBoxView())
|
|
|
|
->setHeaderText($title)
|
|
|
|
->setForm($form)
|
2014-01-10 18:17:37 +01:00
|
|
|
->setFormErrors($errors);
|
2013-10-05 00:13:05 +02:00
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
return $this->buildApplicationPage(
|
|
|
|
array(
|
|
|
|
$crumbs,
|
2013-06-20 01:28:48 +02:00
|
|
|
$welcome_view,
|
2013-10-05 00:13:05 +02:00
|
|
|
$object_box,
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
),
|
|
|
|
array(
|
|
|
|
'title' => $title,
|
|
|
|
'device' => true,
|
|
|
|
));
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadDefaultAccount() {
|
|
|
|
$providers = PhabricatorAuthProvider::getAllEnabledProviders();
|
2013-06-17 15:12:45 +02:00
|
|
|
$account = null;
|
|
|
|
$provider = null;
|
|
|
|
$response = null;
|
|
|
|
|
|
|
|
foreach ($providers as $key => $candidate_provider) {
|
|
|
|
if (!$candidate_provider->shouldAllowRegistration()) {
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
unset($providers[$key]);
|
|
|
|
continue;
|
|
|
|
}
|
2013-06-17 15:12:45 +02:00
|
|
|
if (!$candidate_provider->isDefaultRegistrationProvider()) {
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
unset($providers[$key]);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$providers) {
|
2013-06-17 15:12:45 +02:00
|
|
|
$response = $this->renderError(
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
pht(
|
|
|
|
"There are no configured default registration providers."));
|
2013-06-17 15:12:45 +02:00
|
|
|
return array($account, $provider, $response);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
} else if (count($providers) > 1) {
|
2013-06-17 15:12:45 +02:00
|
|
|
$response = $this->renderError(
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
pht(
|
|
|
|
"There are too many configured default registration providers."));
|
2013-06-17 15:12:45 +02:00
|
|
|
return array($account, $provider, $response);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
}
|
|
|
|
|
2013-06-17 15:12:45 +02:00
|
|
|
$provider = head($providers);
|
|
|
|
$account = $provider->getDefaultExternalAccount();
|
|
|
|
|
|
|
|
return array($account, $provider, $response);
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
}
|
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
private function loadSetupAccount() {
|
|
|
|
$provider = new PhabricatorAuthProviderPassword();
|
2013-10-05 17:02:41 +02:00
|
|
|
$provider->attachProviderConfig(
|
|
|
|
id(new PhabricatorAuthProviderConfig())
|
|
|
|
->setShouldAllowRegistration(1)
|
|
|
|
->setShouldAllowLogin(1)
|
|
|
|
->setIsEnabled(true));
|
|
|
|
|
2013-06-20 01:28:48 +02:00
|
|
|
$account = $provider->getDefaultExternalAccount();
|
|
|
|
$response = null;
|
|
|
|
return array($account, $provider, $response);
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
private function loadProfilePicture(PhabricatorExternalAccount $account) {
|
|
|
|
$phid = $account->getProfileImagePHID();
|
|
|
|
if (!$phid) {
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: Use of omnipotent user is okay here because the registering user
|
|
|
|
// can not control the field value, and we can't use their user object to
|
|
|
|
// do meaningful policy checks anyway since they have not registered yet.
|
|
|
|
// Reaching this means the user holds the account secret key and the
|
|
|
|
// registration secret key, and thus has permission to view the image.
|
|
|
|
|
|
|
|
$file = id(new PhabricatorFileQuery())
|
|
|
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
|
|
|
->withPHIDs(array($phid))
|
|
|
|
->executeOne();
|
|
|
|
if (!$file) {
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
|
|
|
try {
|
|
|
|
$xformer = new PhabricatorImageTransformer();
|
|
|
|
return $xformer->executeProfileTransform(
|
|
|
|
$file,
|
|
|
|
$width = 50,
|
|
|
|
$min_height = 50,
|
|
|
|
$max_height = 50);
|
|
|
|
} catch (Exception $ex) {
|
|
|
|
phlog($ex);
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2013-06-17 01:35:36 +02:00
|
|
|
protected function renderError($message) {
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
return $this->renderErrorPage(
|
|
|
|
pht('Registration Failed'),
|
|
|
|
array($message));
|
|
|
|
}
|
|
|
|
|
2013-11-13 20:24:56 +01:00
|
|
|
private function sendWaitingForApprovalEmail(PhabricatorUser $user) {
|
|
|
|
$title = '[Phabricator] '.pht(
|
|
|
|
'New User "%s" Awaiting Approval',
|
|
|
|
$user->getUsername());
|
|
|
|
|
|
|
|
$body = new PhabricatorMetaMTAMailBody();
|
|
|
|
|
|
|
|
$body->addRawSection(
|
|
|
|
pht(
|
|
|
|
'Newly registered user "%s" is awaiting account approval by an '.
|
|
|
|
'administrator.',
|
|
|
|
$user->getUsername()));
|
|
|
|
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('APPROVAL QUEUE'),
|
|
|
|
PhabricatorEnv::getProductionURI(
|
|
|
|
'/people/query/approval/'));
|
|
|
|
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('DISABLE APPROVAL QUEUE'),
|
|
|
|
PhabricatorEnv::getProductionURI(
|
|
|
|
'/config/edit/auth.require-approval/'));
|
|
|
|
|
|
|
|
$admins = id(new PhabricatorPeopleQuery())
|
|
|
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
|
|
|
->withIsAdmin(true)
|
|
|
|
->execute();
|
|
|
|
|
|
|
|
if (!$admins) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$mail = id(new PhabricatorMetaMTAMail())
|
|
|
|
->addTos(mpull($admins, 'getPHID'))
|
|
|
|
->setSubject($title)
|
|
|
|
->setBody($body->render())
|
|
|
|
->saveAndSend();
|
|
|
|
}
|
|
|
|
|
New Registration Workflow
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 19:13:49 +02:00
|
|
|
}
|