Migrate to OAuth2
Since OAuth 1 will be deprecated soon in OSM, remove it from our code and only use OAuth 2. The code base already handles OAuth2 well, so I removed all the places where OAuth 1 is used. I also removed the need to authenticate from routes that are not "guarded". This will affect users who did not clean the app / did a logout-login / installed the app recently (In the last 1.5 years since #1798 was published).
Closes #1968 Resolves #1985
I have tried to migrate the code to use the following webtokenhandler but the documentation is lacking and I think that it will end up very close to what I did anyway, so this closes this issue without implementing it.
- #1968
FYI: @zstadler
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
What is the planned user experience for those who still use OAuth1? The server shos about 2 such requests per minute
The client will get a 401, current client code doesn't do anything with it, besides showing generic error code according to the relevant operation. I can change the client code to logout, which is the usual behavior for such a case, but this won't affect clients that are not up to date.
Also the rate will be dramatically reduced, as most operations do not require a logged in user.
Codecov Report
Attention: Patch coverage is 88.23529% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 91.02%. Comparing base (
bae2247) to head (81ab25c).
| Files | Patch % | Lines |
|---|---|---|
| IsraelHiking.API/Controllers/OsmController.cs | 66.66% | 1 Missing :warning: |
| ...elHiking.API/Services/Osm/OsmAuthFactoryWrapper.cs | 50.00% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1982 +/- ##
==========================================
+ Coverage 91.01% 91.02% +0.01%
==========================================
Files 93 93
Lines 6821 6808 -13
Branches 982 981 -1
==========================================
- Hits 6208 6197 -11
+ Misses 411 410 -1
+ Partials 202 201 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
How difficult would it be to add user migration code to the client, as you did during the migration to pmtiles? Any simple mechanism would do, even a "Please logout and login" popup, as long as the reliance on OAuth1 credentials is not silently ignored.
I can check the current token and logout the user in case of oauth1. The current code will force the next login to be of oauth2 type. This is the easiest solution I believe as I only need to delete the user info from the state.
Codecov Report
Attention: Patch coverage is 88.23529% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 90.77%. Comparing base (
153f0de) to head (2258547).
| Files | Patch % | Lines |
|---|---|---|
| IsraelHiking.API/Controllers/OsmController.cs | 66.66% | 1 Missing :warning: |
| ...elHiking.API/Services/Osm/OsmAuthFactoryWrapper.cs | 50.00% | 0 Missing and 1 partial :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## main #1982 +/- ##
==========================================
+ Coverage 90.76% 90.77% +0.01%
==========================================
Files 94 94
Lines 8193 8177 -16
Branches 992 991 -1
==========================================
- Hits 7436 7423 -13
+ Misses 531 529 -2
+ Partials 226 225 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.