diff --git a/src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java b/src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java index 57e5714478..9692dd365d 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java @@ -517,8 +517,14 @@ private boolean modifyQuery(String localClusterAlias) throws IOException { for (QueryBuilder queryBuilder : queryBuilders) { TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder; + final var lookupIndex = termsQueryBuilder.termsLookup().index(); + final var lookupId = termsQueryBuilder.termsLookup().id(); - documentAllowlist.add(termsQueryBuilder.termsLookup().index(), termsQueryBuilder.termsLookup().id()); + if (lookupId != null) { + documentAllowlist.add(lookupIndex, lookupId); + } else { + documentAllowlist.add(lookupIndex, DocumentAllowList.ANY_DOCUMENT_ID); + } } } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 9b3edaefc3..1f64179e9d 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -211,7 +211,7 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< && resolvedIndices.local() .namesOfIndices(context.clusterState()) .stream() - .anyMatch(index -> documentAllowList.isAllowed(index, "*"))) { + .anyMatch(index -> documentAllowList.isAllowed(index, DocumentAllowList.ANY_DOCUMENT_ID))) { // The documentAllowList is needed here for Dashboards multi tenancy which can redirect index accesses to indices for which no // normal index privileges are present // If we would not use the documentAllowList here, the index would appear to be protected @@ -481,7 +481,7 @@ public void handleSearchContext(SearchContext searchContext, ThreadPool threadPo // - DLS rules which use "term lookup queries" and thus need to access indices for which no privileges are present // - Dashboards multi tenancy which can redirect index accesses to indices for which no normal index privileges are present - if (!dlsRestriction.isUnrestricted() && documentAllowList.isAllowed(index, "*")) { + if (!dlsRestriction.isUnrestricted() && documentAllowList.isAllowed(index, DocumentAllowList.ANY_DOCUMENT_ID)) { dlsRestriction = DlsRestriction.NONE; log.debug("Lifting DLS for {} due to present document allowlist", index); } diff --git a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java index 96c1616183..3f4b0f4838 100644 --- a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java +++ b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java @@ -163,7 +163,7 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm // - DLS rules which use "term lookup queries" and thus need to access indices for which no privileges are present // - Dashboards multi tenancy which can redirect index accesses to indices for which no normal index privileges are present - if (!dlsRestriction.isUnrestricted() && documentAllowList.isAllowed(index.getName(), "*")) { + if (!dlsRestriction.isUnrestricted() && documentAllowList.isAllowed(index.getName(), DocumentAllowList.ANY_DOCUMENT_ID)) { dlsRestriction = DlsRestriction.NONE; log.debug("Lifting DLS for {} due to present document allowlist", index.getName()); dlsQuery = null; diff --git a/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java b/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java index f1186fa102..1e08bef11f 100644 --- a/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java +++ b/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java @@ -18,7 +18,9 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.ActionRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; import org.opensearch.action.get.GetRequest; +import org.opensearch.action.search.SearchRequest; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.support.ConfigConstants; @@ -32,6 +34,8 @@ public class DocumentAllowList { private static final Logger log = LogManager.getLogger(DocumentAllowList.class); + public static final String ANY_DOCUMENT_ID = "*"; + public static DocumentAllowList get(ThreadContext threadContext) { String header = threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER); @@ -48,34 +52,51 @@ public static DocumentAllowList get(ThreadContext threadContext) { } public static boolean isAllowed(ActionRequest request, ThreadContext threadContext) { - String docAllowListHeader = threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER); + final var documentAllowList = DocumentAllowList.get(threadContext); - if (docAllowListHeader == null) { + if (documentAllowList.isEmpty()) { return false; } - if (!(request instanceof GetRequest)) { + // GetRequest: id-based TLQ resolves via GET; match exact (index, id) entry. + // SearchRequest: query-based TLQ resolves via SEARCH; match wildcard entry. + // GetSettingsRequest: query-based TLQ retrieves the index setting (during the fetch phase). + // Other request types (including writes) are never allowlisted. + if (request instanceof GetRequest getRequest) { + if (documentAllowList.isAllowed(getRequest.index(), getRequest.id())) { + log.debug("Request {} is allowed by {}", request, documentAllowList); + return true; + } + return false; + } else if (request instanceof SearchRequest searchRequest) { + if (isIndicesAllowlisted(documentAllowList, searchRequest.indices())) { + log.debug("Request {} is allowed by {}", request, documentAllowList); + return true; + } + return false; + } else if (request instanceof GetSettingsRequest getSettingsRequest) { + if (isIndicesAllowlisted(documentAllowList, getSettingsRequest.indices())) { + log.debug("Request {} is allowed by {}", request, documentAllowList); + return true; + } return false; } - try { - DocumentAllowList documentAllowList = DocumentAllowList.parse(docAllowListHeader); - GetRequest getRequest = (GetRequest) request; - - if (documentAllowList.isAllowed(getRequest.index(), getRequest.id())) { - if (log.isDebugEnabled()) { - log.debug("Request " + request + " is allowed by " + documentAllowList); - } + return false; + } - return true; - } else { + // allMatch semantics: at privilege-evaluation time, every index in the request must be + // covered. DlsFlsValveImpl uses anyMatch+size()==1 for a different purpose (per-shard bypass). + private static boolean isIndicesAllowlisted(DocumentAllowList documentAllowList, String[] indices) { + if (indices == null || indices.length == 0) { + return false; + } + for (String index : indices) { + if (index == null || !documentAllowList.isAllowed(index, ANY_DOCUMENT_ID)) { return false; } - - } catch (Exception e) { - log.error("Error while handling document allow list: " + docAllowListHeader, e); - return false; } + return true; } private static final DocumentAllowList EMPTY = new DocumentAllowList(); diff --git a/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesEvaluatorImpl.java b/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesEvaluatorImpl.java index 07ba29d4aa..f6004924a7 100644 --- a/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesEvaluatorImpl.java +++ b/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesEvaluatorImpl.java @@ -62,7 +62,6 @@ import org.opensearch.action.bulk.BulkRequest; import org.opensearch.action.bulk.BulkShardRequest; import org.opensearch.action.delete.DeleteAction; -import org.opensearch.action.get.GetRequest; import org.opensearch.action.get.MultiGetAction; import org.opensearch.action.index.IndexAction; import org.opensearch.action.search.MultiSearchAction; @@ -699,31 +698,7 @@ public Iterator iterator() { } private boolean checkDocAllowListHeader(User user, String action, ActionRequest request) { - String docAllowListHeader = threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER); - - if (docAllowListHeader == null) { - return false; - } - - if (!(request instanceof GetRequest)) { - return false; - } - - try { - DocumentAllowList documentAllowList = DocumentAllowList.parse(docAllowListHeader); - GetRequest getRequest = (GetRequest) request; - - if (documentAllowList.isAllowed(getRequest.index(), getRequest.id())) { - log.debug("Request {} is allowed by {}", request, documentAllowList); - return true; - } else { - return false; - } - - } catch (Exception e) { - log.error("Error while handling document allow list: {}", docAllowListHeader, e); - return false; - } + return DocumentAllowList.isAllowed(request, threadContext); } private List toString(List aliases) { diff --git a/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesInterceptor.java b/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesInterceptor.java index aee71e0fa0..acd852dd8f 100644 --- a/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesInterceptor.java +++ b/src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesInterceptor.java @@ -258,12 +258,12 @@ public ReplaceResult replaceDashboardsIndex( private void applyDocumentAllowList(String indexName) { DocumentAllowList documentAllowList = new DocumentAllowList(); - documentAllowList.add(indexName, "*"); + documentAllowList.add(indexName, DocumentAllowList.ANY_DOCUMENT_ID); IndexAbstraction indexAbstraction = clusterStateSupplier.get().getMetadata().getIndicesLookup().get(indexName); if (indexAbstraction instanceof IndexAbstraction.Alias) { for (IndexMetadata index : ((IndexAbstraction.Alias) indexAbstraction).getIndices()) { - documentAllowList.add(index.getIndex().getName(), "*"); + documentAllowList.add(index.getIndex().getName(), DocumentAllowList.ANY_DOCUMENT_ID); } } diff --git a/src/main/java/org/opensearch/security/privileges/actionlevel/nextgen/DashboardsMultitenancySystemIndexHandler.java b/src/main/java/org/opensearch/security/privileges/actionlevel/nextgen/DashboardsMultitenancySystemIndexHandler.java index dc15c6a242..607d7640d3 100644 --- a/src/main/java/org/opensearch/security/privileges/actionlevel/nextgen/DashboardsMultitenancySystemIndexHandler.java +++ b/src/main/java/org/opensearch/security/privileges/actionlevel/nextgen/DashboardsMultitenancySystemIndexHandler.java @@ -224,12 +224,12 @@ private boolean hasPermission(PrivilegesEvaluationContext context) { private void applyDocumentAllowList(String indexName) { DocumentAllowList documentAllowList = new DocumentAllowList(); - documentAllowList.add(indexName, "*"); + documentAllowList.add(indexName, DocumentAllowList.ANY_DOCUMENT_ID); IndexAbstraction indexAbstraction = this.clusterStateSupplier.get().getMetadata().getIndicesLookup().get(indexName); if (indexAbstraction instanceof IndexAbstraction.Alias) { for (IndexMetadata index : indexAbstraction.getIndices()) { - documentAllowList.add(index.getIndex().getName(), "*"); + documentAllowList.add(index.getIndex().getName(), DocumentAllowList.ANY_DOCUMENT_ID); } } diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java index 5bb09e00e5..37d63122a0 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java @@ -56,6 +56,21 @@ public class DlsTermLookupQueryTest extends AbstractDlsFlsTest { + protected String getSecurityConfigName() { + return "securityconfig_tlq.yml"; + } + + protected DynamicSecurityConfig tlqSecurityConfig() { + return new DynamicSecurityConfig().setConfig(getSecurityConfigName()) + .setSecurityInternalUsers("internal_users_tlq.yml") + .setSecurityRoles("roles_tlq.yml") + .setSecurityRolesMapping("roles_mapping_tlq.yml"); + } + + protected void setupWithTlqSecurityConfig() throws Exception { + setup(tlqSecurityConfig()); + } + protected void populateData(Client client) { // user access codes, basis for TLQ query client.index( @@ -88,6 +103,11 @@ protected void populateData(Client client) { .setRefreshPolicy(RefreshPolicy.IMMEDIATE) .source("{ \"bla\": \"blub\" }", XContentType.JSON) ).actionGet(); + client.index( + new IndexRequest("user_access_codes").id("tlq_query_1337") + .setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source("{ \"access_codes\": [1337] }", XContentType.JSON) + ).actionGet(); // need to have keyword for bu field since we're testing aggregations client.admin().indices().create(new CreateIndexRequest("tlqdocuments")).actionGet(); @@ -225,12 +245,7 @@ protected void populateData(Client client) { @Test public void testSimpleSearch_AccessCode_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); HttpResponse response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_1337", "password")); assertThat(response.getStatusCode(), is(200)); @@ -246,12 +261,7 @@ public void testSimpleSearch_AccessCode_1337() throws Exception { @Test public void testSimpleSearch_AccessCode_42() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); HttpResponse response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_42", "password")); assertThat(response.getStatusCode(), is(200)); @@ -269,12 +279,7 @@ public void testSimpleSearch_AccessCode_42() throws Exception { @Test public void testSimpleSearch_AccessCodes_1337_42() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); HttpResponse response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_1337_42", "password")); assertThat(response.getStatusCode(), is(200)); @@ -292,12 +297,7 @@ public void testSimpleSearch_AccessCodes_1337_42() throws Exception { @Test public void testSimpleSearch_AccessCodes_999() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); HttpResponse response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_999", "password")); assertThat(response.getStatusCode(), is(200)); @@ -311,12 +311,7 @@ public void testSimpleSearch_AccessCodes_999() throws Exception { @Test public void testSimpleSearch_AccessCodes_emptyAccessCodes() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("tlqdocuments", "tlq_empty_access_codes", "password"); assertThat(searchResponse.toString(), searchResponse.getHits().getTotalHits().value(), is(0L)); } @@ -324,12 +319,7 @@ public void testSimpleSearch_AccessCodes_emptyAccessCodes() throws Exception { @Test public void testSimpleSearch_AccessCodes_noAccessCodes() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("tlqdocuments", "tlq_no_codes", "password"); assertThat(searchResponse.toString(), searchResponse.getHits().getTotalHits().value(), is(0L)); @@ -337,12 +327,7 @@ public void testSimpleSearch_AccessCodes_noAccessCodes() throws Exception { @Test public void testSimpleSearch_AllIndices_All_AccessCodes_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("_all", "tlq_1337", "password"); @@ -378,12 +363,7 @@ public void testSimpleSearch_AllIndices_All_AccessCodes_1337() throws Exception @Test public void testSimpleSearch_AllIndicesWildcard_AccessCodes_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("*", "tlq_1337", "password"); @@ -419,12 +399,7 @@ public void testSimpleSearch_AllIndicesWildcard_AccessCodes_1337() throws Except @Test public void testSimpleSearch_ThreeIndicesWildcard_AccessCodes_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("tlq*,user*", "tlq_1337", "password"); @@ -461,12 +436,7 @@ public void testSimpleSearch_ThreeIndicesWildcard_AccessCodes_1337() throws Exce @Test public void testSimpleSearch_TwoIndicesConcreteNames_AccessCodes_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); SearchResponse searchResponse = executeSearch("tlqdocuments,tlqdummy", "tlq_1337", "password"); @@ -494,12 +464,7 @@ public void testSimpleSearch_TwoIndicesConcreteNames_AccessCodes_1337() throws E @Test public void testMSearch_ThreeIndices_AccessCodes_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); MultiSearchResponse searchResponse = executeMSearchMatchAll( "tlq_1337", @@ -536,12 +501,7 @@ public void testMSearch_ThreeIndices_AccessCodes_1337() throws Exception { @Test public void testGet_TlqDocumentsIndex_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); // user has 1337, document has 1337 GetResponse searchResponse = executeGet("tlqdocuments", "1", "tlq_1337", "password"); @@ -576,12 +536,7 @@ public void testGet_TlqDocumentsIndex_1337() throws Exception { @Test public void testGet_TlqDocumentsIndex_1337_42() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); // user has 1337 and 42, document has 1337 GetResponse searchResponse = executeGet("tlqdocuments", "1", "tlq_1337_42", "password"); @@ -618,12 +573,7 @@ public void testGet_TlqDocumentsIndex_1337_42() throws Exception { @Test public void testGet_TlqDummyIndex_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); // no restrictions on this index GetResponse searchResponse = executeGet("tlqdummy", "101", "tlq_1337", "password"); @@ -639,12 +589,7 @@ public void testGet_TlqDummyIndex_1337() throws Exception { @Test public void testGet_UserAccessCodesIndex_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); // we expect a security exception here, user has no direct access to // user_access_codes index @@ -655,12 +600,7 @@ public void testGet_UserAccessCodesIndex_1337() throws Exception { @Test public void testMGet_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); Map indicesAndIds = new HashMap<>(); indicesAndIds.put("tlqdocuments", "1"); @@ -697,12 +637,7 @@ public void testMGet_1337() throws Exception { @Test public void testSimpleAggregation_tlqdocuments_AccessCode_1337() throws Exception { - setup( - new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") - .setSecurityInternalUsers("internal_users_tlq.yml") - .setSecurityRoles("roles_tlq.yml") - .setSecurityRolesMapping("roles_mapping_tlq.yml") - ); + setupWithTlqSecurityConfig(); String body = "" + " {\n" @@ -743,6 +678,55 @@ public void testSimpleAggregation_tlqdocuments_AccessCode_1337() throws Exceptio Assert.assertNull("Expected bucket FFF to be absent", agg.getBucketByKey("FFF")); } + // ----------------------------------- + // Test query-based TLQ (no doc ID) + // ----------------------------------- + + @Test + public void testQueryBasedTlq_Search_AccessCode_1337() throws Exception { + + setupWithTlqSecurityConfig(); + + final var response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_query_1337", "password")); + assertThat(response.getStatusCode(), is(200)); + final var xcp = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.getBody()); + final var searchResponse = SearchResponse.fromXContent(xcp); + assertThat(searchResponse.toString(), searchResponse.getHits().getTotalHits().value(), is(10L)); + assertAccessCodesMatch(searchResponse.getHits().getHits(), new Integer[] { 1337 }); + } + + @Test + public void testQueryBasedTlq_Search_Dummy_AccessCode_1337() throws Exception { + + setupWithTlqSecurityConfig(); + + final var response = rh.executeGetRequest("/tlqdummy/_search?pretty", encodeBasicHeader("tlq_query_1337", "password")); + assertThat(response.getStatusCode(), is(200)); + final var xcp = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.getBody()); + final var searchResponse = SearchResponse.fromXContent(xcp); + assertThat(searchResponse.toString(), searchResponse.getHits().getTotalHits().value(), is(5L)); + } + + @Test + public void testQueryBasedTlq_Get_AccessCode_1337() throws Exception { + + setupWithTlqSecurityConfig(); + + // doc 1 has access_codes [1337] - should be accessible + HttpResponse response = rh.executeGetRequest("/tlqdocuments/_doc/1", encodeBasicHeader("tlq_query_1337", "password")); + assertThat(response.getStatusCode(), is(200)); + + // doc 2 has access_codes [42] - should not be accessible + response = rh.executeGetRequest("/tlqdocuments/_doc/2", encodeBasicHeader("tlq_query_1337", "password")); + assertThat(response.getStatusCode(), is(404)); + + // user_access_codes index should not be directly accessible + response = rh.executeGetRequest("/user_access_codes/_doc/tlq_query_1337", encodeBasicHeader("tlq_query_1337", "password")); + assertThat(response.getStatusCode(), is(403)); + } + public static List getDefaultNamedXContents() { Map> map = new HashMap<>(); map.put(TopHitsAggregationBuilder.NAME, (p, c) -> ParsedTopHits.fromXContent(p, (String) c)); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryV4EvaluatorTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryV4EvaluatorTest.java new file mode 100644 index 0000000000..deee565a06 --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryV4EvaluatorTest.java @@ -0,0 +1,39 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.dlsfls; + +import org.junit.Ignore; +import org.junit.Test; + +/** + * Runs all TLQ tests with the V4 (nextgen) privilege evaluator. + * The V4 evaluator does not have the DNFOF_MATCHER that silently absorbs + * missing privileges on internal sub-requests, so this exercises the + * DocumentAllowList bypass path more rigorously. + */ +public class DlsTermLookupQueryV4EvaluatorTest extends DlsTermLookupQueryTest { + + @Override + protected String getSecurityConfigName() { + return "securityconfig_tlq_v4.yml"; + } + + @Override + @Test + @Ignore("expected to fail with V4 evaluator") + public void testMGet_1337() throws Exception { + // V4 evaluator does not silently strip unauthorized indices from explicit _mget requests. + // Legacy uses DNFOF_MATCHER on "indices:data/read/*" to reduce indices; V4 only reduces + // for wildcard/pattern requests or ignore_unavailable=true. Concrete unauthorized index -> 403. + super.testMGet_1337(); + } +} diff --git a/src/test/java/org/opensearch/security/privileges/DocumentAllowListTest.java b/src/test/java/org/opensearch/security/privileges/DocumentAllowListTest.java new file mode 100644 index 0000000000..5434ae8cc9 --- /dev/null +++ b/src/test/java/org/opensearch/security/privileges/DocumentAllowListTest.java @@ -0,0 +1,143 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.privileges; + +import org.junit.Test; + +import org.opensearch.action.delete.DeleteRequest; +import org.opensearch.action.get.GetRequest; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.security.support.ConfigConstants; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class DocumentAllowListTest { + + @Test + public void testIsAllowed_GetRequest_matchingEntry() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("my_index", "doc1"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertTrue(DocumentAllowList.isAllowed(new GetRequest("my_index", "doc1"), threadContext)); + } + + @Test + public void testIsAllowed_GetRequest_noMatch() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("my_index", "doc1"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertFalse(DocumentAllowList.isAllowed(new GetRequest("my_index", "doc2"), threadContext)); + } + + @Test + public void testIsAllowed_SearchRequest_wildcardEntry() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("my_index", "*"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertTrue(DocumentAllowList.isAllowed(new SearchRequest("my_index"), threadContext)); + } + + @Test + public void testIsAllowed_SearchRequest_noMatch() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("other_index", "*"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertFalse(DocumentAllowList.isAllowed(new SearchRequest("my_index"), threadContext)); + } + + @Test + public void testIsAllowed_SearchRequest_multipleIndices_allAllowed() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("index_a", "*"); + allowList.add("index_b", "*"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertTrue(DocumentAllowList.isAllowed(new SearchRequest("index_a", "index_b"), threadContext)); + } + + @Test + public void testIsAllowed_SearchRequest_multipleIndices_partialMatch() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("index_a", "*"); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertFalse(DocumentAllowList.isAllowed(new SearchRequest("index_a", "index_b"), threadContext)); + } + + @Test + public void testIsAllowed_noHeader() { + final var threadContext = new ThreadContext(Settings.EMPTY); + + assertFalse(DocumentAllowList.isAllowed(new GetRequest("my_index", "doc1"), threadContext)); + assertFalse(DocumentAllowList.isAllowed(new SearchRequest("my_index"), threadContext)); + } + + @Test + public void testIsAllowed_DeleteRequest_wildcardEntry_returnsFalse() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("my_index", DocumentAllowList.ANY_DOCUMENT_ID); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertFalse(DocumentAllowList.isAllowed(new DeleteRequest("my_index", "doc1"), threadContext)); + } + + @Test + public void testIsAllowed_IndexRequest_wildcardEntry_returnsFalse() { + final var threadContext = new ThreadContext(Settings.EMPTY); + final var allowList = new DocumentAllowList(); + allowList.add("my_index", DocumentAllowList.ANY_DOCUMENT_ID); + threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_DOC_ALLOWLIST_HEADER, allowList.toString()); + + assertFalse(DocumentAllowList.isAllowed(new IndexRequest("my_index"), threadContext)); + } + + @Test + public void testParseToString_roundTrip_withWildcard() { + final var original = new DocumentAllowList(); + original.add("my_index", "*"); + original.add("other_index", "doc1"); + + final var serialized = original.toString(); + final var parsed = DocumentAllowList.parse(serialized); + + assertThat(parsed, is(original)); + } + + @Test + public void testParseToString_roundTrip_withSpecialChars() { + final var original = new DocumentAllowList(); + original.add("my_index", "id/with|special\\chars"); + + final var serialized = original.toString(); + final var parsed = DocumentAllowList.parse(serialized); + + assertThat(parsed, is(original)); + assertTrue(parsed.isAllowed("my_index", "id/with|special\\chars")); + } +} diff --git a/src/test/resources/dlsfls/internal_users_tlq.yml b/src/test/resources/dlsfls/internal_users_tlq.yml index dff0a67633..9b8e86cf38 100644 --- a/src/test/resources/dlsfls/internal_users_tlq.yml +++ b/src/test/resources/dlsfls/internal_users_tlq.yml @@ -26,3 +26,7 @@ tlq_empty_access_codes: tlq_no_codes: hash: "$2y$12$SP9z.rBgEHTlueKkiqSK/OxqB2PLJN/eRoNJ8WOPoHWIpirvbFAAy" # "password" backend_roles: ["os_dls_tlq_lookup"] + +tlq_query_1337: + hash: "$2y$12$SP9z.rBgEHTlueKkiqSK/OxqB2PLJN/eRoNJ8WOPoHWIpirvbFAAy" # "password" + backend_roles: ["os_dls_tlq_query_lookup"] diff --git a/src/test/resources/dlsfls/roles_mapping_tlq.yml b/src/test/resources/dlsfls/roles_mapping_tlq.yml index 9c146360f7..03acafde7d 100644 --- a/src/test/resources/dlsfls/roles_mapping_tlq.yml +++ b/src/test/resources/dlsfls/roles_mapping_tlq.yml @@ -5,3 +5,6 @@ _meta: os_dls_tlq_lookup: backend_roles: ["os_dls_tlq_lookup"] + +os_dls_tlq_query_lookup: + backend_roles: ["os_dls_tlq_query_lookup"] diff --git a/src/test/resources/dlsfls/roles_tlq.yml b/src/test/resources/dlsfls/roles_tlq.yml index 1420a7a965..06fe8244d4 100644 --- a/src/test/resources/dlsfls/roles_tlq.yml +++ b/src/test/resources/dlsfls/roles_tlq.yml @@ -16,3 +16,17 @@ os_dls_tlq_lookup: - "tlqdummy" allowed_actions: - "*" + +os_dls_tlq_query_lookup: + cluster_permissions: + - "*" + index_permissions: + - index_patterns: + - "tlqdocuments" + dls: "{ \"terms\": { \"access_codes\": { \"index\": \"user_access_codes\", \"path\": \"access_codes\", \"query\": { \"term\": { \"_id\": \"${user.name}\" } } } } }" + allowed_actions: + - "*" + - index_patterns: + - "tlqdummy" + allowed_actions: + - "*" diff --git a/src/test/resources/dlsfls/securityconfig_tlq_v4.yml b/src/test/resources/dlsfls/securityconfig_tlq_v4.yml new file mode 100644 index 0000000000..77c8697812 --- /dev/null +++ b/src/test/resources/dlsfls/securityconfig_tlq_v4.yml @@ -0,0 +1,17 @@ +--- +_meta: + type: "config" + config_version: 2 +config: + dynamic: + do_not_fail_on_forbidden: true + privileges_evaluation_type: "v4" + authc: + authentication_domain_basic_internal: + http_enabled: true + transport_enabled: true + order: 0 + http_authenticator: + type: "basic" + authentication_backend: + type: "intern"