Optimized tenant privilege evaluation

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Changed
 - Use extendedPlugins in integrationTest framework for sample resource plugin testing ([#5322](https://github.com/opensearch-project/security/pull/5322))
 - Refactor ResourcePermissions to refer to action groups as access levels ([#5335](https://github.com/opensearch-project/security/pull/5335))
+- Introduced new, performance-optimized implementation for tenant privileges ([#5339](https://github.com/opensearch-project/security/pull/5339))
 
 - Performance improvements: Immutable user object ([#5212])
 

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

@@ -46,7 +46,9 @@
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.security.privileges.DocumentAllowList;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.privileges.PrivilegesInterceptor;
+import org.opensearch.security.privileges.TenantPrivileges;
 import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
 import org.opensearch.security.securityconf.DynamicConfigModel;
 import org.opensearch.security.user.User;
@@ -84,32 +86,6 @@ public PrivilegesInterceptorImpl(
         super(resolver, clusterService, client, threadPool);
     }
 
-    private boolean isTenantAllowed(
-        final ActionRequest request,
-        final String action,
-        final User user,
-        final Map<String, Boolean> tenants,
-        final String requestedTenant
-    ) {
-
-        if (!tenants.keySet().contains(requestedTenant)) {
-            log.warn("Tenant {} is not allowed for user {}", requestedTenant, user.getName());
-            return false;
-        } else {
-
-            if (log.isDebugEnabled()) {
-                log.debug("request " + request.getClass());
-            }
-
-            if (tenants.get(requestedTenant) == Boolean.FALSE && !READ_ONLY_ALLOWED_ACTIONS.contains(action)) {
-                log.warn("Tenant {} is not allowed to write (user: {})", requestedTenant, user.getName());
-                return false;
-            }
-        }
-
-        return true;
-    }
-
     /**
      * return Boolean.TRUE to prematurely deny request
      * return Boolean.FALSE to prematurely allow request
@@ -123,7 +99,8 @@ public ReplaceResult replaceDashboardsIndex(
         final User user,
         final DynamicConfigModel config,
         final Resolved requestedResolved,
-        final Map<String, Boolean> tenants
+        final PrivilegesEvaluationContext context,
+        final TenantPrivileges tenantPrivileges
     ) {
 
         final boolean enabled = config.isDashboardsMultitenancyEnabled();// config.dynamic.kibana.multitenancy_enabled;
@@ -154,20 +131,28 @@ public ReplaceResult replaceDashboardsIndex(
         final boolean dashboardsIndexOnly = !user.getName().equals(dashboardsServerUsername)
             && resolveToDashboardsIndexOrAlias(requestedResolved, dashboardsIndexName);
         final boolean isTraceEnabled = log.isTraceEnabled();
+
+        TenantPrivileges.ActionType actionType = getActionTypeForAction(action);
+
         if (requestedTenant == null || requestedTenant.length() == 0) {
             if (isTraceEnabled) {
                 log.trace("No tenant, will resolve to " + dashboardsIndexName);
             }
 
-            if (dashboardsIndexOnly && !isTenantAllowed(request, action, user, tenants, "global_tenant")) {
+            if (dashboardsIndexOnly && !tenantPrivileges.hasTenantPrivilege(context, "global_tenant", actionType)) {
                 return ACCESS_DENIED_REPLACE_RESULT;
             }
 
             return CONTINUE_EVALUATION_REPLACE_RESULT;
         }
 
-        if (USER_TENANT.equals(requestedTenant)) {
+        boolean isPrivateTenant;
+
+        if (USER_TENANT.equals(requestedTenant) || user.getName().equals(requestedTenant)) {
             requestedTenant = user.getName();
+            isPrivateTenant = true;
+        } else {
+            isPrivateTenant = false;
         }
 
         if (isDebugEnabled && !user.getName().equals(dashboardsServerUsername)) {
@@ -181,7 +166,7 @@ public ReplaceResult replaceDashboardsIndex(
             final String tenantIndexName = toUserIndexName(dashboardsIndexName, requestedTenant);
             if (indices.size() == 1
                 && indices.iterator().next().startsWith(tenantIndexName)
-                && isTenantAllowed(request, action, user, tenants, requestedTenant)) {
+                && (isPrivateTenant || tenantPrivileges.hasTenantPrivilege(context, requestedTenant, actionType))) {
                 return ACCESS_GRANTED_REPLACE_RESULT;
             }
         }
@@ -195,7 +180,7 @@ && isTenantAllowed(request, action, user, tenants, requestedTenant)) {
                 log.debug("is user tenant: " + requestedTenant.equals(user.getName()));
             }
 
-            if (!isTenantAllowed(request, action, user, tenants, requestedTenant)) {
+            if (!isPrivateTenant && !tenantPrivileges.hasTenantPrivilege(context, requestedTenant, actionType)) {
                 return ACCESS_DENIED_REPLACE_RESULT;
             }
 
@@ -238,6 +223,14 @@ private void applyDocumentAllowList(String indexName) {
         documentAllowList.applyTo(threadPool.getThreadContext());
     }
 
+    static TenantPrivileges.ActionType getActionTypeForAction(String action) {
+        if (READ_ONLY_ALLOWED_ACTIONS.contains(action)) {
+            return TenantPrivileges.ActionType.READ;
+        } else {
+            return TenantPrivileges.ActionType.WRITE;
+        }
+    }
+
     private String getConcreteIndexName(String name, Map<String, IndexAbstraction> indicesLookup) {
         for (int i = 1; i < Integer.MAX_VALUE; i++) {
             String concreteName = name.concat("_" + i);

src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java

@@ -32,6 +32,7 @@
 import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
 import org.opensearch.security.dlic.rest.validation.ValidationResult;
 import org.opensearch.security.hasher.PasswordHasher;
+import org.opensearch.security.privileges.PrivilegesEvaluationContext;
 import org.opensearch.security.securityconf.Hashed;
 import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
@@ -127,8 +128,7 @@ private void userAccount(
         final TransportAddress remoteAddress,
         final SecurityDynamicConfiguration<?> configuration
     ) {
-        final var securityRoles = securityApiDependencies.privilegesEvaluator().mapRoles(user, remoteAddress);
-
+        PrivilegesEvaluationContext context = securityApiDependencies.privilegesEvaluator().createContext(user, null);
         ok(
             channel,
             (builder, params) -> builder.startObject()
@@ -139,8 +139,8 @@ private void userAccount(
                 .field("user_requested_tenant", user.getRequestedTenant())
                 .field("backend_roles", user.getRoles())
                 .field("custom_attribute_names", user.getCustomAttributesMap().keySet())
-                .field("tenants", securityApiDependencies.privilegesEvaluator().mapTenants(user, securityRoles))
-                .field("roles", securityRoles)
+                .field("tenants", securityApiDependencies.privilegesEvaluator().tenantPrivileges().tenantMap(context))
+                .field("roles", context.getMappedRoles())
                 .endObject()
         );
     }

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

@@ -93,14 +93,14 @@
 import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
 import org.opensearch.security.securityconf.ConfigModel;
-import org.opensearch.security.securityconf.DynamicConfigFactory;
 import org.opensearch.security.securityconf.DynamicConfigModel;
 import org.opensearch.security.securityconf.FlattenedActionGroups;
 import org.opensearch.security.securityconf.impl.CType;
 import org.opensearch.security.securityconf.impl.DashboardSignInOption;
 import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7;
 import org.opensearch.security.securityconf.impl.v7.RoleV7;
+import org.opensearch.security.securityconf.impl.v7.TenantV7;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.security.user.User;
@@ -157,6 +157,7 @@ public class PrivilegesEvaluator {
     private final Settings settings;
     private final Map<String, Set<String>> pluginToClusterActions;
     private final AtomicReference<ActionPrivileges> actionPrivileges = new AtomicReference<>();
+    private final AtomicReference<TenantPrivileges> tenantPrivileges = new AtomicReference<>();
 
     public PrivilegesEvaluator(
         final ClusterService clusterService,
@@ -200,16 +201,13 @@ public PrivilegesEvaluator(
 
         if (configurationRepository != null) {
             configurationRepository.subscribeOnChange(configMap -> {
-                try {
-                    SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration = configurationRepository.getConfiguration(
-                        CType.ACTIONGROUPS
-                    );
-                    SecurityDynamicConfiguration<RoleV7> rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES);
-
-                    this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration);
-                } catch (Exception e) {
-                    log.error("Error while updating ActionPrivileges object with {}", configMap, e);
-                }
+                SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration = configurationRepository.getConfiguration(
+                    CType.ACTIONGROUPS
+                );
+                SecurityDynamicConfiguration<RoleV7> rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES);
+                SecurityDynamicConfiguration<TenantV7> tenantConfiguration = configurationRepository.getConfiguration(CType.TENANTS);
+
+                this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration, tenantConfiguration);
             });
         }
 
@@ -226,15 +224,15 @@ public PrivilegesEvaluator(
 
     void updateConfiguration(
         SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration,
-        SecurityDynamicConfiguration<RoleV7> rolesConfiguration
+        SecurityDynamicConfiguration<RoleV7> rolesConfiguration,
+        SecurityDynamicConfiguration<TenantV7> tenantConfiguration
     ) {
-        if (rolesConfiguration != null) {
-            SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsWithStatics = actionGroupsConfiguration != null
-                ? DynamicConfigFactory.addStatics(actionGroupsConfiguration.clone())
-                : DynamicConfigFactory.addStatics(SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS));
-            FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsWithStatics);
+        FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsConfiguration.withStaticConfig());
+        rolesConfiguration = rolesConfiguration.withStaticConfig();
+        tenantConfiguration = tenantConfiguration.withStaticConfig();
+        try {
             ActionPrivileges actionPrivileges = new ActionPrivileges(
-                DynamicConfigFactory.addStatics(rolesConfiguration.clone()),
+                rolesConfiguration,
                 flattenedActionGroups,
                 () -> clusterStateSupplier.get().metadata().getIndicesLookup(),
                 settings,
@@ -247,6 +245,14 @@ void updateConfiguration(
             if (oldInstance != null) {
                 oldInstance.shutdown();
             }
+        } catch (Exception e) {
+            log.error("Error while updating ActionPrivileges", e);
+        }
+
+        try {
+            this.tenantPrivileges.set(new TenantPrivileges(rolesConfiguration, tenantConfiguration, flattenedActionGroups));
+        } catch (Exception e) {
+            log.error("Error while updating TenantPrivileges", e);
         }
     }
 
@@ -455,7 +461,8 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
                             user,
                             dcm,
                             requestedResolved,
-                            mapTenants(context)
+                            context,
+                            this.tenantPrivileges.get()
                         );
 
                         if (isDebugEnabled) {
@@ -517,7 +524,8 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
                 user,
                 dcm,
                 requestedResolved,
-                mapTenants(context)
+                context,
+                this.tenantPrivileges.get()
             );
 
             if (isDebugEnabled) {
@@ -594,19 +602,8 @@ public Set<String> mapRoles(final User user, final TransportAddress caller) {
         return this.configModel.mapSecurityRoles(user, caller);
     }
 
-    public Map<String, Boolean> mapTenants(PrivilegesEvaluationContext privilegesEvaluationContext) {
-        return this.configModel.mapTenants(privilegesEvaluationContext);
-    }
-
-    public Map<String, Boolean> mapTenants(User user, Set<String> mappedRoles) {
-        return this.configModel.mapTenants(
-            new PrivilegesEvaluationContext(user, ImmutableSet.copyOf(mappedRoles), null, null, null, irr, resolver, clusterStateSupplier)
-        );
-    }
-
-    public Set<String> getAllConfiguredTenantNames() {
-
-        return configModel.getAllConfiguredTenantNames();
+    public TenantPrivileges tenantPrivileges() {
+        return this.tenantPrivileges.get();
     }
 
     public boolean multitenancyEnabled() {

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

@@ -26,8 +26,6 @@
 
 package org.opensearch.security.privileges;
 
-import java.util.Map;
-
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
@@ -84,7 +82,8 @@ public ReplaceResult replaceDashboardsIndex(
         final User user,
         final DynamicConfigModel config,
         final Resolved requestedResolved,
-        final Map<String, Boolean> tenants
+        final PrivilegesEvaluationContext context,
+        final TenantPrivileges tenantPrivileges
     ) {
         throw new RuntimeException("not implemented");
     }