Merge pull request quarkusio#34499 from sberyozkin/oidc_generate_pkce…

…_secret

Simplify and improve OIDC PKCE secret initialization

docs/src/main/asciidoc/security-openid-connect-providers.adoc

@@ -343,6 +343,16 @@ quarkus.oidc.client-id=<Client ID>
 quarkus.oidc.credentials.secret=<Client Secret>
 ----
 
+[NOTE]
+====
+Twitter provider requires Proof Key for Code Exchange (PKCE) which is supported by the `quarkus.oidc.provider=twitter` declaration.
+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
+note that this secret should be 32 characters long, and an error will be reported if it is less than 16 characters long.
+====
+
 === Spotify
 
 Create a https://developer.spotify.com/documentation/general/guides/authorization/app-settings/[Spotify application]:

extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java

@@ -1,12 +1,24 @@
 package io.quarkus.oidc.test;
 
+import static org.awaitility.Awaitility.given;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.net.URI;
-
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.awaitility.core.ThrowingRunnable;
 import org.eclipse.microprofile.config.spi.ConfigSource;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -107,6 +119,8 @@ public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, Inte
 
  webClient.getCookieManager().clearCookies();
  }
+ checkPkceSecretGenerated();
+
  }
 
  private void useTenantConfigResolver() throws IOException, InterruptedException {
@@ -144,4 +158,37 @@ private WebClient createWebClient() {
  webClient.setCssErrorHandler(new SilentCssErrorHandler());
  return webClient;
  }
+
+ protected static void checkPkceSecretGenerated() {
+ AtomicBoolean checkPassed = new AtomicBoolean();
+ given().pollInterval(100, TimeUnit.MILLISECONDS)
+ .atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(new ThrowingRunnable() {
+ @Override
+ public void run() throws Throwable {
+ final Path logDirectory = Paths.get(".", "target");
+ Path accessLogFilePath = logDirectory.resolve("quarkus.log");
+ boolean fileExists = Files.exists(accessLogFilePath);
+ if (!fileExists) {
+ accessLogFilePath = logDirectory.resolve("target/quarkus.log");
+ fileExists = Files.exists(accessLogFilePath);
+ }
+ assertTrue(fileExists, "quarkus log is missing");
+
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(new ByteArrayInputStream(Files.readAllBytes(accessLogFilePath)),
+ StandardCharsets.UTF_8))) {
+ String line = null;
+ while ((line = reader.readLine()) != null) {
+ if (line.contains(
+ "Secret key for encrypting PKCE code verifier is missing, auto-generating it")) {
+ checkPassed.set(true);
+ }
+ }
+ }
+ }
+ });
+ assertTrue(checkPassed.get(), "Can not confirm Secret key for encrypting PKCE code verifier has been generated");
+ }
+
 }

extensions/oidc/deployment/src/test/resources/application-dev-mode.properties

@@ -6,5 +6,8 @@ quarkus.oidc.credentials.client-secret.provider.name=vault-secret-provider
 quarkus.oidc.credentials.client-secret.provider.key=secret-from-vault-typo
 quarkus.oidc.application-type=web-app
 quarkus.oidc.logout.path=/protected/logout
-
+quarkus.oidc.authentication.pkce-required=true
 quarkus.log.category."com.gargoylesoftware.htmlunit.javascript.host.css.CSSStyleSheet".level=FATAL
+quarkus.log.category."io.quarkus.oidc.runtime.TenantConfigContext".level=DEBUG
+quarkus.log.file.enable=true
+

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

