Skip to content

Commit 2db163e

Browse files
authored
[8.19] [Transform] Fix transform validation to reject PUT and _start when user lacks remote index permissions (#142403) (#142455)
* [Transform] Fix transform validation to reject PUT and _start when user lacks remote index permissions (#142403) When a transform is configured with a remote (cross-cluster) source index and the user lacks permissions to access it, the _preview API correctly fails -- but PUT _transform and _start silently succeed, allowing unauthorized transforms to be created and started. The root cause is that validateQuery in AbstractCompositeAggFunction only checks the response status code, which is OK even when IndicesOptions.LENIENT_EXPAND_OPEN causes unauthorized indices to be silently ignored. The search returns null aggregations in this case, but unlike preview(), validateQuery() never checks for that condition. This PR introduces a SourceAccessDiagnostics class that inspects the SearchResponse for security-related failures at both the CCS cluster level (SKIPPED/FAILED clusters with ElasticsearchSecurityException) and the shard level (FORBIDDEN/UNAUTHORIZED status). A null-aggregation check is added to validateQuery(), but -- critically -- it only rejects the request when a security failure is positively identified. When no security failure is found, validation passes through silently. This distinction avoids the regression that caused PR #95318 to be reverted in #95562: that earlier change unconditionally failed on null aggregations, which broke integrations (such as Elastic Defend) that create and start transforms with wildcard source patterns before any matching indices exist. Since defer_validation only defers from PUT to _start, there was no way for those integrations to bypass the check. Our approach preserves backward compatibility for the empty-indices case while catching the unauthorized-remote-index case. The preview() method also delegates to the same diagnostics class, so all three APIs now produce consistent, actionable error messages when a security failure is detected, falling back to the original generic message otherwise. The multi-cluster YAML integration tests are updated to verify that both PUT _transform and _start now reject unauthorized remote transforms. A new test case creates a transform with defer_validation: true and confirms that _start catches the permission issue. Unit tests for SourceAccessDiagnostics cover cluster-level SKIPPED/FAILED scenarios, shard-level security exceptions, FORBIDDEN/UNAUTHORIZED status codes, and the fallback to the generic message for non-security failures. Fixes #95367 (cherry picked from commit 0e44984) # Conflicts: # x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java * fix compilation error * Add diagnostics for remote CCS clusters with zero shards Enhance the SourceAccessDiagnostics class to identify remote CCS clusters that return zero shards due to permission issues. This update includes a new method to check for such scenarios and updates the documentation accordingly. Additionally, new unit tests are added to verify the correct behavior when accessing remote clusters with insufficient permissions, ensuring that appropriate error messages are returned. This change improves the clarity of diagnostics related to security exceptions in cross-cluster searches. * fix unit test specifics for 8.19 * Update transform configuration in multi-cluster test to include defer_validation and modify description
1 parent f545c96 commit 2db163e

File tree

6 files changed

+515
-12
lines changed

6 files changed

+515
-12
lines changed

docs/changelog/142403.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
area: "Transform"
2+
issues:
3+
- 95367
4+
pr: 142403
5+
summary: Fix transform validation to reject PUT and `_start` when user lacks remote index permissions
6+
type: bug

x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityTransformIT.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.Map;
2424
import java.util.concurrent.atomic.AtomicReference;
2525

26+
import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue;
2627
import static org.hamcrest.Matchers.containsString;
2728
import static org.hamcrest.Matchers.equalTo;
2829
import static org.hamcrest.Matchers.nullValue;
@@ -201,7 +202,7 @@ public void testCrossClusterTransform() throws Exception {
201202
// Delete the transform
202203
assertOK(performRequestWithRemoteTransformUser(new Request("DELETE", "/_transform/simple-remote-transform")));
203204

204-
// Create a transform targeting an index without permission
205+
// Attempt to create a transform targeting an index without permission - should fail validation
205206
final var putTransformRequest2 = new Request("PUT", "/_transform/invalid");
206207
putTransformRequest2.setJsonEntity("""
207208
{
@@ -213,14 +214,14 @@ public void testCrossClusterTransform() throws Exception {
213214
}
214215
}
215216
""");
216-
assertOK(performRequestWithRemoteTransformUser(putTransformRequest2));
217-
// It errors when trying to preview it
218-
final ResponseException e = expectThrows(
217+
final ResponseException putException = expectThrows(
219218
ResponseException.class,
220-
() -> performRequestWithRemoteTransformUser(new Request("GET", "/_transform/invalid/_preview"))
219+
() -> performRequestWithRemoteTransformUser(putTransformRequest2)
221220
);
222-
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400));
223-
assertThat(e.getMessage(), containsString("Source indices have been deleted or closed"));
221+
assertThat(putException.getResponse().getStatusLine().getStatusCode(), equalTo(400));
222+
// Assert on response body so the test is robust across locales and exception message formatting
223+
final String putReason = (String) extractValue("error.reason", entityAsMap(putException.getResponse()));
224+
assertThat(putReason, containsString("lacks the required permissions"));
224225
}
225226
}
226227

