Skip to content

Commit

Permalink
Fallback to realm authc if ApiKey fails (#46538)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tvernum authored Sep 10, 2019
1 parent 5bb796f commit 18f0d53
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,15 +331,16 @@ void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListener<Authentic
}

if (credentials != null) {
final String docId = credentials.getId();
final GetRequest getRequest = client
.prepareGet(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, credentials.getId())
.prepareGet(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, docId)
.setFetchSource(true)
.request();
executeAsyncWithOrigin(ctx, SECURITY_ORIGIN, getRequest, ActionListener.<GetResponse>wrap(response -> {
if (response.isExists()) {
try (ApiKeyCredentials ignore = credentials) {
final Map<String, Object> source = response.getSource();
validateApiKeyCredentials(source, credentials, clock, listener);
validateApiKeyCredentials(docId, source, credentials, clock, listener);
}
} else {
credentials.close();
Expand Down Expand Up @@ -434,17 +435,22 @@ private List<RoleDescriptor> parseRoleDescriptors(final String apiKeyId, final M

/**
* Validates the ApiKey using the source map
* @param docId the identifier of the document that was retrieved from the security index
* @param source the source map from a get of the ApiKey document
* @param credentials the credentials provided by the user
* @param listener the listener to notify after verification
*/
void validateApiKeyCredentials(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
void validateApiKeyCredentials(String docId, Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
ActionListener<AuthenticationResult> listener) {
final String docType = (String) source.get("doc_type");
final Boolean invalidated = (Boolean) source.get("api_key_invalidated");
if (invalidated == null) {
listener.onResponse(AuthenticationResult.terminate("api key document is missing invalidated field", null));
if ("api_key".equals(docType) == false) {
listener.onResponse(
AuthenticationResult.unsuccessful("document [" + docId + "] is [" + docType + "] not an api key", null));
} else if (invalidated == null) {
listener.onResponse(AuthenticationResult.unsuccessful("api key document is missing invalidated field", null));
} else if (invalidated) {
listener.onResponse(AuthenticationResult.terminate("api key has been invalidated", null));
listener.onResponse(AuthenticationResult.unsuccessful("api key has been invalidated", null));
} else {
final String apiKeyHash = (String) source.get("api_key_hash");
if (apiKeyHash == null) {
Expand Down Expand Up @@ -478,7 +484,7 @@ void validateApiKeyCredentials(Map<String, Object> source, ApiKeyCredentials cre
listener.onResponse(AuthenticationResult.unsuccessful("invalid credentials", null));
} else {
apiKeyAuthCache.invalidate(credentials.getId(), listenableCacheEntry);
validateApiKeyCredentials(source, credentials, clock, listener);
validateApiKeyCredentials(docId, source, credentials, clock, listener);
}
}, listener::onFailure),
threadPool.generic(), threadPool.getThreadContext());
Expand Down Expand Up @@ -528,7 +534,7 @@ private void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredenti
authResultMetadata.put(API_KEY_ID_KEY, credentials.getId());
listener.onResponse(AuthenticationResult.success(apiKeyUser, authResultMetadata));
} else {
listener.onResponse(AuthenticationResult.terminate("api key is expired", null));
listener.onResponse(AuthenticationResult.unsuccessful("api key is expired", null));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Arrays;
Expand All @@ -56,6 +57,7 @@

import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -159,25 +161,110 @@ public void testAuthenticationIsSkippedIfLicenseDoesNotAllowIt() throws Exceptio
assertThat(auth.getUser(), nullValue());
}

public void mockKeyDocument(ApiKeyService service, String id, String key, User user) throws IOException {
final Authentication authentication = new Authentication(user, new RealmRef("realm1", "native", "node01"), null, Version.CURRENT);
final XContentBuilder docSource = service.newDocument(new SecureString(key.toCharArray()), "test", authentication,
Collections.singleton(SUPERUSER_ROLE_DESCRIPTOR), Instant.now(), Instant.now().plusSeconds(3600), null, Version.CURRENT);
public void testAuthenticationFailureWithInvalidatedApiKey() throws Exception {
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
final ApiKeyService service = createApiKeyService(settings);

final String id = randomAlphaOfLength(12);
final String key = randomAlphaOfLength(16);

mockKeyDocument(service, id, key, new User("hulk", "superuser"), true, Duration.ofSeconds(3600));

final AuthenticationResult auth = tryAuthenticate(service, id, key);
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
assertThat(auth.getUser(), nullValue());
assertThat(auth.getMessage(), containsString("invalidated"));
}

public void testAuthenticationFailureWithInvalidCredentials() throws Exception {
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
final ApiKeyService service = createApiKeyService(settings);

final String id = randomAlphaOfLength(12);
final String realKey = randomAlphaOfLength(16);
final String wrongKey = "#" + realKey.substring(1);

mockKeyDocument(service, id, realKey, new User("hulk", "superuser"));

final AuthenticationResult auth = tryAuthenticate(service, id, wrongKey);
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
assertThat(auth.getUser(), nullValue());
assertThat(auth.getMessage(), containsString("invalid credentials"));
}

public void testAuthenticationFailureWithExpiredKey() throws Exception {
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
final ApiKeyService service = createApiKeyService(settings);

final String id = randomAlphaOfLength(12);
final String key = randomAlphaOfLength(16);

mockKeyDocument(service, id, key, new User("hulk", "superuser"), false, Duration.ofSeconds(-1));

final AuthenticationResult auth = tryAuthenticate(service, id, key);
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
assertThat(auth.getUser(), nullValue());
assertThat(auth.getMessage(), containsString("expired"));
}

/**
* We cache valid and invalid responses. This test verifies that we handle these correctly.
*/
public void testMixingValidAndInvalidCredentials() throws Exception {
final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build();
final ApiKeyService service = createApiKeyService(settings);

final String id = randomAlphaOfLength(12);
final String realKey = randomAlphaOfLength(16);

mockKeyDocument(service, id, realKey, new User("hulk", "superuser"));

for (int i = 0; i < 3; i++) {
final String wrongKey = "=" + randomAlphaOfLength(14) + "@";
AuthenticationResult auth = tryAuthenticate(service, id, wrongKey);
assertThat(auth.getStatus(), is(AuthenticationResult.Status.CONTINUE));
assertThat(auth.getUser(), nullValue());
assertThat(auth.getMessage(), containsString("invalid credentials"));

auth = tryAuthenticate(service, id, realKey);
assertThat(auth.getStatus(), is(AuthenticationResult.Status.SUCCESS));
assertThat(auth.getUser(), notNullValue());
assertThat(auth.getUser().principal(), is("hulk"));
}
}

private void mockKeyDocument(ApiKeyService service, String id, String key, User user) throws IOException {
mockKeyDocument(service, id, key, user, false, Duration.ofSeconds(3600));
}

private void mockKeyDocument(ApiKeyService service, String id, String key, User user, boolean invalidated,
Duration expiry) throws IOException {
final Authentication authentication = new Authentication(user, new RealmRef("realm1", "native",
"node01"), null, Version.CURRENT);
XContentBuilder docSource = service.newDocument(new SecureString(key.toCharArray()), "test", authentication,
Collections.singleton(SUPERUSER_ROLE_DESCRIPTOR), Instant.now(), Instant.now().plus(expiry), null,
Version.CURRENT);
if (invalidated) {
Map<String, Object> map = XContentHelper.convertToMap(BytesReference.bytes(docSource), true, XContentType.JSON).v2();
map.put("api_key_invalidated", true);
docSource = XContentBuilder.builder(XContentType.JSON.xContent()).map(map);
}
SecurityMocks.mockGetRequest(client, id, BytesReference.bytes(docSource));
}

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

final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
service.authenticateWithApiKeyIfPresent(threadContext, future);
final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
service.authenticateWithApiKeyIfPresent(threadContext, future);

final AuthenticationResult auth = future.get();
assertThat(auth, notNullValue());
return auth;
final AuthenticationResult auth = future.get();
assertThat(auth, notNullValue());
return auth;
}
}

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

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

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

sourceMap.put("expiration_time", Clock.systemUTC().instant().minus(1L, ChronoUnit.HOURS).toEpochMilli());
future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
result = future.get();
assertNotNull(result);
assertFalse(result.isAuthenticated());

sourceMap.remove("expiration_time");
creds = new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(randomAlphaOfLength(15).toCharArray()));
future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
result = future.get();
assertNotNull(result);
assertFalse(result.isAuthenticated());

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

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

creds = new ApiKeyCredentials(creds.getId(), new SecureString("foobar".toCharArray()));
future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
result = future.actionGet();
assertThat(result.isAuthenticated(), is(false));
final CachedApiKeyHashResult shouldBeSame = service.getFromCache(creds.getId());
Expand All @@ -375,7 +464,7 @@ public void testApiKeyCache() {
sourceMap.put("api_key_hash", new String(hasher.hash(new SecureString("foobar".toCharArray()))));
creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString("foobar1".toCharArray()));
future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
result = future.actionGet();
assertThat(result.isAuthenticated(), is(false));
cachedApiKeyHashResult = service.getFromCache(creds.getId());
Expand All @@ -384,15 +473,15 @@ public void testApiKeyCache() {

creds = new ApiKeyCredentials(creds.getId(), new SecureString("foobar2".toCharArray()));
future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
result = future.actionGet();
assertThat(result.isAuthenticated(), is(false));
assertThat(service.getFromCache(creds.getId()), not(sameInstance(cachedApiKeyHashResult)));
assertThat(service.getFromCache(creds.getId()).success, is(false));

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

Map<String, Object> sourceMap = new HashMap<>();
sourceMap.put("doc_type", "api_key");
sourceMap.put("api_key_hash", new String(hash));
sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all")));
sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all")));
Expand All @@ -420,7 +510,7 @@ public void testApiKeyCacheDisabled() {
ApiKeyService service = createApiKeyService(settings);
ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray()));
PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
service.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
service.validateApiKeyCredentials(creds.getId(), sourceMap, creds, Clock.systemUTC(), future);
AuthenticationResult result = future.actionGet();
assertThat(result.isAuthenticated(), is(true));
CachedApiKeyHashResult cachedApiKeyHashResult = service.getFromCache(creds.getId());
Expand Down
Loading

0 comments on commit 18f0d53

Please sign in to comment.