openstreetmap / merkaartor

Home of Merkaartor, an openstreetmap mapping program.
http://merkaartor.be/
Other
291 stars 77 forks source link

Deprecation of HTTP Basic Auth and OAuth 1.0a #286

Open opk12 opened 11 months ago

opk12 commented 11 months ago

Thank you for this app. Just a heads-up if you didn't know, that OSM is slowly deprecating HTTP Basic Auth and OAuth 1.0a in favor of OAuth 2 (see announcement and tracking issue).

Related: #104

Krakonos commented 11 months ago

Thanks for the notice. I'm looking into this with a higher priority. There is OAuth2 library in Qt. However, reading osm docs it appears I need to generate client_id and secret. However, I can't keep the secret secret.

I looked briefly into josm and could not find a secret in the source code, not sure if they just inject it at buildtime though.

Do you have any visibility into the preferred way to do it?

opk12 commented 11 months ago

Ah, this was briefly discussed today in the IRC chat #osm-dev (OFTC webchat, Matrix) and BTW there's also #josm if you ever need (OFTC, Matrix).

OAuth 2: How to best deal with the secret from step 1 at wiki.osm.org/wiki/OAuth in an open source app? If it's published to the GitHub repo, it won't be a secret anymore?

In that case you should mark the app as non-confidential when you register it Best practice is also to use PKCE when authorising see https://www.oauth.com/oauth2-servers/pkce/

I'm going to add a brief mention in that wiki article; feel free to link other OSM docs where you think it should be mentioned.

Krakonos commented 11 months ago

Thanks, that's helpful. I got it working. However, I'm not sure how to check if it's doing what it's supposed to do.

  1. The code seems to work without the secret entirely.
  2. When removing code_challenge and code_verifier, nothing really breaks.

I would expect the OSM server should require at least one of these things to be present?

Anyway, it looks like I can successfully use the OAuth2 flow with OSM API, so I'll integrate it with the rest of the system. The only big downside I see now is I seem to require a separate client id for every server (main/test api/3rd party servers).

tsmock commented 11 months ago
  1. It should, assuming that PKCE is used. Technically, I don't think you need to use PKCE with OAuth 2.0, but you should. It is highly likely to be a mandatory part of the OAuth 2.1 specification (it is part of the current draft).
  2. I would keep both code_challenge and code_verifier if you are using PKCE -- if removing them doesn't currently break something, then it is entirely possible that this is a bug in doorkeeper (the OAuth 2 library the OSM website is using) that will be fixed at some point. Or they aren't enforcing some of the recommended security practices yet.

When I was adding support for OAuth 2 in JOSM, I implemented a bunch of "optional" stuff that is likely to become mandatory with OAuth 2.1 and didn't implement some things that were optional in OAuth 2.0 but were removed/deprecated in the OAuth 2.1 draft.

If you are writing your own OAuth lib instead of using someone else's, I would highly recommend implementing PKCE. But you should probably use someone else's OAuth library.

I don't know how you are intending to implement the OAuth redirect, but in JOSM I'm using the remote control endpoint /oauth_authorization.

The only big downside I see now is I seem to require a separate client id for every server

Yep. That is a downside. I'm currently only planning on hardcoding the main api and test api oauth client ids in JOSM. There is an RFC for dynamic client registration, but it isn't currently implemented in the library the OSM website is using. There is also an RFC for getting OAuth endpoints, which also isn't implemented in the library that the OSM website is using.

EDIT: The OSM website now implements the RFC for getting OAuth endpoints, see https://www.openstreetmap.org/.well-known/oauth-authorization-server .

mmd-osm commented 1 month ago

Basic Auth and OAuth 1.0a have been turned off on July 1st now. Merkaartor upload to osm is currently not working anymore.

nataraj-hates-MS-for-stealing-github commented 1 month ago

I got it working

Have it been commit to any branch or repo? Can you share that code? Merkaartot is useless right now, and I'd like to continue using it.

Krakonos commented 1 month ago

Work in progress on my personal branch. I have some local stuff not there yet. I've got the auth working + the UI, but need to put that together.

https://github.com/Krakonos/merkaartor/commits/oauth2

Krakonos commented 1 month ago

Well, I pushed the recent version. The refactor is mostly OK, download works. But it seems since last time I tested the flow, something changed and my Qt oauth component sends redirect url as http://localhost:1337/ instead of http://127.0.0.1:1337/. Unfortunately, I can not set localhost on the osm.org website, due to non-IP redirect addresses needing https.

I'll continue tomorrow, but you can test if it works for you.

One caveat is it will NOT work against other APIs than main osm.org due to hardcoded client_id. I will at least add client id for the testing API tomorrow.

Krakonos commented 1 month ago

Ok, so the IP is fixed, well, hacked. However, now I'm stuck getting the actual bearer token, and the QT handler eats the error (it likely is in the body, which it ignores).

     8.588 (WW) [ default ] Token request error:  1 "Error transferring https://www.openstreetmap.org/oauth2/token - server replied: "
     8.588 (WW) [ qt.networkauth.oauth2 ] Token request failed: "Error transferring https://www.openstreetmap.org/oauth2/token - server replied: "
Krakonos commented 1 month ago

Just pushed to the branch (https://github.com/Krakonos/merkaartor/commits/oauth2 ) with basic functionality.

What works:

What does not work:

If you've got time, I'd appreciate some rough testing. So far it works for me (tm).

dreirund commented 1 month ago

I now cannot upload data anymore.

When I upload, I get the error message

There was an error uploading this request (403)
Server message is 'Error transferring https://www.openstreetmap.org/api/0.6/changeset/create - server replied: Forbidden'

So Merkaartor seems no longer usable right now, making this an important/ deal breaking issue.

(I use latest release 0.19.0 from my distribution. If you have already fixes for this, maybe do a new release?)

Krakonos commented 1 month ago

If you can, please test the branch marked above. I'm busy over the week, but I think I'll merge & release it over the weekend.

dreirund commented 1 month ago

I'm busy over the week, but I think I'll merge & release it over the weekend.

OK, great!, so I will just wait until my distribution picks up.

nataraj-hates-MS-for-stealing-github commented 1 month ago

If you can, please test the branch marked above. I'm busy over the week, but I think I'll merge & release it over the weekend.

Sorry for delay...

I managed building the code. But not from the first attempt. What have I done:

CMakeFiles/merkaartor_autogen.dir/build.make:77: *** target pattern contains no '%'.  Stop.
make[1]: *** [CMakeFiles/Makefile2:281: CMakeFiles/merkaartor_autogen.dir/all] Error 2
make: *** [Makefile:166: all] Error 2

Same story I get when I build it master branch.

I looked at the make file with error, found protobuf mentioned there, and tried to configure with cmake -DPROTOBUF=OFF ..

This helped. So I built it and run it.

I tried to login via oauth2 method, but failed:

I am using KDE on this computer, Then I pressed "Login button" it opened knqueror browser (I never changed browser defaults here), then konqueror automatically reopend that page in firefox (I do not know why, it changed to that behavior after one update, I've never configured it). This page has URL "https://www.openstreetmap.org/api/oauth2/authorize?client_id=wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps&code_challenge=k-8NhjKbEjfP_hPtscyFkRO_ZmUghWjySZhDkyupp5E&code_challenge_method=S256&redirect_uri=http://127.0.0.1:1337/&response_type=code&scope=read_prefs%20write_prefs%20write_api%20read_gpx%20write_gpx%20write_notes&state=BNF39NRK" and it said, if I translate it back from Russian:

File not found

Could not find a file/directory/API operation with the same name on the OpenStreetMap server (HTTP 404)

Feel free to contact the OpenStreetMap community if you find a broken link/error. Write down the exact URL of your request.

In the console Merkaartor wrote following lines:

    41.814 (DD) [ default ] Start login...
    41.814 (WW) [ default ] Unknown OSM API host  "www.openstreetmap.org" , assuming alias to openstreetmap.org
    41.814 (DD) [ default ] Base URL:  QUrl("http://www.openstreetmap.org/api/0.6")
    41.814 (DD) [ default ] Authorization URL:  QUrl("http://www.openstreetmap.org/api/oauth2/authorize")
    41.814 (DD) [ default ] Access Token URL:  QUrl("http://www.openstreetmap.org/api/oauth2/token")
    41.815 (DD) [ default ] Stage:  1 with params QMultiMap(("client_id", QVariant(QString, "wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps"))("redirect_uri", QVariant(QString, "http://127.0.0.1:1337/"))("response_type", QVariant(QString, "code"))("scope", QVariant(QString, "read_prefs write_prefs write_api read_gpx write_gpx write_notes"))("state", QVariant(QString, "BNF39NRK")))
    41.815 (DD) [ default ] Requesting Authorization.
    41.815 (DD) [ default ] Stage:  1 with params QMultiMap(("client_id", QVariant(QString, "wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps"))("code_challenge", QVariant(QString, "k-8NhjKbEjfP_hPtscyFkRO_ZmUghWjySZhDkyupp5E"))("code_challenge_method", QVariant(QString, "S256"))("redirect_uri", QVariant(QString, "http://127.0.0.1:1337/"))("response_type", QVariant(QString, "code"))("scope", QVariant(QString, "read_prefs write_prefs write_api read_gpx write_gpx write_notes"))("state", QVariant(QString, "BNF39NRK")))
    48.115 (WW) [ default ] OAuth2 reply handler timed out.
    48.115 (DD) [ default ] Failed!

I do not know how does this OAuth2 communicate with the browser, but I whould suspect, that this issue with redirecting might break everythig.

As far as I know Qt has some limited html-rendering engine that can be used to handle that OAuth2 stuff. Guess better not try to rely on OS browser facility, as it is not reliable, and use browser of our own, if it is ever possible.

nataraj-hates-MS-for-stealing-github commented 1 month ago

Tried to do the same thing on laplop with LXQt:

   256.167 (DD) [ default ] Start login...
   256.167 (WW) [ default ] Unknown OSM API host  "" , assuming alias to openstreetmap.org
   256.167 (DD) [ default ] Base URL:  QUrl("")
   256.167 (DD) [ default ] Authorization URL:  QUrl("oauth2/authorize")
   256.167 (DD) [ default ] Access Token URL:  QUrl("oauth2/token")
   256.167 (DD) [ default ] Stage:  1 with params QMultiMap(("client_id", QVariant(QString, "wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps"))("redirect_uri", QVariant(QString, "http://127.0.0.1:1337/"))("response_type", QVariant(QString, "code"))("scope", QVariant(QString, "read_prefs write_prefs write_api read_gpx write_gpx write_notes"))("state", QVariant(QString, "jRASHX8Y")))
   256.167 (DD) [ default ] Requesting Authorization.
   256.167 (DD) [ default ] Stage:  1 with params QMultiMap(("client_id", QVariant(QString, "wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps"))("code_challenge", QVariant(QString, "29hhwtj0rbuj3dvgqVc2B_kvDCOHJl4uLbC5a8mtU3E"))("code_challenge_method", QVariant(QString, "S256"))("redirect_uri", QVariant(QString, "http://127.0.0.1:1337/"))("response_type", QVariant(QString, "code"))("scope", QVariant(QString, "read_prefs write_prefs write_api read_gpx write_gpx write_notes"))("state", QVariant(QString, "jRASHX8Y")))
xdg-open: file 'oauth2/authorize?client_id=wv3ui28EyHjH0c4C1Wuz6_I-o47ithPAOt7Qt1ov9Ps&code_challenge=29hhwtj0rbuj3dvgqVc2B_kvDCOHJl4uLbC5a8mtU3E&code_challenge_method=S256&redirect_uri=http://127.0.0.1:1337/&response_type=code&scope=read_prefs%20write_prefs%20write_api%20read_gpx%20write_gpx%20write_notes&state=jRASHX8Y' does not exist
   261.999 (WW) [ default ] OAuth2 reply handler timed out.
   261.999 (DD) [ default ] Failed!

My idea is still the same: external browser is not reliable...

Krakonos commented 1 month ago

I think I'm able to reproduce your issue. The URL entered has to be https://www.openstreetmap.org/, which is different to what it used to be. This needs to be adressed and perhaps rewritten automatically to facilitate migration.

Notice that http changes to https, and the /api/0.6 goes away. After these modifications, it seems to work with redirects.

I could embed a browser window instead, but that window is not necessarily trusted, and some features might be missing - as one example, users tend to have password managers that are not usable in this scenario.

Also, there might be a second flow necessary that doesn't use redirect, which could be problematic, but a code shown in browser instead. In this instance, we can also show the login URL user needs to navigate to. This can be then done on a different computer even, so I think that is a feasible backup solution.

Edit: The second issue seems you have empty URL field, can you check the contents there?

nataraj-hates-MS-for-stealing-github commented 1 month ago

Adding https://www.openstreetmap.org/ to the URL solves that problem on both laptops.... Both successfully opens login dialogs

I would suggest to change that dialog to the radio button choice: First is fixed " https://www.openstreetmap.org/" second is a field for custom URL. 98% of users will need the first choice.

But whenever I properly enter login, password and confirm that I trust merkaartor app, I am redirected to http://127.0.0.1:1337/?code=XXXXXXXXXXXXX page, that reports that connection can not be established.

telnet does not want to connect that address/port too

May be if we use built in browser, there will be no need in built in server to response on 127.0.0.1:1337, so we just grab that code from the redirect http request without processing it? :-)

nataraj-hates-MS-for-stealing-github commented 1 month ago

users tend to have password managers that are not usable in this scenario.

Hm... This I did not think about...

but that window is not necessarily trusted,

As for me, I would not trust any external browser app. But that is my preferences :-)

