Page MenuHomePhabricator

Fix flag set on block by SpecialBlock::processForm if $wgBlockAllowsUTEdit is false
Closed, ResolvedPublicJun 2 2020

Description

The flag ipb_allow_usertalk should be set to false for every block made on a wiki where $wgBlockAllowsUTEdit is false. This was inadvertently changed in 02cb7aefef02d - since then, the flag is true if the config is false. (This only affects blocks made via SpecialBlock::processForm.)

In other words, the block appears to let the blocked user edit their own talk page, but the config prohibits blocked users from editing their own talk pages. Luckily, the config "wins", since the config overrides the flag when checking if a blocked user can edit their talk page. So despite the bug, users are still blocked from their own talk page in practice.

However, in the log and on Special:BlockList, it looks as though the user can edit their own talk page. Also, if a wiki decides to change $wgBlockAllowsUTEdit from false to true, then users blocked before 02cb7aefef02d will still be blocked from editing their talk page, but those blocked after will now be able to edit their talk page.

We should fix the flag to match the config.

This is particularly needed ahead of T189073, in which we refactor SpecialBlock::processForm into a blocking service. Currently, extensions are setting the correct flags, but once they start calling the new service, they will be setting the wrong flag, unless we fix this first. We encountered this problem in T249562: Checkuser should allow reblocking users and tag pages even if users are already blocked.


SpecialBlock::processForm looks at the form field named 'DisableUTEdit', the config $wgBlockAllowsUTEditand whether the UserTalk namespace is blocked. It sets ipb_allow_usertalk based on these.

For the sake of testing and reviewing, here is what should happen (was discussed in T210475):

  • If the UserTalk namespace is not blocked, then ipb_allow_usertalk is always true
  • If the UserTalk namespace is blocked and $wgBlockAllowsUTEdit is false, then ipb_allow_usertalk is always false (this is the bug - it's currently always true)
  • Otherwise, ipb_allow_usertalk is determined by the form field

(NB all sitewide blocks and some partial blocks block the UserTalk namespace.)

Event Timeline

Change 596690 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/core@master] SpecialBlock: Fix flag set on block if $wgBlockAllowsUTEdit is false

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

What happens if the config value changes (or worse, is not consistently one or the other?) Should the existing values in the database be updated or should they remain as-is?

(Assuming this task is about the MediaWiki-Blocks code project, hence adding that project tag so other people who don't know or don't care about team tags can also find this task when searching via projects. Please set appropriate project tags when possible. Thanks! :)

@dbarratt It's a good question - I think it's worth discussing what should happen if the flag changes, since it doesn't seem to have been discussed elsewhere. Here's a task for that: T253051

For this task, we should restore the behaviour that existed in SpecialBlock::processForm before it was accidentally changed, since that will allow extensions to call the new service without changing their behaviour.

Change 596690 merged by jenkins-bot:
[mediawiki/core@master] SpecialBlock: Fix flag set on block if $wgBlockAllowsUTEdit is false

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

ARamirez_WMF set Due Date to Jun 2 2020, 4:00 AM.May 21 2020, 3:16 AM
ARamirez_WMF changed the subtype of this task from "Task" to "Deadline".
dom_walden added a subscriber: dom_walden.
  • If the UserTalk namespace is not blocked, then ipb_allow_usertalk is always true
  • If the UserTalk namespace is blocked and $wgBlockAllowsUTEdit is false, then ipb_allow_usertalk is always false (this is the bug - it's currently always true)
  • Otherwise, ipb_allow_usertalk is determined by the form field

I tested the Special:Block form in the cases of:

It behaved consistently with my understanding of the acceptance criteria above (although I still find it a bit confusing at times).

To be more systematic, I wrote a script using the block API (which also uses SpecialBlock::processForm) to create a number of blocks with different combinations of block parameters, with $wgBlockAllowsUTEdit = false and $wgBlockAllowsUTEdit = true.

The script queried the database and API (action=query&list=blocks) to check the correct value of ipb_allow_usertalk, based on the below logic (done as pseudocode):

if partial and namespace restriction not equals User_talk:
    expected ipb_allow_usertalk value = true

else if (sitewide or namespace restriction equals User_talk) and not $wgBlockAllowsUTEdit:
    expected ipb_allow_usertalk value = false

else:
    expected ipb_allow_usertalk value = value of allowusertalk parameter that was POSTed

which I think accurately replicates the acceptance criteria, but perhaps @Tchanders can double check for me.

One interesting thing: If I am partially blocking Eve with only a page restriction on "User_talk:Eve":

  • If $wgBlockAllowsUTEdit = false then ipb_allow_usertalk is true but Eve is blocked from "User_talk:Eve"
  • If $wgBlockAllowsUTEdit = true then the allowusertalk checkbox is disabled* and ipb_allow_usertalk is true but Eve is blocked from "User_talk:Eve"

I guess this is consistent with the acceptance criteria insofar as the User_talk namespace is not being blocked here. But it still might be a bit confusing.

(* With JS disabled you get a validation error if you submit with the box checked)

[...] which I think accurately replicates the acceptance criteria, but perhaps @Tchanders can double check for me.

That's correct.

One interesting thing: If I am partially blocking Eve with only a page restriction on "User_talk:Eve":

  • If $wgBlockAllowsUTEdit = false then ipb_allow_usertalk is true but Eve is blocked from "User_talk:Eve"
  • If $wgBlockAllowsUTEdit = true then the allowusertalk checkbox is disabled* and ipb_allow_usertalk is true but Eve is blocked from "User_talk:Eve"

I guess this is consistent with the acceptance criteria insofar as the User_talk namespace is not being blocked here. But it still might be a bit confusing.

That's a good point - have filed a task for it, but is probably low priority: T253382

Niharika closed this task as Resolved.Jul 31 2020, 9:46 PM