From bc36dcb7b2da64c7a3d3504bf468800fab119372 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Fri, 31 May 2024 17:57:35 +0200 Subject: [PATCH] [Backport 2.x] Refactor ActionGroup REST API test and partial fix #4166 (#4371) (#4388) --- .../api/AbstractApiIntegrationTest.java | 47 +- ...bstractConfigEntityApiIntegrationTest.java | 251 ++++++++ .../ActionGroupsRestApiIntegrationTest.java | 250 ++++++++ .../test/framework/TestSecurityConfig.java | 33 + .../dlic/rest/api/ActionGroupsApiAction.java | 1 + .../dlic/rest/api/ActionGroupsApiTest.java | 598 ------------------ .../dlic/rest/api/SecurityInfoActionTest.java | 51 -- .../legacy/LegacyActionGroupsApiTests.java | 23 - .../legacy/LegacySecurityInfoActionTests.java | 23 - 9 files changed, 563 insertions(+), 714 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java create mode 100644 src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/SecurityInfoActionTest.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacyActionGroupsApiTests.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityInfoActionTests.java diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java index 4381359b27..d5a0e41a6d 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java @@ -45,8 +45,8 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.hamcrest.Matchers.notNullValue; import static org.opensearch.security.CrossClusterSearchTests.PLUGINS_SECURITY_RESTAPI_ROLES_ENABLED; import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; @@ -182,7 +182,7 @@ private static String removeDashes(final String content) { } protected static String[] allRestAdminPermissions() { - final var permissions = new String[ENDPOINTS_WITH_PERMISSIONS.size() + 3]; // 2 actions for SSL + update config action + final var permissions = new String[ENDPOINTS_WITH_PERMISSIONS.size() + 1]; // 1 additional action for SSL update certs var counter = 0; for (final var e : ENDPOINTS_WITH_PERMISSIONS.entrySet()) { if (e.getKey() == Endpoint.SSL) { @@ -209,6 +209,11 @@ protected static String restAdminPermission(Endpoint endpoint, String action) { } } + protected String randomRestAdminPermission() { + final var permissions = List.of(allRestAdminPermissions()); + return randomFrom(permissions); + } + @AfterClass public static void stopCluster() throws IOException { if (localCluster != null) localCluster.close(); @@ -240,7 +245,7 @@ protected void withUser( final CertificateData certificateData, final CheckedConsumer restClientHandler ) throws Exception { - try (TestRestClient client = localCluster.getRestClient(user, password, certificateData)) { + try (final TestRestClient client = localCluster.getRestClient(user, password, certificateData)) { restClientHandler.accept(client); } } @@ -274,6 +279,12 @@ protected String apiPath(final String... path) { return fullPath.toString(); } + void badRequestWithMessage(final CheckedSupplier endpointCallback, final String expectedMessage) + throws Exception { + final var response = badRequest(endpointCallback); + assertThat(response.getBody(), response.getTextFromJsonBody("/message"), is(expectedMessage)); + } + TestRestClient.HttpResponse badRequest(final CheckedSupplier endpointCallback) throws Exception { final var response = endpointCallback.get(); @@ -286,9 +297,16 @@ TestRestClient.HttpResponse created(final CheckedSupplier endpointCallback, final String expectedMessage) + throws Exception { + final var response = forbidden(endpointCallback); + assertThat(response.getBody(), response.getTextFromJsonBody("/message"), is(expectedMessage)); + } + TestRestClient.HttpResponse forbidden(final CheckedSupplier endpointCallback) throws Exception { final var response = endpointCallback.get(); assertThat(response.getBody(), response.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); @@ -319,6 +337,12 @@ TestRestClient.HttpResponse notFound(final CheckedSupplier endpointCallback, final String expectedMessage) + throws Exception { + final var response = notFound(endpointCallback); + assertThat(response.getBody(), response.getTextFromJsonBody("/message"), is(expectedMessage)); + } + TestRestClient.HttpResponse ok(final CheckedSupplier endpointCallback) throws Exception { final var response = endpointCallback.get(); assertThat(response.getBody(), response.getStatusCode(), equalTo(HttpStatus.SC_OK)); @@ -330,7 +354,7 @@ TestRestClient.HttpResponse unauthorized(final CheckedSupplier endpointCallback) throws Exception { - final var response = endpointCallback.get(); - assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("`null` is not allowed as json array element")); - } - } diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java new file mode 100644 index 0000000000..dda5209942 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java @@ -0,0 +1,251 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.api; + +import java.util.Optional; +import java.util.StringJoiner; + +import org.hamcrest.Matcher; +import org.junit.Test; + +import org.opensearch.common.CheckedSupplier; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.oneOf; +import static org.opensearch.security.api.PatchPayloadHelper.addOp; +import static org.opensearch.security.api.PatchPayloadHelper.patch; +import static org.opensearch.security.api.PatchPayloadHelper.removeOp; +import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; + +public abstract class AbstractConfigEntityApiIntegrationTest extends AbstractApiIntegrationTest { + + static { + clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + testSecurityConfig.withRestAdminUser(REST_ADMIN_USER, allRestAdminPermissions()); + } + + interface TestDescriptor { + + String entityJsonProperty(); + + default ToXContentObject entityPayload() { + return entityPayload(null, null, null); + } + + default ToXContentObject reservedEntityPayload() { + return entityPayload(null, true, null); + } + + default ToXContentObject hiddenEntityPayload() { + return entityPayload(true, null, null); + } + + default ToXContentObject staticEntityPayload() { + return entityPayload(null, null, true); + } + + ToXContentObject entityPayload(final Boolean hidden, final Boolean reserved, final Boolean _static); + + ToXContentObject jsonPropertyPayload(); + + default Optional restAdminLimitedUser() { + return Optional.empty(); + } + + } + + private final String path; + + private final TestDescriptor testDescriptor; + + public AbstractConfigEntityApiIntegrationTest(final String path, final TestDescriptor testDescriptor) { + this.path = path; + this.testDescriptor = testDescriptor; + } + + @Override + protected String apiPath(String... paths) { + final StringJoiner fullPath = new StringJoiner("/").add(super.apiPath(path)); + if (paths != null) { + for (final var p : paths) { + fullPath.add(p); + } + } + return fullPath.toString(); + } + + @Test + public void forbiddenForRegularUsers() throws Exception { + withUser(NEW_USER, client -> { + forbidden(() -> client.putJson(apiPath("some_entity"), EMPTY_BODY)); + forbidden(() -> client.get(apiPath())); + forbidden(() -> client.get(apiPath("some_entity"))); + forbidden(() -> client.putJson(apiPath("some_entity"), EMPTY_BODY)); + forbidden(() -> client.patch(apiPath(), EMPTY_BODY)); + forbidden(() -> client.patch(apiPath("some_entity"), EMPTY_BODY)); + forbidden(() -> client.delete(apiPath("some_entity"))); + }); + } + + @Test + public void availableForAdminUser() throws Exception { + final var hiddenEntityName = randomAsciiAlphanumOfLength(10); + final var reservedEntityName = randomAsciiAlphanumOfLength(10); + withUser( + ADMIN_USER_NAME, + localCluster.getAdminCertificate(), + client -> created(() -> client.putJson(apiPath(hiddenEntityName), testDescriptor.hiddenEntityPayload())) + ); + withUser( + ADMIN_USER_NAME, + localCluster.getAdminCertificate(), + client -> created(() -> client.putJson(apiPath(reservedEntityName), testDescriptor.reservedEntityPayload())) + ); + + // can't see hidden resources + withUser(ADMIN_USER_NAME, client -> { + verifyNoHiddenEntities(() -> client.get(apiPath())); + creationOfReadOnlyEntityForbidden( + client, + (builder, params) -> testDescriptor.hiddenEntityPayload().toXContent(builder, params), + (builder, params) -> testDescriptor.reservedEntityPayload().toXContent(builder, params), + (builder, params) -> testDescriptor.staticEntityPayload().toXContent(builder, params) + ); + verifyUpdateAndDeleteHiddenConfigEntityForbidden(hiddenEntityName, client); + verifyUpdateAndDeleteReservedConfigEntityForbidden(reservedEntityName, client); + verifyCrudOperations(null, null, client); + verifyBadRequestOperations(client); + }); + } + + @Test + public void availableForTLSAdminUser() throws Exception { + withUser(ADMIN_USER_NAME, localCluster.getAdminCertificate(), this::availableForSuperAdminUser); + } + + @Test + public void availableForRESTAdminUser() throws Exception { + withUser(REST_ADMIN_USER, this::availableForSuperAdminUser); + if (testDescriptor.restAdminLimitedUser().isPresent()) { + withUser(testDescriptor.restAdminLimitedUser().get(), this::availableForSuperAdminUser); + } + } + + void availableForSuperAdminUser(final TestRestClient client) throws Exception { + creationOfReadOnlyEntityForbidden(client, (builder, params) -> testDescriptor.staticEntityPayload().toXContent(builder, params)); + verifyCrudOperations(true, null, client); + verifyCrudOperations(null, true, client); + verifyCrudOperations(null, null, client); + verifyBadRequestOperations(client); + forbiddenToCreateEntityWithRestAdminPermissions(client); + forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(client); + } + + void verifyNoHiddenEntities(final CheckedSupplier endpointCallback) throws Exception { + final var body = ok(endpointCallback).bodyAsJsonNode(); + final var pretty = body.toPrettyString(); + final var it = body.elements(); + while (it.hasNext()) { + final var e = it.next(); + assertThat(pretty, not(e.get("hidden").asBoolean())); + } + } + + void creationOfReadOnlyEntityForbidden(final TestRestClient client, final ToXContentObject... entities) throws Exception { + for (final var configEntity : entities) { + assertInvalidKeys( + badRequest(() -> client.putJson(apiPath(randomAsciiAlphanumOfLength(10)), configEntity)), + is(oneOf("static", "hidden", "reserved")) + ); + badRequest(() -> client.patch(apiPath(), patch(addOp(randomAsciiAlphanumOfLength(10), configEntity)))); + } + } + + void assertNullValuesInArray(final TestRestClient.HttpResponse response) throws Exception { + assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("`null` is not allowed as json array element")); + } + + void assertInvalidKeys(final TestRestClient.HttpResponse response, final String expectedInvalidKeys) { + assertInvalidKeys(response, equalTo(expectedInvalidKeys)); + } + + void assertInvalidKeys(final TestRestClient.HttpResponse response, final Matcher expectedInvalidKeysMatcher) { + assertThat(response.getBody(), response.getTextFromJsonBody("/status"), is("error")); + assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("Invalid configuration")); + assertThat(response.getBody(), response.getTextFromJsonBody("/invalid_keys/keys"), expectedInvalidKeysMatcher); + } + + 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")); + assertThat(response.getBody(), response.getTextFromJsonBody("/specify_one_of/keys"), containsString(expectedSpecifyOneOfKeys)); + } + + void assertMissingMandatoryKeys(final TestRestClient.HttpResponse response, final String expectedKeys) { + assertThat(response.getBody(), response.getTextFromJsonBody("/status"), is("error")); + assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("Invalid configuration")); + assertThat(response.getBody(), response.getTextFromJsonBody("/missing_mandatory_keys/keys"), containsString(expectedKeys)); + } + + void verifyUpdateAndDeleteHiddenConfigEntityForbidden(final String hiddenEntityName, final TestRestClient client) throws Exception { + final var expectedErrorMessage = "Resource '" + hiddenEntityName + "' is not available."; + notFound(() -> client.putJson(apiPath(hiddenEntityName), testDescriptor.entityPayload()), expectedErrorMessage); + notFound( + () -> client.patch( + apiPath(hiddenEntityName), + patch(replaceOp(testDescriptor.entityJsonProperty(), testDescriptor.jsonPropertyPayload())) + ), + expectedErrorMessage + ); + notFound(() -> client.patch(apiPath(), patch(replaceOp(hiddenEntityName, testDescriptor.entityPayload()))), expectedErrorMessage); + notFound(() -> client.patch(apiPath(hiddenEntityName), patch(removeOp(testDescriptor.entityJsonProperty()))), expectedErrorMessage); + notFound(() -> client.patch(apiPath(), patch(removeOp(hiddenEntityName))), expectedErrorMessage); + notFound(() -> client.delete(apiPath(hiddenEntityName)), expectedErrorMessage); + } + + void verifyUpdateAndDeleteReservedConfigEntityForbidden(final String reservedEntityName, final TestRestClient client) throws Exception { + final var expectedErrorMessage = "Resource '" + reservedEntityName + "' is reserved."; + forbidden(() -> client.putJson(apiPath(reservedEntityName), testDescriptor.entityPayload()), expectedErrorMessage); + forbidden( + () -> client.patch( + apiPath(reservedEntityName), + patch(replaceOp(testDescriptor.entityJsonProperty(), testDescriptor.entityJsonProperty())) + ), + expectedErrorMessage + ); + forbidden( + () -> client.patch(apiPath(), patch(replaceOp(reservedEntityName, testDescriptor.entityPayload()))), + expectedErrorMessage + ); + forbidden(() -> client.patch(apiPath(), patch(removeOp(reservedEntityName))), expectedErrorMessage); + forbidden( + () -> client.patch(apiPath(reservedEntityName), patch(removeOp(testDescriptor.entityJsonProperty()))), + expectedErrorMessage + ); + forbidden(() -> client.delete(apiPath(reservedEntityName)), expectedErrorMessage); + } + + void forbiddenToCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {} + + void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {} + + abstract void verifyBadRequestOperations(final TestRestClient client) throws Exception; + + abstract void verifyCrudOperations(final Boolean hidden, final Boolean reserved, final TestRestClient client) throws Exception; +} diff --git a/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java new file mode 100644 index 0000000000..e42ed2a447 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java @@ -0,0 +1,250 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.api; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.security.dlic.rest.api.Endpoint; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.oneOf; +import static org.opensearch.security.api.PatchPayloadHelper.addOp; +import static org.opensearch.security.api.PatchPayloadHelper.patch; +import static org.opensearch.security.api.PatchPayloadHelper.removeOp; +import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; + +public class ActionGroupsRestApiIntegrationTest extends AbstractConfigEntityApiIntegrationTest { + + private final static String REST_API_ADMIN_ACTION_GROUPS_ONLY = "rest_api_admin_action_groups_only"; + + private final static String REST_ADMIN_PERMISSION_ACTION_GROUP = "rest-admin-permissions-action-group"; + + static { + testSecurityConfig.withRestAdminUser(REST_API_ADMIN_ACTION_GROUPS_ONLY, restAdminPermission(Endpoint.ACTIONGROUPS)) + .actionGroups( + new TestSecurityConfig.ActionGroup( + REST_ADMIN_PERMISSION_ACTION_GROUP, + TestSecurityConfig.ActionGroup.Type.INDEX, + allRestAdminPermissions() + ) + ); + } + + public ActionGroupsRestApiIntegrationTest() { + super("actiongroups", new TestDescriptor() { + @Override + public String entityJsonProperty() { + return "allowed_actions"; + } + + @Override + public ToXContentObject entityPayload(final Boolean hidden, final Boolean reserved, final Boolean _static) { + return actionGroup(hidden, reserved, _static, "a", "b"); + } + + @Override + public ToXContentObject jsonPropertyPayload() { + return allowedActionsArray("a", "b", "c"); + } + + @Override + public Optional restAdminLimitedUser() { + return Optional.of(REST_API_ADMIN_ACTION_GROUPS_ONLY); + } + }); + } + + static ToXContentObject actionGroup(final String... allowedActions) { + return actionGroup(null, null, allowedActions); + } + + static ToXContentObject actionGroup(final Boolean hidden, final Boolean reserved, final String... allowedActions) { + return actionGroup(hidden, reserved, null, allowedActions); + } + + static ToXContentObject actionGroup( + final Boolean hidden, + final Boolean reserved, + final Boolean _static, + final String... allowedActions + ) { + return (builder, params) -> { + builder.startObject(); + // TODO exclude in checking null value for the type + final var possibleTypes = new ArrayList(); + possibleTypes.add(TestSecurityConfig.ActionGroup.Type.INDEX.type()); + possibleTypes.add(TestSecurityConfig.ActionGroup.Type.CLUSTER.type()); + possibleTypes.add(null); + final var actionGroupType = randomFrom(possibleTypes); + if (actionGroupType != null) { + builder.field("type", actionGroupType); + } + if (allowedActions != null) { + builder.field("allowed_actions"); + allowedActionsArray(allowedActions).toXContent(builder, params); + } else { + builder.startArray("allowed_actions").endArray(); + } + if (reserved != null) { + builder.field("reserved", reserved); + } + if (hidden != null) { + builder.field("hidden", hidden); + } + if (_static != null) { + builder.field("static", _static); + } + return builder.endObject(); + }; + } + + 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()))); + forbidden(() -> client.patch(apiPath(), patch(addOp("new_rest_admin_action_group", actionGroup(randomRestAdminPermission()))))); + } + + @Override + void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { + // update + 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"))) + ) + ); + // remove + forbidden(() -> client.patch(apiPath(), patch(removeOp(REST_ADMIN_PERMISSION_ACTION_GROUP)))); + forbidden(() -> client.patch(apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP), patch(removeOp("allowed_actions")))); + forbidden(() -> client.delete(apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP))); + } + + @Override + void verifyCrudOperations(final Boolean hidden, final Boolean reserved, final TestRestClient client) throws Exception { + created(() -> client.putJson(apiPath("new_action_group"), actionGroup(hidden, reserved, "a", "b"))); + assertActionGroup(ok(() -> client.get(apiPath("new_action_group"))), "new_action_group", List.of("a", "b")); + + ok(() -> client.putJson(apiPath("new_action_group"), actionGroup(hidden, reserved, "c", "d"))); + assertActionGroup(ok(() -> client.get(apiPath("new_action_group"))), "new_action_group", List.of("c", "d")); + + ok(() -> client.delete(apiPath("new_action_group"))); + notFound(() -> client.get(apiPath("new_action_group"))); + + 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"))))); + 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")))); + notFound(() -> client.get(apiPath("new_action_group_for_patch"))); + } + + @Override + void verifyBadRequestOperations(final TestRestClient client) throws Exception { + // put + badRequest(() -> client.putJson(apiPath("some_action_group"), EMPTY_BODY)); + badRequestWithMessage( + () -> client.putJson(apiPath("kibana_user"), actionGroup("a", "b")), + "kibana_user is an existing role. A action group cannot be named with an existing role name." + ); + badRequestWithMessage( + () -> client.putJson(apiPath("reference_itself"), actionGroup("reference_itself")), + "reference_itself cannot be an allowed_action of itself" + ); + + assertMissingMandatoryKeys( + badRequest(() -> client.putJson(apiPath("some_action_group"), allowedActionsArray("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); + 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("allowed_actions"); + allowedActionsArray("g", null, "f").toXContent(builder, params); + return builder.endObject(); + }))); + // patch + badRequest(() -> client.patch(apiPath("some_action_group"), EMPTY_BODY)); + badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", EMPTY_BODY)))); + badRequest(() -> client.patch(apiPath(), patch(replaceOp("some_action_group", EMPTY_BODY)))); + + badRequestWithMessage( + () -> client.patch(apiPath(), patch(addOp("kibana_user", actionGroup("a")))), + "kibana_user is an existing role. A action group cannot be named with an existing role name." + ); + badRequestWithMessage( + () -> client.patch(apiPath(), patch(addOp("reference_itself", actionGroup("reference_itself")))), + "reference_itself cannot be an allowed_action of itself" + ); + + assertMissingMandatoryKeys( + badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", allowedActionsArray("a", "b", "c"))))), + "allowed_actions" + ); + + badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", (ToXContentObject) (builder, parameter) -> { + builder.startObject(); + unknownJsonFields.toXContent(builder, parameter); + return builder.endObject(); + })))); + assertNullValuesInArray( + badRequest(() -> client.patch(apiPath(), patch(addOp("some_action_group", (ToXContentObject) (builder, params) -> { + builder.startObject().field("allowed_actions"); + allowedActionsArray("g", null, "f").toXContent(builder, params); + return builder.endObject(); + })))) + ); + } + + void assertActionGroup(final TestRestClient.HttpResponse response, final String actionGroupName, final List allowedActions) { + assertThat(response.getBody(), not(response.getBooleanFromJsonBody("/" + actionGroupName + "/hidden"))); + assertThat(response.getBody(), not(response.getBooleanFromJsonBody("/" + actionGroupName + "/reserved"))); + assertThat(response.getBody(), not(response.getBooleanFromJsonBody("/" + actionGroupName + "/static"))); + assertThat( + response.getBody(), + response.getTextFromJsonBody("/" + actionGroupName + "/type"), + oneOf(TestSecurityConfig.ActionGroup.Type.INDEX.type(), TestSecurityConfig.ActionGroup.Type.CLUSTER.type(), "") + ); + assertThat(response.getBody(), response.getTextArrayFromJsonBody("/" + actionGroupName + "/allowed_actions"), is(allowedActions)); + } + +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java index 79f10a76cf..e4e7f8f4de 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java @@ -313,6 +313,8 @@ public String type() { private Boolean reserved = null; + private Boolean _static = null; + public ActionGroup(String name, Type type, String... allowedActions) { this(name, null, type, allowedActions); } @@ -333,16 +335,38 @@ public ActionGroup hidden(boolean hidden) { return this; } + public boolean hidden() { + return hidden != null && hidden; + } + public ActionGroup reserved(boolean reserved) { this.reserved = reserved; return this; } + public boolean reserved() { + return reserved != null && reserved; + } + + public ActionGroup _static(boolean _static) { + this._static = _static; + return this; + } + + public boolean _static() { + return _static != null && _static; + } + + public List allowedActions() { + return allowedActions; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); if (hidden != null) builder.field("hidden", hidden); if (reserved != null) builder.field("reserved", reserved); + if (_static != null) builder.field("static", _static); builder.field("type", type.type()); builder.field("allowed_actions", allowedActions); if (description != null) builder.field("description", description); @@ -366,6 +390,7 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(name, description, type, allowedActions, hidden, reserved); } + } public static final class User implements UserCredentialsHolder, ToXContentObject { @@ -605,6 +630,8 @@ public static class RoleMapping implements ToXContentObject { private Boolean reserved; + private Boolean _static; + private final String description; private List backendRoles = new ArrayList<>(); @@ -632,6 +659,11 @@ public RoleMapping reserved(boolean reserved) { return this; } + public RoleMapping _static(boolean _static) { + this._static = _static; + return this; + } + public RoleMapping users(String... users) { this.users.addAll(Arrays.asList(users)); return this; @@ -652,6 +684,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(); if (hidden != null) builder.field("hidden", hidden); if (reserved != null) builder.field("reserved", reserved); + if (_static != null) builder.field("static", _static); if (users != null && !users.isEmpty()) builder.field("users", users); if (hosts != null && !hosts.isEmpty()) builder.field("hosts", hosts); if (description != null) builder.field("description", description); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java index 3032054e64..1c58e9ac6e 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java @@ -175,6 +175,7 @@ public Settings settings() { public Map allowedKeys() { final ImmutableMap.Builder allowedKeys = ImmutableMap.builder(); if (isCurrentUserAdmin()) { + allowedKeys.put("hidden", DataType.BOOLEAN); allowedKeys.put("reserved", DataType.BOOLEAN); } allowedKeys.put("allowed_actions", DataType.ARRAY); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java deleted file mode 100644 index bee312d4e7..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java +++ /dev/null @@ -1,598 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.dlic.rest.api; - -import java.util.List; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import org.apache.http.Header; -import org.apache.http.HttpStatus; -import org.junit.Assert; -import org.junit.Test; - -import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.dlic.rest.validation.RequestContentValidator; -import org.opensearch.security.test.helper.file.FileHelper; -import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; - -import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; - -public class ActionGroupsApiTest extends AbstractRestApiUnitTest { - private final String ENDPOINT; - - protected String getEndpointPrefix() { - return PLUGINS_PREFIX; - } - - public ActionGroupsApiTest() { - ENDPOINT = getEndpointPrefix() + "/api/actiongroups"; - } - - @Test - public void testActionGroupsApi() throws Exception { - - setup(); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = true; - - // create index - setupStarfleetIndex(); - - // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - rh.sendAdminCertificate = true; - verifyGetForSuperAdmin(new Header[0]); - rh.sendAdminCertificate = true; - verifyDeleteForSuperAdmin(new Header[0], true); - rh.sendAdminCertificate = true; - verifyPutForSuperAdmin(new Header[0], true); - rh.sendAdminCertificate = true; - verifyPatchForSuperAdmin(new Header[0], true); - } - - void verifyGetForSuperAdmin(final Header[] header) throws Exception { - // --- GET_UT - // GET_UT, actiongroup exists - HttpResponse response = rh.executeGetRequest(ENDPOINT + "/CRUD_UT", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - List permissions = settings.getAsList("CRUD_UT.allowed_actions"); - Assert.assertNotNull(permissions); - Assert.assertEquals(2, permissions.size()); - Assert.assertTrue(permissions.contains("READ_UT")); - Assert.assertTrue(permissions.contains("OPENDISTRO_SECURITY_WRITE")); - - // GET_UT, actiongroup does not exist - response = rh.executeGetRequest(ENDPOINT + "/nothinghthere", header); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // GET_UT, old endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // GET_UT, old endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // GET_UT, new endpoint which replaces configuration endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // GET_UT, old endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // GET_UT, new endpoint which replaces configuration endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // GET_UT, new endpoint which replaces configuration endpoint - response = rh.executeGetRequest(ENDPOINT, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - } - - void verifyDeleteForSuperAdmin(final Header[] header, final boolean userAdminCert) throws Exception { - // -- DELETE - // Non-existing role - rh.sendAdminCertificate = userAdminCert; - - HttpResponse response = rh.executeDeleteRequest(ENDPOINT + "/idonotexist", header); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // remove action group READ_UT, read access not possible since - // opendistro_security_role_starfleet - // uses this action group. - response = rh.executeDeleteRequest(ENDPOINT + "/READ_UT", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - - // put picard in captains role. Role opendistro_security_role_captains uses the CRUD_UT - // action group - // which uses READ_UT and WRITE action groups. We removed READ_UT, so only - // WRITE is possible - addUserWithPassword("picard", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_OK); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - - // now remove also CRUD_UT groups, write also not possible anymore - rh.sendAdminCertificate = true; - response = rh.executeDeleteRequest(ENDPOINT + "/CRUD_UT", new Header[0]); - rh.sendAdminCertificate = false; - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - } - - void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) throws Exception { - // -- PUT - // put with empty payload, must fail - rh.sendAdminCertificate = userAdminCert; - HttpResponse response = rh.executePutRequest(ENDPOINT + "/SOMEGROUP", "", header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(RequestContentValidator.ValidationError.PAYLOAD_MANDATORY.message(), settings.get("reason")); - - // put new configuration with invalid payload, must fail - response = rh.executePutRequest(ENDPOINT + "/SOMEGROUP", FileHelper.loadFile("restapi/actiongroup_not_parseable.json"), header); - settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertEquals(RequestContentValidator.ValidationError.BODY_NOT_PARSEABLE.message(), settings.get("reason")); - - response = rh.executePutRequest(ENDPOINT + "/CRUD_UT", FileHelper.loadFile("restapi/actiongroup_crud.json"), header); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); - - rh.sendAdminCertificate = false; - - // write access allowed again, read forbidden, since READ_UT group is still missing - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - - // restore READ_UT action groups - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/READ_UT", FileHelper.loadFile("restapi/actiongroup_read.json"), header); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); - - rh.sendAdminCertificate = false; - // read/write allowed again - checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - - // -- PUT, new JSON format including readonly flag, disallowed in REST API - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/CRUD_UT", FileHelper.loadFile("restapi/actiongroup_readonly.json"), header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // -- DELETE read only resource, must be forbidden - // superAdmin can delete read only resource - rh.sendAdminCertificate = userAdminCert; - response = rh.executeDeleteRequest(ENDPOINT + "/GET_UT", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // -- PUT read only resource, must be forbidden - // superAdmin can add/update read only resource - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/GET_UT", FileHelper.loadFile("restapi/actiongroup_read.json"), header); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); - Assert.assertFalse(response.getBody().contains("Resource 'GET_UT' is read-only.")); - - // PUT with role name - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/kibana_user", FileHelper.loadFile("restapi/actiongroup_read.json"), header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue( - response.getBody().contains("kibana_user is an existing role. A action group cannot be named with an existing role name.") - ); - - // PUT with self-referencing action groups - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/reference_itself", "{\"allowed_actions\": [\"reference_itself\"]}", header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("reference_itself cannot be an allowed_action of itself")); - - // -- GET_UT hidden resource, must be 404 but super admin can find it - rh.sendAdminCertificate = userAdminCert; - response = rh.executeGetRequest(ENDPOINT + "/INTERNAL", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("\"hidden\":true")); - - // -- DELETE hidden resource, must be 404 - rh.sendAdminCertificate = userAdminCert; - response = rh.executeDeleteRequest(ENDPOINT + "/INTERNAL", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("'INTERNAL' deleted.")); - - // -- PUT hidden resource, must be forbidden - rh.sendAdminCertificate = userAdminCert; - response = rh.executePutRequest(ENDPOINT + "/INTERNAL", FileHelper.loadFile("restapi/actiongroup_read.json"), header); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); - } - - void verifyPatchForSuperAdmin(final Header[] header, final boolean userAdminCert) throws Exception { - // -- PATCH - // PATCH on non-existing resource - rh.sendAdminCertificate = userAdminCert; - HttpResponse response = rh.executePatchRequest( - ENDPOINT + "/imnothere", - "[{ \"op\": \"add\", \"path\": \"/a/b/c\", \"value\": [ \"foo\", \"bar\" ] }]", - header - ); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // PATCH read only resource, must be forbidden - // SuperAdmin can patch read only resource - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT + "/GET_UT", - "[{ \"op\": \"add\", \"path\": \"/description\", \"value\": \"foo\" }]", - header - ); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // PATCH with self-referencing action groups - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT + "/GET_UT", - "[{ \"op\": \"add\", \"path\": \"/allowed_actions/-\", \"value\": \"GET_UT\" }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("GET_UT cannot be an allowed_action of itself")); - - // bulk PATCH with self-referencing action groups - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"add\", \"path\": \"/BULKNEW1\", \"value\": {\"allowed_actions\": [\"BULKNEW1\"] } }," - + "{ \"op\": \"add\", \"path\": \"/BULKNEW2\", \"value\": {\"allowed_actions\": [\"READ_UT\"] } }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("BULKNEW1 cannot be an allowed_action of itself")); - - // PATCH hidden resource, must be not found, can be found by superadmin, but fails with no path exist error - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT + "/INTERNAL", - "[{ \"op\": \"add\", \"path\": \"/a/b/c\", \"value\": [ \"foo\", \"bar\" ] }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - // PATCH value of hidden flag, must fail with validation error - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest(ENDPOINT + "/CRUD_UT", "[{ \"op\": \"add\", \"path\": \"/hidden\", \"value\": true }]", header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue( - response.getBody(), - response.getBody().matches(".*\"invalid_keys\"\\s*:\\s*\\{\\s*\"keys\"\\s*:\\s*\"hidden\"\\s*\\}.*") - ); - - // PATCH with relative JSON pointer, must fail - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT + "/CRUD_UT", - "[{ \"op\": \"add\", \"path\": \"1/INTERNAL/allowed_actions/-\", " + "\"value\": \"OPENDISTRO_SECURITY_DELETE\" }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - // PATCH new format - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT + "/CRUD_UT", - "[{ \"op\": \"add\", \"path\": \"/allowed_actions/-\", " + "\"value\": \"OPENDISTRO_SECURITY_DELETE\" }]", - header - ); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executeGetRequest(ENDPOINT + "/CRUD_UT", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - List permissions = settings.getAsList("CRUD_UT.allowed_actions"); - Assert.assertNotNull(permissions); - Assert.assertEquals(3, permissions.size()); - Assert.assertTrue(permissions.contains("READ_UT")); - Assert.assertTrue(permissions.contains("OPENDISTRO_SECURITY_WRITE")); - Assert.assertTrue(permissions.contains("OPENDISTRO_SECURITY_DELETE")); - - // -- PATCH on whole config resource - // PATCH read only resource, must be forbidden - // SuperAdmin can patch read only resource - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"add\", \"path\": \"/GET_UT/a\", \"value\": [ \"foo\", \"bar\" ] }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest(ENDPOINT, "[{ \"op\": \"add\", \"path\": \"/GET_UT/description\", \"value\": \"foo\" }]", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // PATCH hidden resource, must be bad request - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"add\", \"path\": \"/INTERNAL/a\", \"value\": [ \"foo\", \"bar\" ] }]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - // PATCH delete read only resource, must be forbidden - // SuperAdmin can delete read only resource - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest(ENDPOINT, "[{ \"op\": \"remove\", \"path\": \"/GET_UT\" }]", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // PATCH delete hidden resource, must be bad request - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest(ENDPOINT, "[{ \"op\": \"remove\", \"path\": \"/INTERNAL\" }]", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("\"message\":\"Resource updated.")); - - // PATCH value of hidden flag, must fail with validation error - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest(ENDPOINT, "[{ \"op\": \"add\", \"path\": \"/CRUD_UT/hidden\", \"value\": true }]", header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue(response.getBody().matches(".*\"invalid_keys\"\\s*:\\s*\\{\\s*\"keys\"\\s*:\\s*\"hidden\"\\s*\\}.*")); - - // add new resource with hidden flag, must fail with validation error - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"add\", \"path\": \"/NEWNEWNEW\", \"value\": {\"allowed_actions\": [\"indices:data/write*\"], \"hidden\":true }}]", - header - ); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertTrue(response.getBody().matches(".*\"invalid_keys\"\\s*:\\s*\\{\\s*\"keys\"\\s*:\\s*\"hidden\"\\s*\\}.*")); - - // add new valid resources - rh.sendAdminCertificate = userAdminCert; - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"add\", \"path\": \"/BULKNEW1\", \"value\": {\"allowed_actions\": [\"indices:data/*\", \"cluster:monitor/*\"] } }," - + "{ \"op\": \"add\", \"path\": \"/BULKNEW2\", \"value\": {\"allowed_actions\": [\"READ_UT\"] } }]", - header - ); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executeGetRequest(ENDPOINT + "/BULKNEW1", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - permissions = settings.getAsList("BULKNEW1.allowed_actions"); - Assert.assertNotNull(permissions); - Assert.assertEquals(2, permissions.size()); - Assert.assertTrue(permissions.contains("indices:data/*")); - Assert.assertTrue(permissions.contains("cluster:monitor/*")); - - response = rh.executeGetRequest(ENDPOINT + "/BULKNEW2", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - permissions = settings.getAsList("BULKNEW2.allowed_actions"); - Assert.assertNotNull(permissions); - Assert.assertEquals(1, permissions.size()); - Assert.assertTrue(permissions.contains("READ_UT")); - - // delete resource - response = rh.executePatchRequest(ENDPOINT, "[{ \"op\": \"remove\", \"path\": \"/BULKNEW1\" }]", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executeGetRequest(ENDPOINT + "/BULKNEW1", header); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // assert other resource is still there - response = rh.executeGetRequest(ENDPOINT + "/BULKNEW2", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - permissions = settings.getAsList("BULKNEW2.allowed_actions"); - Assert.assertNotNull(permissions); - Assert.assertEquals(1, permissions.size()); - Assert.assertTrue(permissions.contains("READ_UT")); - } - - @Test - public void testActionGroupsApiForRestAdmin() throws Exception { - setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build()); - rh.sendAdminCertificate = false; - // create index - setupStarfleetIndex(); - final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); - - // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - verifyGetForSuperAdmin(new Header[] { restApiAdminHeader }); - verifyDeleteForSuperAdmin(new Header[] { restApiAdminHeader }, false); - verifyPutForSuperAdmin(new Header[] { restApiAdminHeader }, false); - verifyPatchForSuperAdmin(new Header[] { restApiAdminHeader }, false); - } - - @Test - public void testActionGroupsApiForActionGroupsRestApiAdmin() throws Exception { - setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build()); - rh.sendAdminCertificate = false; - // create index - setupStarfleetIndex(); - final Header restApiAdminActionGroupsHeader = encodeBasicHeader("rest_api_admin_actiongroups", "rest_api_admin_actiongroups"); - - // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); - verifyGetForSuperAdmin(new Header[] { restApiAdminActionGroupsHeader }); - verifyDeleteForSuperAdmin(new Header[] { restApiAdminActionGroupsHeader }, false); - verifyPutForSuperAdmin(new Header[] { restApiAdminActionGroupsHeader }, false); - verifyPatchForSuperAdmin(new Header[] { restApiAdminActionGroupsHeader }, false); - } - - @Test - public void testCreateOrUpdateRestApiAdminActionGroupForbidden() throws Exception { - setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build()); - rh.sendAdminCertificate = false; - final var userHeaders = List.of( - encodeBasicHeader("admin", "admin"), - encodeBasicHeader("test", "test"), - encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"), - encodeBasicHeader("rest_api_admin_actiongroups", "rest_api_admin_actiongroups") - ); - for (final var userHeader : userHeaders) { - // attempt to create new action group with REST admin permissions - verifyPutForbidden("new_rest_api_admin_group", restAdminAllowedActions(), userHeader); - verifyPatchForbidden(createPatchRestAdminPermissionsPayload("new_rest_api_admin_group", "add"), userHeader); - - // attempt to update existing action group which has REST admin permissions - verifyPutForbidden("rest_admin_action_group", restAdminAllowedActions(), userHeader); - verifyPatchForbidden(createPatchRestAdminPermissionsPayload("rest_admin_action_group", "replace"), userHeader); - - // attempt to update existing action group with REST admin permissions - verifyPutForbidden("OPENDISTRO_SECURITY_CLUSTER_ALL", restAdminAllowedActions(), userHeader); - verifyPatchForbidden(createPatchRestAdminPermissionsPayload("OPENDISTRO_SECURITY_CLUSTER_ALL", "replace"), userHeader); - - // attempt to delete - verifyDeleteForbidden("rest_admin_action_group", userHeader); - verifyPatchForbidden(createPatchRestAdminPermissionsPayload("rest_admin_action_group", "remove"), userHeader); - } - } - - void verifyPutForbidden(final String actionGroupName, final String payload, final Header... header) { - HttpResponse response = rh.executePutRequest(ENDPOINT + "/" + actionGroupName, payload, header); - Assert.assertEquals(response.getBody(), HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - } - - void verifyPatchForbidden(final String payload, final Header... header) { - HttpResponse response = rh.executePatchRequest(ENDPOINT, payload, header); - Assert.assertEquals(response.getBody(), HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - } - - void verifyDeleteForbidden(final String actionGroupName, final Header... header) { - HttpResponse response = rh.executeDeleteRequest(ENDPOINT + "/" + actionGroupName, header); - Assert.assertEquals(response.getBody(), HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - } - - String restAdminAllowedActions() throws JsonProcessingException { - final ObjectNode rootNode = DefaultObjectMapper.objectMapper.createObjectNode(); - rootNode.set("allowed_actions", clusterPermissionsForRestAdmin("cluster/*")); - return DefaultObjectMapper.objectMapper.writeValueAsString(rootNode); - } - - private String createPatchRestAdminPermissionsPayload(final String actionGroup, final String op) throws JsonProcessingException { - final ArrayNode rootNode = DefaultObjectMapper.objectMapper.createArrayNode(); - final ObjectNode opAddObjectNode = DefaultObjectMapper.objectMapper.createObjectNode(); - final ObjectNode allowedActionsNode = DefaultObjectMapper.objectMapper.createObjectNode(); - allowedActionsNode.set("allowed_actions", clusterPermissionsForRestAdmin("cluster/*")); - if ("add".equals(op)) { - opAddObjectNode.put("op", "add").put("path", "/" + actionGroup).set("value", allowedActionsNode); - rootNode.add(opAddObjectNode); - } - - if ("remove".equals(op)) { - final ObjectNode opRemoveObjectNode = DefaultObjectMapper.objectMapper.createObjectNode(); - opRemoveObjectNode.put("op", "remove").put("path", "/" + actionGroup); - rootNode.add(opRemoveObjectNode); - } - - if ("replace".equals(op)) { - final ObjectNode replaceRemoveObjectNode = DefaultObjectMapper.objectMapper.createObjectNode(); - replaceRemoveObjectNode.put("op", "replace") - .put("path", "/" + actionGroup + "/allowed_actions") - .set("value", clusterPermissionsForRestAdmin("*")); - - rootNode.add(replaceRemoveObjectNode); - } - return DefaultObjectMapper.objectMapper.writeValueAsString(rootNode); - } - - @Test - public void testActionGroupsApiForNonSuperAdmin() throws Exception { - - setupWithRestRoles(); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - rh.sendHTTPClientCredentials = true; - - HttpResponse response; - - // Delete read only actiongroups - response = rh.executeDeleteRequest(ENDPOINT + "/create_index", new Header[0]); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - - // Put read only actiongroups - response = rh.executePutRequest(ENDPOINT + "/create_index", FileHelper.loadFile("restapi/actiongroup_crud.json"), new Header[0]); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - - // Patch single read only actiongroups - response = rh.executePatchRequest( - ENDPOINT + "/create_index", - "[{ \"op\": \"replace\", \"path\": \"/description\", \"value\": \"foo\" }]", - new Header[0] - ); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - - // Patch multiple read only actiongroups - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"replace\", \"path\": \"/create_index/description\", \"value\": \"foo\" }]", - new Header[0] - ); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - - response = rh.executeGetRequest(ENDPOINT + "/INTERNAL", new Header[0]); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // Delete hidden actiongroups - response = rh.executeDeleteRequest(ENDPOINT + "/INTERNAL", new Header[0]); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // Put hidden actiongroups - response = rh.executePutRequest(ENDPOINT + "/INTERNAL", FileHelper.loadFile("restapi/actiongroup_crud.json"), new Header[0]); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // Patch hidden actiongroups - response = rh.executePatchRequest( - ENDPOINT + "/INTERNAL", - "[{ \"op\": \"replace\", \"path\": \"/description\", \"value\": \"foo\" }]", - new Header[0] - ); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - // Patch multiple hidden actiongroups - response = rh.executePatchRequest( - ENDPOINT, - "[{ \"op\": \"replace\", \"path\": \"/INTERNAL/description\", \"value\": \"foo\" }]", - new Header[0] - ); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - - } - - @Test - public void checkNullElementsInArray() throws Exception { - setup(); - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = true; - - String body = FileHelper.loadFile("restapi/actiongroup_null_array_element.json"); - HttpResponse response = rh.executePutRequest(ENDPOINT + "/CRUD_UT", body, new Header[0]); - Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - Assert.assertEquals(RequestContentValidator.ValidationError.NULL_ARRAY_ELEMENT.message(), settings.get("reason")); - } - -} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityInfoActionTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityInfoActionTest.java deleted file mode 100644 index 0799525eb8..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityInfoActionTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.dlic.rest.api; - -import org.apache.http.HttpStatus; -import org.junit.Assert; -import org.junit.Test; - -import org.opensearch.common.settings.Settings; -import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.test.helper.rest.RestHelper; - -import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; - -public class SecurityInfoActionTest extends AbstractRestApiUnitTest { - private final String ENDPOINT; - - protected String getEndpointPrefix() { - return PLUGINS_PREFIX; - } - - public SecurityInfoActionTest() { - ENDPOINT = getEndpointPrefix() + "/authinfo"; - } - - @Test - public void testSecurityInfoAPI() throws Exception { - Settings settings = Settings.builder() - .put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true) - .build(); - setup(settings); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = true; - RestHelper.HttpResponse response = rh.executeGetRequest(ENDPOINT); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - rh.sendAdminCertificate = false; - response = rh.executeGetRequest(ENDPOINT); - Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response.getStatusCode()); - } -} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacyActionGroupsApiTests.java b/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacyActionGroupsApiTests.java deleted file mode 100644 index e92f046f65..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacyActionGroupsApiTests.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.dlic.rest.api.legacy; - -import org.opensearch.security.dlic.rest.api.ActionGroupsApiTest; - -import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; - -public class LegacyActionGroupsApiTests extends ActionGroupsApiTest { - @Override - protected String getEndpointPrefix() { - return LEGACY_OPENDISTRO_PREFIX; - } -} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityInfoActionTests.java b/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityInfoActionTests.java deleted file mode 100644 index 75c5238f7f..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityInfoActionTests.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.dlic.rest.api.legacy; - -import org.opensearch.security.dlic.rest.api.SecurityInfoActionTest; - -import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; - -public class LegacySecurityInfoActionTests extends SecurityInfoActionTest { - @Override - protected String getEndpointPrefix() { - return LEGACY_OPENDISTRO_PREFIX; - } -}