search

italia / spid-cie-oidc-java

The SPID/CIE OIDC Federation Relying Party, written in Java
Apache License 2.0
21 stars 4 forks source link

JWTHelper#getIssuedAt possibly wrong #34

Closed giacgbj closed 1 month ago

giacgbj commented 1 month ago

The code at https://github.com/italia/spid-cie-oidc-java/blob/1e5c09521fda4a1c2f703d7bead13bde5d480e48/starter-kit/src/main/java/it/spid/cie/oidc/helper/JWTHelper.java#L168 seems to work only when run in the UTC time zone.

Shouldn't it be changed to the following?

return Instant.now().getEpochSecond();
cretara commented 1 month ago

Another possibility is to write 

    ZoneOffset zoneOffsetSystemDefault = ZoneOffset.systemDefault().getRules().getStandardOffset(Instant.now());
    long epochSecond = LocalDateTime.now().toEpochSecond(zoneOffsetSystemDefault);

in order to have epochSecond indipendent from TimeZone in which application is running

mmariuzzo commented 1 month ago

IMHO applications should always use -Duser.timezone=GMT to simplify date conversion and avoid summer/winter time change.

For example, it is a best practices for Authentication Servers to use GMT/UTC timezone.

Any contribution is welcome. Maybe we can use

ZonedDateTime.getInstant().now().getEpochSecond();
cretara commented 1 month ago

IMHO applications should always use -Duser.timezone=GMT to simplify date conversion and avoid summer/winter time change.

For example, it is a best practices for Authentication Servers to use GMT/UTC timezone.

Any contribution is welcome. Maybe we can use

ZonedDateTime.getInstant().now().getEpochSecond();

This wouldn't be platform agnostic solution, since you are forcing timezone by setting it in JVM properly. In case of this application runs in cloud in USA, for example: I sueggest to use code agnostic for JWT creation timestamp that would be indipendent by particular solution chosen

mmariuzzo commented 1 month ago

I'm using cloud services hosted around the world. One thing is the backend time (usually GMT), another is the frontend time: which is translated using the logged user timezone or other logic. In Italy I've seen more applications attested on CET/CEST only to have italian time on application logs :-(

Apart this, @cretara you hit the spot: this is a Kit and could not enforce something.

AFAIK ZonedDateTime is a good solution because it creates an Instant attested on JVM runtime timezone. With getInstan().now().getEpochSeconds() we have corrisponding seconds on GMT timezone.

This project is opensource and contributions are welcomed. Please provide a PR and me or @rglauco will analyze and merge