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 968a7d069ea17..093d45fcdb238 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 @@ -889,8 +889,8 @@ public enum ResponseMode { * 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; + @ConfigItem(defaultValue = "false") + public boolean failOnMissingStateParam = false; /** * If this property is set to 'true' then an OIDC UserInfo endpoint will be called. 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 bd36bbb81e98c..6ed98a9c7b61d 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 @@ -270,8 +270,10 @@ private Uni stateCookieIsNotMatched(OidcTenantConfig oidcTenan if (!oidcTenantConfig.authentication.allowMultipleCodeFlows || context.request().path().equals(getRedirectPath(oidcTenantConfig, context))) { if (oidcTenantConfig.authentication.failOnMissingStateParam) { + removeStateCookies(oidcTenantConfig, context, cookies); return Uni.createFrom().failure(new AuthenticationCompletionException()); - } else { + } + if (!oidcTenantConfig.authentication.allowMultipleCodeFlows) { removeStateCookies(oidcTenantConfig, context, cookies); } } 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 7e4b58d206b52..3f92f20e648f5 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -112,7 +112,7 @@ quarkus.oidc.tenant-https.authentication.pkce-required=true quarkus.oidc.tenant-https.authentication.nonce-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-https.authentication.fail-on-missing-state-param=true quarkus.oidc.tenant-nonce.auth-server-url=${quarkus.oidc.auth-server-url} quarkus.oidc.tenant-nonce.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 75e3afffc6d24..ae138fd9d60b3 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 @@ -253,8 +253,11 @@ public void testStateCookieIsPresentButStateParamNot() throws Exception { // 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); + assertTrue(endpointLocation.startsWith("https://localhost:8081/tenant-https")); + + String code = getCode(URI.create(endpointLocation).getQuery()); + + endpointLocation = "http://localhost:8081/tenant-https"; // State cookie is present Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); @@ -262,22 +265,26 @@ public void testStateCookieIsPresentButStateParamNot() throws Exception { verifyCodeVerifierAndNonce(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())); + webResponse = webClient.loadWebResponse(new WebRequest(URI.create(endpointLocation + "?code=" + code).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(); } } + private String getCode(String query) { + final String[] pairs = query.split("&"); + for (String pair : pairs) { + if (pair.startsWith("code")) { + return pair.split("=")[1]; + } + } + fail("Authorization code is missing"); + return null; + } + @Test public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception { try (final WebClient webClient = createWebClient()) { @@ -825,6 +832,41 @@ public void testIdTokenInjectionJwtMethod() throws IOException, InterruptedExcep } } + @Test + public void testIdTokenInjectionJwtMethodMissingStateQueryParam() throws IOException, InterruptedException { + try (final WebClient webClient = createWebClient()) { + webClient.getOptions().setRedirectEnabled(false); + WebResponse webResponse = webClient + .loadWebResponse( + new WebRequest(URI.create("http://localhost:8081/web-app/callback-jwt-before-redirect").toURL())); + Cookie stateCookie = getNonUniqueStateCookie(webClient, "tenant-jwt"); + + assertEquals(stateCookie.getName(), "q_auth_tenant-jwt"); + assertNotNull(getStateCookieStateParam(stateCookie)); + assertNull(getStateCookieSavedPath(stateCookie)); + + HtmlPage page = webClient.getPage(webResponse.getResponseHeaderValue("location")); + 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 + String endpointLocation = webResponse.getResponseHeaderValue("location"); + assertTrue(endpointLocation.startsWith("http://localhost:8081/web-app/callback-jwt-after-redirect")); + endpointLocation = "http://localhost:8081/web-app/callback-jwt-after-redirect"; + webResponse = webClient.loadWebResponse(new WebRequest(URI.create(endpointLocation).toURL())); + assertEquals(302, webResponse.getStatusCode()); + assertNotNull(getNonUniqueStateCookie(webClient, "tenant-jwt")); + + webClient.getCookieManager().clearCookies(); + } + } + @Test public void testIdTokenInjectionJwtMethodButPostMethodUsed() throws IOException, InterruptedException { try (final WebClient webClient = createWebClient()) {