Selaa lähdekoodia

[Transform] Improve error message when user lacks privilege in _preview endpoint (#72002)

Przemysław Witek 4 vuotta sitten
vanhempi
commit
51181d5d52

+ 6 - 7
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/action/PreviewTransformAction.java

@@ -44,6 +44,8 @@ public class PreviewTransformAction extends ActionType<PreviewTransformAction.Re
     public static final PreviewTransformAction INSTANCE = new PreviewTransformAction();
     public static final String NAME = "cluster:admin/transform/preview";
 
+    public static final String DUMMY_DEST_INDEX_FOR_PREVIEW = "unused-transform-preview-index";
+
     private PreviewTransformAction() {
         super(NAME, PreviewTransformAction.Response::new);
     }
@@ -63,18 +65,15 @@ public class PreviewTransformAction extends ActionType<PreviewTransformAction.Re
 
         public static Request fromXContent(final XContentParser parser) throws IOException {
             Map<String, Object> content = parser.map();
-            // dest.index and ID are not required for Preview, so we just supply our own
+            // dest.index is not required for _preview, so we just supply our own
             Map<String, String> tempDestination = new HashMap<>();
-            tempDestination.put(DestConfig.INDEX.getPreferredName(), "unused-transform-preview-index");
+            tempDestination.put(DestConfig.INDEX.getPreferredName(), DUMMY_DEST_INDEX_FOR_PREVIEW);
             // Users can still provide just dest.pipeline to preview what their data would look like given the pipeline ID
             Object providedDestination = content.get(TransformField.DESTINATION.getPreferredName());
             if (providedDestination instanceof Map) {
                 @SuppressWarnings("unchecked")
-                Map<String, String> destMap = (Map<String, String>) providedDestination;
-                String pipeline = destMap.get(DestConfig.PIPELINE.getPreferredName());
-                if (pipeline != null) {
-                    tempDestination.put(DestConfig.PIPELINE.getPreferredName(), pipeline);
-                }
+                Map<String, String> providedDestinationAsMap = (Map<String, String>) providedDestination;
+                tempDestination.putAll(providedDestinationAsMap);
             }
             content.put(TransformField.DESTINATION.getPreferredName(), tempDestination);
             content.putIfAbsent(TransformField.ID.getPreferredName(), "transform-preview");

+ 60 - 5
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/action/PreviewTransformActionRequestTests.java

@@ -22,6 +22,8 @@ import org.elasticsearch.xpack.core.transform.transforms.pivot.PivotConfigTests;
 import java.io.IOException;
 
 import static org.elasticsearch.xpack.core.transform.transforms.SourceConfigTests.randomSourceConfig;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 
 public class PreviewTransformActionRequestTests extends AbstractSerializingTransformTestCase<Request> {
 
@@ -60,13 +62,65 @@ public class PreviewTransformActionRequestTests extends AbstractSerializingTrans
         return new Request(config);
     }
 
+    public void testParsingOverwritesIdField() throws IOException {
+        testParsingOverwrites(
+            "",
+            "\"dest\": {"
+                + "\"index\": \"bar\","
+                + "\"pipeline\": \"baz\""
+                + "},",
+            "transform-preview",
+            "bar",
+            "baz"
+        );
+    }
+
+    public void testParsingOverwritesDestField() throws IOException {
+        testParsingOverwrites(
+            "\"id\": \"bar\",",
+            "",
+            "bar",
+            "unused-transform-preview-index",
+            null
+        );
+    }
+
+    public void testParsingOverwritesIdAndDestIndexFields() throws IOException {
+        testParsingOverwrites(
+            "",
+            "\"dest\": {"
+                + "\"pipeline\": \"baz\""
+            + "},",
+            "transform-preview",
+            "unused-transform-preview-index",
+            "baz"
+        );
+    }
+
     public void testParsingOverwritesIdAndDestFields() throws IOException {
-        // id & dest fields will be set by the parser
+        testParsingOverwrites(
+            "",
+            "",
+            "transform-preview",
+            "unused-transform-preview-index",
+            null
+        );
+    }
+
+    private void testParsingOverwrites(
+        String transformIdJson,
+        String destConfigJson,
+        String expectedTransformId,
+        String expectedDestIndex,
+        String expectedDestPipeline
+    ) throws IOException {
         BytesArray json = new BytesArray(
             "{ "
-                + "\"source\":{"
-                + "   \"index\":\"foo\", "
+                + transformIdJson
+                + "\"source\": {"
+                + "   \"index\": \"foo\", "
                 + "   \"query\": {\"match_all\": {}}},"
+                + destConfigJson
                 + "\"pivot\": {"
                 + "\"group_by\": {\"destination-field2\": {\"terms\": {\"field\": \"term-field\"}}},"
                 + "\"aggs\": {\"avg_response\": {\"avg\": {\"field\": \"responsetime\"}}}"
@@ -83,8 +137,9 @@ public class PreviewTransformActionRequestTests extends AbstractSerializingTrans
         ) {
 
             Request request = Request.fromXContent(parser);
-            assertEquals("transform-preview", request.getConfig().getId());
-            assertEquals("unused-transform-preview-index", request.getConfig().getDestination().getIndex());
+            assertThat(request.getConfig().getId(), is(equalTo(expectedTransformId)));
+            assertThat(request.getConfig().getDestination().getIndex(), is(equalTo(expectedDestIndex)));
+            assertThat(request.getConfig().getDestination().getPipeline(), is(equalTo(expectedDestPipeline)));
         }
     }
 }

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

@@ -309,7 +309,7 @@ teardown:
             "dest": { "index": "simple-remote-transform" },
             "pivot": {
               "group_by": { "user": {"terms": {"field": "user"}}},
-              "aggs": {"avg_stars": {"avg": {"field": "stars"}}}
+              "aggs": { "avg_stars": {"avg": {"field": "stars"}}}
             }
           }
 
