From c06365ca94c9a1d15e85b578a1ae48168bf0bca7 Mon Sep 17 00:00:00 2001 From: Prabhas Kurapati <66924475+prabhask5@users.noreply.github.com> Date: Mon, 29 Jan 2024 06:54:56 -0800 Subject: [PATCH] [BUG-2556] Add new DLS filtering test (#3908) Signed-off-by: Prabhas Kurapati --- .../security/DlsIntegrationTests.java | 205 +++++++++++++++++- 1 file changed, 204 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/DlsIntegrationTests.java b/src/integrationTest/java/org/opensearch/security/DlsIntegrationTests.java index aa7202cddf..3e3ac61502 100644 --- a/src/integrationTest/java/org/opensearch/security/DlsIntegrationTests.java +++ b/src/integrationTest/java/org/opensearch/security/DlsIntegrationTests.java @@ -10,12 +10,15 @@ package org.opensearch.security; import java.io.IOException; +import java.io.Serializable; import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.function.BiFunction; +import java.util.stream.Collectors; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.apache.commons.lang3.tuple.Pair; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -36,6 +39,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions.Type.ADD; import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.client.RequestOptions.DEFAULT; @@ -57,6 +61,7 @@ import static org.opensearch.test.framework.matcher.SearchResponseMatchers.isSuccessfulSearchResponse; import static org.opensearch.test.framework.matcher.SearchResponseMatchers.numberOfTotalHitsIsEqualTo; import static org.opensearch.test.framework.matcher.SearchResponseMatchers.searchHitContainsFieldWithValue; +import static org.opensearch.test.framework.matcher.SearchResponseMatchers.searchHitsContainDocumentsInAnyOrder; @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) @@ -82,6 +87,7 @@ public class DlsIntegrationTests { static final String FIRST_INDEX_ALIAS_FILTERED_BY_TWINS_ARTIST = FIRST_INDEX_NAME.concat("-filtered-by-twins-artist"); static final String FIRST_INDEX_ALIAS_FILTERED_BY_FIRST_ARTIST = FIRST_INDEX_NAME.concat("-filtered-by-first-artist"); static final String ALL_INDICES_ALIAS = "_all"; + static final String UNION_TEST_INDEX_NAME = "my_index1"; static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); @@ -158,6 +164,62 @@ public class DlsIntegrationTests { .on("*") ); + /** + * Test role 1 for DLS filtering with two (non)overlapping roles. This role imposes a filter where the user can only access documents where the sensitive field is false. This role is applied at a higher level for all index patterns. + */ + static final TestSecurityConfig.Role ROLE_NON_SENSITIVE_ONLY = new TestSecurityConfig.Role("test_role_1").clusterPermissions( + "cluster_composite_ops_ro" + ).indexPermissions("read").dls("{\"match\":{\"sensitive\":false}}").on("*"); + + /** + * Test role 2 for DLS filtering with two overlapping roles. This role does not impose any filter, and combined with TEST_ROLE_ONE should yield a union that does not impose any filter. This role is applied at a lower level for index patterns my_index*. + */ + static final TestSecurityConfig.Role ROLE_ALLOW_ALL = new TestSecurityConfig.Role("test_role_2").clusterPermissions( + "cluster_composite_ops_ro" + ).indexPermissions("read").dls("{\"match_all\": {}}").on("my_index*"); + + /** + * Test role 3 for DLS filtering with two nonoverlapping roles. This role imposes a filter where the user can only access documents where the genre field is History, and combined with TEST_ROLE_ONE should yield a union that allows the user to access every document except the one with genre Science and sensitive true. This role is applied at a lower level for index patterns my_index*. + */ + static final TestSecurityConfig.Role ROLE_MATCH_HISTORY_GENRE_ONLY = new TestSecurityConfig.Role("test_role_3").clusterPermissions( + "cluster_composite_ops_ro" + ).indexPermissions("read").dls("{\"match\":{\"genre\":\"History\"}}").on("my_index*"); + + /** + * User with DLS permission to only be able to access documents with false sensitive property. + */ + static final TestSecurityConfig.User USER_NON_SENSITIVE_ONLY = new TestSecurityConfig.User("test_role_1_user").roles( + ROLE_NON_SENSITIVE_ONLY + ); + + /** + * User with DLS permission to access all documents. + */ + static final TestSecurityConfig.User USER_ALLOW_ALL = new TestSecurityConfig.User("test_role_2_user").roles(ROLE_ALLOW_ALL); + + /** + * User with DLS permission to access documents with genre property matching History. + */ + static final TestSecurityConfig.User USER_MATCH_HISTORY_GENRE_ONLY = new TestSecurityConfig.User("test_role_3_user").roles( + ROLE_MATCH_HISTORY_GENRE_ONLY + ); + + /** + * User with overlapping DLS permissions to access documents with false sensitive property and access all documents- should yield accessing all documents. + */ + static final TestSecurityConfig.User USER_UNION_OF_OVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_ALLOW_ALL = new TestSecurityConfig.User( + "test_union_of_overlapping_roles_user" + ).roles(ROLE_NON_SENSITIVE_ONLY, ROLE_ALLOW_ALL); + + /** + * User with non-overlapping DLS permissions to access documents with false sensitive property and genre property matching History. + */ + static final TestSecurityConfig.User USER_UNION_OF_NONOVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_HISTORY_GENRE_ONLY = + new TestSecurityConfig.User("test_union_of_non_overlapping_roles_user").roles( + ROLE_NON_SENSITIVE_ONLY, + ROLE_MATCH_HISTORY_GENRE_ONLY + ); + @ClassRule public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) .anonymousAuth(false) @@ -172,7 +234,12 @@ public class DlsIntegrationTests { READ_WHERE_FIELD_ARTIST_MATCHES_ARTIST_STRING, READ_WHERE_STARS_LESS_THAN_THREE, READ_WHERE_FIELD_ARTIST_MATCHES_ARTIST_TWINS_OR_FIELD_STARS_GREATER_THAN_FIVE, - READ_WHERE_FIELD_ARTIST_MATCHES_ARTIST_TWINS_OR_MATCHES_ARTIST_FIRST + READ_WHERE_FIELD_ARTIST_MATCHES_ARTIST_TWINS_OR_MATCHES_ARTIST_FIRST, + USER_NON_SENSITIVE_ONLY, + USER_ALLOW_ALL, + USER_MATCH_HISTORY_GENRE_ONLY, + USER_UNION_OF_OVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_ALLOW_ALL, + USER_UNION_OF_NONOVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_HISTORY_GENRE_ONLY ) .build(); @@ -218,6 +285,21 @@ public class DlsIntegrationTests { } }; + static final TreeMap> UNION_ROLE_TEST_DATA = new TreeMap<>() { + { + put("1", Map.of("genre", "History", "date", "01-01-2020", "sensitive", true)); + put("2", Map.of("genre", "History", "date", "01-01-2020", "sensitive", true)); + put("3", Map.of("genre", "History", "date", "01-01-2020", "sensitive", true)); + put("4", Map.of("genre", "History", "date", "01-01-2020", "sensitive", true)); + put("5", Map.of("genre", "History", "date", "01-01-2020", "sensitive", true)); + put("6", Map.of("genre", "Math", "date", "01-01-2020", "sensitive", false)); + put("7", Map.of("genre", "Math", "date", "01-01-2020", "sensitive", false)); + put("8", Map.of("genre", "Math", "date", "01-01-2020", "sensitive", false)); + put("9", Map.of("genre", "Math", "date", "01-01-2020", "sensitive", false)); + put("10", Map.of("genre", "Science", "date", "01-01-2020", "sensitive", true)); + } + }; + @BeforeClass public static void createTestData() { try (Client client = cluster.getInternalNodeClient()) { @@ -275,6 +357,10 @@ public static void createTestData() { ) ) .actionGet(); + + UNION_ROLE_TEST_DATA.forEach((index, document) -> { + client.prepareIndex(UNION_TEST_INDEX_NAME).setId(index).setRefreshPolicy(IMMEDIATE).setSource(document).get(); + }); } } @@ -517,4 +603,121 @@ public void testAggregateAndComputeStarRatings() throws IOException { assertThat(((ParsedAvg) actualAggregation).getValue(), is(1.5)); } } + + @Test + public void testOverlappingRoleUnionSearchFiltering() throws Exception { + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(USER_NON_SENSITIVE_ONLY)) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 4); + + assertThat( + searchResponse, + searchHitsContainDocumentsInAnyOrder( + UNION_ROLE_TEST_DATA.entrySet() + .stream() + .filter(e -> e.getValue().get("sensitive").equals(false)) + .map(e -> Pair.of(UNION_TEST_INDEX_NAME, e.getKey())) + .collect(Collectors.toList()) + ) + ); + } + + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(USER_ALLOW_ALL)) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 10); + } + + try ( + RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient( + USER_UNION_OF_OVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_ALLOW_ALL + ) + ) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 10); + + // shows that roles are additive and the overlapping role with less filtering is used + assertThat( + searchResponse, + searchHitsContainDocumentsInAnyOrder( + UNION_ROLE_TEST_DATA.keySet().stream().map(id -> Pair.of(UNION_TEST_INDEX_NAME, id)).collect(Collectors.toList()) + ) + ); + } + } + + @Test + @SuppressWarnings("unchecked") + public void testNonOverlappingRoleUnionSearchFiltering() throws Exception { + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(USER_NON_SENSITIVE_ONLY)) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 4); + + assertThat( + searchResponse, + searchHitsContainDocumentsInAnyOrder( + UNION_ROLE_TEST_DATA.entrySet() + .stream() + .filter(e -> e.getValue().get("sensitive").equals(false)) + .map(e -> Pair.of(UNION_TEST_INDEX_NAME, e.getKey())) + .collect(Collectors.toList()) + ) + ); + } + + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(USER_MATCH_HISTORY_GENRE_ONLY)) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 5); + + assertThat( + searchResponse, + searchHitsContainDocumentsInAnyOrder( + UNION_ROLE_TEST_DATA.entrySet() + .stream() + .filter(e -> e.getValue().get("genre").equals("History")) + .map(e -> Pair.of(UNION_TEST_INDEX_NAME, e.getKey())) + .collect(Collectors.toList()) + ) + ); + } + + try ( + RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient( + USER_UNION_OF_NONOVERLAPPING_ROLES_NON_SENSITIVE_ONLY_AND_HISTORY_GENRE_ONLY + ) + ) { + SearchRequest searchRequest = new SearchRequest(UNION_TEST_INDEX_NAME); + SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT); + + assertSearchResponseHitsEqualTo(searchResponse, 9); + + assertThat( + searchResponse, + searchHitsContainDocumentsInAnyOrder( + UNION_ROLE_TEST_DATA.keySet() + .stream() + .filter(id -> !id.equals("10")) + .map(id -> Pair.of(UNION_TEST_INDEX_NAME, id)) + .collect(Collectors.toList()) + ) + ); + + // shows that the roles are additive, but excludes one document since the DLS filters for both roles do not account for this + assertThat(searchResponse, not(searchHitsContainDocumentsInAnyOrder(Pair.of(UNION_TEST_INDEX_NAME, "10")))); + } + } + + private void assertSearchResponseHitsEqualTo(SearchResponse searchResponse, int hits) throws Exception { + assertThat(searchResponse, isSuccessfulSearchResponse()); + assertThat(searchResponse, numberOfTotalHitsIsEqualTo(hits)); + } }