[Transform] Secondary credentials used with transforms should only require source and destination index privileges, not transform privileges

docs/changelog/94420.yaml

@@ -0,0 +1,5 @@
+pr: 94420
+summary: Secondary credentials used with transforms should only require source and destination index privileges, not transform privileges
+area: Transform
+type: bug
+issues: []

x-pack/plugin/transform/qa/multi-node-tests/build.gradle

@@ -49,4 +49,5 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   user username: "x_pack_rest_user", password: "x-pack-test-password"
   user username: "john_junior", password: "x-pack-test-password", role: "transform_admin"
   user username: "bill_senior", password: "x-pack-test-password", role: "transform_admin,source_index_access"
+  user username: "not_a_transform_admin", password: "x-pack-test-password", role: "source_index_access"
 }

x-pack/plugin/transform/qa/multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/transform/integration/TransformInsufficientPermissionsIT.java

@@ -44,6 +44,8 @@ public class TransformInsufficientPermissionsIT extends TransformRestTestCase {
     private static final String JUNIOR_HEADER = basicAuthHeaderValue(JUNIOR_USERNAME, TEST_PASSWORD_SECURE_STRING);
     private static final String SENIOR_USERNAME = "bill_senior";
     private static final String SENIOR_HEADER = basicAuthHeaderValue(SENIOR_USERNAME, TEST_PASSWORD_SECURE_STRING);
+    private static final String NOT_A_TRANSFORM_ADMIN = "not_a_transform_admin";
+    private static final String NOT_A_TRANSFORM_ADMIN_HEADER = basicAuthHeaderValue(NOT_A_TRANSFORM_ADMIN, TEST_PASSWORD_SECURE_STRING);
 
     private static final int NUM_USERS = 28;
 
@@ -87,7 +89,6 @@ private void testTransformPermissionsNoDeferValidation(boolean unattended) throw
                     .build()
             )
         );
-
         assertThat(e.getResponse().getStatusLine().getStatusCode(), is(equalTo(403)));
         assertThat(
             e.getMessage(),
@@ -171,6 +172,40 @@ public void testTransformPermissionsDeferValidationNoUnattended() throws Excepti
         assertThat(extractValue(getTransformStats(transformId), "health", "status"), is(equalTo("green")));
     }
 