@@ -325,7 +325,7 @@ teardown:
             "dest": { "index": "simple-remote-transform-2" },
             "pivot": {
               "group_by": { "user": {"terms": {"field": "user"}}},
-              "aggs": {"avg_stars": {"avg": {"field": "stars"}}}
+              "aggs": { "avg_stars": {"avg": {"field": "stars"}}}
             }
           }
   - match: { acknowledged: true }
@@ -343,7 +343,7 @@ teardown:
 ---
 "Batch transform preview from remote cluster when the user is not authorized":
   - do:
-      catch: /Source indices have been deleted or closed./
+      catch: /Cannot preview transform \[transform-preview\] because user bob lacks all the required permissions for indices. \[my_remote_cluster:remote_test_index, simple-remote-transform-2\]/
       headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" }  # This is bob
       transform.preview_transform:
         body: >
@@ -352,6 +352,18 @@ teardown:
             "dest": { "index": "simple-remote-transform-2" },
             "pivot": {
               "group_by": { "user": {"terms": {"field": "user"}}},
-              "aggs": {"avg_stars": {"avg": {"field": "stars"}}}
+              "aggs": { "avg_stars": {"avg": {"field": "stars"}}}
+            }
+          }
+  - do:
+      catch: /Cannot preview transform \[transform-preview\] because user bob lacks all the required permissions for indices. \[my_remote_cluster:test_index\]/
+      headers: { Authorization: "Basic Ym9iOnRyYW5zZm9ybS1wYXNzd29yZA==" }  # This is bob
+      transform.preview_transform:
+        body: >
+          {
+            "source": { "index": "my_remote_cluster:test_index" },
+            "pivot": {
+              "group_by": { "user": {"terms": {"field": "user"}}},
+              "aggs": { "avg_stars": {"avg": {"field": "stars"}}}
             }
           }

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

@@ -124,3 +124,18 @@ teardown:
             - '{"user": "e", "stars": 3, "date" : "2018-10-29T12:12:12.123456789Z"}'
             - '{"index": {"_index": "remote_test_index_2"}}'
             - '{"user": "d", "stars": 4, "date" : "2018-10-29T12:14:12.123456789Z"}'
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: remote_test_index_2
+        body:
+          aggs:
+            user:
+              terms:
+                field: user
+
+  - match: { _shards.total: 3 }
+  - match: { hits.total: 2 }
+  - length: { aggregations.user.buckets: 2 }
+  - match: { aggregations.user.buckets.0.key: "d" }
+  - match: { aggregations.user.buckets.0.doc_count: 1 }

+ 125 - 0
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransformPrivilegeChecker.java

@@ -0,0 +1,125 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.transform.action;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.IndicesOptions;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.common.Strings;
+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.permission.ResourcePrivileges;
+import org.elasticsearch.xpack.core.security.support.Exceptions;
+import org.elasticsearch.xpack.core.transform.transforms.TransformConfig;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static java.util.stream.Collectors.toList;
+
+/**
+ * {@link TransformPrivilegeChecker} is responsible for checking whether the user has the right privileges in order to work with transform.
+ */
+final class TransformPrivilegeChecker {
+
+    static void checkPrivileges(
+        String operationName,
+        SecurityContext securityContext,
+        IndexNameExpressionResolver indexNameExpressionResolver,
+        ClusterState clusterState,
+        Client client,
+        TransformConfig config,
+        boolean checkDestIndexPrivileges,
+        ActionListener<Void> listener
+    ) {
+        final String username = securityContext.getUser().principal();
+
+        ActionListener<HasPrivilegesResponse> hasPrivilegesResponseListener = ActionListener.wrap(
+            response -> handlePrivilegesResponse(operationName, username, config.getId(), response, listener),
+            listener::onFailure
+        );
+
+        HasPrivilegesRequest hasPrivilegesRequest =
+            buildPrivilegesRequest(config, indexNameExpressionResolver, clusterState, username, checkDestIndexPrivileges);
+        client.execute(HasPrivilegesAction.INSTANCE, hasPrivilegesRequest, hasPrivilegesResponseListener);
+    }
+
+    private static HasPrivilegesRequest buildPrivilegesRequest(
+        TransformConfig config,
+        IndexNameExpressionResolver indexNameExpressionResolver,
+        ClusterState clusterState,
+        String username,
+        boolean checkDestIndexPrivileges
+    ) {
+        List<RoleDescriptor.IndicesPrivileges> indicesPrivileges = new ArrayList<>(2);
+
+        RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
+            .indices(config.getSource().getIndex())
+            // We need to read the source indices mapping to deduce the destination mapping, hence the need for view_index_metadata
+            .privileges("read", "view_index_metadata")
+            .build();
+        indicesPrivileges.add(sourceIndexPrivileges);
+
+        if (checkDestIndexPrivileges) {
+            final String destIndex = config.getDestination().getIndex();
+            final String[] concreteDest =
+                indexNameExpressionResolver.concreteIndexNames(clusterState, IndicesOptions.lenientExpandOpen(), destIndex);
+
+            List<String> destPrivileges = new ArrayList<>(3);
+            destPrivileges.add("read");
+            destPrivileges.add("index");
+            // If the destination index does not exist, we can assume that we may have to create it on start.
+            // We should check that the creating user has the privileges to create the index.
+            if (concreteDest.length == 0) {
+                destPrivileges.add("create_index");
+            }
+            RoleDescriptor.IndicesPrivileges destIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
+                .indices(destIndex)
+                .privileges(destPrivileges)
+                .build();
+            indicesPrivileges.add(destIndexPrivileges);
+        }
+
+        HasPrivilegesRequest privRequest = new HasPrivilegesRequest();
+        privRequest.username(username);
+        privRequest.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]);
+        privRequest.clusterPrivileges(Strings.EMPTY_ARRAY);
+        privRequest.indexPrivileges(indicesPrivileges.toArray(RoleDescriptor.IndicesPrivileges[]::new));
+        return privRequest;
+    }
+
+    private static void handlePrivilegesResponse(
+        String operationName,
+        String username,
+        String transformId,
+        HasPrivilegesResponse privilegesResponse,
+        ActionListener<Void> listener
+    ) {
+        if (privilegesResponse.isCompleteMatch()) {
+            listener.onResponse(null);
+        } else {
+            List<String> indices = privilegesResponse.getIndexPrivileges().stream().map(ResourcePrivileges::getResource).collect(toList());
+            listener.onFailure(
+                Exceptions.authorizationError(
+                    "Cannot {} transform [{}] because user {} lacks all the required permissions for indices: {}",
+                    operationName,
+                    transformId,
+                    username,
+                    indices
+                )
+            );
+        }
+    }
+
+    private TransformPrivilegeChecker() {}
+}

