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

Adjusting expiration time to be 7 days #78

Merged
merged 3 commits into from Oct 16, 2017
Merged

Conversation

emersonmellado
Copy link
Contributor

No description provided.

@@ -36,7 +36,7 @@ function generateToken(req, GUID, opts) {
// By default, expire the token after 7 days.
// NOTE: the value for 'exp' needs to be in seconds since
// the epoch as per the spec!
var expiresDefault = Math.floor(new Date().getTime()/1000) + 7*24*60*60;
var expiresDefault = Math.floor(Date.now() + (1000 * 60 * 60 * 24 * 7));
Copy link
Member

Choose a reason for hiding this comment

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

expiresIn is defined in seconds not Milliseconds.
see: https://www.npmjs.com/package/jsonwebtoken
image
and:
image

else if( url === '/logout') { app.logout(req, res, app.done); } // end session
else if( url === '/exit') { app.exit(res); } // for testing ONLY
else { app.notFound(res); } // 404 error
var path = url.parse(req.url).pathname;
Copy link
Member

Choose a reason for hiding this comment

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

good plan. 👍

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@emersonmellado thanks for this PR! the update to url/path is good. 👍
However the expiration time needs to be in seconds so you need to divide by 1000 before (or after) the multiplication. 😉

@emersonmellado
Copy link
Contributor Author

Wow. Thanks @nelsonic. I didn't know about this doc.

Apparently they have a simpler way to set the Expiration by using a 7d notation. Check it out! :)

@nelsonic
Copy link
Member

Yeah, noted the String "7d" for expiresIn.
image

It's worth noting that this is a convenience of the jsonwebtoken Node Module and not part of the proposed standard. but this code works so your PR is good. 👍
Thanks again!

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@emersonmellado thanks very much for making these updates. 🎉

@nelsonic nelsonic merged commit 660e2e0 into dwyl:master Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants