Testing small change to DlsFlsValveImpl

CHANGELOG.md

@@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Features
 
 ### Enhancements
+- Introduce new dynamic setting (plugins.security.dls.write_blocked) to block all writes when restrictions apply ([#5828](https://github.com/opensearch-project/security/pull/5828))
+
 - Support nested JWT claims in role DLS queries ([#5687](https://github.com/opensearch-project/security/issues/5687))
 ### Bug Fixes
 - Fix IllegalArgumentException when resolved indices are empty in PrivilegesEvaluator ([#5770](https://github.com/opensearch-project/security/pull/5797))

bwc-test/src/test/java/org/opensearch/security/bwc/SecurityBackwardsCompatibilityIT.java

@@ -337,7 +337,7 @@ private void ingestData(String index) throws IOException {
                 bulkRequestBody.append(Song.randomSong().asJson() + "\n");
             }
             List<Response> responses = RestHelper.requestAgainstAllNodes(
-                testUserRestClient,
+                adminClient(),
                 "POST",
                 "_bulk?refresh=wait_for",
                 new StringEntity(bulkRequestBody.toString(), APPLICATION_NDJSON)
@@ -413,30 +413,31 @@ private boolean resourceExists(String url) throws IOException {
      */
     private void createTestRoleIfNotExists(String role) throws IOException {
         String url = "_plugins/_security/api/roles/" + role;
-        String roleSettings = "{\n"
-            + "  \"cluster_permissions\": [\n"
-            + "    \"unlimited\"\n"
-            + "  ],\n"
-            + "  \"index_permissions\": [\n"
-            + "    {\n"
-            + "      \"index_patterns\": [\n"
-            + "        \"test_index*\"\n"
-            + "      ],\n"
-            + "      \"dls\": \"{ \\\"bool\\\": { \\\"must\\\": { \\\"match\\\": { \\\"genre\\\": \\\"rock\\\" } } } }\",\n"
-            + "      \"fls\": [\n"
-            + "        \"~lyrics\"\n"
-            + "      ],\n"
-            + "      \"masked_fields\": [\n"
-            + "        \"artist\"\n"
-            + "      ],\n"
-            + "      \"allowed_actions\": [\n"
-            + "        \"read\",\n"
-            + "        \"write\"\n"
-            + "      ]\n"
-            + "    }\n"
-            + "  ],\n"
-            + "  \"tenant_permissions\": []\n"
-            + "}\n";
+        String roleSettings = """
+            {
+              "cluster_permissions": [
+                "unlimited"
+              ],
+              "index_permissions": [
+                {
+                  "index_patterns": [
+                    "test_index*"
+                  ],
+                  "dls": "{ \\\"bool\\\": { \\\"must\\\": { \\\"match\\\": { \\\"genre\\\": \\\"rock\\\" } } } }",
+                  "fls": [
+                    "~lyrics"
+                  ],
+                  "masked_fields": [
+                    "artist"
+                  ],
+                  "allowed_actions": [
+                    "read"
+                  ]
+                }
+              ],
+              "tenant_permissions": []
+            }
+            """;
         Response response = RestHelper.makeRequest(adminClient(), "PUT", url, RestHelper.toHttpEntity(roleSettings));
 
         assertThat(response.getStatusLine().getStatusCode(), anyOf(equalTo(200), equalTo(201)));

src/integrationTest/java/org/opensearch/security/TlsHostnameVerificationTests.java

@@ -10,48 +10,66 @@
 package org.opensearch.security;
 
 import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
-import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 
-import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.test.framework.certificate.TestCertificates;
 import org.opensearch.test.framework.cluster.ClusterManager;
 import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.log.LogCapturingAppender;
 import org.opensearch.test.framework.log.LogsRule;
 
-import static org.opensearch.common.network.NetworkModule.TRANSPORT_SSL_ENFORCE_HOSTNAME_VERIFICATION_KEY;
+import static java.util.concurrent.Executors.newSingleThreadExecutor;
+import static org.awaitility.Awaitility.await;
 
 public class TlsHostnameVerificationTests {
 
     @Rule
     public LogsRule logsRule = new LogsRule("org.opensearch.transport.netty4.ssl.SecureNetty4Transport");
 
-    public LocalCluster.Builder clusterBuilder = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
+    public LocalCluster.Builder clusterBuilder = new LocalCluster.Builder().clusterManager(ClusterManager.DEFAULT)
         .anonymousAuth(false)
         .loadConfigurationIntoIndex(false)
-        .nodeSettings(Map.of(ConfigConstants.SECURITY_SSL_ONLY, true, TRANSPORT_SSL_ENFORCE_HOSTNAME_VERIFICATION_KEY, true))
+        .nodeSettings(
+            Map.of("plugins.security.ssl_only", true, "transport.ssl.enforce_hostname_verification", true, "cluster.join.timeout", "10s")
+        )
         .sslOnly(true);
 
     @Test
     public void clusterShouldStart_nodesSanIpsAreValid() {
         // Note: We cannot use hostnames in this environment. However, IP addresses also work as valid SANs which are also
         // subject to hostname verification. Thus, we use here certificates with IP SANs
-        TestCertificates testCertificates = new TestCertificates(ClusterManager.THREE_CLUSTER_MANAGERS.getNodes(), "127.0.0.1");
+        TestCertificates testCertificates = new TestCertificates(ClusterManager.DEFAULT.getNodes(), "127.0.0.1");
         try (LocalCluster cluster = clusterBuilder.testCertificates(testCertificates).build()) {
             cluster.before();
-        } catch (Exception e) {
-            Assert.fail("Cluster should start, no exception expected but got: " + e.getMessage());
         }
     }
 
     @Test
     public void clusterShouldNotStart_nodesSanIpsAreInvalid() {
-        TestCertificates testCertificates = new TestCertificates(ClusterManager.THREE_CLUSTER_MANAGERS.getNodes(), "127.0.0.2");
-        try (LocalCluster cluster = clusterBuilder.testCertificates(testCertificates).build()) {
-            cluster.before();
-            Assert.fail("Cluster should not start, an exception expected");
+        TestCertificates testCertificates = new TestCertificates(ClusterManager.DEFAULT.getNodes(), "127.0.0.2");
+        try (
+            LocalCluster cluster = clusterBuilder.testCertificates(testCertificates).build();
+            ExecutorService executorService = newSingleThreadExecutor()
+        ) {
+            Future<Void> clusterFuture = executorService.submit(() -> {
+                cluster.before();
+                return null;
+            });
+            await().alias("expect error message about hostname verification")
+                .pollDelay(10, TimeUnit.MILLISECONDS)
+                .until(
+                    () -> LogCapturingAppender.getLogMessagesAsString()
+                        .stream()
+                        .anyMatch(
+                            message -> message.contains("(certificate_unknown) No subject alternative names matching IP address 127.0.0.1")
+                        )
+                );
+            clusterFuture.cancel(true);
         } catch (Exception e) {
             logsRule.assertThatContain("No subject alternative names matching IP address 127.0.0.1 found");
         }

src/integrationTest/java/org/opensearch/security/dlsfls/DlsWriteBlockedIntegrationTest.java

@@ -0,0 +1,133 @@
+/*
+ * 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.dlsfls;
+
+import java.io.IOException;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import org.opensearch.test.framework.TestSecurityConfig.Role;
+import org.opensearch.test.framework.TestSecurityConfig.User;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.TestRestClient;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+import static org.opensearch.test.framework.matcher.RestMatchers.isCreated;
+
+/**
+ * Integration tests for DLS_WRITE_BLOCKED setting which blocks write operations
+ * when users have DLS, FLS, or Field Masking restrictions.
+ */
+public class DlsWriteBlockedIntegrationTest {
+
+    private static final String DLS_INDEX = "dls_index";
+    private static final String FLS_INDEX = "fls_index";
+    private static final String NO_RESTRICTION_INDEX = "no_restriction_index";
+
+    static final User ADMIN_USER = new User("admin").roles(ALL_ACCESS);
+
+    static final User DLS_USER = new User("dls_user").roles(
+        new Role("dls_role").clusterPermissions("*").indexPermissions("*").dls("{\"term\": {\"dept\": \"sales\"}}").on(DLS_INDEX)
+    );
+
+    static final User FLS_USER = new User("fls_user").roles(
+        new Role("fls_role").clusterPermissions("*").indexPermissions("*").fls("public").on(FLS_INDEX)
+    );
+
+    @ClassRule
+    public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
+        .anonymousAuth(false)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(ADMIN_USER, DLS_USER, FLS_USER)
+        .build();
+
+    @BeforeClass
+    public static void createTestData() throws IOException {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            client.putJson(DLS_INDEX + "/_doc/1?refresh=true", "{\"dept\":\"sales\",\"amount\":100}");
+            client.putJson(FLS_INDEX + "/_doc/1?refresh=true", "{\"public\":\"data\",\"secret\":\"hidden\"}");
+            client.putJson(NO_RESTRICTION_INDEX + "/_doc/1?refresh=true", "{\"data\":\"value1\"}");
+        }
+    }
+
+    private void setDlsWriteBlocked(boolean enabled) throws IOException {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            client.putJson("_cluster/settings", String.format("{\"transient\":{\"plugins.security.dls.write_blocked\":%b}}", enabled));
+        }
+    }
+
+    @Test
+    public void testDlsUser_CanWrite_WhenSettingDisabled() throws IOException {
+        setDlsWriteBlocked(false);
+        try (TestRestClient client = cluster.getRestClient(DLS_USER)) {
+            var response = client.putJson(DLS_INDEX + "/_doc/test1?refresh=true", "{\"dept\":\"sales\",\"amount\":400}");
+
+            assertThat(response, isCreated());
+        }
+    }
+
+    @Test
+    public void testDlsUser_CannotWrite_WhenSettingEnabled() throws IOException {
+        setDlsWriteBlocked(true);
+        try (TestRestClient client = cluster.getRestClient(DLS_USER)) {
+            var response = client.putJson(DLS_INDEX + "/_doc/test2?refresh=true", "{\"dept\":\"sales\",\"amount\":400}");
+
+            assertThat(response.getStatusCode(), is(500));
+            assertThat(response.getBody(), containsString("is not supported when FLS or DLS or Fieldmasking is activated"));
+        }
+    }
+
+    @Test
+    public void testFlsUser_CanWrite_WhenSettingDisabled() throws IOException {
+        setDlsWriteBlocked(false);
+        try (TestRestClient client = cluster.getRestClient(FLS_USER)) {
+            var response = client.putJson(FLS_INDEX + "/_doc/test3?refresh=true", "{\"public\":\"new_data\",\"secret\":\"new_secret\"}");
+
+            assertThat(response.getStatusCode(), is(201));
+        }
+    }
+
+    @Test
+    public void testFlsUser_CannotWrite_WhenSettingEnabled() throws IOException {
+        setDlsWriteBlocked(true);
+        try (TestRestClient client = cluster.getRestClient(FLS_USER)) {
+            var response = client.putJson(FLS_INDEX + "/_doc/test4?refresh=true", "{\"public\":\"new_data\",\"secret\":\"new_secret\"}");
+
+            assertThat(response.getStatusCode(), is(500));
+            assertThat(response.getBody(), containsString("is not supported when FLS or DLS or Fieldmasking is activated"));
+        }
+    }
+
+    @Test
+    public void testAdminUser_CanWrite_WhenSettingEnabled() throws IOException {
+        setDlsWriteBlocked(true);
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            var response = client.putJson(DLS_INDEX + "/_doc/test6?refresh=true", "{\"dept\":\"admin\",\"amount\":999}");
+
+            assertThat(response.getStatusCode(), is(201));
+        }
+    }
+
+    @Test
+    public void testDlsUser_CanRead_WhenSettingEnabled() throws IOException {
+        setDlsWriteBlocked(true);
+        try (TestRestClient client = cluster.getRestClient(DLS_USER)) {
+            var response = client.get(DLS_INDEX + "/_search");
+
+            assertThat(response.getStatusCode(), is(200));
+        }
+    }
+}

src/integrationTest/java/org/opensearch/security/privileges/int_tests/ClusterConfig.java

@@ -12,9 +12,12 @@
 package org.opensearch.security.privileges.int_tests;
 
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import org.junit.rules.ExternalResource;
+
 import org.opensearch.test.framework.cluster.LocalCluster;
 
 /**
@@ -46,8 +49,6 @@ public enum ClusterConfig {
     final boolean systemIndexPrivilegeEnabled;
     final boolean allowsEmptyResultSets;
 
-    private LocalCluster cluster;
-
     ClusterConfig(
         String name,
         Function<LocalCluster.Builder, LocalCluster.Builder> clusterConfiguration,
@@ -62,25 +63,38 @@ public enum ClusterConfig {
         this.allowsEmptyResultSets = allowsEmptyResultSets;
     }
 
-    LocalCluster cluster(Supplier<LocalCluster.Builder> clusterBuilder) {
-        if (cluster == null) {
-            cluster = this.clusterConfiguration.apply(clusterBuilder.get()).build();
-            cluster.before();
-        }
-        return cluster;
+    @Override
+    public String toString() {
+        return name;
     }
 
-    void shutdown() {
-        if (cluster != null) {
-            try {
-                cluster.close();
-            } catch (Exception e) {}
-            cluster = null;
+    public static class ClusterInstances extends ExternalResource {
+        private final Supplier<LocalCluster.Builder> clusterBuilder;
+
+        public ClusterInstances(Supplier<LocalCluster.Builder> clusterBuilder) {
+            this.clusterBuilder = clusterBuilder;
         }
-    }
 
-    @Override
-    public String toString() {
-        return name;
+        private Map<ClusterConfig, LocalCluster> configToInstanceMap = new ConcurrentHashMap<>();
+
+        public LocalCluster get(ClusterConfig config) {
+            LocalCluster cluster = configToInstanceMap.get(config);
+            if (cluster == null) {
+                cluster = config.clusterConfiguration.apply(clusterBuilder.get()).build();
+                cluster.before();
+                configToInstanceMap.put(config, cluster);
+            }
+
+            return cluster;
+        }
+
+        @Override
+        protected void after() {
+            for (Map.Entry<ClusterConfig, LocalCluster> entry : configToInstanceMap.entrySet()) {
+                entry.getValue().stopSafe();
+            }
+            configToInstanceMap.clear();
+        };
+
     }
 }

src/integrationTest/java/org/opensearch/security/privileges/int_tests/CrossClusterAuthorizationIntTests.java

@@ -16,7 +16,6 @@
 import java.util.List;
 
 import com.google.common.collect.ImmutableList;
-import org.junit.AfterClass;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -189,12 +188,10 @@ static LocalCluster.Builder clusterBuilder() {
             .indices(LocalIndices.index_a1, LocalIndices.index_a2);
     }
 
-    @AfterClass
-    public static void stopClusters() {
-        for (ClusterConfig clusterConfig : ClusterConfig.values()) {
-            clusterConfig.shutdown();
-        }
-    }
+    @ClassRule
+    public static final ClusterConfig.ClusterInstances clusters = new ClusterConfig.ClusterInstances(
+        CrossClusterAuthorizationIntTests::clusterBuilder
+    );
 
     final TestSecurityConfig.User user;
     final LocalCluster cluster;
@@ -479,7 +476,7 @@ public static Collection<Object[]> params() {
     public CrossClusterAuthorizationIntTests(ClusterConfig clusterConfig, TestSecurityConfig.User user, String description)
         throws Exception {
         this.user = user;
-        this.cluster = clusterConfig.cluster(CrossClusterAuthorizationIntTests::clusterBuilder);
+        this.cluster = clusters.get(clusterConfig);
         this.clusterConfig = clusterConfig;
     }