oscerd commented on code in PR #29058:
URL: https://github.com/apache/camel/pull/29058#discussion_r4471089067
##########
components/camel-keycloak/src/main/java/org/apache/camel/component/keycloak/security/KeycloakPublicKeyResolver.java:
##########
@@ -197,7 +238,7 @@ public String getExpectedIssuer() {
* Clears the key cache.
*/
public void clearCache() {
- keyCache.clear();
- lastRefreshTime = 0;
+ keyCache = Map.of();
+ lastRefreshAttemptNanos = 0;
}
Review Comment:
Applied in c8b3e710bedd: `clearCache()` takes the lock around both writes.
_Claude Code on behalf of oscerd_
##########
components/camel-keycloak/src/test/java/org/apache/camel/component/keycloak/security/KeycloakJwksCacheStampedeTest.java:
##########
@@ -0,0 +1,285 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.keycloak.security;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.interfaces.RSAPublicKey;
+import java.util.Base64;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import com.sun.net.httpserver.HttpServer;
+import org.apache.camel.CamelAuthorizationException;
+import org.apache.camel.CamelContext;
+import org.apache.camel.Exchange;
+import org.apache.camel.impl.DefaultCamelContext;
+import org.apache.camel.support.DefaultExchange;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.keycloak.jose.jws.JWSBuilder;
+import org.keycloak.representations.AccessToken;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.awaitility.Awaitility.await;
+
+/**
+ * Tests verifying that {@link KeycloakPublicKeyResolver} and {@link
KeycloakSecurityProcessor} rate limit remote JWKS
+ * refreshes for unknown key IDs, preventing cache stampede.
+ */
+class KeycloakJwksCacheStampedeTest {
+
+ private HttpServer server;
+ private int serverPort;
+ private AtomicInteger jwksRequestCount;
+ private KeyPair keyPair;
+ private String jwksResponse;
+ private CamelContext context;
+
+ @BeforeEach
+ void setUp() throws Exception {
+ context = new DefaultCamelContext();
+ context.start();
+
+ KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+ keyGen.initialize(2048);
+ keyPair = keyGen.generateKeyPair();
+ RSAPublicKey rsaPub = (RSAPublicKey) keyPair.getPublic();
+
+ jwksResponse = "{\"keys\":[" + String.format(
+
"{\"kty\":\"RSA\",\"use\":\"sig\",\"kid\":\"valid-key-1\",\"n\":\"%s\",\"e\":\"%s\"}",
+ b64url(rsaPub.getModulus()),
b64url(rsaPub.getPublicExponent())) + "]}";
+
+ jwksRequestCount = new AtomicInteger();
+ server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+
server.createContext("/realms/test-realm/protocol/openid-connect/certs",
httpExchange -> {
+ jwksRequestCount.incrementAndGet();
+ byte[] bytes = jwksResponse.getBytes(StandardCharsets.UTF_8);
+ httpExchange.getResponseHeaders().set("Content-Type",
"application/json");
+ httpExchange.sendResponseHeaders(200, bytes.length);
+ httpExchange.getResponseBody().write(bytes);
+ httpExchange.close();
+ });
+ server.start();
+ serverPort = server.getAddress().getPort();
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (server != null) {
+ server.stop(0);
+ }
+ if (context != null) {
+ context.stop();
+ }
+ }
+
+ private static String b64url(BigInteger value) {
+ byte[] bytes = value.toByteArray();
+ if (bytes.length > 1 && bytes[0] == 0) {
+ byte[] unsigned = new byte[bytes.length - 1];
+ System.arraycopy(bytes, 1, unsigned, 0, unsigned.length);
+ bytes = unsigned;
+ }
+ return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes);
+ }
+
+ @Test
+ void testDirectResolverRateLimitsJwksRefreshOnUnknownKid() throws
Exception {
+ String serverUrl = "http://127.0.0.1:"; + serverPort;
+ KeycloakPublicKeyResolver resolver = new
KeycloakPublicKeyResolver(serverUrl, "test-realm");
+
+ // 1. Initial request with known kid loads the cache (1 network
request)
+ assertThat(resolver.getPublicKey("valid-key-1")).isNotNull();
+ assertThat(jwksRequestCount.get()).isEqualTo(1);
+
+ // 2. Subsequent requests for known kid hit cache (no network requests)
+ assertThat(resolver.getPublicKey("valid-key-1")).isNotNull();
+ assertThat(jwksRequestCount.get()).isEqualTo(1);
+
+ // 3. Requests with unknown kids within the cooldown window do NOT
trigger remote JWKS refreshes
+ int unknownKidAttempts = 5;
+ for (int i = 0; i < unknownKidAttempts; i++) {
+ final String unknownKid = "unknown-kid-" + i;
+ assertThatThrownBy(() -> resolver.getPublicKey(unknownKid))
+ .isInstanceOf(IOException.class)
+ .hasMessageContaining("No public key found in Keycloak
JWKS for kid: " + unknownKid);
+ }
+
+ // Outbound request count must still be 1 (stampede prevented)
+ assertThat(jwksRequestCount.get())
+ .as("Unknown kids within cooldown window did not trigger
remote JWKS fetches")
+ .isEqualTo(1);
+ }
+
+ @Test
+ void testConcurrentRequestsWithUnknownKidsTriggerSingleFetch() throws
Exception {
+ String serverUrl = "http://127.0.0.1:"; + serverPort;
+ KeycloakPublicKeyResolver resolver = new
KeycloakPublicKeyResolver(serverUrl, "test-realm");
+
+ // Initial fetch loads cache
+ assertThat(resolver.getPublicKey("valid-key-1")).isNotNull();
+ assertThat(jwksRequestCount.get()).isEqualTo(1);
+
+ int threadCount = 10;
+ ExecutorService executor = Executors.newFixedThreadPool(threadCount);
+ CountDownLatch startLatch = new CountDownLatch(1);
+ CountDownLatch doneLatch = new CountDownLatch(threadCount);
+ AtomicInteger failureCount = new AtomicInteger();
+
+ for (int i = 0; i < threadCount; i++) {
+ final String unknownKid = "concurrent-unknown-kid-" + i;
+ executor.submit(() -> {
+ try {
+ startLatch.await();
+ resolver.getPublicKey(unknownKid);
+ } catch (IOException expected) {
+ failureCount.incrementAndGet();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ } finally {
+ doneLatch.countDown();
+ }
+ });
+ }
+
+ // Release all threads simultaneously
+ startLatch.countDown();
+ assertThat(doneLatch.await(5, TimeUnit.SECONDS)).isTrue();
+ executor.shutdown();
+
+ assertThat(failureCount.get()).isEqualTo(threadCount);
+ // All concurrent threads for unknown kids must not trigger separate
remote fetches
+ assertThat(jwksRequestCount.get()).isEqualTo(1);
Review Comment:
Applied in c8b3e710bedd: the test no longer warms up the cache. With an
empty cache and no attempt on record, every thread passes the unlocked check in
`getPublicKey`, so the re-check under the lock is what keeps the fetches down
to one. The test still asserts one JWKS request and ten rejected tokens, and
then that a known `kid` resolves from the cache without another fetch.
_Claude Code on behalf of oscerd_
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -13,6 +13,14 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.23 to 4.24
+=== camel-keycloak - JWKS public key resolution rate limiting
+
+`KeycloakSecurityPolicy` now enforces rate limiting on public key refresh
calls to Keycloak's JWKS endpoint (`/protocol/openid-connect/certs`) to prevent
cache stampedes during key rotation or when requests with unknown `kid` (key
ID) headers are processed.
+
+The new configuration option `minJwksRefreshInterval` defines the minimum
cooldown interval in seconds between consecutive JWKS refresh attempts,
defaulting to `10` seconds. If an incoming token carries an unknown `kid` and a
refresh was attempted within the cooldown window, the cached keys are reused
without invoking the remote JWKS endpoint.
Review Comment:
Applied in c8b3e710bedd, with the entry moved next to the other
camel-keycloak section of the guide as part of the rebase: a token with an
unknown `kid` is rejected while the cooldown suppresses the refresh, not
validated against the cached keys; the refresh runs on the request that needs
it, there is no background refresh; and the "Rate Limiting on Key Rotation"
section of keycloak-security.adoc says the same.
_Claude Code on behalf of oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
