Page MenuHomePhabricator
Create Task
Maniphest T272079

PermissionManager:: 'edit' right check should also check for 'create' if the page does not exist
Closed, ResolvedPublic
Actions

Description

Currently EditPage calls PermissionManager::userCan for edit right and manually falls back to checking for 'create' right if the page does not exist. This fallback should be moved into PermissionManager itself and applied always.

Order of operation:

  • 1. Find all places where PermissionManager::userCan is used for 'edit' right, check if 'create' right is also checked, if not, figure out why and whether it should. We need to discuss the findings at this point and decide whether the idea is viable.
  • 2. Move the fallback into PermissionManager. Somewhere around here probably.
  • 3. Cover the new behavior by tests
  • 4. Simplify all the calling code in core and perhaps deployed extensions.

Details

Show related patches Customize query in gerrit

Event Timeline

• Pchelolo created this task.Jan 14 2021, 8:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 14 2021, 8:20 PM
• Pchelolo added a project: Platform Team Workboards (MW Expedition).Jan 14 2021, 8:20 PM
• Pchelolo moved this task from Unsorted pile to Authority pile on the Platform Team Workboards (MW Expedition) board.Jan 14 2021, 11:10 PM
• Pchelolo triaged this task as Low priority.Jan 19 2021, 6:29 PM
Aklapper removed a subscriber: Platform Team Workboards (MW Expedition).Jan 20 2021, 9:33 PM
• Pchelolo reassigned this task from Peter.ovchyn to roman-stolar.Jun 3 2021, 6:49 PM
• Pchelolo added a subscriber: Peter.ovchyn.
roman-stolar moved this task from Authority pile to Doing on the Platform Team Workboards (MW Expedition) board.Jun 7 2021, 1:53 PM
roman-stolar added a comment.EditedJun 8 2021, 1:29 PM

Known places where PermissionManager::userCan is used for 'edit' right:

  • Core (WikiImporter.php, PermissionManagerTest.php, UserAuthority.php)

Extensions:

  • BlueSpiceContextMenu (Edit.php, EditUserTalk.php)
  • BlueSpiceSocial (Entity.php, EntityList.php)
  • BlueSpiceExtendedSearch (Autocomplete.php, Query.php, SearchCenter.php)
  • BlueSpiceBookshelf (BookEditData.php, AddToBookTool.php, AddAddToBookEntry.php)
  • PageForms (PFFormEditAction.php, PFHelperFormAction.php)
  • Wikibase (RepoHooks.php, EditEntityAction.php, EntityContentDiffView.php, OutputPageEditability.php)
  • BlueSpiceInsertCategory (AddInsertCategoryAction.php, Category.php)
  • CollaborationKit (CollaborationHubContentHandler.php, CollaborationListContent.php, SpecialCreateCollaborationHub.php, SpecialCreateHubFeature.php)
  • Translate (TranslateSandboxTest.php, PageTranslationTaggingTest.php)
  • BlueSpiceArticleInfo (Flyout.php)
  • BlueSpiceSaferEdit (Base.php, SaferEditManager.php)
  • LiquidThreads (LqtView.php, Thread.php, TalkpageView.php)
  • BlueSpicePageTemplates (BSPageTemplateList.php, Extension.php)
  • BlueSpiceSocialProfile (Profile.php)
  • Commentbox (Hooks.php, SpecialAddComment.php)
  • Jade (PageEntityProposalSetStorage.php)
  • MassMessage (MassMessageHooks.php, SpecialCreateMassMessageList.php, SpecialEditMassMessageList.php)
  • PageSchemas (PSEditSchemaAction.php, PSEditSchema.php)
  • SpamDiffTool (SpamDiffTool.php, SpamDiffToolHooks.php)
  • TinyMCE (TinyMCEHooks.php, TinyMCEAction.php)
  • Video (VideoHistoryList.php)
  • BlueSpiceChecklist (BSApiChecklistTasks.php.php)
  • ContentTranslation (Hooks.php)
  • DeleteUserPages (DeleteUserPages.php)
  • ForcePreview (ForcePreview.php)
  • GettingStarted (BasePageFilter.php)
  • LogEntry (SpecialLogEntry.php)
  • PopcornEditor (PopcornEditorHooks.php)
  • SVGEdit (SVGEdit.hooks.php)
  • WikiLambda (ZObjectStore.php)

