Skip to content

Commit

Permalink
Revert "[Backport 2.x] Remove static metaFields list and get version-…
Browse files Browse the repository at this point in the history
…specific values from core" (#4474)

Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura authored Jun 18, 2024
1 parent cdc792c commit 9674301
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 124 deletions.
1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,6 @@ dependencies {
exclude(group: 'org.hamcrest', module: 'hamcrest')
}
integrationTestImplementation 'com.unboundid:unboundid-ldapsdk:4.0.14'
integrationTestImplementation "org.opensearch.plugin:mapper-size:${opensearch_version}"
integrationTestImplementation "org.apache.httpcomponents:httpclient-cache:4.5.14"
integrationTestImplementation "org.apache.httpcomponents:httpclient:4.5.14"
integrationTestImplementation "org.apache.httpcomponents:fluent-hc:4.5.14"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.15.0
* Refactor ActionGroup REST API test and partial fix #4166 ([#4371](https://github.com/opensearch-project/security/pull/4371))
* Support multiple audience for jwt authentication ([#4363](https://github.com/opensearch-project/security/pull/4363))
* Configure masking algorithm default ([#4345](https://github.com/opensearch-project/security/pull/4345))
* Remove static metaFields list and get version-specific values from core ([#4412](https://github.com/opensearch-project/security/pull/4412))

### Bug Fixes
* Add cat/alias support for DNFOF ([#4440](https://github.com/opensearch-project/security/pull/4440))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.junit.runner.RunWith;

import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.action.fieldcaps.FieldCapabilitiesRequest;
import org.opensearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.opensearch.action.get.GetRequest;
Expand All @@ -44,15 +43,10 @@
import org.opensearch.action.search.SearchScrollRequest;
import org.opensearch.client.Client;
import org.opensearch.client.RestHighLevelClient;
import org.opensearch.index.mapper.SourceFieldMapper;
import org.opensearch.index.mapper.size.SizeFieldMapper;
import org.opensearch.index.query.MatchAllQueryBuilder;
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.plugin.mapper.MapperSizePlugin;
import org.opensearch.search.aggregations.Aggregation;
import org.opensearch.search.aggregations.metrics.ParsedAvg;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
Expand All @@ -61,7 +55,6 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
Expand Down Expand Up @@ -101,7 +94,6 @@
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.searchHitDoesContainField;
import static org.opensearch.test.framework.matcher.SearchResponseMatchers.searchHitDoesNotContainField;
import static org.opensearch.test.framework.matcher.SearchResponseMatchers.searchHitsContainDocumentWithId;

Expand Down Expand Up @@ -220,7 +212,6 @@ public class FlsAndFieldMaskingTests {
.nodeSettings(
Map.of("plugins.security.restapi.roles_enabled", List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()))
)
.plugin(MapperSizePlugin.class)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(
ADMIN_USER,
Expand Down Expand Up @@ -435,10 +426,6 @@ public void flsEnabledFieldsAreHiddenForNormalUsers() throws IOException {

private static List<String> createIndexWithDocs(String indexName, Song... songs) {
try (Client client = cluster.getInternalNodeClient()) {
client.admin()
.indices()
.create(new CreateIndexRequest(indexName).mapping(Map.of("_size", Map.of("enabled", true))))
.actionGet();
return Stream.of(songs).map(song -> {
IndexResponse response = client.index(new IndexRequest(indexName).setRefreshPolicy(IMMEDIATE).source(song.asMap()))
.actionGet();
Expand Down Expand Up @@ -482,14 +469,6 @@ private static void assertSearchHitsDoNotContainField(SearchResponse response, S
.forEach(index -> assertThat(response, searchHitDoesNotContainField(index, excludedField)));
}

private static void assertSearchHitsDoContainField(SearchResponse response, String includedField) {
assertThat(response, isSuccessfulSearchResponse());
assertThat(response.getHits().getHits().length, greaterThan(0));
IntStream.range(0, response.getHits().getHits().length)
.boxed()
.forEach(index -> assertThat(response, searchHitDoesContainField(index, includedField)));
}

@Test
public void searchForDocuments() throws IOException {
// FIELD MASKING
Expand Down Expand Up @@ -832,28 +811,4 @@ public void getFieldCapabilities() throws IOException {
}
}

@Test
public void flsWithIncludesRulesIncludesFieldMappersFromPlugins() throws IOException {
String indexName = "fls_includes_index";
TestSecurityConfig.Role userRole = new TestSecurityConfig.Role("fls_include_stars_reader").clusterPermissions(
"cluster_composite_ops_ro"
).indexPermissions("read").fls(FIELD_STARS).on("*");
TestSecurityConfig.User user = createUserWithRole("fls_includes_user", userRole);
List<String> docIds = createIndexWithDocs(indexName, SONGS[0], SONGS[1]);

try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(user)) {
SearchRequest searchRequest = new SearchRequest(indexName);
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
MatchAllQueryBuilder matchAllQueryBuilder = QueryBuilders.matchAllQuery();
searchSourceBuilder.storedFields(List.of(SizeFieldMapper.NAME, SourceFieldMapper.NAME));
searchSourceBuilder.query(matchAllQueryBuilder);
searchRequest.source(searchSourceBuilder);
SearchResponse searchResponse = restHighLevelClient.search(searchRequest, DEFAULT);

assertSearchHitsDoContainField(searchResponse, FIELD_STARS);
assertThat(searchResponse.toString(), containsString(SizeFieldMapper.NAME));
assertSearchHitsDoNotContainField(searchResponse, FIELD_ARTIST);
}
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ public static Matcher<SearchResponse> searchHitDoesNotContainField(int hitIndex,
return new SearchHitDoesNotContainFieldMatcher(hitIndex, fieldName);
}

public static Matcher<SearchResponse> searchHitDoesContainField(int hitIndex, String fieldName) {
return new SearchHitDoesContainFieldMatcher(hitIndex, fieldName);
}

public static Matcher<SearchResponse> searchHitsContainDocumentWithId(int hitIndex, String indexName, String documentId) {
return new SearchHitsContainDocumentWithIdMatcher(hitIndex, indexName, documentId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
package org.opensearch.security.configuration;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
Expand All @@ -28,7 +26,7 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.core.index.shard.ShardId;
import org.opensearch.index.IndexService;
import org.opensearch.index.mapper.SeqNoFieldMapper;
import org.opensearch.index.mapper.IgnoredFieldMapper;
import org.opensearch.index.query.QueryShardContext;
import org.opensearch.index.shard.ShardUtils;
import org.opensearch.security.auditlog.AuditLog;
Expand All @@ -40,9 +38,24 @@

public class SecurityFlsDlsIndexSearcherWrapper extends SecurityIndexSearcherWrapper {

private final Set<String> metaFields;
public static final Set<String> META_FIELDS_BEFORE_7DOT8 = Collections.unmodifiableSet(
new HashSet<>(Arrays.asList("_timestamp", "_ttl", "_type"))
// TODO: the list is outdated. It is necessary to change how meta fields are handled in the near future.
// We may consider using MapperService.isMetadataField() instead of relying on the static set or
// (if it is too costly or does not meet requirements) use IndicesModule.getBuiltInMetadataFields()
// for OpenSearch version specific Set of meta fields
private static final Set<String> metaFields = Sets.newHashSet(
"_source",
"_version",
"_field_names",
"_seq_no",
"_primary_term",
"_id",
IgnoredFieldMapper.NAME,
"_index",
"_routing",
"_size",
"_timestamp",
"_ttl",
"_type"
);
private final ClusterService clusterService;
private final IndexService indexService;
Expand All @@ -62,11 +75,6 @@ public SecurityFlsDlsIndexSearcherWrapper(
final Salt salt
) {
super(indexService, settings, adminDNs, evaluator);
Set<String> metadataFieldsCopy = new HashSet<>(indexService.mapperService().getMetadataFields());
SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID();
metadataFieldsCopy.add(sequenceIDFields.primaryTerm.name());
metadataFieldsCopy.addAll(META_FIELDS_BEFORE_7DOT8);
metaFields = metadataFieldsCopy;
ciol.setIs(indexService);
this.clusterService = clusterService;
this.indexService = indexService;
Expand Down

0 comments on commit 9674301

Please sign in to comment.