diff --git a/docs/changelog/120617.yaml b/docs/changelog/120617.yaml new file mode 100644 index 0000000000000..cdf93ef4e71f2 --- /dev/null +++ b/docs/changelog/120617.yaml @@ -0,0 +1,5 @@ +pr: 120617 +summary: Fix queries with document level security on lookup indexes +area: ES|QL +type: bug +issues: [120509] diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 5ab0112d822ce..5987f75f4f198 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -96,5 +96,11 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> task.skipTest("esql/180_match_operator/match with non text field", "Match operator can now be used on non-text fields") task.skipTest("esql/180_match_operator/match with functions", "Error message changed") task.skipTest("esql/40_unsupported_types/semantic_text declared in mapping", "The semantic text field format changed") + task.skipTest("esql/190_lookup_join/Alias as lookup index", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-repeated-alias", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-repeated-index", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-pattern-multiple", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-pattern-single", "LOOKUP JOIN does not support index aliases for now") + }) 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 index 5adac8fdd70d0..a809bd50a45b8 100644 --- 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 @@ -57,6 +57,10 @@ public class EsqlSecurityIT extends ESRestTestCase { .user("user4", "x-pack-test-password", "user4", false) .user("user5", "x-pack-test-password", "user5", false) .user("fls_user", "x-pack-test-password", "fls_user", false) + .user("fls_user2", "x-pack-test-password", "fls_user2", false) + .user("fls_user3", "x-pack-test-password", "fls_user3", false) + .user("fls_user4_1", "x-pack-test-password", "fls_user4_1", false) + .user("dls_user", "x-pack-test-password", "dls_user", false) .user("metadata1_read2", "x-pack-test-password", "metadata1_read2", false) .user("alias_user1", "x-pack-test-password", "alias_user1", false) .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 public void indexDocuments() throws IOException { Settings lookupSettings = Settings.builder().put("index.mode", "lookup").build(); String mapping = """ - "properties":{"value": {"type": "double"}, "org": {"type": "keyword"}} + "properties":{"value": {"type": "double"}, "org": {"type": "keyword"}, "other": {"type": "keyword"}} """; createIndex("index", Settings.EMPTY, mapping); @@ -163,6 +167,32 @@ public void indexDocuments() throws IOException { """); assertOK(client().performRequest(aliasRequest)); } + + createMultiRoleUsers(); + } + + private void createMultiRoleUsers() throws IOException { + Request request = new Request("POST", "_security/user/dls_user2"); + request.setJsonEntity(""" + { + "password" : "x-pack-test-password", + "roles" : [ "dls_user", "dls_user2" ], + "full_name" : "Test Role", + "email" : "test.role@example.com" + } + """); + assertOK(client().performRequest(request)); + + request = new Request("POST", "_security/user/fls_user4"); + request.setJsonEntity(""" + { + "password" : "x-pack-test-password", + "roles" : [ "fls_user4_1", "fls_user4_2" ], + "full_name" : "Test Role", + "email" : "test.role@example.com" + } + """); + assertOK(client().performRequest(request)); } protected MapMatcher responseMatcher(Map result) { @@ -553,25 +583,130 @@ public void testLookupJoinIndexAllowed() throws Exception { ); assertThat(respMap.get("values"), equalTo(List.of(List.of(40.0, "sales")))); - // Alias, should find the index and the row - resp = runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org"); + // Aliases are not allowed in LOOKUP JOIN + var resp2 = expectThrows( + ResponseException.class, + () -> runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org") + ); + + assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]")); + assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + + // Aliases are not allowed in LOOKUP JOIN, regardless of alias filters + resp2 = expectThrows( + ResponseException.class, + () -> runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org") + ); + assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]")); + assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + @SuppressWarnings("unchecked") + public void testLookupJoinDocLevelSecurity() throws Exception { + assumeTrue( + "Requires LOOKUP JOIN capability", + EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName())) + ); + + Response resp = runESQLCommand("dls_user", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) + ); + + assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, null)))); + + resp = runESQLCommand("dls_user", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) + ); + assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing")))); + + // same, but with a user that has two dls roles that allow him more visibility + + resp = runESQLCommand("dls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); assertOK(resp); respMap = entityAsMap(resp); assertThat( respMap.get("columns"), equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) ); - assertThat(respMap.get("values"), equalTo(List.of(List.of(31.0, "sales")))); - // Alias, for a row that's filtered out - resp = runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org"); + assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, "sales")))); + + resp = runESQLCommand("dls_user2", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); assertOK(resp); respMap = entityAsMap(resp); assertThat( respMap.get("columns"), equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) ); - assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(123.0, null)))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing")))); + + } + + @SuppressWarnings("unchecked") + public void testLookupJoinFieldLevelSecurity() throws Exception { + assumeTrue( + "Requires LOOKUP JOIN capability", + EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName())) + ); + + Response resp = runESQLCommand("fls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword") + ) + ) + ); + + resp = runESQLCommand("fls_user3", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword"), + Map.of("name", "other", "type", "keyword") + ) + ) + + ); + + resp = runESQLCommand("fls_user4", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword") + ) + ) + ); + + ResponseException error = expectThrows( + ResponseException.class, + () -> runESQLCommand("fls_user4_1", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value") + ); + assertThat(error.getMessage(), containsString("Unknown column [value] in right side of join")); + assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); } public void testLookupJoinIndexForbidden() throws Exception { 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 index f46e7ef56f3a1..745ae43cf640c 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml @@ -93,6 +93,53 @@ fls_user: field_security: grant: [ value ] +fls_user2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org", "value" ] + +fls_user3: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org", "value", "other" ] + +fls_user4_1: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org" ] + +fls_user4_2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "value" ] + +dls_user: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + query: '{"match": {"org": "marketing"}}' + +dls_user2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + query: '{"match": {"org": "sales"}}' + + logs_foo_all: cluster: [] indices: diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 12a25c9ce2453..182328b54c4c5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -751,7 +751,12 @@ public enum Cap { /** * Support named argument for function in map format. */ - OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot()); + OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot()), + + /** + * Disabled support for index aliases in lookup joins + */ + LOOKUP_JOIN_NO_ALIASES(JOIN_LOOKUP_V12.isEnabled()); private final boolean enabled; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index fc1b7f6329ab3..552e90e0e90f9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -237,36 +237,6 @@ private LogicalPlan resolveIndex(UnresolvedRelation plan, IndexResolution indexR EsIndex esIndex = indexResolution.get(); - if (plan.indexMode().equals(IndexMode.LOOKUP)) { - String indexResolutionMessage = null; - - var indexNameWithModes = esIndex.indexNameWithModes(); - if (indexNameWithModes.size() != 1) { - indexResolutionMessage = "invalid [" - + table - + "] resolution in lookup mode to [" - + indexNameWithModes.size() - + "] indices"; - } else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) { - indexResolutionMessage = "invalid [" - + table - + "] resolution in lookup mode to an index in [" - + indexNameWithModes.values().iterator().next() - + "] mode"; - } - - if (indexResolutionMessage != null) { - return new UnresolvedRelation( - plan.source(), - plan.table(), - plan.frozen(), - plan.metadataFields(), - plan.indexMode(), - indexResolutionMessage, - plan.commandName() - ); - } - } var attributes = mappingAsAttributes(plan.source(), esIndex.mapping()); attributes.addAll(plan.metadataFields()); return new EsRelation( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java index 961d74794961f..cb2582db2ad33 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java @@ -12,7 +12,6 @@ import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.UnavailableShardsException; import org.elasticsearch.action.support.ChannelActionListener; -import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; @@ -23,7 +22,6 @@ import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.compute.data.Block; @@ -67,15 +65,6 @@ import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportResponse; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.core.ClientHelper; -import org.elasticsearch.xpack.core.XPackSettings; -import org.elasticsearch.xpack.core.security.SecurityContext; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.support.Exceptions; -import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.Alias; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; @@ -93,7 +82,6 @@ import java.util.Objects; import java.util.concurrent.Executor; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.IntStream; /** @@ -132,10 +120,10 @@ */ public abstract class AbstractLookupService { private final String actionName; - private final ClusterService clusterService; + protected final ClusterService clusterService; private final LookupShardContextFactory lookupShardContextFactory; - private final TransportService transportService; - private final Executor executor; + protected final TransportService transportService; + protected final Executor executor; private final BigArrays bigArrays; private final BlockFactory blockFactory; private final LocalCircuitBreaker.SizeSettings localBreakerSettings; @@ -218,97 +206,43 @@ protected static QueryList termQueryList( * Perform the actual lookup. */ public final void lookupAsync(R request, CancellableTask parentTask, ActionListener> outListener) { - ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - ActionListener> listener = ContextPreservingActionListener.wrapPreservingContext(outListener, threadContext); - hasPrivilege(listener.delegateFailureAndWrap((delegate, ignored) -> { - ClusterState clusterState = clusterService.state(); - GroupShardsIterator shardIterators = clusterService.operationRouting() - .searchShards(clusterState, new String[] { request.index }, Map.of(), "_local"); - if (shardIterators.size() != 1) { - delegate.onFailure(new EsqlIllegalArgumentException("target index {} has more than one shard", request.index)); - return; - } - ShardIterator shardIt = shardIterators.get(0); - ShardRouting shardRouting = shardIt.nextOrNull(); - ShardId shardId = shardIt.shardId(); - if (shardRouting == null) { - delegate.onFailure(new UnavailableShardsException(shardId, "target index is not available")); - return; - } - DiscoveryNode targetNode = clusterState.nodes().get(shardRouting.currentNodeId()); - T transportRequest = transportRequest(request, shardId); - // TODO: handle retry and avoid forking for the local lookup - try (ThreadContext.StoredContext unused = threadContext.stashWithOrigin(ClientHelper.ENRICH_ORIGIN)) { - transportService.sendChildRequest( - targetNode, - actionName, - transportRequest, - parentTask, - TransportRequestOptions.EMPTY, - new ActionListenerResponseHandler<>( - delegate.map(LookupResponse::takePages), - in -> readLookupResponse(in, blockFactory), - executor - ) - ); - } - })); - } - - /** - * Get the privilege required to perform the lookup. - *

- * If null is returned, no privilege check will be performed. - *

- */ - @Nullable - protected abstract String getRequiredPrivilege(); - - private void hasPrivilege(ActionListener outListener) { - final Settings settings = clusterService.getSettings(); - String privilegeName = getRequiredPrivilege(); - if (privilegeName == null - || settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false - || XPackSettings.SECURITY_ENABLED.get(settings) == false) { - outListener.onResponse(null); + ClusterState clusterState = clusterService.state(); + GroupShardsIterator shardIterators = clusterService.operationRouting() + .searchShards(clusterState, new String[] { request.index }, Map.of(), "_local"); + if (shardIterators.size() != 1) { + outListener.onFailure(new EsqlIllegalArgumentException("target index {} has more than one shard", request.index)); return; } - final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext); - final User user = securityContext.getUser(); - if (user == null) { - outListener.onFailure(new IllegalStateException("missing or unable to read authentication info on request")); + ShardIterator shardIt = shardIterators.get(0); + ShardRouting shardRouting = shardIt.nextOrNull(); + ShardId shardId = shardIt.shardId(); + if (shardRouting == null) { + outListener.onFailure(new UnavailableShardsException(shardId, "target index is not available")); return; } - HasPrivilegesRequest request = new HasPrivilegesRequest(); - request.username(user.principal()); - request.clusterPrivileges(privilegeName); - request.indexPrivileges(new RoleDescriptor.IndicesPrivileges[0]); - request.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]); - ActionListener listener = outListener.delegateFailureAndWrap((l, resp) -> { - if (resp.isCompleteMatch()) { - l.onResponse(null); - return; - } - String detailed = resp.getClusterPrivileges() - .entrySet() - .stream() - .filter(e -> e.getValue() == false) - .map(e -> "privilege [" + e.getKey() + "] is missing") - .collect(Collectors.joining(", ")); - String message = "user [" - + user.principal() - + "] doesn't have " - + "sufficient privileges to perform enrich lookup: " - + detailed; - l.onFailure(Exceptions.authorizationError(message)); - }); - transportService.sendRequest( - transportService.getLocalNode(), - HasPrivilegesAction.NAME, - request, + DiscoveryNode targetNode = clusterState.nodes().get(shardRouting.currentNodeId()); + T transportRequest = transportRequest(request, shardId); + // TODO: handle retry and avoid forking for the local lookup + sendChildRequest(parentTask, outListener, targetNode, transportRequest); + } + + protected void sendChildRequest( + CancellableTask parentTask, + ActionListener> delegate, + DiscoveryNode targetNode, + T transportRequest + ) { + transportService.sendChildRequest( + targetNode, + actionName, + transportRequest, + parentTask, TransportRequestOptions.EMPTY, - new ActionListenerResponseHandler<>(listener, HasPrivilegesResponse::new, executor) + new ActionListenerResponseHandler<>( + delegate.map(LookupResponse::takePages), + in -> readLookupResponse(in, blockFactory), + executor + ) ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java index acb4206ad7af8..480b69ecd8e60 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java @@ -8,10 +8,16 @@ package org.elasticsearch.xpack.esql.enrich; import org.elasticsearch.TransportVersions; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionListenerResponseHandler; +import org.elasticsearch.action.support.ContextPreservingActionListener; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BlockFactory; import org.elasticsearch.compute.data.BlockStreamInput; @@ -23,9 +29,20 @@ import org.elasticsearch.index.mapper.RangeType; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.TaskId; +import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.ClientHelper; +import org.elasticsearch.xpack.core.XPackSettings; +import org.elasticsearch.xpack.core.security.SecurityContext; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; +import org.elasticsearch.xpack.core.security.support.Exceptions; +import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.action.EsqlQueryAction; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; @@ -36,6 +53,7 @@ import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; /** * {@link EnrichLookupService} performs enrich lookup for a given input page. @@ -90,11 +108,6 @@ protected QueryList queryList(TransportRequest request, SearchExecutionContext c }; } - @Override - protected String getRequiredPrivilege() { - return ClusterPrivilegeResolver.MONITOR_ENRICH.name(); - } - @Override protected LookupResponse createLookupResponse(List pages, BlockFactory blockFactory) throws IOException { if (pages.size() != 1) { @@ -270,4 +283,70 @@ protected void innerRelease() { } } } + + @Override + protected void sendChildRequest( + CancellableTask parentTask, + ActionListener> delegate, + DiscoveryNode targetNode, + TransportRequest transportRequest + ) { + ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); + ActionListener> listener = ContextPreservingActionListener.wrapPreservingContext(delegate, threadContext); + hasEnrichPrivilege(listener.delegateFailureAndWrap((l, ignored) -> { + // Since we just checked the needed privileges + // we can access the index regardless of the user/role that is executing the query + try (ThreadContext.StoredContext unused = threadContext.stashWithOrigin(ClientHelper.ENRICH_ORIGIN)) { + super.sendChildRequest(parentTask, l, targetNode, transportRequest); + } + })); + } + + protected void hasEnrichPrivilege(ActionListener outListener) { + final Settings settings = clusterService.getSettings(); + String privilegeName = ClusterPrivilegeResolver.MONITOR_ENRICH.name(); + if (privilegeName == null + || settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false + || XPackSettings.SECURITY_ENABLED.get(settings) == false) { + outListener.onResponse(null); + return; + } + final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); + final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext); + final User user = securityContext.getUser(); + if (user == null) { + outListener.onFailure(new IllegalStateException("missing or unable to read authentication info on request")); + return; + } + HasPrivilegesRequest request = new HasPrivilegesRequest(); + request.username(user.principal()); + request.clusterPrivileges(privilegeName); + request.indexPrivileges(new RoleDescriptor.IndicesPrivileges[0]); + request.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]); + ActionListener listener = outListener.delegateFailureAndWrap((l, resp) -> { + if (resp.isCompleteMatch()) { + l.onResponse(null); + return; + } + String detailed = resp.getClusterPrivileges() + .entrySet() + .stream() + .filter(e -> e.getValue() == false) + .map(e -> "privilege [" + e.getKey() + "] is missing") + .collect(Collectors.joining(", ")); + String message = "user [" + + user.principal() + + "] doesn't have " + + "sufficient privileges to perform enrich lookup: " + + detailed; + l.onFailure(Exceptions.authorizationError(message)); + }); + transportService.sendRequest( + transportService.getLocalNode(), + HasPrivilegesAction.NAME, + request, + TransportRequestOptions.EMPTY, + new ActionListenerResponseHandler<>(listener, HasPrivilegesResponse::new, executor) + ); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java index 9bea212a56aa8..131d8ddfa5ccd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java @@ -90,11 +90,6 @@ protected AbstractLookupService.LookupResponse readLookupResponse(StreamInput in return new LookupResponse(in, blockFactory); } - @Override - protected String getRequiredPrivilege() { - return null; - } - public static class Request extends AbstractLookupService.Request { private final String matchField; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java index 4e009156072df..c29cf0ec7f414 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java @@ -7,9 +7,13 @@ package org.elasticsearch.xpack.esql.plan.logical.join; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware; +import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.plan.logical.EsRelation; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.SurrogateLogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.UsingJoinType; @@ -17,12 +21,13 @@ import java.util.List; import static java.util.Collections.emptyList; +import static org.elasticsearch.xpack.esql.common.Failure.fail; import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT; /** * Lookup join - specialized LEFT (OUTER) JOIN between the main left side and a lookup index (index_mode = lookup) on the right. */ -public class LookupJoin extends Join implements SurrogateLogicalPlan { +public class LookupJoin extends Join implements SurrogateLogicalPlan, PostAnalysisVerificationAware { public LookupJoin(Source source, LogicalPlan left, LogicalPlan right, List joinFields) { this(source, left, right, new UsingJoinType(LEFT, joinFields), emptyList(), emptyList(), emptyList()); @@ -71,4 +76,31 @@ protected NodeInfo info() { config().rightFields() ); } + + @Override + public void postAnalysisVerification(Failures failures) { + super.postAnalysisVerification(failures); + right().forEachDown(EsRelation.class, esr -> { + var indexNameWithModes = esr.indexNameWithModes(); + if (indexNameWithModes.size() != 1) { + failures.add( + fail(esr, "invalid [{}] resolution in lookup mode to [{}] indices", esr.indexPattern(), indexNameWithModes.size()) + ); + } else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) { + failures.add( + fail( + esr, + "invalid [{}] resolution in lookup mode to an index in [{}] mode", + esr.indexPattern(), + indexNameWithModes.values().iterator().next() + ) + ); + } + + // this check is crucial for security: ES|QL would use the concrete indices, so it would bypass the security on the alias + if (esr.concreteIndices().contains(esr.indexPattern()) == false) { + failures.add(fail(this, "Aliases and index patterns are not allowed for LOOKUP JOIN [{}]", esr.indexPattern())); + } + }); + } } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml index e8c9df0d3287e..f72cdd65b275c 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml @@ -102,7 +102,10 @@ non-lookup index: - contains: { error.reason: "Found 1 problem\nline 1:45: invalid [test] resolution in lookup mode to an index in [standard] mode" } --- + "Alias as lookup index": + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -117,6 +120,8 @@ non-lookup index: --- alias-repeated-alias: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -131,6 +136,8 @@ alias-repeated-alias: --- alias-repeated-index: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -145,6 +152,8 @@ alias-repeated-index: --- alias-pattern-multiple: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -156,6 +165,8 @@ alias-pattern-multiple: --- alias-pattern-single: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml new file mode 100644 index 0000000000000..6f9b70b0d94f1 --- /dev/null +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml @@ -0,0 +1,68 @@ +--- +setup: + - requires: + test_runner_features: [capabilities, contains] + capabilities: + - method: POST + path: /_query + parameters: [] + capabilities: [lookup_join_no_aliases] + reason: "uses LOOKUP JOIN" + + - do: + cluster.put_component_template: + name: my_settings + body: + template: + settings: + index: + mode: lookup + + + - do: + cluster.put_component_template: + name: my_mappings + body: + template: + mappings: + properties: + "@timestamp": + type: date + x: + type: keyword + + - do: + indices.put_index_template: + name: my_index_template + body: + index_patterns: my_data_stream* + data_stream: {} + composed_of: [ "my_mappings", "my_settings" ] + priority: 500 + + + - do: + bulk: + index: "my_data_stream" + refresh: true + body: + - { "index": { } } + - { "x": "foo", "y": "y1" } + - { "index": { } } + - { "x": "bar", "y": "y2" } + + + +--- +"data streams not supported in LOOKUP JOIN": + - do: + esql.query: + body: + query: 'row x = "foo" | LOOKUP JOIN my_data_stream ON x' + catch: "bad_request" + + - match: { error.type: "verification_exception" } + - contains: { error.reason: "Found 1 problem\nline 1:17: Aliases and index patterns are not allowed for LOOKUP JOIN [my_data_stream]" } + + +