Browse Source

Service Accounts - fix privilege for get credentials (#72368)

The action of "get service account credentials" should be covered by the
manage_service_account privilege. But a bug makes it require read access for
the security index. This PR addresses it by executing the request with the
security origin.
Yang Wang 4 years ago
parent
commit
03459b0aed

+ 2 - 0
x-pack/plugin/security/qa/service-account/build.gradle

@@ -12,6 +12,7 @@ testClusters.javaRestTest {
   testDistribution = 'DEFAULT'
   numberOfNodes = 2
 
+  extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')
   extraConfigFile 'node.key', file('src/javaRestTest/resources/ssl/node.key')
   extraConfigFile 'node.crt', file('src/javaRestTest/resources/ssl/node.crt')
   extraConfigFile 'ca.crt', file('src/javaRestTest/resources/ssl/ca.crt')
@@ -41,4 +42,5 @@ testClusters.javaRestTest {
 
   user username: "test_admin", password: 'x-pack-test-password', role: "superuser"
   user username: "elastic/fleet-server", password: 'x-pack-test-password', role: "superuser"
+  user username: "service_account_manager", password: 'x-pack-test-password', role: "service_account_manager"
 }

+ 14 - 6
x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java

@@ -126,9 +126,17 @@ public class ServiceAccountIT extends ESRestTestCase {
         return "https";
     }
 
+    @Override
+    protected Settings restAdminSettings() {
+        final String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token)
+            .put(CERTIFICATE_AUTHORITIES, caPath)
+            .build();
+    }
+
     @Override
     protected Settings restClientSettings() {
-        String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
+        final String token = basicAuthHeaderValue("service_account_manager", new SecureString("x-pack-test-password".toCharArray()));
         return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token)
             .put(CERTIFICATE_AUTHORITIES, caPath)
             .build();
@@ -176,7 +184,7 @@ public class ServiceAccountIT extends ESRestTestCase {
         if (securityIndexExists) {
             final Request createRoleRequest = new Request("POST", "_security/role/dummy_role");
             createRoleRequest.setJsonEntity("{\"cluster\":[]}");
-            assertOK(client().performRequest(createRoleRequest));
+            assertOK(adminClient().performRequest(createRoleRequest));
         }
         final Request request = new Request("GET", "_security/_authenticate");
         request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + INVALID_SERVICE_TOKEN));
@@ -193,14 +201,14 @@ public class ServiceAccountIT extends ESRestTestCase {
     public void testAuthenticateShouldWorkWithOAuthBearerToken() throws IOException {
         final Request oauthTokenRequest = new Request("POST", "_security/oauth2/token");
         oauthTokenRequest.setJsonEntity("{\"grant_type\":\"password\",\"username\":\"test_admin\",\"password\":\"x-pack-test-password\"}");
-        final Response oauthTokenResponse = client().performRequest(oauthTokenRequest);
+        final Response oauthTokenResponse = adminClient().performRequest(oauthTokenRequest);
         assertOK(oauthTokenResponse);
         final Map<String, Object> oauthTokenResponseMap = responseAsMap(oauthTokenResponse);
         final String accessToken = (String) oauthTokenResponseMap.get("access_token");
 
         final Request request = new Request("GET", "_security/_authenticate");
         request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + accessToken));
-        final Response response = client().performRequest(request);
+        final Response response = adminClient().performRequest(request);
         assertOK(response);
         final Map<String, Object> responseMap = responseAsMap(response);
         assertThat(responseMap.get("username"), equalTo("test_admin"));
@@ -209,7 +217,7 @@ public class ServiceAccountIT extends ESRestTestCase {
         final String refreshToken = (String) oauthTokenResponseMap.get("refresh_token");
         final Request refreshTokenRequest = new Request("POST", "_security/oauth2/token");
         refreshTokenRequest.setJsonEntity("{\"grant_type\":\"refresh_token\",\"refresh_token\":\"" + refreshToken + "\"}");
-        final Response refreshTokenResponse = client().performRequest(refreshTokenRequest);
+        final Response refreshTokenResponse = adminClient().performRequest(refreshTokenRequest);
         assertOK(refreshTokenResponse);
     }
 