Skins:

  • BlueSpiceCalumma (MobileFeaturedActionsButton.php, MobileMoreMenu.php, FeaturedActionsData.php, EditButton.php)
  • BlueSky (BlueSkyTemplate.php)
  • MinervaNeue (MinervaPagePermissions.php)
  • Mirage (InterfaceMessageModule.php)

Also I'm investigating those list to checking for 'create' right if the page does not exist, in general it's looks like some places didn't have this check and other have, we just need to understand in what case it optional and it what required to didn't brake something when will move 'create' right check to PermissionManager.

• Pchelolo added a comment.Jun 8 2021, 1:49 PM

Can you make a list that lacks the 'create' right check, I suspect it was just forgotten, I can help to verify that.

roman-stolar added a comment.EditedJun 8 2021, 2:10 PM

@Pchelolo I plan to do it as a next step. Thanks for help!

roman-stolar added a comment.EditedJun 9 2021, 1:21 PM

Known places where PermissionManager::userCan is used for 'edit' right, but the 'create' rights didn't checked:

  • Core (PermissionManagerTest.php) - but it's just a direct test check.
  • Core (UserAuthority.php) - use PermissionManager::userCan under the UserAuthority::internalCan method, so it's similar idea with the same effect, probably there shouldn't be any additional check for 'create' right here.

