Page MenuHomePhabricator
Create Task
Maniphest T211048

Audit classes that override Action::requiresUnblock method
Closed, ResolvedPublic
Actions

Description

The requiresUnblock method on an action returns true if a user can be blocked from the action; otherwise it returns false.

The default implementation, on Action, returns true. Some subclasses of Action override it to return false. Some subclasses of these subclasses should re-override it to return true, but don't (example in T210953).

This was not a problem until T208862, which checks requiresUnblock in Title::getUserBlock, meaning that it is important that descendants of Action return the correct result.

Before we can go ahead with T208862, we need to do an audit of all subclasses of (subclasses of Action for which requiresUnblock returns false), and possibly correct some of them.

Related Objects
Search...

Event Timeline

Tchanders created this task.Dec 3 2018, 7:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2018, 7:50 PM
Tchanders added subscribers: Anomie, Addshore, Legoktm.EditedDec 3 2018, 8:44 PM

Method

  1. Search: https://codesearch.wmflabs.org/search/?q=requiresUnblock&i=nope&files=&repos=
  2. For each class where requiresUnblock returns false, search "extends <className>"; do this iteratively for all descendants until no more results

Results
Classes not in bold explicitly override requiresUnblock. Classes in bold are their subclasses and need checking.

  • (core) HistoryAction
    • (Wikibase) HistoryEntityAction false
    • (WikiLexicalData) WikidataPageHistory false
  • (core) InfoAction
  • (core) PurgeAction
    • (PurgeClickThrough) PurgeClickThrough false
    • (Flow) PurgeAction false
  • (core) RawAction
  • (core) SpecialPageAction
  • (core) WatchAction
    • (core) UnwatchAction false
  • (core) SpecialChangeEmail
  • (core) SpecialRandomInCategory
  • (core) SpecialRedirect
  • (OATHAuth) SpecialDisableOATHForUser
  • (OATHAuth) SpecialOATHDisable
  • (OATHAuth) SpecialOATHEnable
  • (CentralAuth) SpecialGlobalRenameProgress
  • (CentralAuth) SpecialGlobalRenameRequest
  • (CirrusSearch) Dump
  • (CiteThisPage) SpecialCiteThisPage
  • (EducationProgram) ViewAction
    • (EducationProgram) ViewCourseAction false
    • (EducationProgram) ViewOrgAction false
    • (ProofreadPage) PageViewAction false
  • (Wikibase) ViewEntityAction
    • (Wikibase) EditEntityAction
      • (Wikibase) SubmitEntityAction true
    • (WikibaseLexeme) ViewLexemeAction false
    • (WikibaseMediaInfo) ViewMediaInfoAction false
  • (WikibaseQualityConstraints) CheckConstraintsRdf

@Addshore @Anomie @Legoktm - do you have any thoughts about whether any of these need updating?

Anomie added a comment.Dec 3 2018, 9:00 PM
In T211048#4795558, @Tchanders wrote:
  1. For each class where requiresUnblock returns false, search "extends <className>"; do this iteratively for all descendants until no more results

Keep in mind PHP namespaces when you're doing that.

Tchanders added a comment.Dec 3 2018, 9:20 PM

@Anomie Good point. searching "extends.*<className>" throws up one more result (PurgeAction in Flow) - updated the list above.

• TBolliger added a project: MediaWiki-User-management.Dec 3 2018, 9:57 PM
• TBolliger moved this task from Backlog to User blocking on the MediaWiki-User-management board.
Tchanders added a comment.Dec 4 2018, 6:44 PM

I've updated the list of classes that don't explicitly override requiresUnblock to state the return value that they inherit (see above).

• TBolliger added subscribers: Lydia_Pintscher, • TBolliger.Dec 4 2018, 10:22 PM
In T211048#4795558, @Tchanders wrote:
  • (core) HistoryAction
    • (Wikibase) HistoryEntityAction false
    • (WikiLexicalData) WikidataPageHistory false

From what I've found, HistoryAction in core just prints the history of a page. Viewing history should not be blocked for any user for any type of block.

James F last touched HistoryEntityAction, maybe he can give more info.

  • (core) PurgeAction
    • (PurgeClickThrough) PurgeClickThrough false
    • (Flow) PurgeAction false

PurgeClickThrough generates this page, which is functional for blocked users:

Screen Shot 2018-12-04 at 2.05.26 PM.png (928×1 px, 171 KB)

