Page MenuHomePhabricator
Create Task
Maniphest T210953

Wikidata is editable for blocked users
Closed, ResolvedPublic
Actions

Assigned To
• TBolliger
Authored By
Sjoerddebruin
Dec 2 2018, 6:37 PM
Tags
Referenced Files
F27363644: image.png
Dec 3 2018, 12:18 PM
Subscribers
abian
Addshore
• aezell
Aklapper
dbarratt
dmaza
gerritbot
View All 20 Subscribers
Tokens
"The World Burns" token, awarded by Liuxinyu970226."The World Burns" token, awarded by Addshore."The World Burns" token, awarded by abian.

Description

The following undesired situations are possible:

  1. A blocked anonymous user (IP address) can perform all kind of edits on Wikidata: https://www.wikidata.org/wiki/Special:Contributions/84.81.160.164
  2. A blocked registered or anonymous user can edit Wikibase entities: https://www.wikidata.org/wiki/Special:Contributions/Abian, https://www.wikidata.org/wiki/Special:Contributions/Crazykidd

Most of the identified accounts started to edit on November 30. See T210962.

Open questions:

  • Is this behavior Wikidata.org specific? Is it in the Wikibase extension?
  • Is T210830 the same?

Details

Show related patches Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 Resolved• TBolligerT210953 Wikidata is editable for blocked users
Restricted Task

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Sjoerddebruin triaged this task as Unbreak Now! priority.Dec 2 2018, 6:44 PM
Lydia_Pintscher moved this task from Incoming to Ready to estimate on the Wikidata-Campsite board.Dec 2 2018, 6:45 PM
Sjoerddebruin updated the task description. (Show Details)Dec 2 2018, 6:46 PM
Sjoerddebruin added a subscriber: Trijnstel.Dec 2 2018, 6:51 PM
Trijnstel added a subscriber: acl*stewards.Dec 2 2018, 7:04 PM
Lydia_Pintscher added a subscriber: Tarrow.Dec 2 2018, 7:26 PM
abian added a subscriber: MisterSynergy.Dec 2 2018, 7:39 PM
abian added a subtask: Restricted Task.Dec 2 2018, 7:57 PM
Tarrow added a comment.Dec 2 2018, 8:19 PM

Anyone, got any clues as to when the problem started?

abian added a comment.Dec 2 2018, 8:22 PM
In T210953#4792509, @Tarrow wrote:

Anyone, got any clues as to when the problem started?

Not for now. T210962 should let us estimate it.

MisterSynergy added a comment.Dec 2 2018, 8:30 PM

I authored T210830 on Nov 30 at around noon, based on an edit very early that day. That task is very likely about the same problem.

abian updated the task description. (Show Details)Dec 2 2018, 8:44 PM
Tarrow added a comment.Dec 2 2018, 8:53 PM

I looked though the commits deployed in the last two trains and nothing jumped out at me.

Someone could double check the editentity refactoring I guess but I was the reviewer of it and didn't notice anything out of place.

After this is done we should remove the comment on extensions/Wikibase/repo/includes/EditEntity/MediawikiEditEntity.php#476 this confused me for a moment

Addshore awarded a token.Dec 2 2018, 10:34 PM
Addshore added a comment.Dec 2 2018, 10:45 PM

I just tested on test.wikidata.org for both creating an item and editing and editing existing terms were successfully stopped for a blocked account.

edits to statement using so wbsetclaim and wbremoveclaims however went through

Sjoerddebruin added a comment.Dec 2 2018, 11:08 PM

Might be related to T182983.

Addshore added a comment.Dec 2 2018, 11:26 PM
In T210953#4792655, @Sjoerddebruin wrote:

Might be related to T182983.

I tested but nope.

It looks like reverting https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/471993/ from core fixes the issue

Addshore added subscribers: dbarratt, • aezell, Mooeypoo.Dec 2 2018, 11:28 PM
abian updated the task description. (Show Details)Dec 2 2018, 11:38 PM
abian updated the task description. (Show Details)
Addshore added a comment.EditedDec 2 2018, 11:43 PM

It looks like this stems from Title::checkUserBlock and then Action::requiresUnblock.
ViewEntityAction::requiresUnblock returns false and has done since 2012.
EditEntityView extends this, but doesnt override the methods, so has the same behaviour as a view...
This method has a confusing name...
I'm not yet sure why this didn't work while testing terms, perhaps it uses a different action? hmpf..

Addshore added a comment.Dec 2 2018, 11:45 PM

Patch https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/477179/

Addshore added a comment.Dec 2 2018, 11:46 PM

For deployed branch https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/477180/

Legoktm subscribed.Dec 3 2018, 12:01 AM
In T210953#4792662, @Addshore wrote:
In T210953#4792655, @Sjoerddebruin wrote:

Might be related to T182983.

I tested but nope.

It looks like reverting https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/471993/ from core fixes the issue

