Conversation
|
@bhousel thanks so much for the initial feedback via Slack! Much appreciated. I've let @HelNershingThapa know that an option is to make a new release of the libraries that is based on OAuth2 and anyone using them can choose which version to use. And that likely review will be post SOTM-US happening next week. Thank you in advance! |
|
Thanks @HelNershingThapa & @petya-kangalova ! |
|
Hi @bhousel ! Hope SOTM-US went well- I checked with our colleague Rob but he said he missed you at SOTM-US. Let us know if you will be able to take a look at the issues this week or that might not be possible. Thank you again in advance! |
| "dependencies": { | ||
| "ohauth": "~1.0.1", | ||
| "resolve-url": "~0.2.1", | ||
| "ohauth": "git+https://github.com/HelNershingThapa/ohauth.git", |
There was a problem hiding this comment.
I don't feel really comfortable pinning a dependency to your ohauth fork.
Can you instead build on a more widely used OAuth2 library like
https://www.npmjs.com/package/simple-oauth2
https://www.npmjs.com/package/client-oauth2
There was a problem hiding this comment.
This is only for the time being - until changes on this PR get published. After that, it could be replaced with the newly published version of ohauth.
There was a problem hiding this comment.
But that ohauth library is for OAuth1..
Sorry I didn't realize you submitted pull requests to both places.. but I think the correct way forward is:
- osm-auth should use a supported OAuth2 client library to talk to the OSM API.
- ohauth is a historical implementation of OAuth1 that we won't need anymore.
|
Thanks, I was confused by the changes to There's a lot of other stuff in this project that is becoming difficult to work with, so i'll freshen things up and try to get a new v2 release out real soon. |
|
Thank you so much for reviewing these PRs @bhousel ! |
This PR implements OAuth2.0 authorization framework.
Summary of changes:
timenonce(),getAuth(), etc. that were used to generate signatures have been removed since they are no longer required.Some changes were also made on the ohauth dependency - so for the time being, this PR uses my own forked ohauth repo as a dependency. I've also submitted a PR to ohauth upstream. Once those changes get verified, we could use the npm package after it gets published.