+ 121 - 66
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPreviewTransformAction.java

@@ -33,11 +33,14 @@ import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.ingest.IngestService;
 import org.elasticsearch.license.License;
 import org.elasticsearch.license.RemoteClusterLicenseChecker;
+import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.core.ClientHelper;
+import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.common.validation.SourceDestValidator;
+import org.elasticsearch.xpack.core.security.SecurityContext;
 import org.elasticsearch.xpack.core.transform.TransformField;
 import org.elasticsearch.xpack.core.transform.action.PreviewTransformAction;
 import org.elasticsearch.xpack.core.transform.action.PreviewTransformAction.Request;
@@ -60,10 +63,14 @@ import java.util.Map;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.xpack.core.transform.action.PreviewTransformAction.DUMMY_DEST_INDEX_FOR_PREVIEW;
 
 public class TransportPreviewTransformAction extends HandledTransportAction<Request, Response> {
 
     private static final int NUMBER_OF_PREVIEW_BUCKETS = 100;
+    private final XPackLicenseState licenseState;
+    private final SecurityContext securityContext;
+    private final IndexNameExpressionResolver indexNameExpressionResolver;
     private final Client client;
     private final ThreadPool threadPool;
     private final ClusterService clusterService;
@@ -73,6 +80,7 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
 
     @Inject
     public TransportPreviewTransformAction(
+        XPackLicenseState licenseState,
         TransportService transportService,
         ActionFilters actionFilters,
         Client client,
@@ -84,6 +92,7 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
     ) {
         this(
             PreviewTransformAction.NAME,
+            licenseState,
             transportService,
             actionFilters,
             client,
@@ -97,6 +106,7 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
 
     protected TransportPreviewTransformAction(
         String name,
+        XPackLicenseState licenseState,
         TransportService transportService,
         ActionFilters actionFilters,
         Client client,
@@ -107,6 +117,11 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
         IngestService ingestService
     ) {
         super(name, transportService, actionFilters, Request::new);
+        this.licenseState = licenseState;
+        this.securityContext = XPackSettings.SECURITY_ENABLED.get(settings)
+            ? new SecurityContext(settings, threadPool.getThreadContext())
+            : null;
+        this.indexNameExpressionResolver = indexNameExpressionResolver;
         this.client = client;
         this.threadPool = threadPool;
         this.clusterService = clusterService;
@@ -139,28 +154,64 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
         }
 
         final TransformConfig config = request.getConfig();
-        sourceDestValidator.validate(
-            clusterState,
-            config.getSource().getIndex(),
-            config.getDestination().getIndex(),
-            config.getDestination().getPipeline(),
-            SourceDestValidations.getValidationsForPreview(config.getAdditionalValidations()),
-            ActionListener.wrap(r -> {
-                // create the function for validation
-                final Function function = FunctionFactory.create(config);
-                function.validateConfig(ActionListener.wrap(functionValidationResponse -> {
-                    getPreview(
-                        config.getId(), // note: @link{PreviewTransformAction} sets an id, so this is never null
-                        function,
-                        config.getSource(),
-                        config.getDestination().getPipeline(),
-                        config.getDestination().getIndex(),
-                        config.getSyncConfig(),
-                        listener
-                    );
-                }, listener::onFailure));
-            }, listener::onFailure)
+        final Function function = FunctionFactory.create(config);
+
+        // <4> Validate transform query
+        ActionListener<Boolean> validateConfigListener = ActionListener.wrap(
+            validateConfigResponse -> {
+                getPreview(
+                    config.getId(), // note: @link{PreviewTransformAction} sets an id, so this is never null
+                    function,
+                    config.getSource(),
+                    config.getDestination().getPipeline(),
+                    config.getDestination().getIndex(),
+                    config.getSyncConfig(),
+                    listener
+                );
+            },
+            listener::onFailure
+        );
+
+        // <3> Validate transform function config
+        ActionListener<Boolean> validateSourceDestListener = ActionListener.wrap(
+            validateSourceDestResponse -> {
+                function.validateConfig(validateConfigListener);
+            },
+            listener::onFailure
+        );
+
+        // <2> Validate source and destination indices
+        ActionListener<Void> checkPrivilegesListener = ActionListener.wrap(
+            aVoid -> {
+                sourceDestValidator.validate(
+                    clusterState,
+                    config.getSource().getIndex(),
+                    config.getDestination().getIndex(),
+                    config.getDestination().getPipeline(),
+                    SourceDestValidations.getValidationsForPreview(config.getAdditionalValidations()),
+                    validateSourceDestListener
+                );
+            },
+            listener::onFailure
         );
+
+        // <1> Early check to verify that the user can create the destination index and can read from the source
+        if (licenseState.isSecurityEnabled()) {
+            TransformPrivilegeChecker.checkPrivileges(
+                "preview",
+                securityContext,
+                indexNameExpressionResolver,
+                clusterState,
+                client,
+                config,
+                // We don't want to check privileges for a dummy (placeholder) index and the placeholder is inserted as config.dest.index
+                // early in the REST action so the only possibility we have here is string comparison.
+                DUMMY_DEST_INDEX_FOR_PREVIEW.equals(config.getDestination().getIndex()) == false,
+                checkPrivilegesListener
+            );
+        } else { // No security enabled, just create the transform
+            checkPrivilegesListener.onResponse(null);
+        }
     }
 
     @SuppressWarnings("unchecked")
@@ -194,52 +245,56 @@ public class TransportPreviewTransformAction extends HandledTransportAction<Requ
             warnings.forEach(HeaderWarning::addWarning);
             listener.onResponse(new Response(docs, generatedDestIndexSettings));
         }, listener::onFailure);
-        function.deduceMappings(client, source, ActionListener.wrap(deducedMappings -> {
-            mappings.set(deducedMappings);
-            function.preview(
-                client,
-                ClientHelper.filterSecurityHeaders(threadPool.getThreadContext().getHeaders()),
-                source,
-                deducedMappings,
-                NUMBER_OF_PREVIEW_BUCKETS,
-                ActionListener.wrap(docs -> {
-                    if (pipeline == null) {
-                        TransformDestIndexSettings generatedDestIndexSettings = TransformIndex.createTransformDestIndexSettings(
-                            mappings.get(),
-                            transformId,
-                            Clock.systemUTC()
-                        );
-                        List<String> warnings = TransformConfigLinter.getWarnings(function, source, syncConfig);
-                        warnings.forEach(HeaderWarning::addWarning);
-                        listener.onResponse(new Response(docs, generatedDestIndexSettings));
-                    } else {
-                        List<Map<String, Object>> results = docs.stream().map(doc -> {
-                            Map<String, Object> src = new HashMap<>();
-                            String id = (String) doc.get(TransformField.DOCUMENT_ID_FIELD);
-                            src.put("_source", doc);
-                            src.put("_id", id);
-                            src.put("_index", dest);
-                            return src;
-                        }).collect(Collectors.toList());
-
-                        try (XContentBuilder builder = jsonBuilder()) {
-                            builder.startObject();
-                            builder.field("docs", results);
-                            builder.endObject();
-                            var pipelineRequest = new SimulatePipelineRequest(BytesReference.bytes(builder), XContentType.JSON);
-                            pipelineRequest.setId(pipeline);
-                            ClientHelper.executeAsyncWithOrigin(
-                                client,
-                                ClientHelper.TRANSFORM_ORIGIN,
-                                SimulatePipelineAction.INSTANCE,
-                                pipelineRequest,
-                                pipelineResponseActionListener
-                            );
-                        }
+
+        ActionListener<List<Map<String, Object>>> previewListener = ActionListener.wrap(
+            docs -> {
+                if (pipeline == null) {
+                    TransformDestIndexSettings generatedDestIndexSettings = TransformIndex.createTransformDestIndexSettings(
+                        mappings.get(),
+                        transformId,
+                        Clock.systemUTC()
+                    );
+                    List<String> warnings = TransformConfigLinter.getWarnings(function, source, syncConfig);
+                    warnings.forEach(HeaderWarning::addWarning);
+                    listener.onResponse(new Response(docs, generatedDestIndexSettings));
+                } else {
+                    List<Map<String, Object>> results = docs.stream().map(doc -> {
+                        Map<String, Object> src = new HashMap<>();
+                        String id = (String) doc.get(TransformField.DOCUMENT_ID_FIELD);
+                        src.put("_source", doc);
+                        src.put("_id", id);
+                        src.put("_index", dest);
+                        return src;
+                    }).collect(Collectors.toList());
+
+                    try (XContentBuilder builder = jsonBuilder()) {
+                        builder.startObject();
+                        builder.field("docs", results);
+                        builder.endObject();
+                        var pipelineRequest = new SimulatePipelineRequest(BytesReference.bytes(builder), XContentType.JSON);
+                        pipelineRequest.setId(pipeline);
+                        client.execute(SimulatePipelineAction.INSTANCE, pipelineRequest, pipelineResponseActionListener);
                     }
-                }, listener::onFailure)
-            );
+                }
+            },
+            listener::onFailure
+        );
+
+        ActionListener<Map<String, String>> deduceMappingsListener = ActionListener.wrap(
+            deducedMappings -> {
+                mappings.set(deducedMappings);
+                function.preview(
+                    client,
+                    ClientHelper.filterSecurityHeaders(threadPool.getThreadContext().getHeaders()),
+                    source,
+                    deducedMappings,
+                    NUMBER_OF_PREVIEW_BUCKETS,
+                    previewListener
+                );
+            },
+            listener::onFailure
+        );
 
-        }, listener::onFailure));
+        function.deduceMappings(client, source, deduceMappingsListener);
     }
 }

