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

allow Numeric for iat field on OIDC tokens #4518

Merged
merged 2 commits into from Dec 17, 2020

Conversation

mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Dec 17, 2020

A partner reported having issues with the iat field from I suspect the changes in #4504. ruby-jwt allows Numeric instead of integer (https://github.com/jwt/ruby-jwt/blob/master/lib/jwt/verify.rb#L48), so my hunch is that we're receiving a Float.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

For additional consideration, here or otherwise:

  • Should we update error texts to remove reference to "must be an integer", since it can now be a non-integer numeric?
  • Irrespective if this is causing issues for partners that didn't occur previously, should we want to only allow integer timestamps?
  • Test coverage for whatever we decide

@mitchellhenke
Copy link
Contributor Author

mitchellhenke commented Dec 17, 2020

Should we update error texts to remove reference to "must be an integer", since it can now be a non-integer numeric?

Yep, updated in 202b887

Irrespective if this is causing issues for partners that didn't occur previously, should we want to only allow integer timestamps?

I don't have a lot of concern about floats in this case since we're not doing any math on them beyond truncating to integer and doing a comparison. I think it'd be fine to restrict to integers if we gave some advance warning.

Test coverage for whatever we decide

This was more difficult than anticipated since our JWT encoding library used in the tests doesn't allow floats to be encoded, though it will be supported in the next release: jwt/ruby-jwt#327

@mitchellhenke mitchellhenke merged commit 8c45e88 into master Dec 17, 2020
@mitchellhenke mitchellhenke deleted the mitchellhenke/allow-any-numeric-for-oidc-iat branch December 17, 2020 16:37
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