Page MenuHomePhabricator

Deprecate 'GetBlockedStatus' hook and reduce visibility of User::mBlock, User::mBlockedBy and User::mHideName
Closed, ResolvedPublic

Description

Problem
The GetBlockedStatus hook allows extensions to modify properties on the User object. Since several of the block-related properties are made public it is theoretically possible for extensions to modify these into a state that is not compatible with any block that can actually are made. (Example: an extension could theoretically set User::mBlock to null, but User::mBlockedby to a username and User::mHideName to true.) This makes blocks difficult to reason about - e.g. see T228948.

Proposed Solution
Now that we have a way to enforce multiple blocks, extensions that are only adding a block could do so before the CompositeBlock is constructed in BlockManager::getUserBlock(), so that their block can be added to any other blocks, rather than replacing them.

We could still allow extensions to modify the final block, but rather than allow them to alter the User, we could pass the array of constituent blocks, just before constructing the CompositeBlock.

It seems the use cases for setting these User properties are related to the GetBlockedStatus hook, so we could reduce their visibility if we make these changes.

  • Deprecate GetBlockedStatus
  • Deprecate mBlock, mBlockedby and mHideName as public properties on User

Event Timeline

dbarratt updated the task description. (Show Details)Jul 25 2019, 6:28 PM

Change 525305 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] Ensure block hooks keep user state consistent with realistic blocks

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

Change 525665 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/TorBlock@master] Use ModifyBlock hook instead of deprecated GetBlockedStatus

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

Change 525305 merged by jenkins-bot:
[mediawiki/core@master] Ensure block hooks keep user state consistent with realistic blocks

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

Change 534793 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] Deprecate GetBlockedStatus hook

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

Tchanders updated the task description. (Show Details)
Tchanders moved this task from Ready to Review on the Anti-Harassment (The Letter Song) board.
Tchanders updated the task description. (Show Details)Sep 6 2019, 2:59 PM

Change 534793 merged by jenkins-bot:
[mediawiki/core@master] Deprecate the GetBlockedStatus hook

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

Tchanders updated the task description. (Show Details)Sep 10 2019, 6:24 PM

Change 542771 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] Deprecate public User properties related to blocks

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

Change 542771 merged by jenkins-bot:
[mediawiki/core@master] Deprecate public User properties related to blocks

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

Tchanders closed this task as Resolved.Wed, Oct 23, 3:48 PM
Tchanders updated the task description. (Show Details)
Tchanders moved this task from Review to Done on the Anti-Harassment (The Letter Song) board.