Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move OSM authentication to OAuth 2.0 #6144

Closed
pnorman opened this issue Sep 27, 2023 · 21 comments
Closed

Move OSM authentication to OAuth 2.0 #6144

pnorman opened this issue Sep 27, 2023 · 21 comments
Assignees
Labels

Comments

@pnorman
Copy link

pnorman commented Sep 27, 2023

Is your feature request related to a problem? Please describe.
OAuth 1.0a is deprecated, and programs which use it to authenticate requests to OpenStreetMap should move to OAuth 2.0. A date has not yet been set for turning off OAuth 1.0a and HTTP Basic.

It's best to move to OAuth 2.0 well in advance of any turn-off, because users may take some time to upgrade software and to re-authenticate.

See openstreetmap/operations#867 for details.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@pnorman pnorman added the Enhancement New feature or request, an improvement of some existing feature label Sep 27, 2023
@biodranik biodranik added Editor OSM Editor and removed Enhancement New feature or request, an improvement of some existing feature labels Sep 27, 2023
@Jean-BaptisteC Jean-BaptisteC added this to the Next Release milestone Sep 28, 2023
@mmd-osm
Copy link

mmd-osm commented Dec 23, 2023

I think you're using OrganicMapsTestUser for your Unit tests on master.apis.dev.openstreetmap.org. Are you using Basic Authentication or OAuth 1.0a there?

I need to know if we have to take additional steps before merging the OAuth 1.0a removal in CGImap. Since we have an automated deployment in place, that would immediately break OAuth 1.0a for changeset uploads on the dev instance.
Our goal is to avoid any disruptive and unexpected changes on the dev instance.

Relevant commit is: zerebubuth/openstreetmap-cgimap#354

NB: Production is not impacted by any of this at this point in time.

@biodranik
Copy link
Member

The same OAuth1.0a as in production is used in debugging/CI on the dev server. Do you have any recommendations or examples on how to properly migrate the manual auth code?

@mmd-osm
Copy link

mmd-osm commented Dec 23, 2023

The same OAuth1.0a as in production is used in debugging/CI on the dev server.

Thank you for your quick reply. I will discuss this point with our sysadmins, to make sure that we're not breaking anything here.

Do you have any recommendations or examples on how to properly migrate the manual auth code?

My recommendation here would be very clear to reach out to both @tsmock (JOSM) and @westnordost (StreetComplete). They have either already completed the switch to OAuth 2.0, or are currently in the process of rolling out OAuth 2.0 as part of a beta version. I hope they're ok to mention them here, since I believe they could provide very valuable insights.

Are you looking for solutions for both Java and C++? JOSM and StreetComplete should cover Java/Kotlin. @tsmock also had some discussions with Merkaartor, a C++ app. I'm not sure if this would be helpful in your case.

You can of course also post to openstreetmap/operations#867, which is the central issue to manage the transition. Besides all admins/core maintainers I would expect that you could reach out to a larger group of app developers this way.

Linking to a few relevant issues/PRs.

@westnordost
Copy link

westnordost commented Dec 23, 2023

Hey @biodranik , I summarized it here:

MarcusWolschon/osmeditor4android#2401 (comment)

Feel free to ask me questions, I very recently concerned myself with it so my knowledge of it is fresh.

In a nutshell, using an OAuth2 access token is as simple as adding Authorization: Bearer <the OAuth2 access token> in the header of each HTTPS request. For getting the authorization token, the flow is similar but not exactly the same as for OAuth 1.0a.

@biodranik
Copy link
Member

Thanks, @westnordost, that is helpful. The problem is that liboauthcpp used in OM does not support OAuth2: sirikata/liboauthcpp#24

Implementing the spec manually in C++ is not an easy task. If anyone is aware of simple and lightweight C++ implementations without networking code, please let us know.

@westnordost
Copy link

westnordost commented Dec 24, 2023

You don't need a library for passing the access token. For getting the access token, ... maybe, but it is really not a lot. I implemented the logic in ~120 source lines of code, so that's the order of magnitude of code you would like to have outsourced into a lib. (The links to the source code are contained in my previously linked comment.)

For getting the access token, indeed to implement the whole spec would be quite a large undertaking, so an OAuth2 library that does that would not be lightweight.

Also, note that to get the access token at the end, the result from the server must be parsed as a JSON. So, a library that does all that (120 source lines of code worth of logic) for you will very likely pull in another dependency to a JSON parsing library, i.e. maybe a different one that you are using for other stuff. Not to mention the dependency to actually do HTTP requests. Oh, and, and optional feature that is nevertheless supported by OSM and recommended to be used is to include a PKCE key exchange in the process, which means a library that implements that (I think it is pretty standard these days) also needs a dependency to some sha256 hasher.

I am writing this because you mentioned that it should be lightweight. If you are looking for a library without networking code and without JSON parsing code, to be honest, there is not much code left. The exchange that happens to get the access token is basically nothing else than a bunch of HTTPS requests and reading the responses.

@westnordost
Copy link

(I edited my post significantly to include more information)

@rtsisyk
Copy link
Member

rtsisyk commented Dec 28, 2023

This issue is getting more important because of #7000

@rtsisyk
Copy link
Member

rtsisyk commented Dec 28, 2023

Probably it will be easier to implement auth in Java and Swift separately instead of doing this business in C++.

@biodranik
Copy link
Member

@rtsisyk as it was mentioned by @westnordost and @Zverik, the decision to use login and password was a deliberate one, forced by issues with using Webview or browser login.

To clarify: the existing OAuth1.0a solution was using webview from the start. And it can be adopted to using external web browser if necessary without migrating to OAuth2.

@rtsisyk rtsisyk removed this from the Next Release milestone Dec 29, 2023
@rtsisyk
Copy link
Member

rtsisyk commented Dec 29, 2023

@rtsisyk as it was mentioned by @westnordost and @Zverik, the decision to use login and password was a deliberate one, forced by issues with using Webview or browser login.

I didn't get the problem with WebView. OAuth2 is definitely better than asking the user to enter their password.

@westnordost
Copy link

As far as I know it is easy to circumvent this restriction by setting the user agent of the WebView to something other than the default. (You could test this by trying to login with OAuth with Google on OSM within STreetComplete.)

But anyway, using the browser is probably the better idea anyway. Though, expect issue tickets to appear that complain about that the login doesn't work on their browser or maybe even the default browser of certain Android distributions such as LineageOS (last time I checked).

Also, the information may be outdated, but back when I looked into the topic, the recommendation (or even obligation?) for apps on the Apple App store was the exact opposite, "to not break the native experience".

But take my comment with a grain of salt, because this was quite some years ago. Browser support may have been fixed, Apple may have either changed their policies or I misremember it in the first place.

@biodranik
Copy link
Member

As far as I know it is easy to circumvent this restriction by setting the user agent of the WebView to something other than the default. (You could test this by trying to login with OAuth with Google on OSM within STreetComplete.)

Thanks for the hint!

Apple is ok with the app login. It's a Google Play issue only.

CC @rtsisyk

@mmd-osm

This comment was marked as resolved.

@RedAuburn
Copy link
Sponsor Member

Do not submit website forms in an automated manner or on behalf of users. Website forms are intended only for users interacting directly with the website.

the registration button takes you to the OSM site, you can't create an account in-app.
The login is using OAuth, not the website forms 👍️

@biodranik
Copy link
Member

...And the login is not automated, users are doing it manually.

@rtsisyk
Copy link
Member

rtsisyk commented Feb 1, 2024

We are looking for Android engineers to implement new OSM authorization in the app. This work will be financially rewarded based on fair estimation of time and efforts spent. Please post your proposal in this thread.

@westnordost
Copy link

This is not a proposal, just a few comments after a quick look at the source code in order to help make clear the scope of it:

The biggest part of the current OAuth 1.0a authentication is implemented in osm_oauth.cpp.

The normal OAuth authorization flow (OAuth 1.0a and OAuth 2.0 are quite similar) would be to

  1. get the request token
  2. open the authorize URL on osm.org (passing the request token) in a webview or browser, let user login there and let him click on the authorize-button so that the verifier is sent back to the app via the callback URL
  3. get the verifier from the callback URL parameter and use that to get the access token

OM is currently using the following curious approach to OAuth authorization instead. As far as I understand:

  1. In its own UI, let user enter username+password, or alternatively through Google, Facebook, ...
  2. act like a web browser that logs in to osm.org/login with these credentials and save the session cookie
  3. get the request token as normal
  4. act like a web browser that opens the authorize URL on osm.org (passing the request token and with the session cookie set) and extract the verifier from the link that would open when one clicked the authorize-button in the HTML
  5. get the verifier from the callback URL parameter and use that to get the access token

I believe it is done that way to avoid the user leaving the app's UI for authorization but certainly is somewhat of a misuse of OAuth. Could just as well use HTTP Basic Auth, then. However, this is akin to how JOSM did it in the past, as far as I know.
But anyway, this is the reason why there is so much code.

Obviously, I strongly recommend just doing the normal OAuth 2.0 authorization flow, which is akin to the OAuth 1.0a but even a little simpler (actually). It is less code, it is more trustworthy and more future-proof (less chance that something breaks, when e.g. the HTML on osm.org changes). Google additionally requires (but it can be tricked) to use the browser instead of a WebView.
Of course, it is up for OM folks to decide how it should be implemented.

Note that OAuth 2.0 requires some basic JSON parsing, as the access token response is in JSON.

The Android part of the Login is in OsmLoginFragment.java and OsmOAuth.java, which is naturally almost empty because it does almost nothing else than provide the UI for entering the username+password and initiate the authorization in native code.

This also means that to implement OAuth 2.0 authorization only for Android but not for iOS makes no sense, as the main part is implemented in native code.

For further help on implementing OAuth 2.0, see MarcusWolschon/osmeditor4android#2401 (comment) , it also includes links to implementation.

@rtsisyk
Copy link
Member

rtsisyk commented Mar 28, 2024

I am removing this ticket from Release Blockers because it is not a blocker and it is actually going into the subsequent release (after the current one).

@rtsisyk rtsisyk removed this from the Release Blockers milestone Mar 28, 2024
@pastk
Copy link
Contributor

pastk commented Apr 11, 2024

Done in #7333 thanks to @strump !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

9 participants