Page MenuHomePhabricator

VisualEditor, MobileFrontend, and other tools using action=edit do not auto-block IP addresses
Open, Needs TriagePublic

Description

If a username is blocked and auto-block of IPs is enabled, if that user edits from a different IP address with the VisualEditor this does not cause this different IP address to be blocked.

Reproduction:

  1. Login as $user from one IP address[1]
  2. Go to Special:Block and block $user (need admin perms); make sure "Automatically block the last IP address used by this user, and any subsequent IP addresses they try to edit from" is checked
  3. In Special:BlockList you will see two rows in the table, the block for $user and another of the form "Autoblock #nnnnn"
  4. From a different IP address, login as $user and with VisualEditor as the default editor[2] attempt to edit an article (make sure you aren't taken to Source Editor first); you will be blocked
  5. In Special:BlockList you will not see any new rows in the table
  6. Logout and attempt to edit the article again (you may need to clear your cookies if the wiki sets a block cookie)

Actual:
You won't be blocked from editing from the second IP address.

Expected:
If our documentation is correct, attempting to edit from VisualEditor as the blocked $user should also block any new IP addresses they are using.

This does happen when you use the Source Editor instead in step 4. This leads me to believe it is related to VisualEditor (although a similar bug appears to afflict MobileFrontend, which I will raise separately.)

Even if, in step 4, you attempt to submit the edited article, this still does not cause the IP to be blocked.

I have seen this happen for both sitewide blocks and partial page blocks.

Speculation:
In mediawiki core includes/EditPage.php there is a function "spreadAnyEditBlock()" which appears to cause any new IP addresses to be blocked. Perhaps VisualEditor also needs to call this. I don't really understand the code well enough though...

Environment Tested:
https://test.wikipedia.org
MediaWiki 1.33.0-wmf.17 (484a1d0) 19:23, 14 February 2019
VisualEditor 0.1.1 (5ff1ea4) 22:04, 11 February 2019

Notes:

  1. I was using a VPN for the first IP address
  2. I don't know if you can set this for a user...

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 15 2019, 2:30 PM
dom_walden updated the task description. (Show Details)Feb 15 2019, 2:34 PM
dom_walden updated the task description. (Show Details)Feb 15 2019, 2:49 PM
dom_walden added a comment.EditedFeb 15 2019, 3:12 PM

If you are logged in as a blocked user on a new IP and attempt to edit from the mobile version of a site (without the VisualEditor extension), this also does not cause the new IP address to be blocked. I don't know if I should raise this as a separate bug for MobileFrontend, or whether both bugs have the same root causes.

matmarex moved this task from To Triage to External and Administrivia on the VisualEditor board.
matmarex added a subscriber: matmarex.

VisualEditor and MobileFrontend both use the action=edit API for saving changes.

Change 491300 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] API: Spread autoblocks from action=edit and action=move

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

Change 491300 merged by jenkins-bot:
[mediawiki/core@master] API: Spread autoblocks from action=edit and action=move

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

Anomie added a subscriber: Anomie.Feb 20 2019, 8:09 PM

The fix should be deployed to Wikimedia wikis with 1.33.0-wmf.19 or later.

This seems resolved as far as I'm concerned. I'll leave it to the VE team to close it since I know they like to do additional verification first.

Thank you for fixing this!

We mostly use our "additional verification" process for things somebody on the team worked on (and that are tagged with VisualEditor (Current work)). I was going to just close this task, but on second thought, this could probably use proper QA verification. Especially since VisualEditor uses a wrapper around the action=edit API.

matmarex renamed this task from VisualEditor does not auto-block IP addresses to VisualEditor, MobileFrontend, and other tools using action=edit do not auto-block IP addresses.Feb 20 2019, 8:24 PM
matmarex edited projects, added VisualEditor (Current work); removed VisualEditor.
matmarex moved this task from Incoming to QA on the VisualEditor (Current work) board.
Esanders moved this task from Inbox to Low Priority on the Editing QA board.Feb 22 2019, 7:10 PM
EvanProdromou added a subscriber: EvanProdromou.