+ 25 - 98
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java

@@ -15,7 +15,6 @@ import org.elasticsearch.ResourceAlreadyExistsException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
-import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.client.Client;
@@ -24,7 +23,6 @@ import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.ingest.IngestService;
@@ -38,12 +36,6 @@ import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.core.XPackPlugin;
 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.permission.ResourcePrivileges;
-import org.elasticsearch.xpack.core.security.support.Exceptions;
 import org.elasticsearch.xpack.core.transform.TransformMessages;
 import org.elasticsearch.xpack.core.transform.action.PutTransformAction;
 import org.elasticsearch.xpack.core.transform.action.PutTransformAction.Request;
@@ -56,10 +48,8 @@ import org.elasticsearch.xpack.transform.transforms.Function;
 import org.elasticsearch.xpack.transform.transforms.FunctionFactory;
 
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 public class TransportPutTransformAction extends AcknowledgedTransportMasterNodeAction<Request> {
 
@@ -131,49 +121,6 @@ public class TransportPutTransformAction extends AcknowledgedTransportMasterNode
         this.auditor = transformServices.getAuditor();
     }
 
-    static HasPrivilegesRequest buildPrivilegeCheck(
-        TransformConfig config,
-        IndexNameExpressionResolver indexNameExpressionResolver,
-        ClusterState clusterState,
-        String username
-    ) {
-        final String destIndex = config.getDestination().getIndex();
-        final String[] concreteDest = indexNameExpressionResolver.concreteIndexNames(
-            clusterState,
-            IndicesOptions.lenientExpandOpen(),
-            config.getDestination().getIndex()
-        );
-        List<String> srcPrivileges = new ArrayList<>(2);
-        srcPrivileges.add("read");
-
-        List<String> destPrivileges = new ArrayList<>(3);
-        destPrivileges.add("read");
-        destPrivileges.add("index");
-        // If the destination index does not exist, we can assume that we may have to create it on start.
-        // We should check that the creating user has the privileges to create the index.
-        if (concreteDest.length == 0) {
-            destPrivileges.add("create_index");
-            // We need to read the source indices mapping to deduce the destination mapping
-            srcPrivileges.add("view_index_metadata");
-        }
-        RoleDescriptor.IndicesPrivileges destIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
-            .indices(destIndex)
-            .privileges(destPrivileges)
-            .build();
-
-        RoleDescriptor.IndicesPrivileges sourceIndexPrivileges = RoleDescriptor.IndicesPrivileges.builder()
-            .indices(config.getSource().getIndex())
-            .privileges(srcPrivileges)
-            .build();
-
-        HasPrivilegesRequest privRequest = new HasPrivilegesRequest();
-        privRequest.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]);
-        privRequest.username(username);
-        privRequest.clusterPrivileges(Strings.EMPTY_ARRAY);
-        privRequest.indexPrivileges(sourceIndexPrivileges, destIndexPrivileges);
-        return privRequest;
-    }
-
     @Override
     protected void masterOperation(Task task, Request request, ClusterState clusterState, ActionListener<AcknowledgedResponse> listener) {
         XPackPlugin.checkReadyForXPackCustomMetadata(clusterState);
@@ -192,28 +139,33 @@ public class TransportPutTransformAction extends AcknowledgedTransportMasterNode
             return;
         }
 
