Page MenuHomePhabricator

Temp account autocreation on edit fails with "No active login attempt is in progress for your session"
Closed, ResolvedPublic

Description

Creating a temporary account via an edit fails in my local environment using:

  • CACHE_MEMCACHED for session management
  • database replication for local wiki and loginwiki

This also fails on beta cluster wikis.

It looks like Create an autocreate log when a temporary account is created on edit is the problematic commit, according to git bisect, but not sure why.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change #1052323 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] AuthManager: Read from primary in temp account autocreation

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

Something in AuthManager::acquireActorId is involved in the session issues for temp account auto creation, when called in the context of autoCreateUser and creating a log entry.

Something in AuthManager::acquireActorId is involved in the session issues for temp account auto creation, when called in the context of autoCreateUser and creating a log entry.

The bug disappears if you comment out $logEntry->insert(); in AuthManager::autoCreateUser.

Could we put the log creation in a deferred update or job? I'm thinking that putting the log entry last might fix the issue, as the log entry will create an actor ID for the local user. That might be the cause of the problems.

BTW I have no issues locally with this, so it seems to be the interaction with MediaWiki-extensions-CentralAuth specifically.

Could we put the log creation in a deferred update or job? I'm thinking that putting the log entry last might fix the issue, as the log entry will create an actor ID for the local user. That might be the cause of the problems.

Nice idea. I can confirm that putting the log entry insertion in a deferred update will work. Not sure if there are other side effects to that, though.

BTW I have no issues locally with this, so it seems to be the interaction with MediaWiki-extensions-CentralAuth specifically.

I think you need to have CentralAuth, multiple wikis, and DB replication for the wikis.

I think I understand what's going on. When Kosta showed this problem earlier, there were two calls to AuthManager::autoCreateUser(); the first actually autocreated, and the second should have followed this code path:

if ( $localId ) {
	$user->setId( $localId );
	$user->loadFromId( $flags );
	if ( $login ) {
		$remember = $source === self::AUTOCREATE_SOURCE_TEMP;
		$this->setSessionDataForUser( $user, $remember );
	}
	return Status::newGood()->warning( 'userexists' );
}

but loadFromId() loaded from the replica where the user didn't exist yet, so $user ended up as the anonymous user, and `setSessionDataForUser()˙effectively logged the user out.

$localId comes from UserIdentityLookup::getUserIdentityByName() which checks the (in-process) actor cache before checking the DB. The actor cache gets set by acquireActorId() (which is apparently only called when $log is true). So loading the user from the replica succeeds (except it's actually loaded from in-memory cache), and autoCreateUser() thinks it's safe to use the replica for further lookups for that user, but it actually isn't.

CentralAuth and multiple wikis don't seem to be related, other than that CentralAuth adds extra DB operations during autocreation so it might affect the speed of replication.

ActorCache seems suspect more generally as well since it has no concept of query flags at all. Here that fails in the more unusual direction where we make a replica read and assume that getting a result means the user is present in the replica (probably the calling code's fault as the interface makes no such contract). But, as far as I see, it could also fail in the opposite direction where we try to read from primary and get a cached value that was the result of an earlier replica read (e.g. during an ActorStore::findActorId() call).

So the high-brow fix would be to refactor ActorCache to keep track of what source it was loaded from. Looks pretty complicated though.

BTW IIRC this error occurred when editing a page again right after creating a temporary account, which seems like a bug somewhere in EditPage - there isn't really any reason to autocreate in that situation.

Change #1052440 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] auth: Fix DB logic in AuthManager::autoCreateUser()

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

Change #1052323 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: Read from primary in account autocreation

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

This can skip QA, I think. Temp account creation on beta cluster is now unbroken.