Extensions:

  • BlueSpiceContextMenu (Edit.php, EditUserTalk.php) - check 'edit' right while converting IMenuItem to array in BSApiContextMenuTasks.php.
  • BlueSpiceSocial (Entity.php) - used when we want to return a list of actions available for user to do with Entity, 'edit' right check enabled only when config 'IsEditable' available.
  • BlueSpiceSocial (EntityList.php) - used in method EntityList::renderPreloadedEntities, have a check if Title exist but no check for 'create' right.
  • BlueSpiceExtendedSearch (Autocomplete.php, Query.php) - used in method setPageCreatable for both classes have check if Title exist and right check for 'createpage'. Is it similar to 'create' right?
  • BlueSpiceExtendedSearch (SearchCenter.php) - used when we execute SearchCenter to pass status if userCanExport.
  • BlueSpiceBookshelf (BookEditData.php) - used when we execute BookEditData in case it based on Title.
  • BlueSpiceBookshelf (AddToBookTool.php, AddAddToBookEntry.php) - used in method AddToBookTool::skipProcessing, check if Title exist, no check for 'create' right.
  • PageForms (PFFormEditAction.php, PFHelperFormAction.php) - used only MW 1.33+ when we display Edit action for current article.
  • Wikibase (RepoHooks.php) - used for modify line endings on history page and after the structured navigation links in SkinTemplates, some notice that we use here PermissionManager::quickUserCan it's just the same call of PermissionManager::userCan but with specific rules for DB.
  • Wikibase (EditEntityAction.php) - used to display undo form, check right for 'read' or 'edit' there.
  • Wikibase (EntityContentDiffView.php) - used when we get a header for a specified revision, some notice that we use here PermissionManager::quickUserCan.
  • Wikibase (OutputPageEditability.php) - used to check if OutputPage editable, some notice that we use here PermissionManager::quickUserCan.
  • BlueSpiceInsertCategory (AddInsertCategoryAction.php) - used in method AddInsertCategoryAction::skipProcessing, check if Title exist, no check for 'create' right.
  • BlueSpiceInsertCategory (Category.php) - used to make change link and icon.
  • CollaborationKit (CollaborationListContent.php) - used to determine if current user should be given the edit interface for a page.
  • Translate (TranslateSandboxTest.php, PageTranslationTaggingTest.php) - it's just a test, not sure if we needed there 'create' right check.
  • BlueSpiceArticleInfo (Flyout.php) - to collect data if user can edit.
  • BlueSpiceSaferEdit (Base.php) - used to check if user can edit.
  • BlueSpiceSaferEdit (SaferEditManager.php) - used to method SaferEditManager::saveUserEditing.
  • LiquidThreads (LqtView.php) - used to action with threads, some notice that we use here PermissionManager::quickUserCan.
  • LiquidThreads (TalkpageView.php) - used when we show the contents of the actual talkpage article if it exists, some notice that we use here PermissionManager::quickUserCan.
  • BlueSpicePageTemplates (Extension.php) - used in method Extension::onMessagesPreLoad to automatically modifies "noarticletext" message. Check when no context, doing right check for 'createpage'. Is it similar to 'create' right?
  • BlueSpiceSocialProfile (Profile.php) - used when will want to get actions, check 'edit' right if related title enabled.
  • Commentbox (Hooks.php, SpecialAddComment.php) - used with actions/interface related to comment, have a check if Title exist, no check for 'create' right.
  • MassMessage (MassMessageHooks.php) - used when adding content and styles, some notice that we use here PermissionManager::quickUserCan.
  • MassMessage (SpecialEditMassMessageList.php) - used in method SpecialEditMassMessageList::setParameter, have a check if Title exist.
  • PageSchemas (PSEditSchemaAction.php) - used only MW 1.33+ when we display Edit action for current article.
  • PageSchemas (PSEditSchema.php) - used when category has been selected and we need to displays an interface to let users create and edit the <PageSchema> XML.
  • SpamDiffTool (SpamDiffTool.php, SpamDiffToolHooks.php) - used with action/interface related to spam diff.
  • TinyMCE (TinyMCEHooks.php, TinyMCEAction.php) - used with action/interface related to TinyMCE.
  • Video (VideoHistoryList.php) - used in method VideoHistoryList::videoHistoryLine.
  • BlueSpiceChecklist (BSApiChecklistTasks.php.php) - used in method BSApiChecklistTasks::task_saveOptionsList.
  • ContentTranslation (Hooks.php) - used when load the new article campaign for VisualEditor if it's relevant.
  • DeleteUserPages (DeleteUserPages.php) - used when check for quick permission to delete user page, only MW 1.33+.
  • ForcePreview (ForcePreview.php) - used when forces unprivileged users to preview before saving.
  • GettingStarted (BasePageFilter.php) - used in method BasePageFilter::isAllowedPage.
  • LogEntry (SpecialLogEntry.php) - used when execute SpecialLogEntry, only MW 1.33+.
  • PopcornEditor (PopcornEditorHooks.php) - used to check if editor links could be triggered on this page.
  • SVGEdit (SVGEdit.hooks.php) - used to check if editor links could be triggered on this page.
  • WikiLambda (ZObjectStore.php) - used when we create or update a ZObject it in the Database.

Skins:

  • BlueSpiceCalumma (MobileFeaturedActionsButton.php, MobileMoreMenu.php, FeaturedActionsData.php, EditButton.php) - used used with action/interface related to feature action button/menu/data.
  • BlueSky (BlueSkyTemplate.php) - used when we want to get edit menu.
  • Mirage (InterfaceMessageModule.php) - used when we want to get body content of Message module.
gerritbot added a comment.Jun 9 2021, 6:08 PM

Change 699053 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/core@master] WIP: Move the fallback to check the 'create' right into PermissionManager when we checking 'edit' right.

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

gerritbot added a project: Patch-For-Review.Jun 9 2021, 6:08 PM
roman-stolar updated the task description. (Show Details)Jun 10 2021, 10:27 AM
roman-stolar updated the task description. (Show Details)
gerritbot added a comment.Jun 13 2021, 11:46 AM

Change 699529 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/Wikibase@master] Fix tests that corrupt by permission manager updates.

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

gerritbot added a comment.Jun 14 2021, 2:12 PM

Change 699529 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Make tests not fail when getNamespace is called more than once.

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.11; 2021-06-21).Jun 14 2021, 3:00 PM
gerritbot added a comment.Jun 14 2021, 3:03 PM

