I’ll see if I can git bisect this down to a specific commit to blame…

Okay, git bisect says Deprecate $wgMWOAuthSharedUserIDs is the first bad commit. (I retried each CI job twice, i.e. three builds per commit, to hopefully rule out any flakiness. The results seem quite consistent.)

I’m working around this for now on the main branch by checking out the commit just before the “bad” one.

In case it’s useful, the failed builds are:

• timeouts:
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672361
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672362
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672378 (looks a bit different than the others because here I disabled the PHP-level timeout, but that just turned it into a Selenium-level timeout a bit later)
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672428
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672437
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672445
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672448
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672450
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672462
• OOMs:
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672356
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672375
  • https://gitlab.wikimedia.org/repos/m3api/m3api-oauth2/-/jobs/672430

What $wgMWOAuthCentralWiki, $wgMWOAuthSharedUserIDs and $wgMWOAuthSharedUserSource settings are you using?

rEOAUc7c9d5f9b5f1: Deprecate $wgMWOAuthSharedUserIDs is a noop with CentralAuth. For a non-farm wiki it will replace some User::new* calls with LocalCentralIdLookup; maybe that's not cached as much as it should be?

But then I think for CI we would use the local lookup anyway, whether or not CentralAuth is installed, so not sure why the error doesn't occur in Wikimedia CI. Maybe it just has higher limits?

In T409637#11357020, @Tgr wrote:

What $wgMWOAuthCentralWiki, $wgMWOAuthSharedUserIDs and $wgMWOAuthSharedUserSource settings are you using?

Whichever the default values are – the only changed OAuth settings in this CI setup are $wgOAuth2GrantExpirationInterval, $wgOAuth2PrivateKey and $wgOAuth2PublicKey.

But then I think for CI we would use the local lookup anyway, whether or not CentralAuth is installed, so not sure why the error doesn't occur in Wikimedia CI. Maybe it just has higher limits?

Could be… though I don’t see any significant difference in the CI times between Add Slovak special page aliases and Deprecate $wgMWOAuthSharedUserIDs, neither in the test build nor in gate-and-submit.

In T409637#11357020, @Tgr wrote:

For a non-farm wiki it will replace some User::new* calls with LocalCentralIdLookup; maybe that's not cached as much as it should be?

Relevant task: T387105: LocalIdLookup should use a cache

Hm, I think OAuth might be not so much “resource-hungry” as “broken”? When running the m3api-oauth2 integration tests against a local wiki, with very little custom OAuth configuration, it encounters this error:

The only OAuth-related lines in my LocalSettings.php are:

wfLoadExtension( 'OAuth' );
$wgGroupPermissions['sysop']['mwoauthproposeconsumer'] = true;
$wgGroupPermissions['sysop']['mwoauthupdateownconsumer'] = true;
$wgGroupPermissions['sysop']['mwoauthmanageconsumer'] = true;
$wgGroupPermissions['sysop']['mwoauthsuppress'] = true;
$wgGroupPermissions['sysop']['mwoauthviewsuppressed'] = true;
$wgGroupPermissions['sysop']['mwoauthviewprivate'] = true;
$wgGroupPermissions['sysop']['mwoauthmanagemygrants'] = true;
$wgOAuth2PrivateKey = __DIR__ . '/private.key';
$wgOAuth2PublicKey = __DIR__ . '/public.key';
$wgMWOAuthSharedUserIDs = true; // temporary due to OAuth commit c7c9d5f9b5, until OAuth stops spamming deprecation warnings for the **default config** >.<                                                                                                                                           

Slightly more readable version of what appears to be the loop:

