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)));