Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion x-pack/plugin/ilm/qa/with-security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"
setupCommand 'setupDummyUser',
'bin/elasticsearch-users',
'useradd', clusterCredentials.username,
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugin/ilm/qa/with-security/roles.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
ilm:
cluster:
- monitor
- manage
indices:
- names: [ 'ilm-*' ]
privileges:
- monitor
- manage
- read
- write
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* 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.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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a JavaDoc here explaining what this test class aims to test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes


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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to be safe? As I understand it, the rest test case uses the admin settings to reset the cluster to a clean state between tests, but here this admin user only has access to the ilm-* indices so I'm not sure if it will be able to clean up the the not-ilm index?

Copy link
Contributor Author

@talevy talevy Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_admin has full administrator authority. the client() is running as the ILM-specific, test_ilm, user. This is why wipeCluster() has not been throwing an exception and failing the tests when running as adminClient().

}

@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());
}

/**
* 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, "");
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<String, Object> mapResponse = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true);
Map<String, Object> indexExplain = (Map<String, Object>) ((Map<String, Object>) 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"));
Map<String, String> stepInfo = (Map<String, String>) 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(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 + "} }");
assertOK(adminClient().performRequest(request));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ protected Response newResponse() {
}

@Override
protected void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws Exception {
protected void masterOperation(Request request, ClusterState state, ActionListener<Response> 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<String, String> 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<Response>(request, listener) {
@Override
Expand All @@ -81,10 +88,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<String, LifecyclePolicyMetadata> newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas());

Map<String, String> 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);
Expand All @@ -99,4 +102,4 @@ public ClusterState execute(ClusterState currentState) throws Exception {
protected ClusterBlockException checkBlock(Request request, ClusterState state) {
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
}
}
}