Disable a Username change for the Registration via a Provider
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	funkhousr
	Mar 31 2016, 1:23 PM

Description

Hey there,

first of all, thanks for Phabricator - we love it!

We are currently setting it up for our University students to collaborate, contribute, etc.
Students log in via an LDAP Provider, which works like a charm.

Root Problem: However, we face a minor issue. When a student registers for the first time - and hence the phabricator account is created - she can select her user and real name. Both names are prefilled from the LDAP/provider perfectly fine, however they are changeable.
This eventually leads to students choosing funny names, so we have a hard time finding who John Snow really is (Who knows that anyways).

After searching for some configurable options we did not find I gave the source code a look and found a very simple solution. The PhabricatorAuthRegisterController on the first login creates a new PhabricatorRegistrationProfile at line 136:

$profile = id(new PhabricatorRegistrationProfile())
  ->setDefaultUsername($default_username)
  ->setDefaultEmail($default_email)
  ->setDefaultRealName($default_realname)
  ->setCanEditUsername(true)
  ->setCanEditEmail(($default_email === null))
  ->setCanEditRealName(true)
  ->setShouldVerifyEmail(false);

There, both setCanEditUsername and setCanEditRealName are statically set to true. If we change both values to false, on login with a provider a new Phabricator account is automatically created with the same Username and RealName as the provider suggest which is is the exact desired behavior for us.

It would be absolutely lovely to have this configurable. Did we miss this option?

If not I would offer to implement it with your suggestions.

Related Objects

Mentioned In: D20110: Remove the highly suspect "Import from LDAP" workflow
Mentioned Here: T8227: Why not just add an option/setting/preference?

Event Timeline

funkhousr created this task.Mar 31 2016, 1:23 PM

You can register an event listener for the TYPE_AUTH_WILLREGISTERUSER event. We are generally moving away from this system, but it works today and I don't have any near-term plans to replace it in this case. This is the documentation:

https://secure.phabricator.com/book/phabricator/article/events/

The body of your listener would be something like:

$profile = $event->getValue('profile');

$profile->setCanEditUsername(false);
$profile->setCanEditRealName(false);

This may not really be worth doing since the cost to keep the hard-coded false patches might be smaller than dealing with this.

I don't want to add options for this because this use case is rare, and needs here vary wildly across installs (which is why we have an event listener in the first place). For example, some installs in the past have wanted to lock these options for specific providers but not other providers, or force the username to a specific value other than the provider default in complex conditions. No reasonable set of config values could accommodate all of these needs. See also T8227.

Thanks @epriestley, it makes sense what you say.

I think we will just stick with the 'hard coding' of these values, then. Looks manageable.

Thanks for you elaborate help!

Sounds good. Let us know if you run into anything else.

@jcarrillo7 see T10700#167773 for solution.

@chad is this really a solution? The events documentation clearly states

"The event system is an artifact of a bygone era. Use of the event system is strongly discouraged. We have been removing events since 2013 and will continue to remove events in the future."

It's a solution you can have in production today, at no cost. Evan noted above we have no immediate plans to remove/replace.

In T10700#196511, @chad wrote:

It's a solution you can have in production today, at no cost. Evan noted above we have no immediate plans to remove/replace.

What happens if/when you guys remove this event? Will phabricator warn on a bad library? Will it be called out in the changelog?

We always do give warnings, yes. I would recommend subscribing to the weekly Changelog.

https://secure.phabricator.com/w/changelog/

It's years away if that, and we wouldn't remove an event handler for registration without significant architecture to move it to.

Thanks for the info!

epriestley mentioned this in D20110: Remove the highly suspect "Import from LDAP" workflow.Feb 7 2019, 8:12 PM

Disable a Username change for the Registration via a ProviderClosed, ResolvedPublicActions

Description

Related Objects

Event Timeline

Disable a Username change for the Registration via a Provider
Closed, ResolvedPublic
Actions