Security fixes

GalLalouche · GalLalouche · commit 6134a8e89e9f · 2025-04-02T19:48:06.000+03:00
diff --git a/docs/reference/elasticsearch/security-privileges.md b/docs/reference/elasticsearch/security-privileges.md
@@ -194,6 +194,9 @@ This section lists the privileges that you can assign to a role.
 `monitor_enrich`
 :   All read-only operations related to managing and executing enrich policies.
 
+`monitor_esql`
+:   All read-only operations related to ES|QL queries.
+
 `monitor_inference`
 :   All read-only operations related to {{infer}}.
 
diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java
@@ -42,6 +42,7 @@
 import static org.elasticsearch.test.MapMatcher.matchesMap;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
 
 public class EsqlSecurityIT extends ESRestTestCase {
     @ClassRule
@@ -69,6 +70,8 @@ public class EsqlSecurityIT extends ESRestTestCase {
         .user("logs_foo_after_2021", "x-pack-test-password", "logs_foo_after_2021", false)
         .user("logs_foo_after_2021_pattern", "x-pack-test-password", "logs_foo_after_2021_pattern", false)
         .user("logs_foo_after_2021_alias", "x-pack-test-password", "logs_foo_after_2021_alias", false)
+        .user("user_without_monitor_privileges", "x-pack-test-password", "user_without_monitor_privileges", false)
+        .user("user_with_monitor_privileges", "x-pack-test-password", "user_with_monitor_privileges", false)
         .build();
 
     @Override
@@ -309,7 +312,7 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep
         json.endObject();
         Request searchRequest = new Request("GET", "/index-user1,index-user2/_search");
         searchRequest.setJsonEntity(Strings.toString(json));
-        searchRequest.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", "metadata1_read2"));
+        setUser(searchRequest, "metadata1_read2");
 
         // ES|QL query on the same index pattern
         var esqlResp = expectThrows(ResponseException.class, () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2"));
@@ -429,13 +432,13 @@ public void testFieldLevelSecurityAllow() throws Exception {
 
     public void testFieldLevelSecurityAllowPartial() throws Exception {
         Request request = new Request("GET", "/index*/_field_caps");
-        request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", "fls_user"));
+        setUser(request, "fls_user");
         request.addParameter("error_trace", "true");
         request.addParameter("pretty", "true");
         request.addParameter("fields", "*");
 
         request = new Request("GET", "/index*/_search");
-        request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", "fls_user"));
+        setUser(request, "fls_user");
         request.addParameter("error_trace", "true");
         request.addParameter("pretty", "true");
 
@@ -761,6 +764,36 @@ public void testFromLookupIndexForbidden() throws Exception {
         assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
     }
 
+    public void testListQueryAllowed() throws Exception {
+        Request request = new Request("GET", "_query/queries");
+        setUser(request, "user_with_monitor_privileges");
+        var resp = client().performRequest(request);
+        assertOK(resp);
+    }
+
+    public void testListQueryForbidden() throws Exception {
+        Request request = new Request("GET", "_query/queries");
+        setUser(request, "user_without_monitor_privileges");
+        var resp = expectThrows(ResponseException.class, () -> client().performRequest(request));
+        assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(403));
+        assertThat(resp.getMessage(), containsString("this action is granted by the cluster privileges [monitor_esql,monitor,manage,all]"));
+    }
+
+    public void testGetQueryAllowed() throws Exception {
+        // This is a bit tricky, since there is no such running query. We just make sure it didn't fail on forbidden privileges.
+        Request request = new Request("GET", "_query/queries/foo:1234");
+        var resp = expectThrows(ResponseException.class, () -> client().performRequest(request));
+        assertThat(resp.getResponse().getStatusLine().getStatusCode(), not(equalTo(404)));
+    }
+
+    public void testGetQueryForbidden() throws Exception {
+        Request request = new Request("GET", "_query/queries/foo:1234");
+        setUser(request, "user_without_monitor_privileges");
+        var resp = expectThrows(ResponseException.class, () -> client().performRequest(request));
+        assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(403));
+        assertThat(resp.getMessage(), containsString("this action is granted by the cluster privileges [monitor_esql,monitor,manage,all]"));
+    }
+
     private void createEnrichPolicy() throws Exception {
         createIndex("songs", Settings.EMPTY, """
             "properties":{"song_id": {"type": "keyword"}, "title": {"type": "keyword"}, "artist": {"type": "keyword"} }
@@ -837,11 +870,16 @@ protected Response runESQLCommand(String user, String command) throws IOExceptio
         json.endObject();
         Request request = new Request("POST", "_query");
         request.setJsonEntity(Strings.toString(json));
-        request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", user));
+        setUser(request, user);
         request.addParameter("error_trace", "true");
         return client().performRequest(request);
     }
 
+    private static void setUser(Request request, String user) {
+        request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("es-security-runas-user", user));
+
+    }
+
     static void addRandomPragmas(XContentBuilder builder) throws IOException {
         if (Build.current().isSnapshot()) {
             Settings pragmas = randomPragmas();
@@ -853,7 +891,7 @@ static void addRandomPragmas(XContentBuilder builder) throws IOException {
         }
     }
 
-    static Settings randomPragmas() {
+    private static Settings randomPragmas() {
         Settings.Builder settings = Settings.builder();
         if (randomBoolean()) {
             settings.put("page_size", between(1, 5));
diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml
@@ -193,3 +193,10 @@ logs_foo_after_2021_alias:
             "@timestamp": {"gte": "2021-01-01T00:00:00"}
           }
         }
+
+user_without_monitor_privileges:
+  cluster: []
+
+user_with_monitor_privileges:
+  cluster:
+    - monitor_esql
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlGetQueryAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlGetQueryAction.java
@@ -8,10 +8,12 @@
 package org.elasticsearch.xpack.esql.plugin;
 
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskRequestBuilder;
+import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskRequest;
 import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskResponse;
-import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequestBuilder;
+import org.elasticsearch.action.admin.cluster.node.tasks.get.TransportGetTaskAction;
+import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest;
 import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse;
+import org.elasticsearch.action.admin.cluster.node.tasks.list.TransportListTasksAction;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.HandledTransportAction;
 import org.elasticsearch.client.internal.node.NodeClient;
@@ -21,9 +23,12 @@
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.tasks.TaskInfo;
 import org.elasticsearch.transport.TransportService;
+import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.esql.action.EsqlGetQueryAction;
 import org.elasticsearch.xpack.esql.action.EsqlGetQueryRequest;
 
+import static org.elasticsearch.xpack.core.ClientHelper.ESQL_ORIGIN;
+
 public class TransportEsqlGetQueryAction extends HandledTransportAction<EsqlGetQueryRequest, EsqlGetQueryResponse> {
     private final NodeClient nodeClient;
 
@@ -35,33 +40,44 @@ public TransportEsqlGetQueryAction(TransportService transportService, NodeClient
 
     @Override
     protected void doExecute(Task task, EsqlGetQueryRequest request, ActionListener<EsqlGetQueryResponse> listener) {
-        new GetTaskRequestBuilder(nodeClient).setTaskId(request.id()).execute(new ActionListener<>() {
-            @Override
-            public void onResponse(GetTaskResponse response) {
-                TaskInfo task = response.getTask().getTask();
-                new ListTasksRequestBuilder(nodeClient).setDetailed(true)
-                    .setActions(DriverTaskRunner.ACTION_NAME)
-                    .setTargetParentTaskId(request.id())
-                    .execute(new ActionListener<>() {
-                        @Override
-                        public void onResponse(ListTasksResponse response) {
-                            listener.onResponse(new EsqlGetQueryResponse(toDetailedQuery(task, response)));
-                        }
+        ClientHelper.executeAsyncWithOrigin(
+            nodeClient,
+            ESQL_ORIGIN,
+            TransportGetTaskAction.TYPE,
+            new GetTaskRequest().setTaskId(request.id()),
+            new ActionListener<>() {
+                @Override
+                public void onResponse(GetTaskResponse response) {
+                    TaskInfo task = response.getTask().getTask();
+                    ClientHelper.executeAsyncWithOrigin(
+                        nodeClient,
+                        ESQL_ORIGIN,
+                        TransportListTasksAction.TYPE,
+                        new ListTasksRequest().setDetailed(true)
+                            .setActions(DriverTaskRunner.ACTION_NAME)
+                            .setTargetParentTaskId(request.id()),
+                        new ActionListener<>() {
+                            @Override
+                            public void onResponse(ListTasksResponse response) {
+                                listener.onResponse(new EsqlGetQueryResponse(toDetailedQuery(task, response)));
+                            }
 
-                        @Override
-                        public void onFailure(Exception e) {
-                            listener.onFailure(e);
+                            @Override
+                            public void onFailure(Exception e) {
+                                listener.onFailure(e);
+                            }
                         }
-                    });
-            }
+                    );
+                }
 
-            @Override
-            public void onFailure(Exception e) {
-                // The underlying root cause is meaningless to the user, but that is what will be shown, so we remove it.
-                var withoutCause = new Exception(e.getMessage());
-                listener.onFailure(withoutCause);
+                @Override
+                public void onFailure(Exception e) {
+                    // The underlying root cause is meaningless to the user, but that is what will be shown, so we remove it.
+                    var withoutCause = new Exception(e.getMessage());
+                    listener.onFailure(withoutCause);
+                }
             }
-        });
+        );
     }
 
     private static EsqlGetQueryResponse.DetailedQuery toDetailedQuery(TaskInfo task, ListTasksResponse response) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlListQueriesAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlListQueriesAction.java
@@ -8,8 +8,9 @@
 package org.elasticsearch.xpack.esql.plugin;
 
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequestBuilder;
+import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest;
 import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse;
+import org.elasticsearch.action.admin.cluster.node.tasks.list.TransportListTasksAction;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.HandledTransportAction;
 import org.elasticsearch.client.internal.node.NodeClient;
@@ -18,13 +19,16 @@
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.tasks.TaskInfo;
 import org.elasticsearch.transport.TransportService;
+import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.esql.action.EsqlListQueriesAction;
 import org.elasticsearch.xpack.esql.action.EsqlListQueriesRequest;
 import org.elasticsearch.xpack.esql.action.EsqlQueryAction;
 import org.elasticsearch.xpack.esql.core.async.AsyncTaskManagementService;
 
 import java.util.List;
 
+import static org.elasticsearch.xpack.core.ClientHelper.ESQL_ORIGIN;
+
 public class TransportEsqlListQueriesAction extends HandledTransportAction<EsqlListQueriesRequest, EsqlListQueriesResponse> {
     private final NodeClient nodeClient;
 
@@ -42,24 +46,28 @@ public TransportEsqlListQueriesAction(TransportService transportService, NodeCli
 
     @Override
     protected void doExecute(Task task, EsqlListQueriesRequest request, ActionListener<EsqlListQueriesResponse> listener) {
-        new ListTasksRequestBuilder(nodeClient).setActions(
-            EsqlQueryAction.NAME,
-            EsqlQueryAction.NAME + AsyncTaskManagementService.ASYNC_ACTION_SUFFIX
-        ).setDetailed(true).execute(new ActionListener<>() {
-            @Override
-            public void onResponse(ListTasksResponse response) {
-                List<EsqlListQueriesResponse.Query> queries = response.getTasks()
-                    .stream()
-                    .map(TransportEsqlListQueriesAction::toQuery)
-                    .toList();
-                listener.onResponse(new EsqlListQueriesResponse(queries));
-            }
+        ClientHelper.executeAsyncWithOrigin(
+            nodeClient,
+            ESQL_ORIGIN,
+            TransportListTasksAction.TYPE,
+            new ListTasksRequest().setActions(EsqlQueryAction.NAME, EsqlQueryAction.NAME + AsyncTaskManagementService.ASYNC_ACTION_SUFFIX)
+                .setDetailed(true),
+            new ActionListener<>() {
+                @Override
+                public void onResponse(ListTasksResponse response) {
+                    List<EsqlListQueriesResponse.Query> queries = response.getTasks()
+                        .stream()
+                        .map(TransportEsqlListQueriesAction::toQuery)
+                        .toList();
+                    listener.onResponse(new EsqlListQueriesResponse(queries));
+                }
 
-            @Override
-            public void onFailure(Exception e) {
-                listener.onFailure(e);
+                @Override
+                public void onFailure(Exception e) {
+                    listener.onFailure(e);
+                }
             }
-        });
+        );
     }
 
     private static EsqlListQueriesResponse.Query toQuery(TaskInfo taskInfo) {
diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml b/x-pack/plugin/src/yamlRestTest/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" : 62 }
+  - length: { "cluster" : 63 }
   - length: { "index" : 24 }

Original file line number	Diff line number	Diff line change
`@@ -193,3 +193,10 @@ logs_foo_after_2021_alias:`
`193`	`193`	`"@timestamp": {"gte": "2021-01-01T00:00:00"}`
`194`	`194`	`}`
`195`	`195`	`}`
	`196`	`+`
	`197`	`+user_without_monitor_privileges:`
	`198`	`+ cluster: []`
	`199`	`+`
	`200`	`+user_with_monitor_privileges:`
	`201`	`+ cluster:`
	`202`	`+ - monitor_esql`