Page MenuHomePhabricator

Add configuration parameter for username normalization to OpenID Connect
Open, Needs TriagePublicFeature

Description

We are planning to transfer from LDAP authentication to OpenIDConnect, using $wgOpenIDConnect_MigrateUsersByUserName=true.

In our current LDAP setup, we make use of $LDAPAuthentication2UsernameNormalizer='strtolower'. This means in the past 'MyUser' in LDAP became 'Myuser' on MediaWiki.

When 'MyUser' logs into the wiki using OpenIDConnect, a new user 'MyUser' is created, so the migration fails.

When I add a line $preferred_username = strtolower( $preferred_username ); just before this line, the migration succeeds.

Would you consider implementing a similar normalizer feature in the OpenIDConnect extension?

Event Timeline

cicalese added a subscriber: Osnard.

Thank you for the feedback. I've merged an earlier task I had created with a similar request. I'm tagging this task with MediaWiki-extensions-Pluggable-Auth, because I think it makes sense to have the functionality there so it can be shared by the different plugins. Tagging @Osnard for his thoughts.

Yes, I have the impression that some "normalizer" mechanism makes sense for most plugins. I believe we should start with three implementations:

  • RAW -> Use username as it comes from the authentication provider
  • LOWERCASE -> Apply strtolower
  • CUSTOMCALLBACK -> This can come in handy in case one wants to apply some transformation (like stripping the @domainpart of an e-mail address)

This should go to Extension:PluggableAuth

cicalese renamed this task from Add configuration parameter $wgOpenIDConnect_UsernameNormalizer to Add configuration parameter for username normalization to PluggableAuth.Mar 14 2024, 2:47 AM

I did some investigations and examined three plugins:

  • SimpleSAMLphp
  • LDAPAuthentication2
  • OpenIDConnect

SimpleSAMLphp

  • Has a system of "user info providers", which are set for each "user info" (username, real name, mail)
  • By default it lowercases the username coming from the SAML IdP
  • This can be configured
  • There is a rawusername but also a GenericCallback
  • The "user info provider" has access to all attributes provided by the IdP
  • The username is nomalized in MediaWiki\Extension\SimpleSAMLphp\SimpleSAMLphp::authenticate, before checking if there such a user in the DB
  • This can lead to cases where there are users "MyCoolUser" (created _before_ the introduction of SAML authn) and "Mycooluser" (created afterwards)
  • There is no "migration" setting
$wgPluggableAuth_Config['Log in using my SAML'] = [
	'plugin' => 'SimpleSAMLphp',
	'data' => [
		// ...
		'userinfoProviders' => [
			'username' => 'rawusername'
		]
	]
];

from

$wgSimpleSAMLphp_MandatoryUserInfoProviders['myusername'] = [
	'factory' => function() {
		return new \MediaWiki\Extension\SimpleSAMLphp\UserInfoProvider\GenericCallback( function( $attributes ) {
			$mailAttribute = 'mail';
			if ( !isset( $attributes[$mailAttribute] ) ) {
				throw new Exception( 'missing email address' );
			}
			$parts = explode( '@', $attributes[$mailAttribute][0] );
			return strtolower( $parts[0] );
		} );
	}
];

from https://www.mediawiki.org/wiki/Extension:SimpleSAMLphp#Define_custom_user_info_provider

LDAPAuthentication2

  • Only provides a config variable $LDAPAuthentication2UsernameNormalizer that can be set to any type of callable
  • By default it will user the raw username provided by the LDAP server
  • It only retrieves the "username" itself and does not have any context information (e.g. mail, real name, groups, ...)
    • If the usernameattribute is set to something else than sAMAccountName, additional information may be available
  • The normalization is applied in MediaWiki\Extension\LDAPAuthentication2\PluggableAuth::authenticate, before checking if there such a user in the DB
  • This can lead to cases where there are users "MyCoolUser" (created _before_ the introduction of LDAP authn) and "Mycooluser" (created afterwards)

