Get cluster actions working

Signed-off-by: Craig Perkins <[email protected]>
cwperks · Oct 25, 2024 · 0a8924d · 0a8924d
1 parent 19d4362
commit 0a8924d
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 24 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java b/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java
@@ -14,7 +14,7 @@
 
 public class RunClusterHealthAction extends ActionType<RunClusterHealthResponse> {
     public static final RunClusterHealthAction INSTANCE = new RunClusterHealthAction();
-    public static final String NAME = "mock:cluster/monitor/health";
+    public static final String NAME = "cluster:mock/monitor/health";
 
     private RunClusterHealthAction() {
         super(NAME, RunClusterHealthResponse::new);

diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -43,6 +43,7 @@
 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;
@@ -69,6 +70,7 @@
 import org.opensearch.SpecialPermission;
 import org.opensearch.Version;
 import org.opensearch.action.ActionRequest;
+import org.opensearch.action.bulk.BulkAction;
 import org.opensearch.action.search.PitService;
 import org.opensearch.action.search.SearchScrollAction;
 import org.opensearch.action.support.ActionFilter;
@@ -2126,7 +2128,11 @@ public SecurityTokenManager getTokenManager() {
 
     @Override
     public PluginSubject getPluginSubject(Plugin plugin) {
-        return new ContextProvidingPluginSubject(threadPool, settings, plugin);
+        Set<String> clusterActions = new HashSet<>();
+        clusterActions.add(BulkAction.NAME);
+        PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin);
+        sf.updatePluginToClusterAction(subject.getPrincipal().getName(), clusterActions);
+        return subject;
     }
 
     @Override

diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java
@@ -453,6 +453,7 @@ public void onFailure(Exception e) {
                     });
                 }
             } else {
+                System.out.println("No permissions for " + user);
                 auditLog.logMissingPrivileges(action, request, task);
                 String err;
                 if (!pres.getMissingSecurityRoles().isEmpty()) {
@@ -529,6 +530,10 @@ private boolean checkImmutableIndices(Object request, ActionListener listener) {
         return false;
     }
 
+    public void updatePluginToClusterAction(String pluginIdentifier, Set<String> clusterActions) {
+        evalp.updatePluginToClusterActions(pluginIdentifier, clusterActions);
+    }
+
     private boolean isRequestIndexImmutable(Object request) {
         final IndexResolverReplacer.Resolved resolved = indexResolverReplacer.resolveRequest(request);
         if (resolved.isLocalAll()) {

diff --git a/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java b/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java
@@ -29,10 +29,11 @@ public class ContextProvidingPluginSubject implements PluginSubject {
     public ContextProvidingPluginSubject(ThreadPool threadPool, Settings settings, Plugin plugin) {
         super();
         this.threadPool = threadPool;
-        this.pluginPrincipal = new NamedPrincipal(plugin.getClass().getCanonicalName());
+        String principal = "plugin:" + plugin.getClass().getCanonicalName();
+        this.pluginPrincipal = new NamedPrincipal(principal);
         // Convention for plugin username. Prefixed with 'plugin:'. ':' is forbidden from usernames, so this
         // guarantees that a user with this username cannot be created by other means.
-        this.pluginUser = new User("plugin:" + pluginPrincipal.getName());
+        this.pluginUser = new User(principal);
     }
 
     @Override

diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
@@ -303,22 +303,6 @@ public PrivilegesEvaluationContext createContext(
             throw new OpenSearchSecurityException("OpenSearch Security is not initialized.");
         }
 
-        if (user.isPluginUser()) {
-            String pluginIdentifier = user.getName();
-            Set<String> clusterActions = pluginToClusterActions.get(pluginIdentifier);
-            if (clusterActions == null) {
-                clusterActions = new HashSet<>();
-                clusterActions.add(BulkAction.NAME);
-                pluginToClusterActions.put(pluginIdentifier, clusterActions);
-                SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration = configurationRepository.getConfiguration(
-                    CType.ACTIONGROUPS
-                );
-                SecurityDynamicConfiguration<RoleV7> rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES);
-
-                this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration);
-            }
-        }
-
         TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
         ImmutableSet<String> mappedRoles = ImmutableSet.copyOf((injectedRoles == null) ? mapRoles(user, caller) : injectedRoles);
 
@@ -412,7 +396,11 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
 
         // check snapshot/restore requests
         if (snapshotRestoreEvaluator.evaluate(request, task, action0, clusterInfoHolder, presponse).isComplete()) {
-            return presponse;
+            if (!presponse.isAllowed()) {
+                return PrivilegesEvaluatorResponse.insufficient(action0, context);
+            } else {
+                return presponse;
+            }
         }
 
         System.out.println("Calling systemIndexAccessEvaluator.evaluate");
@@ -422,18 +410,30 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
         if (systemIndexAccessEvaluator.evaluate(request, task, action0, requestedResolved, presponse, context, actionPrivileges, user)
             .isComplete()) {
             System.out.println("Returning presponse: " + presponse);
-            return presponse;
+            if (!presponse.isAllowed()) {
+                return PrivilegesEvaluatorResponse.insufficient(action0, context);
+            } else {
+                return presponse;
+            }
         }
         System.out.println("After systemIndexAccessEvaluator.evaluate");
 
         // Protected index access
         if (protectedIndexAccessEvaluator.evaluate(request, task, action0, requestedResolved, presponse, mappedRoles).isComplete()) {
-            return presponse;
+            if (!presponse.isAllowed()) {
+                return PrivilegesEvaluatorResponse.insufficient(action0, context);
+            } else {
+                return presponse;
+            }
         }
 
         // check access for point in time requests
         if (pitPrivilegesEvaluator.evaluate(request, context, actionPrivileges, action0, presponse, irr).isComplete()) {
-            return presponse;
+            if (!presponse.isAllowed()) {
+                return PrivilegesEvaluatorResponse.insufficient(action0, context);
+            } else {
+                return presponse;
+            }
         }
 
         final boolean dnfofEnabled = dcm.isDnfofEnabled();
@@ -868,4 +868,8 @@ private List<String> toString(List<AliasMetadata> aliases) {
 
         return Collections.unmodifiableList(ret);
     }
+
+    public void updatePluginToClusterActions(String pluginIdentifier, Set<String> clusterActions) {
+        pluginToClusterActions.put(pluginIdentifier, clusterActions);
+    }
 }
diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java
@@ -188,6 +188,7 @@ public static PrivilegesEvaluatorResponse partiallyOk(
     }
 
     public static PrivilegesEvaluatorResponse insufficient(String missingPrivilege, PrivilegesEvaluationContext context) {
+        System.out.println("missingPrivilege: " + missingPrivilege);
         PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
         response.indexToActionCheckTable = CheckTable.create(ImmutableSet.of("_"), ImmutableSet.of(missingPrivilege));
         return response;
@@ -197,6 +198,7 @@ public static PrivilegesEvaluatorResponse insufficient(
         CheckTable<String, String> indexToActionCheckTable,
         PrivilegesEvaluationContext context
     ) {
+        System.out.println("indexToActionCheckTable: " + indexToActionCheckTable);
         PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
         response.indexToActionCheckTable = indexToActionCheckTable;
         return response;

diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java
@@ -322,6 +322,7 @@ private void evaluateSystemIndicesAccess(
                     );
                 }
                 presponse.allowed = false;
+                presponse.getMissingPrivileges();
                 presponse.markComplete();
             }
             return;
-Original file line number
+Diff line change
@@ Expand Up / @@ -322,6 +322,7 @@ private void evaluateSystemIndicesAccess( @@
                         );
                     }
                     presponse.allowed = false;
+                    presponse.getMissingPrivileges();
                     presponse.markComplete();
                 }
                 return;
@@ Expand Down @@