⚓ T349957 Allow bot credentials to be scoped to edit specific pages only

⚓ T349957 Allow bot credentials to be scoped to edit specific pages only
Page Menu
Phabricator
Create Task
Maniphest
T349957
Allow bot credentials to be scoped to edit specific pages only
Closed, Resolved
Public
Feature
Actions
Edit Task
Edit Related Tasks...
Create Subtask
Edit Parent Tasks
Edit Subtasks
Merge Duplicates In
Close As Duplicate
Edit Related Objects...
Edit Commits
Edit Mocks
Mute Notifications
Protect as security issue
Assigned To
SD0001
Authored By
SD0001
Oct 28 2023, 2:15 PM
2023-10-28 14:15:02 (UTC+0)
Tags
MediaWiki-extensions-OAuth
(Uncategorized)
MediaWiki-Core-AuthManager
(Backlog)
MediaWiki-Platform-Team
(Not planned (Patches welcome!))
MW-1.42-notes (1.42.0-wmf.12; 2024-01-02)
User-notice-archive
(Backlog)
Referenced Files
None
Subscribers
Aklapper
Asartea
Frostly
Izno
Jack_who_built_the_house
Nardog
Novem_Linguae
View All 13 Subscribers
Description
Feature summary
(what you would like to be able to do and where):
Allow bot passwords or owner-only OAuth consumers to be scoped to edit specific pages only.
Use case(s)
(list the steps that you performed to discover that problem, and describe the actual underlying problem which you want to solve. Do not describe only a solution):
To automate deployments of gadgets on-wiki, an interface-admin bot is required. Users who are not interface-admins may have merge access to the git repository where the gadget is developed, and thus also have access to the CD configuration. As these users are entrusted to manage the gadget, they should be able to trigger deployments. However, they should not be able to hijack the CD configuration and trigger edits to other sensitive pages like MediaWiki:Common.js.
Benefits
(why should this be implemented?):
This would enable setting up CD pipelines for gadgets, while keeping the attack surface to a minimum.
(Suggested by
@taavi
in a Discord discussion. cc:
@Soda
@Novem_Linguae
Details
Related Changes in Gerrit:
Subject
Repo
Branch
Lines +/-
Allow setting page restrictions on OAuth access grants
mediawiki/extensions/OAuth
master
+16
-0
Allow setting page restrictions on BotPassword grants
mediawiki/core
master
+165
-8
Refactor HTMLRestrictionsField to allow more restrictions to be added
mediawiki/core
master
+68
-78
Unmark HTMLRestrictionsField as stable to extend
mediawiki/core
master
+0
-1
Customize query in gerrit
Event Timeline
SD0001
created this task.
Oct 28 2023, 2:15 PM
2023-10-28 14:15:02 (UTC+0)
Restricted Application
added a project:
MediaWiki-Platform-Team
View Herald Transcript
Oct 28 2023, 2:15 PM
2023-10-28 14:15:03 (UTC+0)
Restricted Application
added a subscriber:
Aklapper
View Herald Transcript
Nardog
subscribed.
Oct 28 2023, 2:24 PM
2023-10-28 14:24:58 (UTC+0)
Asartea
subscribed.
Oct 28 2023, 2:27 PM
2023-10-28 14:27:59 (UTC+0)
Pppery
awarded a token.
Oct 28 2023, 2:42 PM
2023-10-28 14:42:33 (UTC+0)
Frostly
subscribed.
Oct 28 2023, 9:02 PM
2023-10-28 21:02:21 (UTC+0)
Izno
subscribed.
Oct 29 2023, 3:21 AM
2023-10-29 03:21:54 (UTC+0)
Nux
subscribed.
Oct 29 2023, 10:22 AM
2023-10-29 10:22:30 (UTC+0)
Comment Actions
Ci/cd configuration can be separated. For example I have a Jenkins installation of which configuration is not accessible to anyone. Jenkins pools GitHub and runs Wikiploy. That Wikiploy script can be in a separate repo if I need to. There is no problem in that.
I think a more interesting thing would be to have an account with access to only my own gadget. Like for example say I just want to push my gadgets to enwiki and for whatever reason people don't want me to change more popular and default gadgets.
larissagaulia
moved this task from
Inbox, needs triage
to
Not planned (Patches welcome!)
on the
MediaWiki-Platform-Team
board.
Oct 30 2023, 2:42 PM
2023-10-30 14:42:41 (UTC+0)
SD0001
added a comment.
Edited
Oct 30 2023, 7:27 PM
2023-10-30 19:27:20 (UTC+0)
Comment Actions
This could be implemented as follows:
Modify PermissionManager userHasRight() to take an optional third parameter which is the page id.
Add getAllowedPages() in PermissionManager and invoke it in userHasRight() if page id is passed. This function would call
getAllowedPages() in Session, which calls
getAllowedPages() in SessionBackend, which in turn calls
getAllowedPages() in SessionProvider, which is overridden by the provider implementation that checks if page id is allowed per the provider metadata
getAllowedPages() in BotPasswordSessionProvider
getAllowedPages() in SessionProvider in extensions/OAuth
Modify callers of userHasRight() to pass the page id for page-specific actions like edit, move, delete, and undelete.
Add bp_pages blob field to bot_passwords table (similar to bp_grants which is an array saved as blob). Modify SpecialBotPasswords, BotPassword and BotPasswordStore classes to handle the UI and db persistence.
Modify BotPasswordSessionProvider newSessionForRequest() to persist the pageids in session metadata.
Please let me know if I'm missing something.
Tgr
subscribed.
Edited
Oct 30 2023, 9:46 PM
2023-10-30 21:46:54 (UTC+0)
Comment Actions
I think it would be preferable to add such a feature to the grants system and not specifically bot passwords (a feature mainly intended for B/C and for non-OAuth wikis).
The MWRestrictions class has intentionally been written in an extensible manner so it could include arbitrary other data beyond IPs, without the need for a schema change. It has to fit into a blob (64K) though, which might be a problem depending on the intended use case / usage pattern. But usage patterns which require many pages would complicate things in other ways as well (you couldn't store the page list in memory so you'd have to do the deny-list check as a DB query within the permission check) so I think that's a reasonable limitation.
I don't think it makes much sense to add a page to userHasRight(), that's basically userCan(). Having two "can this action be applied to this page?" checks that give different results would be very confusing. It would be nice to replace usages of title-independent permission checks with title-dependent ones wherever possible, although one should be mindful of performance - title-dependent checks involve checking vastly more things. But the relevant checks for a bot editing a gadget page are already page-dependent, so I think what's really needed here is expose the page to the session - alongside Session::getAllowedUserRights(), have something like Session::userCan(), that's called from PermissionManager::checkPermissionHooks() or some similar place.
(I'll note that the reason such a change is at all possible is that SessionProvider is an abstract class wrapping an interface, and reusers are required to extend the abstract class, not implement the interface, a pattern I wish MediaWiki used more.)
I am a bit skeptical about this use case in general. If it's a default-enabled gadget, there doesn't seem to be much benefit to such a restriction, while it adds code complexity and maintenance burden. Otherwise, it's essentially an ask to differentiate lower-risk gadgets in terms of editing permissions, and a manual page list seems like an awkward way of doing that. Maybe there should just be a different user right for sitewide and non-sitewide gadgets.
Soda
added a comment.
Oct 30 2023, 11:57 PM
2023-10-30 23:57:58 (UTC+0)
Comment Actions
In
T349957#9293278
@Tgr
wrote:
I am a bit skeptical about this use case in general. If it's a default-enabled gadget, there doesn't seem to be much benefit to such a restriction, while it adds code complexity and maintenance burden. Otherwise, it's essentially an ask to differentiate lower-risk gadgets in terms of editing permissions, and a manual page list seems like an awkward way of doing that. Maybe there should just be a different user right for sitewide and non-sitewide gadgets.
I would argue that this feature would be beneficial even outside this specific usecases, since it would allow bot operators in general to follow the principle of least privilege. (for example, a bot editing a few report pages could be given edit access to those specific pages, so even in the unlikely scenario that the bot infrastructure is compromised (or some invalid formatting/api change triggers a bug), the only pages they can disrupt will be the specific pages they will have access to)
SD0001
added a comment.
Oct 31 2023, 4:22 PM
2023-10-31 16:22:54 (UTC+0)
Comment Actions
Thanks for the inputs
@Tgr
. I missed userCan(). MWRestrictions however seems to checked only at session creation, rather than at action level.
Tgr
added a comment.
Oct 31 2023, 5:43 PM
2023-10-31 17:43:11 (UTC+0)
Comment Actions
Yeah, it would still be the session provider's responsibility to pass the information, by storing the page list (or the serialized MWRestrictions object) as session metadata, or maybe storing the bot password ID. MWRestrictions is just a mechanism for storing the information. It would expand its scope a bit, it was imagined as a set of restrictions about the request (not the action), so separate storage would be maybe a little cleaner, but not so much cleaner as to be worth all the schema changes, IMO.
SD0001
added a comment.
Nov 1 2023, 6:33 PM
2023-11-01 18:33:52 (UTC+0)
Comment Actions
Okay. So I implemented the backend updates by including the serialized MWRestrictions in session metadata. The tricky part however seems to be the UI. HTMLRestrictionsField extends HTMLTextAreaField, and is also marked stable to extend (although there is no external use except for OAuth). I suppose we would change it to extend HTMLFormElement instead, but is there a precedence for an HTMLFormElement to actually represent two inputs?
Tgr
added a comment.
Nov 1 2023, 9:03 PM
2023-11-01 21:03:46 (UTC+0)
Comment Actions
In
T349957#9299336
@SD0001
wrote:
HTMLRestrictionsField extends HTMLTextAreaField, and is also marked stable to extend (although there is no external use except for OAuth).
I think that was a mistake and should be reverted. It doesn't really make sense to extend this class - it's not a fundamental component, it's a composite field meant for a specific purpose.
I suppose we would change it to extend HTMLFormElement instead, but is there a precedence for an HTMLFormElement to actually represent two inputs?
Depends on how you define "input". On the model level, it's a single input which returns a single MWRestrictions object. On the UI level, multiple input elements in a single HTMLFormField in a is not that uncommon, e.g. checkmatrix, selectandother/selectorother, autocompleteselect.
Jack_who_built_the_house
subscribed.
Nov 3 2023, 1:12 PM
2023-11-03 13:12:08 (UTC+0)
SD0001
claimed this task.
Nov 4 2023, 6:14 PM
2023-11-04 18:14:33 (UTC+0)
gerritbot
added a comment.
Nov 4 2023, 6:15 PM
2023-11-04 18:15:29 (UTC+0)
Comment Actions
Change 971572 had a related patch set uploaded (by SD0001; author: SD0001):
[mediawiki/core@master] Unmark HTMLRestrictionsField as stable to extend
gerritbot
added a project:
Patch-For-Review
Nov 4 2023, 6:15 PM
2023-11-04 18:15:30 (UTC+0)
Comment Actions
Change 971573 had a related patch set uploaded (by SD0001; author: SD0001):
[mediawiki/core@master] Refactor HTMLRestrictionsField to allow more restrictions to be added
gerritbot
added a comment.
Nov 4 2023, 6:15 PM
2023-11-04 18:15:33 (UTC+0)
Comment Actions
Change 971574 had a related patch set uploaded (by SD0001; author: SD0001):
[mediawiki/core@master] Allow setting page restrictions on BotPassword grants
gerritbot
added a comment.
Nov 4 2023, 6:15 PM
2023-11-04 18:15:50 (UTC+0)
Comment Actions
Change 971575 had a related patch set uploaded (by SD0001; author: SD0001):
[mediawiki/extensions/OAuth@master] Allow setting page restrictions on OAuth access grants
gerritbot
added a comment.
Nov 5 2023, 3:09 AM
2023-11-05 03:09:42 (UTC+0)
Comment Actions
Change 971572
merged
by jenkins-bot:
[mediawiki/core@master] Unmark HTMLRestrictionsField as stable to extend
ReleaseTaggerBot
added a project:
MW-1.42-notes (1.42.0-wmf.4; 2023-11-07)
Nov 5 2023, 4:00 AM
2023-11-05 04:00:40 (UTC+0)
gerritbot
added a comment.
Nov 23 2023, 5:21 PM
2023-11-23 17:21:08 (UTC+0)
Comment Actions
Change 971573
merged
by jenkins-bot:
[mediawiki/core@master] Refactor HTMLRestrictionsField to allow more restrictions to be added
ReleaseTaggerBot
edited projects, added
MW-1.42-notes (1.42.0-wmf.7; 2023-11-28)
; removed
MW-1.42-notes (1.42.0-wmf.4; 2023-11-07)
Nov 23 2023, 6:00 PM
2023-11-23 18:00:44 (UTC+0)
gerritbot
added a comment.
Dec 15 2023, 6:04 AM
2023-12-15 06:04:01 (UTC+0)
Comment Actions
Change 971574
merged
by jenkins-bot:
[mediawiki/core@master] Allow setting page restrictions on BotPassword grants
ReleaseTaggerBot
edited projects, added
MW-1.42-notes (1.42.0-wmf.10; 2023-12-19)
; removed
MW-1.42-notes (1.42.0-wmf.7; 2023-11-28)
Dec 15 2023, 7:00 AM
2023-12-15 07:00:45 (UTC+0)
gerritbot
added a comment.
Dec 19 2023, 7:20 AM
2023-12-19 07:20:13 (UTC+0)
Comment Actions
Change 971575
merged
by jenkins-bot:
[mediawiki/extensions/OAuth@master] Allow setting page restrictions on OAuth access grants
Maintenance_bot
removed a project:
Patch-For-Review
Dec 19 2023, 7:30 AM
2023-12-19 07:30:43 (UTC+0)
ReleaseTaggerBot
edited projects, added
MW-1.42-notes (1.42.0-wmf.12; 2024-01-02)
; removed
MW-1.42-notes (1.42.0-wmf.10; 2023-12-19)
Dec 19 2023, 8:00 AM
2023-12-19 08:00:41 (UTC+0)
SD0001
closed this task as
Resolved
Dec 23 2023, 7:40 AM
2023-12-23 07:40:22 (UTC+0)
taavi
added a project:
User-notice
Dec 23 2023, 10:54 AM
2023-12-23 10:54:26 (UTC+0)
Quiddity
subscribed.
Edited
Jan 4 2024, 7:09 PM
2024-01-04 19:09:15 (UTC+0)
Comment Actions
Re: Tech News - What wording and link(s) would you suggest as the content?
Please suggest 1-3 sentences with 1-2 links (perhaps a documentation page with a screenshot?), or add directly within the next ~27 hours (after which the edition will be frozen for translation). Thanks!
Quiddity
moved this task from
To Triage
to
Announce in next Tech/News
on the
User-notice
board.
Jan 4 2024, 7:09 PM
2024-01-04 19:09:50 (UTC+0)
Quiddity
moved this task from
Announce in next Tech/News
to
In current Tech/News draft
on the
User-notice
board.
Jan 12 2024, 7:53 PM
2024-01-12 19:53:53 (UTC+0)
Quiddity
moved this task from
In current Tech/News draft
to
Already announced/Archive
on the
User-notice
board.
Jan 19 2024, 12:45 AM
2024-01-19 00:45:51 (UTC+0)
Maintenance_bot
edited projects, added
User-notice-archive
; removed
User-notice
Jan 29 2024, 1:30 AM
2024-01-29 01:30:22 (UTC+0)
Log In to Comment
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
Code of Conduct
Disclaimer
CC-BY-SA
GPL
Credits