Skip to content

Commit

Permalink
Provide secondary security headers when calling _validate rather than…
Browse files Browse the repository at this point in the history 
… using secondary auth only
  • Loading branch information
@przemekwitek
przemekwitek committed Mar 9, 2023
1 parent a96ffea commit 549e160
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 139 deletions.
1 change: 1 addition & 0 deletions x-pack/plugin/transform/qa/multi-node-tests/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
50 changes: 50 additions & 0 deletions ...ava/org/elasticsearch/xpack/transform/integration/TransformInsufficientPermissionsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue;
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
Expand All @@ -44,6 +45,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;

Expand Down Expand Up @@ -171,6 +174,53 @@ 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-defer-nounattended";
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 fails in an expected way,
ResponseException e = expectThrows(ResponseException.class, () -> startTransform(config.getId(), RequestOptions.DEFAULT));
assertThat(
e.getMessage(),
allOf(
containsString(Strings.format("Could not create destination index [%s] for transform [%s]", destIndexName, transformId)),
containsString(
Strings.format(
"is unauthorized for user [%s] with effective roles [transform_admin] on indices [%s]",
JUNIOR_USERNAME,
destIndexName
)
)
)
);
}

/**
* defer_validation = true
* unattended = true
Expand Down
9 changes: 9 additions & 0 deletions ...avaRestTest/java/org/elasticsearch/xpack/transform/integration/TransformRestTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 8 additions & 22 deletions ...rc/javaRestTest/java/org/elasticsearch/xpack/transform/integration/TransformUpdateIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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));
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
89 changes: 44 additions & 45 deletions ...m/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,61 +92,60 @@ public TransportPutTransformAction(
@Override
protected void masterOperation(Task task, Request request, ClusterState clusterState, ActionListener<AcknowledgedResponse> listener) {
XPackPlugin.checkReadyForXPackCustomMetadata(clusterState);

TransformConfig config = request.getConfig().setCreateTime(Instant.now()).setVersion(Version.CURRENT);
useSecondaryAuthIfAvailable(securityContext, () -> {
// set headers to run transform as calling user
Map<String, String> filteredHeaders = ClientHelper.getPersistableSafeSecurityHeaders(
threadPool.getThreadContext(),
clusterService.state()
);
config.setHeaders(filteredHeaders);
});

TransformConfig config = request.getConfig()
.setHeaders(filteredHeaders)
.setCreateTime(Instant.now())
.setVersion(Version.CURRENT);

String transformId = config.getId();
// quick check whether a transform has already been created under that name
if (PersistentTasksCustomMetadata.getTaskWithId(clusterState, transformId) != null) {
listener.onFailure(
new ResourceAlreadyExistsException(
TransformMessages.getMessage(TransformMessages.REST_PUT_TRANSFORM_EXISTS, transformId)
)
);
return;
}

// <3> Create the transform
ActionListener<ValidateTransformAction.Response> validateTransformListener = ActionListener.wrap(
validationResponse -> putTransform(request, listener),
listener::onFailure
String transformId = config.getId();
// quick check whether a transform has already been created under that name
if (PersistentTasksCustomMetadata.getTaskWithId(clusterState, transformId) != null) {
listener.onFailure(
new ResourceAlreadyExistsException(TransformMessages.getMessage(TransformMessages.REST_PUT_TRANSFORM_EXISTS, transformId))
);
return;
}

// <2> Validate source and destination indices
ActionListener<Void> checkPrivilegesListener = ActionListener.wrap(
aVoid -> client.execute(
ValidateTransformAction.INSTANCE,
new ValidateTransformAction.Request(config, request.isDeferValidation(), request.timeout()),
validateTransformListener
),
listener::onFailure
);
// <3> Create the transform
ActionListener<ValidateTransformAction.Response> validateTransformListener = ActionListener.wrap(
validationResponse -> putTransform(request, listener),
listener::onFailure
);

// <1> Early check to verify that the user can create the destination index and can read from the source
if (XPackSettings.SECURITY_ENABLED.get(settings) && request.isDeferValidation() == false) {
TransformPrivilegeChecker.checkPrivileges(
"create",
securityContext,
indexNameExpressionResolver,
clusterState,
client,
config,
true,
checkPrivilegesListener
);
} else { // No security enabled, just move on
checkPrivilegesListener.onResponse(null);
}
});
// <2> Validate source and destination indices
ActionListener<Void> checkPrivilegesListener = ActionListener.wrap(
aVoid -> ClientHelper.executeWithHeadersAsync(
config.getHeaders(),
ClientHelper.TRANSFORM_ORIGIN,
client,
ValidateTransformAction.INSTANCE,
new ValidateTransformAction.Request(config, request.isDeferValidation(), request.timeout()),
validateTransformListener
),
listener::onFailure
);

// <1> Early check to verify that the user can create the destination index and can read from the source
if (XPackSettings.SECURITY_ENABLED.get(settings) && request.isDeferValidation() == false) {
TransformPrivilegeChecker.checkPrivileges(
"create",
securityContext,
indexNameExpressionResolver,
clusterState,
client,
config,
true,
checkPrivilegesListener
);
} else { // No security enabled, just move on
checkPrivilegesListener.onResponse(null);
}
}

@Override
Expand Down
Loading

0 comments on commit 549e160

Please sign in to comment.