First shot at optimized DLS/FLS

Signed-off-by: Nils Bandener <[email protected]>
nibix · Aug 9, 2024 · 9dd37da · 9dd37da
1 parent 41647a6
commit 9dd37da
Show file tree

Hide file tree

Showing 8 changed files with 141 additions and 128 deletions.
diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -43,7 +43,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -195,8 +194,6 @@
 import org.opensearch.security.support.ModuleInfo;
 import org.opensearch.security.support.ReflectionHelper;
 import org.opensearch.security.support.SecuritySettings;
-import org.opensearch.security.support.SecurityUtils;
-import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.security.transport.DefaultInterClusterRequestEvaluator;
 import org.opensearch.security.transport.InterClusterRequestEvaluator;
 import org.opensearch.security.transport.SecurityInterceptor;

diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java
@@ -195,7 +195,7 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<
                     doFilterLevelDls = true;
                     log.debug("Doing filter-level DLS due to header");
                     dlsRestrictionMap = config.getDocumentPrivileges()
-                            .getRestrictions(context, resolved.getAllIndicesResolved(clusterService, context.getIndexNameExpressionResolver()));
+                        .getRestrictions(context, resolved.getAllIndicesResolved(clusterService, context.getIndexNameExpressionResolver()));
                 } else {
                     dlsRestrictionMap = config.getDocumentPrivileges()
                         .getRestrictions(context, resolved.getAllIndicesResolved(clusterService, context.getIndexNameExpressionResolver()));
@@ -476,10 +476,10 @@ public boolean hasFlsOrFieldMasking(String index) throws PrivilegesEvaluationExc
         }
 
         DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get();
-        return !config.getFieldPrivileges().isUnrestricted(privilegesEvaluationContext, index) || !config.getFieldMasking().isUnrestricted(privilegesEvaluationContext, index);
+        return !config.getFieldPrivileges().isUnrestricted(privilegesEvaluationContext, index)
+            || !config.getFieldMasking().isUnrestricted(privilegesEvaluationContext, index);
     }
 
-
     @Override
     public boolean hasFieldMasking(String index) throws PrivilegesEvaluationException {
         PrivilegesEvaluationContext privilegesEvaluationContext = this.dlsFlsBaseContext.getPrivilegesEvaluationContext();

diff --git a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java
@@ -157,7 +157,7 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm
 
             DocumentAllowList documentAllowList = DocumentAllowList.get(threadContext);
 
-            if (documentAllowList.isEntryForIndexPresent(index.getName())  && (!flsRule.isAllowAll() || !fmRule.isAllowAll())) {
+            if (documentAllowList.isEntryForIndexPresent(index.getName()) && (!flsRule.isAllowAll() || !fmRule.isAllowAll())) {
                 log.debug("Lifting FLS/FM for {} due to present document allowlist");
                 flsRule = FieldPrivileges.FlsRule.ALLOW_ALL;
                 fmRule = FieldMasking.FieldMaskingRule.ALLOW_ALL;

diff --git a/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java b/src/main/java/org/opensearch/security/privileges/DocumentAllowList.java
@@ -16,6 +16,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.security.support.ConfigConstants;
 
@@ -46,11 +47,9 @@ public static DocumentAllowList get(ThreadContext threadContext) {
 
     private static final DocumentAllowList EMPTY = new DocumentAllowList();
 
-
     private final Set<Entry> entries = new HashSet<>();
 
-    public DocumentAllowList() {
-    }
+    public DocumentAllowList() {}
 
     public void add(String index, String id) {
         this.add(new Entry(index, id));

diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/AbstractRuleBasedPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/AbstractRuleBasedPrivileges.java
@@ -1,3 +1,13 @@
+/*
+ * 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.dlsfls;
 
 import java.util.ArrayList;
@@ -133,14 +143,13 @@ public boolean isUnrestricted(PrivilegesEvaluationContext context, IndexResolver
      * Returns true if there are roles without a rule which imposes restrictions for the particular index.
      * Does consider rules with index wildcards ("*").
      */
-    public boolean isUnrestricted(PrivilegesEvaluationContext context, String index)
-        throws PrivilegesEvaluationException {
+    public boolean isUnrestricted(PrivilegesEvaluationContext context, String index) throws PrivilegesEvaluationException {
         if (context.getMappedRoles().isEmpty()) {
             return false;
         }
 
         if (this.dfmEmptyOverwritesAll
-                && CollectionUtils.containsAny(this.staticIndexRules.rolesWithIndexWildcardWithoutRule, context.getMappedRoles())) {
+            && CollectionUtils.containsAny(this.staticIndexRules.rolesWithIndexWildcardWithoutRule, context.getMappedRoles())) {
             return true;
         }
 
@@ -411,14 +420,6 @@ private Collection<IndexAbstraction> getParents(IndexAbstraction indexAbstractio
         }
     }
 
-    private Set<String> getParentAliases(IndexAbstraction indexAbstraction) {
-        if (indexAbstraction instanceof IndexAbstraction.Index) {
-            return ((IndexAbstraction.Index) indexAbstraction).getWriteIndex().getAliases().keySet();
-        } else {
-            return Collections.emptySet();
-        }
-    }
-
     static class StaticRules<SingleRule> {
 
         static class Index<SingleRule> extends StaticRules<SingleRule> {

diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java
@@ -146,7 +146,8 @@ QueryBuilder evaluate(PrivilegesEvaluationContext context) throws PrivilegesEval
                 try {
                     XContentParser parser = JsonXContent.jsonXContent.createParser(
                         xContentRegistry,
-                        DeprecationHandler.THROW_UNSUPPORTED_OPERATION, effectiveQueryString
+                        DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
+                        effectiveQueryString
                     );
                     return AbstractQueryBuilder.parseInnerQueryBuilder(parser);
                 } catch (Exception e) {