Skip to content

Commit e7662d8

Browse files
przemekwitekprzemekwitek
authored
[Transform] Skip remote clusters when performing up front privileges validation (#91788)
1 parent 5b62b1f commit e7662d8

File tree

4 files changed

+85
-21
lines changed

4 files changed

+85
-21
lines changed

‎docs/changelog/91788.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 91788
2+
summary: Skip remote clusters when performing up front privileges validation
3+
area: Transform
4+
type: bug
5+
issues: []

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ setup:
2626
- do:
2727
security.put_role:
2828
name: "x_cluster_role"
29-
# gh#72715: the my_remote_cluster privileges should not be needed
3029
body: >
3130
{
3231
"cluster": [],
@@ -38,10 +37,6 @@ setup:
3837
{
3938
"names": ["simple-remote-transform*", "simple-local-remote-transform", "same-index-local-and-remote-transform"],
4039
"privileges": ["create_index", "index", "read"]
41-
},
42-
{
43-
"names": ["my_remote_cluster:remote_test_i*", "my_remote_cluster:aliased_test_index", "my_remote_cluster:same_index_local_and_remote"],
44-
"privileges": ["read", "view_index_metadata"]
4540
}
4641
]
4742
}
@@ -156,7 +151,7 @@ teardown:
156151
transform_id: "simple-remote-transform"
157152

158153
- do:
159-
catch: /Cannot preview transform \[simple-remote-transform\] because user bob lacks the required permissions \[my_remote_cluster:remote_test_index\*:\[read, view_index_metadata\], simple-remote-transform:\[\]\]/
154+
catch: /Source indices have been deleted or closed./
160155
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
161156
transform.preview_transform:
162157
transform_id: "simple-remote-transform"
@@ -311,14 +306,13 @@ teardown:
311306
---
312307
"Batch transform from remote cluster when the user is not authorized":
313308
- do:
314-
catch: /Cannot create transform \[simple-remote-transform\] because user bob lacks the required permissions \[my_remote_cluster:remote_test_index:\[read, view_index_metadata\], simple-remote-transform:\[\]\]/
315309
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
316310
transform.put_transform:
317-
transform_id: "simple-remote-transform"
311+
transform_id: "simple-remote-transform-3"
318312
body: >
319313
{
320314
"source": { "index": "my_remote_cluster:remote_test_index" },
321-
"dest": { "index": "simple-remote-transform" },
315+
"dest": { "index": "simple-remote-transform-3" },
322316
"pivot": {
323317
"group_by": { "user": {"terms": {"field": "user"}}},
324318
"aggs": { "avg_stars": {"avg": {"field": "stars"}}}
@@ -342,7 +336,6 @@ teardown:
342336
}
343337
- match: { acknowledged: true }
344338
- do:
345-
catch: /Cannot update transform \[simple-remote-transform-2\] because user bob lacks the required permissions \[my_remote_cluster:remote_test_index:\[read, view_index_metadata\], simple-remote-transform-2:\[\]\]/
346339
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
347340
transform.update_transform:
348341
transform_id: "simple-remote-transform-2"
@@ -355,7 +348,7 @@ teardown:
355348
---
356349
"Batch transform preview from remote cluster when the user is not authorized":
357350
- do:
358-
catch: /Cannot preview transform \[transform-preview\] because user bob lacks the required permissions \[my_remote_cluster:remote_test_index:\[read, view_index_metadata\], simple-remote-transform-2:\[\]\]/
351+
catch: /Source indices have been deleted or closed./
359352
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
360353
transform.preview_transform:
361354
body: >
@@ -368,7 +361,7 @@ teardown:
368361
}
369362
}
370363
- do:
371-
catch: /Cannot preview transform \[transform-preview\] because user bob lacks the required permissions \[my_remote_cluster:test_index:\[read, view_index_metadata\]\]/
364+
catch: /Source indices have been deleted or closed./
372365
headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" } # This is bob
373366
transform.preview_transform:
374367
body: >

