Merge pull request quarkusio#45659 from sberyozkin/unmatched_key_for_…

…oidc_session_reauthentication

Request re-authentication if the OIDC session key is unresolved

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<Boolean> 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`).
+         * <p>
+         * 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.
+         * <p>
+         * 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.
          * <p>
@@ -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();

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<? extends SecurityIdentity> 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)) {

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`).
+         * <p>
+         * 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.
          * <p>

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/builders/AuthenticationConfigBuilder.java

@@ -26,7 +26,8 @@ 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, boolean failOnUnresolvedKid, Optional<Boolean> userInfoRequired,
+            Duration sessionAgeExtension,
             Duration stateCookieAge, boolean javaScriptAutoRedirect, Optional<Boolean> idTokenRequired,
             Optional<Duration> internalIdTokenLifespan, Optional<Boolean> pkceRequired, Optional<String> pkceSecret,
             Optional<String> stateSecret) implements Authentication {
@@ -55,6 +56,7 @@ private record AuthenticationImpl(Optional<ResponseMode> responseMode, Optional<
     private CookieSameSite cookieSameSite;
     private boolean allowMultipleCodeFlows;
     private boolean failOnMissingStateParam;
+    private boolean failOnUnresolvedKid;
     private Optional<Boolean> 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);
     }

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<Boolean> userInfoRequired() {
                 invocationsRecorder.put(ConfigMappingMethods.AUTHENTICATION_USER_INFO_REQUIRED, true);

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

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);