I think we are done with this ticket. Tag us again if there are problems.

@dom_walden is T196575 a duplicate of this task or a different issue?

@dom_walden is T196575 a duplicate of this task or a different issue?

I don't know. I assume setting cookies and spreading autoblocks are different, and it is possible to do one and not the other.

Restricted Application added a project: Core Platform Team. · View Herald TranscriptJul 24 2019, 2:59 PM

@dom_walden: Is it working for you now? :)

@dom_walden: Is it working for you now? :)

Thanks!

I see the autoblock being spread when I do the move and edit actions via the API (api.php?action=move and api.php?action=edit).

However, I don't see it spread when I attempt to edit a page with the VisualEditor while blocked, in the same way I do when attempting to edit a page with the source editor while blocked.

Presumably because simply clicking "Edit" on the page and loading the VisualEditor does not call action=edit.

I don't know if we want VisualEditor to mirror what the source editor does, and aggressively spread an autoblock as soon as a user attempts to edit a page.

Presumably because simply clicking "Edit" on the page and loading the VisualEditor does not call action=edit.
I don't know if we want VisualEditor to mirror what the source editor does, and aggressively spread an autoblock as soon as a user attempts to edit a page.

When you click "Edit" your browser makes two HTTP requests:
https://example.com/api.php?action=visualeditor&format=json&paction=metadata&page=Test&uselang=en&formatversion=2
and
https://example.com/api/rest_v1/page/html/Test?redirect=false&stash=true

I'm not sure if the first is cached or uncached. If it's uncached, then it should spread the autoblock. If it is cached, then it is like the request to "Read" and it should not spread the autoblock.
The second is to the RESTBase web service, which is unaware of the user (afaik).

Visual editor does make some other POST requests before publishing to cache the changes on the server. Since those are POST requests they could spread the autoblock. Otherwise it wont be spread until the actual publish takes place.

So the original patch in February made it so that VE (and mobile wikitext editor) would spread autoblocks when you tried saving your changes, but not when you opened the editor. Since then, we've improved our permission checks and now if you're blocked, you don't get an option to save (like the "view source" mode in old wikitext), so there's no way for the spreading to happen.

I am not sure what is actually the expected behavior. Should autoblocks even be spread when opening the editor? If so, we might need a separate API action for doing that… (in VE we can add that code to action=visualeditor, but this doesn't fix the issue for mobile wikitext editor). Or maybe action=query&prop=revisions should also spread autoblocks?

Hmm, I suppose this is more of a product decision for @Niharika

I am not sure what is actually the expected behavior. Should autoblocks even be spread when opening the editor? If so, we might need a separate API action for doing that… (in VE we can add that code to action=visualeditor, but this doesn't fix the issue for mobile wikitext editor). Or maybe action=query&prop=revisions should also spread autoblocks?

What's the case for the classic editors at the moment? I think that they don't spread the block when the editor is opened, is that correct?

I am not sure what is actually the expected behavior. Should autoblocks even be spread when opening the editor? If so, we might need a separate API action for doing that… (in VE we can add that code to action=visualeditor, but this doesn't fix the issue for mobile wikitext editor). Or maybe action=query&prop=revisions should also spread autoblocks?

What's the case for the classic editors at the moment? I think that they don't spread the block when the editor is opened, is that correct?

From experimenting, they do spread it as soon as it is opened.

Thanks @dom_walden for checking!
I am not entirely sure why the decision was made to spread the block as soon as the source editor is opened. If someone has context behind that, I'd like to look into it. It seems a little too aggressive to me.
Nevertheless, it makes sense for both the standard editors and VE to have the same behavior. I think it is Peter's call to say whether it is worth investing time in changing VE to match the behavior of the standard editors.

Looks like it probably copies the spreading of auto-blocks. That dates back to http://mediawiki.org/wiki/Special:Code/MediaWiki/99323; before that autoblocks were spread on every block check, and before that on every request.

JTannerWMF added subscribers: ppelberg, JTannerWMF.

@ppelberg it is your call to determine if this is high priority or should go in the freezer