Removed all references to SecurityRoles except DLS/FLS

Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>

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

@@ -39,7 +39,10 @@
 import org.opensearch.core.common.transport.TransportAddress;
 import org.opensearch.core.index.Index;
 import org.opensearch.index.IndexService;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
+import org.opensearch.security.privileges.PrivilegesEvaluatorResponse;
+import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.securityconf.ConfigModel;
 import org.opensearch.security.securityconf.SecurityRoles;
 import org.opensearch.security.support.ConfigConstants;
@@ -163,10 +166,12 @@ protected final boolean isBlockedSystemIndexRequest() {
                 // allow request without user from plugin.
                 return systemIndexMatcher.test(index.getName());
             }
-            final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
-            final Set<String> mappedRoles = evaluator.mapRoles(user, caller);
-            final SecurityRoles securityRoles = evaluator.getSecurityRoles(mappedRoles);
-            return !securityRoles.isPermittedOnSystemIndex(index.getName());
+
+            String permission = ConfigConstants.SYSTEM_INDEX_PERMISSION;
+            PrivilegesEvaluationContext context = evaluator.createContext(user, permission);
+            PrivilegesEvaluatorResponse result = evaluator.getActionPrivileges().hasExplicitIndexPrivilege(context, Set.of(permission), IndexResolverReplacer.Resolved.ofIndex(index.getName()));
+
+            return !result.isAllowed();
         }
         return true;
     }

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

@@ -115,6 +115,10 @@ public PrivilegesEvaluatorResponse hasAnyClusterPrivilege(PrivilegesEvaluationCo
         return cluster.providesAnyPrivilege(context, actions, context.getMappedRoles());
     }
 