x-pack/plugin/transform/qa/multi-cluster-tests-with-security/src/test/resources/rest-api-spec/test/multi_cluster/80_transform.yml

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ teardown:
151151
transform_id: "simple-remote-transform"
152152

153153
- do:
154-
catch: /Source indices have been deleted or closed./
154+
catch: /lacks the required permissions/
155155
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
156156
transform.preview_transform:
157157
transform_id: "simple-remote-transform"
@@ -306,6 +306,7 @@ teardown:
306306
---
307307
"Batch transform from remote cluster when the user is not authorized":
308308
- do:
309+
catch: /lacks the required permissions/
309310
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
310311
transform.put_transform:
311312
transform_id: "simple-remote-transform-3"
@@ -319,6 +320,30 @@ teardown:
319320
}
320321
}
321322
323+
---
324+
"Batch transform _start from remote cluster when the user is not authorized":
325+
- do:
326+
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
327+
transform.put_transform:
328+
transform_id: "simple-remote-transform-unauthorized-start"
329+
defer_validation: true
330+
body: >
331+
{
332+
"source": { "index": "my_remote_cluster:remote_test_index" },
333+
"dest": { "index": "simple-remote-transform-unauthorized-start" },
334+
"pivot": {
335+
"group_by": { "user": {"terms": {"field": "user"}}},
336+
"aggs": { "avg_stars": {"avg": {"field": "stars"}}}
337+
}
338+
}
339+
- match: { acknowledged: true }
340+
341+
- do:
342+
catch: /lacks the required permissions/
343+
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
344+
transform.start_transform:
345+
transform_id: "simple-remote-transform-unauthorized-start"
346+
322347
---
323348
"Batch transform update from remote cluster when the user is not authorized":
324349
- do:
@@ -339,16 +364,17 @@ teardown:
339364
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
340365
transform.update_transform:
341366
transform_id: "simple-remote-transform-2"
367+
defer_validation: true
342368
body: >
343369
{
344-
"source": { "index": "my_remote_cluster:remote_test_index" },
370+
"description": "Updated by bob",
345371
"dest": { "index": "simple-remote-transform-2" }
346372
}
347373
348374
---
349375
"Batch transform preview from remote cluster when the user is not authorized":
350376
- do:
351-
catch: /Source indices have been deleted or closed./
377+
catch: /lacks the required permissions/
352378
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
353379
transform.preview_transform:
354380
body: >
@@ -361,7 +387,7 @@ teardown:
361387
}
362388
}
363389
- do:
364-
catch: /Source indices have been deleted or closed./
390+
catch: /lacks the required permissions/
365391
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
366392
transform.preview_transform:
367393
body: >

