From 46adbb2d973c7d2a8cd434d042f13c657ad035a4 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Sun, 7 Jul 2024 19:19:12 +0100 Subject: [PATCH] Increase default OIDC session-age-extension to minimize chances of dropping expired ID tokens --- ...ecurity-oidc-code-flow-authentication.adoc | 26 ++++++++++++-- .../io/quarkus/oidc/OidcTenantConfig.java | 36 +++++++++++++------ .../runtime/CodeAuthenticationMechanism.java | 13 +++++-- .../oidc/runtime/OidcTenantConfig.java | 31 +++++++++++----- .../builders/AuthenticationConfigBuilder.java | 6 ++-- .../runtime/OidcTenantConfigBuilderTest.java | 8 ++--- .../oidc/runtime/OidcTenantConfigImpl.java | 2 +- .../keycloak/CodeFlowAuthorizationTest.java | 6 ++-- 8 files changed, 95 insertions(+), 33 deletions(-) diff --git a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc index 7c656133d0329e..6e37d24451927a 100644 --- a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc +++ b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc @@ -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. diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java index 2111b756bb59eb..7822c5edd7a5e6 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java @@ -1426,7 +1426,7 @@ public Optional userInfoRequired() { } @Override - public Duration sessionAgeExtension() { + public Optional sessionAgeExtension() { return sessionAgeExtension; } @@ -1698,14 +1698,30 @@ public enum ResponseMode { public Optional 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 sessionAgeExtension = Optional.empty(); /** * State cookie age in minutes. @@ -1915,12 +1931,12 @@ public void setVerifyAccessToken(boolean verifyAccessToken) { this.verifyAccessToken = verifyAccessToken; } - public Duration getSessionAgeExtension() { + public Optional getSessionAgeExtension() { return sessionAgeExtension; } public void setSessionAgeExtension(Duration sessionAgeExtension) { - this.sessionAgeExtension = sessionAgeExtension; + this.sessionAgeExtension = Optional.of(sessionAgeExtension); } public Optional getCookiePathHeader() { diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java index 32491900bcb368..30316ad9029f4d 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java @@ -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; @@ -1060,9 +1061,17 @@ public Uni 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 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); diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java index f3c9c1463458c3..7c642276a7430a 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTenantConfig.java @@ -771,15 +771,30 @@ enum ResponseMode { Optional 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 sessionAgeExtension(); /** * State cookie age in minutes. diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java index 09c38218e2505c..ef7aa9864a2c76 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java @@ -26,7 +26,7 @@ private record AuthenticationImpl(Optional responseMode, Optional< Optional addOpenidScope, Map extraParams, Optional> forwardParams, boolean cookieForceSecure, Optional cookieSuffix, String cookiePath, Optional cookiePathHeader, Optional cookieDomain, CookieSameSite cookieSameSite, boolean allowMultipleCodeFlows, - boolean failOnMissingStateParam, Optional userInfoRequired, Duration sessionAgeExtension, + boolean failOnMissingStateParam, Optional userInfoRequired, Optional sessionAgeExtension, Duration stateCookieAge, boolean javaScriptAutoRedirect, Optional idTokenRequired, Optional internalIdTokenLifespan, Optional pkceRequired, Optional pkceSecret, Optional stateSecret) implements Authentication { @@ -56,7 +56,7 @@ private record AuthenticationImpl(Optional responseMode, Optional< private boolean allowMultipleCodeFlows; private boolean failOnMissingStateParam; private Optional userInfoRequired; - private Duration sessionAgeExtension; + private Optional sessionAgeExtension; private Duration stateCookieAge; private boolean javaScriptAutoRedirect; private Optional idTokenRequired; @@ -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; } diff --git a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java index 50fa9f66815ebd..3cfff7ff974c08 100644 --- a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java +++ b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigBuilderTest.java @@ -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()); assertEquals(5, authentication.stateCookieAge().toMinutes()); assertTrue(authentication.javaScriptAutoRedirect()); assertTrue(authentication.idTokenRequired().isEmpty()); @@ -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()); @@ -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()); @@ -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()); diff --git a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java index 73bc8ddf0671bb..52157c205db75f 100644 --- a/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java +++ b/extensions/oidc/runtime/src/test/java/io/quarkus/oidc/runtime/OidcTenantConfigImpl.java @@ -699,7 +699,7 @@ public Optional userInfoRequired() { } @Override - public Duration sessionAgeExtension() { + public Optional sessionAgeExtension() { invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_SESSION_AGE_EXTENSION, true); return null; } diff --git a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java index ed6fce21ac10ab..70b688ec1df4af 100644 --- a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java +++ b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java @@ -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, @@ -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,