Browse Source

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
Jim Ferenczi 4 years ago
parent
commit
f67185f746

+ 5 - 0
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.

+ 1 - 0
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",

+ 4 - 0
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 <<delete-async-search,delete async search>> API for more informations.
+
 `create_snapshot`::
 Privileges to create snapshots for existing repositories. Can also list and view
 details on existing repositories and snapshots.

+ 2 - 1
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"
 }

+ 2 - 2
x-pack/plugin/async-search/qa/security/roles.yml

@@ -56,6 +56,6 @@ user-dls:
           }
         }
 
-user-manage:
+user-cancel:
   cluster:
-    - manage
+    - cancel_task

+ 14 - 9
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.arrayContainingInAnyOrder;
 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 @@ public class AsyncSearchSecurityIT extends ESRestTestCase {
             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 @@ public class AsyncSearchSecurityIT extends ESRestTestCase {
             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));

+ 7 - 7
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java

@@ -47,19 +47,19 @@ public class DeleteAsyncResultsService {
 
     public void deleteResponse(DeleteAsyncResultRequest request,
                              ActionListener<AcknowledgedResponse> 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<Boolean> consumer) {
+    private void hasCancelTaskPrivilegeAsync(Consumer<Boolean> 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 @@ public class DeleteAsyncResultsService {
     }
 
     private void deleteResponseAsync(DeleteAsyncResultRequest request,
-                                     boolean hasManagePrivilege,
+                                     boolean hasCancelTaskPrivilege,
                                      ActionListener<AcknowledgedResponse> 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,

+ 5 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java

@@ -148,6 +148,9 @@ public class ClusterPrivilegeResolver {
     public static final NamedClusterPrivilege MANAGE_LOGSTASH_PIPELINES = new ActionClusterPrivilege("manage_logstash_pipelines",
         Set.of("cluster:admin/logstash/pipeline/*"));
 
+    public static final NamedClusterPrivilege CANCEL_TASK = new ActionClusterPrivilege("cancel_task",
+        Set.of("cluster:admin/tasks/cancel"));
+
     private static final Map<String, NamedClusterPrivilege> VALUES = sortByAccessLevel(List.of(
         NONE,
         ALL,
@@ -187,7 +190,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.

+ 3 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java

@@ -123,7 +123,9 @@ public class ReservedRolesStore implements BiConsumer<Set<String>, ActionListene
                             // 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()

+ 1 - 1
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 + "*")

+ 6 - 0
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 class PrivilegeTests extends ESTestCase {
 
         }
     }
+
+    public void testCancelTasksPrivilege() {
+        verifyClusterActionAllowed(ClusterPrivilegeResolver.CANCEL_TASK, CancelTasksAction.NAME);
+        verifyClusterActionDenied(ClusterPrivilegeResolver.CANCEL_TASK, "cluster:admin/whatever");
+    }
 }

+ 1 - 1
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 }