Title::makeTitleSafe() already does that.

But its result is not used. Please note that the result of calling Title::makeTitleSafe() is stored into the variable $nt, but returned is the variable $preferred_username which holds the unmodified string without capitalized first letter.

Probably this bug does not manifest in Wikimedia accounts where usernames are already capitalized, but my OIDC server returns simple unix login names in preferred_name, so mine is "makub" and not "Makub", thus the login fails .

Thanks for catching that. I will submit a patch. Do you have access to gerrit to test and +1 the patch before I merge it?

I do not have access to gerrit, but I have spent several hours today searching why login through OIDC is failing, and I have found that if I make this one-line change, it starts working. So you may consider the change to be already tested on my system.

OK, I did some testing with this. The current code works for me in a similar situation. I use the email address as the source of the preferred username. I have tested with lowercase email addresses, which result in a lowercase preferred_username. That preferred_username gets correctly converted to have its first letter uppercased when the user is created. I'm curious why that conversion is not happening for you. What software versions are you using and what configuration? I'm not averse to the requested change, but I want to be sure that we are not masking another problem.

I am using MediaWiki 1.34.1 (the latest in this moment) with OpenIDConnect extension downloaded from https://www.mediawiki.org/wiki/Special:ExtensionDistributor/OpenIDConnect which contains file "version" with the following content:

OpenIDConnect: REL1_34
2019-12-19T15:33:38
1db264d

It all runs on Debian 10 with PHP-FPM version 7.3 (7.3.14-1~deb10u1).

I am curious how would the call

$nt = Title::makeTitleSafe( NS_USER, $preferred_username )

modify the value in $preferred_username, when the argument is passed by value and not by reference (as far as I understand PHP).

In T249630#6041554, @Makub wrote:

I am curious how would the call

$nt = Title::makeTitleSafe( NS_USER, $preferred_username )

modify the value in $preferred_username, when the argument is passed by value and not by reference (as far as I understand PHP).

It does not. What you said above is correct, actually the $nt is only used to test the nullness of the $prefered_ username and bailout (the next 2 lines) if the null is true.

Yes, agreed. the call to Title::makeTitleSafe was only intended to check if the preferred username could be used as a valid username. It was not intended to perform any lasting modification to the value. The conversion of the first character of the username, if necessary, is done later when the user is created. It is not clear to me why that is not happening successfully for you. What other configuration are you doing for PluggableAuth and OpenIDConnect?

Maybe the important thing in my setup is that I am not creating new users, I am migrating old users by name, I have

$wgOpenIDConnect_MigrateUsersByUserName = true;

so the original non-capitalized preferred_username is used for searching for the old user.

But getMigratedIdByUserName() already calls Title::makeTitleSafe() which will result in valid username;

private static function getMigratedIdByUserName( $username ) {
   $nt = Title::makeTitleSafe( NS_USER, $username );
	if ( $nt === null ) {
	   wfDebugLog( 'OpenID Connect',
	     'Invalid preferred username for migration: ' . $username . '.' . PHP_EOL );
	    return null;
	}
   $username = $nt->getText();  // Valid username

However the valid username is not being used when the function is finally called:

// The $preferred_username here can be invalid. getPreferredUsername() will just return it provided it's not null.
$preferred_username = $this->getPreferredUsername( $config, $oidc, $realname, $email );
    wfDebugLog( 'OpenID Connect', 'Preferred username: ' . $preferred_username . PHP_EOL ); 

// $wgOpenIDConnect_MigrateUsersByUserName = true; setting
if ( $GLOBALS['wgOpenIDConnect_MigrateUsersByUserName'] === true ) {
     wfDebugLog( 'OpenID Connect', 'Checking for username migration.' . PHP_EOL );
  // It will then be used as argument for getMigratedIdByUserName() below
  $id = $this->getMigratedIdByUserName( $preferred_username );
  if ( $id !== null ) {
     $this->saveExtraAttributes( $id );
        wfDebugLog( 'OpenID Connect', 'Migrated user by username: ' . $preferred_username . '.' . PHP_EOL );
     $username = $preferred_username; // And we still retain it as the username provided we found the id
     return true;
  }
}

So the solution is to make sure getPreferredUsername() return only valid username by calling Title::makeTitleSafe() on $preferred_username even if it's not null.

Change 587903 had a related patch set uploaded (by Cicalese; owner: Cicalese):
[mediawiki/extensions/OpenIDConnect@master] Fixed bug with migrated initial lowercase usernames

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

Change 587903 merged by jenkins-bot:
[mediawiki/extensions/OpenIDConnect@master] Fixed bug with migrated initial lowercase usernames

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