Page MenuHomePhabricator

Review of MediaWiki block-related code
Closed, ResolvedPublic

Description

As we get ready for T194697: Multiblocks — Allow for multiple, simultaneous blocks with different expiration dates., I'm reviewing the includes/block directory and related code, and will link some cleanup commits here.

Details

Event Timeline

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

[mediawiki/core@master] Blocks documentation review

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

Change 955045 merged by jenkins-bot:

[mediawiki/core@master] Blocks documentation review

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

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

[mediawiki/core@master] Blocks cleanup

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

Change 958587 merged by jenkins-bot:

[mediawiki/core@master] Blocks cleanup

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

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

[mediawiki/core@master] Add Block::toArray()

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

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

[mediawiki/core@master] Remove deprecated User properties mOptions, mBlock, mBlockedby and mHideName

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

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

[mediawiki/core@master] Remove User::CHECK_USER_RIGHTS and User::IGNORE_USER_RIGHTS

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

Change 959369 merged by jenkins-bot:

[mediawiki/core@master] Remove deprecated User properties mOptions, mBlock, mBlockedby and mHideName

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

Change 959365 merged by jenkins-bot:

[mediawiki/core@master] Add Block::toArray()

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

Change 959376 merged by jenkins-bot:

[mediawiki/core@master] Remove User::CHECK_USER_RIGHTS and User::IGNORE_USER_RIGHTS

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

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

[mediawiki/core@master] Add cache to BlockManager

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

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

[mediawiki/core@master] Remove Block cache from User

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

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

[mediawiki/core@master] Hard deprecate User::isBlocked(), isBlockedFrom() and isBlockedFromCreateAccount()

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

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

[mediawiki/core@master] Add PermissionManager::$blockManager

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

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

[mediawiki/core@master] Multiblocks preparation in User::getBlock(), PermissionManager and BlockManager

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

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

[mediawiki/extensions/ProofreadPage@master] PageContentHandlerTest: remove assignment to User::$mHideName

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

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

[mediawiki/extensions/CheckUser@master] In SpecialCheckUserTest don't have the test sysop block themselves

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

Change 964646 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] In SpecialCheckUserTest don't have the test sysop block themselves

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

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

[mediawiki/extensions/WikimediaEvents@master] Stop using deprecated method User::isBlockedFromCreateAccount()

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

Change 964615 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] PageContentHandlerTest: remove assignment to User::$mHideName

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

Change 963673 merged by jenkins-bot:

[mediawiki/core@master] Add cache to BlockManager

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

Change 963674 merged by jenkins-bot:

[mediawiki/core@master] Remove Block cache from User

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

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

[mediawiki/core@master] AuthManager: deny auto-creation for globally blocked users

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

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

[mediawiki/extensions/CentralAuth@master] Pass performer parameter to AuthManager::autoCreateUser()

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

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

[mediawiki/core@master] Migrate callers of deprecated method BlockManager::getUserBlock()

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

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

[mediawiki/extensions/Wikibase@master] Update mock for new core method BlockManager::getBlock()

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

Change 964609 merged by jenkins-bot:

[mediawiki/core@master] Add PermissionManager::$blockManager

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

Change 964610 merged by jenkins-bot:

[mediawiki/core@master] Multiblocks preparation in User::getBlock(), PermissionManager and BlockManager

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

Change 966315 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: deny auto-creation for globally blocked users

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

Change 966316 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Pass performer parameter to AuthManager::autoCreateUser()

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

Change 965269 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] Stop using deprecated method User::isBlockedFromCreateAccount()

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

Change 963676 merged by jenkins-bot:

[mediawiki/core@master] Hard deprecate User::isBlocked(), isBlockedFrom() and isBlockedFromCreateAccount()

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

Change 964610 merged by jenkins-bot:

[mediawiki/core@master] Multiblocks preparation in User::getBlock(), PermissionManager and BlockManager

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

@tstarling Since this change, when attempting to edit as an IP from an IP that is blocked and ip masking is enabled, the block message is duplicated.

It does not seem to matter if the IP is blocked by a local database block, global block, system block or a combination of any of these. Whatever the block error message is for the particular block, it is duplicated.

I can reproduce this locally and on dewiki beta.

Example:

duplicate_block_error_message.png (602×1 px, 107 KB)

@tstarling Since this change, when attempting to edit as an IP from an IP that is blocked and ip masking is enabled, the block message is duplicated.

I have raised T350116.

Change 966797 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Avoid mock of deprecated method BlockManager::getUserBlock()

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

Change 966657 merged by jenkins-bot:

[mediawiki/core@master] Migrate callers of deprecated method BlockManager::getUserBlock()

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

I think I have done with this for now.

My only observation is that when editing or creating a page or trying to use discussiontools the code of the block message returned by the API is different. Only for anonymous/IP users on a wiki with IP masking enabled. Before it would return blocked. Now it returns blockedtext-tempuser.

Change 966315 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: deny auto-creation for globally blocked users

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

I didn't understand this change. I don't see how as a globally blocked IP you could create a temporary account, even before this change.