Homec4science

New Registration Workflow

Authored by epriestley <git@epriestley.com> on Jun 16 2013, 19:13.

Description

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

Details

Committed
epriestley <git@epriestley.com>Jun 16 2013, 19:13
Pushed
aubortJan 31 2017, 17:16
Parents
rPH8744cdb69981: Migrate PhabricatorUserLDAPInfo to PhabricatorExternalAccount
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHdb1cf41ec425: New Registration Workflow (authored by epriestley <git@epriestley.com>).Jun 16 2013, 19:13