Page MenuHomePhabricator

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

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.

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
  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.

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

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

  • (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:

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!

Reedy added a subscriber: Reedy.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

dbarratt added a subscriber: dbarratt.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 Normal 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

@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

All of the Wikibase actions look fine now.

TBolliger closed this task as Resolved.Jan 4 2019, 5:58 PM