Skip to content

Commit 18f0d53

Browse files
tvernumtvernum
authored
Fallback to realm authc if ApiKey fails (#46538)
This changes API-Key authentication to always fallback to the realm chain if the API key is not valid. The previous behaviour was inconsistent and would terminate on some failures, but continue to the realm chain for others.
1 parent 5bb796f commit 18f0d53

File tree

3 files changed

+126
-31
lines changed

3 files changed

+126
-31
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,15 +331,16 @@ void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListener<Authentic
331331
}
332332

333333
if (credentials != null) {
334+
final String docId = credentials.getId();
334335
final GetRequest getRequest = client
335-
.prepareGet(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, credentials.getId())
336+
.prepareGet(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, docId)
336337
.setFetchSource(true)
337338
.request();
338339
executeAsyncWithOrigin(ctx, SECURITY_ORIGIN, getRequest, ActionListener.<GetResponse>wrap(response -> {
339340
if (response.isExists()) {
340341
try (ApiKeyCredentials ignore = credentials) {
341342
final Map<String, Object> source = response.getSource();
342-
validateApiKeyCredentials(source, credentials, clock, listener);
343+
validateApiKeyCredentials(docId, source, credentials, clock, listener);
343344
}
344345
} else {
345346
credentials.close();
@@ -434,17 +435,22 @@ private List<RoleDescriptor> parseRoleDescriptors(final String apiKeyId, final M
434435

435436
/**
436437
* Validates the ApiKey using the source map
438+
* @param docId the identifier of the document that was retrieved from the security index
437439
* @param source the source map from a get of the ApiKey document
438440
* @param credentials the credentials provided by the user
439441
* @param listener the listener to notify after verification
440442
*/
441-
void validateApiKeyCredentials(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
443+
void validateApiKeyCredentials(String docId, Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
442444
ActionListener<AuthenticationResult> listener) {
445+
final String docType = (String) source.get("doc_type");
443446
final Boolean invalidated = (Boolean) source.get("api_key_invalidated");
444-
if (invalidated == null) {
445-
listener.onResponse(AuthenticationResult.terminate("api key document is missing invalidated field", null));
447+
if ("api_key".equals(docType) == false) {
448+
listener.onResponse(
449+
AuthenticationResult.unsuccessful("document [" + docId + "] is [" + docType + "] not an api key", null));
450+
} else if (invalidated == null) {
451+
listener.onResponse(AuthenticationResult.unsuccessful("api key document is missing invalidated field", null));
446452
} else if (invalidated) {
447-
listener.onResponse(AuthenticationResult.terminate("api key has been invalidated", null));
453+
listener.onResponse(AuthenticationResult.unsuccessful("api key has been invalidated", null));
448454
} else {
449455
final String apiKeyHash = (String) source.get("api_key_hash");
450456
if (apiKeyHash == null) {
@@ -478,7 +484,7 @@ void validateApiKeyCredentials(Map<String, Object> source, ApiKeyCredentials cre
478484
listener.onResponse(AuthenticationResult.unsuccessful("invalid credentials", null));
479485
} else {
480486
apiKeyAuthCache.invalidate(credentials.getId(), listenableCacheEntry);
481-
validateApiKeyCredentials(source, credentials, clock, listener);
487+
validateApiKeyCredentials(docId, source, credentials, clock, listener);
482488
}
483489
}, listener::onFailure),
484490
threadPool.generic(), threadPool.getThreadContext());
@@ -528,7 +534,7 @@ private void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredenti
528534
authResultMetadata.put(API_KEY_ID_KEY, credentials.getId());
529535
listener.onResponse(AuthenticationResult.success(apiKeyUser, authResultMetadata));
530536
} else {
531-
listener.onResponse(AuthenticationResult.terminate("api key is expired", null));
537+
listener.onResponse(AuthenticationResult.unsuccessful("api key is expired", null));
532538
}
533539
}
534540

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

Lines changed: 112 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.io.IOException;
4646
import java.nio.charset.StandardCharsets;
4747
import java.time.Clock;
48+
import java.time.Duration;
4849
import java.time.Instant;
4950
import java.time.temporal.ChronoUnit;
5051
import java.util.Arrays;
@@ -56,6 +57,7 @@
5657

5758
import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR;
5859
import static org.hamcrest.Matchers.arrayContaining;
60+
import static org.hamcrest.Matchers.containsString;
5961
import static org.hamcrest.Matchers.equalTo;
6062
import static org.hamcrest.Matchers.instanceOf;
6163
import static org.hamcrest.Matchers.is;
@@ -159,25 +161,110 @@ public void testAuthenticationIsSkippedIfLicenseDoesNotAllowIt() throws Exceptio
159161
assertThat(auth.getUser(), nullValue());
160162
}
161163

162-
public void mockKeyDocument(ApiKeyService service, String id, String key, User user) throws IOException {
163-
final Authentication authentication = new Authentication(user, new RealmRef("realm1", "native", "node01"), null, Version.CURRENT);
164-
final XContentBuilder docSource = service.newDocument(new SecureString(key.toCharArray()), "test", authentication,
165-
Collections.singleton(SUPERUSER_ROLE_DESCRIPTOR), Instant.now(), Instant.now().plusSeconds(3600), null, Version.CURRENT);
164+
public void testAuthenticationFailureWithInvalidatedApiKey() throws Exception {
165+
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
166+
final ApiKeyService service = createApiKeyService(settings);
167+
168+
final String id = randomAlphaOfLength(12);
169+
final String key = randomAlphaOfLength(16);
170+
171+
mockKeyDocument(service, id, key, new User("hulk", "superuser"), true, Duration.ofSeconds(3600));
172+
173+
final AuthenticationResult auth = tryAuthenticate(service, id, key);
174+
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
175+
assertThat(auth.getUser(), nullValue());
176+
assertThat(auth.getMessage(), containsString("invalidated"));
177+
}
178+
179+
public void testAuthenticationFailureWithInvalidCredentials() throws Exception {
180+
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
181+
final ApiKeyService service = createApiKeyService(settings);
182+
183+
final String id = randomAlphaOfLength(12);
184+
final String realKey = randomAlphaOfLength(16);
185+
final String wrongKey = "#" + realKey.substring(1);
186+
187+
mockKeyDocument(service, id, realKey, new User("hulk", "superuser"));
188+
189+
final AuthenticationResult auth = tryAuthenticate(service, id, wrongKey);
190+
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
191+
assertThat(auth.getUser(), nullValue());
192+
assertThat(auth.getMessage(), containsString("invalid credentials"));
193+
}
194+
195+
public void testAuthenticationFailureWithExpiredKey() throws Exception {
196+
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
197+
final ApiKeyService service = createApiKeyService(settings);
166198

199+
final String id = randomAlphaOfLength(12);
200+
final String key = randomAlphaOfLength(16);
201+
202+
mockKeyDocument(service, id, key, new User("hulk", "superuser"), false, Duration.ofSeconds(-1));
203+
204+
final AuthenticationResult auth = tryAuthenticate(service, id, key);
205+
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
206+
assertThat(auth.getUser(), nullValue());
207+
assertThat(auth.getMessage(), containsString("expired"));
208+
}
209+
210+
/**
211+
* We cache valid and invalid responses. This test verifies that we handle these correctly.
212+
*/
213+
public void testMixingValidAndInvalidCredentials() throws Exception {
214+
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
215+
final ApiKeyService service = createApiKeyService(settings);
216+
217+
final String id = randomAlphaOfLength(12);
218+
final String realKey = randomAlphaOfLength(16);
219+
220+
mockKeyDocument(service, id, realKey, new User("hulk", "superuser"));
221+
222+
for (int i = 0; i < 3; i++) {
223+
final String wrongKey = "=" + randomAlphaOfLength(14) + "@";
224+
AuthenticationResult auth = tryAuthenticate(service, id, wrongKey);
225+
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
226+
assertThat(auth.getUser(), nullValue());
227+
assertThat(auth.getMessage(), containsString("invalid credentials"));
228+
229+
auth = tryAuthenticate(service, id, realKey);
230+
assertThat(auth.getStatus(), is(AuthenticationResult.Status.SUCCESS));
231+
assertThat(auth.getUser(), notNullValue());
232+
assertThat(auth.getUser().principal(), is("hulk"));
233+
}
234+
}
235+
236+
private void mockKeyDocument(ApiKeyService service, String id, String key, User user) throws IOException {
237+
mockKeyDocument(service, id, key, user, false, Duration.ofSeconds(3600));
238+
}
239+
240+
private void mockKeyDocument(ApiKeyService service, String id, String key, User user, boolean invalidated,
241+
Duration expiry) throws IOException {
242+
final Authentication authentication = new Authentication(user, new RealmRef("realm1", "native",
243+
"node01"), null, Version.CURRENT);
244+
XContentBuilder docSource = service.newDocument(new SecureString(key.toCharArray()), "test", authentication,
245+
Collections.singleton(SUPERUSER_ROLE_DESCRIPTOR), Instant.now(), Instant.now().plus(expiry), null,
246+
Version.CURRENT);
247+
if (invalidated) {
248+
Map<String, Object> map = XContentHelper.convertToMap(BytesReference.bytes(docSource), true, XContentType.JSON).v2();
249+
map.put("api_key_invalidated", true);
250+
docSource = XContentBuilder.builder(XContentType.JSON.xContent()).map(map);
251+
}
167252
SecurityMocks.mockGetRequest(client, id, BytesReference.bytes(docSource));
168253
}
169254

170255
private AuthenticationResult tryAuthenticate(ApiKeyService service, String id, String key) throws Exception {
171256
final ThreadContext threadContext = threadPool.getThreadContext();
172-
final String header = "ApiKey " + Base64.getEncoder().encodeToString((id + ":" + key).getBytes(StandardCharsets.UTF_8));
173-
threadContext.putHeader("Authorization", header);
257+
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
258+
final String header = "ApiKey " + Base64.getEncoder().encodeToString((id + ":" + key).getBytes(StandardCharsets.UTF_8));
259+
threadContext.putHeader("Authorization", header);
174260

175-
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
176-
service.authenticateWithApiKeyIfPresent(threadContext, future);
261+
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
262+
service.authenticateWithApiKeyIfPresent(threadContext, future);
177263

178-
final AuthenticationResult auth = future.get();
179-
assertThat(auth, notNullValue());
180-
return auth;
264+
final AuthenticationResult auth = future.get();
265+
assertThat(auth, notNullValue());
266+
return auth;
267+
}
181268
}
182269

183270
public void testValidateApiKey() throws Exception {
@@ -186,6 +273,7 @@ public void testValidateApiKey() throws Exception {
186273
final char[] hash = hasher.hash(new SecureString(apiKey.toCharArray()));
187274

188275
Map<String, Object> sourceMap = new HashMap<>();
276+
sourceMap.put("doc_type", "api_key");
189277
sourceMap.put("api_key_hash", new String(hash));
190278
sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all")));
191279
sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all")));
@@ -200,7 +288,7 @@ public void testValidateApiKey() throws Exception {
200288
ApiKeyService.ApiKeyCredentials creds =
201289
new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray()));
202290
PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
203-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
291+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
204292
AuthenticationResult result = future.get();
205293
assertNotNull(result);
206294
assertTrue(result.isAuthenticated());
@@ -214,7 +302,7 @@ public void testValidateApiKey() throws Exception {
214302

215303
sourceMap.put("expiration_time", Clock.systemUTC().instant().plus(1L, ChronoUnit.HOURS).toEpochMilli());
216304
future = new PlainActionFuture<>();
217-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
305+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
218306
result = future.get();
219307
assertNotNull(result);
220308
assertTrue(result.isAuthenticated());
@@ -228,23 +316,23 @@ public void testValidateApiKey() throws Exception {
228316

229317
sourceMap.put("expiration_time", Clock.systemUTC().instant().minus(1L, ChronoUnit.HOURS).toEpochMilli());
230318
future = new PlainActionFuture<>();
231-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
319+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
232320
result = future.get();
233321
assertNotNull(result);
234322
assertFalse(result.isAuthenticated());
235323

236324
sourceMap.remove("expiration_time");
237325
creds = new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(randomAlphaOfLength(15).toCharArray()));
238326
future = new PlainActionFuture<>();
239-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
327+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
240328
result = future.get();
241329
assertNotNull(result);
242330
assertFalse(result.isAuthenticated());
243331

244332
sourceMap.put("api_key_invalidated", true);
245333
creds = new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(randomAlphaOfLength(15).toCharArray()));
246334
future = new PlainActionFuture<>();
247-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
335+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
248336
result = future.get();
249337
assertNotNull(result);
250338
assertFalse(result.isAuthenticated());
@@ -344,6 +432,7 @@ public void testApiKeyCache() {
344432
final char[] hash = hasher.hash(new SecureString(apiKey.toCharArray()));
345433

346434
Map<String, Object> sourceMap = new HashMap<>();
435+
sourceMap.put("doc_type", "api_key");
347436
sourceMap.put("api_key_hash", new String(hash));
348437
sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all")));
349438
sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all")));
@@ -356,7 +445,7 @@ public void testApiKeyCache() {
356445
ApiKeyService service = createApiKeyService(Settings.EMPTY);
357446
ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray()));
358447
PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
359-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
448+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
360449
AuthenticationResult result = future.actionGet();
361450
assertThat(result.isAuthenticated(), is(true));
362451
CachedApiKeyHashResult cachedApiKeyHashResult = service.getFromCache(creds.getId());
@@ -365,7 +454,7 @@ public void testApiKeyCache() {
365454

366455
creds = new ApiKeyCredentials(creds.getId(), new SecureString("foobar".toCharArray()));
367456
future = new PlainActionFuture<>();
368-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
457+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
369458
result = future.actionGet();
370459
assertThat(result.isAuthenticated(), is(false));
371460
final CachedApiKeyHashResult shouldBeSame = service.getFromCache(creds.getId());
@@ -375,7 +464,7 @@ public void testApiKeyCache() {
375464
sourceMap.put("api_key_hash", new String(hasher.hash(new SecureString("foobar".toCharArray()))));
376465
creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString("foobar1".toCharArray()));
377466
future = new PlainActionFuture<>();
378-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
467+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
379468
result = future.actionGet();
380469
assertThat(result.isAuthenticated(), is(false));
381470
cachedApiKeyHashResult = service.getFromCache(creds.getId());
@@ -384,15 +473,15 @@ public void testApiKeyCache() {
384473

385474
creds = new ApiKeyCredentials(creds.getId(), new SecureString("foobar2".toCharArray()));
386475
future = new PlainActionFuture<>();
387-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
476+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
388477
result = future.actionGet();
389478
assertThat(result.isAuthenticated(), is(false));
390479
assertThat(service.getFromCache(creds.getId()), not(sameInstance(cachedApiKeyHashResult)));
391480
assertThat(service.getFromCache(creds.getId()).success, is(false));
392481

393482
creds = new ApiKeyCredentials(creds.getId(), new SecureString("foobar".toCharArray()));
394483
future = new PlainActionFuture<>();
395-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
484+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
396485
result = future.actionGet();
397486
assertThat(result.isAuthenticated(), is(true));
398487
assertThat(service.getFromCache(creds.getId()), not(sameInstance(cachedApiKeyHashResult)));
@@ -408,6 +497,7 @@ public void testApiKeyCacheDisabled() {
408497
.build();
409498

410499
Map<String, Object> sourceMap = new HashMap<>();
500+
sourceMap.put("doc_type", "api_key");
411501
sourceMap.put("api_key_hash", new String(hash));
412502
sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all")));
413503
sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all")));
@@ -420,7 +510,7 @@ public void testApiKeyCacheDisabled() {
420510
ApiKeyService service = createApiKeyService(settings);
421511
ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray()));
422512
PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
423-
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
513+
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
424514
AuthenticationResult result = future.actionGet();
425515
assertThat(result.isAuthenticated(), is(true));
426516
CachedApiKeyHashResult cachedApiKeyHashResult = service.getFromCache(creds.getId());

0 commit comments

Comments
 (0)