From 0516308cc65d24cb0f3f4cf09247077705d5c73d Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Mon, 31 Jan 2022 18:41:06 +0000 Subject: [PATCH] Support for OIDC Proof Of Key for Code Exchange (PKCE) --- ...ity-openid-connect-web-authentication.adoc | 18 +++ .../oidc/common/runtime/OidcConstants.java | 6 + .../io/quarkus/oidc/OidcTenantConfig.java | 30 ++++ .../runtime/CodeAuthenticationMechanism.java | 149 ++++++++++++++---- .../runtime/CodeAuthenticationStateBean.java | 29 ++++ .../io/quarkus/oidc/runtime/OidcProvider.java | 4 +- .../oidc/runtime/OidcProviderClient.java | 5 +- .../io/quarkus/oidc/runtime/OidcUtils.java | 32 ++++ .../quarkus/oidc/runtime/PkceStateBean.java | 25 +++ .../oidc/runtime/TenantConfigContext.java | 27 ++++ .../src/main/resources/application.properties | 2 + .../io/quarkus/it/keycloak/CodeFlowTest.java | 90 ++++++++++- 12 files changed, 383 insertions(+), 34 deletions(-) create mode 100644 extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java create mode 100644 extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/PkceStateBean.java diff --git a/docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc b/docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc index 8d8cddbe73cc2..c3697e556c433 100644 --- a/docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc +++ b/docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc @@ -592,6 +592,24 @@ public class CustomTokenStateManager implements TokenStateManager { } ---- +=== Proof Of Key for Code Exchange (PKCE) + +link:https://datatracker.ietf.org/doc/html/rfc7636[Proof Of Key for Code Exchange] (PKCE) minimizes the risk of the authorization code interception. + +While `PKCE` is of primary importance to the public OpenId Connect clients (such as the SPA scripts running in a browser), it can also provide an extra level of protection to Quarkus OIDC `web-app` applications which are confidential OpenId Connect clients capable of securely storing the client secret and using it to exchange the code for the tokens. + +If can enable `PKCE` for your OIDC `web-app` endpoint with a `quarkus.oidc.authentication.pkce-required` property and a 32 characters long secret, for example: + +[source, properties] +---- +quarkus.oidc.authentication.pkce-required +quarkus.oidc.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU +---- + +If you already have a 32 character long client secret then `quarkus.oidc.authentication.pkce-secret` does not have to be set unless you prefer to use a different secret key. + +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 OpenId Connect Provider to authenticate. The `code_verifier` will be 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. The provider will fail the code exchange if a `SHA256` digest of the `code_verifier` does not match the `code_challenge` provided during the authentication request. + === Listening to important authentication events One can register `@ApplicationScoped` bean which will observe important OIDC authentication events. The listener will be updated when a user has logged in for the first time or re-authenticated, as well as when the session has been refreshed. More events may be reported in the future. For example: 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 bdaa51b1a4bc8..d32f9e0900d9f 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 @@ -51,4 +51,10 @@ public final class OidcConstants { public static final String EXPIRES_IN = "expires_in"; public static final String REFRESH_EXPIRES_IN = "refresh_expires_in"; + + public static final String PKCE_CODE_VERIFIER = "code_verifier"; + public static final String PKCE_CODE_CHALLENGE = "code_challenge"; + + public static final String PKCE_CODE_CHALLENGE_METHOD = "code_challenge_method"; + public static final String PKCE_CODE_CHALLENGE_S256 = "S256"; } 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 e093d047523f3..cd05a0c043d7c 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 @@ -589,6 +589,36 @@ public static class Authentication { @ConfigItem(defaultValueDocumentation = "true") public Optional idTokenRequired = Optional.empty(); + /** + * Requires that a Proof Key for Code Exchange (PKCE) is used. + */ + @ConfigItem(defaultValueDocumentation = "false") + public Optional pkceRequired = Optional.empty(); + + /** + * Secret which will be used to encrypt a Proof Key for Code Exchange (PKCE) code verifier in the code flow state. + * This secret must be set if PKCE is required but no client secret is set. + * The length of the secret which will be used to encrypt the code verifier must be 32 characters long. + */ + @ConfigItem + public Optional pkceSecret = Optional.empty(); + + public Optional isPkceRequired() { + return pkceRequired; + } + + public void setPkceRequired(boolean pkceRequired) { + this.pkceRequired = Optional.of(pkceRequired); + } + + public Optional getPkceSecret() { + return pkceSecret; + } + + public void setPkceSecret(String pkceSecret) { + this.pkceSecret = Optional.of(pkceSecret); + } + public Optional getErrorPath() { return errorPath; } 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 205c5a2114019..ced3d368ada55 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 @@ -4,7 +4,11 @@ import static io.quarkus.oidc.runtime.OidcIdentityProvider.REFRESH_TOKEN_GRANT_RESPONSE; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Base64; +import java.util.Base64.Encoder; import java.util.Collections; import java.util.List; import java.util.Map; @@ -50,6 +54,7 @@ public class CodeAuthenticationMechanism extends AbstractOidcAuthenticationMecha static final String COOKIE_DELIM = "|"; static final Pattern COOKIE_PATTERN = Pattern.compile("\\" + COOKIE_DELIM); static final String SESSION_MAX_AGE_PARAM = "session-max-age"; + static final String STATE_COOKIE_RESTORE_PATH = "restore-path"; static final Uni VOID_UNI = Uni.createFrom().voidItem(); static final Integer MAX_COOKIE_VALUE_LENGTH = 4096; static final String NO_OIDC_COOKIES_AVAILABLE = "no_oidc_cookies"; @@ -59,6 +64,7 @@ public class CodeAuthenticationMechanism extends AbstractOidcAuthenticationMecha private final BlockingTaskRunner createTokenStateRequestContext = new BlockingTaskRunner(); private final BlockingTaskRunner getTokenStateRequestContext = new BlockingTaskRunner(); + private final SecureRandom secureRandom = new SecureRandom(); public Uni authenticate(RoutingContext context, IdentityProviderManager identityProviderManager, OidcTenantConfig oidcTenantConfig) { @@ -82,8 +88,9 @@ public Uni apply(TenantConfigContext tenantContext) { // if the state cookie is available then try to complete the code flow and start a new session if (stateCookie != null) { String[] parsedStateCookieValue = COOKIE_PATTERN.split(stateCookie.getValue()); + OidcUtils.removeCookie(context, oidcTenantConfig, stateCookie.getName()); + if (!isStateValid(context, parsedStateCookieValue[0])) { - OidcUtils.removeCookie(context, oidcTenantConfig, stateCookie.getName()); return Uni.createFrom().failure(new AuthenticationCompletionException()); } @@ -95,7 +102,7 @@ public Uni apply(TenantConfigContext tenantContext) { .transformToUni(new Function>() { @Override public Uni apply(TenantConfigContext tenantContext) { - return performCodeFlow(identityProviderManager, context, tenantContext, code, stateCookie, + return performCodeFlow(identityProviderManager, context, tenantContext, code, parsedStateCookieValue); } }); @@ -123,7 +130,6 @@ public Uni apply(TenantConfigContext tenantContext) { } } 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()); } } @@ -295,9 +301,22 @@ && isRedirectFromProvider(context, configContext)) { codeFlowParams.append(AMP).append(OidcConstants.CODE_FLOW_REDIRECT_URI).append(EQ) .append(OidcCommonUtils.urlEncode(redirectUriParam)); + // pkce + PkceStateBean pkceStateBean = createPkceStateBean(configContext); + // state codeFlowParams.append(AMP).append(OidcConstants.CODE_FLOW_STATE).append(EQ) - .append(generateCodeFlowState(context, configContext, redirectPath)); + .append(generateCodeFlowState(context, configContext, redirectPath, + pkceStateBean != null ? pkceStateBean.getCodeVerifier() : null)); + + if (pkceStateBean != null) { + codeFlowParams + .append(AMP).append(OidcConstants.PKCE_CODE_CHALLENGE).append(EQ) + .append(pkceStateBean.getCodeChallenge()); + codeFlowParams + .append(AMP).append(OidcConstants.PKCE_CODE_CHALLENGE_METHOD).append(EQ) + .append(OidcConstants.PKCE_CODE_CHALLENGE_S256); + } // extra redirect parameters, see https://openid.net/specs/openid-connect-core-1_0.html#AuthRequests addExtraParamsToUri(codeFlowParams, configContext.oidcConfig.authentication.getExtraParams()); @@ -321,31 +340,58 @@ private boolean isRedirectFromProvider(RoutingContext context, TenantConfigConte return referer != null && referer.startsWith(configContext.provider.getMetadata().getAuthorizationUri()); } + private PkceStateBean createPkceStateBean(TenantConfigContext configContext) { + if (configContext.oidcConfig.authentication.pkceRequired.orElse(false)) { + PkceStateBean bean = new PkceStateBean(); + + Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + + // code verifier + byte[] codeVerifierBytes = new byte[32]; + secureRandom.nextBytes(codeVerifierBytes); + String codeVerifier = encoder.encodeToString(codeVerifierBytes); + bean.setCodeVerifier(codeVerifier); + + // code challenge + try { + byte[] codeChallengeBytes = OidcUtils.getSha256Digest(codeVerifier.getBytes(StandardCharsets.ISO_8859_1)); + String codeChallenge = encoder.encodeToString(codeChallengeBytes); + bean.setCodeChallenge(codeChallenge); + } catch (Exception ex) { + throw new AuthenticationFailedException(ex); + } + + return bean; + } + return null; + } + private Uni performCodeFlow(IdentityProviderManager identityProviderManager, - RoutingContext context, TenantConfigContext configContext, String code, Cookie stateCookie, - String[] parsedStateCookieValue) { + RoutingContext context, TenantConfigContext configContext, String code, String[] parsedStateCookieValue) { String userPath = null; String userQuery = null; // This is an original redirect from IDP, check if the original request path and query need to be restored - if (parsedStateCookieValue.length == 2) { - int userQueryIndex = parsedStateCookieValue[1].indexOf("?"); + CodeAuthenticationStateBean stateBean = getCodeAuthenticationBean(parsedStateCookieValue, configContext); + if (stateBean != null && stateBean.getRestorePath() != null) { + String restorePath = stateBean.getRestorePath(); + int userQueryIndex = restorePath.indexOf("?"); if (userQueryIndex >= 0) { - userPath = parsedStateCookieValue[1].substring(0, userQueryIndex); - if (userQueryIndex + 1 < parsedStateCookieValue[1].length()) { - userQuery = parsedStateCookieValue[1].substring(userQueryIndex + 1); + userPath = restorePath.substring(0, userQueryIndex); + if (userQueryIndex + 1 < restorePath.length()) { + userQuery = restorePath.substring(userQueryIndex + 1); } } else { - userPath = parsedStateCookieValue[1]; + userPath = restorePath; } } - OidcUtils.removeCookie(context, configContext.oidcConfig, stateCookie.getName()); final String finalUserPath = userPath; final String finalUserQuery = userQuery; - Uni codeFlowTokensUni = getCodeFlowTokensUni(context, configContext, code); + Uni codeFlowTokensUni = getCodeFlowTokensUni(context, configContext, code, + stateBean != null ? stateBean.getCodeVerifier() : null); return codeFlowTokensUni .onItemOrFailure() @@ -426,9 +472,33 @@ public Throwable apply(Throwable tInner) { } + private CodeAuthenticationStateBean getCodeAuthenticationBean(String[] parsedStateCookieValue, + TenantConfigContext configContext) { + if (parsedStateCookieValue.length == 2) { + CodeAuthenticationStateBean bean = new CodeAuthenticationStateBean(); + if (!configContext.oidcConfig.authentication.pkceRequired.orElse(false)) { + bean.setRestorePath(parsedStateCookieValue[1]); + return bean; + } + + JsonObject json = null; + try { + json = OidcUtils.decryptJson(parsedStateCookieValue[1], configContext.getPkceSecretKey()); + } catch (Exception ex) { + LOG.tracef("State cookie value can not be decrypted for the %s tenant", + configContext.oidcConfig.tenantId.get()); + throw new AuthenticationFailedException(ex); + } + bean.setRestorePath(json.getString(STATE_COOKIE_RESTORE_PATH)); + bean.setCodeVerifier(json.getString(OidcConstants.PKCE_CODE_VERIFIER)); + return bean; + } + return null; + } + private String generateInternalIdToken(OidcTenantConfig oidcConfig) { return Jwt.claims().jws().header(INTERNAL_IDTOKEN_HEADER, true) - .sign(KeyUtils.createSecretKeyFromSecret(oidcConfig.credentials.secret.get())); + .sign(KeyUtils.createSecretKeyFromSecret(OidcCommonUtils.clientSecret(oidcConfig.credentials))); } private Uni processSuccessfulAuthentication(RoutingContext context, @@ -499,28 +569,53 @@ private String getRedirectPath(TenantConfigContext configContext, RoutingContext } private String generateCodeFlowState(RoutingContext context, TenantConfigContext configContext, - String redirectPath) { + String redirectPath, String pkceCodeVerifier) { String uuid = UUID.randomUUID().toString(); String cookieValue = uuid; Authentication auth = configContext.oidcConfig.getAuthentication(); boolean restorePath = auth.isRestorePathAfterRedirect() || !auth.redirectPath.isPresent(); - if (restorePath) { - String requestQuery = context.request().query(); - String requestPath = !redirectPath.equals(context.request().path()) || requestQuery != null - ? context.request().path() - : ""; - if (requestQuery != null) { - requestPath += ("?" + requestQuery); + if (restorePath || pkceCodeVerifier != null) { + CodeAuthenticationStateBean extraStateValue = new CodeAuthenticationStateBean(); + if (restorePath) { + String requestQuery = context.request().query(); + String requestPath = !redirectPath.equals(context.request().path()) || requestQuery != null + ? context.request().path() + : ""; + if (requestQuery != null) { + requestPath += ("?" + requestQuery); + } + if (!requestPath.isEmpty()) { + extraStateValue.setRestorePath(requestPath); + } } - if (!requestPath.isEmpty()) { - cookieValue += (COOKIE_DELIM + requestPath); + extraStateValue.setCodeVerifier(pkceCodeVerifier); + if (!extraStateValue.isEmpty()) { + cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext)); } } createCookie(context, configContext.oidcConfig, getStateCookieName(configContext.oidcConfig), cookieValue, 60 * 30); return uuid; } + private String encodeExtraStateValue(CodeAuthenticationStateBean extraStateValue, TenantConfigContext configContext) { + if (extraStateValue.getCodeVerifier() != null) { + JsonObject json = new JsonObject(); + json.put(OidcConstants.PKCE_CODE_VERIFIER, extraStateValue.getCodeVerifier()); + if (extraStateValue.getRestorePath() != null) { + json.put(STATE_COOKIE_RESTORE_PATH, extraStateValue.getRestorePath()); + } + try { + return OidcUtils.encryptJson(json, configContext.getPkceSecretKey()); + } catch (Exception ex) { + throw new AuthenticationFailedException(ex); + } + } else { + return extraStateValue.getRestorePath(); + } + + } + private String generatePostLogoutState(RoutingContext context, TenantConfigContext configContext) { OidcUtils.removeCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext.oidcConfig)); return createCookie(context, configContext.oidcConfig, getPostLogoutCookieName(configContext.oidcConfig), @@ -638,14 +733,14 @@ private Uni refreshTokensUni(TenantConfigContext config } private Uni getCodeFlowTokensUni(RoutingContext context, TenantConfigContext configContext, - String code) { + String code, String codeVerifier) { // 'redirect_uri': typically it must match the 'redirect_uri' query parameter which was used during the code request. String redirectPath = getRedirectPath(configContext, context); String redirectUriParam = buildUri(context, isForceHttps(configContext.oidcConfig), redirectPath); LOG.debugf("Token request redirect_uri parameter: %s", redirectUriParam); - return configContext.provider.getCodeFlowTokens(code, redirectUriParam); + return configContext.provider.getCodeFlowTokens(code, redirectUriParam, codeVerifier); } private String buildLogoutRedirectUri(TenantConfigContext configContext, String idToken, RoutingContext context) { 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 new file mode 100644 index 0000000000000..9c9e5a9765b23 --- /dev/null +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationStateBean.java @@ -0,0 +1,29 @@ +package io.quarkus.oidc.runtime; + +public class CodeAuthenticationStateBean { + + private String restorePath; + + private String codeVerifier; + + public String getRestorePath() { + return restorePath; + } + + public void setRestorePath(String restorePath) { + this.restorePath = restorePath; + } + + public String getCodeVerifier() { + return codeVerifier; + } + + public void setCodeVerifier(String codeVerifier) { + this.codeVerifier = codeVerifier; + } + + public boolean isEmpty() { + return this.restorePath == null && this.codeVerifier == null; + } + +} diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java index def0da2c60f1d..9a77df79537d7 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java @@ -197,8 +197,8 @@ public Uni getUserInfo(String accessToken) { return client.getUserInfo(accessToken); } - public Uni getCodeFlowTokens(String code, String redirectUri) { - return client.getAuthorizationCodeTokens(code, redirectUri); + public Uni getCodeFlowTokens(String code, String redirectUri, String codeVerifier) { + return client.getAuthorizationCodeTokens(code, redirectUri, codeVerifier); } public Uni refreshTokens(String refreshToken) { diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java index d5bcb6c7a4638..c74b325f5efa1 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java @@ -87,11 +87,14 @@ public OidcTenantConfig getOidcConfig() { return oidcConfig; } - public Uni getAuthorizationCodeTokens(String code, String redirectUri) { + public Uni getAuthorizationCodeTokens(String code, String redirectUri, String codeVerifier) { MultiMap codeGrantParams = new MultiMap(io.vertx.core.MultiMap.caseInsensitiveMultiMap()); codeGrantParams.add(OidcConstants.GRANT_TYPE, OidcConstants.AUTHORIZATION_CODE); codeGrantParams.add(OidcConstants.CODE_FLOW_CODE, code); codeGrantParams.add(OidcConstants.CODE_FLOW_REDIRECT_URI, redirectUri); + if (codeVerifier != null) { + codeGrantParams.add(OidcConstants.PKCE_CODE_VERIFIER, codeVerifier); + } return getHttpResponse(metadata.getTokenUri(), codeGrantParams).transform(resp -> getAuthorizationCodeTokens(resp)); } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java index 2c9b9b7e4f120..e018019933129 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java @@ -1,6 +1,8 @@ package io.quarkus.oidc.runtime; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; @@ -10,9 +12,13 @@ import java.util.StringTokenizer; import java.util.regex.Pattern; +import javax.crypto.SecretKey; + import org.eclipse.microprofile.jwt.Claims; import org.eclipse.microprofile.jwt.JsonWebToken; import org.jboss.logging.Logger; +import org.jose4j.jwa.AlgorithmConstraints; +import org.jose4j.jwe.JsonWebEncryption; import org.jose4j.jwt.JwtClaims; import org.jose4j.jwt.consumer.InvalidJwtException; @@ -31,6 +37,8 @@ import io.quarkus.security.identity.AuthenticationRequestContext; import io.quarkus.security.runtime.QuarkusSecurityIdentity; import io.quarkus.security.runtime.QuarkusSecurityIdentity.Builder; +import io.smallrye.jwt.algorithm.ContentEncryptionAlgorithm; +import io.smallrye.jwt.algorithm.KeyEncryptionAlgorithm; import io.smallrye.mutiny.Uni; import io.vertx.core.http.impl.ServerCookie; import io.vertx.core.json.JsonArray; @@ -380,4 +388,28 @@ static OidcTenantConfig resolveProviderConfig(OidcTenantConfig oidcTenantConfig) } } + + public static byte[] getSha256Digest(byte[] value) throws NoSuchAlgorithmException { + MessageDigest sha256 = MessageDigest.getInstance("SHA-256"); + sha256.update(value); + return sha256.digest(); + } + + public static String encryptJson(JsonObject json, SecretKey key) throws Exception { + JsonWebEncryption jwe = new JsonWebEncryption(); + jwe.setAlgorithmHeaderValue(KeyEncryptionAlgorithm.A256KW.getAlgorithm()); + jwe.setEncryptionMethodHeaderParameter(ContentEncryptionAlgorithm.A256GCM.getAlgorithm()); + jwe.setKey(key); + jwe.setPlaintext(json.encode()); + return jwe.getCompactSerialization(); + } + + public static JsonObject decryptJson(String jweString, SecretKey key) throws Exception { + JsonWebEncryption jwe = new JsonWebEncryption(); + jwe.setAlgorithmConstraints(new AlgorithmConstraints(AlgorithmConstraints.ConstraintType.PERMIT, + KeyEncryptionAlgorithm.A256KW.getAlgorithm())); + jwe.setKey(key); + jwe.setCompactSerialization(jweString); + return new JsonObject(jwe.getPlaintextString()); + } } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/PkceStateBean.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/PkceStateBean.java new file mode 100644 index 0000000000000..1a547e6411eb7 --- /dev/null +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/PkceStateBean.java @@ -0,0 +1,25 @@ +package io.quarkus.oidc.runtime; + +public class PkceStateBean { + + private String codeChallenge; + + private String codeVerifier; + + public String getCodeVerifier() { + return codeVerifier; + } + + public void setCodeVerifier(String codeVerifier) { + this.codeVerifier = codeVerifier; + } + + public String getCodeChallenge() { + return codeChallenge; + } + + public void setCodeChallenge(String codeChallenge) { + this.codeChallenge = codeChallenge; + } + +} 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 984c5d3703bbf..e2fe23c97f087 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 @@ -1,6 +1,10 @@ package io.quarkus.oidc.runtime; +import javax.crypto.SecretKey; + import io.quarkus.oidc.OidcTenantConfig; +import io.quarkus.oidc.common.runtime.OidcCommonUtils; +import io.smallrye.jwt.util.KeyUtils; public class TenantConfigContext { @@ -14,6 +18,11 @@ public class TenantConfigContext { */ final OidcTenantConfig oidcConfig; + /** + * PKCE Secret Key + */ + private final SecretKey pkceSecretKey; + final boolean ready; public TenantConfigContext(OidcProvider client, OidcTenantConfig config) { @@ -24,9 +33,27 @@ public TenantConfigContext(OidcProvider client, OidcTenantConfig config, boolean this.provider = client; this.oidcConfig = config; this.ready = ready; + + pkceSecretKey = createPkceSecretKey(config); + } + + private static SecretKey createPkceSecretKey(OidcTenantConfig config) { + if (config.authentication.pkceRequired.orElse(false)) { + String pkceSecret = config.authentication.pkceSecret + .orElse(OidcCommonUtils.clientSecret(config.credentials)); + if (pkceSecret.length() < 32) { + throw new RuntimeException("Secret key for encrypting PKCE code verifier must be at least 32 characters long"); + } + return KeyUtils.createSecretKeyFromSecret(pkceSecret); + } + return null; } public OidcTenantConfig getOidcTenantConfig() { return oidcConfig; } + + public SecretKey getPkceSecretKey() { + return pkceSecretKey; + } } 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 c5362ae070bff..ecad5684f425f 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -97,6 +97,8 @@ 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.error-path=/tenant-https/error +quarkus.oidc.tenant-https.authentication.pkce-required=true +quarkus.oidc.tenant-https.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU quarkus.oidc.tenant-javascript.auth-server-url=${keycloak.url}/realms/quarkus quarkus.oidc.tenant-javascript.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 ada9c0f3fdb0c..f35066ba689cc 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 @@ -10,7 +10,9 @@ import java.io.IOException; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.Base64; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -30,6 +32,7 @@ import io.quarkus.test.common.QuarkusTestResource; import io.quarkus.test.junit.QuarkusTest; import io.restassured.RestAssured; +import io.smallrye.jwt.util.KeyUtils; /** * @author Pedro Igor @@ -150,7 +153,7 @@ public void testCodeFlowScopeErrorWithErrorPage() throws IOException { } @Test - public void testCodeFlowForceHttpsRedirectUri() throws IOException { + public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception { try (final WebClient webClient = createWebClient()) { webClient.getOptions().setRedirectEnabled(false); // Verify X-Forwarded-Prefix header is checked during all the redirects @@ -176,6 +179,10 @@ public void testCodeFlowForceHttpsRedirectUri() throws IOException { // This is a redirect from the OIDC server to the endpoint String endpointLocation = webResponse.getResponseHeaderValue("location"); + + Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); + verifyCodeVerifier(stateCookie, keycloakUrl); + assertTrue(endpointLocation.startsWith("https")); endpointLocation = "http" + endpointLocation.substring(5); assertTrue(endpointLocation.contains("/xforwarded")); @@ -185,6 +192,7 @@ public void testCodeFlowForceHttpsRedirectUri() throws IOException { webClient.addRequestHeader("X-Forwarded-Prefix", "/xforwarded"); webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); + assertNull(getStateCookie(webClient, "tenant-https_test")); // This is a redirect from quarkus-oidc which drops the query parameters String endpointLocationWithoutQuery = webResponse.getResponseHeaderValue("location"); @@ -205,7 +213,7 @@ public void testCodeFlowForceHttpsRedirectUri() throws IOException { } @Test - public void testCodeFlowForceHttpsRedirectUriWithQuery() throws IOException { + public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception { try (final WebClient webClient = createWebClient()) { webClient.getOptions().setRedirectEnabled(false); @@ -234,7 +242,11 @@ public void testCodeFlowForceHttpsRedirectUriWithQuery() throws IOException { URI endpointLocationUri = URI.create(endpointLocation); assertNotNull(endpointLocationUri.getRawQuery()); + Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); + verifyCodeVerifier(stateCookie, keycloakUrl); + webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); + assertNull(getStateCookie(webClient, "tenant-https_test")); // This is a redirect from quarkus-oidc which drops the code and state query parameters String endpointLocationWithoutQuery = webResponse.getResponseHeaderValue("location"); @@ -252,9 +264,75 @@ public void testCodeFlowForceHttpsRedirectUriWithQuery() throws IOException { } } - private void verifyLocationHeader(WebClient webClient, String loc, String tenant, String path, boolean forceHttps) { + @Test + public void testCodeFlowForceHttpsRedirectUriAndPkceMissingCodeVerifier() throws Exception { + try (final WebClient webClient = createWebClient()) { + webClient.getOptions().setRedirectEnabled(false); + // Verify X-Forwarded-Prefix header is checked during all the redirects + webClient.addRequestHeader("X-Forwarded-Prefix", "/xforwarded"); + + 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", "xforwarded%2Ftenant-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 + String endpointLocation = webResponse.getResponseHeaderValue("location"); + + Cookie stateCookie = getStateCookie(webClient, "tenant-https_test"); + verifyCodeVerifier(stateCookie, keycloakUrl); + + assertTrue(endpointLocation.startsWith("https")); + endpointLocation = "http" + endpointLocation.substring(5); + assertTrue(endpointLocation.contains("/xforwarded")); + endpointLocation = endpointLocation.replace("/xforwarded", ""); + URI endpointLocationUri = URI.create(endpointLocation); + assertNotNull(endpointLocationUri.getRawQuery()); + webClient.addRequestHeader("X-Forwarded-Prefix", "/xforwarded"); + + String cookieValueWithoutCodeVerifier = stateCookie.getValue().split("\\|")[0]; + Cookie stateCookieWithoutCodeVerifier = new Cookie(stateCookie.getDomain(), stateCookie.getName(), + cookieValueWithoutCodeVerifier); + webClient.getCookieManager().clearCookies(); + webClient.getCookieManager().addCookie(stateCookieWithoutCodeVerifier); + Cookie stateCookie2 = getStateCookie(webClient, "tenant-https_test"); + assertEquals(cookieValueWithoutCodeVerifier, stateCookie2.getValue()); + + webResponse = webClient.loadWebResponse(new WebRequest(endpointLocationUri.toURL())); + assertEquals(401, webResponse.getStatusCode()); + assertNull(getStateCookie(webClient, "tenant-https_test")); + + webClient.getCookieManager().clearCookies(); + } + } + + private void verifyCodeVerifier(Cookie stateCookie, String keycloakUrl) throws Exception { + String encodedState = stateCookie.getValue().split("\\|")[1]; + + String codeVerifier = OidcUtils.decryptJson(encodedState, + KeyUtils.createSecretKeyFromSecret("eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU")).getString("code_verifier"); + String codeChallenge = Base64.getUrlEncoder().withoutPadding() + .encodeToString(OidcUtils.getSha256Digest(codeVerifier.getBytes(StandardCharsets.US_ASCII))); + + assertTrue(keycloakUrl.contains("code_challenge=" + codeChallenge)); + } + + private void verifyLocationHeader(WebClient webClient, String loc, String tenant, String path, boolean httpsScheme) { assertTrue(loc.startsWith("http://localhost:8180/auth/realms/quarkus/protocol/openid-connect/auth")); - String scheme = forceHttps ? "https" : "http"; + String scheme = httpsScheme ? "https" : "http"; assertTrue(loc.contains("redirect_uri=" + scheme + "%3A%2F%2Flocalhost%3A8081%2F" + path)); assertTrue(loc.contains("state=" + getStateCookieStateParam(webClient, tenant))); assertTrue(loc.contains("scope=openid+profile+email+phone")); @@ -262,6 +340,10 @@ private void verifyLocationHeader(WebClient webClient, String loc, String tenant assertTrue(loc.contains("client_id=quarkus-app")); assertTrue(loc.contains("max-age=60")); + if (httpsScheme) { + assertTrue(loc.contains("code_challenge")); + assertTrue(loc.contains("code_challenge_method=S256")); + } } @Test