Skip to content

Commit e33a1c3

Browse files
luigidellaquilaluigidellaquila
authored
ES|QL: Fix queries with document level security on lookup indexes (elastic#120617) (elastic#120798)
1 parent 602bf1a commit e33a1c3

File tree

12 files changed

+437
-150
lines changed

12 files changed

+437
-150
lines changed

‎docs/changelog/120617.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 120617
2+
summary: Fix queries with document level security on lookup indexes
3+
area: ES|QL
4+
type: bug
5+
issues: [120509]

‎x-pack/plugin/build.gradle‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,5 +212,11 @@ tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
212212
task.skipTest("esql/180_match_operator/match with non text field", "Match operator can now be used on non-text fields")
213213
task.skipTest("esql/180_match_operator/match with functions", "Error message changed")
214214
task.skipTest("esql/40_unsupported_types/semantic_text declared in mapping", "The semantic text field format changed")
215+
task.skipTest("esql/190_lookup_join/Alias as lookup index", "LOOKUP JOIN does not support index aliases for now")
216+
task.skipTest("esql/190_lookup_join/alias-repeated-alias", "LOOKUP JOIN does not support index aliases for now")
217+
task.skipTest("esql/190_lookup_join/alias-repeated-index", "LOOKUP JOIN does not support index aliases for now")
218+
task.skipTest("esql/190_lookup_join/alias-pattern-multiple", "LOOKUP JOIN does not support index aliases for now")
219+
task.skipTest("esql/190_lookup_join/alias-pattern-single", "LOOKUP JOIN does not support index aliases for now")
220+
215221
})
216222

‎x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java‎

