Skip to content

Commit

Permalink
Refactor Roles REST API test and partial fix #4166 (#4433)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrey Pleskach <ples@aiven.io>
  • Loading branch information
@willyborankin
willyborankin authored Jun 20, 2024
1 parent 3dae7a7 commit 962eafa
Show file tree
Hide file tree
Showing 9 changed files with 556 additions and 955 deletions.
11 changes: 10 additions & 1 deletion src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions ...egrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

package org.opensearch.security.api;

import java.util.Map;
import java.util.Optional;
import java.util.StringJoiner;

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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"));
Expand Down
61 changes: 32 additions & 29 deletions src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand All @@ -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())));
Expand All @@ -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))));
Expand All @@ -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"))));
Expand All @@ -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))));
Expand All @@ -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();
}))));

Expand All @@ -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) {
Expand Down
Loading

0 comments on commit 962eafa

Please sign in to comment.