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

Login screen for OAuth 2 authorization shows "sign up" tab that triggers googles android policy violation bot #5118

Open simonpoole opened 2 weeks ago

simonpoole commented 2 weeks ago

Problem

Google is refusing to update Vespucci because of an account creation/deletion policy violation https://support.google.com/googleplay/android-developer/answer/10144311?visit_id=638601961422620667-1927855622&rd=1#account_deletion .

While as always google doesn't tell you what the problem actually is, it is likely that the new login screen layout from https://github.com/openstreetmap/openstreetmap-website/pull/4455 triggers this (yes in principle you've always been able to create an account there but now it is much more prominent).

The problem with this is less technical (we could add a button that opens the page with the account deletion button https://www.openstreetmap.org/account/edit), but that if we do this google will hold us liable for the account deletion process by the OSMF which is naturally a non-starter.

Solution proposal

Support an additional parameter to the authorization end point url ( https://www.openstreetmap.org/oauth2/authorize), or a different url that displays a plain vanilla login screen with no other functionality.

It should be noted that there is absolutely no guarantee that this fixes the issue for google and any work done on this could easily be completely in vain.

simonpoole commented 2 weeks ago

A rough approximation of what it used to look like on mobile

image

vs. now

Screenshot_1724601668

simonpoole commented 2 weeks ago

Looking at https://github.com/openstreetmap/openstreetmap-website/blob/master/app/views/users/new.html.erb it would seem as if @client_app_name could be used to determine if we are authorizing or not.

OT: the text shown when client_app_name is set should probably be changed seems a bit weird right now,

tomhughes commented 2 weeks ago

Getting rid of account creation seems like an entirely backward way to satisfy that policy requirement, even if we accept that an creating an account on a web page triggered from an app counts as creating an account "within the app".

What's not clear to me is how you would satisfy that policy in the way they intend... Obviously if there actually was a "create account" button in the app you could have a "delete account" button but as there is no such thing in this case it's not obvious how to provide an equally prominent deletion option for a creation option that doesn't really exist.

mmd-osm commented 2 weeks ago

By the way, we also have a direct link to delete a user account, which is shown once you click the delete button on the account/edit page: https://www.openstreetmap.org/account/deletion

It is safe to call this page, you would have to confirm the deletion there by pressing the respective button.

image

In case of recent edits a cool down period applies (7 days in production)

image

if we do this google will hold us liable for the account deletion process by the OSMF which is naturally a non-starter.

I didn't really get the reasoning behind point. Care to elaborate why this is a non-starter?

simonpoole commented 2 weeks ago

See the policy document I linked above, section 'Account deletion policy' the 'you' there is the Google dev account holder, -not- the OSMF.

mmd-osm commented 2 weeks ago

Ok, this makes sense. The policy doesn't seem to cover the use case, where an app doesn't "own" or "manage" the account, and this is left to another platform on which you have built your app. I cannot imagine we're the only ones facing this issue. Do you know how other PaaS-based apps are handling this?

eginhard commented 2 weeks ago

Do you know how other PaaS-based apps are handling this?

This scenario came up recently for AnkiDroid - a flashcard app that is independent of the AnkiWeb service that manages data syncing. I didn't follow the details, but maybe there is something useful: https://github.com/ankidroid/Anki-Android/issues/16256

mmd-osm commented 2 weeks ago

Thanks for your feedback! So if I got this right, they have created an "account deletion" landing page for the Android app, which then points to the platform deletion page. Screenshots are up on: https://github.com/ankidroid/ankidroiddocs/pull/140

AntonKhorev commented 2 weeks ago

A per-app option to remove account creation links and everything else unnecessary? We already have an option to skip the permissions confirmation dialog, available only to admins.

mmd-osm commented 2 weeks ago

Hmm, yes we probably have all information available in https://github.com/openstreetmap/openstreetmap-website/blob/a45467d0bc754560cdfe6c4f4450449699ef2aff/app/controllers/concerns/session_methods.rb#L15 already but only extract the app name at the moment.

HolgerJeromin commented 2 weeks ago

Perhaps I do not understand, but would the whole idea destroy the quite likely flow:

Map with streetcomplete, try to uploaded data, create login, use login, upload data. With other editors this is less likely that a user has no account, but still possible.

simonpoole commented 2 weeks ago

With other editors this is less likely

This is mainly an issue for OsmAnd and maps.me/om the majority of SC users are existing contributors.

OsmAnd will likely run in to the same issue see https://osmand.net/docs/user/plugins/osm-editing/#authorization, OM no idea.

@HolgerJeromin just to clarify: while osmand etc might have the most users actually creating accounts it doesn't change the severity of the issue, as all users need to authorise their apps.

simonpoole commented 2 weeks ago

A per-app option to remove account creation links and everything else unnecessary? We already have an option to skip the permissions confirmation dialog, available only to admins.

IMHO this would be an elegant solution as it wouldn't require any changes for unaffected apps and in general would not require coding for those that are affected.

In any case I've implemented plan B https://github.com/MarcusWolschon/osmeditor4android/pull/2673 and will be submitting this to google so that we at least have some level of positive confirmation that the sign-up tab is what is causing the issue.

simonpoole commented 2 weeks ago

FWIW (that's not very much) seems as if Google has accepted the update today in which I zap the navigation header and the tabs with JS.

With other words I'll give a clean solution as outlined by Anton a look ASAP if nobody beats me to it.

HolgerJeromin commented 2 weeks ago

For the clean solution we should add a text like "If you do not have an account, please visit osm.org in your normal browser". Clumsy but perhaps the best thing we can do.