Refactor Roles REST API test and partial fix opensearch-project#4166 (o…

…pensearch-project#4433)

Signed-off-by: Andrey Pleskach <[email protected]>

src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java

@@ -93,6 +93,11 @@ public static void initData() {
  client.prepareIndex(LIMITED_USER_INDEX).setId(ID_1).setRefreshPolicy(IMMEDIATE).setSource("foo", "bar").get();
  client.prepareIndex(PROHIBITED_INDEX).setId(ID_2).setRefreshPolicy(IMMEDIATE).setSource("three", "four").get();
  }
+ try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+ Awaitility.await()
+ .alias("Load default configuration")
+ .until(() -> client.securityHealth().getTextFromJsonBody("/status"), equalTo("UP"));
+ }
  }
 
  @Test
@@ -258,7 +263,11 @@ public void testParallelTenantPutRequests() throws Exception {
 
  AtomicInteger numCreatedResponses = new AtomicInteger();
  AsyncActions.getAll(conflictingRequests, 1, TimeUnit.SECONDS).forEach((response) -> {
- assertThat(response.getStatusCode(), anyOf(equalTo(HttpStatus.SC_CREATED), equalTo(HttpStatus.SC_CONFLICT)));
+ assertThat(
+ response.getBody(),
+ response.getStatusCode(),
+ anyOf(equalTo(HttpStatus.SC_CREATED), equalTo(HttpStatus.SC_CONFLICT))
+ );
  if (response.getStatusCode() == HttpStatus.SC_CREATED) numCreatedResponses.getAndIncrement();
  });
  assertThat(numCreatedResponses.get(), equalTo(1)); // should only be one 201

src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java

@@ -11,6 +11,7 @@
 
 package org.opensearch.security.api;
 
+import java.util.Map;
 import java.util.Optional;
 import java.util.StringJoiner;
 
@@ -79,6 +80,33 @@ public AbstractConfigEntityApiIntegrationTest(final String path, final TestDescr
  this.testDescriptor = testDescriptor;
  }
 
+ static ToXContentObject configJsonArray(final String... values) {
+ return (builder, params) -> {
+ builder.startArray();
+ if (values != null) {
+ for (final var v : values) {
+ if (v == null) {
+ builder.nullValue();
+ } else {
+ builder.value(v);
+ }
+ }
+ }
+ return builder.endArray();
+ };
+ }
+
+ static String[] generateArrayValues(boolean useNulls) {
+ final var length = randomIntBetween(1, 5);
+ final var values = new String[length];
+ final var nullIndex = randomIntBetween(0, length - 1);
+ for (var i = 0; i < values.length; i++) {
+ if (useNulls && i == nullIndex) values[i] = null;
+ else values[i] = randomAsciiAlphanumOfLength(10);
+ }
+ return values;
+ }
+
  @Override
  protected String apiPath(String... paths) {
  final StringJoiner fullPath = new StringJoiner("/").add(super.apiPath(path));
@@ -191,6 +219,12 @@ void assertInvalidKeys(final TestRestClient.HttpResponse response, final Matcher
  assertThat(response.getBody(), response.getTextFromJsonBody("/invalid_keys/keys"), expectedInvalidKeysMatcher);
  }
 
+ void assertWrongDataType(final TestRestClient.HttpResponse response, final Map<String, String> expectedMessages) {
+ assertThat(response.getBody(), response.getTextFromJsonBody("/status"), is("error"));
+ for (final var p : expectedMessages.entrySet())
+ assertThat(response.getBody(), response.getTextFromJsonBody("/" + p.getKey()), is(p.getValue()));
+ }
+
  void assertSpecifyOneOf(final TestRestClient.HttpResponse response, final String expectedSpecifyOneOfKeys) {
  assertThat(response.getBody(), response.getTextFromJsonBody("/status"), is("error"));
  assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("Invalid configuration"));

src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java

@@ -12,6 +12,7 @@
 package org.opensearch.security.api;
 
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 
 import org.opensearch.core.xcontent.ToXContentObject;
@@ -59,7 +60,7 @@ public ToXContentObject entityPayload(final Boolean hidden, final Boolean reserv
 
  @Override
  public ToXContentObject jsonPropertyPayload() {
- return allowedActionsArray("a", "b", "c");
+ return configJsonArray("a", "b", "c");
  }
 
  @Override
@@ -89,7 +90,7 @@ static ToXContentObject actionGroup(
  builder.field("type", randomType());
  if (allowedActions != null) {
  builder.field("allowed_actions");
- allowedActionsArray(allowedActions).toXContent(builder, params);
+ configJsonArray(allowedActions).toXContent(builder, params);
  } else {
  builder.startArray("allowed_actions").endArray();
  }
@@ -110,20 +111,6 @@ static String randomType() {
  return randomFrom(List.of(TestSecurityConfig.ActionGroup.Type.CLUSTER.type(), TestSecurityConfig.ActionGroup.Type.INDEX.type()));
  }
 
- static ToXContentObject allowedActionsArray(final String... allowedActions) {
- return (builder, params) -> {
- builder.startArray();
- for (final var allowedAction : allowedActions) {
- if (allowedAction == null) {
- builder.nullValue();
- } else {
- builder.value(allowedAction);
- }
- }
- return builder.endArray();
- };
- }
-
  @Override
  void forbiddenToCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {
  forbidden(() -> client.putJson(apiPath("new_rest_admin_action_group"), actionGroup(randomRestAdminPermission())));
@@ -136,10 +123,7 @@ void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final Test
  forbidden(() -> client.putJson(apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP), actionGroup()));
  forbidden(() -> client.patch(apiPath(), patch(replaceOp(REST_ADMIN_PERMISSION_ACTION_GROUP, actionGroup("a", "b")))));
  forbidden(
- () -> client.patch(
- apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP),
- patch(replaceOp("allowed_actions", allowedActionsArray("c", "d")))
- )
+ () -> client.patch(apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP), patch(replaceOp("allowed_actions", configJsonArray("c", "d"))))
  );
  // remove
  forbidden(() -> client.patch(apiPath(), patch(removeOp(REST_ADMIN_PERMISSION_ACTION_GROUP))));
@@ -161,7 +145,7 @@ void verifyCrudOperations(final Boolean hidden, final Boolean reserved, final Te
  ok(() -> client.patch(apiPath(), patch(addOp("new_action_group_for_patch", actionGroup(hidden, reserved, "e", "f")))));
  assertActionGroup(ok(() -> client.get(apiPath("new_action_group_for_patch"))), "new_action_group_for_patch", List.of("e", "f"));
 
- ok(() -> client.patch(apiPath("new_action_group_for_patch"), patch(replaceOp("allowed_actions", allowedActionsArray("g", "h")))));
+ ok(() -> client.patch(apiPath("new_action_group_for_patch"), patch(replaceOp("allowed_actions", configJsonArray("g", "h")))));
  assertActionGroup(ok(() -> client.get(apiPath("new_action_group_for_patch"))), "new_action_group_for_patch", List.of("g", "h"));
 
  ok(() -> client.patch(apiPath(), patch(removeOp("new_action_group_for_patch"))));
@@ -183,32 +167,39 @@ void verifyBadRequestOperations(final TestRestClient client) throws Exception {
 
  badRequestWithMessage(() -> client.putJson(apiPath("some_action_group"), (builder, params) -> {
  builder.startObject().field("type", "asdasdsad").field("allowed_actions");
- allowedActionsArray("g", "f").toXContent(builder, params);
+ configJsonArray("g", "f").toXContent(builder, params);
  return builder.endObject();
  }), "Invalid action group type: asdasdsad. Supported types are: cluster, index.");
 
  assertMissingMandatoryKeys(
- badRequest(() -> client.putJson(apiPath("some_action_group"), allowedActionsArray("a", "b", "c"))),
+ badRequest(() -> client.putJson(apiPath("some_action_group"), configJsonArray("a", "b", "c"))),
  "allowed_actions"
  );
 
  assertMissingMandatoryKeys(
- badRequest(() -> client.putJson(apiPath("some_action_group"), allowedActionsArray("a", "b", "c"))),
+ badRequest(() -> client.putJson(apiPath("some_action_group"), configJsonArray("a", "b", "c"))),
  "allowed_actions"
  );
 
  final ToXContentObject unknownJsonFields = (builder, params) -> {
  builder.startObject().field("a", "b").field("c", "d").field("allowed_actions");
- allowedActionsArray("g", "h").toXContent(builder, params);
+ configJsonArray("g", "h").toXContent(builder, params);
  return builder.endObject();
  };
  assertInvalidKeys(badRequest(() -> client.putJson(apiPath("some_action_group"), unknownJsonFields)), "a,c");
 
  assertNullValuesInArray(badRequest(() -> client.putJson(apiPath("some_action_group"), (builder, params) -> {
  builder.startObject().field("type", randomType()).field("allowed_actions");
- allowedActionsArray("g", null, "f").toXContent(builder, params);
+ configJsonArray("g", null, "f").toXContent(builder, params);
  return builder.endObject();
  })));
+ assertWrongDataType(
+ client.putJson(
+ apiPath("some_action_group"),
+ (builder, params) -> builder.startObject().field("allowed_actions", "a").endObject()
+ ),
+ Map.of("allowed_actions", "Array expected")
+ );
  // patch
  badRequest(() -> client.patch(apiPath("some_action_group"), EMPTY_BODY));
  badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", EMPTY_BODY))));
@@ -224,12 +215,12 @@ void verifyBadRequestOperations(final TestRestClient client) throws Exception {
  );
 
  assertMissingMandatoryKeys(
- badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", allowedActionsArray("a", "b", "c"))))),
+ badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", configJsonArray("a", "b", "c"))))),
  "allowed_actions"
  );
  badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", (ToXContentObject) (builder, params) -> {
  builder.startObject().field("type", "aaaa").field("allowed_actions");
- allowedActionsArray("g", "f").toXContent(builder, params);
+ configJsonArray("g", "f").toXContent(builder, params);
  return builder.endObject();
  }))));
 
@@ -241,10 +232,22 @@ void verifyBadRequestOperations(final TestRestClient client) throws Exception {
  assertNullValuesInArray(
  badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", (ToXContentObject) (builder, params) -> {
  builder.startObject().field("type", randomType()).field("allowed_actions");
- allowedActionsArray("g", null, "f").toXContent(builder, params);
+ configJsonArray("g", null, "f").toXContent(builder, params);
  return builder.endObject();
  }))))
  );
+ assertWrongDataType(
+ client.patch(
+ apiPath(),
+ patch(
+ addOp(
+ "some_action_group",
+ (ToXContentObject) (builder, params) -> builder.startObject().field("allowed_actions", "a").endObject()
+ )
+ )
+ ),
+ Map.of("allowed_actions", "Array expected")
+ );
  }
 
  void assertActionGroup(final TestRestClient.HttpResponse response, final String actionGroupName, final List<String> allowedActions) {