@@ -915,8 +915,17 @@ public enum ResponseMode {
 
  /**
  * 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.
+ * This secret should be at least 32 characters long.
+ * <p/>
+ * 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 PKCE code verifier encryption secret if it is less than 32 characters
+ * long.
+ * </p>
+ * The secret will be auto-generated if it remains uninitialized after checking all of these properties.
+ * <p/>
+ * Error will be reported if the secret length is less than 16 characters.
  */
  @ConfigItem
  public Optional<String> pkceSecret = Optional.empty();

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

@@ -11,7 +11,6 @@
 import io.quarkus.oidc.OIDCException;
 import io.quarkus.oidc.OidcTenantConfig;
 import io.quarkus.oidc.common.runtime.OidcCommonUtils;
-import io.smallrye.jwt.util.KeyUtils;
 
 public class TenantConfigContext {
  private static final Logger LOG = Logger.getLogger(TenantConfigContext.class);
@@ -54,15 +53,41 @@ public TenantConfigContext(OidcProvider client, OidcTenantConfig config, boolean
 
  private static SecretKey createPkceSecretKey(OidcTenantConfig config) {
  if (config.authentication.pkceRequired.orElse(false)) {
- String pkceSecret = config.authentication.pkceSecret
- .orElse(OidcCommonUtils.clientSecret(config.credentials));
- if (pkceSecret == null) {
- throw new RuntimeException("Secret key for encrypting PKCE code verifier is missing");
+ 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, "
+ + "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;
+ }
  }
- if (pkceSecret.length() != 32) {
- throw new RuntimeException("Secret key for encrypting PKCE code verifier must be 32 characters long");
+ try {
+ if (pkceSecret == null) {
+ LOG.debug("Secret key for encrypting PKCE code verifier is missing, auto-generating it");
+ SecretKey key = generateSecretKey();
+ return key;
+ }
+ byte[] secretBytes = pkceSecret.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"
+ + " for the strongest state cookie encryption to be produced."
+ + " Please update 'quarkus.oidc.authentication.pkce-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");
+ } else {
+ LOG.debug(errorMessage);
+ }
+ }
+ return new SecretKeySpec(OidcUtils.getSha256Digest(secretBytes), "AES");
+ } catch (Exception ex) {
+ throw new OIDCException(ex);
  }
- return KeyUtils.createSecretKeyFromSecret(pkceSecret);
  }
  return null;
  }
@@ -75,19 +100,12 @@ private static SecretKey createTokenEncSecretKey(OidcTenantConfig config) {
  } else {
  LOG.debug("'quarkus.oidc.token-state-manager.encryption-secret' is not configured, "
  + "trying to use the configured client secret");
- encSecret = OidcCommonUtils.clientSecret(config.credentials);
- if (encSecret == null) {
- LOG.debug("Client secret is not configured, "
- + "trying to use the configured 'client_jwt_secret' secret");
- encSecret = OidcCommonUtils.jwtSecret(config.credentials);
- }
+ encSecret = fallbackToClientSecret(config);
  }
  try {
  if (encSecret == null) {
  LOG.warn("Secret key for encrypting tokens in a session cookie is missing, auto-generating it");
- KeyGenerator keyGenerator = KeyGenerator.getInstance("AES");
- keyGenerator.init(256);
- return keyGenerator.generateKey();
+ return generateSecretKey();
  }
  byte[] secretBytes = encSecret.getBytes(StandardCharsets.UTF_8);
  if (secretBytes.length < 32) {
@@ -111,6 +129,22 @@ private static SecretKey createTokenEncSecretKey(OidcTenantConfig config) {
  return null;
  }
 
+ private static String fallbackToClientSecret(OidcTenantConfig config) {
+ String encSecret = OidcCommonUtils.clientSecret(config.credentials);
+ if (encSecret == null) {
+ LOG.debug("Client secret is not configured, "
+ + "trying to use the configured 'client_jwt_secret' secret");
+ encSecret = OidcCommonUtils.jwtSecret(config.credentials);
+ }
+ return encSecret;
+ }
+
+ private static SecretKey generateSecretKey() throws Exception {
+ KeyGenerator keyGenerator = KeyGenerator.getInstance("AES");
+ keyGenerator.init(256);
+ return keyGenerator.generateKey();
+ }
+
  public OidcTenantConfig getOidcTenantConfig() {
  return oidcConfig;
  }

integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java

@@ -38,7 +38,6 @@
 import io.quarkus.test.junit.QuarkusTest;
 import io.quarkus.test.keycloak.client.KeycloakTestClient;
 import io.restassured.RestAssured;
-import io.smallrye.jwt.util.KeyUtils;
 import io.vertx.core.json.JsonObject;
 
 /**
@@ -358,8 +357,9 @@ public void testCodeFlowForceHttpsRedirectUriAndPkceMissingCodeVerifier() throws
  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");
+ byte[] secretBytes = "eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU".getBytes(StandardCharsets.UTF_8);
+ SecretKey key = new SecretKeySpec(OidcUtils.getSha256Digest(secretBytes), "AES");
+ String codeVerifier = OidcUtils.decryptJson(encodedState, key).getString("code_verifier");
  String codeChallenge = Base64.getUrlEncoder().withoutPadding()
  .encodeToString(OidcUtils.getSha256Digest(codeVerifier.getBytes(StandardCharsets.US_ASCII)));