From e820de07826b6926cdf1e0c5171ac36022a4bb3b Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 16 Feb 2021 10:56:17 +0100 Subject: [PATCH] Add a cluster privilege to cancel tasks and delete async searches (#68679) This change adds a new cluster privilege cancel_task that allows to: Cancel running tasks (_tasks/_cancel). Cancel and delete async searches. Today the 'manage' cluster privilege is required to cancel tasks and to delete async searches when security features are enabled. This new focused privilege allows to handle tasks and searches only. The change also adds the privilege to the internal 'kibana_system' and '_async_search' roles. They both need to be able to cancel tasks and delete async searches. Relates #67965 --- docs/reference/search/async-search.asciidoc | 5 ++++ .../security/get-builtin-privileges.asciidoc | 1 + .../authorization/privileges.asciidoc | 4 ++++ .../async-search/qa/security/build.gradle | 3 ++- .../plugin/async-search/qa/security/roles.yml | 4 ++-- .../xpack/search/AsyncSearchSecurityIT.java | 23 +++++++++++-------- .../core/async/DeleteAsyncResultsService.java | 14 +++++------ .../privilege/ClusterPrivilegeResolver.java | 6 ++++- .../authz/store/ReservedRolesStore.java | 4 +++- .../core/security/user/AsyncSearchUser.java | 2 +- .../authz/privilege/PrivilegeTests.java | 6 +++++ .../test/privileges/11_builtin.yml | 2 +- 12 files changed, 51 insertions(+), 23 deletions(-) diff --git a/docs/reference/search/async-search.asciidoc b/docs/reference/search/async-search.asciidoc index 91afbf334e188..1327bb58e66f2 100644 --- a/docs/reference/search/async-search.asciidoc +++ b/docs/reference/search/async-search.asciidoc @@ -315,3 +315,8 @@ Otherwise, the saved search results are deleted. DELETE /_async_search/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsdzoxMDc= -------------------------------------------------- // TEST[continued s/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsdzoxMDc=/\${body.id}/] + +If the {es} {security-features} are enabled, the deletion of a specific async +search is restricted to: + * The authenticated user that submitted the original search request. + * Users that have the `cancel_task` cluster privilege. diff --git a/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc b/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc index 757e639d85435..cd7caffa85ffd 100644 --- a/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc +++ b/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc @@ -62,6 +62,7 @@ A successful call returns an object with "cluster" and "index" fields. { "cluster" : [ "all", + "cancel_task", "create_snapshot", "delegate_pki", "grant_api_key", diff --git a/x-pack/docs/en/security/authorization/privileges.asciidoc b/x-pack/docs/en/security/authorization/privileges.asciidoc index 9232c9344fb01..bd724acf98730 100644 --- a/x-pack/docs/en/security/authorization/privileges.asciidoc +++ b/x-pack/docs/en/security/authorization/privileges.asciidoc @@ -12,6 +12,10 @@ This section lists the privileges that you can assign to a role. All cluster administration operations, like snapshotting, node shutdown/restart, settings update, rerouting, or managing users and roles. +`cancel_task`:: +Privileges to cancel tasks and delete async searches. +See <> API for more informations. + `create_snapshot`:: Privileges to create snapshots for existing repositories. Can also list and view details on existing repositories and snapshots. diff --git a/x-pack/plugin/async-search/qa/security/build.gradle b/x-pack/plugin/async-search/qa/security/build.gradle index 8ccaca3c4b3f5..5f9a991e03f93 100644 --- a/x-pack/plugin/async-search/qa/security/build.gradle +++ b/x-pack/plugin/async-search/qa/security/build.gradle @@ -12,9 +12,10 @@ testClusters.all { setting 'xpack.license.self_generated.type', 'trial' setting 'xpack.security.enabled', 'true' extraConfigFile 'roles.yml', file('roles.yml') + user username: "test_kibana_user", password: "x-pack-test-password", role: "kibana_system" user username: "test-admin", password: 'x-pack-test-password', role: "test-admin" user username: "user1", password: 'x-pack-test-password', role: "user1" user username: "user2", password: 'x-pack-test-password', role: "user2" user username: "user-dls", password: 'x-pack-test-password', role: "user-dls" - user username: "user-manage", password: 'x-pack-test-password', role: "user-manage" + user username: "user-cancel", password: 'x-pack-test-password', role: "user-cancel" } diff --git a/x-pack/plugin/async-search/qa/security/roles.yml b/x-pack/plugin/async-search/qa/security/roles.yml index 52c247505afc6..a5f3c14b22ac0 100644 --- a/x-pack/plugin/async-search/qa/security/roles.yml +++ b/x-pack/plugin/async-search/qa/security/roles.yml @@ -56,6 +56,6 @@ user-dls: } } -user-manage: +user-cancel: cluster: - - manage + - cancel_task diff --git a/x-pack/plugin/async-search/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/search/AsyncSearchSecurityIT.java b/x-pack/plugin/async-search/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/search/AsyncSearchSecurityIT.java index ff607838bf327..e6a1d49d49dcc 100644 --- a/x-pack/plugin/async-search/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/search/AsyncSearchSecurityIT.java +++ b/x-pack/plugin/async-search/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/search/AsyncSearchSecurityIT.java @@ -44,6 +44,7 @@ import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; public class AsyncSearchSecurityIT extends ESRestTestCase { /** @@ -126,8 +127,8 @@ private void testCase(String user, String other) throws Exception { ResponseException exc = expectThrows(ResponseException.class, () -> getAsyncSearch(id, other)); assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404)); - // user-manage cannot access the result - exc = expectThrows(ResponseException.class, () -> getAsyncSearch(id, "user-manage")); + // user-cancel cannot access the result + exc = expectThrows(ResponseException.class, () -> getAsyncSearch(id, "user-cancel")); assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404)); // other cannot delete the result @@ -145,13 +146,17 @@ private void testCase(String user, String other) throws Exception { Response delResp = deleteAsyncSearch(id, user); assertOK(delResp); - // check that user with 'manage' privileges can delete an async - // search submitted by a different user - Response newResp = submitAsyncSearch(indexName, "foo:bar", TimeValue.timeValueSeconds(10), user); - assertOK(newResp); - String newId = extractResponseId(newResp); - delResp = deleteAsyncSearch(newId, "user-manage"); - assertOK(delResp); + // check that users with the 'cancel_task' privilege can delete an async + // search submitted by a different user. + for (String runAs : new String[] { "user-cancel", "test_kibana_user" }) { + Response newResp = submitAsyncSearch(indexName, "foo:bar", TimeValue.timeValueSeconds(10), user); + assertOK(newResp); + String newId = extractResponseId(newResp); + exc = expectThrows(ResponseException.class, () -> getAsyncSearch(id, runAs)); + assertThat(exc.getResponse().getStatusLine().getStatusCode(), greaterThan(400)); + delResp = deleteAsyncSearch(newId, runAs); + assertOK(delResp); + } } ResponseException exc = expectThrows(ResponseException.class, () -> submitAsyncSearch("index-" + other, "*", TimeValue.timeValueSeconds(10), user)); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java index 2d533604d2847..b917d63178217 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java @@ -47,19 +47,19 @@ public DeleteAsyncResultsService(AsyncTaskIndexService listener) { - hasManagePrivilegeAsync(resp -> deleteResponseAsync(request, resp, listener)); + hasCancelTaskPrivilegeAsync(resp -> deleteResponseAsync(request, resp, listener)); } /** - * Checks if the authenticated user has the right privilege (manage) to + * Checks if the authenticated user has the right privilege (cancel_task) to * delete async search submitted by another user. */ - private void hasManagePrivilegeAsync(Consumer consumer) { + private void hasCancelTaskPrivilegeAsync(Consumer consumer) { final Authentication current = store.getAuthentication(); if (current != null) { HasPrivilegesRequest req = new HasPrivilegesRequest(); req.username(current.getUser().principal()); - req.clusterPrivileges(ClusterPrivilegeResolver.MANAGE.name()); + req.clusterPrivileges(ClusterPrivilegeResolver.CANCEL_TASK.name()); req.indexPrivileges(new RoleDescriptor.IndicesPrivileges[]{}); req.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[]{}); try { @@ -75,17 +75,17 @@ private void hasManagePrivilegeAsync(Consumer consumer) { } private void deleteResponseAsync(DeleteAsyncResultRequest request, - boolean hasManagePrivilege, + boolean hasCancelTaskPrivilege, ActionListener listener) { try { AsyncExecutionId searchId = AsyncExecutionId.decode(request.getId()); - AsyncTask task = hasManagePrivilege ? store.getTask(taskManager, searchId, AsyncTask.class) : + AsyncTask task = hasCancelTaskPrivilege ? store.getTask(taskManager, searchId, AsyncTask.class) : store.getTaskAndCheckAuthentication(taskManager, searchId, AsyncTask.class); if (task != null) { //the task was found and gets cancelled. The response may or may not be found, but we will return 200 anyways. task.cancelTask(taskManager, () -> deleteResponseFromIndex(searchId, true, listener), "cancelled by user"); } else { - if (hasManagePrivilege) { + if (hasCancelTaskPrivilege) { deleteResponseFromIndex(searchId, false, listener); } else { store.ensureAuthenticatedUserCanDeleteFromIndex(searchId, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java index bea2757056941..cb0259cef3528 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java @@ -160,6 +160,9 @@ public class ClusterPrivilegeResolver { public static final NamedClusterPrivilege MANAGE_LOGSTASH_PIPELINES = new ActionClusterPrivilege("manage_logstash_pipelines", Collections.unmodifiableSet(Sets.newHashSet("cluster:admin/logstash/pipeline/*"))); + public static final NamedClusterPrivilege CANCEL_TASK = new ActionClusterPrivilege("cancel_task", + Collections.unmodifiableSet(Sets.newHashSet("cluster:admin/tasks/cancel"))); + private static final Map VALUES = sortByAccessLevel(Arrays.asList( NONE, ALL, @@ -199,7 +202,8 @@ public class ClusterPrivilegeResolver { DELEGATE_PKI, MANAGE_OWN_API_KEY, MANAGE_ENRICH, - MANAGE_LOGSTASH_PIPELINES)); + MANAGE_LOGSTASH_PIPELINES, + CANCEL_TASK)); /** * Resolves a {@link NamedClusterPrivilege} from a given name if it exists. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java index 5891110f34ef7..3fbe41c1cc5e3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java @@ -123,7 +123,9 @@ private static Map initializeReservedRoles() { // The symbolic constant for this one is in SecurityActionMapper, so not accessible from X-Pack core "cluster:admin/analyze", // To facilitate using the file uploader functionality - "monitor_text_structure" + "monitor_text_structure", + // To cancel tasks and delete async searches + "cancel_task" }, new RoleDescriptor.IndicesPrivileges[] { RoleDescriptor.IndicesPrivileges.builder() diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/AsyncSearchUser.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/AsyncSearchUser.java index c526dc5ea7147..56d154bd2b683 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/AsyncSearchUser.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/AsyncSearchUser.java @@ -17,7 +17,7 @@ public class AsyncSearchUser extends User { public static final AsyncSearchUser INSTANCE = new AsyncSearchUser(); public static final String ROLE_NAME = UsernamesField.ASYNC_SEARCH_ROLE; public static final Role ROLE = Role.builder(new RoleDescriptor(ROLE_NAME, - null, + new String[] { "cancel_task" }, new RoleDescriptor.IndicesPrivileges[] { RoleDescriptor.IndicesPrivileges.builder() .indices(RestrictedIndicesNames.ASYNC_SEARCH_PREFIX + "*") diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java index 70cab2bab9ba9..1af274c964600 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.core.security.authz.privilege; import org.apache.lucene.util.automaton.Operations; +import org.elasticsearch.action.admin.cluster.node.tasks.cancel.CancelTasksAction; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.enrich.action.DeleteEnrichPolicyAction; @@ -281,4 +282,9 @@ public void testIngestPipelinePrivileges() { } } + + public void testCancelTasksPrivilege() { + verifyClusterActionAllowed(ClusterPrivilegeResolver.CANCEL_TASK, CancelTasksAction.NAME); + verifyClusterActionDenied(ClusterPrivilegeResolver.CANCEL_TASK, "cluster:admin/whatever"); + } } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml index 350408eba40b4..4274291bc9262 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml @@ -15,5 +15,5 @@ setup: # This is fragile - it needs to be updated every time we add a new cluster/index privilege # I would much prefer we could just check that specific entries are in the array, but we don't have # an assertion for that - - length: { "cluster" : 39 } + - length: { "cluster" : 40 } - length: { "index" : 19 }