+    /**
+     * defer_validation = true
+     * unattended       = false
+     */
+    @SuppressWarnings("unchecked")
+    public void testNoTransformAdminRoleInSecondaryAuth() throws Exception {
+        String transformId = "transform-permissions-no-admin-role";
+        String sourceIndexName = transformId + "-index";
+        String destIndexName = sourceIndexName + "-dest";
+        createReviewsIndex(sourceIndexName, 10, NUM_USERS, TransformIT::getUserIdForRow, TransformIT::getDateStringForRow);
+
+        TransformConfig config = createConfig(transformId, sourceIndexName, destIndexName, false);
+
+        // PUT with defer_validation should work even though the secondary auth does not have transform_admin role
+        putTransform(
+            transformId,
+            Strings.toString(config),
+            RequestOptions.DEFAULT.toBuilder()
+                .addHeader(SECONDARY_AUTH_KEY, NOT_A_TRANSFORM_ADMIN_HEADER)
+                .addParameter("defer_validation", String.valueOf(true))
+                .build()
+        );
+
+        // _update should work even though the secondary auth does not have transform_admin role
+        updateConfig(
+            transformId,
+            "{}",
+            RequestOptions.DEFAULT.toBuilder().addHeader(SECONDARY_AUTH_KEY, NOT_A_TRANSFORM_ADMIN_HEADER).build()
+        );
+
+        // _start works because user not_a_transform_admin has data access
+        startTransform(config.getId(), RequestOptions.DEFAULT);
+    }
+
     /**
      * defer_validation = true
      * unattended       = true
@@ -212,6 +247,37 @@ public void testTransformPermissionsDeferValidationUnattended() throws Exception
         assertThat(extractValue(getTransformStats(transformId), "health", "status"), is(equalTo("green")));
     }
 
+    public void testPreviewRequestFailsPermissionsCheck() throws Exception {
+        String transformId = "transform-permissions-preview";
+        String sourceIndexName = transformId + "-index";
+        String destIndexName = sourceIndexName + "-dest";
+        createReviewsIndex(sourceIndexName, 10, NUM_USERS, TransformIT::getUserIdForRow, TransformIT::getDateStringForRow);
+
+        TransformConfig config = createConfig(transformId, sourceIndexName, destIndexName, false);
+
+        ResponseException e = expectThrows(
+            ResponseException.class,
+            () -> previewTransform(Strings.toString(config), RequestOptions.DEFAULT.toBuilder().addHeader(AUTH_KEY, JUNIOR_HEADER).build())
+        );
+        assertThat(e.getResponse().getStatusLine().getStatusCode(), is(equalTo(403)));
+        assertThat(
+            e.getMessage(),
+            containsString(
+                String.format(
+                    Locale.ROOT,
+                    "Cannot preview transform [%s] because user %s lacks the required permissions "
+                        + "[%s:[read, view_index_metadata], %s:[create_index, index, read]]",
+                    transformId,
+                    JUNIOR_USERNAME,
+                    sourceIndexName,
+                    destIndexName
+                )
+            )
+        );
+
+        previewTransform(Strings.toString(config), RequestOptions.DEFAULT.toBuilder().addHeader(AUTH_KEY, SENIOR_HEADER).build());
+    }
+
     @Override
     protected Settings restAdminSettings() {
         return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", TEST_ADMIN_HEADER).build();

x-pack/plugin/transform/qa/single-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/transform/integration/TransformRestTestCase.java

@@ -562,6 +562,15 @@ protected static Map<String, Object> getTransforms(int from, int size) throws IO
         return entityAsMap(response);
     }
 
+    protected Map<String, Object> getTransformConfig(String transformId, String authHeader) throws IOException {
+        Request getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, authHeader);
+        Map<String, Object> transforms = entityAsMap(client().performRequest(getRequest));
+        assertEquals(1, XContentMapValues.extractValue("count", transforms));
+        @SuppressWarnings("unchecked")
+        Map<String, Object> transformConfig = ((List<Map<String, Object>>) transforms.get("transforms")).get(0);
+        return transformConfig;
+    }
+
     protected static String getTransformState(String transformId) throws IOException {
         Map<?, ?> transformStatsAsMap = getTransformStateAndStats(transformId);
         return transformStatsAsMap == null ? null : (String) XContentMapValues.extractValue("state", transformStatsAsMap);

x-pack/plugin/transform/qa/single-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/transform/integration/TransformUpdateIT.java

@@ -117,12 +117,9 @@ public void testUpdateDeprecatedSettings() throws Exception {
 
         createTransformRequest.setJsonEntity(config);
         Map<String, Object> createTransformResponse = entityAsMap(client().performRequest(createTransformRequest));
-
         assertThat(createTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE));
-        Request getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_USER);
-        Map<String, Object> transforms = entityAsMap(client().performRequest(getRequest));
-        assertEquals(1, XContentMapValues.extractValue("count", transforms));
-        Map<String, Object> transform = ((List<Map<String, Object>>) XContentMapValues.extractValue("transforms", transforms)).get(0);
+
+        Map<String, Object> transform = getTransformConfig(transformId, BASIC_AUTH_VALUE_TRANSFORM_USER);
         assertThat(XContentMapValues.extractValue("pivot.max_page_search_size", transform), equalTo(555));
 
         final Request updateRequest = createRequestWithAuth(
@@ -137,10 +134,7 @@ public void testUpdateDeprecatedSettings() throws Exception {
         assertNull(XContentMapValues.extractValue("pivot.max_page_search_size", updateResponse));
         assertThat(XContentMapValues.extractValue("settings.max_page_search_size", updateResponse), equalTo(555));
 
-        getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_USER);
-        transforms = entityAsMap(client().performRequest(getRequest));
-        assertEquals(1, XContentMapValues.extractValue("count", transforms));
-        transform = ((List<Map<String, Object>>) XContentMapValues.extractValue("transforms", transforms)).get(0);
+        transform = getTransformConfig(transformId, BASIC_AUTH_VALUE_TRANSFORM_USER);
 
         assertNull(XContentMapValues.extractValue("pivot.max_page_search_size", transform));
         assertThat(XContentMapValues.extractValue("settings.max_page_search_size", transform), equalTo(555));
@@ -210,14 +204,10 @@ private void updateTransferRightsTester(boolean useSecondaryAuthHeaders) throws
 
         createTransformRequest.setJsonEntity(config);
         Map<String, Object> createTransformResponse = entityAsMap(client().performRequest(createTransformRequest));
-
         assertThat(createTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE));
-        Request getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2);
-        Map<String, Object> transforms = entityAsMap(client().performRequest(getRequest));
-        assertEquals(1, XContentMapValues.extractValue("count", transforms));
+
+        Map<String, Object> transformConfig = getTransformConfig(transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2);
         // Confirm the roles were recorded as expected in the stored headers
-        @SuppressWarnings("unchecked")
-        Map<String, Object> transformConfig = ((List<Map<String, Object>>) transforms.get("transforms")).get(0);
         assertThat(transformConfig.get("authorization"), equalTo(Map.of("roles", List.of("transform_admin", DATA_ACCESS_ROLE_2))));
 
         // create a 2nd, identical one
@@ -231,16 +221,14 @@ private void updateTransferRightsTester(boolean useSecondaryAuthHeaders) throws
 
         // getting the transform with the just deleted admin 2 user should fail
         try {
-            client().performRequest(getRequest);
+            getTransformConfig(transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2);
             fail("request should have failed");
         } catch (ResponseException e) {
             assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(401));
         }
 
         // get the transform with admin 1
-        getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1);
-        transforms = entityAsMap(client().performRequest(getRequest));
-        assertEquals(1, XContentMapValues.extractValue("count", transforms));
+        transformConfig = getTransformConfig(transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1);
 
         // start using admin 1, but as the header is still admin 2
         // This fails as the stored header is still admin 2
@@ -278,9 +266,7 @@ private void updateTransferRightsTester(boolean useSecondaryAuthHeaders) throws
         assertOK(client().performRequest(updateRequest));
 
         // get should still work
-        getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformIdCloned, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1);
-        transforms = entityAsMap(client().performRequest(getRequest));
-        assertEquals(1, XContentMapValues.extractValue("count", transforms));
+        getTransformConfig(transformIdCloned, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1);
 
         // start with updated configuration should succeed
         if (useSecondaryAuthHeaders) {

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

@@ -20,6 +20,7 @@
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.engine.VersionConflictEngineException;
 import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
+import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.SecurityContext;
@@ -99,6 +100,7 @@ public TransformConfig getConfig() {
      */
 
     public static void updateTransform(
+        ThreadPool threadPool,
         SecurityContext securityContext,
         IndexNameExpressionResolver indexNameExpressionResolver,
         ClusterState clusterState,
@@ -204,10 +206,9 @@ private static void validateTransform(
         TimeValue timeout,
         ActionListener<Map<String, String>> listener
     ) {
-        ClientHelper.executeWithHeadersAsync(
-            config.getHeaders(),
-            ClientHelper.TRANSFORM_ORIGIN,
+        ClientHelper.executeAsyncWithOrigin(
             client,
+            ClientHelper.TRANSFORM_ORIGIN,
             ValidateTransformAction.INSTANCE,
             new ValidateTransformAction.Request(config, deferValidation, timeout),
             ActionListener.wrap(response -> listener.onResponse(response.getDestIndexMappings()), listener::onFailure)

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

@@ -38,7 +38,6 @@
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentFactory;
 import org.elasticsearch.xcontent.XContentType;
-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;
@@ -65,7 +64,7 @@
 
 import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.xpack.core.transform.action.PreviewTransformAction.DUMMY_DEST_INDEX_FOR_PREVIEW;
-import static org.elasticsearch.xpack.transform.utils.SecondaryAuthorizationUtils.useSecondaryAuthIfAvailable;
+import static org.elasticsearch.xpack.transform.utils.SecondaryAuthorizationUtils.getSecondarySecurityHeaders;
 
 public class TransportPreviewTransformAction extends HandledTransportAction<Request, Response> {
 
@@ -138,19 +137,16 @@ protected void doExecute(Task task, Request request, ActionListener<Response> li
 
         // <4> Validate transform query
         ActionListener<Boolean> validateConfigListener = ActionListener.wrap(
-            validateConfigResponse -> useSecondaryAuthIfAvailable(
-                securityContext,
-                () -> getPreview(
-                    parentTaskId,
-                    request.timeout(),
-                    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
-                )
+            validateConfigResponse -> getPreview(
+                parentTaskId,
+                request.timeout(),
+                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
         );
@@ -209,6 +205,8 @@ private void getPreview(
 
         final SetOnce<Map<String, String>> mappings = new SetOnce<>();
 
+        final Map<String, String> filteredHeaders = getSecondarySecurityHeaders(threadPool, securityContext, clusterService.state());
+
         ActionListener<SimulatePipelineResponse> pipelineResponseActionListener = ActionListener.wrap(simulatePipelineResponse -> {
             List<Map<String, Object>> docs = new ArrayList<>(simulatePipelineResponse.getResults().size());
             List<Map<String, Object>> errors = new ArrayList<>();
@@ -276,14 +274,14 @@ private void getPreview(
             function.preview(
                 parentTaskAssigningClient,
                 timeout,
-                ClientHelper.getPersistableSafeSecurityHeaders(threadPool.getThreadContext(), clusterService.state()),
+                filteredHeaders,
                 source,
                 deducedMappings,
                 NUMBER_OF_PREVIEW_BUCKETS,
                 previewListener
             );
         }, listener::onFailure);
 
-        function.deduceMappings(parentTaskAssigningClient, source, deduceMappingsListener);
+        function.deduceMappings(parentTaskAssigningClient, filteredHeaders, source, deduceMappingsListener);
     }
 }