diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 6cbabb3989..4e04e50a19 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -99,7 +99,7 @@ public DlsFlsValveImpl(Settings settings, Client nodeClient, ClusterService clus } /** - * + * * @param request * @param listener * @return false on error @@ -143,9 +143,9 @@ public boolean invoke(String action, ActionRequest request, final ActionListener if (doFilterLevelDls) { setDlsModeHeader(Mode.FILTER_LEVEL); - log.debug("Doing filter-level DLS because query contains TLQ"); + log.debug("Doing filter-level DLS because the query contains a TLQ"); } else { - log.debug("Doing lucene-level DLS because query does not contain TLQ"); + log.debug("Doing lucene-level DLS because the query does not contain a TLQ"); } } } diff --git a/src/main/java/org/opensearch/security/configuration/DlsQueryParser.java b/src/main/java/org/opensearch/security/configuration/DlsQueryParser.java index 0797a1e41f..cf533dcb91 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsQueryParser.java +++ b/src/main/java/org/opensearch/security/configuration/DlsQueryParser.java @@ -53,7 +53,7 @@ public final class DlsQueryParser { static { //Match all documents but not the nested ones - //Nested document types start with __ + //Nested document types start with __ //https://discuss.elastic.co/t/whats-nested-documents-layout-inside-the-lucene/59944/9 NON_NESTED_QUERY = new BooleanQuery.Builder().add(new MatchAllDocsQuery(), Occur.FILTER) .add(new PrefixQuery(new Term("_type", "__")), Occur.MUST_NOT).build(); @@ -80,7 +80,7 @@ public BooleanQuery.Builder parse(Set unparsedDlsQueries, QueryShardCont if (unparsedDlsQueries == null || unparsedDlsQueries.isEmpty()) { return null; } - + boolean hasNestedMapping = queryShardContext.getMapperService().hasNested(); BooleanQuery.Builder dlsQueryBuilder = new BooleanQuery.Builder(); @@ -118,8 +118,7 @@ public QueryBuilder parse(String unparsedDlsQuery) { public QueryBuilder call() throws Exception { final XContentParser parser = JsonXContent.jsonXContent.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, unparsedDlsQuery); - final QueryBuilder qb = AbstractQueryBuilder.parseInnerQueryBuilder(parser); - return qb; + return AbstractQueryBuilder.parseInnerQueryBuilder(parser); } }); diff --git a/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java b/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java index 5f8f7f5d56..7ea22ba1ba 100644 --- a/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java +++ b/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java @@ -21,6 +21,12 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.support.ConfigConstants; +/** + * Defines which indices and documents are implicitly accessible although a user does not have + * explicit permissions for it. This is required for executing TLQ in DLS queries. In this case + * the user does not have direct access to the index for the term lookup. However, we need to allow + * access for executing the actual TLQ. The document allow list is scoped to individual requests. + */ public class DocumentAllowList { private final Set entries = new HashSet<>(); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 28a702591d..8ac47f3fa4 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -700,7 +700,7 @@ private boolean checkDocAllowListHeader(User user, String action, ActionRequest if (documentAllowList.isAllowed(getRequest.index(), getRequest.id())) { if (log.isDebugEnabled()) { - log.debug("Request " + request + " is whitelisted by " + documentAllowList); + log.debug("Request " + request + " is allowed by " + documentAllowList); } return true; @@ -709,7 +709,7 @@ private boolean checkDocAllowListHeader(User user, String action, ActionRequest } } catch (Exception e) { - log.error("Error while handling document whitelist: " + docAllowListHeader, e); + log.error("Error while handling document allow list: " + docAllowListHeader, e); return false; } } diff --git a/src/main/java/org/opensearch/security/queries/QueryBuilderTraverser.java b/src/main/java/org/opensearch/security/queries/QueryBuilderTraverser.java index 629b90f198..a10b4bf882 100644 --- a/src/main/java/org/opensearch/security/queries/QueryBuilderTraverser.java +++ b/src/main/java/org/opensearch/security/queries/QueryBuilderTraverser.java @@ -99,60 +99,46 @@ public boolean check(QueryBuilder queryBuilder) { if (queryBuilder instanceof BoolQueryBuilder) { BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder; - return check(boolQueryBuilder.must()) || check(boolQueryBuilder.mustNot()) || check(boolQueryBuilder.should()) || check(boolQueryBuilder.filter()); } else if (queryBuilder instanceof BoostingQueryBuilder) { BoostingQueryBuilder boostingQueryBuilder = (BoostingQueryBuilder) queryBuilder; - return check(boostingQueryBuilder.positiveQuery()) || check(boostingQueryBuilder.negativeQuery()); } else if (queryBuilder instanceof ConstantScoreQueryBuilder) { ConstantScoreQueryBuilder constantScoreQueryBuilder = (ConstantScoreQueryBuilder) queryBuilder; - return check(constantScoreQueryBuilder.innerQuery()); } else if (queryBuilder instanceof DisMaxQueryBuilder) { DisMaxQueryBuilder disMaxQueryBuilder = (DisMaxQueryBuilder) queryBuilder; - return check(disMaxQueryBuilder.innerQueries()); } else if (queryBuilder instanceof FieldMaskingSpanQueryBuilder) { FieldMaskingSpanQueryBuilder fieldMaskingSpanQueryBuilder = (FieldMaskingSpanQueryBuilder) queryBuilder; - return check(fieldMaskingSpanQueryBuilder.innerQuery()); } else if (queryBuilder instanceof FunctionScoreQueryBuilder) { FunctionScoreQueryBuilder functionScoreQueryBuilder = (FunctionScoreQueryBuilder) queryBuilder; - return check(functionScoreQueryBuilder.query()); } else if (queryBuilder instanceof NestedQueryBuilder) { NestedQueryBuilder nestedQueryBuilder = (NestedQueryBuilder) queryBuilder; - return check(nestedQueryBuilder.query()); } else if (queryBuilder instanceof SpanContainingQueryBuilder) { SpanContainingQueryBuilder spanContainingQueryBuilder = (SpanContainingQueryBuilder) queryBuilder; - return check(spanContainingQueryBuilder.bigQuery()) || check(spanContainingQueryBuilder.littleQuery()); } else if (queryBuilder instanceof SpanFirstQueryBuilder) { SpanFirstQueryBuilder spanFirstQueryBuilder = (SpanFirstQueryBuilder) queryBuilder; - return check(spanFirstQueryBuilder.innerQuery()); } else if (queryBuilder instanceof SpanMultiTermQueryBuilder) { SpanMultiTermQueryBuilder spanMultiTermQueryBuilder = (SpanMultiTermQueryBuilder) queryBuilder; - return check(spanMultiTermQueryBuilder.innerQuery()); } else if (queryBuilder instanceof SpanNearQueryBuilder) { SpanNearQueryBuilder spanNearQueryBuilder = (SpanNearQueryBuilder) queryBuilder; - return check(spanNearQueryBuilder.clauses()); } else if (queryBuilder instanceof SpanNotQueryBuilder) { SpanNotQueryBuilder spanNotQueryBuilder = (SpanNotQueryBuilder) queryBuilder; - return check(spanNotQueryBuilder.excludeQuery()) || check(spanNotQueryBuilder.includeQuery()); } else if (queryBuilder instanceof SpanOrQueryBuilder) { SpanOrQueryBuilder spanOrQueryBuilder = (SpanOrQueryBuilder) queryBuilder; - return check(spanOrQueryBuilder.clauses()); } else if (queryBuilder instanceof SpanWithinQueryBuilder) { SpanWithinQueryBuilder spanWithinQueryBuilder = (SpanWithinQueryBuilder) queryBuilder; - return check(spanWithinQueryBuilder.bigQuery()) || check(spanWithinQueryBuilder.littleQuery()); } else { return false; diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index f7b3e495de..aec7d881e3 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -357,7 +357,7 @@ public EvaluatedDlsFlsConfig getDlsFls(User user, IndexNameExpressionResolver re if (!containsDlsFlsConfig()) { if(log.isDebugEnabled()) { - log.debug("No fls or dls found for {} in {} sg roles", user, roles.size()); + log.debug("No fls or dls found for {} in {} security roles", user, roles.size()); } return EvaluatedDlsFlsConfig.EMPTY; @@ -367,8 +367,8 @@ public EvaluatedDlsFlsConfig getDlsFls(User user, IndexNameExpressionResolver re Map> flsFields = new HashMap>(); Map> maskedFieldsMap = new HashMap>(); - for (SecurityRole sgr : roles) { - for (IndexPattern ip : sgr.getIpatterns()) { + for (SecurityRole role : roles) { + for (IndexPattern ip : role.getIpatterns()) { Set concreteIndices; concreteIndices = ip.getResolvedIndexPattern(user, resolver, cs); String dls = ip.getDlsQuery(user); diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index b0f4327194..3e2608e7f3 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -285,16 +285,26 @@ public void handleResponse(T response) { contextToRestore.restore(); + final boolean isDebugEnabled = log.isDebugEnabled(); if (response instanceof ClusterSearchShardsResponse) { if (flsResponseHeader != null && !flsResponseHeader.isEmpty()) { + if (isDebugEnabled) { + log.debug("add flsResponseHeader as transient"); + } threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_FLS_FIELDS_CCS, flsResponseHeader.get(0)); } if (dlsResponseHeader != null && !dlsResponseHeader.isEmpty()) { + if (isDebugEnabled) { + log.debug("add dlsResponseHeader as transient"); + } threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_DLS_QUERY_CCS, dlsResponseHeader.get(0)); } if (maskedFieldsResponseHeader != null && !maskedFieldsResponseHeader.isEmpty()) { + if (isDebugEnabled) { + log.debug("add maskedFieldsResponseHeader as transient"); + } threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_CCS, maskedFieldsResponseHeader.get(0)); } } diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java index 48daeb098f..556252685e 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java @@ -118,4 +118,4 @@ protected MultiGetResponse executeMGet(String user, String password, Map getDefaultNamedXContents() { .collect(Collectors.toList()); return entries; } -} \ No newline at end of file +} diff --git a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java index b77e8e9079..a9ed1e07ec 100644 --- a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java +++ b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java @@ -228,9 +228,7 @@ protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslO builder.putList("plugins.security.authcz.admin_dn", "CN=kirk,OU=client,O=client,l=tEst, C=De"); builder.put(ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST, false); } - // disable disk threshold by default, this sometimes interferes with - // running the tests locally and is not needed for unit and integration tests - builder.put("cluster.routing.allocation.disk.threshold_enabled", false); + builder.put(other); return builder; diff --git a/src/test/resources/dlsfls/internal_users_tlq.yml b/src/test/resources/dlsfls/internal_users_tlq.yml index 2e9ccfc4ab..5bbec586f0 100644 --- a/src/test/resources/dlsfls/internal_users_tlq.yml +++ b/src/test/resources/dlsfls/internal_users_tlq.yml @@ -26,7 +26,3 @@ tlq_empty_access_codes: tlq_no_codes: hash: "$2y$12$SP9z.rBgEHTlueKkiqSK/OxqB2PLJN/eRoNJ8WOPoHWIpirvbFAAy" # "password" backend_roles: ["os_dls_tlq_lookup"] - -tlq_no_entry_in_user_index: - hash: "$2y$12$SP9z.rBgEHTlueKkiqSK/OxqB2PLJN/eRoNJ8WOPoHWIpirvbFAAy" # "password" - backend_roles: ["os_dls_tlq_lookup"] diff --git a/src/test/resources/dlsfls/securityconfig_tlq.yml b/src/test/resources/dlsfls/securityconfig_tlq.yml index 5dd6c5aefc..02c78087ce 100644 --- a/src/test/resources/dlsfls/securityconfig_tlq.yml +++ b/src/test/resources/dlsfls/securityconfig_tlq.yml @@ -11,7 +11,6 @@ config: transport_enabled: true order: 0 http_authenticator: - challenge: false type: "basic" authentication_backend: type: "intern"