but a code shown in browser instead.

This sounds as a good idea... Much better then running local web server :-)

Krakonos commented 1 month ago

Yeah, the port is a problem due to how the Qt library handles the thing. I will implement the manual flow as a backup and release the port flow as is and try to figure out how to improve it. There are other issues with it - mainly the osm.org site returns errors in response body, but the flow does not give me API to retrieve that body in the error handler. I'm not sure if this can be fixed by inheritance, but TBH at that point it might be easier to roll a custom "library", since the OAuth2 flow in Qt requires so much boilerplate around it, it doesn't really give us much...

nataraj-hates-MS-for-stealing-github commented 1 month ago

If you get stuck with it, please let me know. I do not have much time, and I have not beet working with Qt for more than 15 years, so it would not be easy for me, but this issue should be fixed...

Krakonos commented 1 month ago

Well, I pushed a few UI changes with a slightly better error reporting and better messaging. I got stuck on the manual flow though, I don't think I have found "The Qt way" to do it and it's painful. I'll continue tomorrow.

nataraj-hates-MS-for-stealing-github commented 1 month ago

Should I test anything?

Krakonos commented 1 month ago

You can have a look at the UI, but functionality should be mostly unchanged (except for some error handling, which is still sub-par due to deficiencies in the qt library). I'll find some time today to work on the manual (OOB) flow.

