From f003a419a54d50e6fea0ac834c2b41b4da441916 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 23 Mar 2020 14:13:15 +1100 Subject: [PATCH] Add exception metadata for disabled features (#53941) This change adds a new exception with consistent metadata for when security features are not enabled. This allows clients to be able to tell that an API failed due to a configuration option, and respond accordingly. Relates: kibana#55255 Resolves: #52311, #47759 Backport of: #52811 --- .../xpack/security/authc/ApiKeyService.java | 4 +- .../xpack/security/authc/TokenService.java | 4 +- .../support/FeatureNotEnabledException.java | 40 +++++++++++++++++++ .../security/authc/ApiKeyServiceTests.java | 18 +++++++++ .../security/authc/TokenServiceTests.java | 25 +++++++----- 5 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 659fa70f2777a..8969195b0cfd8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -70,6 +70,8 @@ import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.user.User; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException.Feature; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.io.Closeable; @@ -586,7 +588,7 @@ private void ensureEnabled() { throw LicenseUtils.newComplianceException("api keys"); } if (enabled == false) { - throw new IllegalStateException("api keys are not enabled"); + throw new FeatureNotEnabledException(Feature.API_KEY_SERVICE, "api keys are not enabled"); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index b81f16b6f3508..984feb59d7eec 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -89,6 +89,8 @@ import org.elasticsearch.xpack.core.security.authc.TokenMetaData; import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException.Feature; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import javax.crypto.Cipher; @@ -1459,7 +1461,7 @@ private void ensureEnabled() { throw LicenseUtils.newComplianceException("security tokens"); } if (enabled == false) { - throw new IllegalStateException("security tokens are not enabled"); + throw new FeatureNotEnabledException(Feature.TOKEN_SERVICE, "security tokens are not enabled"); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java new file mode 100644 index 0000000000000..d79c49657d9fd --- /dev/null +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FeatureNotEnabledException.java @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.security.support; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.rest.RestStatus; + +public class FeatureNotEnabledException extends ElasticsearchException { + + public static final String DISABLED_FEATURE_METADATA = "es.disabled.feature"; + + /** + * The features names here are constants that form part of our API contract. + * Callers (e.g. Kibana) may be dependent on these strings. Do not change them without consideration of BWC. + */ + public enum Feature { + TOKEN_SERVICE("security_tokens"), + API_KEY_SERVICE("api_keys"); + + private final String featureName; + + Feature(String featureName) { + this.featureName = featureName; + } + } + + public FeatureNotEnabledException(Feature feature, String message, Object... args) { + super(message, args); + addMetadata(DISABLED_FEATURE_METADATA, feature.featureName); + } + + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index b55b6c12f04e9..064047bf0279d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.security.authc; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; @@ -38,6 +39,7 @@ import org.elasticsearch.xpack.security.authc.ApiKeyService.ApiKeyRoleDescriptors; import org.elasticsearch.xpack.security.authc.ApiKeyService.CachedApiKeyHashResult; import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.elasticsearch.xpack.security.test.SecurityMocks; import org.junit.After; @@ -59,8 +61,10 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicInteger; +import static org.elasticsearch.test.TestMatchers.throwableWithMessage; import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR; import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -435,6 +439,20 @@ public void testGetRolesForApiKey() throws Exception { } } + public void testApiKeyServiceDisabled() throws Exception { + final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), false).build(); + final ApiKeyService service = createApiKeyService(settings); + + ElasticsearchException e = expectThrows(ElasticsearchException.class, + () -> service.getApiKeys(randomAlphaOfLength(6), randomAlphaOfLength(8), null, null, new PlainActionFuture<>())); + + assertThat(e, instanceOf(FeatureNotEnabledException.class)); + // Older Kibana version looked for this exact text: + assertThat(e, throwableWithMessage("api keys are not enabled")); + // Newer Kibana versions will check the metadata for this string literal: + assertThat(e.getMetadata(FeatureNotEnabledException.DISABLED_FEATURE_METADATA), contains("api_keys")); + } + public void testApiKeyCache() { final String apiKey = randomAlphaOfLength(16); Hasher hasher = randomFrom(Hasher.PBKDF2, Hasher.BCRYPT4, Hasher.BCRYPT); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java index df88a44df84a7..e9fe5783a4903 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.security.authc; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; @@ -54,6 +55,7 @@ import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult; import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.core.watcher.watch.ClockMock; +import org.elasticsearch.xpack.security.support.FeatureNotEnabledException; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.elasticsearch.xpack.security.test.SecurityMocks; import org.hamcrest.Matchers; @@ -63,7 +65,6 @@ import org.junit.BeforeClass; import javax.crypto.SecretKey; - import java.io.IOException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -79,8 +80,11 @@ import static java.time.Clock.systemUTC; import static org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase.randomBytes; import static org.elasticsearch.test.ClusterServiceUtils.setState; +import static org.elasticsearch.test.TestMatchers.throwableWithMessage; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.mockito.Matchers.any; @@ -561,20 +565,23 @@ public void testTokenServiceDisabled() throws Exception { .put(XPackSettings.TOKEN_SERVICE_ENABLED_SETTING.getKey(), false) .build(), Clock.systemUTC(), client, licenseState, securityContext, securityMainIndex, securityTokensIndex, clusterService); - IllegalStateException e = expectThrows(IllegalStateException.class, + ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> tokenService.createOAuth2Tokens(null, null, null, true, null)); - assertEquals("security tokens are not enabled", e.getMessage()); + assertThat(e, throwableWithMessage("security tokens are not enabled")); + assertThat(e, instanceOf(FeatureNotEnabledException.class)); + // Client can check the metadata for this value, and depend on an exact string match: + assertThat(e.getMetadata(FeatureNotEnabledException.DISABLED_FEATURE_METADATA), contains("security_tokens")); PlainActionFuture future = new PlainActionFuture<>(); tokenService.getAndValidateToken(null, future); assertNull(future.get()); - e = expectThrows(IllegalStateException.class, () -> { - PlainActionFuture invalidateFuture = new PlainActionFuture<>(); - tokenService.invalidateAccessToken((String) null, invalidateFuture); - invalidateFuture.actionGet(); - }); - assertEquals("security tokens are not enabled", e.getMessage()); + PlainActionFuture invalidateFuture = new PlainActionFuture<>(); + e = expectThrows(ElasticsearchException.class, () -> tokenService.invalidateAccessToken((String) null, invalidateFuture)); + assertThat(e, throwableWithMessage("security tokens are not enabled")); + assertThat(e, instanceOf(FeatureNotEnabledException.class)); + // Client can check the metadata for this value, and depend on an exact string match: + assertThat(e.getMetadata(FeatureNotEnabledException.DISABLED_FEATURE_METADATA), contains("security_tokens")); } public void testBytesKeyEqualsHashCode() {