From 68c9a5986c74b0342f763aed398f07392eaad4be Mon Sep 17 00:00:00 2001 From: Dan Cristian Cecoi Date: Mon, 6 May 2024 12:49:19 +0100 Subject: [PATCH 1/8] fix build errors caused by filterIndices method being moved from SnapshotUtils to IndexUtils (#4313) Signed-off-by: Dan Cecoi Co-authored-by: Dan Cecoi --- .../opensearch/security/resolver/IndexResolverReplacer.java | 4 ++-- .../opensearch/security/support/SnapshotRestoreHelper.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java index d7aeb776c7..4a4e714348 100644 --- a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java @@ -82,6 +82,7 @@ import org.opensearch.cluster.metadata.IndexAbstraction; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.util.IndexUtils; import org.opensearch.core.index.Index; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.reindex.ReindexRequest; @@ -91,7 +92,6 @@ import org.opensearch.security.support.SnapshotRestoreHelper; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.snapshots.SnapshotInfo; -import org.opensearch.snapshots.SnapshotUtils; import org.opensearch.transport.RemoteClusterService; import org.opensearch.transport.TransportRequest; @@ -694,7 +694,7 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid ); provider.provide(new String[] { "*" }, request, false); } else { - final List requestedResolvedIndices = SnapshotUtils.filterIndices( + final List requestedResolvedIndices = IndexUtils.filterIndices( snapshotInfo.indices(), restoreRequest.indices(), restoreRequest.indicesOptions() diff --git a/src/main/java/org/opensearch/security/support/SnapshotRestoreHelper.java b/src/main/java/org/opensearch/security/support/SnapshotRestoreHelper.java index 2c85177c1e..b8f5d22e7c 100644 --- a/src/main/java/org/opensearch/security/support/SnapshotRestoreHelper.java +++ b/src/main/java/org/opensearch/security/support/SnapshotRestoreHelper.java @@ -37,12 +37,12 @@ import org.opensearch.SpecialPermission; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; import org.opensearch.action.support.PlainActionFuture; +import org.opensearch.common.util.IndexUtils; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.snapshots.SnapshotId; import org.opensearch.snapshots.SnapshotInfo; -import org.opensearch.snapshots.SnapshotUtils; import org.opensearch.threadpool.ThreadPool; public class SnapshotRestoreHelper { @@ -56,7 +56,7 @@ public static List resolveOriginalIndices(RestoreSnapshotRequest restore log.warn("snapshot repository '{}', snapshot '{}' not found", restoreRequest.repository(), restoreRequest.snapshot()); return null; } else { - return SnapshotUtils.filterIndices(snapshotInfo.indices(), restoreRequest.indices(), restoreRequest.indicesOptions()); + return IndexUtils.filterIndices(snapshotInfo.indices(), restoreRequest.indices(), restoreRequest.indicesOptions()); } } From 07d8bb937b16c3e5c7b5a3e3a4325b9dd6b07847 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 08:26:48 -0400 Subject: [PATCH 2/8] Bump org.apache.camel:camel-xmlsecurity from 3.22.1 to 3.22.2 (#4320) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 4fe975e00b..eddad5e3fa 100644 --- a/build.gradle +++ b/build.gradle @@ -610,7 +610,7 @@ dependencies { runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.2' runtimeOnly 'org.ow2.asm:asm:9.7' - testImplementation 'org.apache.camel:camel-xmlsecurity:3.22.1' + testImplementation 'org.apache.camel:camel-xmlsecurity:3.22.2' //OpenSAML implementation 'net.shibboleth.utilities:java-support:8.4.2' From e3803d9c7a56de78a0042f5521a0efa7c7db3fe7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 09:00:47 -0400 Subject: [PATCH 3/8] Bump com.google.errorprone:error_prone_annotations from 2.27.0 to 2.27.1 (#4323) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index eddad5e3fa..7979e31e10 100644 --- a/build.gradle +++ b/build.gradle @@ -494,7 +494,7 @@ configurations { // For integrationTest force "org.apache.httpcomponents:httpclient:4.5.14" force "org.apache.httpcomponents:httpcore:4.4.16" - force "com.google.errorprone:error_prone_annotations:2.27.0" + force "com.google.errorprone:error_prone_annotations:2.27.1" force "org.checkerframework:checker-qual:3.42.0" force "ch.qos.logback:logback-classic:1.5.6" } @@ -605,7 +605,7 @@ dependencies { runtimeOnly 'com.eclipsesource.minimal-json:minimal-json:0.9.5' runtimeOnly 'commons-codec:commons-codec:1.17.0' runtimeOnly 'org.cryptacular:cryptacular:1.2.6' - compileOnly 'com.google.errorprone:error_prone_annotations:2.27.0' + compileOnly 'com.google.errorprone:error_prone_annotations:2.27.1' runtimeOnly 'com.sun.istack:istack-commons-runtime:4.2.0' runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.2' runtimeOnly 'org.ow2.asm:asm:9.7' From 10afbfe4210bf087b920b2ad1361ec356ba61381 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 09:01:53 -0400 Subject: [PATCH 4/8] Bump org.scala-lang:scala-library from 2.13.13 to 2.13.14 (#4321) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 7979e31e10..85b66e2841 100644 --- a/build.gradle +++ b/build.gradle @@ -471,7 +471,7 @@ configurations { resolutionStrategy { force 'commons-codec:commons-codec:1.17.0' force 'org.slf4j:slf4j-api:1.7.36' - force 'org.scala-lang:scala-library:2.13.13' + force 'org.scala-lang:scala-library:2.13.14' force "com.fasterxml.jackson:jackson-bom:${versions.jackson}" force "com.fasterxml.jackson.core:jackson-core:${versions.jackson}" force "com.fasterxml.jackson.datatype:jackson-datatype-jdk8:${versions.jackson}" @@ -699,7 +699,7 @@ dependencies { testRuntimeOnly ("org.springframework:spring-core:${spring_version}") { exclude(group:'org.springframework', module: 'spring-jcl' ) } - testRuntimeOnly 'org.scala-lang:scala-library:2.13.13' + testRuntimeOnly 'org.scala-lang:scala-library:2.13.14' testRuntimeOnly 'com.typesafe.scala-logging:scala-logging_3:3.9.5' testRuntimeOnly('org.apache.zookeeper:zookeeper:3.9.2') { exclude(group:'ch.qos.logback', module: 'logback-classic' ) From 5c18a36e278cd0e47bc80b545b06b37320608465 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 6 May 2024 15:17:57 +0200 Subject: [PATCH 5/8] Add space between definitions in gradle build (#4304) Signed-off-by: Andrey Pleskach --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index 85b66e2841..746b50342e 100644 --- a/build.gradle +++ b/build.gradle @@ -25,6 +25,7 @@ buildscript { opensearch_build = version_tokens[0] + '.0' common_utils_version = System.getProperty("common_utils.version", '3.0.0.0-SNAPSHOT') + kafka_version = '3.7.0' apache_cxf_version = '4.0.4' open_saml_version = '4.3.2' From 3b94522339ee512142d3a99a2e8d3f69f2ac80be Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 09:41:23 -0400 Subject: [PATCH 6/8] Bump org.checkerframework:checker-qual from 3.42.0 to 3.43.0 (#4322) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 746b50342e..c4cf77a5e8 100644 --- a/build.gradle +++ b/build.gradle @@ -496,7 +496,7 @@ configurations { force "org.apache.httpcomponents:httpclient:4.5.14" force "org.apache.httpcomponents:httpcore:4.4.16" force "com.google.errorprone:error_prone_annotations:2.27.1" - force "org.checkerframework:checker-qual:3.42.0" + force "org.checkerframework:checker-qual:3.43.0" force "ch.qos.logback:logback-classic:1.5.6" } } @@ -650,7 +650,7 @@ dependencies { runtimeOnly 'org.apache.ws.xmlschema:xmlschema-core:2.3.1' runtimeOnly 'org.apache.santuario:xmlsec:2.3.4' runtimeOnly "com.github.luben:zstd-jni:${versions.zstd}" - runtimeOnly 'org.checkerframework:checker-qual:3.42.0' + runtimeOnly 'org.checkerframework:checker-qual:3.43.0' runtimeOnly "org.bouncycastle:bcpkix-jdk18on:${versions.bouncycastle}" runtimeOnly 'org.scala-lang.modules:scala-java8-compat_3:1.0.2' From faac5e3bfdd55aad69a4bb2cfc8c31c6988d46ea Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Tue, 7 May 2024 16:40:15 +0200 Subject: [PATCH 7/8] REST API tests refactoring (Part 4) (#4255) Signed-off-by: Andrey Pleskach --- .../api/ConfigRestApiIntegrationTest.java | 117 ++++++++++ ...DefaultApiAvailabilityIntegrationTest.java | 15 +- .../security/api/PatchPayloadHelper.java | 66 ++++++ .../rest/api/SecurityConfigApiActionTest.java | 211 ------------------ .../LegacySecurityConfigApiActionTests.java | 23 -- 5 files changed, 186 insertions(+), 246 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java create mode 100644 src/integrationTest/java/org/opensearch/security/api/PatchPayloadHelper.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java delete mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java diff --git a/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java new file mode 100644 index 0000000000..3ccafda1d2 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java @@ -0,0 +1,117 @@ +/* + * 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.StringJoiner; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.Test; + +import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.dlic.rest.api.Endpoint; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.opensearch.security.api.PatchPayloadHelper.patch; +import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; +import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; +import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION; + +public class ConfigRestApiIntegrationTest extends AbstractApiIntegrationTest { + + final static String REST_API_ADMIN_CONFIG_UPDATE = "rest-api-admin-config-update"; + + static { + clusterSettings.put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + testSecurityConfig.withRestAdminUser(REST_ADMIN_USER, allRestAdminPermissions()) + .withRestAdminUser(REST_API_ADMIN_CONFIG_UPDATE, restAdminPermission(Endpoint.CONFIG, SECURITY_CONFIG_UPDATE)); + } + + private String securityConfigPath(final String... path) { + final var fullPath = new StringJoiner("/").add(super.apiPath("securityconfig")); + if (path != null) for (final var p : path) + fullPath.add(p); + return fullPath.toString(); + } + + @Test + public void forbiddenForRegularUsers() throws Exception { + withUser(NEW_USER, client -> { + forbidden(() -> client.get(securityConfigPath())); + forbidden(() -> client.putJson(securityConfigPath("config"), EMPTY_BODY)); + forbidden(() -> client.patch(securityConfigPath(), EMPTY_BODY)); + verifyNotAllowedMethods(client); + }); + } + + @Test + public void partiallyAvailableForAdminUser() throws Exception { + withUser(ADMIN_USER_NAME, client -> ok(() -> client.get(securityConfigPath()))); + withUser(ADMIN_USER_NAME, client -> { + badRequest(() -> client.putJson(securityConfigPath("xxx"), EMPTY_BODY)); + forbidden(() -> client.putJson(securityConfigPath("config"), EMPTY_BODY)); + forbidden(() -> client.patch(securityConfigPath(), EMPTY_BODY)); + }); + withUser(ADMIN_USER_NAME, this::verifyNotAllowedMethods); + } + + @Test + public void availableForTlsAdminUser() throws Exception { + withUser(ADMIN_USER_NAME, localCluster.getAdminCertificate(), client -> ok(() -> client.get(securityConfigPath()))); + withUser(ADMIN_USER_NAME, localCluster.getAdminCertificate(), this::verifyUpdate); + } + + @Test + public void availableForRestAdminUser() throws Exception { + withUser(REST_ADMIN_USER, client -> ok(() -> client.get(securityConfigPath()))); + withUser(REST_ADMIN_USER, this::verifyUpdate); + withUser(REST_API_ADMIN_CONFIG_UPDATE, this::verifyUpdate); + } + + void verifyUpdate(final TestRestClient client) throws Exception { + badRequest(() -> client.putJson(securityConfigPath("xxx"), EMPTY_BODY)); + verifyNotAllowedMethods(client); + + final var configJson = ok(() -> client.get(securityConfigPath())).bodyAsJsonNode(); + final var authFailureListeners = DefaultObjectMapper.objectMapper.createObjectNode(); + authFailureListeners.set( + "ip_rate_limiting", + DefaultObjectMapper.objectMapper.createObjectNode() + .put("type", "ip") + .put("allowed_tries", 10) + .put("time_window_seconds", 3_600) + .put("block_expiry_seconds", 600) + .put("max_blocked_clients", 100_000) + .put("max_tracked_clients", 100_000) + ); + authFailureListeners.set( + "internal_authentication_backend_limiting", + DefaultObjectMapper.objectMapper.createObjectNode() + .put("type", "username") + .put("authentication_backend", "intern") + .put("allowed_tries", 10) + .put("time_window_seconds", 3_600) + .put("block_expiry_seconds", 600) + .put("max_blocked_clients", 100_000) + .put("max_tracked_clients", 100_000) + ); + final var dynamicConfigJson = (ObjectNode) configJson.get("config").get("dynamic"); + dynamicConfigJson.set("auth_failure_listeners", authFailureListeners); + ok(() -> client.putJson(securityConfigPath("config"), DefaultObjectMapper.writeValueAsString(configJson.get("config"), false))); + ok(() -> client.patch(securityConfigPath(), patch(replaceOp("/config/dynamic/hosts_resolver_mode", "other")))); + } + + void verifyNotAllowedMethods(final TestRestClient client) throws Exception { + methodNotAllowed(() -> client.postJson(securityConfigPath(), EMPTY_BODY)); + methodNotAllowed(() -> client.delete(securityConfigPath())); + } + +} diff --git a/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java index eaecb275db..f3b701dc38 100644 --- a/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java @@ -17,6 +17,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; +import static org.opensearch.security.api.PatchPayloadHelper.patch; +import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; public class DefaultApiAvailabilityIntegrationTest extends AbstractApiIntegrationTest { @@ -49,18 +51,7 @@ private void verifySecurityConfigApi(final TestRestClient client) throws Excepti methodNotAllowed(() -> client.putJson(apiPath("securityconfig"), EMPTY_BODY)); methodNotAllowed(() -> client.postJson(apiPath("securityconfig"), EMPTY_BODY)); methodNotAllowed(() -> client.delete(apiPath("securityconfig"))); - forbidden( - () -> client.patch( - apiPath("securityconfig"), - (builder, params) -> builder.startArray() - .startObject() - .field("op", "replace") - .field("path", "/a/b/c") - .field("value", "other") - .endObject() - .endArray() - ) - ); + forbidden(() -> client.patch(apiPath("securityconfig"), patch(replaceOp("/a/b/c", "other")))); } @Test diff --git a/src/integrationTest/java/org/opensearch/security/api/PatchPayloadHelper.java b/src/integrationTest/java/org/opensearch/security/api/PatchPayloadHelper.java new file mode 100644 index 0000000000..d7bae323bd --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/api/PatchPayloadHelper.java @@ -0,0 +1,66 @@ +/* + * 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.Locale; + +import org.opensearch.core.xcontent.ToXContentObject; + +interface PatchPayloadHelper extends ToXContentObject { + + enum Op { + ADD, + REPLACE, + REMOVE; + } + + static ToXContentObject addOp(final String path, final T value) { + return operation(Op.ADD, path, value); + } + + static ToXContentObject replaceOp(final String path, final T value) { + return operation(Op.REPLACE, path, value); + } + + static ToXContentObject removeOp(final String path) { + return operation(Op.REMOVE, path, null); + } + + private static ToXContentObject operation(final Op op, final String path, final T value) { + return (builder, params) -> { + final var opPath = path.startsWith("/") ? path : "/" + path; + builder.startObject().field("op", op.name().toLowerCase(Locale.ROOT)).field("path", opPath); + if (value != null) { + if (value instanceof ToXContentObject) { + builder.field("value", (ToXContentObject) value); + } else if (value instanceof String) { + builder.field("value", (String) value); + } else if (value instanceof Boolean) { + builder.field("value", (Boolean) value); + } else { + throw new IllegalArgumentException("Unsupported java type " + value.getClass()); + } + } + return builder.endObject(); + }; + } + + static ToXContentObject patch(final ToXContentObject... operations) { + return (builder, params) -> { + builder.startArray(); + for (final var o : operations) + o.toXContent(builder, EMPTY_PARAMS); + return builder.endArray(); + }; + } + +} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java deleted file mode 100644 index c4066d11a2..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java +++ /dev/null @@ -1,211 +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.hc.core5.http.Header; -import org.apache.http.HttpStatus; -import org.junit.Assert; -import org.junit.Test; - -import org.opensearch.common.settings.Settings; -import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.support.ConfigConstants; -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; - -public class SecurityConfigApiActionTest extends AbstractRestApiUnitTest { - private final String ENDPOINT; - - protected String getEndpointPrefix() { - return PLUGINS_PREFIX; - } - - public SecurityConfigApiActionTest() { - ENDPOINT = getEndpointPrefix() + "/api"; - } - - @Test - public void testSecurityConfigApiReadForSuperAdmin() throws Exception { - - setup(); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = true; - - verifyResponsesWithoutPermissionOrUnsupportedFlag(); - } - - @Test - public void testSecurityConfigApiReadRestApiUser() throws Exception { - - setupWithRestRoles(); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - - final var restApiHeader = encodeBasicHeader("test", "test"); - verifyResponsesWithoutPermissionOrUnsupportedFlag(restApiHeader); - } - - private void verifyResponsesWithoutPermissionOrUnsupportedFlag(final Header... headers) { - HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", headers); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - response = rh.executePutRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - - response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - - response = rh.executePatchRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", headers); - Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode()); - - response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", headers); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - } - - @Test - public void testSecurityConfigApiWriteWithUnsupportedFlagForSuperAdmin() 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; - - verifyWriteOperations(); - } - - @Test - public void testSecurityConfigApiWriteWithFullListOfPermissions() throws Exception { - - Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); - setupWithRestRoles(settings); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - - final var restAdminFullAccess = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); - verifyWriteOperations(restAdminFullAccess); - } - - @Test - public void testSecurityConfigApiWriteWithOnePermission() throws Exception { - Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); - setupWithRestRoles(settings); - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - final var updateOnlyRestApiHeader = encodeBasicHeader("rest_api_admin_config_update", "rest_api_admin_config_update"); - verifyWriteOperations(updateOnlyRestApiHeader); - } - - private void verifyWriteOperations(final Header... header) throws Exception { - HttpResponse response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - response = rh.executePutRequest(ENDPOINT + "/securityconfig/xxx", FileHelper.loadFile("restapi/securityconfig.json"), header); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", FileHelper.loadFile("restapi/securityconfig.json"), header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", FileHelper.loadFile("restapi/invalid_config.json"), header); - Assert.assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, response.getStatusCode()); - Assert.assertTrue(response.getContentType(), response.isJsonContentType()); - Assert.assertTrue(response.getBody().contains("Unrecognized field")); - - response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - response = rh.executePostRequest(ENDPOINT + "/securityconfig", "{\"xxx\": 1}", header); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - - response = rh.executePatchRequest( - ENDPOINT + "/securityconfig", - "[{\"op\": \"replace\",\"path\": \"/config/dynamic/hosts_resolver_mode\",\"value\": \"other\"}]", - header - ); - Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); - - response = rh.executeDeleteRequest(ENDPOINT + "/securityconfig", header); - Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); - } - - @Test - public void testSecurityConfigForPatchWithUnsupportedFlag() 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; - verifyPatch(); - } - - @Test - public void testSecurityConfigForPatchWithFullPermissions() throws Exception { - - Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); - setupWithRestRoles(settings); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - - // non-default config - final var restAdminFullAccess = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); - verifyPatch(restAdminFullAccess); - } - - @Test - public void testSecurityConfigForPatchWithOnePermission() throws Exception { - - Settings settings = Settings.builder().put(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, true).build(); - setupWithRestRoles(settings); - - rh.keystore = "restapi/kirk-keystore.jks"; - rh.sendAdminCertificate = false; - - // non-default config - final var updateOnlyRestApiHeader = encodeBasicHeader("rest_api_admin_config_update", "rest_api_admin_config_update"); - verifyPatch(updateOnlyRestApiHeader); - } - - private void verifyPatch(final Header... header) throws Exception { - String updatedConfig = FileHelper.loadFile("restapi/securityconfig_nondefault.json"); - - // update config - HttpResponse response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", updatedConfig, header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // make patch request - response = rh.executePatchRequest( - ENDPOINT + "/securityconfig", - "[{\"op\": \"add\",\"path\": \"/config/dynamic/do_not_fail_on_forbidden\",\"value\": \"false\"}]", - header - ); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // get config - response = rh.executeGetRequest(ENDPOINT + "/securityconfig", header); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - // verify configs are same - Assert.assertEquals(DefaultObjectMapper.readTree(updatedConfig), DefaultObjectMapper.readTree(response.getBody()).get("config")); - } - -} diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java b/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java deleted file mode 100644 index 7d94de03bd..0000000000 --- a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.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.SecurityConfigApiActionTest; - -import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; - -public class LegacySecurityConfigApiActionTests extends SecurityConfigApiActionTest { - @Override - protected String getEndpointPrefix() { - return LEGACY_OPENDISTRO_PREFIX; - } -} From bac93dc42a6e2182c6c8a6d220e92e6fc1d3510a Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Wed, 8 May 2024 16:20:39 +0200 Subject: [PATCH 8/8] Fix flaky tests (#4329) Signed-off-by: Andrey Pleskach --- .../test/framework/cluster/LocalOpenSearchCluster.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java index 8a14daeb2d..0aee3705b5 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java @@ -105,8 +105,6 @@ public class LocalOpenSearchCluster { private File snapshotDir; - private int nodeCounter = 0; - public LocalOpenSearchCluster( String clusterName, ClusterManager clusterManager, @@ -302,10 +300,9 @@ private CompletableFuture startNodes( Iterator httpPortIterator = httpPorts.iterator(); List> futures = new ArrayList<>(); - for (NodeSettings nodeSettings : nodeSettingList) { - Node node = new Node(nodeCounter, nodeSettings, transportPortIterator.next(), httpPortIterator.next()); + for (var i = 0; i < nodeSettingList.size(); i++) { + Node node = new Node(i, nodeSettingList.get(i), transportPortIterator.next(), httpPortIterator.next()); futures.add(node.start()); - nodeCounter += 1; } return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])); } @@ -518,7 +515,6 @@ private Settings getOpenSearchSettings() { .build(); if (nodeSettingsSupplier != null) { - // TODO node number return Settings.builder().put(settings).put(nodeSettingsSupplier.get(nodeNumber)).build(); } return settings;