quarkus
quarkus copied to clipboard
io.quarkus.oidc.OidcSession uses Instant to present duration
Describe the bug
In, io.quarkus.oidc.OidcSession (https://github.com/quarkusio/quarkus/blob/main/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcSession.java), it uses Instant to present duration.
/**
* Return an {@linkplain:Instant} indicating how long will it take for the current session to expire.
*
* @return
*/
Instant expiresIn();
Based on Javadoc, it is an instantaneous point on the time-line. But the comment says "how long", it should be Duration.
When using it in app, it seems that the value is filled with Duration too.
e.g. One value I got is "expiresIn": "1970-01-01T00:23:05Z". I guess it means the session has 23 hours and 5 minutes to expire.
Personally, I prefer to use Instant but it should assigned the correct time value (a duration value is a little hard to predict the ending time without base time) and the comment should be updated.
Expected behavior
Using Instant but it should assigned the correct time value (a duration value is a little hard to predict the ending time without base time) and the comment should be updated.
Actual behavior
It uses Instant with the wrong value and comment is misleading.
How to Reproduce?
Just any endpoint with injection of OidcSession.
Output of uname -a or ver
Darwin Kernel Version 21.4.0
Output of java -version
openjdk version "18" 2022-03-2
GraalVM version (if different from Java)
No response
Quarkus version or git rev
2.8.2.Final
Build tool (ie. output of mvnw --version or gradlew --version)
Gradle 7.4.1
Additional information
No response
/cc @pedroigor, @sberyozkin
I agree, this does not look correct.
However OidcSession is part of the public API, so we can just change it. We likely need to introduce a new method and deprecate the existing one.
ID token's expiry is a number of seconds since the epoch, see https://openid.net/specs/openid-connect-core-1_0.html#IDToken. So this method says in how many seconds from the current time the session will expire, it is not meant to return the whole duration during which the token can be valid for, this instant value will be different every time the already authenticated user returns. The Java docs should indeed clarify it.
That expiry date shown in the description looks wrong to me. Perhaps we can add a new method returning a precise Date when the session will expire
After checking the implementation (https://github.com/quarkusio/quarkus/blob/main/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcSessionImpl.java)
@Override
public Instant expiresIn() {
final long nowSecs = System.currentTimeMillis() / 1000;
return Instant.ofEpochSecond(idToken.getExpirationTime() - nowSecs);
}
It returns actually a duration (from the meaning, not Java class) when the method is triggered.
Hmm. It does look wrong to me now, as indeed it returns a duration from the epoch, it was really meant to be idToken.getExpirationTime() - nowSecs only. What this method returns now is indeed of no much use, the idea was to give user a hint when, starting from now, the session will expire, in 30 mins or in 2 hours etc.
I'll fix it as proposed by Georgios
Actually, I'll keep this method undeprecated and just return an Instant representing the expiration time found in Id Token - which is exactly what Instant.ofEpochSecond is for, return Instant.ofEpochSecond(idToken.getExpirationTime()).
But indeed, will also add a method returning Duration capturing an idToken.getExpirationTime() - nowSecs interval