Since this is security sensitive, I'd rather revert the core patch since we there's a decent chance other extensions are affected, and this is a significant breaking change.

Addshore added a comment.Dec 3 2018, 12:12 AM
In T210953#4792678, @Legoktm wrote:

Since this is security sensitive, I'd rather revert the core patch since we there's a decent chance other extensions are affected, and this is a significant breaking change.

I just checked through all of the other actions and everything else looks okay.
The issue in Wikibase was due to the edit action extending the view action.

I'll let you do the core revert now though and I'll head to bed and remove my .6 patch for Wikibase.
We will get the patch on master merged in EU morning time tomorrow.

dbarratt added a comment.Dec 3 2018, 12:12 AM
In T210953#4792670, @Addshore wrote:

It looks like this stems from Title::checkUserBlock and then Action::requiresUnblock.
ViewEntityAction::requiresUnblock returns false and has done since 2012.
EditEntityView extends this, but doesnt override the methods, so has the same behaviour as a view...
This method has a confusing name...
I'm not yet sure why this didn't work while testing terms, perhaps it uses a different action? hmpf..

It sounds like EditEntityView:: requiresUnblock() should return true since the user is required to be unblocked in order to preform the action. Does changing it to true resolve the issue?

Legoktm added a comment.Dec 3 2018, 12:36 AM

I've reverted https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/471993/ on wmf.6, and confirmed on test.wikidata.org that it fixes the issue. I'd appreciate it if someone could verify on wikidata.org itself before we make this task public.

dbarratt added subscribers: • TBolliger, dmaza, Tchanders.Dec 3 2018, 12:39 AM

@TBolliger FYI T208862 is being reverted, so plan accordingly. :)

jrbs subscribed.Dec 3 2018, 6:49 AM
Jalexander subscribed.Dec 3 2018, 6:58 AM
Addshore added a comment.Dec 3 2018, 12:18 PM
In T210953#4792684, @Legoktm wrote:

I've reverted https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/471993/ on wmf.6, and confirmed on test.wikidata.org that it fixes the issue. I'd appreciate it if someone could verify on wikidata.org itself before we make this task public.

Confirmed it is no longer an issue on wikidata / testwikidata

image.png (287×964 px, 34 KB)

I guess we can make this public now.
Though we should also merge the change in Wikibase master to avoid this being deployed unpatched with this weeks train

Addshore edited projects, added Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)); removed Wikidata-Campsite.Dec 3 2018, 1:04 PM
Addshore moved this task from To Do (prioritised from top to bottom) to Peer Review on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.
dmaza added a comment.Dec 3 2018, 2:29 PM

@Addshore @Legoktm are we ready to move forward with the patch on wikibase? What do we need to get https://gerrit.wikimedia.org/r/c/mediawiki/core/+/471993 (or a version of it) back on wmf.6?

Anomie subscribed.Dec 3 2018, 3:20 PM
In T210953#4794135, @dmaza wrote:

@Addshore @Legoktm are we ready to move forward with the patch on wikibase? What do we need to get https://gerrit.wikimedia.org/r/c/mediawiki/core/+/471993 (or a version of it) back on wmf.6?

It looks like @Legoktm wants a review of classes that subclass Action, specifically looking for situations where a subclass overrides requiresUnblock() to return false to ensure that that class and any of its subclasses don't need it to return true after all.

At a quick grep, classes overriding requiresUnblock() to return false are:

  • (core) PurgeAction
  • (core) HistoryAction
  • (core) SpecialPageAction
  • (core) WatchAction
  • (core) RawAction
  • (core) InfoAction
  • (CirrusSearch) CirrusSearch\Dump
  • (WikibaseQualityConstraints) WikibaseQuality\ConstraintReport\Api\CheckConstraintsRdf
  • (Wikibase) Wikibase\ViewEntityAction

I don't have a convenient way to find subclasses of these classes; someone who uses an IDE might be able to help with that.

Also, in extensions not deployed on Wikimedia wikis,

  • (EducationProgram) EducationProgram\ViewAction
dmaza added a comment.Dec 3 2018, 3:44 PM

Is it safe to assume that we only need to check on those 4 extensions ( CirrusSearch, WikibaseQualityConstraints, Wikibase, EducationProgram ) ?
I still think that @Addshore's patch should be merged regardless.

Lucas_Werkmeister_WMDE added a comment.Dec 3 2018, 3:50 PM

@Addshore’s patch is already merged (on master – the wmf.6 backport is abandoned).

Anomie added a comment.Dec 3 2018, 4:01 PM
In T210953#4794388, @dmaza wrote:

Is it safe to assume that we only need to check on those 4 extensions ( CirrusSearch, WikibaseQualityConstraints, Wikibase, EducationProgram ) ?

No, you also need to check any extension that subclasses any of the classes listed (directly or indirectly).

Lucas_Werkmeister_WMDE added a comment.Dec 3 2018, 4:05 PM

