Page MenuHomePhabricator

Use GetUserBlockComplete hook instead of GetBlockedStatus in TorBlock extension
Closed, ResolvedPublic

Description

The GetBlockedStatus hook will be deprecated as outlined in T229035, and will be replaced by a GetUserBlockComplete, which passes a user's block instead of the user object.

TorBlock should use GetUserBlock, and the conditions for removing a block should not change. The conditions are:

$wgTorDisableAdminBlocks &&
TorExitNodes::isExitNode() &&
$user->mBlock instanceof AbstractBlock &&
$user->mBlock->getType() != DatabaseBlock::TYPE_USER

If the block is a composite block, made from multiple original blocks, the block should be removed if all the original blocks fulfill these criteria.

Event Timeline

Tchanders created this task.Aug 2 2019, 7:41 PM
Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptAug 2 2019, 7:41 PM

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

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

@dbarratt You can this locally by enabling TorBlock, setting $wgTorDisableAdminBlocks = true and overriding TorExitNodes::isExitNode() to return true/false. It should then remove any non-user blocks, but keep any user blocks or composite blocks that contain user blocks.

Tchanders added a project: Anti-Harassment.
Tchanders moved this task from Untriaged to The Letter Song on the Anti-Harassment board.
dmaza updated the task description. (Show Details)Aug 27 2019, 10:44 PM

Change 525665 merged by jenkins-bot:
[mediawiki/extensions/TorBlock@master] Use GetUserBlock hook instead of GetBlockedStatus

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

dom_walden added subscribers: Niharika, dom_walden.EditedSep 2 2019, 3:44 PM

By setting $wgTorDisableAdminBlocks = true (which is default), any IP blocks that coincide with Tor exit nodes will not get applied, including System blocks.

User blocks, or composite blocks containing user blocks, still get applied. This includes cookie blocks.

@Niharika It should be noted that TorBlocks does not block Special:PasswordReset. Perhaps that should be raised as a bug, as otherwise there does not seem to be a way to block anon. Tor users from Special:PasswordReset.

Also, the default of the extensions is to not apply Tor blocks to logged in users (they have the torunblocked right). But, IP blocks applying to those users are still removed. Logged in users using Tor might find themselves unblocked, unless they have blocks applying to their username.

On WMF wikis, I believe $wgTorDisableAdminBlocks = false and only users in the ipblock-exempt group have the torunblocked right, so this won't affect production.

But, it will change the behaviour of default installs.

But, it will change the behaviour of default installs.

@dom_walden It was our intention to keep the conditions for removing a block the same with this patch (so IP blocks are removed because they were removed before). Is there something that we've inadvertently changed?

That said, the one thing that should have changed is that previously composite blocks would never be kept, because they are always type IP, whereas now they will be kept if they contain a user block.

But, it will change the behaviour of default installs.

@dom_walden It was our intention to keep the conditions for removing a block the same with this patch (so IP blocks are removed because they were removed before). Is there something that we've inadvertently changed?

Previously, TorBlocks did not remove IP blocks (reading the previous code, I'm not exactly sure what it was meant to do :P). Which is possibly a bug your patch has fixed.

Thanks @dom_walden for explaining. I agree that it seems like we have fixed a bug here. I think we should make a note that we found the behavior wasn't as expected and we fixed it with this ticket on the extension talk page (https://www.mediawiki.org/wiki/Extension_talk:TorBlock) so third party users who might be active there can see it.

Thanks @dom_walden for explaining. I agree that it seems like we have fixed a bug here. I think we should make a note that we found the behavior wasn't as expected and we fixed it with this ticket on the extension talk page (https://www.mediawiki.org/wiki/Extension_talk:TorBlock) so third party users who might be active there can see it.

OK. I'll move this along.

Who should update the documentation?

dbarratt closed this task as Resolved.Sep 5 2019, 5:24 PM