Page MenuHomePhabricator

OAuth login to wikitech fails when running MediaWiki 1.42.0-wmf.4
Closed, ResolvedPublicBUG REPORT

Description

Striker, Horizon, and Stashbot all started having problems when MediaWiki 1.42.0-wmf.4 landed on Wikitech circa 2023-11-08T09:00. All three projects use the mwclient python library with owner-only OAuth credentials to interact with Wikitech.

The symptom seen in debug logs was messages about OAuth nonce values being already consumed. The mwclient library logged this as potentially related to the now resolved T106066: Don't show "Nonce already used" error on memcache failure.

@Krinkle and @matmarex were pinged into help figure out what was going wrong. @matmarex deserves the hero points for eventually figuring out that @tstarling's recent change to includes/user/User.php was implicated. This change led to code in OAuth's src/SessionProvider.php attempting to unstub the User while already unstubbing the User in a classic AuthManger chicken and egg problem of attempting a rights check while unstubbing. Wikitech is specifically affected because it is one of the few Wikimedia managed wikis that sets $wgBlockDisablesLogin = true;.

A hotfix hack on Wikitech that eliminates the double unstubbing is:

--- php-1.42.0-wmf.3/extensions/OAuth/src/SessionProvider.php   2023-10-31 03:00:56.493529422 +0000
+++ php-1.42.0-wmf.4/extensions/OAuth/src/SessionProvider.php   2023-11-08 22:22:31.801610334 +0000
@@ -20,6 +20,7 @@
 use MediaWiki\Hook\RecentChange_saveHook;
 use MediaWiki\Linker\Linker;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Permissions\Authority;
 use MediaWiki\Session\ImmutableSessionProviderWithCookie;
 use MediaWiki\Session\SessionBackend;
 use MediaWiki\Session\SessionInfo;
@@ -227,7 +228,7 @@
                        );
                }
                if ( $localUser->isLocked() ||
-                       ( $this->config->get( 'BlockDisablesLogin' ) && $localUser->getBlock() )
+                       ( $this->config->get( 'BlockDisablesLogin' ) && $localUser->getBlock( Authority::READ_NORMAL, /*$disableIpBlockExemptChecking*/ true ) )
                ) {
                        $this->logger->debug( 'OAuth request for blocked user {user}', $logData );
                        return $this->makeException( 'mwoauth-invalid-authorization-blocked-user' );

Event Timeline

This backtrace was the breadcrumb that lead to realizing there was a double unstubbing being attempted:

MWOAuthDataStore.php line 161 calls wfBacktrace()
OAuthServer.php line 261 calls MediaWiki\Extension\OAuth\Backend\MWOAuthDataStore->lookup_nonce()
OAuthServer.php line 211 calls MediaWiki\Extension\OAuth\Lib\OAuthServer->check_nonce()
OAuthServer.php line 106 calls MediaWiki\Extension\OAuth\Lib\OAuthServer->check_signature()
MWOAuthServer.php line 303 calls MediaWiki\Extension\OAuth\Lib\OAuthServer->verify_request()
SessionProvider.php line 195 calls MediaWiki\Extension\OAuth\Backend\MWOAuthServer->verify_request()
SessionManager.php line 537 calls MediaWiki\Extension\OAuth\SessionProvider->provideSessionInfo()
SessionManager.php line 244 calls MediaWiki\Session\SessionManager->getSessionInfoForRequest()
WebRequest.php line 840 calls MediaWiki\Session\SessionManager->getSessionForRequest()
PermissionManager.php line 1585 calls MediaWiki\Request\WebRequest->getSession()
PermissionManager.php line 1527 calls MediaWiki\Permissions\PermissionManager->getUserPermissions()
UserAuthority.php line 284 calls MediaWiki\Permissions\PermissionManager->userHasRight()
UserAuthority.php line 143 calls MediaWiki\Permissions\UserAuthority->internalAllowed()
User.php line 2298 calls MediaWiki\Permissions\UserAuthority->isAllowed()
User.php line 1435 calls MediaWiki\User\User->isAllowed()
SessionProvider.php line 230 calls MediaWiki\User\User->getBlock()
SessionManager.php line 537 calls MediaWiki\Extension\OAuth\SessionProvider->provideSessionInfo()
SessionManager.php line 244 calls MediaWiki\Session\SessionManager->getSessionInfoForRequest()
WebRequest.php line 840 calls MediaWiki\Session\SessionManager->getSessionForRequest()
SessionManager.php line 165 calls MediaWiki\Request\WebRequest->getSession()
Setup.php line 486 calls MediaWiki\Session\SessionManager::getGlobalSession()
WebStart.php line 92 calls require_once()
api.php line 46 calls require()
api.php line 3 calls require()

This is certainly T350080: 1.42.0-wmf.4 deployment blockers related, but I'm not sure if it is worth blocking the train. I don't think the issue will affect any wikis other than wikitech.wikimedia.org and possibly officewiki. It has been shown not to be an active problem on www.mediawiki.org which is also running MediaWiki 1.42.0-wmf.4.

Change 972913 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):

