From cce7901d30d30f239b28888095177716bd2180dd Mon Sep 17 00:00:00 2001 From: Ralph Ursprung Date: Tue, 23 Jun 2026 17:33:09 +0200 Subject: [PATCH 1/2] Support query-based terms lookup queries in DLS OpenSearch introduced the possibility to use a `query` rather than just an `id` in Terms Lookup Queries in OpenSearch#18195 in release 3.2.0. However, support for this was never added to the security plugin for DLS/FLS. When trying to use `query` based TLQ in a DLS this failed for two reasons: - the visible failure is an NPE because `DlsFilterLevelActionHandler` unconditionally dereferenced `termsLookup().id()`, which returns `null` for query-based lookups. - additionally, the `DocumentAllowList` privilege bypass only handled `GetRequest`, but query-based TLQ internally resolves via `SearchRequest` and additionally uses a `GetSettingsRequest`, causing the privileges evaluator to block the lookup even after the NPE is fixed. This commit adds proper support for `query` based TLQ in DLS, both for the old and the new evaluator, by allowlisting a wildcard entry for the whole index in question rather than a single document in case `query` is used. additionally, `SearchRequest` and `GetSettingsRequest` are now also supported in `DocumentAllowList#isAllowed`. Furthermore, the legacy `PrivilegesEvaluatorImpl#checkDocAllowListHeader` now delegates to `DocumentAllowList#isAllowed` since the old method body was a hand-inlined copy of the same logic (parse header, check request); delegating removes the duplication and picks up `SearchRequest` support without repeating the new code. Header parsing in `DocumentAllowList#isAllowed` is now delegated to `DocumentAllowList#get` to centralise error handling. The wildcard allowlist pattern is already established for Dashboards multi-tenancy and is scoped to the request's thread context, so it cannot grant persistent or write access beyond the TLQ resolution. Existing code with the wildcard has been migrated to use the new constant to make it easier to find it. fixes #6243 Co-authored-by: Claude Opus 4.6 Signed-off-by: Ralph Ursprung --- .../DlsFilterLevelActionHandler.java | 8 +- .../configuration/DlsFlsValveImpl.java | 4 +- .../SecurityFlsDlsIndexSearcherWrapper.java | 2 +- .../privileges/DocumentAllowList.java | 55 ++++--- .../legacy/PrivilegesEvaluatorImpl.java | 27 +--- .../legacy/PrivilegesInterceptor.java | 4 +- ...hboardsMultitenancySystemIndexHandler.java | 4 +- .../dlic/dlsfls/DlsTermLookupQueryTest.java | 69 +++++++++ .../privileges/DocumentAllowListTest.java | 143 ++++++++++++++++++ .../resources/dlsfls/internal_users_tlq.yml | 4 + .../resources/dlsfls/roles_mapping_tlq.yml | 3 + src/test/resources/dlsfls/roles_tlq.yml | 14 ++ 12 files changed, 286 insertions(+), 51 deletions(-) create mode 100644 src/test/java/org/opensearch/security/privileges/DocumentAllowListTest.java 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..9b01c1df54 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryTest.java @@ -88,6 +88,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(); @@ -743,6 +748,70 @@ 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 { + + setup( + new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") + .setSecurityInternalUsers("internal_users_tlq.yml") + .setSecurityRoles("roles_tlq.yml") + .setSecurityRolesMapping("roles_mapping_tlq.yml") + ); + + 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 { + + setup( + new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") + .setSecurityInternalUsers("internal_users_tlq.yml") + .setSecurityRoles("roles_tlq.yml") + .setSecurityRolesMapping("roles_mapping_tlq.yml") + ); + + 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 { + + setup( + new DynamicSecurityConfig().setConfig("securityconfig_tlq.yml") + .setSecurityInternalUsers("internal_users_tlq.yml") + .setSecurityRoles("roles_tlq.yml") + .setSecurityRolesMapping("roles_mapping_tlq.yml") + ); + + // 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/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: + - "*" From 6a67442a5dd72eeff5f2bac84cd19d3c88f5abde Mon Sep 17 00:00:00 2001 From: Ralph Ursprung Date: Wed, 24 Jun 2026 15:41:12 +0200 Subject: [PATCH 2/2] add `DlsTermLookupQueryTest` version for V4 evaluator so far, the tests only used the legacy evaluator. add a new subclass to also test against V4 to ensure that this is also covered. Co-authored-by: Claude Opus 4.6 Signed-off-by: Ralph Ursprung --- .../dlic/dlsfls/DlsTermLookupQueryTest.java | 155 ++++-------------- .../DlsTermLookupQueryV4EvaluatorTest.java | 39 +++++ .../dlsfls/securityconfig_tlq_v4.yml | 17 ++ 3 files changed, 91 insertions(+), 120 deletions(-) create mode 100644 src/test/java/org/opensearch/security/dlic/dlsfls/DlsTermLookupQueryV4EvaluatorTest.java create mode 100644 src/test/resources/dlsfls/securityconfig_tlq_v4.yml 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 9b01c1df54..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( @@ -230,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)); @@ -251,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)); @@ -274,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)); @@ -297,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)); @@ -316,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)); } @@ -329,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)); @@ -342,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"); @@ -383,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"); @@ -424,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"); @@ -466,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"); @@ -499,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", @@ -541,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"); @@ -581,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"); @@ -623,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"); @@ -644,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 @@ -660,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"); @@ -702,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" @@ -755,12 +685,7 @@ public void testSimpleAggregation_tlqdocuments_AccessCode_1337() throws Exceptio @Test public void testQueryBasedTlq_Search_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(); final var response = rh.executeGetRequest("/tlqdocuments/_search?pretty", encodeBasicHeader("tlq_query_1337", "password")); assertThat(response.getStatusCode(), is(200)); @@ -774,12 +699,7 @@ public void testQueryBasedTlq_Search_AccessCode_1337() throws Exception { @Test public void testQueryBasedTlq_Search_Dummy_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(); final var response = rh.executeGetRequest("/tlqdummy/_search?pretty", encodeBasicHeader("tlq_query_1337", "password")); assertThat(response.getStatusCode(), is(200)); @@ -792,12 +712,7 @@ public void testQueryBasedTlq_Search_Dummy_AccessCode_1337() throws Exception { @Test public void testQueryBasedTlq_Get_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(); // doc 1 has access_codes [1337] - should be accessible HttpResponse response = rh.executeGetRequest("/tlqdocuments/_doc/1", encodeBasicHeader("tlq_query_1337", "password")); 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/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"