#235 Replace usage of java.util.Date

[pveeckhout]

Replace usage of java.util.Date with java.time.Instant in JWT claims

This commit replaces the usage of java.util.Date with java.time.Instant in handling JWT claims. This means upgrading from JDK7 to JDK8. This is done for better adherence to ISO 8601 standards and better precision in time-related operations. Classes working with claims have been updated to use the newer Instant class, providing better precision and cross-timezone compatibility. Additionally, overloading convenience methods for setting OffsetDateTime and ZonedDateTime on top of Instant have been added on the JwtBuilder and DefaultJwtBuilder.

Closes #235

[pveeckhout]

There are 2 failing tests:

io.jsonwebtoken.JwtParserTest#testParseRequireCustomInstant_Success
io.jsonwebtoken.JwtParserTest#testParseRequireCustomInstant_Incorrect_Fail

If a custom claim is added with type instant, then there are issues in deserializing it. The Instant is saved as a numeric date according to the RFC standard (epochSeconds), while io.jsonwebtoken.impl.lang.JwtDateConverter#toInstant expected it to be in epochMillis.

A decision needs to be made how this needs to be handled.

[lhazlewood]

@pveeckhout Thanks for taking an interest in helping with this!

However, we're not ready to force a JDK 8 upgrade until remaining JWT functionality is finished. #308 has some additional work in addition to the Java Date/Time changes for #235. #474 might also come into play instead of the existing builder utility methods as well. Some design work needs to be done before we can resolve this in earnest.

I'm happy to leave this PR open of course, but you might have to deal with changes/rebases as we make other changes first. I hope that's ok!

[pveeckhout]

@lhazlewood, My pleasure to help! I am using jjwt in my own projects, and figured it might be time to try and contribute back to project.

I will gladly keep this PR up to date with the other incoming changes.

If there any other issue you figure I could help with, feel free to direct me to them.

[bdemers]

Thanks!
I like this, I was playing around with something similar a little while ago.

I worry a little about folks migrating.
One option would be to add back the old setter methods with Date parameters, potentially marking them as deprecated, though. As much as I prefer using Instant lots of folks are probably still using Date)

This doesn't work for the getters, but we could add deprecated getters, something like Date getExpirationDate() { return Date.from(expiration) }
It would still add a bit of burden on developers when updating, not sure if that's worth it. Or if we should do something else.

Thoughts?

[bdemers]

If adding these other wrappers, keeping the ability to wrap a Date would help folks, and lessen the burden when migrating

[pveeckhout]

personally I would avoid keeping java.util.Date around even as a convenience or temporary warpper

This would require a java version increase, and thus mean a major release. In my opinion it would be the ideal time to 'force' the devs to fix the breaking changes done in the major bump.

ofc, I am not the maintainer and will bend to the will of the code owner. just my 2cents

[bdemers]

Keep the old signatures for the deprecated methods

[pveeckhout]

this data change requires is a major release, I would actually opt to remove these deprecated methods (in another pull request)

I modified these just to ensure it all compiles and not longer requires java.time.Date

[bdemers]

I get that, but the method is marked as deprecated, so that shouldn't be changed.
(this is basically removing a old deprecated method, and adding a new deprecated method)

In general (and when possible) we want to provide an easy upgrade path for users, (e.g. adding an extra setter and deprecating the old one).
That's not possible with getters, (which would be a breaking change).

[pveeckhout]

the deprecated methods haven been reverted to use java.util.Date

[bdemers]

Another potential addition related to using the newer dates api would be to add a convenance method to set the nbf, exp, and iat together, something like:

JwtBuilder validFor(Duration duration) {
  Instant now = Instant.now(clock);
  return this.expiration(now.plus(duration))
    .notBefore(now)
    .issuedAt(now);
}

I'm not sure that name is clear, or even if this idea would provide enough value to folks.

(This doesn't need to be in this PR, or implemented at all, just adding the thought to the discussion)

[pveeckhout]

Another potential addition related to using the newer dates api would be to add a convenance method to set the nbf, exp, and iat together, something like:

JwtBuilder validFor(Duration duration) {
  Instant now = Instant.now(clock);
  return this.expiration(now.plus(duration))
    .notBefore(now)
    .issuedAt(now);
}

I'm not sure that name is clear, or even if this idea would provide enough value to folks.

(This doesn't need to be in this PR, or implemented at all, just adding the thought to the discussion)

fair point, these might be added in another PR that also add some convenience methods, like this one: #883

[tayfun94]

Will someone validate and merge this PR in the future? SonarQube cries everytime I open the JwtUtil class regarding java.util.Date :(