Skip to content

Commit 94f6576

Browse files
ebarlasebarlas
committed
Consider max-age directive of cache-control response header when determining PKC JWK set reload delay.
1 parent 0b4db3a commit 94f6576

File tree

4 files changed

+83
-14
lines changed

4 files changed

+83
-14
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwkSetLoader.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,10 @@ void scheduleReload(TimeValue period) {
320320

321321
void reload() {
322322
doLoad(ActionListener.wrap(res -> {
323-
TimeValue period = calculateNextUrlReload(reloadIntervalMin, reloadIntervalMax, res.expires(), URL_RELOAD_JITTER_PCT);
323+
Instant targetTime = res.expires() != null
324+
? res.expires()
325+
: (res.maxAgeSeconds() != null ? Instant.now().plusSeconds(res.maxAgeSeconds()) : null);
326+
TimeValue period = calculateNextUrlReload(reloadIntervalMin, reloadIntervalMax, targetTime, URL_RELOAD_JITTER_PCT);
324327
logger.debug("Successfully reloaded PKC JWK set from HTTPS URI [{}], reload delay is [{}]", jwkSetPathUri, period);
325328
listener.accept(res.content()); // exception here will be caught by ActionListener.wrap and handled below
326329
scheduleReload(period);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtUtil.java‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,9 @@
7676
import java.util.List;
7777
import java.util.Map;
7878
import java.util.Objects;
79-
import java.util.Optional;
8079
import java.util.function.Supplier;
80+
import java.util.regex.Matcher;
81+
import java.util.regex.Pattern;
8182

8283
import javax.net.ssl.HostnameVerifier;
8384
import javax.net.ssl.SSLContext;
@@ -337,7 +338,8 @@ public void completed(final HttpResponse result) {
337338
listener.onResponse(
338339
new JwksResponse(
339340
inputStream.readAllBytes(),
340-
Optional.ofNullable(result.getFirstHeader("Expires")).map(Header::getValue).orElse(null)
341+
firstHeaderValue(result, "Expires"),
342+
firstHeaderValue(result, "Cache-Control")
341343
)
342344
);
343345
} catch (Exception e) {
@@ -366,6 +368,11 @@ public void cancelled() {
366368
});
367369
}
368370

371+
private static String firstHeaderValue(final HttpResponse response, final String headerName) {
372+
final Header header = response.getFirstHeader(headerName);
373+
return header != null ? header.getValue() : null;
374+
}
375+
369376
public static Path resolvePath(final Environment environment, final String jwkSetPath) {
370377
final Path directoryPath = environment.configDir();
371378
return directoryPath.resolve(jwkSetPath);
@@ -491,9 +498,11 @@ private static boolean containsAtLeastTwoDots(SecureString secureString) {
491498
return false;
492499
}
493500

494-
record JwksResponse(byte[] content, Instant expires) {
495-
JwksResponse(byte[] content, String expires) {
496-
this(content, parseExpires(expires));
501+
record JwksResponse(byte[] content, Instant expires, Integer maxAgeSeconds) {
502+
private static final Pattern MAX_AGE_PATTERN = Pattern.compile("\\bmax-age\\s*=\\s*(\\d+)\\b", Pattern.CASE_INSENSITIVE);
503+
504+
JwksResponse(byte[] content, String expires, String cacheControl) {
505+
this(content, parseExpires(expires), parseMaxAge(cacheControl));
497506
}
498507

499508
/**
@@ -514,5 +523,27 @@ static Instant parseExpires(String expires) {
514523
return null;
515524
}
516525
}
526+
527+
/**
528+
* Parse the Cache-Control header to extract the max-age value.
529+
* @return the parsed max-age value as Integer, or null if the header is null or cannot be parsed
530+
*/
531+
static Integer parseMaxAge(String cacheControl) {
532+
if (cacheControl == null) {
533+
return null;
534+
}
535+
536+
Matcher matcher = MAX_AGE_PATTERN.matcher(cacheControl);
537+
if (matcher.find() == false) {
538+
return null;
539+
}
540+
541+
try {
542+
return Integer.parseInt(matcher.group(1));
543+
} catch (NumberFormatException e) {
544+
LOGGER.debug("Failed to parse max-age value from Cache-Control HTTP response header", e);
545+
return null;
546+
}
547+
}
517548
}
518549
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwkSetLoaderTests.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import static org.hamcrest.Matchers.lessThan;
5252
import static org.hamcrest.Matchers.lessThanOrEqualTo;
5353
import static org.mockito.ArgumentMatchers.any;
54-
import static org.mockito.ArgumentMatchers.anyString;
54+
import static org.mockito.ArgumentMatchers.eq;
5555
import static org.mockito.ArgumentMatchers.isNull;
5656
import static org.mockito.Mockito.doAnswer;
5757
import static org.mockito.Mockito.mock;
@@ -261,7 +261,7 @@ private void verifySchedulingIteration(
261261
ArgumentCaptor<FutureCallback<HttpResponse>> responseFn = ArgumentCaptor.forClass(FutureCallback.class);
262262
verify(httpClient, times(1)).execute(any(HttpGet.class), responseFn.capture());
263263
byte[] bytes = "x".repeat(iteration).getBytes(StandardCharsets.UTF_8);
264-
HttpResponse response = makeHttpResponse(bytes);
264+
HttpResponse response = makeHttpResponse(bytes, randomBoolean());
265265

266266
reset(threadPool);
267267
reset(httpClient);
@@ -287,7 +287,7 @@ private void verifySchedulingIterationWithListenerException(ThreadPool threadPoo
287287
@SuppressWarnings("unchecked")
288288
ArgumentCaptor<FutureCallback<HttpResponse>> responseFn = ArgumentCaptor.forClass(FutureCallback.class);
289289
verify(httpClient, times(1)).execute(any(HttpGet.class), responseFn.capture());
290-
HttpResponse response = makeHttpResponse(new byte[0]);
290+
HttpResponse response = makeHttpResponse(new byte[0], randomBoolean());
291291

292292
reset(threadPool);
293293
reset(httpClient);
@@ -307,21 +307,34 @@ private static void verifyScheduleTime(boolean firstIteration, ArgumentCaptor<Ti
307307
}
308308
}
309309

310-
private static HttpResponse makeHttpResponse(byte[] bytes) throws IOException {
310+
private static HttpResponse makeHttpResponse(byte[] bytes, boolean expiresHeader) throws IOException {
311311
HttpEntity entity = mock(HttpEntity.class);
312-
Header header = mock(Header.class);
313312
StatusLine statusLine = mock(StatusLine.class);
314313
when(statusLine.getStatusCode()).thenReturn(200);
315-
when(header.getValue()).thenReturn(expiresHeader(10)); // expires in 10 minutes
316314
when(entity.getContent()).thenReturn(new ByteArrayInputStream(bytes));
317315
HttpResponse response = mock(HttpResponse.class);
318316
when(response.getStatusLine()).thenReturn(statusLine);
319317
when(response.getEntity()).thenReturn(entity);
320-
when(response.getFirstHeader(anyString())).thenReturn(header);
318+
Header eh = expiresHeader ? expiresHeader(10) : null;
319+
Header cc = expiresHeader ? null : cacheControlHeader(10);
320+
when(response.getFirstHeader(eq("Expires"))).thenReturn(eh);
321+
when(response.getFirstHeader(eq("Cache-Control"))).thenReturn(cc);
321322
return response;
322323
}
323324

324-
private static String expiresHeader(int plusMinutes) {
325+
private static Header expiresHeader(int minutes) {
326+
Header header = mock(Header.class);
327+
when(header.getValue()).thenReturn(expiresHeaderValue(minutes));
328+
return header;
329+
}
330+
331+
private static Header cacheControlHeader(int minutes) {
332+
Header header = mock(Header.class);
333+
when(header.getValue()).thenReturn("max-age=" + (minutes * 60));
334+
return header;
335+
}
336+
337+
private static String expiresHeaderValue(int plusMinutes) {
325338
ZonedDateTime nowUtc = ZonedDateTime.now(ZoneId.of("UTC"));
326339
ZonedDateTime zdt = nowUtc.plusMinutes(plusMinutes);
327340
return zdt.format(DateTimeFormatter.RFC_1123_DATE_TIME);

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwtUtilTests.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
import org.elasticsearch.common.settings.SettingsException;
1212
import org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings;
1313

14+
import java.time.Instant;
15+
import java.time.ZoneId;
16+
import java.time.ZonedDateTime;
17+
import java.time.format.DateTimeFormatter;
18+
1419
import static org.hamcrest.Matchers.equalTo;
1520
import static org.hamcrest.Matchers.is;
1621
import static org.hamcrest.Matchers.notNullValue;
@@ -158,4 +163,21 @@ public void testParseHttpsUriAccepted() {
158163
assertThat(JwtUtil.parseHttpsUri("https://example.com:443/path/jwkset.json"), notNullValue());
159164
assertThat(JwtUtil.parseHttpsUri("https://example.com:8443/path/jwkset.json"), notNullValue());
160165
}
166+
167+
public void testParseExpires() {
168+
ZonedDateTime nowUtc = ZonedDateTime.now(ZoneId.of("UTC"));
169+
Instant parsed = JwtUtil.JwksResponse.parseExpires(nowUtc.format(DateTimeFormatter.RFC_1123_DATE_TIME));
170+
assertThat(parsed.getEpochSecond(), equalTo(nowUtc.toEpochSecond()));
171+
assertThat(JwtUtil.JwksResponse.parseExpires(null), nullValue());
172+
assertThat(JwtUtil.JwksResponse.parseExpires(""), nullValue());
173+
assertThat(JwtUtil.JwksResponse.parseExpires("Jan 2024"), nullValue());
174+
}
175+
176+
public void testParseMaxAge() {
177+
assertThat(JwtUtil.JwksResponse.parseMaxAge("public, max-age=3600, immutable"), equalTo(3600));
178+
assertThat(JwtUtil.JwksResponse.parseMaxAge("max-age=7200"), equalTo(7200));
179+
assertThat(JwtUtil.JwksResponse.parseMaxAge("no-cache, no-store"), nullValue());
180+
assertThat(JwtUtil.JwksResponse.parseMaxAge(""), nullValue());
181+
assertThat(JwtUtil.JwksResponse.parseMaxAge(null), nullValue());
182+
}
161183
}

0 commit comments

Comments
 (0)