Page MenuHomePhabricator

CentralAuthUserUsingDatabaseTest::testAdminLockAndHide fails
Closed, ResolvedPublic

Description

CentralAuth's CentralAuthUserUsingDatabaseTest::testAdminLockAndHide just started failing on totally empty patches with the following logs:

18:58:50 1) CentralAuthUserUsingDatabaseTest::testAdminLockAndHide
18:58:50 row #1 mismatches
18:58:50 Failed asserting that two arrays are equal.
18:58:50 --- Expected
18:58:50 +++ Actual
18:58:50 @@ @@
18:58:50  Array (
18:58:50      0 => 'GlobalUser'
18:58:50 -    1 => '1'
18:58:50 +    1 => '0'
18:58:50      2 => 'lists'
18:58:50  )
18:58:50 
18:58:50 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:2071
18:58:50 /workspace/src/extensions/CentralAuth/tests/phpunit/CentralAuthUserUsingDatabaseTest.php:168
18:58:50 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:452
18:58:50 === Logs generated by test case
18:58:50 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
18:58:50 [localisation] [debug] LocalisationCache using store LCStoreNull []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 [CentralAuthVerbose] [info] Updating cache for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Quick cache invalidation for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 [UserOptionsManager] [debug] Loading options from database {"user_id":12}
18:58:50 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
18:58:50 [DeferredUpdates] [debug] DeferredUpdates::run: started CdnCacheUpdate #11745 []
18:58:50 [squid] [info] CdnCacheUpdate::purge: http://127.0.0.1:9412/index.php/User:GlobalUser http://127.0.0.1:9412/index.php?title=User:GlobalUser&action=history {"private":false}
18:58:50 [DeferredUpdates] [debug] DeferredUpdates::run: ended CdnCacheUpdate #11745, processing time: 0.00028300285339355 []
18:58:50 [CentralAuthVerbose] [info] Updating cache for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Quick cache invalidation for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 [CentralAuthVerbose] [info] Updating cache for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Quick cache invalidation for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 [CentralAuthVerbose] [info] Updating cache for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Quick cache invalidation for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 [CentralAuthVerbose] [info] Updating cache for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Quick cache invalidation for global user GlobalUser []
18:58:50 [CentralAuthVerbose] [info] Loading state for global user {user} from DB {"user":"GlobalUser","private":false}
18:58:50 ===

I am marking this as UBN! and a train blocker because this is Wikimedia-specific core authentication code breaking without any clear explanation, and it should not be deployed to production unless the reason behind it is found.

Event Timeline

taavi triaged this task as Unbreak Now! priority.Oct 25 2021, 4:07 PM
taavi created this task.

I can reproduce it locally, and the value of gu_locked in the database changes during the call to SessionManager::singleton()->invalidateSessionsForUser( $user ); in CentralAuthUser::adminLock().

CentralAuthUser::adminLock()
	public function adminLock() {
		$this->checkWriteMode();
		$dbw = CentralAuthUtils::getCentralDB();
		$dbw->update(
			'globaluser',
			[ 'gu_locked' => 1 ],
			[ 'gu_name' => $this->mName ],
			__METHOD__
		);
		if ( !$dbw->affectedRows() ) {
			return Status::newFatal( 'centralauth-state-mismatch' );
		}

		$this->invalidateCache();
		$user = User::newFromName( $this->mName );
		$dbw->selectField( 'globaluser', 'gu_locked', [ 'gu_name' => $this->mName ] ); // '1'
		SessionManager::singleton()->invalidateSessionsForUser( $user );
		$dbw->selectField( 'globaluser', 'gu_locked', [ 'gu_name' => $this->mName ] ); // '0'! <======

		return Status::newGood();
	}

Change 734347 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/core@master] Revert: \"Update user_touched after saving user options.\"

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

Change 734333 had a related patch set uploaded (by Jforrester; author: Majavah):

[mediawiki/extensions/CentralAuth@master] [DNM] ci test

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

Is there a reason not to add CentralAuth to the main CI gate?

In T294265#7455730, @Majavah wrote:

Is there a reason not to add CentralAuth to the main CI gate?

Yes, CentralAuth breaks all the core unit testing of account-related things (creation/login/etc.) and so disables them all.

Change 734347 merged by jenkins-bot:

[mediawiki/core@master] Revert: \"Update user_touched after saving user options.\"

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

Zabe claimed this task.

CI passes again, I reopened T284354. So I think we are done here.

Change 734386 had a related patch set uploaded (by Ppchelko; author: Ppchelko):

[mediawiki/extensions/CentralAuth@master] CentralAuthUserUsingDatabaseTest: don't constract CAUser

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

Change 734386 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] CentralAuthUserUsingDatabaseTest: don't construct CAUser

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

Change 734333 abandoned by Majavah:

[mediawiki/extensions/CentralAuth@master] [DNM] ci test

Reason:

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