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 023314f20896d2..71d70789bc517c 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 @@ -485,6 +485,21 @@ public static class Authentication { @ConfigItem(defaultValueDocumentation = "false") public Optional forceRedirectHttpsScheme = Optional.empty(); + /** + * Fail the authentication with the HTTP 401 status if the state cookie is missing when the code flow + * has to be completed and either the 'code' or 'error' parameter is detected. + * + * If this property is enabled then an authentication challenge redirecting the user to OpenId Connect provider + * will be issued instead if no state cookie is detected. + * + * Enable this property only if you'd like to support custom 'code' or 'error' query parameters and + * avoid triggering the authorization code completion process. + * + * This property has no effect when the session is already established. + */ + @ConfigItem(defaultValue = "false") + public boolean redirectWithoutStateCookie; + /** * List of scopes */ @@ -700,6 +715,14 @@ public void setCookieSuffix(String cookieSuffix) { this.cookieSuffix = Optional.of(cookieSuffix); } + public boolean isRedirectWithoutStateCookie() { + return redirectWithoutStateCookie; + } + + public void setRedirectWithoutStateCookie(boolean redirectWithoutStateCookie) { + this.redirectWithoutStateCookie = redirectWithoutStateCookie; + } + } @ConfigGroup 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 9ac994053d5723..631488b7465368 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 @@ -63,7 +63,7 @@ public Uni authenticate(RoutingContext context, IdentityProviderManager identityProviderManager, OidcTenantConfig oidcTenantConfig) { final Cookie sessionCookie = context.request().getCookie(getSessionCookieName(oidcTenantConfig)); - // if session already established, try to re-authenticate + // if the session is already established then try to re-authenticate if (sessionCookie != null) { context.put(OidcUtils.SESSION_COOKIE_NAME, sessionCookie.getName()); Uni resolvedContext = resolver.resolveContext(context); @@ -76,21 +76,56 @@ public Uni apply(TenantConfigContext tenantContext) { }); } - final String code = context.request().getParam("code"); - if (code == null) { - return Uni.createFrom().optional(Optional.empty()); + final Cookie stateCookie = context.request().getCookie(getStateCookieName(oidcTenantConfig)); + + // if the state cookie is available then try to complete the code flow and start a new session + if (stateCookie != null) { + + if (!isStateCookieValid(context, stateCookie)) { + return Uni.createFrom().failure(new AuthenticationCompletionException()); + } + + final String code = context.request().getParam(OidcConstants.CODE_FLOW_CODE); + if (code != null) { + // start a new session by starting the code flow dance + Uni resolvedContext = resolver.resolveContext(context); + return resolvedContext.onItem() + .transformToUni(new Function>() { + @Override + public Uni apply(TenantConfigContext tenantContext) { + return performCodeFlow(identityProviderManager, context, tenantContext, code, stateCookie); + } + }); + } else { + LOG.debug("State cookie is present but neither 'code' nor 'error' query parameter is returned"); + OidcUtils.removeCookie(context, oidcTenantConfig, stateCookie.getName()); + return Uni.createFrom().failure(new AuthenticationCompletionException()); + } + } else if (context.request().getParam(OidcConstants.CODE_FLOW_CODE) != null + && !oidcTenantConfig.authentication.redirectWithoutStateCookie) { + LOG.debug( + "The state cookie is missing but either the 'code' or 'error' parameter is available, authentication has failed"); + return Uni.createFrom().failure(new AuthenticationCompletionException()); + } - // start a new session by starting the code flow dance - Uni resolvedContext = resolver.resolveContext(context); - return resolvedContext.onItem() - .transformToUni(new Function>() { - @Override - public Uni apply(TenantConfigContext tenantContext) { - return performCodeFlow(identityProviderManager, context, tenantContext, code); - } - }); + // return an empty identity - this will lead to a challenge redirecting the user to OpenId Connect provider + // unless some other authentication mechanism creates the identity. + return Uni.createFrom().optional(Optional.empty()); + + } + private boolean isStateCookieValid(RoutingContext context, Cookie stateCookie) { + List values = context.queryParam("state"); + // IDP must return a 'state' query parameter and the value of the state cookie must start with this parameter's value + if (values.size() != 1) { + LOG.debug("State parameter can not be empty or multi-valued"); + return false; + } else if (!stateCookie.getValue().startsWith(values.get(0))) { + LOG.debug("State cookie value does not match the state query parameter value"); + return false; + } + return true; } private Uni reAuthenticate(Cookie sessionCookie, @@ -251,42 +286,25 @@ public Uni apply(Void t) { } private Uni performCodeFlow(IdentityProviderManager identityProviderManager, - RoutingContext context, TenantConfigContext configContext, String code) { - - Cookie stateCookie = context.getCookie(getStateCookieName(configContext)); + RoutingContext context, TenantConfigContext configContext, String code, Cookie stateCookie) { String userPath = null; String userQuery = null; - if (stateCookie != null) { - List values = context.queryParam("state"); - // IDP must return a 'state' query parameter and the value of the state cookie must start with this parameter's value - if (values.size() != 1) { - LOG.debug("State parameter can not be empty or multi-valued"); - return Uni.createFrom().failure(new AuthenticationCompletionException()); - } else if (!stateCookie.getValue().startsWith(values.get(0))) { - LOG.debug("State cookie value does not match the state query parameter value"); - return Uni.createFrom().failure(new AuthenticationCompletionException()); - } else { - // This is an original redirect from IDP, check if the original request path and query need to be restored - String[] pair = COOKIE_PATTERN.split(stateCookie.getValue()); - if (pair.length == 2) { - int userQueryIndex = pair[1].indexOf("?"); - if (userQueryIndex >= 0) { - userPath = pair[1].substring(0, userQueryIndex); - if (userQueryIndex + 1 < pair[1].length()) { - userQuery = pair[1].substring(userQueryIndex + 1); - } - } else { - userPath = pair[1]; - } + + // This is an original redirect from IDP, check if the original request path and query need to be restored + String[] pair = COOKIE_PATTERN.split(stateCookie.getValue()); + if (pair.length == 2) { + int userQueryIndex = pair[1].indexOf("?"); + if (userQueryIndex >= 0) { + userPath = pair[1].substring(0, userQueryIndex); + if (userQueryIndex + 1 < pair[1].length()) { + userQuery = pair[1].substring(userQueryIndex + 1); } - OidcUtils.removeCookie(context, configContext.oidcConfig, stateCookie.getName()); + } else { + userPath = pair[1]; } - } else { - // State cookie must be available to minimize the risk of CSRF - LOG.debug("The state cookie is missing after a redirect from IDP, authentication has failed"); - return Uni.createFrom().failure(new AuthenticationCompletionException()); } + OidcUtils.removeCookie(context, configContext.oidcConfig, stateCookie.getName()); final String finalUserPath = userPath; final String finalUserQuery = userQuery; @@ -463,13 +481,13 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext cookieValue += (COOKIE_DELIM + requestPath); } } - createCookie(context, configContext.oidcConfig, getStateCookieName(configContext), cookieValue, 60 * 30); + createCookie(context, configContext.oidcConfig, getStateCookieName(configContext.oidcConfig), cookieValue, 60 * 30); return uuid; } private String generatePostLogoutState(RoutingContext context, TenantConfigContext configContext) { - OidcUtils.removeCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext)); - return createCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext), + OidcUtils.removeCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext.oidcConfig)); + return createCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext.oidcConfig), UUID.randomUUID().toString(), 60 * 30).getValue(); } @@ -639,12 +657,12 @@ public Void apply(Void t) { }); } - private static String getStateCookieName(TenantConfigContext configContext) { - return OidcUtils.STATE_COOKIE_NAME + getCookieSuffix(configContext.oidcConfig); + private static String getStateCookieName(OidcTenantConfig oidcConfig) { + return OidcUtils.STATE_COOKIE_NAME + getCookieSuffix(oidcConfig); } - private static String getPostLogoutCookieName(TenantConfigContext configContext) { - return OidcUtils.POST_LOGOUT_COOKIE_NAME + getCookieSuffix(configContext.oidcConfig); + private static String getPostLogoutCookieName(OidcTenantConfig oidcConfig) { + return OidcUtils.POST_LOGOUT_COOKIE_NAME + getCookieSuffix(oidcConfig); } private static String getSessionCookieName(OidcTenantConfig oidcConfig) { diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/ProtectedResource3.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/ProtectedResource3.java new file mode 100644 index 00000000000000..161449c0a4d1ee --- /dev/null +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/ProtectedResource3.java @@ -0,0 +1,19 @@ +package io.quarkus.it.keycloak; + +import javax.ws.rs.GET; +import javax.ws.rs.InternalServerErrorException; +import javax.ws.rs.Path; + +import io.quarkus.security.Authenticated; + +@Path("/web-app3") +@Authenticated +public class ProtectedResource3 { + + @GET + public String getName() { + // CodeFlowTest#testAuthenticationCompletionFailedNoStateCookie checks that if a state cookie is missing + // then 401 is returned when a redirect targets the endpoint requiring authentication + throw new InternalServerErrorException("This method must not be invoked"); + } +} diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java index fb6dee72eff5b5..5177d547dae558 100644 --- a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantHttps.java @@ -25,7 +25,7 @@ public String getTenant() { @GET @Path("query") - public String getTenantWithQuery(@QueryParam("a") String value) { - return getTenant() + "?a=" + value; + public String getTenantWithQuery(@QueryParam("code") String value) { + return getTenant() + "?code=" + value; } } 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 ba799461de52d8..b0bb27762d142f 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -1,4 +1,4 @@ -# Default tenant configuration +# Default tenant configurationf quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus quarkus.oidc.client-id=quarkus-app quarkus.oidc.credentials.secret=secret @@ -103,6 +103,7 @@ quarkus.oidc.tenant-https.authentication.extra-params.max-age=60 quarkus.oidc.tenant-https.application-type=web-app quarkus.oidc.tenant-https.authentication.force-redirect-https-scheme=true quarkus.oidc.tenant-https.authentication.cookie-suffix=test +quarkus.oidc.tenant-https.authentication.redirect-without-state-cookie=true quarkus.oidc.tenant-javascript.auth-server-url=${keycloak.url}/realms/quarkus quarkus.oidc.tenant-javascript.client-id=quarkus-app @@ -144,9 +145,6 @@ quarkus.oidc.tenant-split-tokens.application-type=web-app quarkus.http.auth.permission.roles1.paths=/index.html quarkus.http.auth.permission.roles1.policy=authenticated -quarkus.http.auth.permission.callback.paths=/web-app3 -quarkus.http.auth.permission.callback.policy=authenticated - quarkus.http.auth.permission.logout.paths=/tenant-logout quarkus.http.auth.permission.logout.policy=authenticated @@ -169,9 +167,5 @@ quarkus.http.proxy.allow-forwarded=true quarkus.log.category."io.quarkus.oidc.runtime.CodeAuthenticationMechanism".min-level=TRACE quarkus.log.category."io.quarkus.oidc.runtime.CodeAuthenticationMechanism".level=TRACE -quarkus.log.category."io.vertx.ext.auth.oauth2.impl.OAuth2UserImpl".min-level=TRACE -quarkus.log.category."io.vertx.ext.auth.oauth2.impl.OAuth2UserImpl".level=TRACE -#quarkus.log.category."org.apache.http".min-level=TRACE -#quarkus.log.category."org.apache.http".level=TRACE quarkus.log.category."com.gargoylesoftware.htmlunit.javascript.host.css.CSSStyleSheet".level=FATAL 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 b8eb0507bda2dd..a91f2f632fc507 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 @@ -46,6 +46,8 @@ public void testCodeFlowNoConsent() throws IOException { .loadWebResponse(new WebRequest(URI.create("http://localhost:8081/index.html").toURL())); verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), null, "web-app", false); + webClient.getCookieManager().clearCookies(); + webClient.getOptions().setRedirectEnabled(true); HtmlPage page = webClient.getPage("http://localhost:8081/index.html"); @@ -147,7 +149,7 @@ public void testCodeFlowForceHttpsRedirectUriWithQuery() throws IOException { WebResponse webResponse = webClient .loadWebResponse( - new WebRequest(URI.create("http://localhost:8081/tenant-https/query?a=b").toURL())); + new WebRequest(URI.create("http://localhost:8081/tenant-https/query?code=b").toURL())); String keycloakUrl = webResponse.getResponseHeaderValue("location"); verifyLocationHeader(webClient, keycloakUrl, "tenant-https_test", "tenant-https", true); @@ -178,10 +180,10 @@ public void testCodeFlowForceHttpsRedirectUriWithQuery() throws IOException { endpointLocationWithoutQuery = "http" + endpointLocationWithoutQuery.substring(5); URI endpointLocationWithoutQueryUri = URI.create(endpointLocationWithoutQuery); - assertEquals("a=b", endpointLocationWithoutQueryUri.getRawQuery()); + assertEquals("code=b", endpointLocationWithoutQueryUri.getRawQuery()); page = webClient.getPage(endpointLocationWithoutQueryUri.toURL()); - assertEquals("tenant-https:reauthenticated?a=b", page.getBody().asText()); + assertEquals("tenant-https:reauthenticated?code=b", page.getBody().asText()); Cookie sessionCookie = getSessionCookie(webClient, "tenant-https_test"); assertNotNull(sessionCookie); webClient.getCookieManager().clearCookies(); @@ -241,6 +243,7 @@ public Boolean call() throws Exception { }); webClient.getOptions().setRedirectEnabled(true); + webClient.getCookieManager().clearCookies(); page = webClient.getPage("http://localhost:8081/index.html"); assertEquals("Sign in to quarkus", page.getTitleText()); @@ -321,6 +324,8 @@ public Boolean call() throws Exception { // session invalidated because it ended at the OP, should redirect to login page at the OP webClient.getOptions().setRedirectEnabled(true); + webClient.getCookieManager().clearCookies(); + page = webClient.getPage("http://localhost:8081/tenant-logout"); assertNull(getSessionCookie(webClient, "tenant-logout")); assertEquals("Sign in to logout-realm", page.getTitleText()); @@ -510,6 +515,8 @@ public void testIdTokenInjectionWithoutRestoredPathDifferentRoot() throws IOExce assertNull(getSessionCookie(webClient, "tenant-2")); webClient.getOptions().setRedirectEnabled(true); + webClient.getCookieManager().clearCookies(); + page = webClient.getPage("http://localhost:8081/web-app2/name"); assertEquals("Sign in to quarkus", page.getTitleText());