Maniphest T291983

Create a BlockRestrictionStoreFactory in order to make BlockRestrictionStore a proper cross-wiki store
Closed, ResolvedPublic
Actions

Description

Blocks can be issued cross-wiki. Thus BlockRestrictionStore should become a proper cross-wiki store by injecting the correct LoadBalancer into it. A new BlockRestrictionStoreFactory service should be created for doing so.

A LBFactory should be injected into BlockRestrictionStoreFactory which then fetches and injects the correct LoadBalancer into a BlockRestrictionStore.

Currently if a cross-wiki block with restrictions is being created, the block is being stored in the foreign database, but the restrictions are being stored in the local one. This is definetly not wanted.

Details

Subject	Repo	Branch	Lines +/-
Don't allow passing a db to BlockRestrictionStore::loadByBlockId()	mediawiki/core	master	+5 -10
Deprecate passing a db to BlockRestrictionStore::loadByBlockId()	mediawiki/core	master	+9 -0
Fetch correct BlockRestrictionStore	mediawiki/extensions/CentralAuth	master	+2 -2
Introduce a BlockRestrictionStoreFactory	mediawiki/core	master	+142 -12

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Resolved	Security	• Pchelolo	T281972 ActorStore::checkDatabaseDomain: InvalidArgumentException: DB connection domain does not match when suppressing via Special:CentralAuth (CVE-2021-36128)
Resolved	Security	Zabe	T284873 Regression: CentralAuth Lock+Suppress is issuing local blocks without autoblock
Open		None	T291994 Properly support cross-wiki blocking
Resolved		Zabe	T291983 Create a BlockRestrictionStoreFactory in order to make BlockRestrictionStore a proper cross-wiki store

Event Timeline

Zabe created this task.Sep 28 2021, 6:03 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 28 2021, 6:03 PM

Zabe claimed this task.Sep 28 2021, 6:03 PM

Zabe updated the task description. (Show Details)Sep 28 2021, 6:06 PM

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

[mediawiki/core@master] Introduce a BlockRestrictionStoreFactory

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

gerritbot added a project: Patch-For-Review.Sep 28 2021, 6:07 PM

Zabe triaged this task as High priority.Sep 28 2021, 6:26 PM

Zabe mentioned this in T291994: Properly support cross-wiki blocking.

Zabe added a parent task: T291994: Properly support cross-wiki blocking.

At least loading of the restrictions 'support' cross-wiki, cause there's an $db parameter to loadByBlockId. Once RestrictionStore is properly cross-wiki, that parameter can be removed.

This is an interesting one. Ideally BlockRestrictionStore shouldn't really be a public service - there should be no reason for external code to call it instead of interacting with blocks.

There's currently two direct usages of BlockRestrictionStore outside of blocks code:

CentralAuthUser::localUserData - this is just manually implementing cross-wiki block loading, and it should be replaced with proper support you're trying to implement.
WikibaseLexeme in a test is inserting the restrictions, but that's easily fixed by setting restrictions on a block before inserting the block instead of after.

All other usages are in core. The usages that are outside of the blocks namespace are mostly cause the code there is querying blocks table directly.

TLDR: I think BlockRestrictionStoreFactory should be @internal. It future when more block-related code is cleaned up, it should be an internal service that only block-related code can depend on.

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

[mediawiki/extensions/CentralAuth@master] Fetch correct BlockRestrictionStore

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

Zabe added a project: MediaWiki-extensions-CentralAuth.Oct 4 2021, 9:22 PM

Zabe added a project: Anti-Harassment.Oct 17 2021, 8:46 PM

phuedx moved this task from Untriaged to MediaWiki-Blocks on the Anti-Harassment board.Oct 18 2021, 2:54 PM

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

[mediawiki/core@master] Deprecate passing a db to BlockRestrictionStore::loadByBlockId()

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

Change 724475 merged by jenkins-bot:

[mediawiki/core@master] Introduce a BlockRestrictionStoreFactory

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