-        client.execute(
-            ValidateTransformAction.INSTANCE,
-            new ValidateTransformAction.Request(config, request.isDeferValidation()),
-            ActionListener.wrap(
-                validationResponse -> {
-                    // Early check to verify that the user can create the destination index and can read from the source
-                    if (licenseState.isSecurityEnabled() && request.isDeferValidation() == false) {
-                        final String username = securityContext.getUser().principal();
-                        HasPrivilegesRequest privRequest = buildPrivilegeCheck(config, indexNameExpressionResolver, clusterState, username);
-                        ActionListener<HasPrivilegesResponse> privResponseListener = ActionListener.wrap(
-                            r -> handlePrivsResponse(username, request, r, listener),
-                            listener::onFailure
-                        );
+        // <3> Create the transform
+        ActionListener<ValidateTransformAction.Response> validateTransformListener = ActionListener.wrap(
+            validationResponse -> {
+                putTransform(request, listener);
+            },
+            listener::onFailure
+        );
 
-                        client.execute(HasPrivilegesAction.INSTANCE, privRequest, privResponseListener);
-                    } else { // No security enabled, just create the transform
-                        putTransform(request, listener);
-                    }
-                },
-                listener::onFailure
-            )
+        // <2> Validate source and destination indices
+        ActionListener<Void> checkPrivilegesListener = ActionListener.wrap(
+            aVoid -> {
+                client.execute(
+                    ValidateTransformAction.INSTANCE,
+                    new ValidateTransformAction.Request(config, request.isDeferValidation()),
+                    validateTransformListener
+                );
+            },
+            listener::onFailure
         );
+
+        // <1> Early check to verify that the user can create the destination index and can read from the source
+        if (licenseState.isSecurityEnabled() && request.isDeferValidation() == false) {
+            TransformPrivilegeChecker.checkPrivileges(
+                "create", securityContext, indexNameExpressionResolver, clusterState, client, config, true, checkPrivilegesListener);
+        } else { // No security enabled, just move on
+            checkPrivilegesListener.onResponse(null);
+        }
     }
 
     @Override
