-
Notifications
You must be signed in to change notification settings - Fork 343
Introduce new dynamic setting (plugins.security.dls.write_blocked) to block all writes when restrictions apply
#5828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1d2ea3a
d9369b6
b2ef519
59cf0c3
1dd9918
ec66b0a
a5d84da
63419cd
35a0f57
442b84a
4647d54
a745520
328199d
f2ca600
6ffa252
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| /* | ||
| * 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 com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; | ||
| import org.junit.BeforeClass; | ||
| import org.junit.ClassRule; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
|
|
||
| import org.opensearch.security.support.ConfigConstants; | ||
| 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; | ||
|
|
||
| /** | ||
| * Integration tests for DLS_WRITE_BLOCKED setting which blocks write operations | ||
| * when users have DLS, FLS, or Field Masking restrictions. | ||
| */ | ||
| @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) | ||
| @ThreadLeakScope(ThreadLeakScope.Scope.NONE) | ||
| 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\":{\"%s\":%b}}", ConfigConstants.SECURITY_DLS_WRITE_BLOCKED, 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.getStatusCode(), is(201)); | ||
| } | ||
| } | ||
|
|
||
| @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)); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,11 +85,14 @@ | |
| import org.opensearch.security.setting.OpensearchDynamicSetting; | ||
| import org.opensearch.security.support.ConfigConstants; | ||
| import org.opensearch.security.support.HeaderHelper; | ||
| import org.opensearch.security.support.SecuritySettings; | ||
| import org.opensearch.security.support.WildcardMatcher; | ||
| import org.opensearch.threadpool.ThreadPool; | ||
| import org.opensearch.transport.client.Client; | ||
|
|
||
| import static org.opensearch.security.privileges.PrivilegesEvaluatorImpl.isClusterPerm; | ||
| import static org.opensearch.security.support.ConfigConstants.SECURITY_DLS_WRITE_BLOCKED; | ||
| import static org.opensearch.security.support.ConfigConstants.SECURITY_DLS_WRITE_BLOCKED_ENABLED_DEFAULT; | ||
|
|
||
| public class DlsFlsValveImpl implements DlsFlsRequestValve { | ||
|
|
||
|
|
@@ -109,6 +112,7 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve { | |
| private final AdminDNs adminDNs; | ||
| private final OpensearchDynamicSetting<Boolean> resourceSharingEnabledSetting; | ||
| private final ResourcePluginInfo resourcePluginInfo; | ||
| private volatile boolean dlsWriteBlockedEnabled; | ||
|
|
||
| public DlsFlsValveImpl( | ||
| Settings settings, | ||
|
|
@@ -142,6 +146,12 @@ public DlsFlsValveImpl( | |
| config.updateClusterStateMetadataAsync(clusterService, threadPool); | ||
| } | ||
| }); | ||
| this.dlsWriteBlockedEnabled = settings.getAsBoolean(SECURITY_DLS_WRITE_BLOCKED, SECURITY_DLS_WRITE_BLOCKED_ENABLED_DEFAULT); | ||
| if (clusterService.getClusterSettings() != null) { | ||
| clusterService.getClusterSettings().addSettingsUpdateConsumer(SecuritySettings.DLS_WRITE_BLOCKED, newDlsWriteBlockedEnabled -> { | ||
| dlsWriteBlockedEnabled = newDlsWriteBlockedEnabled; | ||
| }); | ||
| } | ||
| this.resourceSharingEnabledSetting = resourceSharingEnabledSetting; | ||
| } | ||
|
|
||
|
|
@@ -333,11 +343,21 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< | |
|
|
||
| if (request instanceof BulkShardRequest) { | ||
| for (BulkItemRequest inner : ((BulkShardRequest) request).items()) { | ||
| if (inner.request() instanceof UpdateRequest) { | ||
| if (dlsWriteBlockedEnabled) { | ||
| listener.onFailure( | ||
| new OpenSearchSecurityException("Update is not supported when FLS or DLS or Fieldmasking is activated") | ||
| new OpenSearchSecurityException( | ||
| inner.request().getClass().getSimpleName() | ||
| + " is not supported when FLS or DLS or Fieldmasking is activated" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this message should change to "...when write block is active". thoughts?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that the message is bad, but I think it can be reworded differently. Maybe, |
||
| ) | ||
| ); | ||
| return false; | ||
| } else { | ||
| if (inner.request() instanceof UpdateRequest) { | ||
| listener.onFailure( | ||
| new OpenSearchSecurityException("Update is not supported when FLS or DLS or Fieldmasking is activated") | ||
| ); | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change it to adminClient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bc the test user has restrictions on the index. Its better to use admin client here which has no restrictions.
This block is for seeding test data for bwc tests and if we made the default for this new setting true then the calls here would fail since the test user has restrictions.