- Notifications
You must be signed in to change notification settings - Fork 1.2k
Added API endpoint for token refresh#172
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
Conversation
…e {complete: true} option is on
…dpoint to refresh a token. A refresh is considered to have the same token returned, but with a later expiry time. Can take into account the header + payload of a decoded token ie : {complete: true} More in depth testing with comparision of equality Testing failures and async mode Added description for refresh in README.md
Kiura commented Feb 4, 2016
+1 |
1 similar comment
matthewchung74 commented Feb 9, 2016
+1 |
Very nice job, I will merge this! |
👍 Excellent! Thank you! |
mateeyow commented Mar 3, 2016
Quick question, when will this be merged? Any ETA? |
pmeijer commented Mar 9, 2016
+1 |
arturskeeled commented Mar 17, 2016
+1 Any news on when is this being implemented? |
@mateeyow@arturskeeled I'm not sure when the next major release will be coming out, but I have just updated my forked version with the refresh endpoint : https://github.com/jppellerin/node-jsonwebtoken. You could use this for development purposes. Disclaimer: I have not tested the merge thoroughly.. |
rickli1989 commented Apr 8, 2016
Waiting for this to get released |
rickli1989 commented Apr 15, 2016
When is the next major release for this ? |
RWOverdijk commented May 3, 2016
👍 we'd really like to implement refresh tokens. |
VMBindraban commented May 12, 2016
Any ETA yet? |
RWOverdijk commented May 25, 2016
Bump... |
RWOverdijk commented Jun 22, 2016
Last bump. I don't want to fork but I really need this. |
@RWOverdijk : I have just updated my fork with the newest version and fixed the tests. I can keep this up to date with the newest features here until it gets merged in. If you want some assurances, I can tag you a version. That's the best I can do. |
@@ -172,6 +172,28 @@ console.log(decoded.header); | |||
console.log(decoded.payload) | |||
``` | |||
### jwt.refresh(token, expiresIn, secretOrPrivateKey [, callback]) | |||
Will refresh the given token. The token is __expected__ to be *decoded* and *valid*. No checks will be performed on the token. The function will copy the values of the token, give it a new expiry time based on the given `expiresIn` parameter and will return a new signed token using the `sign` function and given secretOrPrivateKey. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐⭐⭐⭐⭐
mitchellporter commented Jul 24, 2016
Is there a reason this hasn't been merged yet? |
morgondag commented Jul 27, 2016
forking is the place. |
Isaac6702 commented Jul 28, 2016
Now you can use this? |
I am still considering this feature, I think as it is it doesn't add too much value over |
@jfromaniello Thanks for the feedback. The goal (for what we are using it for) is to extract all fields of a token and return it with a new I didn't include verification since the token needs to be decoded for this to work. Therefore, it assumes the user has manipulated this - thus it would be safe to assume that it has been validated. I agree that if one uses this API the wrong way and refreshes a non-valid JWT, then a valid one will be returned, which has cause for concern. The value this endpoint has is to save the logic of copying over the fields of the existing token and re-signing it. I can agree with you that it is not something extremely difficult to do, but is something that is useful and that many people are using (or so it seems by the interest of this pull request). I would assume that this is going to be more and more common as people are moving towards serverless architectures/completely stateless. All that being said: I think that verification would be a good idea. I also think it is a useful endpoint (I may be biased since I'm using this). |
```js | ||
// ... | ||
var originalDecoded = jwt.decode(token, {complete: true}); | ||
var refreshed = jwt.refresh(originalDecoded, 3600, secret); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be var refreshed = jwt.refresh(**token**, 3600, secret);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the point of this. You have to decode and validate it, and you will have the payload, why not just call sign on that payload instead of having a refresh function that will decode it again?
@jfromaniello Maybe this pull request should be rejected if it's not going to be merged? It has been over a year now. |
omar-haris commented May 22, 2017
@jppellerin can you give me an example to use it ? |
natealcedo commented May 23, 2017
Whats the update on this pull request? It's been over a year. I agree with @jppellerin that it should either get merged if not rejected. @jppellerin Have you considered making a fork of this and pushing to npm? |
@fiftysoft : An example is in the readme docs of the pull request. @ndaljr : I have considered making a fork, but the problem is keeping up with the rest of the library. I don't want to have the code duplicated and potentially falling behind on updates. If this was to become it's own module, it would have to only extend it. Which, I might put some thought into this. |
oshalygin commented Jun 30, 2017
Any status on this? |
Fixed by documentation: #371 |
Well this feels like a big slap in the face for the people who were waiting for it. |
@VMBindraban: Since you still had to decode and validate it before using the refresh function, that function added very little value over just deleting old exp/iat and using sign instead. Keeping the API and the code clean and small without unnecessary complexity is more important than a convenience method that is hardly even convenient. |
BrOrlandi commented Jul 26, 2017
I have found a problem in the refresh function that's causing it to generate the same token. |
What if the original token gets stolen? |
RWOverdijk commented May 22, 2020
deepakchandola717 commented May 22, 2020
I mean this feature could pose a security issue if any app is using it. There's no point in making it expire only in first place. |
RWOverdijk commented May 22, 2020
You're commenting on an issue from 2016 dude. But to answer your question: every form of session/auth has the same issue. Cookies: get the token. JWT: get the token. oauth: get the token. Just make it really really difficult to steal the token. 😄 |
deepakchandola717 commented May 22, 2020
Ok you just think why do we set an expiry to the token in first place and why do we have two different types of token. |
deepakchandola717 commented May 22, 2020
This is a very old issue and every one was asking why feature wasn't merged. Thought this could be a reason. |
According to issue #122, there was some interest in having an endpoint to refresh a token.
A refresh is considered to have the same token returned, but with a later expiry time.
This function takes in a decoded token, copies the attributes of this token over, and with that data, proceeds to signing a new token. The only piece of data that is replaced is the
expiresIn
value, which is passed in the function parameters. By extension, theiat
value is also changed since a new token is created.