Skip to content

Commit

Permalink
Increase default OIDC session-age-extension to minimize chances of dr…
Browse files Browse the repository at this point in the history
…opping expired ID tokens
  • Loading branch information
sberyozkin committed Dec 17, 2024
1 parent cfa2677 commit 46adbb2
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 33 deletions.
26 changes: 24 additions & 2 deletions docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1316,14 +1316,36 @@ If you work with a Quarkus OIDC `web-app` application, then the Quarkus OIDC cod
To use the refresh token, you should carefully configure the session cookie age.
The session age should be longer than the ID token lifespan and close to or equal to the refresh token lifespan.

You calculate the session age by adding the lifespan value of the current ID token and the values of the `quarkus.oidc.authentication.session-age-extension` and `quarkus.oidc.token.lifespan-grace` properties.
You calculate the session cookie age by adding the lifespan value of the current ID token and the values of the `quarkus.oidc.authentication.session-age-extension` and `quarkus.oidc.token.lifespan-grace` properties.

[TIP]
====
You use only the `quarkus.oidc.authentication.session-age-extension` property to significantly extend the session lifespan, if required.
You use the `quarkus.oidc.token.lifespan-grace` property only to consider some small clock skews.
====

[TIP]
====
Use the quarkus.oidc.authentication.session-age-extension property to extend the session lifespan and ensure the application can access expired ID tokens when needed.
By default, the session cookie's max-age property is set to the value of the ID token life span.
After the session cookie expires and the user attempts to access the application again, the authenticated user is redirected to the OIDC provider to re-authenticate.
However, if `quarkus.oidc.authentication.session-age-extension` is set to a reasonable idle-time duration of several hours, longer than the life-span of an ID token, the expired ID token can still be made available to the application.
This allows the application to refresh the token or redirect the user to a session expired page, as the session cookie storing the ID token remains available during the current authenticated user's request.
For example, expect a typical user activity to last 30 minutes.
Therefore, you have configured the OIDC provider to issue ID tokens, which are valid for 30 minutes only.
However, you would also like to refresh a user's ID token and renew the session after this user returns from a one-hour break.
In this case, set this property to 90 minutes; otherwise, Quarkus will not be able to detect a session cookie, and the user will be required to re-authenticate.
The important point is that extending the session cookie's age with `quarkus.oidc.authentication.session-age-extension` maximizes the chances of making an expired ID token available to the application.
This token, associated with the cookie, can then be used for further processing.
It improves a user experience; otherwise, an authenticated user may be surprised to see an immediate re-authentication challenge if the ID token and the session cookie that holds it have expired.
By default, this property will be set to 1 hour if either `quarkus.oidc.token.refresh-expired` or `quarkus.oidc.authentication.session-expired-page` features are enabled since they can only be effective if an expired ID token is available.
====

When the current authenticated user returns to the protected Quarkus endpoint and the ID token associated with the session cookie has expired, then, by default, the user is automatically redirected to the OIDC Authorization endpoint to re-authenticate.
The OIDC provider might challenge the user again if the session between the user and this OIDC provider is still active, which might happen if the session is configured to last longer than the ID token.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ public Optional<Boolean> userInfoRequired() {
}

