Directory

⚓ T290790 Group OAuth grants by riskiness
Page MenuHomePhabricator

Group OAuth grants by riskiness
Closed, ResolvedPublic

Description

It would be good to group OAuth grants in terms of risk. This could be used to:

  • Highlight risky grants in the consumer review interface, e.g. with a different color, to avoid OAuth admins accidentally granting risky requests without proper review.
  • Group grants in the consumer proposal interface (T58022), to orient developers in terms of how much scrutiny they might expect, and to simplify the interface by hiding grants that are rarely needed.
  • In the future, maybe approve non-risky proposals without review.

Grants could be grouped into the following categories:

  • Not risky: permissions which most users have (so whatever a malicious actor could achieve with an app, could just as easily be achieved with spam accounts) - edit, move, upload, patrol, etc. This would include the grants basic, editpage, createeditmovepage, uploadfile, uploadeditmovefile, patrol, rollback, shortenurls, and the two OAuth-specific authentication-only grants (one of which also shares the user's email address, but that's common these days). Also viewmywatchlist and editmywatchlist as the watchlist isn't particularly sensitive information. Also privateinfo which sounds scare but in practice the only thing it's used for is email addresses and ReadingLists lists. Also highvolume which can be used for vandalism, but an app with access to many users could edit at high volumes anyway, even if it does not have permission to edit at high volume with a single user, so adding this permission doesn't really increase risk.
  • Vandalism risk: permissions which typically require admin or similar access, and can be used to cause havoc but not for anything truly damaging - deletion, blocking, page protection, editing protected pages. This would include the grants editprotected, editinterface, blockusers, viewdeleted, delete, protect, mergehistory, globalblock, createlocalaccount.
  • Security or privacy risk: permissions that require higher-than-admin access, and can do serious damage - JS editing, checkuser, oversight. This would include the grants import, editmycssjs, editsiteconfig, viewrestrictedlogs, oversight, checkuser. Also setglobalaccountstatus (account locking) because it can be used to hamper a security response by locking out all other accounts with the same permission. Also editmyoptions because it is hard to predict what the options will include (e.g. for Wikitech at one point it included SSH keys). Also sendemail because it can be used for social engineering (after T266969: Should emails sent via OAuth be identified as such? is fixed, this could go back to not risky). Also createaccount because it includes the tboverride-account and override-antispoof rights which can be used for social engineering.
  • Internal: permissions that are too risky to hand out to anyone and are only used in Wikimedia production infrastructure. This would include the grants oauthmanageownclient and oath.

Implementation would be similar to $wgGrantPermissionGroups: a core configuration key which maps grants to groups (except the set of groups would not be extensible here), and an extension.json key for adding custom grants.

Event Timeline

@JJMC89 what vandalism risk do you see with createlocalaccount?

It grants centralauth-createlocal, which is used to bypass account creation blocks and is restricted to administrators.

I think it would be helpful for discussions like this one to enumerate which exact grants we each see as potentially dangerous. For me, I think that list is:

I didn't put those in any particular order. I just read the table at https://meta.wikimedia.org/wiki/Special:ListGrants from top to bottom and listed them as I found them. I think they largely fall into these categories:

  • rights that allow changing how an individual account functions (css, js, options)
  • rights that allow changing how things function for everyone on a wiki (interface, site config)
  • rights that allow viewing/modifying restricted content (my watchlist, logs, suppressed, CU)
  • rights that allow non-public communication (email)
  • rights that allow changing authentication/authorization (account creation overrides, account oauth client management, oath)

The differences from the list in the task description:

  • editmyoptions, which I thought non-risky but I guess it's hard to know for sure when an extension puts something sensitive in user preferences (like SSH keys on wikitech).
  • createaccount (good point)
  • watchlist (I don't see the harm)

See also T330540#8646592 - delete and mergehistory might deserve a separate category because they can be used for highly disruptive, hard-to-undo vandalism. Maybe protect / editprotected too, via vandalism of templates that are transcluded on lots of pages?

And maybe editinterface should be in the security risk section, just in case there are system messages with raw HTML that we haven't found yet.

Incorporated the comments.

Change 910822 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] [WIP] Group grants by riskiness

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

I think setglobalaccountstatus is significantly riskier than blockusers and that setglobalaccountstatus has a considerable security risk attached. I have two main reasons for this. First, blocking users always affects a single wiki, which lowers the impact of maliciously installed blocks. While it is not possible for admins to unblock themselves (T150826), the standard escalation system (=get a steward) works, and it would work even if stewards were themselves impacted by the malicious blocks, as stewards (and some other global functionaries) continue to have the unblockself right. The maximum derailing impact I can imagine with blocks is that all local admins plus stewards are blocked, and even that merely means whichever steward notices this first needs to first unblock themselves before taking other actions.

