From b58b30e0161872692424e8ccdddfffe4e45f1df1 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 13 Aug 2018 15:30:46 -0700 Subject: [PATCH 1/4] add user authentication test for ILM --- .../plugin/ilm/qa/with-security/build.gradle | 7 +- x-pack/plugin/ilm/qa/with-security/roles.yml | 11 ++ .../xpack/security/PermissionsIT.java | 135 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/ilm/qa/with-security/roles.yml create mode 100644 x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java diff --git a/x-pack/plugin/ilm/qa/with-security/build.gradle b/x-pack/plugin/ilm/qa/with-security/build.gradle index 4d91988c7f773..1faa49de86314 100644 --- a/x-pack/plugin/ilm/qa/with-security/build.gradle +++ b/x-pack/plugin/ilm/qa/with-security/build.gradle @@ -13,7 +13,7 @@ task copyILMRestTests(type: Copy) { include 'rest-api-spec/test/ilm/**' } -def clusterCredentials = [username: System.getProperty('tests.rest.cluster.username', 'test_user'), +def clusterCredentials = [username: System.getProperty('tests.rest.cluster.username', 'test_admin'), password: System.getProperty('tests.rest.cluster.password', 'x-pack-test-password')] integTestRunner { @@ -29,6 +29,11 @@ integTestCluster { setting 'xpack.monitoring.enabled', 'false' setting 'xpack.ml.enabled', 'false' setting 'xpack.license.self_generated.type', 'trial' + extraConfigFile 'roles.yml', 'roles.yml' + setupCommand 'setupIlmUser', + 'bin/elasticsearch-users', + 'useradd', "test_ilm", + '-p', 'x-pack-test-password', '-r', "ilm_admin" setupCommand 'setupDummyUser', 'bin/elasticsearch-users', 'useradd', clusterCredentials.username, diff --git a/x-pack/plugin/ilm/qa/with-security/roles.yml b/x-pack/plugin/ilm/qa/with-security/roles.yml new file mode 100644 index 0000000000000..4023573ce1429 --- /dev/null +++ b/x-pack/plugin/ilm/qa/with-security/roles.yml @@ -0,0 +1,11 @@ +ilm_admin: + cluster: + - monitor + - manage + indices: + - names: [ 'ilm-*' ] + privileges: + - monitor + - manage + - read + - write \ No newline at end of file diff --git a/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java new file mode 100644 index 0000000000000..ec4b7cb8c43dd --- /dev/null +++ b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java @@ -0,0 +1,135 @@ +/* + * 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; + +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction; +import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy; +import org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings; +import org.elasticsearch.xpack.core.indexlifecycle.Phase; +import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Map; + +import static java.util.Collections.singletonMap; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; +import static org.hamcrest.Matchers.equalTo; + +public class PermissionsIT extends ESRestTestCase { + + private String deletePolicy = "deletePolicy"; + private Settings indexSettingsWithPolicy; + + @Override + protected Settings restClientSettings() { + String token = basicAuthHeaderValue("test_ilm", new SecureString("x-pack-test-password".toCharArray())); + return Settings.builder() + .put(ThreadContext.PREFIX + ".Authorization", token) + .build(); + } + + @Override + protected Settings restAdminSettings() { + String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray())); + return Settings.builder() + .put(ThreadContext.PREFIX + ".Authorization", token) + .build(); + } + + @Before + public void init() throws Exception { + Request request = new Request("PUT", "/_cluster/settings"); + XContentBuilder pollIntervalEntity = JsonXContent.contentBuilder(); + pollIntervalEntity.startObject(); + { + pollIntervalEntity.startObject("transient"); + { + pollIntervalEntity.field(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s"); + }pollIntervalEntity.endObject(); + } pollIntervalEntity.endObject(); + request.setJsonEntity(Strings.toString(pollIntervalEntity)); + assertOK(adminClient().performRequest(request)); + indexSettingsWithPolicy = Settings.builder() + .put(LifecycleSettings.LIFECYCLE_NAME, deletePolicy) + .put("number_of_shards", 1) + .put("number_of_replicas", 0) + .build(); + createNewSingletonPolicy(deletePolicy,"delete", new DeleteAction()); + } + + public void testCanManageIndexAndPolicyDifferentUsers() throws Exception { + String index = "ilm-00001"; + createIndexAsAdmin(index, indexSettingsWithPolicy, ""); + assertBusy(() -> assertFalse(indexExists(index))); + } + + /** + * This tests the awkward behavior where an admin can have permissions to create a policy, + * but then not have permissions to operate on an index that was later associated with that policy by another + * user + */ + @SuppressWarnings("unchecked") + public void testCanManageIndexWithNoPermissions() throws Exception { + createIndexAsAdmin("not-ilm", indexSettingsWithPolicy, ""); + Request request = new Request("GET", "/not-ilm/_ilm/explain"); + // test_ilm user does not have permissions on this index + ResponseException exception = expectThrows(ResponseException.class, () -> client().performRequest(request)); + assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); + + assertBusy(() -> { + Response response = adminClient().performRequest(request); + assertOK(response); + try (InputStream is = response.getEntity().getContent()) { + Map mapResponse = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + Map indexExplain = (Map) ((Map) mapResponse.get("indices")).get("not-ilm"); + assertThat(indexExplain.get("managed"), equalTo(true)); + assertThat(indexExplain.get("step"), equalTo("error")); + assertThat(indexExplain.get("failed_step"), equalTo("readonly")); + assertThat(indexExplain.get("step_info"), equalTo("permissionsss!")); + } + }); + } + + private void createNewSingletonPolicy(String policy, String phaseName, LifecycleAction action) throws IOException { + Phase phase = new Phase(phaseName, TimeValue.ZERO, singletonMap(action.getWriteableName(), action)); + LifecyclePolicy lifecyclePolicy = + new LifecyclePolicy(TimeseriesLifecycleType.INSTANCE, policy, singletonMap(phase.getName(), phase)); + XContentBuilder builder = jsonBuilder(); + lifecyclePolicy.toXContent(builder, null); + final StringEntity entity = new StringEntity( + "{ \"policy\":" + Strings.toString(builder) + "}", ContentType.APPLICATION_JSON); + Request request = new Request("PUT", "_ilm/" + policy); + request.setEntity(entity); + client().performRequest(request); + } + + private void createIndexAsAdmin(String name, Settings settings, String mapping) throws IOException { + Request request = new Request("PUT", "/" + name); + request.setJsonEntity("{\n \"settings\": " + Strings.toString(settings) + + ", \"mappings\" : {" + mapping + "} }"); + adminClient().performRequest(request); + } +} From 1af0dad7a7d8c1dde0e968330412a5fdccfd0244 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 20 Aug 2018 15:10:09 -0700 Subject: [PATCH 2/4] fix test --- .../xpack/security/PermissionsIT.java | 22 +++++++++---------- .../action/TransportPutLifecycleAction.java | 11 +++++----- .../security/authz/AuthorizationUtils.java | 2 ++ .../authz/AuthorizationUtilsTests.java | 2 +- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java index ec4b7cb8c43dd..4cbddb5d0f4bf 100644 --- a/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java +++ b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java @@ -26,7 +26,6 @@ import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy; import org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings; import org.elasticsearch.xpack.core.indexlifecycle.Phase; -import org.elasticsearch.xpack.core.indexlifecycle.TimeseriesLifecycleType; import org.junit.Before; import java.io.IOException; @@ -64,12 +63,10 @@ public void init() throws Exception { Request request = new Request("PUT", "/_cluster/settings"); XContentBuilder pollIntervalEntity = JsonXContent.contentBuilder(); pollIntervalEntity.startObject(); - { - pollIntervalEntity.startObject("transient"); - { - pollIntervalEntity.field(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s"); - }pollIntervalEntity.endObject(); - } pollIntervalEntity.endObject(); + pollIntervalEntity.startObject("transient"); + pollIntervalEntity.field(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s"); + pollIntervalEntity.endObject(); + pollIntervalEntity.endObject(); request.setJsonEntity(Strings.toString(pollIntervalEntity)); assertOK(adminClient().performRequest(request)); indexSettingsWithPolicy = Settings.builder() @@ -106,17 +103,18 @@ public void testCanManageIndexWithNoPermissions() throws Exception { Map mapResponse = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); Map indexExplain = (Map) ((Map) mapResponse.get("indices")).get("not-ilm"); assertThat(indexExplain.get("managed"), equalTo(true)); - assertThat(indexExplain.get("step"), equalTo("error")); + assertThat(indexExplain.get("step"), equalTo("ERROR")); assertThat(indexExplain.get("failed_step"), equalTo("readonly")); - assertThat(indexExplain.get("step_info"), equalTo("permissionsss!")); + Map stepInfo = (Map) indexExplain.get("step_info"); + assertThat(stepInfo.get("type"), equalTo("security_exception")); + assertThat(stepInfo.get("reason"), equalTo("action [indices:admin/settings/update] is unauthorized for user [test_ilm]")); } }); } private void createNewSingletonPolicy(String policy, String phaseName, LifecycleAction action) throws IOException { Phase phase = new Phase(phaseName, TimeValue.ZERO, singletonMap(action.getWriteableName(), action)); - LifecyclePolicy lifecyclePolicy = - new LifecyclePolicy(TimeseriesLifecycleType.INSTANCE, policy, singletonMap(phase.getName(), phase)); + LifecyclePolicy lifecyclePolicy = new LifecyclePolicy(policy, singletonMap(phase.getName(), phase)); XContentBuilder builder = jsonBuilder(); lifecyclePolicy.toXContent(builder, null); final StringEntity entity = new StringEntity( @@ -130,6 +128,6 @@ private void createIndexAsAdmin(String name, Settings settings, String mapping) Request request = new Request("PUT", "/" + name); request.setJsonEntity("{\n \"settings\": " + Strings.toString(settings) + ", \"mappings\" : {" + mapping + "} }"); - adminClient().performRequest(request); + assertOK(adminClient().performRequest(request)); } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java index db8f9ef65f250..c1f9a0c73ae74 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java @@ -59,7 +59,10 @@ protected Response newResponse() { } @Override - protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { + protected void masterOperation(Request request, ClusterState state, ActionListener listener) { + Map filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream() + .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); clusterService.submitStateUpdateTask("put-lifecycle-" + request.getPolicy().getName(), new AckedClusterStateUpdateTask(request, listener) { @Override @@ -81,10 +84,6 @@ public ClusterState execute(ClusterState currentState) throws Exception { } // NORELEASE Check if current step exists in new policy and if not move to next available step SortedMap newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas()); - - Map filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream() - .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); LifecyclePolicyMetadata lifecyclePolicyMetadata = new LifecyclePolicyMetadata(request.getPolicy(), filteredHeaders); newPolicies.put(lifecyclePolicyMetadata.getName(), lifecyclePolicyMetadata); IndexLifecycleMetadata newMetadata = new IndexLifecycleMetadata(newPolicies, OperationMode.RUNNING); @@ -99,4 +98,4 @@ public ClusterState execute(ClusterState currentState) throws Exception { protected ClusterBlockException checkBlock(Request request, ClusterState state) { return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java index 5d9176b18976e..02679a1dfc0dc 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationUtils.java @@ -24,6 +24,7 @@ import java.util.function.Predicate; import static org.elasticsearch.xpack.core.ClientHelper.DEPRECATION_ORIGIN; +import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.MONITORING_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.PERSISTENT_TASK_ORIGIN; @@ -111,6 +112,7 @@ public static void switchUserBasedOnActionOriginAndExecute(ThreadContext threadC case DEPRECATION_ORIGIN: case PERSISTENT_TASK_ORIGIN: case ROLLUP_ORIGIN: + case INDEX_LIFECYCLE_ORIGIN: securityContext.executeAsUser(XPackUser.INSTANCE, consumer, Version.CURRENT); break; default: diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java index a581d1abbb5df..905247cfad9a6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java @@ -140,7 +140,7 @@ public void testSwitchAndExecuteXpackUser() throws Exception { threadContext.putHeader(headerName, headerValue); threadContext.putTransient(ClientHelper.ACTION_ORIGIN_TRANSIENT_NAME, randomFrom(ClientHelper.ML_ORIGIN, ClientHelper.WATCHER_ORIGIN, ClientHelper.DEPRECATION_ORIGIN, - ClientHelper.MONITORING_ORIGIN, ClientHelper.PERSISTENT_TASK_ORIGIN)); + ClientHelper.MONITORING_ORIGIN, ClientHelper.PERSISTENT_TASK_ORIGIN, ClientHelper.INDEX_LIFECYCLE_ORIGIN)); AuthorizationUtils.switchUserBasedOnActionOriginAndExecute(threadContext, securityContext, consumer); From dfaa600f96c7e3821d95cfaa4cabac4418a32693 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 21 Aug 2018 06:40:39 -0700 Subject: [PATCH 3/4] add comments --- .../org/elasticsearch/xpack/security/PermissionsIT.java | 7 +++++++ .../indexlifecycle/action/TransportPutLifecycleAction.java | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java index 4cbddb5d0f4bf..9e71bfe994d03 100644 --- a/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java +++ b/x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java @@ -77,6 +77,13 @@ public void init() throws Exception { createNewSingletonPolicy(deletePolicy,"delete", new DeleteAction()); } + /** + * Tests that a policy that simply deletes an index after 0s succeeds when an index + * with user `test_admin` is created referencing a policy created by `test_ilm` when both + * users have read/write permissions on the the index. The goal is to verify that one + * does not need to be the same user who created both the policy and the index to have the + * index be properly managed by ILM. + */ public void testCanManageIndexAndPolicyDifferentUsers() throws Exception { String index = "ilm-00001"; createIndexAsAdmin(index, indexSettingsWithPolicy, ""); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java index c1f9a0c73ae74..79e907bb28cb8 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java @@ -60,6 +60,10 @@ protected Response newResponse() { @Override protected void masterOperation(Request request, ClusterState state, ActionListener listener) { + // headers from the thread context stored by the AuthenticationService to be shared between the + // REST layer and the Transport layer here must be accessed within this thread and not in the + // cluster state thread in the ClusterStateUpdateTask below since that thread does not share the + // same context, and therefore does not have access to the appropriate security headers. Map filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream() .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey())) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); From 595cdad5eb56e87e9b5e64ec4035b6012870f19a Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 21 Aug 2018 09:02:27 -0700 Subject: [PATCH 4/4] rename role to ilm --- x-pack/plugin/ilm/qa/with-security/build.gradle | 2 +- x-pack/plugin/ilm/qa/with-security/roles.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/ilm/qa/with-security/build.gradle b/x-pack/plugin/ilm/qa/with-security/build.gradle index 1faa49de86314..124db5135d83e 100644 --- a/x-pack/plugin/ilm/qa/with-security/build.gradle +++ b/x-pack/plugin/ilm/qa/with-security/build.gradle @@ -33,7 +33,7 @@ integTestCluster { setupCommand 'setupIlmUser', 'bin/elasticsearch-users', 'useradd', "test_ilm", - '-p', 'x-pack-test-password', '-r', "ilm_admin" + '-p', 'x-pack-test-password', '-r', "ilm" setupCommand 'setupDummyUser', 'bin/elasticsearch-users', 'useradd', clusterCredentials.username, diff --git a/x-pack/plugin/ilm/qa/with-security/roles.yml b/x-pack/plugin/ilm/qa/with-security/roles.yml index 4023573ce1429..baf89bea34568 100644 --- a/x-pack/plugin/ilm/qa/with-security/roles.yml +++ b/x-pack/plugin/ilm/qa/with-security/roles.yml @@ -1,4 +1,4 @@ -ilm_admin: +ilm: cluster: - monitor - manage