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.

Sign up for GitHub

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

Jump to bottom

'iat' added to payload should be an int #33

Closed
mamciek opened this issue Apr 17, 2015 · 7 comments
Closed

'iat' added to payload should be an int #33

mamciek opened this issue Apr 17, 2015 · 7 comments

Comments

@mamciek
Copy link

mamciek commented Apr 17, 2015

It encoded as string now, and it should be a number:
https://tools.ietf.org/html/draft-ietf-oauth-json-web-token-32#section-4.1.6

"Its value MUST be a number containing a NumericDate value"
@odino
Copy link
Contributor

odino commented Apr 18, 2015

Thanks man, good catch! Would you mind sending a PR? :)

daleattree pushed a commit to daleattree/jose that referenced this issue Oct 27, 2015
Made it an integer as per issue namshi#33
fdbf4ac
@daleattree daleattree mentioned this issue Oct 27, 2015
Closed
@cirpo
Copy link
Contributor

cirpo commented Dec 30, 2015

the problem is that changing forcing it to be an int as done in the PR old tokens might be invalid.
We still need to figure out a possible solution

@shark0der
Copy link

I stumbled across this issue as well. The tokens generated usign this lib are not compatible with https://github.com/auth0/node-jsonwebtoken and we were forced to use an ugly workaround. As per RFC, exp, nbf and iat - all 3 should be numeric. Right now I don't know whether only iat was fixed or all 3.

As for backward compatibility, I think a version bump would be enough so developers won't upgrade by mistake.

@odino
Copy link
Contributor

odino commented Feb 21, 2016

@shark0der it's not only BC, you have to think that all old tokens will be considered invalid, so we need a mechanism to allow for an easier migration -- else imagine all your logged in users will be logged out as soon as you deploy. For some it's not a problem, for some others its quite annoying.

Anyhow, we'll think of something with @cirpo this week and will let you know -- feel also free to come up with a PR that address the issue of verifying old tokens where iat and other parameters are strings.

@odino odino mentioned this issue Mar 2, 2016
Closed
@stuartm
Copy link

stuartm commented Mar 2, 2016

Ok, I missed this when I created my pull request. No guarantees of a more extensive patch from my end only because we're not using the verification in this application. Good to know that others were already looking at the issue however.

@shark0der
Copy link

@odino what I actually meant to say was a major/minor version bump, not just the patch number (considering a major.minor.patch versioning scheme). This way, packages that still depend on jose will not update by mistake.

@odino
Copy link
Contributor

odino commented Mar 7, 2016

👍 -- I think what we will do is to go for the major version bump and will share a snippet on how to overcome having everyone logging out

geshan pushed a commit to geshan/jose that referenced this issue Jul 10, 2016
Fixes namshi#33 - iat added to payload should be an int
af5b7b3 
- [x] Test to check backwards compatibility added
geshan pushed a commit to geshan/jose that referenced this issue Jul 11, 2016
Fixes namshi#33 - iat added to payload should be an int
3c684d5 
- [x] Test to check backwards compatibility added
- [x] Exp should be an integer too
- [x] supports string as of now for backwards compatibility
- [x] Uses time function instead of $now->getTimestamp()
geshan pushed a commit to geshan/jose that referenced this issue Jul 11, 2016
Fixes namshi#33 - iat added to payload should be an int
0eb0d6c 
- [x] Adds tests to check backwards compatibility added
- [x] Makes exp an integer too
- [x] Supports string as of now for backwards compatibility
- [x] Uses time function instead of $now->getTimestamp()
- [x] Adds tests section in readme
geshan pushed a commit to geshan/jose that referenced this issue Jul 12, 2016
Fixes namshi#33 - iat added to payload should be an int
3064e8d 
- [x] Makes iat and exp as integer
- [x] Uses time function instead of $now->getTimestamp()
- [x] Adds tests section and backwards compatibility notice in readme
- [x] Adds tests for iat and exp to be integer
geshan pushed a commit to geshan/jose that referenced this issue Jul 12, 2016
Fixes namshi#33 - iat added to payload should be an int
f488430 
- [x] Makes iat and exp as integer
- [x] Uses time function instead of $now->getTimestamp()
- [x] Adds tests section and backwards compatibility notice in readme
- [x] Adds tests for iat and exp to be integer
geshan pushed a commit to geshan/jose that referenced this issue Jul 12, 2016
Fixes namshi#33 - iat added to payload should be an int
1da5b2b 
- [x] Makes iat and exp as integer
- [x] Uses time function instead of $now->getTimestamp()
- [x] Adds tests section and backwards compatibility notice in readm
- [x] Adds tests for iat and exp to be integer
- [x] Uses `setExpectedException` in new tests to support php 5.5 with phpunit 4.x
geshan pushed a commit to geshan/jose that referenced this issue Jul 12, 2016
Fixes namshi#33 - iat added to payload should be an int
4ee2722 
- [x] Makes iat and exp as integer
- [x] Uses time function instead of $now->getTimestamp()
- [x] Adds tests section and backwards compatibility notice in readm
- [x] Adds tests for iat and exp to be integer
- [x] Uses `setExpectedExceptionRegExp` in new tests to support php 5.5 with phpunit 4.x
@odino odino closed this as completed in 0f4e398 Jul 13, 2016
odino added a commit that referenced this issue Jul 13, 2016
@odino
Merge pull request #100 from geshan/issue-33
2617ff8 
Fixes #33 - iat added to payload should be an int
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@cirpo @mamciek @odino @stuartm @shark0der