diff --git a/CHANGELOG.md b/CHANGELOG.md index 211e3cca58..6f1fa2b69a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix test failure related to change in core to add content-encoding to response headers ([#5897](https://github.com/opensearch-project/security/pull/5897)) ### Refactoring +- Refactor plugin system index tests to use parameterized test pattern ([#5895](https://github.com/opensearch-project/security/pull/5895)) ### Maintenance - Bump `spring_version` from 7.0.1 to 7.0.2 ([#5852](https://github.com/opensearch-project/security/pull/5852)) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java similarity index 73% rename from src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java rename to src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java index 4b78fd8d14..e626c57ecf 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java @@ -9,10 +9,20 @@ */ package org.opensearch.security.privileges.int_tests; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; import org.opensearch.core.rest.RestStatus; +import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; +import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; @@ -23,35 +33,58 @@ import static org.hamcrest.Matchers.equalTo; import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1.SYSTEM_INDEX_1; import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2.SYSTEM_INDEX_2; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; -/** - * Abstract base class defining the contract for plugin system index access privileges. - *

+/* * This class serves as a single source of truth for the privilege rules governing plugin users' * access to their own system indices. It enforces consistency across different cluster configurations * (e.g., with and without explicit system index permission enabled) by defining the complete set of * tests that must pass in any environment. - *

- * Concrete implementations, such as {@link PluginSystemIndexPermissionEnabledIntTests} and - * {@link PluginSystemIndexPermissionDisabledIntTests}, provide the specific {@link LocalCluster} - * environments in which these contract tests are executed. This design leverages polymorphism - * to dynamically bind the test logic to different runtime configurations. */ -public abstract class AbstractPluginSystemIndexIntTests { +@RunWith(Parameterized.class) +public class PluginSystemIndexIntTests { + + final LocalCluster cluster; + final ClusterConfig clusterConfig; + + static LocalCluster.Builder clusterBuilder() { + return new LocalCluster.Builder().singleNode() + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(USER_ADMIN) + .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class); + } + + @ClassRule + public static final ClusterConfig.ClusterInstances clusters = new ClusterConfig.ClusterInstances( + PluginSystemIndexIntTests::clusterBuilder + ); - protected abstract LocalCluster getCluster(); + @Parameters(name = "{0}") + public static Collection params() { + List result = new ArrayList<>(); + + for (ClusterConfig clusterConfig : ClusterConfig.values()) { + result.add(new Object[] { clusterConfig }); + } + return result; + } + + public PluginSystemIndexIntTests(ClusterConfig clusterConfig) { + this.clusterConfig = clusterConfig; + this.cluster = clusters.get(clusterConfig); + } @Before public void setup() { - try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.delete(SYSTEM_INDEX_1); } } @Test public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); @@ -61,7 +94,7 @@ public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() { @Test public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByOtherPlugin() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_2); assertThat( @@ -76,7 +109,7 @@ public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByO @Test public void testPluginShouldNotBeAbleToRunClusterActions() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.get("try-cluster-health/plugin"); assertThat( @@ -91,7 +124,7 @@ public void testPluginShouldNotBeAbleToRunClusterActions() { @Test public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1 + "?runAs=user"); assertThat(response, RestMatchers.isForbidden("/error/root_cause/0/reason", "no permissions for [] and User [name=admin")); @@ -100,7 +133,7 @@ public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToInd @Test public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); @@ -109,7 +142,7 @@ public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() { @Test public void testPluginShouldBeAbleSearchOnItsSystemIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); @@ -123,7 +156,7 @@ public void testPluginShouldBeAbleSearchOnItsSystemIndex() { @Test public void testPluginShouldBeAbleGetOnItsSystemIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); @@ -143,7 +176,7 @@ public void testPluginShouldBeAbleGetOnItsSystemIndex() { @Test public void testPluginShouldBeAbleUpdateOnItsSystemIndex() { - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); @@ -163,11 +196,11 @@ public void testPluginShouldBeAbleUpdateOnItsSystemIndex() { @Test public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() { - try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.put(SYSTEM_INDEX_1); client.put(SYSTEM_INDEX_2); } - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-bulk-mixed-index"); assertThat( @@ -181,11 +214,11 @@ public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWher @Test public void testPluginShouldNotBeAbleToSearchOnMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() { - try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.put(SYSTEM_INDEX_1); client.put(SYSTEM_INDEX_2); } - try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.get("search-on-mixed-system-index"); assertThat( diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java deleted file mode 100644 index 37b8a938db..0000000000 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * 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. - * - */ -package org.opensearch.security.privileges.int_tests; - -import java.util.List; -import java.util.Map; - -import org.junit.ClassRule; - -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; -import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; - -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; -import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; -import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; - -/** - * plugins.security.system_indices.permission.enabled is not set or false - */ -public class PluginSystemIndexPermissionDisabledIntTests extends AbstractPluginSystemIndexIntTests { - - public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); - - @ClassRule - public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .anonymousAuth(false) - .authc(AUTHC_DOMAIN) - .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true - // SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY is not set (defaults to false) - ) - ) - .build(); - - @Override - protected LocalCluster getCluster() { - return cluster; - } -} diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java deleted file mode 100644 index 001fd2c007..0000000000 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * 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. - * - */ -package org.opensearch.security.privileges.int_tests; - -import java.util.List; -import java.util.Map; - -import org.junit.ClassRule; - -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; -import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; - -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY; -import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; -import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; - -/** - * plugins.security.system_indices.permission.enabled = true - */ -public class PluginSystemIndexPermissionEnabledIntTests extends AbstractPluginSystemIndexIntTests { - - public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); - - @ClassRule - public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .anonymousAuth(false) - .authc(AUTHC_DOMAIN) - .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true, - SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY, - true - ) - ) - .build(); - - @Override - protected LocalCluster getCluster() { - return cluster; - } -}