+    public PrivilegesEvaluatorResponse hasExplicitClusterPrivilege(PrivilegesEvaluationContext context, String action) {
+        return cluster.providesExplicitPrivilege(context, action, context.getMappedRoles());
+    }
+
     public PrivilegesEvaluatorResponse hasIndexPrivilege(
         PrivilegesEvaluationContext context,
         Set<String> actions,
@@ -191,6 +195,9 @@ static class ClusterPrivileges {
          * Maps names of actions to the roles that provide a privilege for the respective action.
          * Note that the mapping is not comprehensive, additionally the data structures rolesWithWildcardPermissions
          * and rolesToActionMatcher need to be considered for a full view of the privileges.
+         *
+         * This does not include privileges obtained via "*" action patterns. This is both meant as a
+         * optimization and to support explicit privileges.
          */
         private final ImmutableMap<String, ImmutableSet<String>> actionToRoles;
 
@@ -205,6 +212,9 @@ static class ClusterPrivileges {
          * This is only used as a last resort if the test with actionToRole and rolesWithWildcardPermissions failed.
          * This is only necessary for actions which are not contained in the list of "well-known" actions provided
          * during construction.
+         *
+         * This does not include privileges obtained via "*" action patterns. This is both meant as a
+         * optimization and to support explicit privileges.
          */
         private final ImmutableMap<String, WildcardMatcher> rolesToActionMatcher;
 
@@ -224,6 +234,7 @@ static class ClusterPrivileges {
             ImmutableSet<String> wellKnownClusterActions
         ) {
             Map<String, Set<String>> actionToRoles = new HashMap<>();
+            Map<String, Set<String>> explicitPermissionToRoles = new HashMap<>();
             ImmutableSet.Builder<String> rolesWithWildcardPermissions = ImmutableSet.builder();
             ImmutableMap.Builder<String, WildcardMatcher> rolesToActionMatcher = ImmutableMap.builder();
 
@@ -236,13 +247,6 @@ static class ClusterPrivileges {
                     // This list collects all the matchers for action names that will be found for the current role
                     List<WildcardMatcher> wildcardMatchers = new ArrayList<>();
 
-                    // Special case: Roles with a wildcard "*" giving privileges for all actions. We will not resolve
-                    // this stuff, but just note separately that this role just gets all the cluster privileges.
-                    if (permissionPatterns.contains("*")) {
-                        rolesWithWildcardPermissions.add(roleName);
-                        continue;
-                    }
-
                     for (String permission : permissionPatterns) {
                         // If we have a permission which does not use any pattern, we just simply add it to the
                         // "actionToRoles" map.
@@ -252,6 +256,10 @@ static class ClusterPrivileges {
 
                         if (WildcardMatcher.isExact(permission)) {
                             actionToRoles.computeIfAbsent(permission, k -> new HashSet<>()).add(roleName);
+                        } else if (permission.equals("*")) {
+                            // Special case: Roles with a wildcard "*" giving privileges for all actions. We will not resolve
+                            // this stuff, but just note separately that this role just gets all the cluster privileges.
+                            rolesWithWildcardPermissions.add(roleName);
                         } else {
                             WildcardMatcher wildcardMatcher = WildcardMatcher.from(permission);
                             Set<String> matchedActions = wildcardMatcher.getMatchAny(
@@ -316,6 +324,39 @@ PrivilegesEvaluatorResponse providesPrivilege(PrivilegesEvaluationContext contex
             return PrivilegesEvaluatorResponse.insufficient(action, context);
         }
 
+        /**
+         * Checks whether this instance provides explicit privileges for the combination of the provided action and the
+         * provided roles.
+         *
+         * Explicit means here that the privilege is not granted via a "*" action privilege wildcard. Other patterns
+         * are possible. See also: https://github.com/opensearch-project/security/pull/2411 and https://github.com/opensearch-project/security/issues/3038
+         *
+         * Returns a PrivilegesEvaluatorResponse with allowed=true if privileges are available.
+         * Otherwise, allowed will be false and missingPrivileges will contain the name of the given action.
+         */
+        PrivilegesEvaluatorResponse providesExplicitPrivilege(PrivilegesEvaluationContext context, String action, Set<String> roles) {
+
+            // 1: Check well-known actions - this should cover most cases
+            ImmutableSet<String> rolesWithPrivileges = this.actionToRoles.get(action);
+
+            if (rolesWithPrivileges != null && CollectionUtils.containsAny(roles, rolesWithPrivileges)) {
+                return PrivilegesEvaluatorResponse.ok();
+            }
+
+            // 2: Only if everything else fails: Check the matchers in case we have a non-well-known action
+            if (!this.wellKnownClusterActions.contains(action)) {
+                for (String role : roles) {
+                    WildcardMatcher matcher = this.rolesToActionMatcher.get(role);
+
+                    if (matcher != null && matcher.test(action)) {
+                        return PrivilegesEvaluatorResponse.ok();
+                    }
+                }
+            }
+
+            return PrivilegesEvaluatorResponse.insufficient(action, context);
+        }
+
         /**
          * Checks whether this instance provides privileges for the combination of any of the provided actions and the
          * provided roles. Returns a PrivilegesEvaluatorResponse with allowed=true if privileges are available.

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

@@ -253,22 +253,13 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
         this.dcm = dcm;
     }
 
-    ActionPrivileges getActionPrivileges() {
+    public ActionPrivileges getActionPrivileges() {
         return this.actionPrivileges;
     }
 
-    public SecurityRoles getSecurityRoles(Set<String> roles) {
-        return configModel.getSecurityRoles().filter(roles);
-    }
-
-    public boolean hasRestAdminPermissions(final User user, final TransportAddress remoteAddress, final String permissions) {
-        final Set<String> userRoles = mapRoles(user, remoteAddress);
-        return hasRestAdminPermissions(userRoles, permissions);
-    }
-
-    private boolean hasRestAdminPermissions(final Set<String> roles, String permission) {
-        final SecurityRoles securityRoles = getSecurityRoles(roles);
-        return securityRoles.hasExplicitClusterPermissionPermission(permission);
+    public boolean hasRestAdminPermissions(final User user, final TransportAddress remoteAddress, final String permission) {
+        PrivilegesEvaluationContext context = createContext(user, permission);
+        return this.actionPrivileges.hasExplicitClusterPrivilege(context, permission).isAllowed();
     }
 
     public boolean isInitialized() {

src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java

@@ -30,6 +30,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -458,13 +459,20 @@ public final static class Resolved {
             SearchRequest.DEFAULT_INDICES_OPTIONS
         );
 
+
+        private static final IndicesOptions EXACT_INDEX_OPTIONS = new IndicesOptions(
+                EnumSet.of(IndicesOptions.Option.FORBID_ALIASES_TO_MULTIPLE_INDICES),
+                EnumSet.noneOf(IndicesOptions.WildcardStates.class)
+        );
+
         private final Set<String> aliases;
         private final Set<String> allIndices;
         private final Set<String> originalRequested;
         private final Set<String> remoteIndices;
         private final boolean isLocalAll;
         private final IndicesOptions indicesOptions;
 
+
         public Resolved(
             final ImmutableSet<String> aliases,
             final ImmutableSet<String> allIndices,
@@ -563,6 +571,11 @@ public boolean equals(Object obj) {
             } else if (!remoteIndices.equals(other.remoteIndices)) return false;
             return true;
         }
+
+        public static Resolved ofIndex(String index) {
+            ImmutableSet<String> indexSet = ImmutableSet.of(index);
+            return new Resolved(ImmutableSet.of(), indexSet, indexSet, ImmutableSet.of(), EXACT_INDEX_OPTIONS);
+        }
     }
 
     private List<String> renamedIndices(final RestoreSnapshotRequest request, final List<String> filteredIndices) {

src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java

@@ -96,6 +96,8 @@ public void clusterAction_notWellKnown() throws Exception {
                 isForbidden(missingPrivileges("cluster:monitor/nodes/something/else"))
             );
         }
+
+        // TODO explicit and stuff
     }
 
     /**
@@ -173,7 +175,29 @@ public void negative_wrongRole() throws Exception {
             public void negative_wrongAction() throws Exception {
                 PrivilegesEvaluationContext ctx = ctx("test_role");
                 PrivilegesEvaluatorResponse result = subject.hasIndexPrivilege(ctx, otherActions, resolved("index_a11"));
-                assertThat(result, isForbidden(missingPrivileges(otherActions)));
+
+                if (actionSpec.givenPrivs.contains("*")) {
+                    assertThat(result, isAllowed());
+                } else {
+                    assertThat(result, isForbidden(missingPrivileges(otherActions)));
+                }
+            }
+
+            @Test
+            public void positive_hasExplicit_full() {
+                PrivilegesEvaluationContext ctx = ctx("test_role");
+                PrivilegesEvaluatorResponse result = subject.hasExplicitIndexPrivilege(ctx, requiredActions, resolved("index_a11"));
+
+                if (actionSpec.givenPrivs.contains("*")) {
+                    // The * is forbidden for explicit privileges
+                    assertThat(result, isForbidden(missingPrivileges(requiredActions)));
+                } else if (!requiredActions.contains("indices:data/read/search")) {
+                    // For test purposes, we have designated "indices:data/read/search" as a action requiring explicit privileges
+                    // Other actions are not covered here
+                    assertThat(result, isForbidden(missingPrivileges(requiredActions)));
+                } else {
+                    assertThat(result, isAllowed());
+                }
             }
 
             private boolean covers(PrivilegesEvaluationContext ctx, String... indices) {
@@ -198,6 +222,9 @@ public static Collection<Object[]> params() {
                     new IndexSpec().givenIndexPrivs("alias_a1*") //
                 )) {
                     for (ActionSpec actionSpec : Arrays.asList(
+                            new ActionSpec("wildcard")//
+                                    .givenPrivs("*")
+                                    .requiredPrivs("indices:data/read/search"), //
                         new ActionSpec("constant, well known")//
                             .givenPrivs("indices:data/read/search")
                             .requiredPrivs("indices:data/read/search"), //
@@ -239,7 +266,9 @@ public IndicesAndAliases(IndexSpec indexSpec, ActionSpec actionSpec, Statefulnes
                     : ImmutableSet.of("indices:foobar/unknown");
                 this.indexSpec.indexMetadata = INDEX_METADATA;
 
-                this.subject = new ActionPrivileges(roles, FlattenedActionGroups.EMPTY, () -> INDEX_METADATA);
+                this.subject = new ActionPrivileges(roles, FlattenedActionGroups.EMPTY, () -> INDEX_METADATA,  WellKnownActions.CLUSTER_ACTIONS,
+                        WellKnownActions.INDEX_ACTIONS,
+                        WellKnownActions.INDEX_ACTIONS);
 
                 if (statefulness == Statefulness.STATEFUL) {
                     this.subject.updateStatefulIndexPrivileges(INDEX_METADATA);