Openstreetmap-website: OAuth2 support?

[HarelM]

Hi,

Any change to support OAuth 2 protocol. I currently have a security issue due to client side cuonsumer key vulnerability with OAuth 1. Also it's getting harder to support this old format in new packages (Mainly in .Net server side code).

P.S. I'm the co-founder and main maintener of https://israelhiking.osm.org.il and I have decided to use the OSM users as my site users to encourage people to contribute to OSM mapping.

[tomhughes]

Well the problem is OAuth2 isn't really a protocol, it's more of a toolkit that can you can use to design a protocol.

I've no objection to supporting it if somebody wants to do the work, but I don't really have the time or energy to do enough reading and research myself to come up with something.

I'm not sure what you mean by "client side consumer key vulnerability" though - if you just mean the fact that you need to have a key on the client side to identify the client then I'm not sure how OAuth2 is supposed to help as every OAuth2 system I'm ever seen still has that?

[HarelM]

Unfortunately I'm not a Ruby developer, so I can't help much in this area.
I guess that there are packages who offer this kind of support. googling it for a few second return this results:
https://github.com/doorkeeper-gem/doorkeeper
As I see it, it is so common that there must be a package that is debugged and production worthy that you can take and use out of the box.
Again, not a Ruby developer so this is just my experience with other languages talking...

[ghost]

Before you start considering OAuth 2, note that its lead developer called it bad before resigning from his role. He wrote about his reasons here: https://hueniverse.com/oauth-2-0-and-the-road-to-hell-8eec45921529

[HarelM]

The article is from 2012. Google, Facebook, Microsoft and probably other major software companies have adopted this standard.
OAuth 1, which is currently used, is worse in terms of security and I'm not aware of a standard that replaced OAuth 2.
I would say that at the very least, support both until a new standard replaces them both...

[tomhughes]

I am currently investigating this further - because the oauth-plugin gem we currently use is largely unmaintained I'm reluctant to use it's OAuth 2 support so am currently looking at https://github.com/doorkeeper-gem/doorkeeper instead.

That doesn't do OAuth 1 so the two will likely be largely separate though it may be possible to share the registered client list.

[mmd-osm]

It would be good to know if you expect some necessary changes on cgimap, which currently only supports OAuth 1.0a. zerebubuth has written that implementation, and we probably need to involve him, in case the plan is to run some mixed 1.0a/2.0 scenario for all API endpoints.

[tomhughes]

I guess so - certainly we won't be able to do a hard cutover. I've mostly just been reading so far though so not ready to offer any code or anything.

[mmd-osm]