‎x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransformPrivilegeChecker.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.cluster.ClusterState;
1414
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1515
import org.elasticsearch.common.Strings;
16+
import org.elasticsearch.license.RemoteClusterLicenseChecker;
1617
import org.elasticsearch.xpack.core.security.SecurityContext;
1718
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction;
1819
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest;
@@ -23,9 +24,11 @@
2324
import org.elasticsearch.xpack.core.transform.transforms.TransformConfig;
2425

2526
import java.util.ArrayList;
27+
import java.util.Arrays;
2628
import java.util.List;
2729
import java.util.Map;
2830

31+
import static java.util.function.Predicate.not;
2932
import static java.util.stream.Collectors.joining;
3033
import static java.util.stream.Collectors.toList;
3134
import static org.elasticsearch.xpack.transform.utils.SecondaryAuthorizationUtils.useSecondaryAuthIfAvailable;
@@ -60,7 +63,11 @@ static void checkPrivileges(
6063
username,
6164
checkDestIndexPrivileges
6265
);
63-
client.execute(HasPrivilegesAction.INSTANCE, hasPrivilegesRequest, hasPrivilegesResponseListener);
66+
if (hasPrivilegesRequest.indexPrivileges().length == 0) {
67+
listener.onResponse(null);
68+
} else {
69+
client.execute(HasPrivilegesAction.INSTANCE, hasPrivilegesRequest, hasPrivilegesResponseListener);
70+
}
6471
});
6572
}
6673