FWIW, I can tell you that CheckConstraintsRdf in WikibaseQualityConstraints is safe (it’s a read-only action), and I’m confident it has no subclasses (it would not make sense for any other extension to subclass it).

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".Dec 3 2018, 4:38 PM
Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptDec 3 2018, 4:38 PM
gerritbot subscribed.Dec 3 2018, 4:56 PM

Change 477307 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@master] Override requiresWrite in EditEntityAction

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

gerritbot added a project: Patch-For-Review.Dec 3 2018, 4:56 PM
• TBolliger added a project: Anti-Harassment (AHT Sprint 34).Dec 3 2018, 4:59 PM
Addshore moved this task from incoming to in progress on the Wikidata board.Dec 3 2018, 5:37 PM
gerritbot added a comment.Dec 3 2018, 5:48 PM

Change 477270 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update comment in ViewEntityAction

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.8; 2018-12-11).Dec 3 2018, 6:00 PM
WMDE-leszek mentioned this in T211038: Create integration/edge-to-edge ensuring blocked users cannot edit Wikibase entities.Dec 3 2018, 6:09 PM
gerritbot added a comment.Dec 3 2018, 6:17 PM

Change 477307 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Override requiresWrite in EditEntityAction

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

Tchanders mentioned this in T211048: Audit classes that override Action::requiresUnblock method.Dec 3 2018, 7:50 PM
Lydia_Pintscher mentioned this in T210830: Do not allow edits from the client for blocked users on the repository.Dec 3 2018, 8:28 PM
Jdforrester-WMF mentioned this in T211052: SlotRoleHandler's introduction breaks WBMI; edits now fail with "content is not allowed on page" error.Dec 3 2018, 8:30 PM
abian closed subtask Restricted Task as Resolved.Dec 4 2018, 10:24 AM
Addshore moved this task from Peer Review to Test (Verification) on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.Dec 4 2018, 10:59 AM
WMDE-leszek added a comment.Dec 4 2018, 1:53 PM

Shouldn't the priority of this also be lowered, or do we still consider it "Unbreak Now"? It is been a few days too much to call it "now" IMO.

Lydia_Pintscher lowered the priority of this task from Unbreak Now! to High.Dec 4 2018, 4:43 PM
Lydia_Pintscher moved this task from Test (Verification) to Done on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.
Tchanders added a comment.Dec 5 2018, 3:13 PM

Just pointing to T211048, where we're auditing the affected classes - any feedback very welcome!

Lucas_Werkmeister_WMDE added a comment.Dec 5 2018, 4:28 PM

@Tchanders FYI, in another change we noticed that we’d also failed to override requiresWrite – could that require a similar investigation?

Anomie added a comment.Dec 5 2018, 6:15 PM
In T210953#4800621, @Lucas_Werkmeister_WMDE wrote:

@Tchanders FYI, in another change we noticed that we’d also failed to override requiresWrite – could that require a similar investigation?

requiresWrite does not matter for this task.

Lucas_Werkmeister_WMDE added a comment.Dec 5 2018, 6:29 PM

Yes, but it’s a similar class of potential error, if I understand correctly, just not as critical.

• TBolliger closed this task as Resolved.Dec 5 2018, 7:15 PM
• TBolliger claimed this task.
• TBolliger moved this task from Ready to Done on the Anti-Harassment (AHT Sprint 34) board.

Marking as resolved because the immediate problem has been fixed. Anti-Harassment is working through T211048: Audit classes that override Action::requiresUnblock method.

Thank you.

Liuxinyu970226 awarded a token.Dec 9 2018, 7:17 AM
Liuxinyu970226 unsubscribed.
gerritbot added a comment.Dec 15 2018, 11:23 AM

Change 479874 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_29] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:23 AM

Change 479875 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_30] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:23 AM

Change 479876 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_31] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:23 AM

Change 479877 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_32] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:30 AM

Change 479874 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_29] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:30 AM

Change 479875 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_30] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:31 AM

Change 479876 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_31] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:32 AM

Change 479877 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_32] Override requiresUnblock in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:32 AM

Change 479878 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_29] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:32 AM

Change 479879 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_30] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:32 AM

Change 479880 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_31] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:33 AM

Change 479881 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@REL1_32] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:33 AM

Change 479878 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_29] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:33 AM

Change 479879 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_30] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:33 AM

Change 479880 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_31] Override requiresWrite in EditEntityAction

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

gerritbot added a comment.Dec 15 2018, 11:33 AM

Change 479881 merged by Addshore:
[mediawiki/extensions/Wikibase@REL1_32] Override requiresWrite in EditEntityAction

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

• chasemp added a project: Security.Feb 10 2020, 10:54 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:14 PM
Tchanders mentioned this in T250427: Create #MediaWiki-Blocks component.Apr 20 2020, 10:26 PM
Aklapper removed subscribers: Anomie, • TBolliger.Oct 16 2020, 5:40 PM
MarcoAurelio removed a subscriber: acl*stewards.Nov 28 2020, 12:01 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits