Skip to content

Commit

Permalink
Validate workflow names (#96462)
Browse files Browse the repository at this point in the history
This PR adds validation of workflow names when creating/updating/granting API keys.
The only allowed workflow names are the ones defined statically.

Relates to #96215
  • Loading branch information
slobodanadamovic authored Jun 12, 2023
1 parent 300c011 commit e8f7e31
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
import org.elasticsearch.xpack.core.security.authz.restriction.WorkflowResolver;
import org.elasticsearch.xpack.core.security.support.MetadataUtils;

import java.util.Arrays;
Expand Down Expand Up @@ -86,7 +87,13 @@ public static ActionRequestValidationException validate(
);
}
if (roleDescriptor.hasWorkflowsRestriction()) {
// TODO: Validate workflow names here!
for (String workflowName : roleDescriptor.getRestriction().getWorkflows()) {
try {
WorkflowResolver.resolveWorkflowByName(workflowName);
} catch (IllegalArgumentException e) {
validationException = addValidationError(e.getMessage(), validationException);
}
}
}
return validationException;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public void testMetadataKeyValidation() {
}

public void testRoleDescriptorValidation() {
final String[] unknownWorkflows = randomArray(1, 2, String[]::new, () -> randomAlphaOfLengthBetween(4, 10));
final var request = new BulkUpdateApiKeyRequest(
randomList(1, 5, () -> randomAlphaOfLength(10)),
List.of(
Expand All @@ -97,7 +98,9 @@ public void testRoleDescriptorValidation() {
null,
null,
Map.of("_key", "value"),
null
null,
null,
new RoleDescriptor.Restriction(unknownWorkflows)
)
),
null
Expand All @@ -109,5 +112,8 @@ public void testRoleDescriptorValidation() {
assertThat(ve.validationErrors().get(2), containsStringIgnoringCase("application name"));
assertThat(ve.validationErrors().get(3), containsStringIgnoringCase("Application privilege names"));
assertThat(ve.validationErrors().get(4), containsStringIgnoringCase("role descriptor metadata keys may not start with "));
for (int i = 0; i < unknownWorkflows.length; i++) {
assertThat(ve.validationErrors().get(5 + i), containsStringIgnoringCase("unknown workflow [" + unknownWorkflows[i] + "]"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public void testMetadataKeyValidation() {
}

public void testRoleDescriptorValidation() {
final String[] unknownWorkflows = randomArray(1, 2, String[]::new, () -> randomAlphaOfLengthBetween(4, 10));
final CreateApiKeyRequest request1 = new CreateApiKeyRequest(
randomAlphaOfLength(5),
List.of(
Expand All @@ -104,7 +105,9 @@ public void testRoleDescriptorValidation() {
null,
null,
Map.of("_key", "value"),
null
null,
null,
new RoleDescriptor.Restriction(unknownWorkflows)
)
),
null
Expand All @@ -116,6 +119,9 @@ public void testRoleDescriptorValidation() {
assertThat(ve1.validationErrors().get(2), containsStringIgnoringCase("application name"));
assertThat(ve1.validationErrors().get(3), containsStringIgnoringCase("Application privilege names"));
assertThat(ve1.validationErrors().get(4), containsStringIgnoringCase("role descriptor metadata keys may not start with "));
for (int i = 0; i < unknownWorkflows.length; i++) {
assertThat(ve1.validationErrors().get(5 + i), containsStringIgnoringCase("unknown workflow [" + unknownWorkflows[i] + "]"));
}
}

public void testSerialization() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.restriction.WorkflowResolver;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -71,6 +72,10 @@ public void testMetadataKeyValidation() {
}

public void testRoleDescriptorValidation() {
final List<String> unknownWorkflows = randomList(1, 2, () -> randomAlphaOfLengthBetween(4, 10));
final List<String> workflows = new ArrayList<>(unknownWorkflows.size() + 1);
workflows.addAll(unknownWorkflows);
workflows.add(WorkflowResolver.SEARCH_APPLICATION_QUERY_WORKFLOW.name());
final var request1 = new UpdateApiKeyRequest(
randomAlphaOfLength(10),
List.of(
Expand All @@ -88,7 +93,9 @@ public void testRoleDescriptorValidation() {
null,
null,
Map.of("_key", "value"),
null
null,
null,
new RoleDescriptor.Restriction(workflows.toArray(String[]::new))
)
),
null
Expand All @@ -100,5 +107,8 @@ public void testRoleDescriptorValidation() {
assertThat(ve1.validationErrors().get(2), containsStringIgnoringCase("application name"));
assertThat(ve1.validationErrors().get(3), containsStringIgnoringCase("Application privilege names"));
assertThat(ve1.validationErrors().get(4), containsStringIgnoringCase("role descriptor metadata keys may not start with "));
for (int i = 0; i < unknownWorkflows.size(); i++) {
assertThat(ve1.validationErrors().get(5 + i), containsStringIgnoringCase("unknown workflow [" + unknownWorkflows.get(i) + "]"));
}
}
}

0 comments on commit e8f7e31

Please sign in to comment.