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/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,13 +129,14 @@ 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(

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

@@ -0,0 +1,85 @@
+/*
+ * 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 final 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;
+    }
+
+}

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

@@ -140,8 +140,6 @@
     private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator;
     private final TermsAggregationEvaluator termsAggregationEvaluator;
     private final PitPrivilegesEvaluator pitPrivilegesEvaluator;
-    private final boolean dlsFlsEnabled;
-    private final boolean dfmEmptyOverwritesAll;
     private DynamicConfigModel dcm;
     private final NamedXContentRegistry namedXContentRegistry;
 
@@ -155,7 +153,6 @@
         final PrivilegesInterceptor privilegesInterceptor,
         final ClusterInfoHolder clusterInfoHolder,
         final IndexResolverReplacer irr,
-        boolean dlsFlsEnabled,
         NamedXContentRegistry namedXContentRegistry
     ) {
 
@@ -180,8 +177,6 @@
         termsAggregationEvaluator = new TermsAggregationEvaluator();
         pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
         this.namedXContentRegistry = namedXContentRegistry;
-        this.dlsFlsEnabled = dlsFlsEnabled;
-        this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false);
     }
 
     @Subscribe
@@ -226,18 +221,45 @@
         }
     }
 
-    public PrivilegesEvaluatorResponse evaluate(
-        final User user,
-        String action0,
-        final ActionRequest request,
-        Task task,
-        final Set<String> injectedRoles
-    ) {
+    public PrivilegesEvaluationContext createContext(User user, String action0, ActionRequest request, Task task, Set<String> injectedRoles)
+        throws PrivilegesEvaluatorResponse.NotAllowedException {
+        if (!isInitialized()) {
+            throw new OpenSearchSecurityException("OpenSearch Security is not initialized.");
+        }
+
+        TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
+        ImmutableSet<String> mappedRoles = ImmutableSet.copyOf((injectedRoles == null) ? mapRoles(user, caller) : injectedRoles);
+        final String injectedRolesValidationString = threadContext.getTransient(
+            ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION
+        );
+        if (injectedRolesValidationString != null) {
+            HashSet<String> injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(",")));
+            if (!mappedRoles.containsAll(injectedRolesValidationSet)) {
+                PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse();
+                presponse.allowed = false;
+                presponse.missingSecurityRoles.addAll(injectedRolesValidationSet);
+                presponse.reason("Injected roles validation failed");
+                log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user);
+                throw new PrivilegesEvaluatorResponse.NotAllowedException(presponse);
+            }
+            mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet);
+        }
+
+        return new PrivilegesEvaluationContext(user, mappedRoles, action0, request, task, irr);
+    }
+
+    public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) {
 
         if (!isInitialized()) {
             throw new OpenSearchSecurityException("OpenSearch Security is not initialized.");
         }
 
+        String action0 = context.getAction();
+        ImmutableSet<String> mappedRoles = context.getMappedRoles();
+        User user = context.getUser();
+        ActionRequest request = context.getRequest();
+        Task task = context.getTask();
+
         if (action0.startsWith("internal:indices/admin/upgrade")) {
             action0 = "indices:admin/upgrade";
         }
@@ -250,23 +272,8 @@
             action0 = PutMappingAction.NAME;
         }
 
-        final PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse();
+        PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse();
 
-        final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
-        Set<String> mappedRoles = (injectedRoles == null) ? mapRoles(user, caller) : injectedRoles;
-        final String injectedRolesValidationString = threadContext.getTransient(
-            ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION
-        );
-        if (injectedRolesValidationString != null) {
-            HashSet<String> injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(",")));
-            if (!mappedRoles.containsAll(injectedRolesValidationSet)) {
-                presponse.allowed = false;
-                presponse.missingSecurityRoles.addAll(injectedRolesValidationSet);
-                log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user);
-                return presponse;
-            }
-            mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet);
-        }
         presponse.resolvedSecurityRoles.addAll(mappedRoles);
         final SecurityRoles securityRoles = getSecurityRoles(mappedRoles);
 
@@ -305,8 +312,7 @@
             return presponse;
         }
 
-        final Resolved requestedResolved = irr.resolveRequest(request);
-        presponse.resolved = requestedResolved;
+        final Resolved requestedResolved = context.getResolvedRequest();
 
         if (isDebugEnabled) {
             log.debug("RequestedResolved : {}", requestedResolved);
@@ -349,14 +355,6 @@
             log.trace("dnfof enabled? {}", dnfofEnabled);
         }
 
-        presponse.evaluatedDlsFlsConfig = getSecurityRoles(mappedRoles).getDlsFls(
-            user,
-            dfmEmptyOverwritesAll,
-            resolver,
-            clusterService,
-            namedXContentRegistry
-        );
-
         final boolean serviceAccountUser = user.isServiceAccount();
         if (isClusterPerm(action0)) {
             if (serviceAccountUser) {
@@ -435,7 +433,11 @@
         final String[] allIndexPermsRequiredA = allIndexPermsRequired.toArray(new String[0]);
 
         if (isDebugEnabled) {
-            log.debug("Requested {} from {}", allIndexPermsRequired, caller);
+            log.debug(
+                "Requested {} from {}",
+                allIndexPermsRequired,
+                threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS)
+            );
         }
 
         presponse.missingPrivileges.clear();