From 095f4fe98d006fbf0f265d826ed7ddf2695acaf7 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Wed, 26 Jul 2023 13:16:01 +0100 Subject: [PATCH] Support OIDC authorization code flow nonce --- ...ecurity-oidc-code-flow-authentication.adoc | 6 +- .../security-openid-connect-providers.adoc | 2 +- .../oidc/common/runtime/OidcConstants.java | 1 + .../io/quarkus/oidc/OidcTenantConfig.java | 47 +++++++ .../runtime/CodeAuthenticationMechanism.java | 53 ++++++-- .../runtime/CodeAuthenticationStateBean.java | 12 +- .../oidc/runtime/TenantConfigContext.java | 47 ++++--- .../it/keycloak/CustomTenantResolver.java | 9 ++ .../src/main/resources/application.properties | 10 ++ .../io/quarkus/it/keycloak/CodeFlowTest.java | 118 ++++++++++++++++-- 10 files changed, 265 insertions(+), 40 deletions(-) diff --git a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc index 02ae3fb7860e6..ff302a9276be1 100644 --- a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc +++ b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc @@ -469,15 +469,15 @@ link:https://datatracker.ietf.org/doc/html/rfc7636[Proof Key for Code Exchange] While PKCE is of primary importance to public OpenID Connect clients, such as SPA scripts running in a browser, it can also provide an extra level of protection to Quarkus OIDC `web-app` applications. With PKCE, Quarkus OIDC `web-app` applications are confidential OpenID Connect clients capable of securely storing the client secret and using it to exchange the code for the tokens. -You can enable `PKCE` for your OIDC `web-app` endpoint with a `quarkus.oidc.authentication.pkce-required` property and a 32-character secret, as shown in the following example: +You can enable `PKCE` for your OIDC `web-app` endpoint with a `quarkus.oidc.authentication.pkce-required` property and a 32-character secret whixh is required to encrypt the PKCE code verifier in the state cookie, as shown in the following example: [source, properties] ---- quarkus.oidc.authentication.pkce-required=true -quarkus.oidc.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU +quarkus.oidc.authentication.state-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU ---- -If you already have a 32-characters client secret then you do not need to set the `quarkus.oidc.authentication.pkce-secret` property unless you prefer to use a different secret key. +If you already have a 32-characters client secret then you do not need to set the `quarkus.oidc.authentication.pkce-secret` property unless you prefer to use a different secret key. This secret will be auto-generated if it is not configured and if the fallback to the client secret is not possible in case of the client secret being less than 16 characters long. The secret key is required for encrypting a randomly generated `PKCE` `code_verifier` while the user is being redirected with the `code_challenge` query parameter to an OIDC provider to authenticate. The `code_verifier` is decrypted when the user is redirected back to Quarkus and sent to the token endpoint alongside the `code`, client secret, and other parameters to complete the code exchange. diff --git a/docs/src/main/asciidoc/security-openid-connect-providers.adoc b/docs/src/main/asciidoc/security-openid-connect-providers.adoc index 9010a4f53894b..2d173565d4c2f 100644 --- a/docs/src/main/asciidoc/security-openid-connect-providers.adoc +++ b/docs/src/main/asciidoc/security-openid-connect-providers.adoc @@ -386,7 +386,7 @@ Twitter provider requires Proof Key for Code Exchange (PKCE) which is supported Quarkus has to encrypt the current PKCE code verifier in a state cookie while the authorization code flow with Twitter is in progress and it will generate a secure random secret key for encrypting it. -You can provide your own secret key for encrypting the PKCE code verifier if you prefer with the `quarkus.oidc.authentication.pkce-secret` property but +You can provide your own secret key for encrypting the PKCE code verifier if you prefer with the `quarkus.oidc.authentication.state-secret` property but note that this secret should be 32 characters long, and an error will be reported if it is less than 16 characters long. ==== diff --git a/extensions/oidc-common/runtime/src/main/java/io/quarkus/oidc/common/runtime/OidcConstants.java b/extensions/oidc-common/runtime/src/main/java/io/quarkus/oidc/common/runtime/OidcConstants.java index 931070f98e9df..ebeee451130c9 100644 --- a/extensions/oidc-common/runtime/src/main/java/io/quarkus/oidc/common/runtime/OidcConstants.java +++ b/extensions/oidc-common/runtime/src/main/java/io/quarkus/oidc/common/runtime/OidcConstants.java @@ -73,4 +73,5 @@ public final class OidcConstants { public static final String ID_TOKEN_SID_CLAIM = "sid"; public static final String OPENID_SCOPE = "openid"; + public static final String NONCE = "nonce"; } 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 e97c36d45e9a3..df152c651d16f 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 @@ -781,6 +781,15 @@ public enum ResponseMode { @ConfigItem public Optional> scopes = Optional.empty(); + /** + * Require that ID token includes `nonce` claim which must match `nonce` authentication request query parameter. + * Enabling this property can help mitigate replay attacks. + * Do not enable this property if your OpenId Connect provider does not support setting `nonce` in ID token + * or if you work with OAuth2 provider such as `GitHub` which does not issue ID tokens. + */ + @ConfigItem + public boolean nonceRequired = false; + /** * Add the 'openid' scope automatically to the list of scopes. This is required for OpenId Connect providers * but will not work for OAuth2 providers such as Twitter OAuth2 which does not accept that scope and throws an error. @@ -955,10 +964,30 @@ public enum ResponseMode { * The secret will be auto-generated if it remains uninitialized after checking all of these properties. *

* Error will be reported if the secret length is less than 16 characters. + * */ @ConfigItem + @Deprecated(forRemoval = true) public Optional pkceSecret = Optional.empty(); + /** + * Secret which will be used to encrypt Proof Key for Code Exchange (PKCE) code verifier and/or nonce in the code flow + * state. + * This secret should be at least 32 characters long. + *

+ * If this secret is not set, the client secret configured with + * either `quarkus.oidc.credentials.secret` or `quarkus.oidc.credentials.client-secret.value` will be checked. + * Finally, `quarkus.oidc.credentials.jwt.secret` which can be used for `client_jwt_secret` authentication will be + * checked. Client secret will not be used as a state encryption secret if it is less than 32 characters + * long. + *

+ * The secret will be auto-generated if it remains uninitialized after checking all of these properties. + *

+ * Error will be reported if the secret length is less than 16 characters. + */ + @ConfigItem + public Optional stateSecret = Optional.empty(); + public Optional getInternalIdTokenLifespan() { return internalIdTokenLifespan; } @@ -975,10 +1004,12 @@ public void setPkceRequired(boolean pkceRequired) { this.pkceRequired = Optional.of(pkceRequired); } + @Deprecated(forRemoval = true) public Optional getPkceSecret() { return pkceSecret; } + @Deprecated(forRemoval = true) public void setPkceSecret(String pkceSecret) { this.pkceSecret = Optional.of(pkceSecret); } @@ -1158,6 +1189,22 @@ public boolean isAllowMultipleCodeFlows() { public void setAllowMultipleCodeFlows(boolean allowMultipleCodeFlows) { this.allowMultipleCodeFlows = allowMultipleCodeFlows; } + + public boolean isNonceRequired() { + return nonceRequired; + } + + public void setNonceRequired(boolean nonceRequired) { + this.nonceRequired = nonceRequired; + } + + public Optional getStateSecret() { + return stateSecret; + } + + public void setStateSecret(Optional stateSecret) { + this.stateSecret = stateSecret; + } } /** 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 4182e78cc28d8..8d9fcf511023e 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 @@ -623,9 +623,12 @@ && isRedirectFromProvider(context, configContext)) { PkceStateBean pkceStateBean = createPkceStateBean(configContext); // state + String nonce = configContext.oidcConfig.authentication.nonceRequired ? UUID.randomUUID().toString() + : null; + codeFlowParams.append(AMP).append(OidcConstants.CODE_FLOW_STATE).append(EQ) .append(generateCodeFlowState(context, configContext, redirectPath, requestQueryParams, - pkceStateBean != null ? pkceStateBean.getCodeVerifier() : null)); + (pkceStateBean != null ? pkceStateBean.getCodeVerifier() : null), nonce)); if (pkceStateBean != null) { codeFlowParams @@ -636,6 +639,10 @@ && isRedirectFromProvider(context, configContext)) { .append(OidcConstants.PKCE_CODE_CHALLENGE_S256); } + if (nonce != null) { + codeFlowParams.append(AMP).append(OidcConstants.NONCE).append(EQ).append(nonce); + } + // extra redirect parameters, see https://openid.net/specs/openid-connect-core-1_0.html#AuthRequests addExtraParamsToUri(codeFlowParams, configContext.oidcConfig.authentication.getExtraParams()); @@ -739,6 +746,9 @@ public Uni apply(final AuthorizationCodeTokens tokens, final T internalIdToken = true; } } else { + if (!verifyNonce(configContext.oidcConfig, stateBean, tokens.getIdToken())) { + return Uni.createFrom().failure(new AuthenticationCompletionException()); + } internalIdToken = false; } @@ -814,6 +824,21 @@ public Throwable apply(Throwable tInner) { }); } + private static boolean verifyNonce(OidcTenantConfig oidcConfig, CodeAuthenticationStateBean stateBean, String idToken) { + if (oidcConfig.authentication.nonceRequired) { + if (stateBean != null && stateBean.getNonce() != null) { + JsonObject idTokenClaims = OidcUtils.decodeJwtContent(idToken); + if (stateBean.getNonce().equals(idTokenClaims.getString(OidcConstants.NONCE))) { + return true; + } + } + LOG.errorf("ID token 'nonce' does not match the authentication request 'nonce' value"); + return false; + } else { + return true; + } + } + private static Object errorMessage(Throwable t) { return t.getCause() != null ? t.getCause().getMessage() : t.getMessage(); } @@ -822,14 +847,16 @@ private CodeAuthenticationStateBean getCodeAuthenticationBean(String[] parsedSta TenantConfigContext configContext) { if (parsedStateCookieValue.length == 2) { CodeAuthenticationStateBean bean = new CodeAuthenticationStateBean(); - if (!configContext.oidcConfig.authentication.pkceRequired.orElse(false)) { + Authentication authentication = configContext.oidcConfig.authentication; + boolean pkceRequired = authentication.pkceRequired.orElse(false); + if (!pkceRequired && !authentication.nonceRequired) { bean.setRestorePath(parsedStateCookieValue[1]); return bean; } JsonObject json = null; try { - json = OidcUtils.decryptJson(parsedStateCookieValue[1], configContext.getPkceSecretKey()); + json = OidcUtils.decryptJson(parsedStateCookieValue[1], configContext.getStateEncryptionKey()); } catch (Exception ex) { LOG.errorf("State cookie value can not be decrypted for the %s tenant", configContext.oidcConfig.tenantId.get()); @@ -837,6 +864,7 @@ private CodeAuthenticationStateBean getCodeAuthenticationBean(String[] parsedSta } bean.setRestorePath(json.getString(STATE_COOKIE_RESTORE_PATH)); bean.setCodeVerifier(json.getString(OidcConstants.PKCE_CODE_VERIFIER)); + bean.setNonce(json.getString(OidcConstants.NONCE)); return bean; } return null; @@ -943,12 +971,13 @@ private String getRedirectPath(OidcTenantConfig oidcConfig, RoutingContext conte } private String generateCodeFlowState(RoutingContext context, TenantConfigContext configContext, - String redirectPath, MultiMap requestQueryWithoutForwardedParams, String pkceCodeVerifier) { + String redirectPath, MultiMap requestQueryWithoutForwardedParams, String pkceCodeVerifier, String nonce) { String uuid = UUID.randomUUID().toString(); String cookieValue = uuid; - boolean restorePath = isRestorePath(configContext.oidcConfig.getAuthentication()); - if (restorePath || pkceCodeVerifier != null) { + Authentication authentication = configContext.oidcConfig.getAuthentication(); + boolean restorePath = isRestorePath(authentication); + if (restorePath || pkceCodeVerifier != null || nonce != null) { CodeAuthenticationStateBean extraStateValue = new CodeAuthenticationStateBean(); if (restorePath) { String requestQuery = context.request().query(); @@ -978,6 +1007,7 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext } } extraStateValue.setCodeVerifier(pkceCodeVerifier); + extraStateValue.setNonce(nonce); if (!extraStateValue.isEmpty()) { cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext)); } @@ -997,14 +1027,19 @@ private boolean isRestorePath(Authentication auth) { } private String encodeExtraStateValue(CodeAuthenticationStateBean extraStateValue, TenantConfigContext configContext) { - if (extraStateValue.getCodeVerifier() != null) { + if (extraStateValue.getCodeVerifier() != null || extraStateValue.getNonce() != null) { JsonObject json = new JsonObject(); - json.put(OidcConstants.PKCE_CODE_VERIFIER, extraStateValue.getCodeVerifier()); + if (extraStateValue.getCodeVerifier() != null) { + json.put(OidcConstants.PKCE_CODE_VERIFIER, extraStateValue.getCodeVerifier()); + } + if (extraStateValue.getNonce() != null) { + json.put(OidcConstants.NONCE, extraStateValue.getNonce()); + } if (extraStateValue.getRestorePath() != null) { json.put(STATE_COOKIE_RESTORE_PATH, extraStateValue.getRestorePath()); } try { - return OidcUtils.encryptJson(json, configContext.getPkceSecretKey()); + return OidcUtils.encryptJson(json, configContext.getStateEncryptionKey()); } catch (Exception ex) { LOG.errorf("State containing the code verifier can not be encrypted: %s", ex.getMessage()); throw new AuthenticationCompletionException(ex); diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java index 9c9e5a9765b23..c98528f81f8b8 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java @@ -6,6 +6,8 @@ public class CodeAuthenticationStateBean { private String codeVerifier; + private String nonce; + public String getRestorePath() { return restorePath; } @@ -22,8 +24,16 @@ public void setCodeVerifier(String codeVerifier) { this.codeVerifier = codeVerifier; } + public String getNonce() { + return nonce; + } + + public void setNonce(String nonce) { + this.nonce = nonce; + } + public boolean isEmpty() { - return this.restorePath == null && this.codeVerifier == null; + return this.restorePath == null && this.codeVerifier == null && nonce == null; } } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java index 066f4f5e6fb4b..203512b60ae85 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java @@ -11,6 +11,7 @@ import io.quarkus.oidc.OIDCException; import io.quarkus.oidc.OidcTenantConfig; import io.quarkus.oidc.common.runtime.OidcCommonUtils; +import io.quarkus.runtime.configuration.ConfigurationException; public class TenantConfigContext { private static final Logger LOG = Logger.getLogger(TenantConfigContext.class); @@ -28,7 +29,7 @@ public class TenantConfigContext { /** * PKCE Secret Key */ - private final SecretKey pkceSecretKey; + private final SecretKey stateSecretKey; /** * Token Encryption Secret Key @@ -47,39 +48,47 @@ public TenantConfigContext(OidcProvider client, OidcTenantConfig config, boolean this.ready = ready; boolean isService = OidcUtils.isServiceApp(config); - pkceSecretKey = !isService && provider != null && provider.client != null ? createPkceSecretKey(config) : null; + stateSecretKey = !isService && provider != null && provider.client != null ? createStateSecretKey(config) : null; tokenEncSecretKey = !isService && provider != null && provider.client != null ? createTokenEncSecretKey(config) : null; } - private static SecretKey createPkceSecretKey(OidcTenantConfig config) { - if (config.authentication.pkceRequired.orElse(false)) { - String pkceSecret = null; - if (config.authentication.pkceSecret.isPresent()) { - pkceSecret = config.authentication.pkceSecret.get(); - } else { - LOG.debug("'quarkus.oidc.token-state-manager.encryption-secret' is not configured, " + private static SecretKey createStateSecretKey(OidcTenantConfig config) { + if (config.authentication.pkceRequired.orElse(false) || config.authentication.nonceRequired) { + String stateSecret = null; + if (config.authentication.pkceSecret.isPresent() && config.authentication.getStateSecret().isPresent()) { + throw new ConfigurationException( + "Both 'quarkus.oidc.authentication.state-secret' and 'quarkus.oidc.authentication.pkce-secret' are configured"); + } + if (config.authentication.getStateSecret().isPresent()) { + stateSecret = config.authentication.getStateSecret().get(); + } else if (config.authentication.pkceSecret.isPresent()) { + stateSecret = config.authentication.pkceSecret.get(); + } + + if (stateSecret == null) { + LOG.debug("'quarkus.oidc.authentication.state-secret' is not configured, " + "trying to use the configured client secret"); String possiblePkceSecret = fallbackToClientSecret(config); if (possiblePkceSecret != null && possiblePkceSecret.length() < 32) { LOG.debug("Client secret is less than 32 characters long, the pkce secret will be generated"); } else { - pkceSecret = possiblePkceSecret; + stateSecret = possiblePkceSecret; } } try { - if (pkceSecret == null) { - LOG.debug("Secret key for encrypting PKCE code verifier is missing, auto-generating it"); + if (stateSecret == null) { + LOG.debug("Secret key for encrypting state cookie is missing, auto-generating it"); SecretKey key = generateSecretKey(); return key; } - byte[] secretBytes = pkceSecret.getBytes(StandardCharsets.UTF_8); + byte[] secretBytes = stateSecret.getBytes(StandardCharsets.UTF_8); if (secretBytes.length < 32) { - String errorMessage = "Secret key for encrypting PKCE code verifier in a state cookie should be at least 32 characters long" + String errorMessage = "Secret key for encrypting the state cookie should be at least 32 characters long" + " for the strongest state cookie encryption to be produced." - + " Please update 'quarkus.oidc.authentication.pkce-secret' or update the configured client secret."; + + " Please update 'quarkus.oidc.authentication.state-secret' or update the configured client secret."; if (secretBytes.length < 16) { - throw new RuntimeException( - "Secret key for encrypting PKCE code verifier is less than 32 characters long"); + throw new ConfigurationException( + "Secret key for encrypting the state cookie is less than 16 characters long"); } else { LOG.debug(errorMessage); } @@ -149,8 +158,8 @@ public OidcTenantConfig getOidcTenantConfig() { return oidcConfig; } - public SecretKey getPkceSecretKey() { - return pkceSecretKey; + public SecretKey getStateEncryptionKey() { + return stateSecretKey; } public SecretKey getTokenEncSecretKey() { diff --git a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java index 23f7cb1c5183f..8763bf2ef5ebf 100644 --- a/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java +++ b/integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java @@ -65,6 +65,15 @@ public String resolve(RoutingContext context) { } } + if (path.contains("tenant-nonce")) { + if (context.getCookie("q_session_tenant-nonce") != null) { + context.put("reauthenticated", "true"); + return context.get(OidcUtils.TENANT_ID_ATTRIBUTE); + } else { + return "tenant-nonce"; + } + } + if (path.contains("tenant-xhr")) { return "tenant-xhr"; } 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 213df8cabe444..7e4b58d206b52 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -109,10 +109,20 @@ quarkus.oidc.tenant-https.authentication.force-redirect-https-scheme=true quarkus.oidc.tenant-https.authentication.cookie-suffix=test quarkus.oidc.tenant-https.authentication.error-path=/tenant-https/error 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-nonce.auth-server-url=${quarkus.oidc.auth-server-url} +quarkus.oidc.tenant-nonce.client-id=quarkus-app +quarkus.oidc.tenant-nonce.credentials.secret=secret +quarkus.oidc.tenant-nonce.authentication.scopes=profile,email,phone +quarkus.oidc.tenant-nonce.authentication.extra-params.max-age=60 +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-javascript.auth-server-url=${quarkus.oidc.auth-server-url} quarkus.oidc.tenant-javascript.client-id=quarkus-app quarkus.oidc.tenant-javascript.credentials.secret=secret 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 f378f53cc8b65..9ae62850ca88b 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 @@ -196,7 +196,7 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception { Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); assertNull(stateCookie.getSameSite()); - verifyCodeVerifier(stateCookie, keycloakUrl); + verifyCodeVerifierAndNonce(stateCookie, keycloakUrl); assertTrue(endpointLocation.startsWith("https")); endpointLocation = "http" + endpointLocation.substring(5); @@ -259,7 +259,7 @@ public void testStateCookieIsPresentButStateParamNot() throws Exception { // State cookie is present Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); assertNull(stateCookie.getSameSite()); - verifyCodeVerifier(stateCookie, keycloakUrl); + 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())); @@ -313,7 +313,7 @@ public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception assertNotNull(endpointLocationUri.getRawQuery()); Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); - verifyCodeVerifier(stateCookie, keycloakUrl); + verifyCodeVerifierAndNonce(stateCookie, keycloakUrl); webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); assertNull(getStateCookie(webClient, "tenant-https_test")); @@ -350,6 +350,97 @@ public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception } } + @Test + public void testCodeFlowNonce() throws Exception { + try (final WebClient webClient = createWebClient()) { + webClient.getOptions().setRedirectEnabled(false); + + WebResponse webResponse = webClient + .loadWebResponse( + new WebRequest(URI.create("http://localhost:8081/tenant-nonce").toURL())); + String keycloakUrl = webResponse.getResponseHeaderValue("location"); + verifyLocationHeader(webClient, keycloakUrl, "tenant-nonce", "tenant-nonce", false); + + 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 + String endpointLocation = webResponse.getResponseHeaderValue("location"); + + Cookie stateCookie = getStateCookie(webClient, "tenant-nonce"); + verifyNonce(stateCookie, keycloakUrl); + + URI endpointLocationUri = URI.create(endpointLocation); + + webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); + assertEquals(302, webResponse.getStatusCode()); + assertNull(getStateCookie(webClient, "tenant-nonce")); + + String endpointLocationWithoutQuery = webResponse.getResponseHeaderValue("location"); + URI endpointLocationWithoutQueryUri = URI.create(endpointLocationWithoutQuery); + + page = webClient.getPage(endpointLocationWithoutQueryUri.toURL()); + assertEquals("tenant-nonce:reauthenticated", page.getBody().asNormalizedText()); + Cookie sessionCookie = getSessionCookie(webClient, "tenant-nonce"); + assertNotNull(sessionCookie); + webClient.getCookieManager().clearCookies(); + } + } + + @Test + public void testCodeFlowMissingNonce() throws Exception { + try (final WebClient webClient = createWebClient()) { + webClient.getOptions().setRedirectEnabled(false); + + WebResponse webResponse = webClient + .loadWebResponse( + new WebRequest(URI.create("http://localhost:8081/tenant-nonce").toURL())); + String keycloakUrl = webResponse.getResponseHeaderValue("location"); + verifyLocationHeader(webClient, keycloakUrl, "tenant-nonce", "tenant-nonce", false); + + 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 + String endpointLocation = webResponse.getResponseHeaderValue("location"); + + Cookie stateCookie = getStateCookie(webClient, "tenant-nonce"); + verifyNonce(stateCookie, keycloakUrl); + + URI endpointLocationUri = URI.create(endpointLocation); + + String cookieValueWithoutNonce = stateCookie.getValue().split("\\|")[0]; + Cookie stateCookieWithoutNonce = new Cookie(stateCookie.getDomain(), stateCookie.getName(), + cookieValueWithoutNonce); + webClient.getCookieManager().clearCookies(); + webClient.getCookieManager().addCookie(stateCookieWithoutNonce); + Cookie stateCookie2 = getStateCookie(webClient, "tenant-nonce"); + assertEquals(cookieValueWithoutNonce, stateCookie2.getValue()); + + webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); + assertEquals(401, webResponse.getStatusCode()); + assertNull(getStateCookie(webClient, "tenant-nonce")); + + webClient.getCookieManager().clearCookies(); + } + } + @Test public void testCodeFlowForceHttpsRedirectUriAndPkceMissingCodeVerifier() throws Exception { try (final WebClient webClient = createWebClient()) { @@ -379,7 +470,7 @@ public void testCodeFlowForceHttpsRedirectUriAndPkceMissingCodeVerifier() throws String endpointLocation = webResponse.getResponseHeaderValue("location"); Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); - verifyCodeVerifier(stateCookie, keycloakUrl); + verifyCodeVerifierAndNonce(stateCookie, keycloakUrl); assertTrue(endpointLocation.startsWith("https")); endpointLocation = "http" + endpointLocation.substring(5); @@ -405,16 +496,29 @@ public void testCodeFlowForceHttpsRedirectUriAndPkceMissingCodeVerifier() throws } } - private void verifyCodeVerifier(Cookie stateCookie, String keycloakUrl) throws Exception { + private void verifyCodeVerifierAndNonce(Cookie stateCookie, String keycloakUrl) throws Exception { String encodedState = stateCookie.getValue().split("\\|")[1]; byte[] secretBytes = "eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU".getBytes(StandardCharsets.UTF_8); SecretKey key = new SecretKeySpec(OidcUtils.getSha256Digest(secretBytes), "AES"); - String codeVerifier = OidcUtils.decryptJson(encodedState, key).getString("code_verifier"); + JsonObject json = OidcUtils.decryptJson(encodedState, key); + String codeVerifier = json.getString("code_verifier"); String codeChallenge = Base64.getUrlEncoder().withoutPadding() .encodeToString(OidcUtils.getSha256Digest(codeVerifier.getBytes(StandardCharsets.US_ASCII))); - assertTrue(keycloakUrl.contains("code_challenge=" + codeChallenge)); + String nonce = json.getString("nonce"); + assertTrue(keycloakUrl.contains("nonce=" + nonce)); + } + + private void verifyNonce(Cookie stateCookie, String keycloakUrl) throws Exception { + String encodedState = stateCookie.getValue().split("\\|")[1]; + + byte[] secretBytes = "eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU".getBytes(StandardCharsets.UTF_8); + SecretKey key = new SecretKeySpec(OidcUtils.getSha256Digest(secretBytes), "AES"); + JsonObject json = OidcUtils.decryptJson(encodedState, key); + assertFalse(keycloakUrl.contains("code_challenge=")); + String nonce = json.getString("nonce"); + assertTrue(keycloakUrl.contains("nonce=" + nonce)); } private void verifyLocationHeader(WebClient webClient, String loc, String tenant, String path, boolean httpsScheme) {