Just 1.36 to go?

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

[mediawiki/extensions/CentralAuth@REL1_36] Cross-wiki block should pass correct wiki blocker

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

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

[mediawiki/core@REL1_36] UserIdentityValue: Introduce convenience static factory methods

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

In T281972#7075007, @Reedy wrote:

Just 1.36 to go?

I think so.

AFAICS CheckUser was not mentioned in the first stacks, so it is probably something there that causes this error sometimes.

This is definitely a different error, you're right. Will have a look.

Okey. The problem is pretty clear.

Now that we are ably to correctly insert cross-wiki block with a correct blocker, CheckUser is doing auto blocking of IPs that are associated with the block. But CheckUser is oblivious towards which wiki the block actually belongs to, so it does everything in the context of metawiki.

BTW, none of these errors are regressions. They just indicate before some of these blocks were inserted with invalid blockers..

I guess the best fix is to actually make blocks wiki-aware and support cross-wiki blocking properly. How urgent is this?

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

[mediawiki/core@master] Follow-up bbc75d404: Update @since tag as we're back-porting

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

Change 688290 merged by jenkins-bot:

[mediawiki/core@REL1_36] UserIdentityValue: Introduce convenience static factory methods

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

Change 689379 merged by jenkins-bot:

[mediawiki/core@master] Follow-up bbc75d404: Update @since tag as we're back-porting

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

Change 688289 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@REL1_36] Cross-wiki block should pass correct wiki blocker

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

https://873gear.com/irc/uploads/1d222380b2567746/image.png

Account was not locked, attempted to lock/hide

@Operator873: Please provide a stack trace, as text.

Let me try to summarize where are we at right now:

• CentralAuth-issued suppresions don't work, because making xwiki autoblocks is not possible. The proper solution for that would be T274817: Convert DatabaseBlock and AbstractBlock to UserIdentity, and make them cross-wiki aware., which is something the platform team would likely work at one point.
• Simple workaround would be to disable autoblocks. Stewards do not autoblocks to work for locks, because there is currently no such mechanism. In case we need an autoblock-like mechanism, we manually checkuser the user and block the IP manually. So, let's do that for now, and let T274817 be the proper solution. I'll upload a patch momentarily.

@Pchelolo Could you please review this one here, and provide a +2 as a Phabricator comment? I can then deploy it (or maybe it can actually go through Gerrit instead?).

Is there any plan to review this patch? If an user doxxes someone on the username, or creates an account with a phone number, there is no way to hide that. This is a big security issue that has a patch pending review for 15 days. I understand we are all volunteers, but it'd be really good for this to be reviewed.

@Urbanecm +2 for the patch.

20:11 <urbanecm> !log Deployed security patch for T281972
20:11 <stashbot> Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log

Thanks @Pchelolo, appreciated. Deployed. A proper fix can happen later.

In T281972#7129975, @Urbanecm wrote:

@sbassett Hi, would you mind doing the final honors (backports here; should need only master)? Thanks!

Sure, tracking at T276237 and T279733 for now. This will definitely get backported before the end of this quarter (2021-06-30) but I can't guarantee when, unless someone else wants to run with it. Though I'd question if we even want this on master since it's kind of a Wikimedia production-specific config change if I'm reading this bug and the patch comment correctly.

In T281972#7130744, @sbassett wrote:

In T281972#7129975, @Urbanecm wrote:

@sbassett Hi, would you mind doing the final honors (backports here; should need only master)? Thanks!

Sure, tracking at T276237 and T279733 for now. This will definitely get backported before the end of this quarter (2021-06-30) but I can't guarantee when, unless someone else wants to run with it. Though I'd question if we even want this on master since it's kind of a Wikimedia production-specific config change if I'm reading this bug and the patch comment correctly.

