diff --git a/CHANGELOG.md b/CHANGELOG.md index 74b103ff1f..38b9c93095 100644 --- a/CHANGELOG.md +++ b/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]) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/TenantPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/TenantPrivilegesTest.java new file mode 100644 index 0000000000..6c549a911d --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/privileges/TenantPrivilegesTest.java @@ -0,0 +1,416 @@ +/* + * 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 java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.fasterxml.jackson.core.JsonProcessingException; +import org.apache.commons.io.IOUtils; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Suite; + +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.security.securityconf.DynamicConfigFactory; +import org.opensearch.security.securityconf.FlattenedActionGroups; +import org.opensearch.security.securityconf.impl.CType; +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.user.User; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Unit tests for the class TenantPrivileges + */ +@RunWith(Suite.class) +@Suite.SuiteClasses({ TenantPrivilegesTest.ParameterizedByRoleConfiguration.class, TenantPrivilegesTest.Misc.class }) +public class TenantPrivilegesTest { + + /** + * A parameterized test class for the method TenantPrivileges.hasTenantPrivileges(). Tests the return value based + * on different kinds of roles.yml configurations. + */ + @RunWith(Parameterized.class) + public static class ParameterizedByRoleConfiguration { + TenantPrivilegeSpec spec; + TenantPrivileges subject; + + @Test + public void positive_read() throws Exception { + boolean result = subject.hasTenantPrivilege(ctx("test_role"), "tenant_a1", TenantPrivileges.ActionType.READ); + + if (spec.isEmpty() || spec.isInvalid()) { + assertFalse(result); + } else { + assertTrue(result); + } + } + + @Test + public void write() throws Exception { + boolean result = subject.hasTenantPrivilege(ctx("test_role"), "tenant_a1", TenantPrivileges.ActionType.WRITE); + if (spec.isWrite() && !spec.isInvalid()) { + assertTrue(result); + } else { + assertFalse(result); + } + } + + @Test + public void negative_wrongRole() throws Exception { + assertFalse(subject.hasTenantPrivilege(ctx("other_role"), "tenant_a1", TenantPrivileges.ActionType.READ)); + } + + @Test + public void negative_wrongTenant() throws Exception { + assertFalse(subject.hasTenantPrivilege(ctx("test_role"), "tenant_a3", TenantPrivileges.ActionType.READ)); + } + + @Test + public void wrongTenantByUserAttr() throws Exception { + boolean result = subject.hasTenantPrivilege( + ctxWithDifferentUserAttr("test_role"), + "tenant_a1", + TenantPrivileges.ActionType.READ + ); + + if (spec.tenantsContainUserAttr() || spec.isEmpty() || spec.isInvalid()) { + assertFalse(result); + } else { + assertTrue(result); + } + } + + @Parameterized.Parameters(name = "{0}") + public static Collection params() { + List result = new ArrayList<>(); + + result.add( + new Object[] { + new TenantPrivilegeSpec("constant tenant; write").tenantPatterns("tenant_a1") + .allowedActions("kibana:saved_objects/*/write") } + ); + result.add( + new Object[] { + new TenantPrivilegeSpec("constant tenant; read").tenantPatterns("tenant_a1") + .allowedActions("kibana:saved_objects/*/read") } + ); + result.add( + new Object[] { + new TenantPrivilegeSpec("tenant pattern; write").tenantPatterns("tenant_a*") + .allowedActions("kibana:saved_objects/*/write") } + ); + result.add( + new Object[] { + new TenantPrivilegeSpec("tenant pattern; read").tenantPatterns("tenant_a*") + .allowedActions("kibana:saved_objects/*/read") } + ); + result.add( + new Object[] { + new TenantPrivilegeSpec("tenant w/ user attribute; write").tenantPatterns("tenant_${attrs.dept_no}") + .allowedActions("kibana:saved_objects/*/write") } + ); + result.add( + new Object[] { + new TenantPrivilegeSpec("tenant w/ user attribute; read").tenantPatterns("tenant_${attrs.dept_no}") + .allowedActions("kibana:saved_objects/*/read") } + ); + result.add(new Object[] { new TenantPrivilegeSpec("no tenant config") }); + result.add( + new Object[] { + new TenantPrivilegeSpec("invalid tenant pattern").tenantPatterns("/.*|{/") + .allowedActions("kibana:saved_objects/*/write") } + ); + + return result; + } + + public ParameterizedByRoleConfiguration(TenantPrivilegeSpec spec) throws Exception { + this.spec = spec; + SecurityDynamicConfiguration roles = spec.toRolesConfig(); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + tenant_a2: {} + tenant_b1: {} + tenant_b2: {} + """, CType.TENANTS); + this.subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + } + + public static class TenantPrivilegeSpec { + String description; + List tenantPatterns = new ArrayList<>(); + List allowedActions = new ArrayList<>(); + + TenantPrivilegeSpec(String description) { + this.description = description; + } + + TenantPrivilegeSpec tenantPatterns(String... tenantPatterns) { + this.tenantPatterns = Arrays.asList(tenantPatterns); + return this; + } + + TenantPrivilegeSpec allowedActions(String... allowedActions) { + this.allowedActions = Arrays.asList(allowedActions); + return this; + } + + boolean tenantsContainUserAttr() { + return this.tenantPatterns.stream().anyMatch(t -> t.contains("${")); + } + + boolean isWrite() { + return this.allowedActions.contains("kibana:saved_objects/*/write"); + } + + boolean isEmpty() { + return this.allowedActions.isEmpty() && this.tenantPatterns.isEmpty(); + } + + boolean isInvalid() { + return description.contains("invalid"); + } + + SecurityDynamicConfiguration toRolesConfig() { + try { + if (!isEmpty()) { + return SecurityDynamicConfiguration.fromMap( + ImmutableMap.of( + "test_role", + ImmutableMap.of( + "tenant_permissions", + List.of(ImmutableMap.of("tenant_patterns", this.tenantPatterns, "allowed_actions", this.allowedActions)) + ) + ), + CType.ROLES + ); + } else { + return SecurityDynamicConfiguration.fromMap( + ImmutableMap.of("test_role", ImmutableMap.of("description", "empty role")), + CType.ROLES + ); + } + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + @Override + public String toString() { + return description; + } + } + + } + + public static class Misc { + @Test + public void tenantMap() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml(""" + test_role: + tenant_permissions: + - tenant_patterns: + - tenant_a* + allowed_actions: + - "kibana:saved_objects/*/read" + - tenant_patterns: + - tenant_a1 + allowed_actions: + - "kibana:saved_objects/*/write" + """, CType.ROLES); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + tenant_a2: {} + tenant_b1: {} + tenant_b2: {} + """, CType.TENANTS); + + TenantPrivileges subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + assertEquals(Map.of("test_user", true, "tenant_a1", true, "tenant_a2", false), subject.tenantMap(ctx("test_role"))); + } + + @Test + public void allTenantNames() throws Exception { + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + tenant_a2: {} + tenant_b1: {} + tenant_b2: {} + """, CType.TENANTS); + + TenantPrivileges subject = new TenantPrivileges( + SecurityDynamicConfiguration.empty(CType.ROLES), + tenants, + FlattenedActionGroups.EMPTY + ); + assertEquals(Set.of("tenant_a1", "tenant_a2", "tenant_b1", "tenant_b2"), subject.allTenantNames()); + } + + @Test + public void allAccess() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml( + testResource("/static_config/static_roles.yml"), + CType.ROLES + ); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml( + testResource("/static_config/static_tenants.yml"), + CType.TENANTS + ); + SecurityDynamicConfiguration actionGroups = SecurityDynamicConfiguration.fromYaml( + testResource("/static_config/static_action_groups.yml"), + CType.ACTIONGROUPS + ); + + TenantPrivileges subject = new TenantPrivileges(roles, tenants, new FlattenedActionGroups(actionGroups)); + assertTrue(subject.hasTenantPrivilege(ctx("all_access"), "global_tenant", TenantPrivileges.ActionType.WRITE)); + } + + @Test + public void invalidDynamicTenantPattern() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml(""" + test_role: + tenant_permissions: + - tenant_patterns: + - "/${user.roles}a{/" + allowed_actions: + - "kibana:saved_objects/*/read" + """, CType.ROLES); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + """, CType.TENANTS); + + TenantPrivileges subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + assertFalse(subject.hasTenantPrivilege(ctx("test_role"), "tenant_a1", TenantPrivileges.ActionType.READ)); + } + + /** + * This tests legacy behavior which should be removed during the next major release; + * see https://github.com/opensearch-project/security/issues/5356 + */ + @Test + public void implicitGlobalTenantAccessGrantedByKibanaUserRole_granted() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml(""" + test_role: {} + """, CType.ROLES); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + global_tenant: {} + """, CType.TENANTS); + + TenantPrivileges subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + + assertTrue(subject.hasTenantPrivilege(ctx("kibana_user"), "global_tenant", TenantPrivileges.ActionType.WRITE)); + assertTrue(subject.hasTenantPrivilege(ctx("kibana_user"), "global_tenant", TenantPrivileges.ActionType.READ)); + + assertFalse(subject.hasTenantPrivilege(ctx("not_kibana_user"), "global_tenant", TenantPrivileges.ActionType.WRITE)); + assertFalse(subject.hasTenantPrivilege(ctx("not_kibana_user"), "global_tenant", TenantPrivileges.ActionType.READ)); + + roles = SecurityDynamicConfiguration.fromYaml(""" + test_role: + tenant_permissions: + - tenant_patterns: + - "*" + allowed_actions: + - "kibana:saved_objects/*/read" + """, CType.ROLES); + + subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + + assertTrue(subject.hasTenantPrivilege(ctx("kibana_user"), "global_tenant", TenantPrivileges.ActionType.WRITE)); + assertTrue(subject.hasTenantPrivilege(ctx("kibana_user"), "global_tenant", TenantPrivileges.ActionType.READ)); + + } + + /** + * This tests legacy behavior which should be removed during the next major release; + * see https://github.com/opensearch-project/security/issues/5356 + */ + @Test + public void implicitGlobalTenantAccessGrantedByKibanaUserRole_notGranted() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml(""" + test_role: + tenant_permissions: + - tenant_patterns: + - "*" + allowed_actions: + - "kibana:saved_objects/*/read" + """, CType.ROLES); + SecurityDynamicConfiguration tenants = SecurityDynamicConfiguration.fromYaml(""" + tenant_a1: {} + global_tenant: {} + """, CType.TENANTS); + + TenantPrivileges subject = new TenantPrivileges(roles, tenants, FlattenedActionGroups.EMPTY); + + assertFalse(subject.hasTenantPrivilege(ctx("test_role", "kibana_user"), "global_tenant", TenantPrivileges.ActionType.WRITE)); + assertTrue(subject.hasTenantPrivilege(ctx("test_role", "kibana_user"), "global_tenant", TenantPrivileges.ActionType.READ)); + } + + } + + static PrivilegesEvaluationContext ctx(String... roles) { + User user = new User("test_user").withAttributes(ImmutableMap.of("attrs.dept_no", "a1")); + return new PrivilegesEvaluationContext( + user, + ImmutableSet.copyOf(roles), + null, + null, + null, + null, + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + null + ); + } + + static PrivilegesEvaluationContext ctxWithDifferentUserAttr(String... roles) { + User user = new User("test_user").withAttributes(ImmutableMap.of("attrs.dept_no", "a10")); + return new PrivilegesEvaluationContext( + user, + ImmutableSet.copyOf(roles), + null, + null, + null, + null, + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + null + ); + } + + static String testResource(String fileName) throws IOException { + InputStream in = DynamicConfigFactory.class.getResourceAsStream(fileName); + + if (in == null) { + throw new FileNotFoundException("could not find " + fileName); + } + + return IOUtils.toString(in, StandardCharsets.UTF_8); + } +} diff --git a/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java b/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java index 8d71041106..6a64eada3e 100644 --- a/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java +++ b/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 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 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 indicesLookup) { for (int i = 1; i < Integer.MAX_VALUE; i++) { String concreteName = name.concat("_" + i); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java index 8f473ff3d2..c6a01ecad9 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java +++ b/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() ); } diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 8b0e28d883..0ddbee349c 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -93,7 +93,6 @@ 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; @@ -101,6 +100,7 @@ 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> pluginToClusterActions; private final AtomicReference actionPrivileges = new AtomicReference<>(); + private final AtomicReference tenantPrivileges = new AtomicReference<>(); public PrivilegesEvaluator( final ClusterService clusterService, @@ -200,16 +201,13 @@ public PrivilegesEvaluator( if (configurationRepository != null) { configurationRepository.subscribeOnChange(configMap -> { - try { - SecurityDynamicConfiguration actionGroupsConfiguration = configurationRepository.getConfiguration( - CType.ACTIONGROUPS - ); - SecurityDynamicConfiguration rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES); - - this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration); - } catch (Exception e) { - log.error("Error while updating ActionPrivileges object with {}", configMap, e); - } + SecurityDynamicConfiguration actionGroupsConfiguration = configurationRepository.getConfiguration( + CType.ACTIONGROUPS + ); + SecurityDynamicConfiguration rolesConfiguration = configurationRepository.getConfiguration(CType.ROLES); + SecurityDynamicConfiguration tenantConfiguration = configurationRepository.getConfiguration(CType.TENANTS); + + this.updateConfiguration(actionGroupsConfiguration, rolesConfiguration, tenantConfiguration); }); } @@ -226,15 +224,15 @@ public PrivilegesEvaluator( void updateConfiguration( SecurityDynamicConfiguration actionGroupsConfiguration, - SecurityDynamicConfiguration rolesConfiguration + SecurityDynamicConfiguration rolesConfiguration, + SecurityDynamicConfiguration tenantConfiguration ) { - if (rolesConfiguration != null) { - SecurityDynamicConfiguration 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 mapRoles(final User user, final TransportAddress caller) { return this.configModel.mapSecurityRoles(user, caller); } - public Map mapTenants(PrivilegesEvaluationContext privilegesEvaluationContext) { - return this.configModel.mapTenants(privilegesEvaluationContext); - } - - public Map mapTenants(User user, Set mappedRoles) { - return this.configModel.mapTenants( - new PrivilegesEvaluationContext(user, ImmutableSet.copyOf(mappedRoles), null, null, null, irr, resolver, clusterStateSupplier) - ); - } - - public Set getAllConfiguredTenantNames() { - - return configModel.getAllConfiguredTenantNames(); + public TenantPrivileges tenantPrivileges() { + return this.tenantPrivileges.get(); } public boolean multitenancyEnabled() { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesInterceptor.java b/src/main/java/org/opensearch/security/privileges/PrivilegesInterceptor.java index 750c7d2b41..0ae809bc9d 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesInterceptor.java +++ b/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 tenants + final PrivilegesEvaluationContext context, + final TenantPrivileges tenantPrivileges ) { throw new RuntimeException("not implemented"); } diff --git a/src/main/java/org/opensearch/security/privileges/TenantPrivileges.java b/src/main/java/org/opensearch/security/privileges/TenantPrivileges.java new file mode 100644 index 0000000000..2395ba548e --- /dev/null +++ b/src/main/java/org/opensearch/security/privileges/TenantPrivileges.java @@ -0,0 +1,281 @@ +/* + * 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 java.util.Collection; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.security.securityconf.FlattenedActionGroups; +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.securityconf.impl.v7.TenantV7; +import org.opensearch.security.support.WildcardMatcher; + +import com.selectivem.collections.DeduplicatingCompactSubSetBuilder; +import com.selectivem.collections.ImmutableCompactSubSet; + +/** + * Container class for pre-processed tenant privileges. Instances of this class are immutable. + * New instances are created when the role or when the tenant configuration changes. The creation is managed + * by the class PrivilegesEvaluator. + *

+ * For tenant privileges that do not use user attributes, this class provides O(1) complexity for tenant privilege + * evaluation. + */ +public class TenantPrivileges { + + /** + * Specifies whether an action is just reading data from a tenant or whether it is writing data to a tenant. + * Used by the hasTenantPrivileges() API. + */ + public enum ActionType { + READ, + WRITE; + } + + public static final TenantPrivileges EMPTY = new TenantPrivileges( + SecurityDynamicConfiguration.empty(CType.ROLES), + SecurityDynamicConfiguration.empty(CType.TENANTS), + FlattenedActionGroups.EMPTY + ); + + private static final List READ = ImmutableList.of(ActionType.READ); + private static final List READ_WRITE = ImmutableList.of(ActionType.READ, ActionType.WRITE); + + private static final Logger log = LogManager.getLogger(TenantPrivileges.class); + + /** + * Stores all names of tenants, as represented in the tenants.yml configuration. This is independent of the role config. + */ + private final ImmutableSet allTenantNames; + + /** + * A map from tenant name to ActionType to a set of roles which provide privileges for the respective ActionType. + * The structure is optimized so that no temporary filtered objects need to be created during privileges evaluation. + */ + private final ImmutableMap>> tenantToActionTypeToRoles; + + /** + * A map from role name to ActionType to strings with dynamic tenant patterns (i.e., tenant patterns that contain + * user attributes like ${user.name}). This is only used for configurations that use dynamic tenant patterns. + */ + private final ImmutableMap>> rolesToActionTypeToDynamicTenantPattern; + + public TenantPrivileges( + SecurityDynamicConfiguration roles, + SecurityDynamicConfiguration definedTenants, + FlattenedActionGroups actionGroups + ) { + this.allTenantNames = ImmutableSet.copyOf(definedTenants.getCEntries().keySet()); + + Map roleEntries = roles.getCEntries(); + + DeduplicatingCompactSubSetBuilder roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>(roleEntries.keySet()); + Map>> tenantToActionTypeToRoles = new HashMap<>(); + Map>> rolesToActionTypeToDynamicTenantPattern = new HashMap<>(); + + for (Map.Entry entry : roleEntries.entrySet()) { + try { + String roleName = entry.getKey(); + RoleV7 role = entry.getValue(); + + roleSetBuilder.next(roleName); + + for (RoleV7.Tenant tenantPermissions : role.getTenant_permissions()) { + List actionTypes = resolveActionType(tenantPermissions.getAllowed_actions(), actionGroups); + for (String tenantPattern : tenantPermissions.getTenant_patterns()) { + if (isDynamic(tenantPattern)) { + // If a tenant pattern contains a user attribute (like ${user.name}), we can only + // do the tenant pattern matching during the actual tenant privilege evaluation, when the user is known. + // Thus, we just keep these patterns here unprocessed + for (ActionType actionType : actionTypes) { + rolesToActionTypeToDynamicTenantPattern.computeIfAbsent(roleName, (k) -> new EnumMap<>(ActionType.class)) + .computeIfAbsent(actionType, (k) -> new HashSet<>()) + .add(tenantPattern); + } + } else { + // If a tenant pattern contains no user attribute, we can do all the pattern matching on + // tenant names in advance + for (String tenant : WildcardMatcher.from(tenantPattern).iterateMatching(this.allTenantNames)) { + for (ActionType actionType : actionTypes) { + tenantToActionTypeToRoles.computeIfAbsent(tenant, (k) -> new EnumMap<>(ActionType.class)) + .computeIfAbsent(actionType, (k) -> roleSetBuilder.createSubSetBuilder()) + .add(roleName); + } + } + } + } + } + } catch (Exception e) { + log.error("Unexpected exception while processing role: {}\nIgnoring role.", entry.getKey(), e); + } + } + + DeduplicatingCompactSubSetBuilder.Completed completedRoleSetBuilder = roleSetBuilder.build(); + + this.tenantToActionTypeToRoles = tenantToActionTypeToRoles.entrySet() + .stream() + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, entry -> build(entry.getValue(), completedRoleSetBuilder))); + + this.rolesToActionTypeToDynamicTenantPattern = rolesToActionTypeToDynamicTenantPattern.entrySet() + .stream() + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, entry -> buildDynamicPatternMap(entry.getValue()))); + } + + /** + * Checks whether the user identified by the given context has privileges to perform actions of the given actionType + * on the given tenant. Returns true if the user has privileges, false otherwise. + */ + public boolean hasTenantPrivilege(PrivilegesEvaluationContext context, String tenant, ActionType actionType) { + // First check the non-dynamic tenant permission configurations; that's the fast path + Map> actionTypeToRoles = this.tenantToActionTypeToRoles.get(tenant); + if (actionTypeToRoles != null) { + ImmutableCompactSubSet roles = actionTypeToRoles.get(actionType); + + if (roles != null && roles.containsAny(context.getMappedRoles())) { + return true; + } + } + + // If we did not find anything with the non-dynamic tenant permission, we will check the dynamic ones + // First, however, we check if that tenant is valid at all. If not, we abort early. + if (!this.allTenantNames.contains(tenant)) { + return false; + } + + // Now check the dynamic tenant names + for (String role : context.getMappedRoles()) { + ImmutableMap> actionTypeToDynamicTenantPattern = this.rolesToActionTypeToDynamicTenantPattern + .get(role); + if (actionTypeToDynamicTenantPattern == null) { + continue; + } + + ImmutableList dynamicTenantPatterns = actionTypeToDynamicTenantPattern.get(actionType); + if (dynamicTenantPatterns == null) { + continue; + } + + for (String dynamicTenantPattern : dynamicTenantPatterns) { + try { + if (context.getRenderedMatcher(dynamicTenantPattern).test(tenant)) { + return true; + } + } catch (Exception e) { + log.error( + "Error while evaluating dynamic tenant pattern {} of role {}. Ignoring pattern.", + dynamicTenantPattern, + role, + e + ); + } + } + } + + // The following code block exists only for legacy reasons; it carries over a weird logic from ConfigModelV7: + // https://github.com/opensearch-project/security/blob/344673a455de956f6a8f3217e61d0636b46a3527/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java#L230-L232 + // This gives users r/w access to the global tenant if they do not have explicitly configured access to it. + // As this is surprising and undocumented behavior, it should be removed; possibly, in the next major release + // of OpenSearch; see https://github.com/opensearch-project/security/issues/5356 + if ("global_tenant".equals(tenant) && context.getMappedRoles().contains("kibana_user")) { + if (actionTypeToRoles == null) { + return true; + } + + ImmutableCompactSubSet readRoles = actionTypeToRoles.get(ActionType.READ); + if (readRoles == null || !readRoles.containsAny(context.getMappedRoles())) { + return true; + } + } + + return false; + } + + /** + * Returns all tenant names, as configured in the tenants.yml config + */ + public ImmutableSet allTenantNames() { + return this.allTenantNames; + } + + /** + * Builds an old-style map which lists all tenants a user has access to and sets the value to + * true if the user has read/write access. If a user has read only access, the value will be false. + *

+ * Note: This only exists to provide backwards compatibility; the way the information is presented + * here is less than optimal, as it is not self-explanatory. + */ + public Map tenantMap(PrivilegesEvaluationContext context) { + HashMap result = new HashMap<>(); + + for (String tenant : this.allTenantNames) { + if (hasTenantPrivilege(context, tenant, ActionType.WRITE)) { + result.put(tenant, true); + } else if (hasTenantPrivilege(context, tenant, ActionType.READ)) { + result.put(tenant, false); + } + } + + // Additionally, the private tenant is represented in the result map as the user's name. + // This is also not ideal, but this is also for backwards compatibility. + result.put(context.getUser().getName(), true); + + return result; + } + + static List resolveActionType(Collection allowedActions, FlattenedActionGroups actionGroups) { + ImmutableSet permissions = actionGroups.resolve(allowedActions); + if (permissions.contains("kibana:saved_objects/*/write")) { + return READ_WRITE; + } else { + return READ; + } + } + + static boolean isDynamic(String tenantPattern) { + return tenantPattern.contains("${"); + } + + private static ImmutableMap> build( + Map> source, + DeduplicatingCompactSubSetBuilder.Completed completedRoleSetBuilder + ) { + EnumMap> result = new EnumMap<>(ActionType.class); + + for (Map.Entry> sourceEntry : source.entrySet()) { + result.put(sourceEntry.getKey(), sourceEntry.getValue().build(completedRoleSetBuilder)); + } + + return ImmutableMap.copyOf(result); + } + + private static ImmutableMap> buildDynamicPatternMap(Map> source) { + EnumMap> result = new EnumMap<>(ActionType.class); + + for (Map.Entry> sourceEntry : source.entrySet()) { + result.put(sourceEntry.getKey(), ImmutableList.copyOf(sourceEntry.getValue())); + } + + return ImmutableMap.copyOf(result); + } +} diff --git a/src/main/java/org/opensearch/security/rest/SecurityInfoAction.java b/src/main/java/org/opensearch/security/rest/SecurityInfoAction.java index 392e4a38ed..41b8cc98be 100644 --- a/src/main/java/org/opensearch/security/rest/SecurityInfoAction.java +++ b/src/main/java/org/opensearch/security/rest/SecurityInfoAction.java @@ -32,7 +32,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Set; import com.google.common.collect.ImmutableList; import org.apache.logging.log4j.LogManager; @@ -49,6 +48,7 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestController; import org.opensearch.rest.RestRequest; +import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.support.Base64Helper; import org.opensearch.security.support.ConfigConstants; @@ -122,7 +122,7 @@ public void accept(RestChannel channel) throws Exception { final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); final TransportAddress remoteAddress = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); - final Set securityRoles = evaluator.mapRoles(user, remoteAddress); + PrivilegesEvaluationContext context = evaluator.createContext(user, null); builder.startObject(); builder.field("user", user == null ? null : user.toString()); @@ -131,8 +131,8 @@ public void accept(RestChannel channel) throws Exception { builder.field("remote_address", remoteAddress); builder.field("backend_roles", user == null ? null : user.getRoles()); builder.field("custom_attribute_names", user == null ? null : user.getCustomAttributesMap().keySet()); - builder.field("roles", securityRoles); - builder.field("tenants", evaluator.mapTenants(user, securityRoles)); + builder.field("roles", context.getMappedRoles()); + builder.field("tenants", evaluator.tenantPrivileges().tenantMap(context)); builder.field("principal", (String) threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL)); builder.field("peer_certificates", certs != null && certs.length > 0 ? certs.length + "" : "0"); builder.field("sso_logout_url", (String) threadContext.getTransient(ConfigConstants.SSO_LOGOUT_URL)); diff --git a/src/main/java/org/opensearch/security/rest/TenantInfoAction.java b/src/main/java/org/opensearch/security/rest/TenantInfoAction.java index b2c0566e3d..47c4b61cc2 100644 --- a/src/main/java/org/opensearch/security/rest/TenantInfoAction.java +++ b/src/main/java/org/opensearch/security/rest/TenantInfoAction.java @@ -215,7 +215,7 @@ private String tenantNameForIndex(String index) { final int expectedHash = Integer.parseInt(indexParts[1]); final String sanitizedName = indexParts[2]; - for (String tenant : evaluator.getAllConfiguredTenantNames()) { + for (String tenant : evaluator.tenantPrivileges().allTenantNames()) { if (tenant.hashCode() == expectedHash && sanitizedName.equals(tenant.toLowerCase().replaceAll("[^a-z0-9]+", ""))) { return tenant; } diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModel.java b/src/main/java/org/opensearch/security/securityconf/ConfigModel.java index 89b0a38364..a1546de0f4 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModel.java @@ -26,18 +26,11 @@ package org.opensearch.security.securityconf; -import java.util.Map; import java.util.Set; import org.opensearch.core.common.transport.TransportAddress; -import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.user.User; public abstract class ConfigModel { - - public abstract Map mapTenants(PrivilegesEvaluationContext privilegesEvaluationContext); - public abstract Set mapSecurityRoles(User user, TransportAddress caller); - - public abstract Set getAllConfiguredTenantNames(); } diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index c31b062bd9..e811a267a8 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -18,40 +18,21 @@ package org.opensearch.security.securityconf; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.TreeSet; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.regex.Pattern; -import java.util.stream.Collectors; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; -import com.google.common.collect.MultimapBuilder.SetMultimapBuilder; -import com.google.common.collect.SetMultimap; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.ExceptionsHelper; -import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.transport.TransportAddress; -import org.opensearch.security.privileges.PrivilegesEvaluationContext; -import org.opensearch.security.privileges.UserAttributes; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; -import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; import org.opensearch.security.securityconf.impl.v7.RoleMappingsV7; 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.HostResolverMode; import org.opensearch.security.support.WildcardMatcher; @@ -61,23 +42,17 @@ public class ConfigModelV7 extends ConfigModel { protected final Logger log = LogManager.getLogger(this.getClass()); private ConfigConstants.RolesMappingResolution rolesMappingResolution; - private FlattenedActionGroups actionGroups; - private TenantHolder tenantHolder; private RoleMappingHolder roleMappingHolder; private SecurityDynamicConfiguration roles; - private SecurityDynamicConfiguration tenants; public ConfigModelV7( SecurityDynamicConfiguration roles, SecurityDynamicConfiguration rolemappings, - SecurityDynamicConfiguration actiongroups, - SecurityDynamicConfiguration tenants, DynamicConfigModel dcm, Settings opensearchSettings ) { this.roles = roles; - this.tenants = tenants; try { rolesMappingResolution = ConfigConstants.RolesMappingResolution.valueOf( @@ -91,154 +66,9 @@ public ConfigModelV7( rolesMappingResolution = ConfigConstants.RolesMappingResolution.MAPPING_ONLY; } - actionGroups = actiongroups != null ? new FlattenedActionGroups(actiongroups) : FlattenedActionGroups.EMPTY; - tenantHolder = new TenantHolder(roles, tenants); roleMappingHolder = new RoleMappingHolder(rolemappings, dcm.getHostsResolverMode()); } - public Set getAllConfiguredTenantNames() { - return Collections.unmodifiableSet(tenants.getCEntries().keySet()); - } - - private class TenantHolder { - - private SetMultimap> tenantsMM = null; - - public TenantHolder(SecurityDynamicConfiguration roles, SecurityDynamicConfiguration definedTenants) { - final Set>>>> futures = new HashSet<>(roles.getCEntries().size()); - - final ExecutorService execs = Executors.newFixedThreadPool(10); - - for (Entry securityRole : roles.getCEntries().entrySet()) { - - if (securityRole.getValue() == null) { - continue; - } - - Future>>> future = execs.submit( - new Callable>>>() { - @Override - public Tuple>> call() throws Exception { - final Set> tuples = new HashSet<>(); - final List tenants = securityRole.getValue().getTenant_permissions(); - if (tenants != null) { - - for (RoleV7.Tenant tenant : tenants) { - - // find Wildcarded tenant patterns - List matchingTenants = WildcardMatcher.from(tenant.getTenant_patterns()) - .getMatchAny(definedTenants.getCEntries().keySet(), Collectors.toList()); - for (String matchingTenant : matchingTenants) { - tuples.add( - new Tuple( - matchingTenant, - actionGroups.resolve(tenant.getAllowed_actions()).contains("kibana:saved_objects/*/write") - ) - ); - } - // find parameter substitution specified tenant - Pattern parameterPattern = Pattern.compile("^\\$\\{attr"); - List matchingParameterTenantList = tenant.getTenant_patterns() - .stream() - .filter(parameterPattern.asPredicate()) - .collect(Collectors.toList()); - for (String matchingParameterTenant : matchingParameterTenantList) { - tuples.add( - new Tuple( - matchingParameterTenant, - actionGroups.resolve(tenant.getAllowed_actions()).contains("kibana:saved_objects/*/write") - ) - ); - } - } - } - - return new Tuple>>(securityRole.getKey(), tuples); - } - } - ); - - futures.add(future); - - } - - execs.shutdown(); - try { - execs.awaitTermination(30, TimeUnit.SECONDS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - log.error("Thread interrupted (1) while loading roles"); - return; - } - - try { - final SetMultimap> tenantsMM_ = SetMultimapBuilder.hashKeys(futures.size()) - .hashSetValues(16) - .build(); - - for (Future>>> future : futures) { - Tuple>> result = future.get(); - tenantsMM_.putAll(result.v1(), result.v2()); - } - - tenantsMM = tenantsMM_; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - log.error("Thread interrupted (2) while loading roles"); - return; - } catch (ExecutionException e) { - log.error("Error while updating roles: {}", e.getCause(), e.getCause()); - throw ExceptionsHelper.convertToOpenSearchException(e); - } - - } - - public Map mapTenants(PrivilegesEvaluationContext privilegesEvaluationContext) { - - if (privilegesEvaluationContext == null || tenantsMM == null) { - return Collections.emptyMap(); - } - - User user = privilegesEvaluationContext.getUser(); - Set roles = privilegesEvaluationContext.getMappedRoles(); - - final Map result = new HashMap<>(roles.size()); - result.put(user.getName(), true); - - tenantsMM.entries() - .stream() - .filter(e -> roles.contains(e.getKey())) - .filter(e -> !user.getName().equals(e.getValue().v1())) - .forEach(e -> { - - // replaceProperties for tenant name because - // at this point e.getValue().v1() can be in this form : "${attr.[internal|jwt|proxy|ldap].*}" - // let's substitute it with the eventual value of the user's attribute - final String tenant = UserAttributes.replaceProperties(e.getValue().v1(), privilegesEvaluationContext); - final boolean rw = e.getValue().v2(); - - if (rw || !result.containsKey(tenant)) { // RW outperforms RO - - // We want to make sure that we add a tenant that exists - // Indeed, because we don't have control over what will be - // passed on as values of users' attributes, we have to make - // sure that we don't allow them to select tenants that do not exist. - if (ConfigModelV7.this.tenants.getCEntries().containsKey(tenant)) { - result.put(tenant, rw); - } - } - }); - - Set _roles = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - _roles.addAll(roles); - if (!result.containsKey("global_tenant") && (_roles.contains("kibana_user") || _roles.contains("all_access"))) { - result.put("global_tenant", true); - } - - return Collections.unmodifiableMap(result); - } - } - private class RoleMappingHolder { private ListMultimap users; @@ -357,11 +187,6 @@ private Set map(final User user, final TransportAddress caller) { } } - @Override - public Map mapTenants(PrivilegesEvaluationContext privilegesEvaluationContext) { - return tenantHolder.mapTenants(privilegesEvaluationContext); - } - public Set mapSecurityRoles(User user, TransportAddress caller) { return roleMappingHolder.map(user, caller); } diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java index 0f5b5dea65..1a871908e0 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java @@ -261,7 +261,7 @@ public void onChange(ConfigurationMap typeToConfig) { // rebuild v7 Models dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.cih); ium = new InternalUsersModelV7(internalusers, roles, rolesmapping); - cm = new ConfigModelV7(roles, rolesmapping, actionGroups, tenants, dcm, opensearchSettings); + cm = new ConfigModelV7(roles, rolesmapping, dcm, opensearchSettings); // notify subscribers eventBus.post(cm); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java index 790f39d12b..220c69609e 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -46,6 +46,7 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.Hashed; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -413,4 +414,16 @@ public boolean isReserved(final String resourceName) { return o instanceof Hideable && ((Hideable) o).isReserved(); } + @JsonIgnore + public boolean isEmpty() { + return centries.isEmpty(); + } + + /** + * Returns a shallow copy of this configuration which additionally contains the static configuration. + */ + @JsonIgnore + public SecurityDynamicConfiguration withStaticConfig() { + return DynamicConfigFactory.addStatics(this.clone()); + } } diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index da35226d62..cfda9bc386 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -166,7 +166,11 @@ PrivilegesEvaluator createPrivilegesEvaluator(SecurityDynamicConfiguration