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

StreetComplete refuses to operate without authorization to upload GPX tracks to OSM #4122

Closed
gdt opened this issue Jun 15, 2022 · 19 comments
Closed
Assignees
Labels
feedback required more info is needed, issue will be likely closed if it is not provided

Comments

@gdt
Copy link

gdt commented Jun 15, 2022

  • summary

I was logged out, and on attempting to log in was presented with the OSM oauth page, which was fine.

  • reproduce
  1. Upgrade from an earlier version that for whatever reason lost credentials.
  2. Solve a few quests (3). (Parking lot type, do you have to pay, surface of parking lane, but surely that does not matter.)
  3. See a screen "You have 3 not yet published changes. You need to login to have them uploaded."
  4. Click LOGIN
  5. Enter username and password, click remember me, click login
  6. On 'Authorize" screen, uncheck "upload GPS traces", because I don't want to permit StreetComplete to do that [1]
  7. Click "Grant Access"
  8. See error toast that says "Authorization failed. All listed permissions must be granted".

It seems to me that not having track upload permission should just cause the app to work normally except that when trying to upload a track, fail with "permission is known not to be granted" rather than "tried and got an error from the server". Or maybe just disable the setting that would upload - doesn't really matter exactly how this is handled, as long as it doesn't cause the app to completely fail to function.

I don't see anything in settings about uploading tracks. So presumably that means tracks are never uploaded?

I get it that "modify map" is not optional; that is the point.

[1] The act of mapping publishes information, basically a location and a time. The way I use SC, that's a place/time when I solve a quest. I know SC fairly recently captures track data and displays it to me, to be helpful in quest solving and understanding what's going on, and that's ok. But I don't want to upload tracks, because that is more information than necessary to map, and it is too hard to figure out how dangerous that is and what is uploaded when. (It seems obvious that track upload should be opt in and clicked for ok, but at the moment I don't understand that.) So, my approach is to just not authorize uploading.

  • versions

StreetComplete 43.2 from F-Droid, on Android 12L (via CalyxOS). (This phone/OS has been using StreetComplete without issues for a while.)

@gdt gdt added the bug label Jun 15, 2022
@Helium314
Copy link
Collaborator

Track upload is opt-in, it only happens if you choose to record a track (long press anywhere on the map) and "append" it to a note.
Creating / uploading is only possible in StreetComplete 44 or later.

@riQQ
Copy link
Collaborator

riQQ commented Jun 15, 2022

Just for context:
It looks like 43.2 and higher was set up to necessarily require the Upload tracks permission.

See
https://github.com/streetcomplete/StreetComplete/releases/tag/v43.2
and
#4028 (comment)

@gdt
Copy link
Author

gdt commented Jun 15, 2022

I gather you are explaining how we got here, which is of course useful, but not allowing the user to decline to grant track upload permission is still a bug.

@westnordost westnordost self-assigned this Jun 15, 2022
@westnordost
Copy link
Member

westnordost commented Jun 15, 2022

@gdt the reason why you would want to disallow track upload is based on a misunderstanding: The blue track left on the map while walking around is not uploaded to OpenStreetMap. No track recordings are uploaded to OSM without the user's explicit intent.

The permission is used to "attach" a GPX track to a note. To do this, long-press on the map, select "Create new track recording". A "stop recording track" button will appear and the color of the track you leave on the map while walking changes to red. When you press this button, the track recording is ended and you are prompted to write a note. When the note is published, the track recorded "in context of" that note is uploaded and linked from the created note.

So in other words, you don't need to not allow GPX upload.

True, these permissions are not strictly necessary for the other things. However, gracefully handling such situation would probably involve showing an explanation to the user that previously did not check the GPX permissions that he has to re-login and check those to use this feature etc.. but that is too much work IMO for a situation that was based on a misunderstanding.

What I could do is to simply not require these permissions. This means that if you actually create a GPX track to "attach" to a note and try to upload it, you will automatically be logged out and prompted to login. Without explanation why you were logged out.

Do you find this better than the current behavior? I am really not sure if the user should be allowed to selectively not allow some permissions. After all, the "create notes" permission is also always required, even if the user never (intends to) create notes.

@westnordost westnordost removed their assignment Jun 15, 2022
@westnordost westnordost added feedback required more info is needed, issue will be likely closed if it is not provided and removed bug labels Jun 15, 2022
@westnordost westnordost self-assigned this Jun 15, 2022
@gdt
Copy link
Author

gdt commented Jun 15, 2022

@gdt the reason why you would want to disallow track upload is based on a misunderstanding: The blue track left on the map while walking around is not uploaded to OpenStreetMap. No track recordings are uploaded to OSM without the user's explicit intent.

It's not so much a misunderstanding as a lack of understanding, combined with the Principle of Least Privilege (hence PLP) as articulated by Saltzer, which says that things should have the smallest amount of permissions that are necessary. I routinely deny track upload permission to everything else in OSM Oauth , such as JOSM. There is a culture within some parts of OSM that all tracks should be published and I've been criticized in the past for not doing so. So I think there is plenty of reason to be shy about this.

Actually I was unhappy to see a track being recorded at all, but with no upload permission it wasn't bothersome enough to complain or uninstall.

I am definitely not accusing you of sneakily uploading tracks. Indeed I would be shocked if any were uploaded without it being really clear and opt in. What you are actually doing seems ok -- it's just that I don't want to go near it because it's too hard to reason about everything especially with future changes, and because it requires more trust, contrary to PLP. It's really that I don't want anything authorized to do anything I don't need it to do; this is sort of like denying Contacts permission to an Android app, which could use it for completion, but should work (without completion) if denied.

The permission is used to "attach" a GPX track to a note. To do this, long-press on the map, select "Create new track recording". A "stop recording track" button will appear and the color of the track you leave on the map while walking changes to red. When you press this button, the track recording is ended and you are prompted to write a note. When the note is published, the track recorded "in context of" that note is uploaded and linked from the created note.

A reasonable feature; I'm not objecting to it existing. It's good that it conly records on request.

So in other words, you don't need to not allow GPX upload.

For me allowing it is a PLP error.

True, these permissions are not strictly necessary for the other things. However, gracefully handling such situation would probably involve showing an explanation to the user that previously did not check the GPX permissions that he has to re-login and check those to use this feature etc.. but that is too much work IMO for a situation that was based on a misunderstanding.

What I could do is to simply not require these permissions. This means that if you actually create a GPX track to "attach" to a note and try to upload it, you will automatically be logged out and prompted to login. Without explanation why you were logged out.

Do you find this better than the current behavior? I am really not sure if the user should be allowed to selectively not allow some permissions. After all, the "create notes" permission is also always required, even if the user never (intends to) create notes.

Yes, I find that better. I would also find a "can't upload GPX track because permission wasn't granted" toast totally fine.

I'd also be fine with just not enabling the note track feature at all if the permission wasn't granted, and not even let one be recorded. And requiring track upload on auth if there are pending tracks to upload.

As for notes, that doesn't bother me as much, because it's really inconceivable that there'd be an note created that wasn't from text entered for a note. And notes and map objects, while separate permissions, are sort of the same thing.

I'm honestly not sure what I'll do if this bug gets wontfix, but this minute I'm leaning to uninstalling. I realize I'm unusual and that it may not make sense for you to address my complaint, even if you lose a user. I won't take it personally, and thanks for the discussion.

@westnordost
Copy link
Member

Yes, I find that better. I would also find a "can't upload GPX track because permission wasn't granted" toast totally fine.

I'd also be fine with just not enabling the note track feature at all if the permission wasn't granted, and not even let one be recorded. And requiring track upload on auth if there are pending tracks to upload.

Well, that is the thing that requires implementation effort. In particular, the app would need to memorize on login which permissions have (not) been granted. It doesn't do that right now. Why does my proposed solution work at all then? Because whenever the OSM API returns a 401 or 403 error on upload, the app logs out the user automatically and prompts to log in again. If one tries to upload GPX traces without OAuth permission, such error is returned.

@westnordost
Copy link
Member

Alright I will just not require these permissions then. A PR that handles this case more gracefully would be accepted.

@gdt
Copy link
Author

gdt commented Jun 15, 2022

Understood about effort. Your proposed "just skip chcking at auth time" works for me and the few other PLP/paranoids and probably non-paranoids won't uncheck anything.

@gdt
Copy link
Author

gdt commented Jun 15, 2022

Thanks - will test when it hits f-droid. (Sorry, am trying to be a mere user of many of the things I use, as getting into more on all of them would be infeasible.)

@mnalis
Copy link
Member

mnalis commented Jun 18, 2022

I've also noticed that the app requires that it can read your private GPS traces. Is that permission really needed (and why?), if SC only needs to upload the traces (which I guess would be only WRITE_GPS_TRACES but not also READ_GPS_TRACES) ?

It might (should!) make people uncomfortable too, so unless there is some technical peculiarity which requires that one must allow reading old private tracks in order to upload new tracks, it should not be requested at all. And if there is technical peculiarity, we should probably note in privacy policy why it is so.

@goldbattle (as PR #3573 author), can you clarify if/why READ_GPS_TRACES is needed to just upload tracks?

@goldbattle
Copy link
Contributor

I think I originally did this to make sure the id and username for creating the url to attach to the note was correct. But I think it could be implemented with the upload only. If you open an enhancement issue, I can try to find time to investigate / address. Hope this helps you.

@gdt
Copy link
Author

gdt commented Jul 14, 2022

An update with this change recently became available f-droid, and after updating it, I was able to log in w/o GPS trace permission and upload my ~25 pending changes accumulated and solve and upload new quests. So this is confirmed fix in the release. Thank you for accomodating those of us who are stingy about permission granting.

@jf99
Copy link

jf99 commented May 13, 2023

@goldbattle This is still/again an issue in SC v52.1 from F-droid. I unticked all permissions except editing the map and the app complains that I must grant all permissions.

@matkoniecz
Copy link
Member

What happens when Permission.READ_PREFERENCES_AND_USER_DETAILS and Permission.WRITE_NOTES are also granted?

@jf99
Copy link

jf99 commented May 22, 2023

@matkoniecz Then it works, indeed. I don't understand though what these two permissions are needed for.

Also, StreetComplete apparently forgot all the edits that I made last week – it did not upload the changes although I logged in several hours ago. It asked me the same questions again and these new answers were uploaded immediately.

@matkoniecz
Copy link
Member

matkoniecz commented May 22, 2023

I don't understand though what these two permissions are needed for.

Notes access is needed to handle notes, for example in "can't say" escape hatch where StreetComplete question makes no sense as data is broken/outdated.

Do not remember is READ_PREFERENCES_AND_USER_DETAILS needed for anything.

apparently forgot all the edits that I made last week –

Have you reinstalled the app?

@Helium314
Copy link
Collaborator

Have you reinstalled the app?

or updated from pre-53 to 53.0?

@jf99
Copy link

jf99 commented May 23, 2023

or updated from pre-53 to 53.0?

Yes, that's probably it.

@westnordost
Copy link
Member

Granting partial permission may not be possible anymore when we migrate from OAuth 1.0a authorization to OAuth 2.0 authorization, see the linked issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback required more info is needed, issue will be likely closed if it is not provided
Projects
None yet
Development

No branches or pull requests

8 participants