@@ -73,13 +80,19 @@ private static HasPrivilegesRequest buildPrivilegesRequest(
7380
) {
7481
List<RoleDescriptor.IndicesPrivileges> indicesPrivileges = new ArrayList<>(2);
7582

76-
RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
77-
.indices(config.getSource().getIndex())
78-
// We need to read the source indices mapping to deduce the destination mapping, hence the need for view_index_metadata
79-
.privileges("read", "view_index_metadata")
80-
.build();
81-
indicesPrivileges.add(sourceIndexPrivileges);
83+
// TODO: Remove this filter once https://github.com/elastic/elasticsearch/issues/67798 is fixed.
84+
String[] sourceIndex = Arrays.stream(config.getSource().getIndex())
85+
.filter(not(RemoteClusterLicenseChecker::isRemoteIndex))
86+
.toArray(String[]::new);
8287

88+
if (sourceIndex.length > 0) {
89+
RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
90+
.indices(sourceIndex)
91+
// We need to read the source indices mapping to deduce the destination mapping, hence the need for view_index_metadata
92+
.privileges("read", "view_index_metadata")
93+
.build();
94+
indicesPrivileges.add(sourceIndexPrivileges);
95+
}
8396
if (checkDestIndexPrivileges) {
8497
final String destIndex = config.getDestination().getIndex();
8598
final String[] concreteDest = indexNameExpressionResolver.concreteIndexNames(

‎x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/action/TransformPrivilegeCheckerTests.java‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static org.hamcrest.Matchers.emptyArray;
4646
import static org.hamcrest.Matchers.equalTo;
4747
import static org.hamcrest.Matchers.is;
48+
import static org.hamcrest.Matchers.nullValue;
4849

4950
public class TransformPrivilegeCheckerTests extends ESTestCase {
5051

@@ -61,6 +62,7 @@ public class TransformPrivilegeCheckerTests extends ESTestCase {
6162
.addPrivilege("view_index_metadata", true)
6263
.addPrivilege("read", false)
6364
.build();
65+
private static final String REMOTE_SOURCE_INDEX_NAME = "some-remote-cluster:some-remote-source-index";
6466
private static final String DEST_INDEX_NAME = "some-dest-index";
6567
private static final ResourcePrivileges DEST_INDEX_NAME_PRIVILEGES = ResourcePrivileges.builder(DEST_INDEX_NAME)
6668
.addPrivilege("index", true)
@@ -103,27 +105,48 @@ public void tearDownClient() {
103105
}
104106

105107
public void testCheckPrivileges_NoCheckDestIndexPrivileges() {
108+
TransformConfig config = new TransformConfig.Builder(TRANSFORM_CONFIG).setSource(
109+
new SourceConfig(SOURCE_INDEX_NAME, REMOTE_SOURCE_INDEX_NAME)
110+
).build();
106111
TransformPrivilegeChecker.checkPrivileges(
107112
OPERATION_NAME,
108113
securityContext,
109114
indexNameExpressionResolver,
110115
ClusterState.EMPTY_STATE,
111116
client,
112-
TRANSFORM_CONFIG,
117+
config,
113118
false,
114119
ActionListener.wrap(aVoid -> {
115120
HasPrivilegesRequest request = client.lastHasPrivilegesRequest;
116121
assertThat(request.username(), is(equalTo(USER_NAME)));
117122
assertThat(request.applicationPrivileges(), is(emptyArray()));
118123
assertThat(request.clusterPrivileges(), is(emptyArray()));
119-
assertThat(request.indexPrivileges(), is(arrayWithSize(1)));
124+
assertThat(request.indexPrivileges(), is(arrayWithSize(1))); // remote index is filtered out
120125
RoleDescriptor.IndicesPrivileges sourceIndicesPrivileges = request.indexPrivileges()[0];
121126
assertThat(sourceIndicesPrivileges.getIndices(), is(arrayContaining(SOURCE_INDEX_NAME)));
122127
assertThat(sourceIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "view_index_metadata")));
123128
}, e -> fail(e.getMessage()))
124129
);
125130
}
126131

132+
public void testCheckPrivileges_NoLocalIndices_NoCheckDestIndexPrivileges() {
133+
TransformConfig config = new TransformConfig.Builder(TRANSFORM_CONFIG).setSource(new SourceConfig(REMOTE_SOURCE_INDEX_NAME))
134+
.build();
135+
TransformPrivilegeChecker.checkPrivileges(
136+
OPERATION_NAME,
137+
securityContext,
138+
indexNameExpressionResolver,
139+
ClusterState.EMPTY_STATE,
140+
client,
141+
config,
142+
false,
143+
ActionListener.wrap(aVoid -> {
144+
// _has_privileges API is not called for the remote index
145+
assertThat(client.lastHasPrivilegesRequest, is(nullValue()));
146+
}, e -> fail(e.getMessage()))
147+
);
148+
}
149+
127150
public void testCheckPrivileges_CheckDestIndexPrivileges_DestIndexDoesNotExist() {
128151
TransformPrivilegeChecker.checkPrivileges(
129152
OPERATION_NAME,
@@ -180,6 +203,36 @@ public void testCheckPrivileges_CheckDestIndexPrivileges_DestIndexExists() {
180203
);
181204
}
182205

206+
public void testCheckPrivileges_NoLocalIndices_CheckDestIndexPrivileges_DestIndexExists() {
207+
ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT)
208+
.metadata(
209+
Metadata.builder()
210+
.put(IndexMetadata.builder(DEST_INDEX_NAME).settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(0))
211+
)
212+
.build();
213+
TransformConfig config = new TransformConfig.Builder(TRANSFORM_CONFIG).setSource(new SourceConfig(REMOTE_SOURCE_INDEX_NAME))
214+
.build();
215+
TransformPrivilegeChecker.checkPrivileges(
216+
OPERATION_NAME,
217+
securityContext,
218+
indexNameExpressionResolver,
219+
clusterState,
220+
client,
221+
config,
222+
true,
223+
ActionListener.wrap(aVoid -> {
224+
HasPrivilegesRequest request = client.lastHasPrivilegesRequest;
225+
assertThat(request.username(), is(equalTo(USER_NAME)));
226+
assertThat(request.applicationPrivileges(), is(emptyArray()));
227+
assertThat(request.clusterPrivileges(), is(emptyArray()));
228+
assertThat(request.indexPrivileges(), is(arrayWithSize(1)));
229+
RoleDescriptor.IndicesPrivileges destIndicesPrivileges = request.indexPrivileges()[0];
230+
assertThat(destIndicesPrivileges.getIndices(), is(arrayContaining(DEST_INDEX_NAME)));
231+
assertThat(destIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "index")));
232+
}, e -> fail(e.getMessage()))
233+
);
234+
}
235+
183236
public void testCheckPrivileges_CheckDestIndexPrivileges_DestIndexExistsWithRetentionPolicy() {
184237
ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT)
185238
.metadata(

0 commit comments

Comments
 (0)