openstreetmap / openstreetmap-website

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

Default value for "Confidential application" in OAuth2 client registration seems to be questionable #3494

Open simonpoole opened 2 years ago

simonpoole commented 2 years ago

Description

According to https://wiki.openstreetmap.org/wiki/OAuth#Registering_your_application_as_OAuth_2.0_consumer the "Confidential application" checkbox should only be checked if the application can keep the key secret, in particular it should not be set for mobile and web apps, with other words for essentially all apps in this age and day.

Shouldn't then the default be to not to check the setting?

@harry-wood

tomhughes commented 2 years ago

I'm pretty sure that's not deliberate, unless it comes from the database in which case that would be the upstream recommended default from doorkeeper.

tomhughes commented 2 years ago

Yes that's exactly it (https://github.com/openstreetmap/openstreetmap-website/blob/master/db/migrate/20201004105659_create_doorkeeper_tables.rb#L12) so that was what doorkeeper's generator was recommending I think.

tomhughes commented 2 years ago

Specifically it comes from https://github.com/doorkeeper-gem/doorkeeper/blob/main/lib/generators/doorkeeper/templates/migration.rb.erb#L15.

tomhughes commented 2 years ago

Some rather confusing discussion at https://github.com/doorkeeper-gem/doorkeeper/issues/1142.

simonpoole commented 2 years ago

Some rather confusing discussion at doorkeeper-gem/doorkeeper#1142.

I think the gist is that when migrating they had to assume that confidential was true for all existing clients, but I don't quite see why out of that should follow that it should be the default for new clients too.

HarelM commented 1 year ago

This might not be the right place to ask this, but I'm currently working on migrating my app from OSM's OAuth 1 to 2. I have a mobile app and a website with both use the same backend server. When I create a non-confidential OAuth app (uncheck the checkbox mentioned here) I still see the client secret. From what I read I shouldn't be using it, so I'm not sure if it should be presented or not - this got me confused. The other question is: are there different capabilities that are offered if I use confidential vs non-confidentail from OSM perspective, i.e. can I edit an OSM entity while using the non-confident OAuth flow (from the user who uses my app that uses the flow I mean)?

tomhughes commented 1 year ago

Why do you think you shouldn't see the client secret? Nothing can work without it.

HarelM commented 1 year ago

Probably my wrong interpretation of the following: https://auth0.com/docs/get-started/applications/confidential-and-public-applications Scroll down to "public applications". The notion is that non confidential applications can store a client secret, so there's no need for it? I'm not sure, I'm trying to figure this out myself, I'm not super familiar with all the ouath stuff unfortunately.

tomhughes commented 1 year ago

Sorry yes you're right that it's not required, but it's also not important to keep hidden because there is no need for the client to even use it - if the client is not confidential then the client_secret parameter can be left out when requesting the access token.

HarelM commented 1 year ago

Thanks for confirming this! and for your super-fast response! From a UX point of view, keeping it may cause, like it did for me, an insecurity as to which client type I selected in the previous page, and an uncertainty that I need to use it although it's not needed. I agree it's not a critical issue, but I would've felt a lot more confident that I don't really need the client_secret if it was just hidden (UI-wise). I can open a different issue if that makes sense or not, whatever works for you.

jmaspons commented 1 year ago

there is no need for the client to even use it - if the client is not confidential then the client_secret parameter can be left out when requesting the access token.

I tried in R with httr2, but it fails without the secret, even for a non-confidential OAuth2 app. Anyway, it's quite cheap to get a new secret, so I suppose nobody will spend the time looking for them