Lines changed: 142 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ public class EsqlSecurityIT extends ESRestTestCase {
5757
.user("user4", "x-pack-test-password", "user4", false)
5858
.user("user5", "x-pack-test-password", "user5", false)
5959
.user("fls_user", "x-pack-test-password", "fls_user", false)
60+
.user("fls_user2", "x-pack-test-password", "fls_user2", false)
61+
.user("fls_user3", "x-pack-test-password", "fls_user3", false)
62+
.user("fls_user4_1", "x-pack-test-password", "fls_user4_1", false)
63+
.user("dls_user", "x-pack-test-password", "dls_user", false)
6064
.user("metadata1_read2", "x-pack-test-password", "metadata1_read2", false)
6165
.user("alias_user1", "x-pack-test-password", "alias_user1", false)
6266
.user("alias_user2", "x-pack-test-password", "alias_user2", false)
@@ -92,7 +96,7 @@ private void indexDocument(String index, int id, double value, String org) throw
9296
public void indexDocuments() throws IOException {
9397
Settings lookupSettings = Settings.builder().put("index.mode", "lookup").build();
9498
String mapping = """
95-
"properties":{"value": {"type": "double"}, "org": {"type": "keyword"}}
99+
"properties":{"value": {"type": "double"}, "org": {"type": "keyword"}, "other": {"type": "keyword"}}
96100
""";
97101

98102
createIndex("index", Settings.EMPTY, mapping);
@@ -163,6 +167,32 @@ public void indexDocuments() throws IOException {
163167
""");
164168
assertOK(client().performRequest(aliasRequest));
165169
}
170+
171+
createMultiRoleUsers();
172+
}
173+
174+
private void createMultiRoleUsers() throws IOException {
175+
Request request = new Request("POST", "_security/user/dls_user2");
176+
request.setJsonEntity("""
177+
{
178+
"password" : "x-pack-test-password",
179+
"roles" : [ "dls_user", "dls_user2" ],
180+
"full_name" : "Test Role",
181+
"email" : "[email protected]"
182+
}
183+
""");
184+
assertOK(client().performRequest(request));
185+
186+
request = new Request("POST", "_security/user/fls_user4");
187+
request.setJsonEntity("""
188+
{
189+
"password" : "x-pack-test-password",
190+
"roles" : [ "fls_user4_1", "fls_user4_2" ],
191+
"full_name" : "Test Role",
192+
"email" : "[email protected]"
193+
}
194+
""");
195+
assertOK(client().performRequest(request));
166196
}
167197

168198
protected MapMatcher responseMatcher(Map<String, Object> result) {
@@ -553,25 +583,130 @@ public void testLookupJoinIndexAllowed() throws Exception {
553583
);
554584
assertThat(respMap.get("values"), equalTo(List.of(List.of(40.0, "sales"))));
555585

556-
// Alias, should find the index and the row
557-
resp = runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org");
586+
// Aliases are not allowed in LOOKUP JOIN
587+
var resp2 = expectThrows(
588+
ResponseException.class,
589+
() -> runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org")
590+
);
591+
592+
assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]"));
593+
assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
594+
595+
// Aliases are not allowed in LOOKUP JOIN, regardless of alias filters
596+
resp2 = expectThrows(
597+
ResponseException.class,
598+
() -> runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org")
599+
);
600+
assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]"));
601+
assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
602+
}
603+
604+
@SuppressWarnings("unchecked")
605+
public void testLookupJoinDocLevelSecurity() throws Exception {
606+
assumeTrue(
607+
"Requires LOOKUP JOIN capability",
608+
EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName()))
609+
);
610+
611+
Response resp = runESQLCommand("dls_user", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org");
612+
assertOK(resp);
613+
Map<String, Object> respMap = entityAsMap(resp);
614+
assertThat(
615+
respMap.get("columns"),
616+
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
617+
);
618+
619+
assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, null))));
620+
621+
resp = runESQLCommand("dls_user", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org");
622+
assertOK(resp);
623+
respMap = entityAsMap(resp);
624+
assertThat(
625+
respMap.get("columns"),
626+
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
627+
);
628+
assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing"))));
629+
630+
// same, but with a user that has two dls roles that allow him more visibility
631+
632+
resp = runESQLCommand("dls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org");
558633
assertOK(resp);
559634
respMap = entityAsMap(resp);
560635
assertThat(
561636
respMap.get("columns"),
562637
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
563638
);
564-
assertThat(respMap.get("values"), equalTo(List.of(List.of(31.0, "sales"))));
565639

566-
// Alias, for a row that's filtered out
567-
resp = runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org");
640+
assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, "sales"))));
641+
642+
resp = runESQLCommand("dls_user2", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org");
568643
assertOK(resp);
569644
respMap = entityAsMap(resp);
570645
assertThat(
571646
respMap.get("columns"),
572647
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
573648
);
574-
assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(123.0, null))));
649+
assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing"))));
650+
651+
}
652+
653+
@SuppressWarnings("unchecked")
654+
public void testLookupJoinFieldLevelSecurity() throws Exception {
655+
assumeTrue(
656+
"Requires LOOKUP JOIN capability",
657+
EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName()))
658+
);
659+
660+
Response resp = runESQLCommand("fls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value");
661+
assertOK(resp);
662+
Map<String, Object> respMap = entityAsMap(resp);
663+
assertThat(
664+
respMap.get("columns"),
665+
equalTo(
666+
List.of(
667+
Map.of("name", "x", "type", "double"),
668+
Map.of("name", "value", "type", "double"),
669+
Map.of("name", "org", "type", "keyword")
670+
)
671+
)
672+
);
673+
674+
resp = runESQLCommand("fls_user3", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value");
675+
assertOK(resp);
676+
respMap = entityAsMap(resp);
677+
assertThat(
678+
respMap.get("columns"),
679+
equalTo(
680+
List.of(
681+
Map.of("name", "x", "type", "double"),
682+
Map.of("name", "value", "type", "double"),
683+
Map.of("name", "org", "type", "keyword"),
684+
Map.of("name", "other", "type", "keyword")
685+
)
686+
)
687+
688+
);
689+
690+
resp = runESQLCommand("fls_user4", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value");
691+
assertOK(resp);
692+
respMap = entityAsMap(resp);
693+
assertThat(
694+
respMap.get("columns"),
695+
equalTo(
696+
List.of(
697+
Map.of("name", "x", "type", "double"),
698+
Map.of("name", "value", "type", "double"),
699+
Map.of("name", "org", "type", "keyword")
700+
)
701+
)
702+
);
703+
704+
ResponseException error = expectThrows(
705+
ResponseException.class,
706+
() -> runESQLCommand("fls_user4_1", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value")
707+
);
708+
assertThat(error.getMessage(), containsString("Unknown column [value] in right side of join"));
709+
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
575710
}
576711

577712
public void testLookupJoinIndexForbidden() throws Exception {

‎x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,53 @@ fls_user:
9393
field_security:
9494
grant: [ value ]
9595

96+
fls_user2:
97+
cluster: []
98+
indices:
99+
- names: [ 'lookup-user2' ]
100+
privileges: [ 'read' ]
101+
field_security:
102+
grant: [ "org", "value" ]
103+
104+
fls_user3:
105+
cluster: []
106+
indices:
107+
- names: [ 'lookup-user2' ]
108+
privileges: [ 'read' ]
109+
field_security:
110+
grant: [ "org", "value", "other" ]
111+
112+
fls_user4_1:
113+
cluster: []
114+
indices:
115+
- names: [ 'lookup-user2' ]
116+
privileges: [ 'read' ]
117+
field_security:
118+
grant: [ "org" ]
119+
120+
fls_user4_2:
121+
cluster: []
122+
indices:
123+
- names: [ 'lookup-user2' ]
124+
privileges: [ 'read' ]
125+
field_security:
126+
grant: [ "value" ]
127+
128+
dls_user:
129+
cluster: []
130+
indices:
131+
- names: [ 'lookup-user2' ]
132+
privileges: [ 'read' ]
133+
query: '{"match": {"org": "marketing"}}'
134+
135+
dls_user2:
136+
cluster: []
137+
indices:
138+
- names: [ 'lookup-user2' ]
139+
privileges: [ 'read' ]
140+
query: '{"match": {"org": "sales"}}'
141+
142+
96143
logs_foo_all:
97144
cluster: []
98145
indices:

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,12 @@ public enum Cap {
629629
/**
630630
* Support named argument for function in map format.
631631
*/
632-
OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot());
632+
OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot()),
633+
634+
/**
635+
* Disabled support for index aliases in lookup joins
636+
*/
637+
LOOKUP_JOIN_NO_ALIASES(JOIN_LOOKUP_V12.isEnabled());
633638

634639
private final boolean enabled;
635640

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -237,36 +237,6 @@ private LogicalPlan resolveIndex(UnresolvedRelation plan, IndexResolution indexR
237237

238238
EsIndex esIndex = indexResolution.get();
239239

240-
if (plan.indexMode().equals(IndexMode.LOOKUP)) {
241-
String indexResolutionMessage = null;
242-
243-
var indexNameWithModes = esIndex.indexNameWithModes();
244-
if (indexNameWithModes.size() != 1) {
245-
indexResolutionMessage = "invalid ["
246-
+ table
247-
+ "] resolution in lookup mode to ["
248-
+ indexNameWithModes.size()
249-
+ "] indices";
250-
} else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) {
251-
indexResolutionMessage = "invalid ["
252-
+ table
253-
+ "] resolution in lookup mode to an index in ["
254-
+ indexNameWithModes.values().iterator().next()
255-
+ "] mode";
256-
}
257-
258-
if (indexResolutionMessage != null) {
259-
return new UnresolvedRelation(
260-
plan.source(),
261-
plan.table(),
262-
plan.frozen(),
263-
plan.metadataFields(),
264-
plan.indexMode(),
265-
indexResolutionMessage,
266-
plan.commandName()
267-
);
268-
}
269-
}
270240
var attributes = mappingAsAttributes(plan.source(), esIndex.mapping());
271241
attributes.addAll(plan.metadataFields());
272242
return new EsRelation(

0 commit comments

Comments
 (0)