@@ -221,31 +173,6 @@ public class TransportPutTransformAction extends AcknowledgedTransportMasterNode
         return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
     }
 
-    private void handlePrivsResponse(
-        String username,
-        Request request,
-        HasPrivilegesResponse privilegesResponse,
-        ActionListener<AcknowledgedResponse> listener
-    ) {
-        if (privilegesResponse.isCompleteMatch()) {
-            putTransform(request, listener);
-        } else {
-            List<String> indices = privilegesResponse.getIndexPrivileges()
-                .stream()
-                .map(ResourcePrivileges::getResource)
-                .collect(Collectors.toList());
-
-            listener.onFailure(
-                Exceptions.authorizationError(
-                    "Cannot create transform [{}] because user {} lacks all the required permissions for indices: {}",
-                    request.getConfig().getId(),
-                    username,
-                    indices
-                )
-            );
-        }
-    }
-
     private void putTransform(Request request, ActionListener<AcknowledgedResponse> listener) {
 
         final TransformConfig config = request.getConfig();

+ 37 - 75
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportUpdateTransformAction.java

@@ -39,11 +39,6 @@ import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.core.XPackPlugin;
 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.permission.ResourcePrivileges;
-import org.elasticsearch.xpack.core.security.support.Exceptions;
 import org.elasticsearch.xpack.core.transform.TransformMessages;
 import org.elasticsearch.xpack.core.transform.action.UpdateTransformAction;
 import org.elasticsearch.xpack.core.transform.action.UpdateTransformAction.Request;
@@ -67,9 +62,6 @@ import org.elasticsearch.xpack.transform.transforms.TransformTask;
 import java.time.Clock;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
-
-import static org.elasticsearch.xpack.transform.action.TransportPutTransformAction.buildPrivilegeCheck;
 
 public class TransportUpdateTransformAction extends TransportTasksAction<TransformTask, Request, Response, Response> {
 
@@ -219,23 +211,45 @@ public class TransportUpdateTransformAction extends TransportTasksAction<Transfo
                 updateListener = listener;
             }
 
-            client.execute(
-                ValidateTransformAction.INSTANCE,
-                new ValidateTransformAction.Request(updatedConfig, request.isDeferValidation()),
-                ActionListener.wrap(
-                    validationResponse -> {
-                        checkPriviledgesAndUpdateTransform(
-                            request,
-                            clusterState,
-                            updatedConfig,
-                            validationResponse.getDestIndexMappings(),
-                            configAndVersion.v2(),
-                            updateListener);
-                    },
-                    listener::onFailure
-                )
+            // <3> Update the transform
+            ActionListener<ValidateTransformAction.Response> validateTransformListener = ActionListener.wrap(
+                validationResponse -> {
+                    updateTransform(
+                        request,
+                        updatedConfig,
+                        validationResponse.getDestIndexMappings(),
+                        configAndVersion.v2(),
+                        clusterState,
+                        updateListener);
+                },
+                listener::onFailure
             );
 
+            // <2> Validate source and destination indices
+            ActionListener<Void> checkPrivilegesListener = ActionListener.wrap(
+                aVoid -> {
+                    client.execute(
+                        ValidateTransformAction.INSTANCE,
+                        new ValidateTransformAction.Request(updatedConfig, request.isDeferValidation()),
+                        validateTransformListener
+                    );
+                },
+                listener::onFailure);
+
+            // <1> Early check to verify that the user can create the destination index and can read from the source
+            if (licenseState.isSecurityEnabled() && request.isDeferValidation() == false) {
+                TransformPrivilegeChecker.checkPrivileges(
+                    "update",
+                    securityContext,
+                    indexNameExpressionResolver,
+                    clusterState,
+                    client,
+                    updatedConfig,
+                    true,
+                    checkPrivilegesListener);
+            } else { // No security enabled, just move on
+                checkPrivilegesListener.onResponse(null);
+            }
         }, listener::onFailure));
     }
 
@@ -257,58 +271,6 @@ public class TransportUpdateTransformAction extends TransportTasksAction<Transfo
         return tasks.get(0);
     }
 