I feel confident in saying that it is safe for blocked users to purge — both via this tool for wiki pages as well as for Flow pages/threads.

  • (core) WatchAction
    • (core) UnwatchAction false

Blocked users should be able to watch and unwatch any page.

  • (EducationProgram) ViewAction
    • (EducationProgram) ViewCourseAction false
    • (EducationProgram) ViewOrgAction false
    • (ProofreadPage) PageViewAction false

We don't need to worry about the EducationProgram — T125618: Deprecate and remove the EducationProgram extension from Wikimedia servers after June 30, 2018

If we need to change it, prevent a blocked user from performing an action.

  • (Wikibase) ViewEntityAction
    • (Wikibase) EditEntityAction
      • (Wikibase) SubmitEntityAction true
    • (WikibaseLexeme) ViewLexemeAction false
    • (WikibaseMediaInfo) ViewMediaInfoAction false

@Lydia_Pintscher , @Addshore — can you comment on the intended behavior of these extensions? I assume the view actions should be allowed for blocked users but the SubmitEntityAction should be prohibited, but I'm not familiar enough with Wikibase to know for sure. Thank you!

Tchanders mentioned this in T210953: Wikidata is editable for blocked users.Dec 5 2018, 3:13 PM
Tchanders mentioned this in T208862: Title::checkUserBlock should call User::isBlockedFrom for every action that requires unblock.
Tchanders moved this task from Ready to In progress on the Anti-Harassment (AHT Sprint 34) board.Dec 5 2018, 3:24 PM
• TBolliger edited projects, added Anti-Harassment (AHT Sprint 35); removed Anti-Harassment (AHT Sprint 34).Dec 5 2018, 8:05 PM
• TBolliger moved this task from Ready to In progress on the Anti-Harassment (AHT Sprint 35) board.
Reedy subscribed.Dec 7 2018, 7:12 AM

For EducationProgram the extension is undeployed, so for all intents and purposes as you say, you don't need to do anything with it

Reedy added projects: Wikidata, Wikidata-Campsite.Dec 7 2018, 7:13 AM
Lydia_Pintscher moved this task from Incoming to Ready to estimate on the Wikidata-Campsite board.Dec 10 2018, 10:02 AM
Lucas_Werkmeister_WMDE moved this task from Ready to estimate to Wikidata-Campsite-Iteration-∞ (On Hold) on the Wikidata-Campsite board.Dec 11 2018, 3:19 PM
Lucas_Werkmeister_WMDE edited projects, added Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)); removed Wikidata-Campsite.
dbarratt subscribed.EditedDec 17 2018, 8:30 PM

EditEntityAction should be changed to true (which is what caused T210953) otherwise this looks correct.

This was fixed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/479874 so if this is the only change then it all looks good to me!

Addshore triaged this task as Medium priority.Dec 18 2018, 2:37 PM
Addshore moved this task from incoming to in progress on the Wikidata board.Dec 19 2018, 12:47 PM
• TBolliger edited projects, added Anti-Harassment (Alef — א); removed Anti-Harassment (AHT Sprint 35).Jan 2 2019, 7:48 PM
• TBolliger moved this task from Ready to In progress on the Anti-Harassment (Alef — א) board.
• TBolliger added a comment.Jan 3 2019, 9:14 PM

@Addshore — any update? Our team can't proceed without your input on this.

dbarratt reassigned this task from Tchanders to Addshore.Jan 3 2019, 9:20 PM
Restricted Application added a project: User-Addshore. · View Herald TranscriptJan 3 2019, 9:20 PM
Addshore moved this task from To Do (prioritised from top to bottom) to Doing on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.Jan 4 2019, 12:37 PM
Addshore moved this task from Doing to Done on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.Jan 4 2019, 1:37 PM

All of the Wikibase actions look fine now.

Tchanders added a comment.Jan 4 2019, 2:36 PM

Thank you @Addshore

Tchanders moved this task from In progress to Done on the Anti-Harassment (Alef — א) board.Jan 4 2019, 2:37 PM
• TBolliger closed this task as Resolved.Jan 4 2019, 5:58 PM
• TBolliger closed subtask T212149: Mention Action::requiresUnblock method breaking changes on the 1.33 breaking changes notes as Resolved.Jan 14 2019, 7:35 PM
Aklapper removed subscribers: • TBolliger, Anomie.Oct 16 2020, 5:41 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