Fallback to realm authc if ApiKey fails (#46552)

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.

Backport of: #46538
This commit is contained in:
Tim Vernum 2019-09-11 14:33:17 +10:00 committed by GitHub
parent 7a2878b29b
commit 80064652f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 126 additions and 31 deletions

View File

@ -337,15 +337,16 @@ public class ApiKeyService {
}
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();
@ -440,17 +441,22 @@ public class ApiKeyService {
/**
* 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) {
@ -484,7 +490,7 @@ public class ApiKeyService {
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());
@ -534,7 +540,7 @@ public class ApiKeyService {
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));
}
}

View File

@ -45,6 +45,7 @@ import org.junit.Before;
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;
@ -56,6 +57,7 @@ import java.util.Map;
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;
@ -159,16 +161,100 @@ public class ApiKeyServiceTests extends ESTestCase {
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();
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
final String header = "ApiKey " + Base64.getEncoder().encodeToString((id + ":" + key).getBytes(StandardCharsets.UTF_8));
threadContext.putHeader("Authorization", header);
@ -179,6 +265,7 @@ public class ApiKeyServiceTests extends ESTestCase {
assertThat(auth, notNullValue());
return auth;
}
}
public void testValidateApiKey() throws Exception {
final String apiKey = randomAlphaOfLength(16);
@ -186,6 +273,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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")));
@ -200,7 +288,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -214,7 +302,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -228,7 +316,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -236,7 +324,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -244,7 +332,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -344,6 +432,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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")));
@ -356,7 +445,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -365,7 +454,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -375,7 +464,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());
@ -384,7 +473,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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)));
@ -392,7 +481,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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)));
@ -408,6 +497,7 @@ public class ApiKeyServiceTests extends ESTestCase {
.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")));
@ -420,7 +510,7 @@ public class ApiKeyServiceTests extends ESTestCase {
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());

View File

@ -1305,7 +1305,6 @@ public class AuthenticationServiceTests extends ESTestCase {
threadContext.putHeader("Authorization", headerValue);
ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class,
() -> authenticateBlocking("_action", message, null));
assertThat(e.getMessage(), containsString("api key is expired"));
assertEquals(RestStatus.UNAUTHORIZED, e.status());
}
}