The error would be very likely present on any Wikimedia-like wikifarm. The underlying bug is in MediaWiki core (which doesn't support xwiki blocks [an user from wiki A blocks on wiki B] properly, and while xwiki blocks work somehow, autoblocks sometimes fail with this error message, which is why this task exists]). Fixing that bug properly would be possible, but the platform team asked for time to refactor blocks to support xwiki blocks in the proper way, rather than adding hacks at top of hacks.

You're right that the bug very likely apprears only in Wikimedia production, but that's not because we have a special configuration – it is because non-Wikimedia wikis shouldn't use centralauth because of its complexity (and in parts, Wikimedia-specificness), see the big red warning at https://www.mediawiki.org/wiki/Extension:CentralAuth.

I personally vote for including into master (as that'd avoid conflicts as master progresses), but I defer to your judgement.

In T281972#7131468, @Urbanecm wrote:

The error would be very likely present on any Wikimedia-like wikifarm. The underlying bug is in MediaWiki core (which doesn't support xwiki blocks [an user from wiki A blocks on wiki B] properly, and while xwiki blocks work somehow, autoblocks sometimes fail with this error message, which is why this task exists]). Fixing that bug properly would be possible, but the platform team asked for time to refactor blocks to support xwiki blocks in the proper way, rather than adding hacks at top of hacks.

Ok, that all makes sense, thanks. We can plan to backport this to master.

Change 702717 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@REL1_36] SECURITY: Disable autoblocks for CentralAuth-issued suppression blocks

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

I've deleted comments above that contained private information so this task can be made public. They're quoted below with the private info redacted:

In T281972#7087384, @DeltaQuad wrote:

Still currently getting an error.
[8008a778-58be-418c-8be8-72bbc8c57a76] 2021-05-14 07:24:13: Fatal exception of type "InvalidArgumentException"
URL: https://meta.wikimedia.org/w/index.php?title=Special:CentralAuth&target=<redacted>

In T281972#7090229, @Urbanecm wrote:

In T281972#7089889, @Aklapper wrote:

@Operator873: Please provide a stack trace, as text.

Note AFAIK, Operator873 does not have access to logstash, so he cannot see the stack. Here it is through:

Stack trace

[urbanecm@mwlog1002 /srv/mw-log]$ grep -A 27 fccd3bd6-2816-44e2-b525-c068ccf3d42d archive/exception.log-20210515
2021-05-15 00:51:30 [fccd3bd6-2816-44e2-b525-c068ccf3d42d] mw1373 metawiki 1.37.0-wmf.5 exception ERROR: [fccd3bd6-2816-44e2-b525-c068ccf3d42d] /w/index.php?title=Special:CentralAuth&target=<redacted>   InvalidArgumentException: DB connection domain 'metawiki' does not match 'loginwiki' {"exception_url":"/w/index.php?title=Special:CentralAuth&target=<redacted>","reqId":"fccd3bd6-2816-44e2-b525-c068ccf3d42d","caught_by":"entrypoint"}
[Exception InvalidArgumentException] (/srv/mediawiki/php-1.37.0-wmf.5/includes/user/ActorStore.php:704) DB connection domain 'metawiki' does not match 'loginwiki'
  #0 /srv/mediawiki/php-1.37.0-wmf.5/includes/user/ActorStore.php(412): MediaWiki\User\ActorStore->checkDatabaseDomain(Wikimedia\Rdbms\DBConnRef)
  #1 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlockStore.php(360): MediaWiki\User\ActorStore->acquireActorId(MediaWiki\User\UserIdentityValue, Wikimedia\Rdbms\DBConnRef)
  #2 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlockStore.php(166): MediaWiki\Block\DatabaseBlockStore->getArrayForDatabaseBlock(MediaWiki\Block\DatabaseBlock, Wikimedia\Rdbms\DBConnRef)
  #3 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlock.php(690): MediaWiki\Block\DatabaseBlockStore->insertBlock(MediaWiki\Block\DatabaseBlock)
  #4 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CheckUser/src/Hooks.php(708): MediaWiki\Block\DatabaseBlock->doAutoblock(string)
  #5 /srv/mediawiki/php-1.37.0-wmf.5/includes/HookContainer/HookContainer.php(330): MediaWiki\CheckUser\Hooks::doRetroactiveAutoblock(MediaWiki\Block\DatabaseBlock, array)
  #6 /srv/mediawiki/php-1.37.0-wmf.5/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
  #7 /srv/mediawiki/php-1.37.0-wmf.5/includes/HookContainer/HookRunner.php(2981): MediaWiki\HookContainer\HookContainer->run(string, array)
  #8 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlockStore.php(440): MediaWiki\HookContainer\HookRunner->onPerformRetroactiveAutoblock(MediaWiki\Block\DatabaseBlock, array)
  #9 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlockStore.php(206): MediaWiki\Block\DatabaseBlockStore->doRetroactiveAutoblock(MediaWiki\Block\DatabaseBlock)
  #10 /srv/mediawiki/php-1.37.0-wmf.5/includes/block/DatabaseBlock.php(535): MediaWiki\Block\DatabaseBlockStore->insertBlock(MediaWiki\Block\DatabaseBlock, Wikimedia\Rdbms\DBConnRef)
  #11 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/CentralAuthUser.php(1952): MediaWiki\Block\DatabaseBlock->insert(Wikimedia\Rdbms\DBConnRef)
  #12 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/CentralAuthUser.php(1881): CentralAuthUser->doLocalSuppression(boolean, string, string, string)
  #13 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/CentralAuthUser.php(1859): CentralAuthUser->doCrosswikiSuppression(boolean, string, string)
  #14 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/CentralAuthUser.php(1810): CentralAuthUser->suppress(string, string)
  #15 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/specials/SpecialCentralAuth.php(245): CentralAuthUser->adminLockHide(boolean, string, string, RequestContext)
  #16 /srv/mediawiki/php-1.37.0-wmf.5/extensions/CentralAuth/includes/specials/SpecialCentralAuth.php(143): SpecialCentralAuth->doSubmit()
  #17 /srv/mediawiki/php-1.37.0-wmf.5/includes/specialpage/SpecialPage.php(646): SpecialCentralAuth->execute(NULL)
  #18 /srv/mediawiki/php-1.37.0-wmf.5/includes/specialpage/SpecialPageFactory.php(1396): SpecialPage->run(NULL)
  #19 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(313): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, RequestContext)
  #20 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(916): MediaWiki->performRequest()
  #21 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(550): MediaWiki->main()
  #22 /srv/mediawiki/php-1.37.0-wmf.5/index.php(53): MediaWiki->run()
  #23 /srv/mediawiki/php-1.37.0-wmf.5/index.php(46): wfIndexMain()
  #24 /srv/mediawiki/w/index.php(3): require(string)
  #25 {main}
[urbanecm@mwlog1002 /srv/mw-log]$

It looks to match what I offered at T281972#7078879.

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

[mediawiki/core@master] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

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

[mediawiki/core@REL1_37] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

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

[mediawiki/core@REL1_36] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

Change 738400 merged by jenkins-bot:

[mediawiki/core@REL1_37] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

Change 738401 merged by jenkins-bot:

[mediawiki/core@REL1_36] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

Change 738549 merged by jenkins-bot:

[mediawiki/core@master] Follow-Up: I10fbd4b6a: Update @since tags as those were backported

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

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

[mediawiki/extensions/CentralAuth@master] Reenable autoblocks for CentralAuth-issued suppression blocks

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

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

[mediawiki/extensions/CheckUser@master] Respect the wiki when performing autoblocks

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

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

[mediawiki/core@master] Respect the wiki when performing autoblocks

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

"Fun" fact: The second part of this was actually predicted almost a year before it showed up.

From the task description of T258866:

CentralAuthUser::doLocalSuppression is an example that passes the database to insert:
The block is always autoblocking (presumably the autoblocks are not being inserted correctly?)

Change 756131 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Respect the wiki when performing autoblocks

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

Change 763788 merged by jenkins-bot:

[mediawiki/core@master] Respect the wiki when performing autoblocks

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

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

[mediawiki/core@master] block: Create wiki-aware target for autoblocks

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

Change 810381 merged by jenkins-bot:

[mediawiki/core@master] block: Create wiki-aware target for autoblocks

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

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

[mediawiki/core@master] Load potential current ip block from correct wiki

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