Topic on User talk:Cindy.cicalese

Hi! I'm developing a new auth plugin (custom protocol) using PluggableAuth 7.1.0 on MediaWiki 1.41.1.

In method authenticate() if Wiki have no user in DB, the method returns $username, etc. It works. But for existing user I return the $id only which cause the plugin PluggableAuth to throw an Exception.

TypeError: MediaWiki\User\UserFactory::newFromName(): Argument #1 ($name) must be of type string, null given, called in /var/www/html/extensions/PluggableAuth/includes/PrimaryAuthenticationProvider.php on line 161

Why returning $username is mandatory for the existing user? I think there's no reason for it.

The root cause is in PluggableAuthLogin.php lines 124-126

$this->authManager->setAuthenticationSessionData( self::USERNAME_SESSION_KEY, $username );

$this->authManager->setAuthenticationSessionData( self::REALNAME_SESSION_KEY, $realname );

$this->authManager->setAuthenticationSessionData( self::EMAIL_SESSION_KEY, $email );

You're picking the wrong data here returned from authenticate() instead of already fetched by id $user.

The fix might be like this:

$this->authManager->setAuthenticationSessionData( self::USERNAME_SESSION_KEY, $user->getName() );

$this->authManager->setAuthenticationSessionData( self::REALNAME_SESSION_KEY, $user->getRealName() );

$this->authManager->setAuthenticationSessionData( self::EMAIL_SESSION_KEY, $user->getEmail() );

I tested, it fixes the above Exception. Thank you!

If you look at the code of User::getName(), User::getRealName(), and User::getEmail(), they all do quite a bit more than just return the relevant values. It looks like $user->mName, $user->mRealName, and $user->email could be used instead.

Sure, that's up to you. Thanks!

Any chance to get it fixed with some future version?

Please feel free to submit a task in Phabricator tagged with the Mediawiki-extensions-Pluggable-Auth project and a patch in Gerrit referring to the task.

Sorry, I neither have a plan to join the Development Community nor study how Phabricator works. I hope that you agree with the above report and fix that sooner or later. For now I applied a workaround to avoid the issue.