Change 699766 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/Wikibase@master] Make test not fail by adding create right in case that required it

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

gerritbot added a comment.Jun 14 2021, 5:01 PM

Change 699766 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Make test not fail by adding create right in case that required it

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

gerritbot added a comment.Jun 23 2021, 3:56 PM

Change 699053 merged by jenkins-bot:

[mediawiki/core@master] Move the fallback to check the 'create' right into PermissionManager when we checking 'edit' right.

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

roman-stolar updated the task description. (Show Details)Jun 23 2021, 3:57 PM
ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.12; 2021-06-28); removed MW-1.37-notes (1.37.0-wmf.11; 2021-06-21).Jun 23 2021, 4:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jun 23 2021, 4:11 PM
gerritbot added a comment.Jun 24 2021, 12:55 PM

Change 701384 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/CollaborationKit@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a project: Patch-For-Review.Jun 24 2021, 12:55 PM
gerritbot added a comment.Jun 24 2021, 1:01 PM

Change 701385 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/LiquidThreads@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 24 2021, 1:24 PM

Change 701387 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/MassMessage@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 24 2021, 2:11 PM

Change 701392 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/core@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 24 2021, 7:37 PM

Change 701387 merged by jenkins-bot:

[mediawiki/extensions/MassMessage@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 25 2021, 1:22 PM

Change 701541 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/Flow@master] Remove the fallback for create rights (covered in PM edit rights check throught getPermissionErrors method).

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

gerritbot added a comment.Jun 25 2021, 1:30 PM

Change 701544 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/Newsletter@master] Remove the fallback for create rights (covered in PM edit rights check throught getPermissionErrors method).

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

gerritbot added a comment.Jun 25 2021, 1:47 PM

Change 701551 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/VisualEditor@master] Remove the fallback for create rights (covered in PM edit rights check throught getPermissionErrors method).

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

gerritbot added a comment.Jun 25 2021, 2:28 PM

Change 701561 had a related patch set uploaded (by Roman Stolar; author: Roman Stolar):

[mediawiki/extensions/Wikibase@master] Remove the fallback for create rights (covered in PM edit rights check throught quickUserCan method).

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

gerritbot added a comment.Jun 29 2021, 3:01 PM

Change 701384 merged by jenkins-bot:

[mediawiki/extensions/CollaborationKit@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 29 2021, 3:21 PM

Change 701541 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Remove the fallback for create rights (covered in PM edit rights check throught getPermissionErrors method).

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

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.14; 2021-07-12); removed MW-1.37-notes (1.37.0-wmf.12; 2021-06-28).Jun 29 2021, 4:00 PM
gerritbot added a comment.Jun 29 2021, 5:18 PM

Change 701551 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Remove the fallback for create rights

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

gerritbot added a comment.Jun 30 2021, 3:12 PM

Change 701385 merged by jenkins-bot:

[mediawiki/extensions/LiquidThreads@master] Remove the fallback for create rights (covered in PM edit rights check).

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

gerritbot added a comment.Jun 30 2021, 3:16 PM

Change 701392 merged by jenkins-bot:

[mediawiki/core@master] Remove the fallback for create rights (covered in PM edit rights check). Remove key to i18n message, no longer being used.

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

gerritbot added a comment.Jun 30 2021, 3:39 PM

Change 701561 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Remove the fallback for create rights (covered by Authority::probablyCan)

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

Mainframe98 awarded a token.Jun 30 2021, 3:52 PM
gerritbot added a comment.Jun 30 2021, 4:17 PM

Change 701544 merged by jenkins-bot:

[mediawiki/extensions/Newsletter@master] Remove the fallback for create rights (covered in PM edit rights check throught getPermissionErrors method).

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

• Pchelolo closed this task as Resolved.Jun 30 2021, 4:26 PM
roman-stolar updated the task description. (Show Details)Jul 1 2021, 8:16 AM
roman-stolar moved this task from Doing to On the train on the Platform Team Workboards (MW Expedition) board.Nov 29 2021, 10:27 AM
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