Separated DLS/FLS privilege evaluation from action privilege evaluation

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -1118,8 +1118,6 @@ public Collection<Object> createComponents(
 
         final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);
 
-        // DLS-FLS is enabled if not client and not disabled and not SSL only.
-        final boolean dlsFlsEnabled = !SSLConfig.isSslOnlyMode();
         evaluator = new PrivilegesEvaluator(
             clusterService,
             threadPool,
@@ -1130,7 +1128,6 @@ public Collection<Object> createComponents(
             privilegesInterceptor,
             cih,
             irr,
-            dlsFlsEnabled,
             namedXContentRegistry.get()
         );
 
@@ -1169,6 +1166,9 @@ public Collection<Object> createComponents(
             // Don't register if advanced modules is disabled in which case auditlog is instance of NullAuditLog
             dcf.registerDCFListener(auditLog);
         }
+        if (dlsFlsValve instanceof DlsFlsValveImpl) {
+            dcf.registerDCFListener(dlsFlsValve);
+        }
 
         cr.setDynamicConfigFactory(dcf);
 

src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java

@@ -58,6 +58,7 @@
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.security.privileges.DocumentAllowList;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.queries.QueryBuilderTraverser;
 import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
 import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
@@ -74,11 +75,9 @@ public class DlsFilterLevelActionHandler {
     );
 
     public static boolean handle(
-        String action,
-        ActionRequest request,
-        ActionListener<?> listener,
+        PrivilegesEvaluationContext context,
         EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
-        Resolved resolved,
+        ActionListener<?> listener,
         Client nodeClient,
         ClusterService clusterService,
         IndicesService indicesService,
@@ -91,6 +90,9 @@ public static boolean handle(
             return true;
         }
 
+        String action = context.getAction();
+        ActionRequest request = context.getRequest();
+
         if (action.startsWith("cluster:")
             || action.startsWith("indices:admin/template/")
             || action.startsWith("indices:admin/index_template/")) {
@@ -112,11 +114,9 @@ public static boolean handle(
         }
 
         return new DlsFilterLevelActionHandler(
-            action,
-            request,
-            listener,
+            context,
             evaluatedDlsFlsConfig,
-            resolved,
+            listener,
             nodeClient,
             clusterService,
             indicesService,
@@ -142,23 +142,21 @@ public static boolean handle(
     private DocumentAllowList documentAllowlist;
 
     DlsFilterLevelActionHandler(
-        String action,
-        ActionRequest request,
-        ActionListener<?> listener,
+        PrivilegesEvaluationContext context,
         EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
-        Resolved resolved,
+        ActionListener<?> listener,
         Client nodeClient,
         ClusterService clusterService,
         IndicesService indicesService,
         IndexNameExpressionResolver resolver,
         DlsQueryParser dlsQueryParser,
         ThreadContext threadContext
     ) {
-        this.action = action;
-        this.request = request;
+        this.action = context.getAction();
+        this.request = context.getRequest();
         this.listener = listener;
         this.evaluatedDlsFlsConfig = evaluatedDlsFlsConfig;
-        this.resolved = resolved;
+        this.resolved = context.getResolvedRequest();
         this.nodeClient = nodeClient;
         this.clusterService = clusterService;
         this.indicesService = indicesService;

src/main/java/org/opensearch/security/configuration/DlsFlsRequestValve.java

@@ -26,24 +26,16 @@
 
 package org.opensearch.security.configuration;
 
-import org.opensearch.action.ActionRequest;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.search.internal.SearchContext;
 import org.opensearch.search.query.QuerySearchResult;
-import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
-import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.threadpool.ThreadPool;
 
 public interface DlsFlsRequestValve {
 
-    boolean invoke(
-        String action,
-        ActionRequest request,
-        ActionListener<?> listener,
-        EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
-        Resolved resolved
-    );
+    boolean invoke(PrivilegesEvaluationContext context, ActionListener<?> listener);
 
     void handleSearchContext(SearchContext context, ThreadPool threadPool, NamedXContentRegistry namedXContentRegistry);
 
@@ -52,13 +44,7 @@ boolean invoke(
     public static class NoopDlsFlsRequestValve implements DlsFlsRequestValve {
 
         @Override
-        public boolean invoke(
-            String action,
-            ActionRequest request,
-            ActionListener<?> listener,
-            EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
-            Resolved resolved
-        ) {
+        public boolean invoke(PrivilegesEvaluationContext context, ActionListener<?> listener) {
             return true;
         }
 

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

@@ -72,14 +72,18 @@
 import org.opensearch.search.internal.SearchContext;
 import org.opensearch.search.query.QuerySearchResult;
 import org.opensearch.security.OpenSearchSecurityPlugin;
-import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
+import org.opensearch.security.resolver.IndexResolverReplacer;
+import org.opensearch.security.securityconf.ConfigModel;
 import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
 import org.opensearch.security.support.Base64Helper;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.HeaderHelper;
 import org.opensearch.security.support.SecurityUtils;
 import org.opensearch.threadpool.ThreadPool;
 
+import org.greenrobot.eventbus.Subscribe;
+
 public class DlsFlsValveImpl implements DlsFlsRequestValve {
 
     private static final String MAP_EXECUTION_HINT = "map";
@@ -91,6 +95,9 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve {
     private final Mode mode;
     private final DlsQueryParser dlsQueryParser;
     private final IndexNameExpressionResolver resolver;
+    private final boolean dfmEmptyOverwritesAll;
+    private final NamedXContentRegistry namedXContentRegistry;
+    private volatile ConfigModel configModel;
 
     public DlsFlsValveImpl(
         Settings settings,
@@ -107,21 +114,29 @@ public DlsFlsValveImpl(
         this.threadContext = threadContext;
         this.mode = Mode.get(settings);
         this.dlsQueryParser = new DlsQueryParser(namedXContentRegistry);
+        this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false);
+        this.namedXContentRegistry = namedXContentRegistry;
+    }
+
+    @Subscribe
+    public void onConfigModelChanged(ConfigModel configModel) {
+        this.configModel = configModel;
     }
 
     /**
      *
-     * @param request
      * @param listener
      * @return false on error
      */
-    public boolean invoke(
-        String action,
-        ActionRequest request,
-        final ActionListener<?> listener,
-        EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
-        final Resolved resolved
-    ) {
+    @Override
+    public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<?> listener) {
+
+        EvaluatedDlsFlsConfig evaluatedDlsFlsConfig = configModel.getSecurityRoles()
+            .filter(context.getMappedRoles())
+            .getDlsFls(context.getUser(), dfmEmptyOverwritesAll, resolver, clusterService, namedXContentRegistry);
+
+        ActionRequest request = context.getRequest();
+        IndexResolverReplacer.Resolved resolved = context.getResolvedRequest();
 
         if (log.isDebugEnabled()) {
             log.debug(
@@ -288,7 +303,7 @@ public boolean invoke(
             return false;
         }
 
-        if (action.contains("plugins/replication")) {
+        if (context.getAction().contains("plugins/replication")) {
             listener.onFailure(
                 new OpenSearchSecurityException(
                     "Cross Cluster Replication is not supported when FLS or DLS or Fieldmasking is activated",
@@ -324,11 +339,9 @@ public boolean invoke(
 
         if (doFilterLevelDls && filteredDlsFlsConfig.hasDls()) {
             return DlsFilterLevelActionHandler.handle(
-                action,
-                request,
-                listener,
+                context,
                 evaluatedDlsFlsConfig,
-                resolved,
+                listener,
                 nodeClient,
                 clusterService,
                 OpenSearchSecurityPlugin.GuiceHolder.getIndicesService(),

src/main/java/org/opensearch/security/filter/SecurityFilter.java

@@ -82,6 +82,7 @@
 import org.opensearch.security.configuration.CompatConfig;
 import org.opensearch.security.configuration.DlsFlsRequestValve;
 import org.opensearch.security.http.XFFResolver;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.privileges.PrivilegesEvaluatorResponse;
 import org.opensearch.security.resolver.IndexResolverReplacer;
@@ -378,7 +379,8 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
                 log.trace("Evaluate permissions for user: {}", user.getName());
             }
 
-            final PrivilegesEvaluatorResponse pres = eval.evaluate(user, action, request, task, injectedRoles);
+            PrivilegesEvaluationContext context = eval.createContext(user, action, request, task, injectedRoles);
+            PrivilegesEvaluatorResponse pres = eval.evaluate(context);
 
             if (log.isDebugEnabled()) {
                 log.debug(pres.toString());
@@ -387,7 +389,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
             if (pres.isAllowed()) {
                 auditLog.logGrantedPrivileges(action, request, task);
                 auditLog.logIndexEvent(action, request, task);
-                if (!dlsFlsValve.invoke(action, request, listener, pres.getEvaluatedDlsFlsConfig(), pres.getResolved())) {
+                if (!dlsFlsValve.invoke(context, listener)) {
                     return;
                 }
                 final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder();

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java

@@ -0,0 +1,97 @@
+/*
+ * 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 com.google.common.collect.ImmutableSet;
+
+import org.opensearch.action.ActionRequest;
+import org.opensearch.security.resolver.IndexResolverReplacer;
+import org.opensearch.security.user.User;
+import org.opensearch.tasks.Task;
+
+/**
+ * Request-scoped context information for privilege evaluation.
+ *
+ * This class carries metadata about the request and provides caching facilities for data which might need to be
+ * evaluated several times per request.
+ *
+ * As this class is request-scoped, it is only used by a single thread. Thus, no thread synchronization mechanisms
+ * are necessary.
+ */
+public class PrivilegesEvaluationContext {
+    private final User user;
+    private final String action;
+    private final ActionRequest request;
+    private IndexResolverReplacer.Resolved resolvedRequest;
+    private final Task task;
+    private ImmutableSet<String> mappedRoles;
+    private final IndexResolverReplacer indexResolverReplacer;
+
+    public PrivilegesEvaluationContext(
+        User user,
+        ImmutableSet<String> mappedRoles,
+        String action,
+        ActionRequest request,
+        Task task,
+        IndexResolverReplacer indexResolverReplacer
+    ) {
+        this.user = user;
+        this.mappedRoles = mappedRoles;
+        this.action = action;
+        this.request = request;
+        this.task = task;
+        this.indexResolverReplacer = indexResolverReplacer;
+    }
+
+    public User getUser() {
+        return user;
+    }
+
+    public String getAction() {
+        return action;
+    }
+
+    public ActionRequest getRequest() {
+        return request;
+    }
+
+    public IndexResolverReplacer.Resolved getResolvedRequest() {
+        IndexResolverReplacer.Resolved result = this.resolvedRequest;
+
+        if (result == null) {
+            result = indexResolverReplacer.resolveRequest(request);
+            this.resolvedRequest = result;
+        }
+
+        return result;
+    }
+
+    public Task getTask() {
+        return task;
+    }
+
+    public ImmutableSet<String> getMappedRoles() {
+        return mappedRoles;
+    }
+
+    /**
+     * Note: Ideally, mappedRoles would be an unmodifiable attribute. PrivilegesEvaluator however contains logic
+     * related to OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION which first validates roles and afterwards modifies
+     * them again. Thus, we need to be able to set this attribute.
+     *
+     * However, this method should be only used for this one particular phase. Normally, all roles should be determined
+     * upfront and stay constant during the whole privilege evaluation process.
+     */
+    void setMappedRoles(ImmutableSet<String> mappedRoles) {
+        this.mappedRoles = mappedRoles;
+    }
+
+}