On the other hand, the story is different with setglobalaccountstatus. Locking an account cannot be self-reversed because of how global lock is implemented (it logs you out and makes it impossible for you to log back in). In a theoretical case when all stewards got locked, they would be incapacited until and unless a sysadmin intervenes and manually sets gu_locked = 0 from the database. This means an incident involving locks would have to be escalated to the sysadmin (and/or WMF) level. Furthermore, because global locks are global, they are naturally more sensitive.

This means an incident involving locks would have to be escalated to the sysadmin (and/or WMF) level.

That's definitely worse than what you can do with blocks, but still not serious/lasting damage IMO.

This means an incident involving locks would have to be escalated to the sysadmin (and/or WMF) level.

That's definitely worse than what you can do with blocks, but still not serious/lasting damage IMO.

Definitely not lasting damage (I think lasting damage is fairly difficult to make when it comes to on-wiki actions), but more difficult to recover, yes. Whether damage that cannot be handled by what's available on-wiki is serious is up to discussion, but I think calling it meets certain definition of "serious".

Somehow indicating to app authors which grants they shouldn't request unless they are sure they need them is one of the more useful low-effort OAuth tasks. We get a lot of Outreachy-related app requests with all kinds of highly sensitive grants checked.

Change 971338 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] BotPasswords: Show risk level of grants

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

Change 971340 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/CentralAuth@master] Declare grant risk levels

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

Change 971341 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GlobalBlocking@master] Declare grant risk levels

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

Change 971342 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/OATHAuth@master] Declare grant risk levels

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

Change 971346 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Declare grant risk levels

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

Change 971289 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/UrlShortener@master] Declare grant risk levels

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

Change 971348 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/AntiSpoof@master] Declare grant risk levels

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

Change 971351 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/TitleBlacklist@master] Declare grant risk levels

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

Change 971348 abandoned by Gergő Tisza:

[mediawiki/extensions/AntiSpoof@master] Declare grant risk levels

Reason:

ExtensionProcessor seems incapable of dealing with this. I'll set it in core instead.

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

Change 971351 abandoned by Gergő Tisza:

[mediawiki/extensions/TitleBlacklist@master] Declare grant risk levels

Reason:

ExtensionProcessor seems incapable of dealing with this. I'll set it in core instead.

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

Change 971348 restored by Gergő Tisza:

[mediawiki/extensions/AntiSpoof@master] Declare grant risk levels

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

Change 971351 restored by Gergő Tisza:

[mediawiki/extensions/TitleBlacklist@master] Declare grant risk levels

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

Change 971551 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/OAuth@master] Declare grant risk levels

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

Change 971553 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] htmlform: Support HTML tooltips in checkmatrix

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

Change 971622 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/OAuth@master] Show risk level of grants

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

Change 971553 merged by jenkins-bot:

[mediawiki/core@master] htmlform: Support HTML tooltips in checkmatrix

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

Also privateinfo as it's about private information, although this is debatable (we consider OAuth's authentication+email grant not risky, even though the email is also private).

I reviewed privateinfo because someone applied for a grant. As far as I can tell the viewmyprivateinfo permission it provides is only used for the email address, and for reading lists in the ReadingLists extension, (which was a bad idea and should have used a dedicated user right, but isn't super sensitive). OAuth for some reason thinks that realname access also requires it, but it actually doesn't. So I think this isn't really sensitive.

Change 910822 merged by jenkins-bot:

[mediawiki/core@master] authz: Group grants by riskiness

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

Change 971338 merged by jenkins-bot:

[mediawiki/core@master] BotPasswords: Show risk level of grants

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

Change 971289 merged by jenkins-bot:

[mediawiki/extensions/UrlShortener@master] Declare grant risk levels

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

Change 971346 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Declare grant risk levels

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

Change 971551 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Declare grant risk levels

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

Change 971341 merged by jenkins-bot:

[mediawiki/extensions/GlobalBlocking@master] Declare grant risk levels

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

Change 971340 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Declare grant risk levels

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

Change 971342 merged by jenkins-bot:

[mediawiki/extensions/OATHAuth@master] Declare grant risk levels

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

Change 971622 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Show risk level of grants

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

Change 971348 merged by jenkins-bot:

[mediawiki/extensions/AntiSpoof@master] Declare grant risk levels

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

Change 971351 merged by jenkins-bot:

[mediawiki/extensions/TitleBlacklist@master] Declare grant risk levels

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

matmarex removed a project: Patch-For-Review.
matmarex subscribed.

Change 996785 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] grants: Fix risk rating of pivateinfo

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

Change 996785 merged by jenkins-bot:

[mediawiki/core@master] grants: Fix risk rating of 'privateinfo'

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

Change 1009863 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/CheckUser@master] Add grant configuration

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

Change 1009863 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Add grant configuration

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