openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap
https://www.openstreetmap.org/
GNU General Public License v2.0
2.16k stars 908 forks source link

OAuth 2: Granting partial permissions not possible #4360

Open westnordost opened 10 months ago

westnordost commented 10 months ago

Problem

OAuth 2 does not allow granting permissions partially, which was possible with OAuth 1.0a.

This may scare off some privacy / security-aware users from using software that can e.g. create notes, upload GPX tracks etc. but doesn't need these permissions when these features are not used.

Description

Using authorization with OAuth 1.0a, it was possible for users to choose which of the permissions an application requests are granted. Which of these permissions were actually granted then could be queried with /permissions endpoint. This seems to be not possible with OAuth 2.0 scopes anymore.

As the author and maintainer of StreetComplete, I actually did get several requests and inquiries why the app needs the "create note" as well as the "read/write gpx tracks" permission.

As the latter is only used for a relatively minor and difficult to discover feature, it was made optional following https://github.com/streetcomplete/StreetComplete/issues/4122. I.e. the user is able to not grant this particular permission and the app would work normally.

AntonKhorev commented 10 months ago

It is possible, there's just no ui for that. Right now users would have to manually edit the scopes parameter in the url.

westnordost commented 10 months ago

I believe we mean something different. In the authorization step where the user selects on osm.org which permissions requested by the client he is going to grant. I do not mean that a client can request less permissions than for which the client has been registered for.

Note the checkboxes:

Authorization with OAuth 1.0a

oauth1

Authorization with OAuth 2.0

oauth2

tomhughes commented 10 months ago

I'm not sure it's a good idea to allow it though - the user has no way of knowing the effect of disabling permissions that the client has asked for after all.

I would have thought client authors would actually be against this because it will tend to create support load for them when people manage to break things by removing permissions.

woodpeck commented 10 months ago

This is one of my perennial peeves with the HOT tasking manager - if I want to look up some task that somebody has created, I need to grant the tasking manager the permission to "modify the map" which I never trusted it enough to do and which cannot be deselected.

OSMcha, on the other hand, also asks for the permission to "modify the map" but you can simply not grant that, and it will still work (unless you try anything that requires permissions not granted). I am not sure how gracefully it fails if you try.

westnordost commented 9 months ago

Well, client developers had to deal with this situation somehow in OAuth 1.0a. And they were able to deal with it very gracefully, by querying the permissions endpoint. The permissions endpoint still exists, though I am not sure for what it would be useful now. Or less gracefully, just show an error or force a re-login if any particular API call fails due to missing permissions. (StreetComplete does that in case of trying to use the GPX track feature without having granted that permission.)

It would be easier of course for client developers to not deal with this at all. But it would also be easier for (Android) app developers if an app would just have all the permissions it asks for on app install instead of the user being able to grant only a subset of what has been asked for. (As it had been the case before runtime app permissions were introduced in Android 6)

tomhughes commented 9 months ago

The other problem is that is somebody changes their mind their is no way for them to add the extra permissions - that is true for OAuth 1 as well I believe but at least there the client can start a new authorisation but with OAuth 2 that will wind up reusing the existing one unless the user revokes that on the server side.

westnordost commented 9 months ago

Hmm, maybe the preferred OAuth 2 way to do it would be that the client requests different tokens for different purposes.

E.g. HOT could request only the minimum it needs to function on login, and then when using the map-edit feature, it would ask for authorization for this specific permission upon using that feature. Because the user is likely already logged in in his browser at least from the last authorization request, the user is able to quickly authorize it with only one additional click. This is both convenient for the user and not that complex to handle for the client.

Now, for non-browser applications, this flow is a little less convenient. At least StreetComplete and IIRC Vespucci both use a WebView instead of a browser to authorize which then of course does not save the login cookie. Which means, for every new token, the username and password needs to be reentered. (I know, it is recommended to not use a WebView but the device's browser, but we had problems in the past with that. There a quite a number of browsers for Android, and some - e.g. if I remember correctly the default browser for a popular custom rom - fail to forward a redirect URI like streetcomplete://oauth to whichever app registered this uri scheme. Maybe @simonpoole also remembers if there were other reasons to use a WebView. In any case, if login is not possible because of some third party browser, it is something user's tend to blame on the app in which they were not able to login rather than that browser.)

tomhughes commented 9 months ago

Well the problem is that you can't (with our implementation at least) just request multiple tokens because the server aggregates all authorisations for a given client, so when you try and get a new token it will see the existing authorisation and return a token for it without asking the user to authorise again.

That wasn't true for OAuth 1 but that led to people having vast lists of authorisations for the same client if it was a client that didn't save tokens and got a new one every time.

westnordost commented 9 months ago

Oh, but what happens if for a client that registers to have all permissions, I call

authorize?response_type=code&client_id=AaV513LxeuSA1EL5q9jhiwRTi5bZyaQFCuZdYMebNwg&redirect_uri=https%3A%2F%2F127.0.0.1%3Aoauth&scope=read_prefs%20write_prefs