x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/common/AbstractCompositeAggFunction.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public void preview(
8787
final InternalAggregations aggregations = r.getAggregations();
8888
if (aggregations == null) {
8989
listener.onFailure(
90-
new ElasticsearchStatusException("Source indices have been deleted or closed.", RestStatus.BAD_REQUEST)
90+
new ElasticsearchStatusException(SourceAccessDiagnostics.diagnoseSourceAccessFailure(r), RestStatus.BAD_REQUEST)
9191
);
9292
return;
9393
}
@@ -139,6 +139,19 @@ public void validateQuery(
139139
);
140140
return;
141141
}
142+
// Null aggregations may indicate permission issues when accessing remote indices.
143+
// We only fail validation when a security failure is positively identified (e.g.,
144+
// ElasticsearchSecurityException in cluster or shard failures). We deliberately do NOT
145+
// fail when no security failure is found, because null aggregations can also occur when
146+
// a local wildcard index pattern resolves to zero indices -- a legitimate scenario for
147+
// integrations that start transforms before source data exists (see #95562).
148+
if (response.getAggregations() == null) {
149+
String diagnosis = SourceAccessDiagnostics.diagnoseSourceAccessFailure(response);
150+
if (diagnosis.equals(SourceAccessDiagnostics.SOURCE_INDICES_MISSING) == false) {
151+
listener.onFailure(new ValidationException().addValidationError(diagnosis));
152+
return;
153+
}
154+
}
142155
listener.onResponse(true);
143156
}, e -> {
144157
Throwable unwrapped = ExceptionsHelper.unwrapCause(e);
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.transform.transforms.common;
9+
10+
import org.elasticsearch.ElasticsearchSecurityException;
11+
import org.elasticsearch.action.search.SearchResponse;
12+
import org.elasticsearch.action.search.ShardSearchFailure;
13+
import org.elasticsearch.rest.RestStatus;
14+
15+
/**
16+
* Diagnoses the cause of an empty or failed search response against transform source indices.
17+
* Inspects CCS cluster-level and shard-level failures to distinguish permission errors
18+
* from genuinely missing or closed indices.
19+
*/
20+
final class SourceAccessDiagnostics {
21+
22+
static final String SOURCE_INDICES_MISSING = "Source indices have been deleted or closed.";
23+
24+
private SourceAccessDiagnostics() {}
25+
26+
/**
27+
* Inspects a {@link SearchResponse} that returned null aggregations and produces
28+
* a human-readable error message identifying the likely cause.
29+
*
30+
* <p>The method checks, in order:
31+
* <ol>
32+
* <li>CCS cluster-level failures for security exceptions (SKIPPED or FAILED clusters)</li>
33+
* <li>Top-level shard failures for security exceptions</li>
34+
* <li>Remote CCS clusters with 0 total shards (security may have silently filtered all indices)</li>
35+
* <li>Falls back to a generic "deleted or closed" message</li>
36+
* </ol>
37+
*/
38+
static String diagnoseSourceAccessFailure(SearchResponse response) {
39+
String clusterSecurityError = findClusterSecurityFailure(response);
40+
if (clusterSecurityError != null) {
41+
return clusterSecurityError;
42+
}
43+
44+
String shardSecurityError = findShardSecurityFailure(response);
45+
if (shardSecurityError != null) {
46+
return shardSecurityError;
47+
}
48+
49+
String zeroShardsError = findRemoteClusterWithZeroShards(response);
50+
if (zeroShardsError != null) {
51+
return zeroShardsError;
52+
}
53+
54+
return SOURCE_INDICES_MISSING;
55+
}
56+
57+
/**
58+
* Checks CCS cluster-level information for clusters that were SKIPPED or FAILED
59+
* due to a security exception (e.g., user lacks permissions on the remote cluster).
60+
*/
61+
private static String findClusterSecurityFailure(SearchResponse response) {
62+
SearchResponse.Clusters clusters = response.getClusters();
63+
if (clusters == null || clusters.getTotal() == 0) {
64+
return null;
65+
}
66+
67+
for (String alias : clusters.getClusterAliases()) {
68+
SearchResponse.Cluster cluster = clusters.getCluster(alias);
69+
if (cluster == null) {
70+
continue;
71+
}
72+
if (cluster.getStatus() != SearchResponse.Cluster.Status.SKIPPED
73+
&& cluster.getStatus() != SearchResponse.Cluster.Status.FAILED) {
74+
continue;
75+
}
76+
for (ShardSearchFailure failure : cluster.getFailures()) {
77+
if (isSecurityFailure(failure)) {
78+
return "User lacks the required permissions to read source indices on cluster [" + alias + "].";
79+
}
80+
}
81+
}
82+
83+
return null;
84+
}
85+
86+
/**
87+
* Checks top-level shard failures for security exceptions (e.g., user lacks
88+
* permissions on a specific index).
89+
*/
90+
private static String findShardSecurityFailure(SearchResponse response) {
91+
for (ShardSearchFailure failure : response.getShardFailures()) {
92+
if (isSecurityFailure(failure)) {
93+
String index = failure.index();
94+
if (index != null) {
95+
return "User lacks the required permissions to read source index [" + index + "].";
96+
}
97+
return "User lacks the required permissions to read from the source indices.";
98+
}
99+
}
100+
return null;
101+
}
102+
103+
/**
104+
* Checks for remote CCS clusters that completed successfully but searched zero shards.
105+
* This pattern typically occurs when the security module silently filters out all
106+
* matching indices because the user lacks the required permissions. With lenient
107+
* index options, unauthorized indices are treated as unavailable, so no security
108+
* exception is thrown — the search simply returns zero results.
109+
*
110+
* <p>Only remote clusters are inspected. A local cluster with zero shards is considered
111+
* legitimate (e.g., a wildcard pattern before data exists — see #95562).
112+
*/
113+
private static String findRemoteClusterWithZeroShards(SearchResponse response) {
114+
SearchResponse.Clusters clusters = response.getClusters();
115+
if (clusters == null || clusters.getTotal() == 0) {
116+
return null;
117+
}
118+
119+
for (String alias : clusters.getClusterAliases()) {
120+
if (alias.isEmpty()) {
121+
continue; // skip the local cluster
122+
}
123+
SearchResponse.Cluster cluster = clusters.getCluster(alias);
124+
if (cluster == null) {
125+
continue;
126+
}
127+
Integer totalShards = cluster.getTotalShards();
128+
if (cluster.getStatus() == SearchResponse.Cluster.Status.SUCCESSFUL && totalShards != null && totalShards == 0) {
129+
return "User lacks the required permissions to read source indices on cluster [" + alias + "].";
130+
}
131+
}
132+
133+
return null;
134+
}
135+
136+
private static boolean isSecurityFailure(ShardSearchFailure failure) {
137+
if (failure.getCause() instanceof ElasticsearchSecurityException) {
138+
return true;
139+
}
140+
return failure.status() == RestStatus.FORBIDDEN || failure.status() == RestStatus.UNAUTHORIZED;
141+
}
142+
}

0 commit comments

Comments
 (0)