From 30760168263404e628a25fd13a54100d2610810c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 2 Aug 2024 10:16:16 -0400 Subject: [PATCH] [2.16] Revert change to use System Index Registry from core to determine if request matches system indices (#4616) --- ...nsearch-security.release-notes-2.16.0.0.md | 2 - .../opensearch/security/SystemIndexTests.java | 83 ------------------- .../http/ExampleSystemIndexPlugin.java | 27 ------ .../privileges/PrivilegesEvaluator.java | 6 +- ...java => SecurityIndexAccessEvaluator.java} | 17 ++-- ... => SecurityIndexAccessEvaluatorTest.java} | 6 +- 6 files changed, 13 insertions(+), 128 deletions(-) delete mode 100644 src/integrationTest/java/org/opensearch/security/SystemIndexTests.java delete mode 100644 src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java rename src/main/java/org/opensearch/security/privileges/{SystemIndexAccessEvaluator.java => SecurityIndexAccessEvaluator.java} (96%) rename src/test/java/org/opensearch/security/privileges/{SystemIndexAccessEvaluatorTest.java => SecurityIndexAccessEvaluatorTest.java} (99%) diff --git a/release-notes/opensearch-security.release-notes-2.16.0.0.md b/release-notes/opensearch-security.release-notes-2.16.0.0.md index e35300f061..53445cca02 100644 --- a/release-notes/opensearch-security.release-notes-2.16.0.0.md +++ b/release-notes/opensearch-security.release-notes-2.16.0.0.md @@ -4,7 +4,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0 ### Enhancements * Add support for PBKDF2 for password hashing & add support for configuring BCrypt and PBKDF2 ([#4524](https://github.com/opensearch-project/security/pull/4524)) -* Use SystemIndexRegistry from core to determine if request contains system indices ([#4471](https://github.com/opensearch-project/security/pull/4471)) * Separated DLS/FLS privilege evaluation from action privilege evaluation ([#4490](https://github.com/opensearch-project/security/pull/4490)) * Update PULL_REQUEST_TEMPLATE to include an API spec change in the checklist. ([#4533](https://github.com/opensearch-project/security/pull/4533)) * Update PATCH API to fail validation if nothing changes ([#4530](https://github.com/opensearch-project/security/pull/4530)) @@ -26,7 +25,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0 ### Maintenance * Remove unused dependancy Apache CXF ([#4580](https://github.com/opensearch-project/security/pull/4580)) * Remove unnecessary return statements ([#4558](https://github.com/opensearch-project/security/pull/4558)) -* Pass set to SystemIndexRegistry.matchesSystemIndexPattern ([#4569](https://github.com/opensearch-project/security/pull/4569)) * Refactor and update existing ml roles ([#4151](https://github.com/opensearch-project/security/pull/4151)) * Replace JUnit assertEquals() with Hamcrest matchers assertThat() ([#4544](https://github.com/opensearch-project/security/pull/4544)) * Update Gradle to 8.9 ([#4553](https://github.com/opensearch-project/security/pull/4553)) diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java deleted file mode 100644 index 599ffe9ad2..0000000000 --- a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java +++ /dev/null @@ -1,83 +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; - -import java.util.List; -import java.util.Map; - -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.runner.RunWith; - -import org.opensearch.core.rest.RestStatus; -import org.opensearch.security.http.ExampleSystemIndexPlugin; -import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; -import org.opensearch.test.framework.cluster.TestRestClient; -import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -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; - -@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) -@ThreadLeakScope(ThreadLeakScope.Scope.NONE) -public class SystemIndexTests { - - 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(ExampleSystemIndexPlugin.class) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true - ) - ) - .build(); - - @Test - public void adminShouldNotBeAbleToDeleteSecurityIndex() { - try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { - HttpResponse response = client.delete(".opendistro_security"); - - assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); - - // Create regular index - client.put("test-index"); - - // regular user can delete non-system index - HttpResponse response2 = client.delete("test-index"); - - assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus())); - - // regular use can create system index - HttpResponse response3 = client.put(".system-index1"); - - assertThat(response3.getStatusCode(), equalTo(RestStatus.OK.getStatus())); - - // regular user cannot delete system index - HttpResponse response4 = client.delete(".system-index1"); - - assertThat(response4.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); - } - } -} diff --git a/src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java b/src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java deleted file mode 100644 index b4877aae14..0000000000 --- a/src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java +++ /dev/null @@ -1,27 +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.http; - -import java.util.Collection; -import java.util.Collections; - -import org.opensearch.common.settings.Settings; -import org.opensearch.indices.SystemIndexDescriptor; -import org.opensearch.plugins.Plugin; -import org.opensearch.plugins.SystemIndexPlugin; - -public class ExampleSystemIndexPlugin extends Plugin implements SystemIndexPlugin { - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index1", "System index 1"); - return Collections.singletonList(systemIndexDescriptor); - } -} diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index fb320e6bd9..e297d666f2 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -136,7 +136,7 @@ public class PrivilegesEvaluator { private ConfigModel configModel; private final IndexResolverReplacer irr; private final SnapshotRestoreEvaluator snapshotRestoreEvaluator; - private final SystemIndexAccessEvaluator systemIndexAccessEvaluator; + private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator; private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator; private final TermsAggregationEvaluator termsAggregationEvaluator; private final PitPrivilegesEvaluator pitPrivilegesEvaluator; @@ -172,7 +172,7 @@ public PrivilegesEvaluator( this.clusterInfoHolder = clusterInfoHolder; this.irr = irr; snapshotRestoreEvaluator = new SnapshotRestoreEvaluator(settings, auditLog); - systemIndexAccessEvaluator = new SystemIndexAccessEvaluator(settings, auditLog, irr); + securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr); protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog); termsAggregationEvaluator = new TermsAggregationEvaluator(); pitPrivilegesEvaluator = new PitPrivilegesEvaluator(); @@ -328,7 +328,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) } // Security index access - if (systemIndexAccessEvaluator.evaluate( + if (securityIndexAccessEvaluator.evaluate( request, task, action0, diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java similarity index 96% rename from src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java rename to src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java index 38825a9bf1..b984ee93b8 100644 --- a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java @@ -41,7 +41,6 @@ import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; -import org.opensearch.indices.SystemIndexRegistry; import org.opensearch.security.auditlog.AuditLog; import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.resolver.IndexResolverReplacer.Resolved; @@ -57,7 +56,7 @@ * - The term `protected system indices` used here translates to system indices * which have an added layer of security and cannot be accessed by anyone except Super Admin */ -public class SystemIndexAccessEvaluator { +public class SecurityIndexAccessEvaluator { Logger log = LogManager.getLogger(this.getClass()); @@ -73,7 +72,7 @@ public class SystemIndexAccessEvaluator { private final boolean isSystemIndexEnabled; private final boolean isSystemIndexPermissionEnabled; - public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) { + public SecurityIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) { this.securityIndex = settings.get( ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX @@ -84,7 +83,6 @@ public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, In this.systemIndexMatcher = WildcardMatcher.from( settings.getAsList(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, ConfigConstants.SECURITY_SYSTEM_INDICES_DEFAULT) ); - this.superAdminAccessOnlyIndexMatcher = WildcardMatcher.from(this.securityIndex); this.isSystemIndexEnabled = settings.getAsBoolean( ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY, @@ -170,16 +168,15 @@ private boolean requestContainsAnySystemIndices(final Resolved requestedResolved * It will always return security index if it is present in the request, as security index is protected regardless * of feature being enabled or disabled * @param requestedResolved request which contains indices to be matched against system indices - * @return the set of protected system indices present in the request + * @return the list of protected system indices present in the request */ - private Set getAllSystemIndices(final Resolved requestedResolved) { - final Set systemIndices = requestedResolved.getAllIndices() + private List getAllSystemIndices(final Resolved requestedResolved) { + final List systemIndices = requestedResolved.getAllIndices() .stream() .filter(securityIndex::equals) - .collect(Collectors.toSet()); + .collect(Collectors.toList()); if (isSystemIndexEnabled) { systemIndices.addAll(systemIndexMatcher.getMatchAny(requestedResolved.getAllIndices(), Collectors.toList())); - systemIndices.addAll(SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices())); } return systemIndices; } @@ -213,7 +210,7 @@ private List getAllProtectedSystemIndices(final Resolved requestedResolv private boolean requestContainsAnyRegularIndices(final Resolved requestedResolved) { Set allIndices = requestedResolved.getAllIndices(); - Set allSystemIndices = getAllSystemIndices(requestedResolved); + List allSystemIndices = getAllSystemIndices(requestedResolved); List allProtectedSystemIndices = getAllProtectedSystemIndices(requestedResolved); return allIndices.stream().anyMatch(index -> !allSystemIndices.contains(index) && !allProtectedSystemIndices.contains(index)); diff --git a/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java similarity index 99% rename from src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java rename to src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java index fa8a991db9..6def646981 100644 --- a/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java @@ -56,7 +56,7 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) -public class SystemIndexAccessEvaluatorTest { +public class SecurityIndexAccessEvaluatorTest { @Mock private AuditLog auditLog; @@ -73,7 +73,7 @@ public class SystemIndexAccessEvaluatorTest { @Mock ClusterService cs; - private SystemIndexAccessEvaluator evaluator; + private SecurityIndexAccessEvaluator evaluator; private static final String UNPROTECTED_ACTION = "indices:data/read"; private static final String PROTECTED_ACTION = "indices:data/write"; @@ -137,7 +137,7 @@ public void setup( // when trying to resolve Index Names - evaluator = new SystemIndexAccessEvaluator( + evaluator = new SecurityIndexAccessEvaluator( Settings.builder() .put(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, TEST_SYSTEM_INDEX) .put(ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY, isSystemIndexEnabled)