It seems that cgimap (and possibly other clients as well) would at least need to call an Introspection endpoint via HTTP(S) to validate a user-provided token and get further details such as user ids, scopes and whatnot (https://tools.ietf.org/html/rfc7662#section-2).

As we share the same database anyway in the case of cgimap, there might be some way to directly query the respective doorkeeper tables, as we do today with OAuth 1.0a (https://doorkeeper.gitbook.io/guides/internals/database-design).

Most of the work to get this up and running seems to be on the Rails side of the house this time.

[mmd-osm]

~Quick heads up: the new JSON endpoint in #2839 returns an array of users as of now. While this works for _jq_ ( curl ... | jq '.users[0].user.display_name'), array support was only added recently to the Discourse oauth2 plugin (https://github.com/discourse/discourse-oauth2-basic/pull/22).~

Simplified this a bit now: the "user" endpoint returns a single user only ("user.display_name"). An array of users makes only sense for the "users" endpoint really.

[mmd-osm]

For the sake of documenting my doorkeeper mini-proof of concept, I'm listing a few points that I thought might be worthwhile checking.

Gemfile:

+ gem "doorkeeper"

Beyond the default doorkeeper installation, I also installed:

• https://github.com/doorkeeper-gem/doorkeeper/wiki/Customizing-views
• https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-PKCE-flow
• https://github.com/doorkeeper-gem/doorkeeper/wiki/Associate-users-to-OAuth-applications-(ownership)

Respective generate commands:

• rails generate doorkeeper:views
• rails generate doorkeeper:pkce
• rails generate doorkeeper:application_owner
-

Some migrations need an additional safety_assured to pass.

Other bits: https://github.com/mmd-osm/openstreetmap-website/commit/dcf0a3edd7fe5a89fcaeb531047df21473ff3ddd

Discourse oauth2-basic settings:

Discourse theme customization:

button.btn-social.oauth2_basic {
    background-color: gray;
}

button.btn-social.oauth2_basic:hover {
    background-color: green;
}
.fa.d-icon.d-icon-sign-in-alt.svg-icon.svg-string {
    display: none;
}
.btn-social.oauth2_basic:before {
    height: 16px;
    width: 16px;
    background-image: url(http://openstreetmap.org/favicon.ico);
    content: " ";
    background-size: cover;
}

[tomhughes]

So basically you've been duplicating everything I've been doing?

[mmd-osm]

It was a good exercise to get some idea how doorkeeper works, a bit of duplication is perfectly fine for me. Your comment seems to suggest that this makes at least some sense to you, which is a good start.

[mmd-osm]

I guess you've also got the discourse-oauth2-basic plugin up and running? I tried the new json user details endpoint along with Discourse, and that seems to be working ok'ish for my toy example.

[tomhughes]

No of course I haven't. I'm not doing anything with discourse, I'm just generally working on oauth2 support for the API.

[mmd-osm]

~I've seen some issues with a user having to log in first at the rails port and then follow a redirect back to discourse (example: http://railsport.internal/login?referer=http://discourse.internal/auth/oauth2_basic/callback).~ (resolved)

~safe_referer in the app controller sanitises the referer url so that the redirect no longer works. When commenting out all that logic, discourse reports an csrf_detected error...~ (resolved, needs proper referer url when calling /login endpoint).

Long story short, I believe it would make sense to somehow include discourse in the tests. They offer a container image which is good enough for testing. https://hub.docker.com/r/bitnami/discourse/

Inside the discourse container, the oauth2-basic plugin needs to be installed:

git clone https://github.com/discourse/discourse-oauth2-basic.git /opt/bitnami/discourse/plugins/discourse-oauth2-basic
chown -R discourse:discourse /opt/bitnami/discourse/plugins/discourse-oauth2-basic

[tomhughes]

We are not going to include a test that uses a sodding discourse container.

Please just pipe down and let me get on with it which will include writing tests for the supported flows just as we already do for our existing oauth aupport.

[mmd-osm]

Sure, I didn’t really have any unit tests in mind, that would be a bit insane. The idea was more to give this a try in a standalone test, once all is done on the rails port, nothing else.

[gravitystorm]

Please just pipe down and let me get on with it

@tomhughes that's a really unfair thing to say to someone who is a) interested in what is happening b) happily doing actual development work on it and c) a maintainer of cgimap. I don't understand why you are so anti-collaboration on this instead working with @mmd-osm together cooperatively.

[tomhughes]

I just don't understand why he's duplicating work that I've already said I'm doing, and I feel like he's trying to pressure me to go faster. I've said that this is a complex issue that is going to take time for me to address - there are a whole lot of challenging integration issues that need be resolved to get a production quality implementation and the fact that he can basically overtake me by taking shortcuts really just makes it feel like I'm being hounded to speedup.

[mmd-osm]

Apologies if it came across that way, that wasn't really my intention. It’s just that I want to get some first hands idea how this works.

I agree that there’s lots of work to be done for a production ready implementation, in particular all the UI stuff as well as all different flows, and I left out many of those bits right now.

[mmd-osm]

For anyone reading this and scratching their heads what this discussion is all about.
I tried to work out the feasibility of an end-to-end sign up process using Discourse and the Rails port, i.e.

• installing discourse
• registering a new OAuth 2 client on Rails for Discourse application
• configuring the oauth2-basic plugin on Discourse
• Sign up as new user on Discourse, which includes logging in via the Rails port via OAuth2, and redirect back to Discourse
• make sure that Discourse can fetch all details for the new user from our new JSON format user/details endpoint (this includes user id and display_name + avatar image url).
• profit!

I added about 30 lines of code in Rails to the application controller, define a new CanCanCan capability, and add proper redirect to our login page for doorkeeper. That's all demo / throw away code anyway, so it doesn't really matter anyway if there's a bit of duplication here. 30 lines will never suffice for a production ready implementation, no doubt about that.

My focus was to find out if the end-to-end process will eventually work, and we're not missing any important bits nobody thought about before. Quite a different focus, after all. As the end-to-end demo turned out to successful, I'm confident to get this running at some point in the future.

[tomhughes]

I'll add that https://github.com/openstreetmap/openstreetmap-website/compare/master...tomhughes:doorkeeper is the current state of my work.

After some more work at the weekend I no have all the integration with CanCanCan working and have a bunch of tests demonstrating the necessary flows are able to authenticate successfully and then access resources.

There's a few more things I want to play with there, and a bunch of UI work around actually being able to add applications and customising authorisation etc.

[HarelM]

Sound great! Thanks for all the hard work!
Kindly let me know when there's something I can play with that is related to the OSM API. I would very much like to test it and try it out when something is available, even if it's only in the dev env or something.

[mmd-osm]

I'll add that master...tomhughes:doorkeeper is the current state of my work.

I ran a quick test using Postman and Authorization Code (+optionally with PKCE) and that looked good. Due to the additional secret hashing, I had to register some new apps, but that's ok.

PKCE probably only works, because I ran *enable_pkce.rb migration earlier on, and the db still has those two additional columns in oauth_access_grants. Also, AddOwnerToApplication seems to be missing, which would be required to assign an owner to an oauth application (extra work is required to make the /oauth2/applications use the owner field).

I was about to ask, if we would always keep permission and scopes names in sync, or if it would make sense to have some more fine granular scopes (for things like #2631). includes_scope and its mapping onto the existing allow_* fields looks like a clever trick to avoid some extra work (like defining a new capability, or inventing new scopes). I guess that implementation answers my question then.