do omit exp if ExpiresAt is zero value

[nerocrux]

Related issue

• None

Proposed changes

• Make sure exp will be omitted if GetExpiresAt(AccessToken) is nil or zero value (no expiration)
• Currently, even if GetExpiresAt(AccessToken) is zero value, Unix() will be called, and ExpiresAt will be set to -62135596800 because golang's zero time is 0001-01-01T00:00:00Z but not 1970-01-01T00:00:00Z

Checklist

• I have read the contributing guidelines
• 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 green light (please contact hi@ory.sh) from the maintainers to push the changes.
• I signed the Developer's Certificate of Origin
  by signing my commit(s). You can amend your signature to the most recent commit by using git commit --amend -s. If you
  amend the commit, you might need to force push using git push --force HEAD:<branch>. Please be very careful when using
  force push.
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation within the code base (if appropriate)

Further comments

• Currently, even if we introspect RefreshToken, AccessToken's ExpiresAt will be return in exp. Not sure if this is corrent. Maybe it's better to always omit exp field if we introspect RefreshToken, because there is no expiration time for RefreshToken.
• If omit exp for introspect RefreshToken is good, I will fire another PR.

Thank you!

[aeneasr]

Currently, even if we introspect RefreshToken, AccessToken's ExpiresAt will be return in exp. Not sure if this is corrent. Maybe it's better to always omit exp field if we introspect RefreshToken, because there is no expiration time for RefreshToken.

Due to community feedback, I think it would make sense to have refresh tokens expire optionally. Basically you would be able to set a lifespan, but if none is set the refresh token expires after that time. I think the endpoint should still be able to omit the expiry if it's not set!

[aeneasr]

Can you add a test (or modify an existing one) for this?

[nerocrux]

Sure, will do later.

[nerocrux]

@aeneasr I have added some tests for this PR. Please take a look at it if you have time.

Due to community feedback, I think it would make sense to have refresh tokens expire optionally. Basically you would be able to set a lifespan, but if none is set the refresh token expires after that time. I think the endpoint should still be able to omit the expiry if it's not set!

Got it.
By the way, I'm totally agree with your opinion on refresh token's expiration here. I don't think refresh token's expiration will make our system safer, and to manage expiration time of a certain token may increase the complexity.

Have a nice weekend!

[aeneasr]

Agreed, thank you for the changes!