Skip to content

Commit

Permalink
Refactor Roles REST API test and partial fix #4166
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrey Pleskach <ples@aiven.io>
  • Loading branch information
@willyborankin
willyborankin committed Jun 11, 2024
1 parent 20c524a commit 9257162
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 948 deletions.
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
5 changes: 4 additions & 1 deletion src/main/java/org/opensearch/security/dlic/rest/api/RolesApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ public Settings settings() {
@Override
public Map<String, DataType> allowedKeys() {
final ImmutableMap.Builder<String, DataType> allowedKeys = ImmutableMap.builder();
if (isCurrentUserAdmin()) allowedKeys.put("reserved", DataType.BOOLEAN);
if (isCurrentUserAdmin()) {
allowedKeys.put("hidden", DataType.BOOLEAN);
allowedKeys.put("reserved", DataType.BOOLEAN);
}
return allowedKeys.put("cluster_permissions", DataType.ARRAY)
.put("tenant_permissions", DataType.ARRAY)
.put("index_permissions", DataType.ARRAY)
Expand Down
19 changes: 7 additions & 12 deletions src/main/java/org/opensearch/security/dlic/rest/support/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -38,9 +37,9 @@
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.common.transport.TransportAddress;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentParser;
Expand Down Expand Up @@ -125,17 +124,13 @@ public static Object toConfigObject(final JsonNode content, final Class<?> clazz

public static JsonNode convertJsonToJackson(ToXContent jsonContent, boolean omitDefaults) {
try {
Map<String, String> pm = new HashMap<>(1);
pm.put("omit_defaults", String.valueOf(omitDefaults));
ToXContent.MapParams params = new ToXContent.MapParams(pm);

final BytesReference bytes = org.opensearch.core.xcontent.XContentHelper.toXContent(
jsonContent,
MediaTypeRegistry.JSON,
params,
false
return DefaultObjectMapper.readTree(
Strings.toString(
XContentType.JSON,
jsonContent,
new ToXContent.MapParams(Map.of("omit_defaults", String.valueOf(omitDefaults)))
)
);
return DefaultObjectMapper.readTree(bytes.utf8ToString());
} catch (IOException e1) {
throw ExceptionsHelper.convertToOpenSearchException(e1);
}
Expand Down
Loading

0 comments on commit 9257162

Please sign in to comment.