Skip to content
This repository was archived by the owner on Feb 25, 2019. It is now read-only.

exp / expires_in should be set to offset #258

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikb42
Copy link

@ikb42 ikb42 commented Oct 13, 2015

when client.default_max_age is present it should be used as an offset not as a value

when client.default_max_age is present it should be used as an offset not as a value
@christiansmith
Copy link
Member

Not sure the change on line 106 is correct. expires_in should be a duration, not a date/time.

Ran across the same issue on line 90 with @bauglir today while pairing and we fixed it, without including your conditional logic, which I think is more accurate. I'd accept a PR if you still want to address that.

I'm very sorry this slipped through the cracks until now. We're in a difficult transition these last few months.

@ikb42
Copy link
Author

ikb42 commented Apr 7, 2016

Old one. Re line 90, without the conditional logic with just nowSeconds(req.client.default_max_age) the token will be issued with an immediate exp time when req.client.default_max_age is undefined, which is fine as long as req.client.default_max_age is never undefined.
It is undefined at the moment for the default "client_name": "Anvil Connect CLI".

Re line 160, not sure now why I thought this should be changed as well, it may be ok as is. Sorry for leaving this this long.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants