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

JOSM Auth Token #185

Merged
merged 28 commits into from Sep 25, 2019
Merged

JOSM Auth Token #185

merged 28 commits into from Sep 25, 2019

Conversation

ingalls
Copy link
Contributor

@ingalls ingalls commented Sep 12, 2019

Context

An unexpected side affect of allowing full server auth in #184 was that JOSM doesn't send authentication except when creating objects. This effectively broke JOSM integration for servers that ran full authentication.

This PR sidesteps the issue by allowing users to create a URL with the access token in the URL, which can then be used in JOSM, circumventing the need for Basic auth for normally unauthenticated read operations.

Screenshot from 2019-09-12 14-47-31
Screenshot from 2019-09-12 14-47-36

cc/ @ingalls

@ingalls ingalls changed the title Josm Auth Token JOSM Auth Token Sep 12, 2019
@miccolis
Copy link
Contributor

@ingalls can we limit this path based token so that it cannot be used for endpoints that are not data reading? ...or is that here in some way that I'm missing?

@ingalls
Copy link
Contributor Author

ingalls commented Sep 13, 2019

@miccolis I initially had a very rough scope that could be conditionally added to the token to limit it, however it was very rough and at the end of the day the URL based tokem its way better than our basic auth and equivalent to our session token. Both of which are protected by our use of HTTPS on the endpoint.

I'd really like to take on token scope, but I would like to do it "right" and make the scopes equivalent to the auth JSON categories. Basically expose an interface like

- [ ] READ
- [ ] WRITE

OR

Full Auth JSON checkboxes
- [ ] Deltas
- [ ] Users
- [ ] Styles
- [ ] ... etc ...

@ingalls
Copy link
Contributor Author

ingalls commented Sep 13, 2019

WIP of scoped tokens here: https://github.com/mapbox/Hecate/tree/josm-token-scope

@ingalls
Copy link
Contributor Author

ingalls commented Sep 13, 2019

@miccolis I've got to run to the airport, will be back online in a few hours.

  • The branch referenced above should implement a basic read and full scope. Tokens that are authed via the in URL method are given a read scope and as such should not be able to access a Full scoped API endpoint (any write endpoint)
  • Write tests to ensure that an in URL token can't access a full method (setting a meta attribute would be a good candidate for this test)
  • Ensure that when running cargo run -- --auth test/fixtures/auth.default.json, you are able to login via the browser and generate a JOSM URL. (Click on your user icon and it will open user settings, the JOSM URL generation button will be at the bottom) If you run a test before hand that creates a user, then this command the default test user is ingalls:yeaheh. Otherwise you will have to manually create a user in the database as the register API will be protected.

@ingalls
Copy link
Contributor Author

ingalls commented Sep 25, 2019

LGTM 🌮

Copy link
Contributor

@lizziegooding lizziegooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌮

@mattciferri mattciferri merged commit 0162a48 into master Sep 25, 2019
@mattciferri mattciferri deleted the josm-token branch September 25, 2019 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants