Add method to refresh a token using a refresh_token #313
Conversation
|
I'm currently using a modified version of this library to work around #240. I'd love it we could get this patch merged and published so that I can use the upstream code again. Give me a shout if you want changes and I'll respond ASAP. |
Co-Authored-By: modeswitch <github@ack.modeswitch.org>
Co-Authored-By: modeswitch <github@ack.modeswitch.org>
|
@luisrudge Changes applied to the copy-pasted comments. |
src/auth/index.js
Outdated
| * refresh_token: '{REFRESH_TOKEN}', | ||
| * }; | ||
| * | ||
| * auth0.refresh(data, function (err, userData) { |
There was a problem hiding this comment.
I'm sorry, I should've catched this before, but let's change this to refreshToken. Thanks 🎉
There was a problem hiding this comment.
No worries. Updated now.
There was a problem hiding this comment.
Sorry again 😢 I was talking about the method name 😬 from refresh to refreshToken. The refresh_token parameter has to remain the same so the API understands it.
|
@luisrudge Changed |
|
@modeswitch I updated the PR with the correct method name, but I noticed refresh tokens are not supported in OIDC flows. I'll talk internally with the team to make sure we should add that into the SDK. |
|
@luisrudge Ah sorry, I misunderstood your change request! Do you mean that refresh tokens are not supported in OIDC flows in this library, but should be? |
|
I mean that you won't get a refresh_token if you're using an OIDC conformant application (which is the default).
We're moving to an oidc-only sdk in the future and, if that's really the case, doesn't make sense to add this method now to deprecate it in the near future. |
|
One can still obtain a refresh token in an OIDC-conformant application, no?
Edit: I have OIDC-conformant enabled in my Auth0 application that I'm currently using with |
|
@modeswitch I'm sorry 😬 I was only looking at the Implicit flow docs. |
luisrudge
changed the title
|
Next release should happen next week 🎉 Thanks for all the help and patience |
|
@luisrudge No worries! Thanks for getting this merged in. Also, thanks to @dancrumb for writing the original code! |
This uses code from here, with a couple of fixes required to make it work.
Fixes #240