Krakonos commented 1 month ago

OOB flow pushed. You can give it a go. I have it set up so it opens the URL automatically, the dialog needs some work. I used off the shelf dialog, but the URL is too long and not clickable. I will probably roll a custom one, should not be too difficult.

I'm not too happy with the overall solution though - but I looked into the Qt tracker and there seem to be some changes in the works, so maybe i'll be able to improve it soon.

Krakonos commented 1 month ago

Also, changed logging to be hidden by default. Use the following environment variable to show them:

QT_LOGGING_RULES="merk.OsmServer.debug=true" 
Thra11 commented 4 days ago

I've built 4bb122219ddd67fe210b86c6547640a84a589ef1 and tested both OAuth2 redirect and OAuth2 OOB. At first, both failed as they were attempting to load a url that starts with

https://www.openstreetmap.org/api/oauth2/authorize?client_id=...

which returns a 404. It looks like it had remembered the url from an older version with the /api/0.6/ specifier. Once I realised and selected the correct url from the dropdown, login worked and I was able to upload a change.

Krakonos commented 13 hours ago

Thanks, I added a simple migration logic. For testing this, just run the new commit. To retest, edit ~/.config/Merkaartor/Merkaartor.conf section [OsmServers] lines version=1 (set to 0 to force the migration path again).

kkofler commented 12 hours ago

Are you going to make a release any time soon or should distros just package a snapshot to make their package work again?