Allow reauthentication if the OIDC state cookie is not matched

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java

@@ -859,6 +859,30 @@ public enum ResponseMode {
         @ConfigItem(defaultValue = "true")
         public boolean allowMultipleCodeFlows = true;
 
+        /**
+         * Fail with the HTTP 401 error if the state cookie is present but no state query parameter is present.
+         * <p/>
+         * When either multiple authentications are disabled or the redirect URL
+         * matches the original request URL, the stale state cookie might remain in the browser cache from
+         * the earlier failed redirect to an OpenId Connect provider and be visible during the current request.
+         * For example, if Single-page application (SPA) uses XHR to handle redirects to the provider
+         * which does not support CORS for its authorization endpoint, the browser will block it
+         * and the state cookie created by Quarkus will remain in the browser cache.
+         * Quarkus will report an authentication failure when it will detect such an old state cookie but find no matching state
+         * query parameter.
+         * <p/>
+         * Reporting HTTP 401 error is usually the right thing to do in such cases, it will minimize a risk of the
+         * browser redirect loop but also can identify problems in the way SPA or Quarkus application manage redirects.
+         * For example, enabling {@link #javaScriptAutoRedirect} or having the provider redirect to URL configured
+         * with {@link #redirectPath} may be needed to avoid such errors.
+         * <p/>
+         * However, setting this property to `false` may help if the above options are not suitable.
+         * It will cause a new authentication redirect to OpenId Connect provider. Please be aware doing so may increase the
+         * risk of browser redirect loops.
+         */
+        @ConfigItem(defaultValue = "true")
+        public boolean failOnMissingStateParam = true;
+
         /**
          * If this property is set to 'true' then an OIDC UserInfo endpoint will be called.
          * This property will be enabled if `quarkus.oidc.roles.source` is `userinfo`

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java

@@ -106,7 +106,7 @@ public Uni<SecurityIdentity> apply(TenantConfigContext tenantContext) {
                                 @Override
                                 public Uni<? extends SecurityIdentity> apply(MultiMap requestParams) {
                                     return processRedirectFromOidc(context, oidcTenantConfig, identityProviderManager,
-                                            requestParams);
+                                            requestParams, cookies);
                                 }
                             });
                 }
@@ -115,7 +115,7 @@ public Uni<? extends SecurityIdentity> apply(MultiMap requestParams) {
                 return Uni.createFrom().failure(new AuthenticationFailedException());
             } else {
                 return processRedirectFromOidc(context, oidcTenantConfig, identityProviderManager,
-                        context.queryParams());
+                        context.queryParams(), cookies);
             }
         }
 
@@ -136,7 +136,8 @@ private boolean isStateCookieAvailable(Map<String, Cookie> cookies) {
     }
 
     private Uni<SecurityIdentity> processRedirectFromOidc(RoutingContext context, OidcTenantConfig oidcTenantConfig,
-            IdentityProviderManager identityProviderManager, MultiMap requestParams) {
+            IdentityProviderManager identityProviderManager, MultiMap requestParams,
+            Map<String, Cookie> cookies) {
 
         // At this point it has already been detected that some state cookie is available.
         // If the state query parameter is not available or is available but no matching state cookie is found then if
@@ -150,16 +151,14 @@ private Uni<SecurityIdentity> processRedirectFromOidc(RoutingContext context, Oi
 
         List<String> stateQueryParam = requestParams.getAll(OidcConstants.CODE_FLOW_STATE);
         if (stateQueryParam.size() != 1) {
-            LOG.debug("State parameter can not be empty or multi-valued if the state cookie is present");
-            return stateCookieIsMissing(oidcTenantConfig, context);
+            return stateParamIsMissing(oidcTenantConfig, context, cookies, stateQueryParam.size() > 1);
         }
 
         final Cookie stateCookie = context.request().getCookie(
                 getStateCookieName(oidcTenantConfig) + "_" + stateQueryParam.get(0));
 
         if (stateCookie == null) {
-            LOG.debug("Matching state cookie is not found");
-            return stateCookieIsMissing(oidcTenantConfig, context);
+            return stateCookieIsMissing(oidcTenantConfig, context, cookies);
         }
 
         String[] parsedStateCookieValue = COOKIE_PATTERN.split(stateCookie.getValue());
@@ -239,14 +238,44 @@ public Uni<SecurityIdentity> apply(TenantConfigContext tenantContext) {
 
     }
 
-    private Uni<SecurityIdentity> stateCookieIsMissing(OidcTenantConfig oidcTenantConfig, RoutingContext context) {
+    private Uni<SecurityIdentity> stateParamIsMissing(OidcTenantConfig oidcTenantConfig, RoutingContext context,
+            Map<String, Cookie> cookies, boolean multipleStateQueryParams) {
+        if (multipleStateQueryParams) {
+            LOG.warn("State query parameter can not be multi-valued if the state cookie is present");
+            removeStateCookies(oidcTenantConfig, context, cookies);
+            return Uni.createFrom().failure(new AuthenticationCompletionException());
+        }
+        LOG.debug("State parameter can not be empty if the state cookie is present");
+        return stateCookieIsNotMatched(oidcTenantConfig, context, cookies);
+    }
+
+    private Uni<SecurityIdentity> stateCookieIsMissing(OidcTenantConfig oidcTenantConfig, RoutingContext context,
+            Map<String, Cookie> cookies) {
+        LOG.debug("Matching state cookie is not found");
+        return stateCookieIsNotMatched(oidcTenantConfig, context, cookies);
+    }
+
+    private Uni<SecurityIdentity> stateCookieIsNotMatched(OidcTenantConfig oidcTenantConfig, RoutingContext context,
+            Map<String, Cookie> cookies) {
         if (!oidcTenantConfig.authentication.allowMultipleCodeFlows
                 || context.request().path().equals(getRedirectPath(oidcTenantConfig, context))) {
-            return Uni.createFrom().failure(new AuthenticationCompletionException());
-        } else {
-            context.put(NO_OIDC_COOKIES_AVAILABLE, Boolean.TRUE);
-            return Uni.createFrom().optional(Optional.empty());
+            if (oidcTenantConfig.authentication.failOnMissingStateParam) {
+                return Uni.createFrom().failure(new AuthenticationCompletionException());
+            } else {
+                removeStateCookies(oidcTenantConfig, context, cookies);
+            }
+        }
+        context.put(NO_OIDC_COOKIES_AVAILABLE, Boolean.TRUE);
+        return Uni.createFrom().optional(Optional.empty());
+    }
+
+    private void removeStateCookies(OidcTenantConfig oidcTenantConfig, RoutingContext context, Map<String, Cookie> cookies) {
+        for (String name : cookies.keySet()) {
+            if (name.startsWith(OidcUtils.STATE_COOKIE_NAME)) {
+                OidcUtils.removeCookie(context, oidcTenantConfig, name);
+            }
         }
+
     }
 
     private String getRequestParametersAsQuery(URI requestUri, MultiMap requestParams, OidcTenantConfig oidcConfig) {

integration-tests/oidc-code-flow/src/main/resources/application.properties

@@ -109,6 +109,7 @@ quarkus.oidc.tenant-https.authentication.error-path=/tenant-https/error
 quarkus.oidc.tenant-https.authentication.pkce-required=true
 quarkus.oidc.tenant-https.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU
 quarkus.oidc.tenant-https.authentication.cookie-same-site=strict
+quarkus.oidc.tenant-https.authentication.fail-on-missing-state-param=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

@@ -227,6 +227,56 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
         }
     }
 
+    @Test
+    public void testStateCookieIsPresentButStateParamNot() throws Exception {
+        try (final WebClient webClient = createWebClient()) {
+            webClient.getOptions().setRedirectEnabled(false);
+
+            WebResponse webResponse = webClient
+                    .loadWebResponse(
+                            new WebRequest(URI.create("http://localhost:8081/tenant-https").toURL()));
+            String keycloakUrl = webResponse.getResponseHeaderValue("location");
+            verifyLocationHeader(webClient, keycloakUrl, "tenant-https_test", "tenant-https",
+                    true);
+
+            HtmlPage page = webClient.getPage(keycloakUrl);
+
+            assertEquals("Sign in to quarkus", page.getTitleText());
+            HtmlForm loginForm = page.getForms().get(0);
+            loginForm.getInputByName("username").setValueAttribute("alice");
+            loginForm.getInputByName("password").setValueAttribute("alice");
+
+            webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
+            webResponse = loginForm.getInputByName("login").click().getWebResponse();
+            webClient.getOptions().setThrowExceptionOnFailingStatusCode(true);
+
+            // This is a redirect from the OIDC server to the endpoint containing the state and code
+            String endpointLocation = webResponse.getResponseHeaderValue("location");
+            assertTrue(endpointLocation.startsWith("https"));
+            endpointLocation = "http" + endpointLocation.substring(5);
+
+            // State cookie is present
+            Cookie stateCookie = getStateCookie(webClient, "tenant-https_test");
+            assertNull(stateCookie.getSameSite());
+            verifyCodeVerifier(stateCookie, keycloakUrl);
+
+            // Make a call without an extra state query param, status is 401
+            webResponse = webClient.loadWebResponse(new WebRequest(URI.create(endpointLocation + "&state=123").toURL()));
+            assertEquals(401, webResponse.getStatusCode());
+
+            // Make a call without the state query param, confirm the old state cookie is removed, status is 302
+            webResponse = webClient.loadWebResponse(new WebRequest(URI.create("http://localhost:8081/tenant-https").toURL()));
+            assertEquals(302, webResponse.getStatusCode());
+            // the old state cookie has been removed
+            assertNull(webClient.getCookieManager().getCookie(stateCookie.getName()));
+            // new state cookie is created
+            Cookie newStateCookie = getStateCookie(webClient, "tenant-https_test");
+            assertNotEquals(newStateCookie.getName(), stateCookie.getName());
+
+            webClient.getCookieManager().clearCookies();
+        }
+    }
+
     @Test
     public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception {
         try (final WebClient webClient = createWebClient()) {