includes/Request/WebRequest.php(860): MediaWiki\Session\SessionManager->getSessionForRequest()
includes/Permissions/PermissionManager.php(1572): MediaWiki\Request\WebRequest->getSession()
includes/Permissions/PermissionManager.php(1514): MediaWiki\Permissions\PermissionManager->getUserPermissions()
includes/Permissions/UserAuthority.php(271): MediaWiki\Permissions\PermissionManager->userHasRight()
includes/Permissions/UserAuthority.php(130): MediaWiki\Permissions\UserAuthority->internalAllowed()
includes/User/User.php(2129): MediaWiki\Permissions\UserAuthority->isAllowed()
includes/User/CentralId/LocalIdLookup.php(104): MediaWiki\User\User->isAllowed()
includes/User/CentralId/CentralIdLookup.php(238): MediaWiki\User\CentralId\LocalIdLookup->lookupCentralIds()
includes/User/CentralId/CentralIdLookup.php(316): MediaWiki\User\CentralId\CentralIdLookup->nameFromCentralId()
extensions/OAuth/src/Backend/Utils.php(292): MediaWiki\User\CentralId\CentralIdLookup->localUserFromCentralId()
extensions/OAuth/src/ResourceServer.php(180): MediaWiki\Extension\OAuth\Backend\Utils::getLocalUserFromCentralId()
extensions/OAuth/src/ResourceServer.php(157): MediaWiki\Extension\OAuth\ResourceServer->setUser()
extensions/OAuth/src/ResourceServer.php(89): MediaWiki\Extension\OAuth\ResourceServer->setVerifiedInfo()
vendor/league/oauth2-server/src/Middleware/ResourceServerMiddleware.php(54): MediaWiki\Extension\OAuth\ResourceServer->{closure:MediaWiki\Extension\OAuth\ResourceServer::verify():88}()
extensions/OAuth/src/ResourceServer.php(85): League\OAuth2\Server\Middleware\ResourceServerMiddleware->__invoke()
extensions/OAuth/src/SessionProvider.php(279): MediaWiki\Extension\OAuth\ResourceServer->verify()
extensions/OAuth/src/SessionProvider.php(109): MediaWiki\Extension\OAuth\SessionProvider->verifyOAuth2Request()
includes/Session/SessionManager.php(569): MediaWiki\Extension\OAuth\SessionProvider->provideSessionInfo()
includes/Session/SessionManager.php(136): MediaWiki\Session\SessionManager->getSessionInfoForRequest()
includes/Request/WebRequest.php(860): MediaWiki\Session\SessionManager->getSessionForRequest()
<!-- there's a hole in my bucket… -->

If I understand correctly: PermissionManager asks for the $userObj->getRequest()->getSession()->getAllowedUserRights(); the session calls into OAuth; LocalIdLookup::lookupCentralId() checks for the audience’s hideuser right; and back into the PermissionManager we go. I’m not sure where this cycle should best be broken – you probably have a better idea about that. (And also, why this didn’t come up in CI…)

Thanks for tracking it down. Yeah that seems like a bug in Utils::getLocalUserFromCentralId().

This bug is present in previous releases as well: https://github.com/weirdgloop/mediawiki-extensions-OAuth/commit/6b8f1f87a6dcc8646e73cc88320d90467976f319

Thanks, good shout – this patch appears to be sufficient to make the m3api-oauth2 tests pass against my local wiki:

diff --git i/src/Backend/Utils.php w/src/Backend/Utils.php
index 8a64797522..3e7607dcb4 100644
--- i/src/Backend/Utils.php
+++ w/src/Backend/Utils.php
@@ -289,7 +289,7 @@ public static function getCentralUserNameFromId( $userId, $audience = false ) {
 	 */
 	public static function getLocalUserFromCentralId( $userId ) {
 		$lookup = self::getCentralIdLookup();
-		$user = $lookup->localUserFromCentralId( $userId );
+		$user = $lookup->localUserFromCentralId( $userId, CentralIdLookup::AUDIENCE_RAW );
 		if ( $user === null || !$lookup->isAttached( $user ) ) {
 			return false;
 		}

That said, I have no clue whether this is correct / safe, or whether getLocalUserFromCentralId() should itself take an audience argument (like getCentralUserNameFromId() already does, for instance) and have the right caller pass that in.

FWIW this also affects CentralAuth wikis, but due to the way the different CentralIdLookup providers are implemented, on those wikis the loop only happens if the user is hidden (as CentralAuthIdLookup first does the query and then filters out the hidden users, while LocalIdLookup does the filtering in the query).

PermissionManager breaks loops for block checks, I wonder if we should do the same for session permission checks?

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

[mediawiki/extensions/OAuth@master] Use AUDIENCE_RAW in OAuth central user ID lookups

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

Change #1207819 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Use AUDIENCE_RAW in OAuth central user ID lookups

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

In T409637#11356143, @LucasWerkmeister wrote:

I’m working around this for now on the main branch by checking out the commit just before the “bad” one.

Workaround removed, thanks for the fix \o/