, authorize, and then later call

authorize?response_type=code&client_id=AaV513LxeuSA1EL5q9jhiwRTi5bZyaQFCuZdYMebNwg&redirect_uri=https%3A%2F%2F127.0.0.1%3Aoauth&scope=write_api

?

I get the same token, but that token now has all three permissions? (I guess that would be fine for the described use case, wouldn't it?)

tomhughes commented 9 months ago

I honestly don't know but it will come down to https://github.com/doorkeeper-gem/doorkeeper/blob/9fc81d5009aef533ca8116285148cb6e37549ff2/lib/doorkeeper/oauth/client_credentials/creator.rb#L11 I think.

We have reuse_access_token set but it looks like that code may consider the scopes when deciding if an existing token is a match?

tomhughes commented 9 months ago

Actually I think that one is for the client credentials flow but there is likely similar code somewhere for other flows.

simonpoole commented 9 months ago

I know, it is recommended to not use a WebView but the device's browser, but we had problems in the past with that.

Historically, being very nice, it didn't work "reliably" and googles arguments against using a Webview have mostly been self serving in any case.

Given that we, in the 1.0a case, ask for nearly all (that's the nature of the beast) permissions to start with it isn't a big deal for us, though I would prefer if users would at least in principle be able to control things more fine grained.

westnordost commented 9 months ago

Well the problem is that you can't (with our implementation at least) just request multiple tokens because the server aggregates all authorisations for a given client, so when you try and get a new token it will see the existing authorisation and return a token for it without asking the user to authorise again.

I just tested this. I created one access token to access everything, and then one access token for only read_prefs. It works as expected, i.e. the second token really only has access to reading preferences while the other has access to everything.

westnordost commented 9 months ago

So anyway, after having read the RFC, it doesn't look like OAuth 2 is designed to allow this kind of flow we used to have with OAuth 1.0a. It would be a (possibly non-RFC compliant, would need to read the exact wording) extension to OAuth 2.

To summarize:

So, I will close this. Web-apps (such as the mentioned HOT tasking manager) already can implement fine-grained permissions with OAuth 2.

AntonKhorev commented 9 months ago

So anyway, after having read the RFC, it doesn't look like OAuth 2 is designed to allow this kind of flow we used to have with OAuth 1.0a.

It is designed to allow this.

https://datatracker.ietf.org/doc/html/rfc6749#section-3.3

The authorization server MAY fully or partially ignore the scope requested by the client, based on the authorization server policy or the resource owner's instructions.

checkboxes = "the resource owner's instructions"

westnordost commented 9 months ago

I see, and also quite convenient for the client developer, as the granted scopes are returned to the redirect uri.

westnordost commented 9 months ago

In any case it would be helpful to have a decision about if checkboxes are to return for the OAuth 2 implementation (at some point) because it affects client implementations.

I.e. in that case, clients should not assume that everything that has been requested was granted, but must read the scope parameter of the response to learn which permissions have been granted. I.e. similar to but more straightforward than the old OAuth 1.0a auth flow.

My preference then would also be for the checkboxes to return, because while it may be possible to request granular permissions as described in https://github.com/openstreetmap/openstreetmap-website/issues/4360#issuecomment-1822949775 I doubt that any client developer will actually implement that. In any case, whether or not this is done depends on the individual client developers, while actually it should be the user's choice.

westnordost commented 9 months ago

I ended up implementing checking for the granted scopes to be forward compatible to a possible change in the UI on osm.org when the user should grant the authorization request. Was just a few lines of code anyway.

mmd-osm commented 6 months ago

So I tried this out in https://gist.github.com/mmd-osm/a8edb405918795d7d823b9088bd12f41 and replaced the static list by a list of checkboxes. You can deselect some of the scopes before authorizing the request, and resulting token would only include the exact list of scopes which have been selected.

Well the problem is that you can't (with our implementation at least) just request multiple tokens because the server aggregates all authorisations for a given client, so when you try and get a new token it will see the existing authorisation and return a token for it without asking the user to authorise again.

This behavior really depends on the "confidential" setting for the app (next to the obvious "skip_authorization"). In case the app is not flagged as confidential, I'm getting the authorization popup every time when calling /oauth2/authorize. This was added to fix impersonation issues as per CVE-2023-34246.

I could select different scopes, and a new token matching the selected scopes would be generated (if needed).

[29, 38] in ~/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/doorkeeper-5.6.9/app/controllers/doorkeeper/authorizations_controller.rb
    29|     end
    30| 
    31|     private
    32| 
    33|     def render_success
=>  34|       if skip_authorization? || (matching_token? && pre_auth.client.application.confidential?)
    35|         redirect_or_render(authorize_response)
    36|       elsif Doorkeeper.configuration.api_only
    37|         render json: pre_auth

Doorkeeper link

In the end, a user might have multiple tokens for the app, with different scopes:

image

The list of authorized apps would still show a single entry only for this app, which is a bit confusing. Revoking access for the app revokes all tokens, as expected.