kratos icon indicating copy to clipboard operation
kratos copied to clipboard

fix: claims.update_at is float64

Open kghost opened this issue 10 months ago • 4 comments

Related issue(s)

https://community.ory.sh/t/failed-to-unmarshal-updated-at-for-generic-oidc-provider-auth0/2004

When using one ory+hydra as OIDC provider to authenticated another kratos, I got an error that it is unable to unmarshal Claims.updated_at, because kratos-selfservice-ui-node returns the session and convert updated_at to a float64, because javascript/typescript doesn't have int64 type.

Checklist

  • [x] I have read the contributing guidelines.
  • [ ] I have referenced an issue containing the design document if my change introduces a new feature.
  • [x] I am following the contributing code guidelines.
  • [x] I have read the security policy.
  • [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got the approval (please contact [email protected]) from the maintainers to push the changes.
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have added or changed the documentation.

Further Comments

The proposed fix makes it competible with multiple types of updated_at field.

kghost avatar Apr 11 '24 11:04 kghost

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 11 '24 11:04 CLAassistant

Shouldn't this rather be fixed in ory/kratos-selfservice-ui-node?

alnr avatar Apr 12 '24 09:04 alnr

Shouldn't this rather be fixed in ory/kratos-selfservice-ui-node?

Sure, this problem can be fix there, by this PR makes our Kratos more robust without any downside, so can we consider it an improvement ?

kghost avatar Apr 15 '24 11:04 kghost

Shouldn't this rather be fixed in ory/kratos-selfservice-ui-node?

Sure, this problem can be fix there, by this PR makes our Kratos more robust without any downside, so can we consider it an improvement ?

The reason I'm hesitant is that Kratos would accept payloads which are arguably malformed. A unix timestamp is always an integer. If we start accepting floats too, we have to decide on how to handle rounding etc. I'd rather not do that and be more strict. Especially since we could never go back to only accepting integers because that would break existing usage.

alnr avatar Apr 15 '24 13:04 alnr

This issue is fixed in newer versions of ory/kratos-selfservice-ui-node and ory/hydra. Please create an issue if this still occurs. Thanks 👍

alnr avatar Sep 11 '24 13:09 alnr