-    private void handlePrivsResponse(
-        String username,
-        Request request,
-        TransformConfig config,
-        Map<String, String> mappings,
-        SeqNoPrimaryTermAndIndex seqNoPrimaryTermAndIndex,
-        ClusterState clusterState,
-        HasPrivilegesResponse privilegesResponse,
-        ActionListener<Response> listener
-    ) {
-        if (privilegesResponse.isCompleteMatch()) {
-            updateTransform(request, config, mappings, seqNoPrimaryTermAndIndex, clusterState, listener);
-        } else {
-            List<String> indices = privilegesResponse.getIndexPrivileges()
-                .stream()
-                .map(ResourcePrivileges::getResource)
-                .collect(Collectors.toList());
-
-            listener.onFailure(
-                Exceptions.authorizationError(
-                    "Cannot update transform [{}] because user {} lacks all the required permissions for indices: {}",
-                    request.getId(),
-                    username,
-                    indices
-                )
-            );
-        }
-    }
-
-    private void checkPriviledgesAndUpdateTransform(
-        Request request,
-        ClusterState clusterState,
-        TransformConfig config,
-        Map<String, String> mappings,
-        SeqNoPrimaryTermAndIndex seqNoPrimaryTermAndIndex,
-        ActionListener<Response> listener
-    ) {
-        // Early check to verify that the user can create the destination index and can read from the source
-        if (licenseState.isSecurityEnabled() && request.isDeferValidation() == false) {
-            final String username = securityContext.getUser().principal();
-            HasPrivilegesRequest privRequest = buildPrivilegeCheck(config, indexNameExpressionResolver, clusterState, username);
-            ActionListener<HasPrivilegesResponse> privResponseListener = ActionListener.wrap(
-                r -> handlePrivsResponse(username, request, config, mappings, seqNoPrimaryTermAndIndex, clusterState, r, listener),
-                listener::onFailure
-            );
-
-            client.execute(HasPrivilegesAction.INSTANCE, privRequest, privResponseListener);
-        } else { // No security enabled, just create the transform
-            updateTransform(request, config, mappings, seqNoPrimaryTermAndIndex, clusterState, listener);
-        }
-    }
-
     private void updateTransform(
         Request request,
         TransformConfig config,

+ 2 - 2
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportValidateTransformAction.java

@@ -110,8 +110,8 @@ public class TransportValidateTransformAction extends HandledTransportAction<Req
 
         // <5> Final listener
         ActionListener<Map<String, String>> deduceMappingsListener = ActionListener.wrap(
-            mappings -> {
-                listener.onResponse(new Response(mappings));
+            deducedMappings -> {
+                listener.onResponse(new Response(deducedMappings));
             },
             deduceTargetMappingsException -> listener.onFailure(
                 new RuntimeException(

+ 3 - 0
x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/compat/TransportPreviewTransformActionDeprecated.java

@@ -14,6 +14,7 @@ import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.ingest.IngestService;
+import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.core.transform.action.compat.PreviewTransformActionDeprecated;
@@ -23,6 +24,7 @@ public class TransportPreviewTransformActionDeprecated extends TransportPreviewT
 
     @Inject
     public TransportPreviewTransformActionDeprecated(
+        XPackLicenseState licenseState,
         TransportService transportService,
         ActionFilters actionFilters,
         Client client,
@@ -34,6 +36,7 @@ public class TransportPreviewTransformActionDeprecated extends TransportPreviewT
     ) {
         super(
             PreviewTransformActionDeprecated.NAME,
+            licenseState,
             transportService,
             actionFilters,
             client,

+ 186 - 0
x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/action/TransformPrivilegeCheckerTests.java

@@ -0,0 +1,186 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.transform.action;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.action.ActionType;
+import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.indices.TestIndexNameExpressionResolver;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.client.NoOpClient;
+import org.elasticsearch.xpack.core.security.SecurityContext;
+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.user.User;
+import org.elasticsearch.xpack.core.transform.transforms.DestConfig;
+import org.elasticsearch.xpack.core.transform.transforms.SourceConfig;
+import org.elasticsearch.xpack.core.transform.transforms.TransformConfig;
+import org.junit.After;
+import org.junit.Before;
+
+import static org.hamcrest.Matchers.arrayContaining;
+import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.Matchers.emptyArray;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+
+public class TransformPrivilegeCheckerTests extends ESTestCase {
+
+    private static final String OPERATION_NAME = "some-operation";
+    private static final String USER_NAME = "bob";
+    private static final String TRANSFORM_ID = "some-id";
+    private static final String SOURCE_INDEX_NAME = "some-source-index";
+    private static final String DEST_INDEX_NAME = "some-dest-index";
+    private static final TransformConfig TRANSFORM_CONFIG =
+        new TransformConfig.Builder()
+            .setId(TRANSFORM_ID)
+            .setSource(new SourceConfig(SOURCE_INDEX_NAME))
+            .setDest(new DestConfig(DEST_INDEX_NAME, null))
+            .build();
+
+    private final SecurityContext securityContext =
+        new SecurityContext(Settings.EMPTY, null) {
+            public User getUser() {
+                return new User(USER_NAME);
+            }
+        };
+    private final IndexNameExpressionResolver indexNameExpressionResolver = TestIndexNameExpressionResolver.newInstance();
+    private MyMockClient client;
+
+    @Before
+    public void setupClient() {
+        if (client != null) {
+            client.close();
+        }
+        client = new MyMockClient(getTestName());
+    }
+
+    @After
+    public void tearDownClient() {
+        client.close();
+    }
+
+    public void testCheckPrivileges_NoCheckDestIndexPrivileges() {
+        TransformPrivilegeChecker.checkPrivileges(
+            OPERATION_NAME,
+            securityContext,
+            indexNameExpressionResolver,
+            ClusterState.EMPTY_STATE,
+            client,
+            TRANSFORM_CONFIG,
+            false,
+            ActionListener.wrap(
+                aVoid -> {
+                    HasPrivilegesRequest request = client.lastHasPrivilegesRequest;
+                    assertThat(request.username(), is(equalTo(USER_NAME)));
+                    assertThat(request.applicationPrivileges(), is(emptyArray()));
+                    assertThat(request.clusterPrivileges(), is(emptyArray()));
+                    assertThat(request.indexPrivileges(), is(arrayWithSize(1)));
+                    RoleDescriptor.IndicesPrivileges sourceIndicesPrivileges = request.indexPrivileges()[0];
+                    assertThat(sourceIndicesPrivileges.getIndices(), is(arrayContaining(SOURCE_INDEX_NAME)));
+                    assertThat(sourceIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "view_index_metadata")));
+                },
+                e -> fail(e.getMessage())
+            ));
+    }
+
+    public void testCheckPrivileges_CheckDestIndexPrivileges_DestIndexDoesNotExist() {
+        TransformPrivilegeChecker.checkPrivileges(
+            OPERATION_NAME,
+            securityContext,
+            indexNameExpressionResolver,
+            ClusterState.EMPTY_STATE,
+            client,
+            TRANSFORM_CONFIG,
+            true,
+            ActionListener.wrap(
+                aVoid -> {
+                    HasPrivilegesRequest request = client.lastHasPrivilegesRequest;
+                    assertThat(request.username(), is(equalTo(USER_NAME)));
+                    assertThat(request.applicationPrivileges(), is(emptyArray()));
+                    assertThat(request.clusterPrivileges(), is(emptyArray()));
+                    assertThat(request.indexPrivileges(), is(arrayWithSize(2)));
+                    RoleDescriptor.IndicesPrivileges sourceIndicesPrivileges = request.indexPrivileges()[0];
+                    assertThat(sourceIndicesPrivileges.getIndices(), is(arrayContaining(SOURCE_INDEX_NAME)));
+                    assertThat(sourceIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "view_index_metadata")));
+                    RoleDescriptor.IndicesPrivileges destIndicesPrivileges = request.indexPrivileges()[1];
+                    assertThat(destIndicesPrivileges.getIndices(), is(arrayContaining(DEST_INDEX_NAME)));
+                    assertThat(destIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "index", "create_index")));
+                },
+                e -> fail(e.getMessage())
+            ));
+    }
+
+    public void testCheckPrivileges_CheckDestIndexPrivileges_DestIndexExists() {
+        ClusterState clusterState =
+            ClusterState.builder(ClusterName.DEFAULT)
+                .metadata(Metadata.builder()
+                    .put(IndexMetadata.builder(DEST_INDEX_NAME)
+                        .settings(settings(Version.CURRENT))
+                        .numberOfShards(1)
+                        .numberOfReplicas(0)))
+                .build();
+        TransformPrivilegeChecker.checkPrivileges(
+            OPERATION_NAME,
+            securityContext,
+            indexNameExpressionResolver,
+            clusterState,
+            client,
+            TRANSFORM_CONFIG,
+            true,
+            ActionListener.wrap(
+                aVoid -> {
+                    HasPrivilegesRequest request = client.lastHasPrivilegesRequest;
+                    assertThat(request.username(), is(equalTo(USER_NAME)));
+                    assertThat(request.applicationPrivileges(), is(emptyArray()));
+                    assertThat(request.clusterPrivileges(), is(emptyArray()));
+                    assertThat(request.indexPrivileges(), is(arrayWithSize(2)));
+                    RoleDescriptor.IndicesPrivileges sourceIndicesPrivileges = request.indexPrivileges()[0];
+                    assertThat(sourceIndicesPrivileges.getIndices(), is(arrayContaining(SOURCE_INDEX_NAME)));
+                    assertThat(sourceIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "view_index_metadata")));
+                    RoleDescriptor.IndicesPrivileges destIndicesPrivileges = request.indexPrivileges()[1];
+                    assertThat(destIndicesPrivileges.getIndices(), is(arrayContaining(DEST_INDEX_NAME)));
+                    assertThat(destIndicesPrivileges.getPrivileges(), is(arrayContaining("read", "index")));
+                },
+                e -> fail(e.getMessage())
+            ));
+    }
+
+    private static class MyMockClient extends NoOpClient {
+
+        private HasPrivilegesRequest lastHasPrivilegesRequest;
+
+        MyMockClient(String testName) {
+            super(testName);
+        }
+
+        @SuppressWarnings("unchecked")
+        @Override
+        protected <Request extends ActionRequest, Response extends ActionResponse> void doExecute(
+            ActionType<Response> action,
+            Request request,
+            ActionListener<Response> listener
+        ) {
+            if ((request instanceof HasPrivilegesRequest) == false) {
+                fail("Unexpected request type: " + request.getClass().getSimpleName());
+            }
+            // Save the generated request for later.
+            lastHasPrivilegesRequest = (HasPrivilegesRequest) request;
+            listener.onResponse((Response) new HasPrivilegesResponse());
+        }
+    }
+}