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 d32ed85645deb..242cd31f657ff 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 @@ -1420,6 +1420,11 @@ public boolean failOnMissingStateParam() { return failOnMissingStateParam; } + @Override + public boolean failOnUnresolvedKid() { + return failOnUnresolvedKid; + } + @Override public Optional userInfoRequired() { return userInfoRequired; @@ -1684,6 +1689,22 @@ public enum ResponseMode { */ public boolean failOnMissingStateParam = false; + /** + * Fail with the HTTP 401 error if the ID token signature can not be verified during the re-authentication only due to + * an unresolved token key identifier (`kid`). + *

+ * This property might need to be disabled when multiple tab authentications are allowed, with one of the tabs keeping + * an expired ID token with its `kid` + * unresolved due to the verification key set refreshed due to another tab initiating an authorization code flow. In + * such cases, instead of failing with the HTTP 401 error, + * redirecting the user to re-authenticate with the HTTP 302 status may provide better user experience. + *

+ * Note that the HTTP 401 error is always returned if the ID token signature can not be verified due to an unresolved + * kid during an initial ID token verification + * following the authorization code flow completion, before a session cookie is created. + */ + public boolean failOnUnresolvedKid = true; + /** * If this property is set to `true`, an OIDC UserInfo endpoint is called. *

@@ -2042,6 +2063,7 @@ private void addConfigMappingValues(io.quarkus.oidc.runtime.OidcTenantConfig.Aut cookieSameSite = CookieSameSite.valueOf(mapping.cookieSameSite().toString()); allowMultipleCodeFlows = mapping.allowMultipleCodeFlows(); failOnMissingStateParam = mapping.failOnMissingStateParam(); + failOnUnresolvedKid = mapping.failOnUnresolvedKid(); userInfoRequired = mapping.userInfoRequired(); sessionAgeExtension = mapping.sessionAgeExtension(); stateCookieAge = mapping.stateCookieAge(); 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 32491900bcb36..755ca07d481ad 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 @@ -23,6 +23,7 @@ import org.jose4j.jwt.consumer.ErrorCodes; import org.jose4j.jwt.consumer.InvalidJwtException; import org.jose4j.lang.JoseException; +import org.jose4j.lang.UnresolvableKeyException; import io.netty.handler.codec.http.HttpResponseStatus; import io.quarkus.logging.Log; @@ -375,14 +376,31 @@ public Uni apply(Throwable t) { .hasErrorCode(ErrorCodes.EXPIRED); if (!expired) { - String error = logAuthenticationError(context, t); + + Throwable failure = null; + + boolean unresolvedKey = t.getCause() instanceof InvalidJwtException + && (t.getCause().getCause() instanceof UnresolvableKeyException); + if (unresolvedKey + && !configContext.oidcConfig().authentication().failOnUnresolvedKid() + && OidcUtils.isJwtTokenExpired(currentIdToken)) { + // It can happen in multi-tab applications where a user login causes a JWK set refresh + // due to the key rotation, discarding old keys, and the old tab still keeps the session + // whose signature can only be verified with the now discarded key. + LOG.debugf( + "Session can not be verified due to an unresolved key exception, reauthentication is required"); + // Redirect the user to the OIDC provider to re-authenticate + failure = new AuthenticationFailedException(); + } else { + // Failures such as the signature verification failures require 401 status + String error = logAuthenticationError(context, t); + failure = t.getCause() instanceof AuthenticationCompletionException + ? t.getCause() + : new AuthenticationCompletionException(error, t.getCause()); + } + return removeSessionCookie(context, configContext.oidcConfig()) - .replaceWith(Uni.createFrom() - .failure(t - .getCause() instanceof AuthenticationCompletionException - ? t.getCause() - : new AuthenticationCompletionException( - error, t.getCause()))); + .replaceWith(Uni.createFrom().failure(failure)); } // Token has expired, try to refresh if (isRpInitiatedLogout(context, 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 c3b589941c6d3..2b66cccc4633a 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 @@ -756,6 +756,19 @@ enum ResponseMode { @WithDefault("false") boolean failOnMissingStateParam(); + /** + * Fail with the HTTP 401 error if the ID token signature can not be verified during the re-authentication only due to + * an unresolved token key identifier (`kid`). + *

+ * This property might need to be disabled when multiple tab authentications are allowed, with one of the tabs keeping + * an expired ID token with its `kid` + * unresolved due to the verification key set refreshed due to another tab initiating an authorization code flow. In + * such cases, instead of failing with the HTTP 401 error, + * redirecting the user to re-authenticate with the HTTP 302 status may provide better user experience. + */ + @WithDefault("true") + boolean failOnUnresolvedKid(); + /** * If this property is set to `true`, an OIDC UserInfo endpoint is called. *

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 09c38218e2505..e037256ee7921 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,8 @@ 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, boolean failOnUnresolvedKid, Optional userInfoRequired, + Duration sessionAgeExtension, Duration stateCookieAge, boolean javaScriptAutoRedirect, Optional idTokenRequired, Optional internalIdTokenLifespan, Optional pkceRequired, Optional pkceSecret, Optional stateSecret) implements Authentication { @@ -55,6 +56,7 @@ private record AuthenticationImpl(Optional responseMode, Optional< private CookieSameSite cookieSameSite; private boolean allowMultipleCodeFlows; private boolean failOnMissingStateParam; + private boolean failOnUnresolvedKid; private Optional userInfoRequired; private Duration sessionAgeExtension; private Duration stateCookieAge; @@ -98,6 +100,7 @@ public AuthenticationConfigBuilder(OidcTenantConfigBuilder builder) { this.cookieSameSite = authentication.cookieSameSite(); this.allowMultipleCodeFlows = authentication.allowMultipleCodeFlows(); this.failOnMissingStateParam = authentication.failOnMissingStateParam(); + this.failOnUnresolvedKid = authentication.failOnUnresolvedKid(); this.userInfoRequired = authentication.userInfoRequired(); this.sessionAgeExtension = authentication.sessionAgeExtension(); this.stateCookieAge = authentication.stateCookieAge(); @@ -405,6 +408,24 @@ public AuthenticationConfigBuilder failOnMissingStateParam(boolean failOnMissing return this; } + /** + * Sets {@link Authentication#failOnUnreslvedKid()} to true. + * + * @return this builder + */ + public AuthenticationConfigBuilder failOnUnresolvedKid() { + return failOnUnresolvedKid(true); + } + + /** + * @param failOnUnresolvedKid {@link Authentication#failOnUnreslvedKid()} + * @return this builder + */ + public AuthenticationConfigBuilder failOnUnresolvedKid(boolean failOnUnresolvedKid) { + this.failOnUnresolvedKid = failOnUnresolvedKid; + return this; + } + /** * Sets {@link Authentication#userInfoRequired()} to true. * @@ -554,6 +575,7 @@ public Authentication build() { sessionExpiredPath, verifyAccessToken, forceRedirectHttpsScheme, optionalScopes, scopeSeparator, nonceRequired, addOpenidScope, Map.copyOf(extraParams), optionalForwardParams, cookieForceSecure, cookieSuffix, cookiePath, cookiePathHeader, cookieDomain, cookieSameSite, allowMultipleCodeFlows, failOnMissingStateParam, + failOnUnresolvedKid, userInfoRequired, sessionAgeExtension, stateCookieAge, javaScriptAutoRedirect, idTokenRequired, internalIdTokenLifespan, pkceRequired, pkceSecret, stateSecret); } 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 901016e517153..fbdeaeca98292 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 @@ -142,6 +142,7 @@ enum ConfigMappingMethods { AUTHENTICATION_COOKIE_SAME_SITE, AUTHENTICATION_ALLOW_MULTIPLE_CODE_FLOWS, AUTHENTICATION_FAIL_ON_MISSING_STATE_PARAM, + AUTHENTICATION_FAIL_ON_UNRESOLVED_KID, AUTHENTICATION_USER_INFO_REQUIRED, AUTHENTICATION_SESSION_AGE_EXTENSION, AUTHENTICATION_STATE_COOKIE_AGE, @@ -706,6 +707,12 @@ public boolean failOnMissingStateParam() { return false; } + @Override + public boolean failOnUnresolvedKid() { + invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_FAIL_ON_UNRESOLVED_KID, true); + return false; + } + @Override public Optional userInfoRequired() { invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_USER_INFO_REQUIRED, true); diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties index 273e767de6583..4f9a40246930f 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -1,6 +1,6 @@ quarkus.keycloak.devservices.create-realm=false quarkus.keycloak.devservices.show-logs=true -# Default tenant configurationf +# Default tenant configuration quarkus.oidc.client-id=quarkus-app quarkus.oidc.credentials.secret=secret quarkus.oidc.authentication.scopes=profile,email @@ -10,9 +10,11 @@ quarkus.oidc.authentication.cookie-path-header=some-header quarkus.oidc.authentication.cookie-domain=localhost quarkus.oidc.authentication.extra-params.max-age=60 quarkus.oidc.authentication.extra-params.scope=phone +quarkus.oidc.authentication.fail-on-unresolved-kid=false quarkus.oidc.application-type=web-app quarkus.oidc.authentication.cookie-suffix=test quarkus.oidc.token-state-manager.encryption-required=false +quarkus.oidc.token.allow-jwt-introspection=false # OIDC client configuration quarkus.oidc-client.auth-server-url=${quarkus.oidc.auth-server-url} @@ -127,6 +129,9 @@ quarkus.oidc.tenant-nonce.authentication.redirect-path=/tenant-nonce quarkus.oidc.tenant-nonce.application-type=web-app quarkus.oidc.tenant-nonce.authentication.nonce-required=true quarkus.oidc.tenant-nonce.authentication.state-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU +quarkus.oidc.tenant-nonce.token-state-manager.encryption-required=false +quarkus.oidc.tenant-nonce.token.allow-jwt-introspection=false + quarkus.oidc.tenant-javascript.auth-server-url=${quarkus.oidc.auth-server-url} quarkus.oidc.tenant-javascript.client-id=quarkus-app diff --git a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java index d536406b57c5c..32cd323b16251 100644 --- a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java +++ b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java @@ -13,7 +13,9 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.time.Instant; import java.util.Base64; +import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -40,6 +42,8 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.keycloak.client.KeycloakTestClient; import io.restassured.RestAssured; +import io.smallrye.jwt.build.Jwt; +import io.smallrye.jwt.util.KeyUtils; import io.vertx.core.json.JsonObject; /** @@ -102,6 +106,39 @@ public void testCodeFlowNoConsent() throws IOException { assertNotNull(sessionCookie); assertEquals("lax", sessionCookie.getSameSite()); + // try again with the valid session cookie but with RestAssured + RestAssured.given().redirects().follow(false) + .header("Cookie", sessionCookie.getName() + "=" + sessionCookie.getValue()).when() + .get("/web-app/configMetadataScopes") + .then().statusCode(200); + + SecretKey secretKey = KeyUtils.createSecretKeyFromSecret( + "AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow"); + + // Generate an already expired token with some random key id + String expiredTokenWithRandomKid = Jwt.claims() + .issuedAt(Instant.now().minusSeconds(100)) + .expiresAt(Instant.now().minusSeconds(50)) + .jws().keyId(UUID.randomUUID().toString()).sign(secretKey); + String sessionCookie2 = expiredTokenWithRandomKid + "|" + expiredTokenWithRandomKid + "||" + + expiredTokenWithRandomKid; + // Redirect to re-authenticate is expected + RestAssured.given().redirects().follow(false).header("Cookie", "q_session_Default_test=" + sessionCookie2) + .when() + .get("/web-app/configMetadataScopes") + .then().statusCode(302); + + // Generate a valid token with some random key id + String tokenWithRandomKid = Jwt.claims() + .issuedAt(Instant.now()) + .jws().keyId(UUID.randomUUID().toString()).sign(secretKey); + String sessionCookie3 = tokenWithRandomKid + "|" + tokenWithRandomKid + "||" + tokenWithRandomKid; + // 401 is expected + RestAssured.given().redirects().follow(false).header("Cookie", "q_session_Default_test=" + sessionCookie3) + .when() + .get("http://localhost:8081/web-app/configMetadataScopes") + .then().statusCode(401); + webClient.getCookieManager().clearCookies(); } } @@ -447,6 +484,28 @@ private void doTestCodeFlowNonce(boolean wrongRedirect) throws Exception { assertNotNull(sessionCookie); assertEquals("q_session_tenant-nonce", sessionCookie.getName()); + // try again with the valid session cookie but with RestAssured + RestAssured.given().redirects().follow(false) + .header("Cookie", sessionCookie.getName() + "=" + sessionCookie.getValue()).when() + .get("/tenant-nonce") + .then().statusCode(200); + + SecretKey secretKey = KeyUtils.createSecretKeyFromSecret( + "AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow"); + + // Generate an already expired token with some random key id + String expiredTokenWithRandomKid = Jwt.claims() + .issuedAt(Instant.now().minusSeconds(100)) + .expiresAt(Instant.now().minusSeconds(50)) + .jws().keyId(UUID.randomUUID().toString()).sign(secretKey); + String sessionCookie2 = expiredTokenWithRandomKid + "|" + expiredTokenWithRandomKid + "||" + + expiredTokenWithRandomKid; + // 401 is expected because the redirect to re-authenticate is not allowed by default when the key id can not be resolved + RestAssured.given().redirects().follow(false).header("Cookie", "q_session_tenant-nonce=" + sessionCookie2) + .when() + .get("/tenant-nonce") + .then().statusCode(401); + String endpointLocationWithoutQuery = webResponse.getResponseHeaderValue("location"); URI endpointLocationWithoutQueryUri = URI.create(endpointLocationWithoutQuery);