Skip to content

Commit

Permalink
Fix NPE getting metaFields from mapperService on a close index request (
Browse files Browse the repository at this point in the history
opensearch-project#4497)

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks authored Jun 30, 2024
1 parent 6dedfb4 commit 34da6e8
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.hamcrest.Matcher;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

Expand Down Expand Up @@ -59,6 +60,7 @@
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.log.LogsRule;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
Expand Down Expand Up @@ -305,6 +307,9 @@ public class FlsAndFieldMaskingTests {
)
.build();

@Rule
public LogsRule logsRule = new LogsRule("org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper");

/**
* Function that returns id assigned to song with title equal to given title or throws {@link RuntimeException}
* when no song matches.
Expand Down Expand Up @@ -1719,6 +1724,7 @@ private void assertProperSearchResponseForTitleFieldMaskingAndNoTitleFieldAndOnl
});
}

@Test
public void flsWithIncludesRulesIncludesFieldMappersFromPlugins() throws IOException {
String indexName = "fls_includes_index";
TestSecurityConfig.Role userRole = new TestSecurityConfig.Role("fls_include_stars_reader").clusterPermissions(
Expand All @@ -1742,4 +1748,34 @@ public void flsWithIncludesRulesIncludesFieldMappersFromPlugins() throws IOExcep
}
}

@Test
public void testFlsOnAClosedAndReopenedIndex() 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 (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
client.post(indexName + "/_close");
client.post(indexName + "/_open");
logsRule.assertThatContainExactly(indexName + " was closed. Setting metadataFields to empty. Closed index is not searchable.");
}

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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void assertThatContainExactly(String expectedLogMessage) {
*/
public void assertThatContain(String messageFragment) {
List<String> messages = LogCapturingAppender.getLogMessagesAsString();
;

String reason = reasonMessage(messageFragment, messages);
assertThat(reason, messages, hasItem(containsString(messageFragment)));
}
Expand Down
6 changes: 4 additions & 2 deletions src/integrationTest/resources/log4j2-test.properties
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ logger.auditlogs.level = info
# Logger required by test org.opensearch.security.http.JwtAuthenticationTests
logger.httpjwtauthenticator.name = com.amazon.dlic.auth.http.jwt.HTTPJwtAuthenticator
logger.httpjwtauthenticator.level = debug
logger.backendreg.additivity = false
logger.httpjwtauthenticator.appenderRef.capturing.ref = logCapturingAppender

logger.securityflsdlsindexsearcherwrapper.name = org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper
logger.securityflsdlsindexsearcherwrapper.level = debug
logger.securityflsdlsindexsearcherwrapper.appenderRef.capturing.ref = logCapturingAppender

#Required by tests:
# org.opensearch.security.IpBruteForceAttacksPreventionTests
# org.opensearch.security.UserBruteForceAttacksPreventionTests
Expand All @@ -43,5 +46,4 @@ logger.backendreg.appenderRef.capturing.ref = logCapturingAppender
#logger.ldap.name=com.amazon.dlic.auth.ldap.backend.LDAPAuthenticationBackend
logger.ldap.name=com.amazon.dlic.auth.ldap.backend
logger.ldap.level=TRACE
logger.backendreg.additivity = false
logger.ldap.appenderRef.capturing.ref = logCapturingAppender
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
import java.util.function.LongSupplier;

import com.google.common.collect.Sets;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;

import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.index.shard.ShardId;
Expand All @@ -40,6 +43,8 @@

public class SecurityFlsDlsIndexSearcherWrapper extends SecurityIndexSearcherWrapper {

public final Logger log = LogManager.getLogger(this.getClass());

private final Set<String> metaFields;
public static final Set<String> META_FIELDS_BEFORE_7DOT8 = Collections.unmodifiableSet(
new HashSet<>(Arrays.asList("_timestamp", "_ttl", "_type"))
Expand All @@ -62,10 +67,21 @@ 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);
Set<String> metadataFieldsCopy;
if (indexService.getMetadata().getState() == IndexMetadata.State.CLOSE) {
if (log.isDebugEnabled()) {
log.debug(
"{} was closed. Setting metadataFields to empty. Closed index is not searchable.",
indexService.index().getName()
);
}
metadataFieldsCopy = Collections.emptySet();
} else {
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;
Expand Down

0 comments on commit 34da6e8

Please sign in to comment.