@@ -334,7 +342,7 @@ public class ServiceAccountIT extends ESRestTestCase {
     public void testClearCache() throws IOException {
         final Request clearCacheRequest = new Request("POST", "_security/service/elastic/fleet-server/credential/token/"
             + randomFrom("", "*", "api-token-1", "api-token-1,api-token2") + "/_clear_cache");
-        final Response clearCacheResponse = client().performRequest(clearCacheRequest);
+        final Response clearCacheResponse = adminClient().performRequest(clearCacheRequest);
         assertOK(clearCacheResponse);
         final Map<String, Object> clearCacheResponseMap = responseAsMap(clearCacheResponse);
         @SuppressWarnings("unchecked")

+ 3 - 0
x-pack/plugin/security/qa/service-account/src/javaRestTest/resources/roles.yml

@@ -0,0 +1,3 @@
+service_account_manager:
+  cluster:
+    - "manage_service_account"

+ 36 - 11
x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountSingleNodeTests.java

@@ -34,13 +34,37 @@ import org.elasticsearch.xpack.core.security.user.User;
 
 import java.util.Map;
 
+import static org.elasticsearch.test.SecuritySettingsSource.TEST_PASSWORD_HASHED;
 import static org.elasticsearch.test.SecuritySettingsSource.addSSLSettingsForNodePEMFiles;
+import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD;
+import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
 public class ServiceAccountSingleNodeTests extends SecuritySingleNodeTestCase {
 
     private static final String BEARER_TOKEN = "AAEAAWVsYXN0aWMvZmxlZXQtc2VydmVyL3Rva2VuMTpyNXdkYmRib1FTZTl2R09Ld2FKR0F3";
+    private static final String SERVICE_ACCOUNT_MANAGER_NAME = "service_account_manager";
+
+    @Override
+    protected String configUsers() {
+        return super.configUsers()
+            + SERVICE_ACCOUNT_MANAGER_NAME + ":" + TEST_PASSWORD_HASHED + "\n";
+    }
+
+    @Override
+    protected String configRoles() {
+        return super.configRoles()
+            + SERVICE_ACCOUNT_MANAGER_NAME + ":\n"
+            + "  cluster:\n"
+            + "    - 'manage_service_account'\n";
+    }
+
+    @Override
+    protected String configUsersRoles() {
+        return super.configUsersRoles()
+            + SERVICE_ACCOUNT_MANAGER_NAME + ":" + SERVICE_ACCOUNT_MANAGER_NAME + "\n";
+    }
 
     @Override
     protected Settings nodeSettings() {
@@ -80,25 +104,20 @@ public class ServiceAccountSingleNodeTests extends SecuritySingleNodeTestCase {
     public void testApiServiceAccountToken() {
         final IndexServiceAccountTokenStore store = node().injector().getInstance(IndexServiceAccountTokenStore.class);
         final Cache<String, ListenableFuture<CachingServiceAccountTokenStore.CachedResult>> cache = store.getCache();
-        final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
-            new CreateServiceAccountTokenRequest("elastic", "fleet-server", "api-token-1");
-        final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
-            client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
-        assertThat(createServiceAccountTokenResponse.getName(), equalTo("api-token-1"));
+        final SecureString secretValue1 = createApiServiceToken("api-token-1");
         assertThat(cache.count(), equalTo(0));
 
         final AuthenticateRequest authenticateRequest = new AuthenticateRequest("elastic/fleet-server");
-        final AuthenticateResponse authenticateResponse =
-            createServiceAccountClient(createServiceAccountTokenResponse.getValue().toString())
-                .execute(AuthenticateAction.INSTANCE, authenticateRequest).actionGet();
+        final AuthenticateResponse authenticateResponse = createServiceAccountClient(secretValue1.toString())
+            .execute(AuthenticateAction.INSTANCE, authenticateRequest).actionGet();
         assertThat(authenticateResponse.authentication(), equalTo(getExpectedAuthentication("api-token-1")));
         // cache is populated after authenticate
         assertThat(cache.count(), equalTo(1));
 
         final DeleteServiceAccountTokenRequest deleteServiceAccountTokenRequest =
             new DeleteServiceAccountTokenRequest("elastic", "fleet-server", "api-token-1");
-        final DeleteServiceAccountTokenResponse deleteServiceAccountTokenResponse =
-            client().execute(DeleteServiceAccountTokenAction.INSTANCE, deleteServiceAccountTokenRequest).actionGet();
+        final DeleteServiceAccountTokenResponse deleteServiceAccountTokenResponse = createServiceAccountManagerClient()
+            .execute(DeleteServiceAccountTokenAction.INSTANCE, deleteServiceAccountTokenRequest).actionGet();
         assertThat(deleteServiceAccountTokenResponse.found(), is(true));
         // cache is cleared after token deletion
         assertThat(cache.count(), equalTo(0));
@@ -138,6 +157,11 @@ public class ServiceAccountSingleNodeTests extends SecuritySingleNodeTestCase {
         assertThat(cache.count(), equalTo(1));
     }
 
+    private Client createServiceAccountManagerClient() {
+        return client().filterWithHeader(Map.of("Authorization",
+            basicAuthHeaderValue(SERVICE_ACCOUNT_MANAGER_NAME, new SecureString(TEST_PASSWORD.toCharArray()))));
+    }
+
     private Client createServiceAccountClient() {
         return createServiceAccountClient(BEARER_TOKEN);
     }
@@ -160,7 +184,8 @@ public class ServiceAccountSingleNodeTests extends SecuritySingleNodeTestCase {
         final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
             new CreateServiceAccountTokenRequest("elastic", "fleet-server", tokenName);
         final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
-            client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
+            createServiceAccountManagerClient().execute(
+                CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
         assertThat(createServiceAccountTokenResponse.getName(), equalTo(tokenName));
         return createServiceAccountTokenResponse.getValue();
     }

+ 24 - 16
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/IndexServiceAccountTokenStore.java

@@ -32,6 +32,7 @@ import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.CharArrays;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.query.BoolQueryBuilder;
@@ -57,6 +58,7 @@ import java.time.Clock;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.Supplier;
 
 import static org.elasticsearch.action.bulk.TransportSingleItemBulkWriteAction.toSingleItemBulkRequest;
 import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
@@ -145,22 +147,28 @@ public class IndexServiceAccountTokenStore extends CachingServiceAccountTokenSto
         } else if (false == frozenSecurityIndex.isAvailable()) {
             listener.onFailure(frozenSecurityIndex.getUnavailableReason());
         } else {
-            // TODO: wildcard support?
-            final BoolQueryBuilder query = QueryBuilders.boolQuery()
-                .filter(QueryBuilders.termQuery("doc_type", SERVICE_ACCOUNT_TOKEN_DOC_TYPE))
-                .must(QueryBuilders.termQuery("username", accountId.asPrincipal()));
-            final SearchRequest request = client.prepareSearch(SECURITY_MAIN_ALIAS)
-                .setScroll(DEFAULT_KEEPALIVE_SETTING.get(getSettings()))
-                .setQuery(query)
-                .setSize(1000)
-                .setFetchSource(false)
-                .request();
-            request.indicesOptions().ignoreUnavailable();
-
-            logger.trace("Searching tokens for service account [{}]", accountId);
-            ScrollHelper.fetchAllByEntity(client, request,
-                new ContextPreservingActionListener<>(client.threadPool().getThreadContext().newRestorableContext(false), listener),
-                hit -> extractTokenInfo(hit.getId(), accountId));
+            securityIndex.checkIndexVersionThenExecute(listener::onFailure, () -> {
+                final Supplier<ThreadContext.StoredContext> contextSupplier =
+                    client.threadPool().getThreadContext().newRestorableContext(false);
+                try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
+                    // TODO: wildcard support?
+                    final BoolQueryBuilder query = QueryBuilders.boolQuery()
+                        .filter(QueryBuilders.termQuery("doc_type", SERVICE_ACCOUNT_TOKEN_DOC_TYPE))
+                        .must(QueryBuilders.termQuery("username", accountId.asPrincipal()));
+                    final SearchRequest request = client.prepareSearch(SECURITY_MAIN_ALIAS)
+                        .setScroll(DEFAULT_KEEPALIVE_SETTING.get(getSettings()))
+                        .setQuery(query)
+                        .setSize(1000)
+                        .setFetchSource(false)
+                        .request();
+                    request.indicesOptions().ignoreUnavailable();
+
+                    logger.trace("Searching tokens for service account [{}]", accountId);
+                    ScrollHelper.fetchAllByEntity(client, request,
+                        new ContextPreservingActionListener<>(contextSupplier, listener),
+                        hit -> extractTokenInfo(hit.getId(), accountId));
+                }
+            });
         }
     }