Separated DLS/FLS priv evaluation from action privilege evaluation

Signed-off-by: Nils Bandener <[email protected]>

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()
         );
 

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

@@ -31,19 +31,12 @@
 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(String action, ActionRequest request, ActionListener<?> listener, PrivilegesEvaluationContext context);
 
     void handleSearchContext(SearchContext context, ThreadPool threadPool, NamedXContentRegistry namedXContentRegistry);
 
@@ -52,13 +45,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(String action, ActionRequest request, ActionListener<?> listener, PrivilegesEvaluationContext context) {
             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,6 +114,13 @@ 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;
     }
 
     /**
@@ -115,21 +129,22 @@ public DlsFlsValveImpl(
      * @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(String action, ActionRequest request, final ActionListener<?> listener, PrivilegesEvaluationContext context) {
+
+        EvaluatedDlsFlsConfig evaluatedDlsFlsConfig = configModel.getSecurityRoles()
+            .filter(context.getMappedRoles())
+            .getDlsFls(context.getUser(), dfmEmptyOverwritesAll, resolver, clusterService, namedXContentRegistry);
+
+        IndexResolverReplacer.Resolved resolved = context.getResolvedRequest();
 
         if (log.isDebugEnabled()) {
             log.debug(
                 "DlsFlsValveImpl.invoke()\nrequest: "
                     + request
                     + "\nevaluatedDlsFlsConfig: "
                     + evaluatedDlsFlsConfig
-                    + "\nresolved: "
+                    + "\ncontext: "
                     + resolved
                     + "\nmode: "
                     + mode

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,15 @@ 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 = null;
+            PrivilegesEvaluatorResponse pres;
+
+            try {
+                context = eval.createContext(user, action, request, task, injectedRoles);
+                pres = eval.evaluate(context);
+            } catch (PrivilegesEvaluatorResponse.NotAllowedException e) {
+                pres = e.getResponse();
+            }
 
             if (log.isDebugEnabled()) {
                 log.debug(pres.toString());
@@ -387,7 +396,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(action, request, listener, context)) {
                     return;
                 }
                 final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder();

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

@@ -16,10 +16,13 @@
 
 import com.google.common.collect.ImmutableSet;
 
+import org.opensearch.action.ActionRequest;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.security.user.User;
+import org.opensearch.tasks.Task;
 
 /**
  * Request-scoped context information for privilege evaluation.
@@ -31,10 +34,11 @@
  * are necessary.
  */
 public class PrivilegesEvaluationContext {
-    private boolean resolveLocalAll = true;
     private final User user;
     private final String action;
-    private final Object request;
+    private final ActionRequest request;
+    private IndexResolverReplacer.Resolved resolvedRequest;
+    private final Task task;
 
     /**
      * This caches the ready to use WildcardMatcher instances for the current request. Many index patterns have
@@ -43,23 +47,27 @@ public class PrivilegesEvaluationContext {
      */
     private final Map<String, WildcardMatcher> renderedPatternTemplateCache = new HashMap<>();
     private final ImmutableSet<String> mappedRoles;
-
     private final Supplier<ClusterState> clusterStateSupplier;
+    private final IndexResolverReplacer indexResolverReplacer;
     private final IndexNameExpressionResolver indexNameExpressionResolver;
 
     public PrivilegesEvaluationContext(
         User user,
         ImmutableSet<String> mappedRoles,
         String action,
-        Object request,
+        ActionRequest request,
+        Task task,
         Supplier<ClusterState> clusterStateSupplier,
+        IndexResolverReplacer indexResolverReplacer,
         IndexNameExpressionResolver indexNameExpressionResolver
     ) {
         this.user = user;
         this.mappedRoles = mappedRoles;
         this.action = action;
         this.request = request;
+        this.task = task;
         this.clusterStateSupplier = clusterStateSupplier;
+        this.indexResolverReplacer = indexResolverReplacer;
         this.indexNameExpressionResolver = indexNameExpressionResolver;
     }
 
@@ -88,10 +96,25 @@ public String getAction() {
         return action;
     }
 
-    public Object getRequest() {
+    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;
     }
@@ -100,7 +123,16 @@ public PrivilegesEvaluationContext mappedRoles(ImmutableSet<String> mappedRoles)
         if (this.mappedRoles != null && this.mappedRoles.equals(mappedRoles)) {
             return this;
         } else {
-            return new PrivilegesEvaluationContext(user, mappedRoles, action, request, clusterStateSupplier, indexNameExpressionResolver);
+            return new PrivilegesEvaluationContext(
+                user,
+                mappedRoles,
+                action,
+                request,
+                task,
+                clusterStateSupplier,
+                indexResolverReplacer,
+                indexNameExpressionResolver
+            );
         }
     }