From 0a8924d901d85aa93c687f1823ae0f97377857a4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 25 Oct 2024 10:28:00 -0400 Subject: [PATCH] Get cluster actions working Signed-off-by: Craig Perkins --- .../plugin/RunClusterHealthAction.java | 2 +- .../security/OpenSearchSecurityPlugin.java | 8 +++- .../security/filter/SecurityFilter.java | 5 +++ .../ContextProvidingPluginSubject.java | 5 ++- .../privileges/PrivilegesEvaluator.java | 44 ++++++++++--------- .../PrivilegesEvaluatorResponse.java | 2 + .../SystemIndexAccessEvaluator.java | 1 + 7 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java b/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java index b819465a35..4234879bb8 100644 --- 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 { 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 index 7680398468..39e0e0a676 100644 --- 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 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 index 3670bf8319..31ea5ac259 100644 --- 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 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 index 7a096abf08..ab6dddceba 100644 --- 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 index 617c14f7a7..a2e863b0b2 100644 --- 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 clusterActions = pluginToClusterActions.get(pluginIdentifier); - if (clusterActions == null) { - clusterActions = new HashSet<>(); - clusterActions.add(BulkAction.NAME); - pluginToClusterActions.put(pluginIdentifier, clusterActions); - SecurityDynamicConfiguration actionGroupsConfiguration = configurationRepository.getConfiguration( - CType.ACTIONGROUPS - ); - SecurityDynamicConfiguration rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES); - - this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration); - } - } - TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); ImmutableSet 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 toString(List aliases) { return Collections.unmodifiableList(ret); } + + public void updatePluginToClusterActions(String pluginIdentifier, Set 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 index 3330219a4f..98840ac5a6 100644 --- 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 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 index d966f1e020..8fc2cd8539 100644 --- 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;