[mediawiki/extensions/OAuth@master] SessionProvider::provideSessionInfo: Guard against double session unstubbing

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

Mentioned in SAL (#wikimedia-operations) [2023-11-09T09:41:38Z] <jnuche@deploy2002> rebuilt and synchronized wikiversions files: Deploy 1.42.0-wmf.4 to group2 (labswiki staying at 1.42.0-wmf.3 due to T350836)

Change 973118 had a related patch set uploaded (by Jaime Nuche; author: Jaime Nuche):

[operations/mediawiki-config@master] group2 wikis to 1.42.0-wmf.4 (labswiki staying at 1.42.0-wmf.3 due to T350836)

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

Change 973118 merged by jenkins-bot:

[operations/mediawiki-config@master] group2 wikis to 1.42.0-wmf.4 (labswiki staying at 1.42.0-wmf.3 due to T350836)

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

From the incident investigation yesterday in #wikimedia-cloud-admin on IRC:

<MatmaRex>
right
so it looks like it's trying to get the login session while getting the login session
probably caused by this change? https://gerrit.wikimedia.org/r/c/mediawiki/core/+/966657/10/includes/user/User.php
that isAllowed() call is in the backtrace
i'm not sure what is going on here, though (or whether we can revert it)

[…]

<taavi>
hm, could be, wikitech is one of the few prod wikis with $wgBlockDisablesLogin as true
so in theory it should also be broken on all private wikis?

The linked patch is by @tstarling, tagging T345683: Review of MediaWiki block-related code.

The related recursion code also has a pending patch where @daniel and myself have left some review comments:

[mediawiki/core] Add internal recursionCounter to UserGroupManager

This is to prevent infinite recursion when checking APCOND_BLOCKED due
to potentially repeated calls to user->getBlock.
Removes disableIpBlockExemptChecking as obsolete.

Bug: T349608

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/969358

I want to reproduce this and test the fix, but I think the solution here will be for OAuth to ask for a non-request block, by passing null for the $request parameter to BlockManager::getBlock(). It's not OAuth's job to deny requests that are IP-blocked. It's just trying to validate the username that it pulled out of the OAuth headers. So ipblock-exempt permissions should be irrelevant.

Change 973270 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/OAuth@master] Fix BlockDisablesLogin recursion

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

Change 972913 abandoned by BryanDavis:

[mediawiki/extensions/OAuth@master] SessionProvider::provideSessionInfo: Guard against double session unstubbing

Reason:

Superseded by I2b424789ddfabf43171fffef7c9275a929e0c850

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

Change 973247 had a related patch set uploaded (by BryanDavis; author: Tim Starling):

[mediawiki/extensions/OAuth@wmf/1.42.0-wmf.4] Fix BlockDisablesLogin recursion

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

Change 973270 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Fix BlockDisablesLogin recursion

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

Change 973247 merged by jenkins-bot:

[mediawiki/extensions/OAuth@wmf/1.42.0-wmf.4] Fix BlockDisablesLogin recursion

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

Mentioned in SAL (#wikimedia-operations) [2023-11-13T09:06:42Z] <jnuche@deploy2002> Started scap: Backport for [[gerrit:973247|Fix BlockDisablesLogin recursion (T350836 T350080)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-13T09:08:34Z] <jnuche@deploy2002> bd808 and jnuche: Backport for [[gerrit:973247|Fix BlockDisablesLogin recursion (T350836 T350080)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-11-13T09:14:32Z] <jnuche@deploy2002> Finished scap: Backport for [[gerrit:973247|Fix BlockDisablesLogin recursion (T350836 T350080)]] (duration: 07m 49s)

Mentioned in SAL (#wikimedia-operations) [2023-11-13T09:31:19Z] <jnuche@deploy2002> rebuilt and synchronized wikiversions files: labswiki to 1.42.0-wmf.4 (T350836 T350080)

Change 973724 had a related patch set uploaded (by Jaime Nuche; author: Jaime Nuche):

[operations/mediawiki-config@master] labswiki to 1.42.0-wmf.4

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

Change 973724 merged by jenkins-bot:

[operations/mediawiki-config@master] labswiki to 1.42.0-wmf.4

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