@Override
public Duration sessionAgeExtension() {
public Optional<Duration> sessionAgeExtension() {
return sessionAgeExtension;
}

Expand Down Expand Up @@ -1698,14 +1698,30 @@ public enum ResponseMode {
public Optional<Boolean> userInfoRequired = Optional.empty();

/**
* Session age extension in minutes.
* The user session age property is set to the value of the ID token life-span by default and
* the user is redirected to the OIDC provider to re-authenticate once the session has expired.
* If this property is set to a nonzero value, then the expired ID token can be refreshed before
* the session has expired.
* This property is ignored if the `token.refresh-expired` property has not been enabled.
* Session cookie max-age extension.
*
* Session cookie max-age property is set to the value of the ID token life-span by default
* and the authenticated user is redirected to the OIDC provider to re-authenticate, after the session cookie has
* expired and the user has attempted to access the application again.
*
* However, if this property is set to a reasonable idle-time duration of several hours,
* longer than an ID token life-span, then, even if the ID token has expired,
* it can still be made available to the application which may refresh it or redirect the user
* to a session expired page, because the session cookie which keeps ID token is still available
* during the current authenticated user's request.
*
* An important point is that extending the session cookie's age with this property
* is only about maximizing the chances of making even an expired ID token associated
* with this cookie available to the application for further processing.
* It improves a user experience, otherwise, an authenticated user may be surprised by seeing an immediate
* re-authentication challenge if the ID token has expired together with the session cookie which
* holds it.
*
* By default, this property will be set to 1 hour if either 'quarkus.oidc.token.refresh-expired'
* or 'quarkus.oidc.authentication.session-expired-page' features are enabled since they can only
* be effective if an expired ID token is available.
*/
public Duration sessionAgeExtension = Duration.ofMinutes(5);
public Optional<Duration> sessionAgeExtension = Optional.empty();

/**
* State cookie age in minutes.
Expand Down Expand Up @@ -1915,12 +1931,12 @@ public void setVerifyAccessToken(boolean verifyAccessToken) {
this.verifyAccessToken = verifyAccessToken;
}

public Duration getSessionAgeExtension() {
public Optional<Duration> getSessionAgeExtension() {
return sessionAgeExtension;
}

public void setSessionAgeExtension(Duration sessionAgeExtension) {
this.sessionAgeExtension = sessionAgeExtension;
this.sessionAgeExtension = Optional.of(sessionAgeExtension);
}

public Optional<String> getCookiePathHeader() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey;
import java.security.SecureRandom;
import java.time.Duration;
import java.util.Base64;
import java.util.Base64.Encoder;
import java.util.List;
Expand Down Expand Up @@ -1060,9 +1061,17 @@ public Uni<? extends Void> apply(Void t) {
if (configContext.oidcConfig().token().lifespanGrace().isPresent()) {
maxAge += configContext.oidcConfig().token().lifespanGrace().getAsInt();
}
if (configContext.oidcConfig().token().refreshExpired() && tokens.getRefreshToken() != null) {
maxAge += configContext.oidcConfig().authentication().sessionAgeExtension().getSeconds();

Optional<Duration> sessionAgeExtProp = configContext.oidcConfig().authentication()
.sessionAgeExtension();
if (sessionAgeExtProp.isPresent()) {
maxAge += sessionAgeExtProp.get().getSeconds();
} else if ((configContext.oidcConfig().token().refreshExpired() && tokens.getRefreshToken() != null)
|| configContext.oidcConfig().authentication().sessionExpiredPath().isPresent()) {
// Set it to 1 hour (60 x 60) since features expecting an expired ID token are enabled
maxAge += 3600;
}

final long sessionMaxAge = maxAge;
context.put(SESSION_MAX_AGE_PARAM, maxAge);
context.put(TenantConfigContext.class.getName(), configContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -771,15 +771,30 @@ enum ResponseMode {
Optional<Boolean> userInfoRequired();

/**
* Session age extension in minutes.
* The user session age property is set to the value of the ID token life-span by default and
* the user is redirected to the OIDC provider to re-authenticate once the session has expired.
* If this property is set to a nonzero value, then the expired ID token can be refreshed before
* the session has expired.
* This property is ignored if the `token.refresh-expired` property has not been enabled.
* Session cookie max-age extension.
*
* Session cookie max-age property is set to the value of the ID token life-span by default
* and the authenticated user is redirected to the OIDC provider to re-authenticate, after the session cookie has
* expired and the user has attempted to access the application again.
*
* However, if this property is set to a reasonable idle-time duration of several hours,
* longer than an ID token life-span, then, even if the ID token has expired,
* it can still be made available to the application which may refresh it or redirect the user
* to a session expired page, because the session cookie which keeps ID token is still available
* during the current authenticated user's request.
*
* An important point is that extending the session cookie's age with this property
* is only about maximizing the chances of making even an expired ID token associated
* with this cookie available to the application for further processing.
* It improves a user experience, otherwise, an authenticated user may be surprised by seeing an immediate
* re-authentication challenge if the ID token has expired together with the session cookie which
* holds it.
*
* By default, this property will be set to 1 hour if either 'quarkus.oidc.token.refresh-expired'
* or 'quarkus.oidc.authentication.session-expired-page' features are enabled since they can only
* be effective if an expired ID token is available.
*/
@WithDefault("5M")
Duration sessionAgeExtension();
Optional<Duration> sessionAgeExtension();

/**
* State cookie age in minutes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ private record AuthenticationImpl(Optional<ResponseMode> responseMode, Optional<
Optional<Boolean> addOpenidScope, Map<String, String> extraParams, Optional<List<String>> forwardParams,
boolean cookieForceSecure, Optional<String> cookieSuffix, String cookiePath, Optional<String> cookiePathHeader,
Optional<String> cookieDomain, CookieSameSite cookieSameSite, boolean allowMultipleCodeFlows,
boolean failOnMissingStateParam, Optional<Boolean> userInfoRequired, Duration sessionAgeExtension,
boolean failOnMissingStateParam, Optional<Boolean> userInfoRequired, Optional<Duration> sessionAgeExtension,
Duration stateCookieAge, boolean javaScriptAutoRedirect, Optional<Boolean> idTokenRequired,
Optional<Duration> internalIdTokenLifespan, Optional<Boolean> pkceRequired, Optional<String> pkceSecret,
Optional<String> stateSecret) implements Authentication {
Expand Down Expand Up @@ -56,7 +56,7 @@ private record AuthenticationImpl(Optional<ResponseMode> responseMode, Optional<
private boolean allowMultipleCodeFlows;
private boolean failOnMissingStateParam;
private Optional<Boolean> userInfoRequired;
private Duration sessionAgeExtension;
private Optional<Duration> sessionAgeExtension;
private Duration stateCookieAge;
private boolean javaScriptAutoRedirect;
private Optional<Boolean> idTokenRequired;
Expand Down Expand Up @@ -428,7 +428,7 @@ public AuthenticationConfigBuilder userInfoRequired(boolean userInfoRequired) {
* @return this builder
*/
public AuthenticationConfigBuilder sessionAgeExtension(Duration sessionAgeExtension) {
this.sessionAgeExtension = Objects.requireNonNull(sessionAgeExtension);
this.sessionAgeExtension = Optional.of(sessionAgeExtension);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public void testDefaultValues() {
assertTrue(authentication.allowMultipleCodeFlows());
assertFalse(authentication.failOnMissingStateParam());
assertTrue(authentication.userInfoRequired().isEmpty());
assertEquals(5, authentication.sessionAgeExtension().toMinutes());
assertEquals(5, authentication.sessionAgeExtension().orElseThrow().toMinutes());

Check failure on line 155 in extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 46adbb2d973c7d2a8cd434d042f13c657ad035a4

JVM Tests - JDK 17

java.util.NoSuchElementException: No value present at java.base/java.util.Optional.orElseThrow(Optional.java:377) at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
Raw output
java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.orElseThrow(Optional.java:377)
	at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 155 in extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 46adbb2d973c7d2a8cd434d042f13c657ad035a4

JVM Tests - JDK 17 Windows

java.util.NoSuchElementException: No value present at java.base/java.util.Optional.orElseThrow(Optional.java:377) at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
Raw output
java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.orElseThrow(Optional.java:377)
	at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 155 in extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 46adbb2d973c7d2a8cd434d042f13c657ad035a4

JVM Tests - JDK 21

java.util.NoSuchElementException: No value present at java.base/java.util.Optional.orElseThrow(Optional.java:377) at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
Raw output
java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.orElseThrow(Optional.java:377)
	at io.quarkus.oidc.runtime.OidcTenantConfigBuilderTest.testDefaultValues(OidcTenantConfigBuilderTest.java:155)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
assertEquals(5, authentication.stateCookieAge().toMinutes());
assertTrue(authentication.javaScriptAutoRedirect());
assertTrue(authentication.idTokenRequired().isEmpty());
Expand Down Expand Up @@ -549,7 +549,7 @@ public void testSetEveryProperty() {
assertFalse(authentication.allowMultipleCodeFlows());
assertTrue(authentication.failOnMissingStateParam());
assertTrue(authentication.userInfoRequired().orElseThrow());
assertEquals(77, authentication.sessionAgeExtension().toMinutes());
assertEquals(77, authentication.sessionAgeExtension().orElseThrow().toMinutes());
assertEquals(88, authentication.stateCookieAge().toMinutes());
assertFalse(authentication.javaScriptAutoRedirect());
assertFalse(authentication.idTokenRequired().orElseThrow());
Expand Down Expand Up @@ -1285,7 +1285,7 @@ public void testCopyOfAuthenticationConfigBuilder() {
assertFalse(builtFirst.allowMultipleCodeFlows());
assertTrue(builtFirst.failOnMissingStateParam());
assertTrue(builtFirst.userInfoRequired().orElseThrow());
assertEquals(77, builtFirst.sessionAgeExtension().toMinutes());
assertEquals(77, builtFirst.sessionAgeExtension().orElseThrow().toMinutes());
assertEquals(88, builtFirst.stateCookieAge().toMinutes());
assertFalse(builtFirst.javaScriptAutoRedirect());
assertFalse(builtFirst.idTokenRequired().orElseThrow());
Expand Down Expand Up @@ -1335,7 +1335,7 @@ public void testCopyOfAuthenticationConfigBuilder() {
assertFalse(builtSecond.allowMultipleCodeFlows());
assertTrue(builtSecond.failOnMissingStateParam());
assertTrue(builtSecond.userInfoRequired().orElseThrow());
assertEquals(77, builtSecond.sessionAgeExtension().toMinutes());
assertEquals(77, builtSecond.sessionAgeExtension().orElseThrow().toMinutes());
assertEquals(88, builtSecond.stateCookieAge().toMinutes());
assertFalse(builtSecond.javaScriptAutoRedirect());
assertFalse(builtSecond.idTokenRequired().orElseThrow());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ public Optional<Boolean> userInfoRequired() {
}

@Override
public Duration sessionAgeExtension() {
public Optional<Duration> sessionAgeExtension() {
invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_SESSION_AGE_EXTENSION, true);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,10 @@ public void testCodeFlowUserInfoCachedInIdToken() throws Exception {
Cookie sessionCookie = getSessionCookie(webClient, "code-flow-user-info-github-cached-in-idtoken");
Date date = sessionCookie.getExpires();
assertTrue(date.toInstant().getEpochSecond() - issuedAt >= 299 + 300);
// This test enables the token refresh, in this case the cookie age is extended by additional 5 mins
// This test enables the token refresh, in this case the cookie age is extended by additional 1 hour
// to minimize the risk of the browser losing immediately after it has expired, for this cookie
// be returned to Quarkus, analyzed and refreshed
assertTrue(date.toInstant().getEpochSecond() - issuedAt <= 299 + 300 + 3);
assertTrue(date.toInstant().getEpochSecond() - issuedAt - (60 * 60) <= 299 + 300 + 3);

// This is the initial call to the token endpoint where the code was exchanged for tokens
wireMockServer.verify(1,
Expand All @@ -397,7 +397,7 @@ public void testCodeFlowUserInfoCachedInIdToken() throws Exception {
sessionCookie = getSessionCookie(webClient, "code-flow-user-info-github-cached-in-idtoken");
date = sessionCookie.getExpires();
assertTrue(date.toInstant().getEpochSecond() - issuedAt >= 305 + 300);
assertTrue(date.toInstant().getEpochSecond() - issuedAt <= 305 + 300 + 3);
assertTrue(date.toInstant().getEpochSecond() - issuedAt - (60 * 60 * 5) <= 305 + 300 + 3);

// access token must've been refreshed
wireMockServer.verify(1,
Expand Down

0 comments on commit 46adbb2

Please sign in to comment.