OpenIDConnect

  • Does not have a username normalization at the moment
  • Connects username with issuer and subject in a dedicated database
  • Allows to define the "preferred username", from a particular claim, the real name or the email (domain will be stripped)
  • Allows automatic migrations based on the
    • preferred username
    • mail address
  • In case that no username could be found, it will create a username by pattern (User1, User2, ...)

Conclusion:

So, if we wanted to unify this as part of "PluggableAuth", we probably need to post process the values for "username", "realname" and "email" coming from PluggableAuthPlugin::authenticate. This means that any work already performed within the method itself may be overwritten. I'd still propose the following changes:

  1. Build a "modifier" registry (ObjectFactory-Specs):
json

    "attributes": {
        "PluggableAuth": {
            "UserAttributeModifierRegistry": {
                "null": {
                   "class": "..."

We should have

  • null -> No modification (basically like "raw")
  • lowercase
  • callback
  1. Set defaults for the various attributes
json
    "config": {
        "PluggableAuth": {
            "UserAttributeModifiers": {
                "value": {
                    "username":  "lowercase",
                    "realname":  "null",
                    "mail":  "null",
...
             "UserAttributeCallback": {
                "value": ""

(EDIT: As I am thinking about, this probably needs to be part of $wgPluggableAuth_Config[...]['data'])

  1. Define an interface for the modifiers
public function modify( $attributeName, $currentValue, $allAttributes ): string;

This would allow users to just "post process" things coming from the respective plugin.

Thank you for the detailed analysis, @Osnard! It agrees with my analysis of the issue.

There is one additional item to mention. Since OpenID Connect uses the returned subject and issuer to connect an identity to an existing user and also supports migration checks (usurpation of existing users by email address or real name), the username should not be modified if authenticate() returns a non-zero $id. And, modification of the email address and real name would have to be done prior to that migration check, so post-processing would be too late. Further, for post-processing of usernames to be useful for LDAPAuthentication2 and SimpleSAMLphp, we would want to remove the current processing from those plugins and move it to the post-processing step and only check for existence of the user after the post-processing is complete. In that case, those two plugins would always return zero for $id.

My conclusion is that with the current implementation, introducing generic post processing would introduce complexity and potential for errors without a clear benefit.

An alternative would be a reimplementation of the authentication driver code to separate out the authentication decision (has the principal trying to authenticate proven their identity?) from matching the principal to an existing user in the database, with the potential modification code based on the attributes returned in between. My current feeling is that the costs (time for reimplementation, time for end users to adapt to a new configuration mechanism, potential for introduction of bugs) outweigh the benefits of this activity.

My conclusion is that with the current implementation, introducing generic post processing would introduce complexity and potential for errors without a clear benefit.

[...] My current feeling is that the costs (time for reimplementation, time for end users to adapt to a new configuration mechanism, potential for introduction of bugs) outweigh the benefits of this activity.

I agree. Adding a setting to Extension:OpenIDConnect is probably the most reasonable decision at the moment.

cicalese renamed this task from Add configuration parameter for username normalization to PluggableAuth to Add configuration parameter for username normalization to OpenID Connect.Mar 20 2024, 1:44 PM

Change #1013534 had a related patch set uploaded (by Cicalese; author: Cicalese):

[mediawiki/extensions/OpenIDConnect@master] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1013534

Change #1013534 merged by jenkins-bot:

[mediawiki/extensions/OpenIDConnect@master] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1013534

Change #1017134 had a related patch set uploaded (by Cicalese; author: Cicalese):

[mediawiki/extensions/OpenIDConnect@REL1_39] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1017134

Change #1017135 had a related patch set uploaded (by Cicalese; author: Cicalese):

[mediawiki/extensions/OpenIDConnect@REL1_41] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1017135

Change #1017135 merged by jenkins-bot:

[mediawiki/extensions/OpenIDConnect@REL1_41] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1017135

Change #1017134 merged by jenkins-bot:

[mediawiki/extensions/OpenIDConnect@REL1_39] Add processor functions and random username generator

https://gerrit.wikimedia.org/r/1017134