Page MenuHomePhabricator

Allow OAuth 2 apps using client credentials flow to perform actions as the app's owner (i.e. be logged in)
Open, Needs TriagePublic

Description

Per the discussion on T417278 and T407987, the OAuth 2 client credentials flow could fulfill a role similar to bot passwords and owner-only apps, and – unlike them – it is a standard thing and allows us to take advantage of JWTs for access control.

However, our current implementation is that all requests authenticated by client credentials are treated as logged-out. We would like to add a new mode where they are treated as logged-in as the app's owner.

Requirements

(roughly in order of importance)

  • Do not change anything for existing client credentials apps (do not make them act as logged in) [T417278#11672588]
  • Add an explanation when choosing client credentials flow during app registration, since the proposed behavior, while allowed, is not typical [T417278#11634423 T407987#11674162]
  • Make client credentials and authorization code mutually exclusive for newly registered apps [T417278#11648812]
  • Relax review for client credentials apps to be more like owner-only apps [T417278#11650829]
  • Allow filtering app listings by whether the app uses client credentials or authorization code flow [someone said this somewhere, maybe in Slack]
  • ...

Related Objects

Event Timeline

pmiazga subscribed.

Moving to Needs refinement as this looks like something we're going to work on soon.

Per @daniel not having this is a pain point for some Pywikibot users - we do a cookie hack for owner-only consumers to have a gateway-acknowledged JWT on subsequent requests, but that doesn't help bots which move a lot across wikis: on each new domain their first request won't have a cookie and they will eventually get throttled. The current recommendation is for bot owners to add custom code to copy JWT cookies across domains, which is not that great. (We can't set the cookie on parent domains because then it will interfere with private wikis.)

@JTweed-WMF would be great if we could pick this up next quarter.

Change #1312282 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/OAuth@master] Allow OAuth 2 client credentials to perform actions as the app owner

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

Came up with a quick (but working) patch for this. Happy to abandon if Platform team would prefer to work on this instead!

A few ways to do this:

  • Just change retroactively how client credentials apps work. Existing apps might unexpectedly start being authenticated (which is probably fine - I don't think there's any situation in which you'd actively want to be anonymous), and their credentials might unexpectedly become more sensitive than the operator assumes (if some app requested client credentials but also somewhat sensitive grants; that wouldn't make sense since the grants wouldn't work for an anonymous user, but there are always users making nonsensical things under some mistaken assumption, especially if the tool is as poorly documented as the OAuth extension).
  • Make all past clients work as they did before, and new clients work non-anonymously, using some kind of cutoff (probably a timestamp like in @SD0001's patch).
  • Mark DB rows which use the old behavior (maybe via a special grants value, similar to how we use mwoauth-authonly) - slightly more complicated, can be done automatically for third parties without them having to set configuration flags.

There's also the question of whether we want to keep the B/C behavior indefinitely, or shut down the relevant clients at some point and drop B/C. For third parties the schema updater can do that; for Wikimedia we probably want to get a handle on how much they are used, first. -> T433165: [SPIKE] How many OAuth 2 apps use the client credentials flow?

Make client credentials and authorization code mutually exclusive for newly registered apps [T417278#11648812]

This is somewhat orthogonal to the other work, and can be done regardless of what we decide about client credentials being anonymous. Even with the current behavior, doesn't make a lot of sense to have an app with both client credentials and authorization code flow.

Mark DB rows which use the old behavior (maybe via a special grants value, similar to how we use mwoauth-authonly)

We could also just add a new oauth_registered_consumer column for miscellaneous data (as a JSON blob). Not worth it for this task alone, but we'll probably need it anyway if we want richer consumer info (T60193: OAuth consumer metadata is too limited).

Current client credentials based access tokens are not connected to the OAuth client that created them (in the sense that there is no way to query the database for things like "how many access tokens does this app have"). Revoking all access tokens for a given client probably doesn't work either?

I think that will be fixed naturally when we fix the anonymous-ness, but we should make sure that is the case.

Implementation plan, per the last product meeting:

  • Go with @SD0001's proposal of using a cutoff date for client registration, after which the behavior flips and requests are non-anonymous
    • This isn't super helpful for third parties (we don't want to change the behavior of existing apps, but we also don't want to force every existing installation to manually set a configuration flag), but also doesn't make the situation any worse. We are leaving this problem for another time.
  • Prevent future apps from having both client credential and auth code or refresh token grant. The patch does this already (thanks!) but we'll probably want to follow with some higher-level UI change like T434145: Simplified "front page" for OAuth app registration.

We might as well do T417268: Disallow client credentials grant type for non-confidential clients while we are at it.

Change #1312282 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Allow OAuth 2 client credentials to perform actions as the app owner

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

  • This isn't super helpful for third parties (we don't want to change the behavior of existing apps, but we also don't want to force every existing installation to manually set a configuration flag), but also doesn't make the situation any worse. We are leaving this problem for another time.

The config flag can be made more intuitive for third-parties by renaming it to just $wgOAuth2ClientCredentialsOwnerAuth (removed Cutover) which accepts true/false, and also happens to accept a cutover date.