From 30fcc0fa951df1538de57795537527883061d5ff Mon Sep 17 00:00:00 2001 From: Nils Bandener Date: Fri, 12 Jul 2024 22:15:56 +0200 Subject: [PATCH] Adapted RestLayerPrivilegesEvaluator Signed-off-by: Nils Bandener --- .../security/OpenSearchSecurityPlugin.java | 3 +- .../security/filter/SecurityRestFilter.java | 19 +-- .../security/privileges/ActionPrivileges.java | 44 +++++ .../PrivilegesEvaluationContext.java | 2 +- .../privileges/PrivilegesEvaluator.java | 42 +++-- .../RestLayerPrivilegesEvaluator.java | 83 ++-------- .../impl/SecurityDynamicConfiguration.java | 13 ++ .../privileges/ActionPrivilegesTest.java | 4 +- .../RestLayerPrivilegesEvaluatorTest.java | 154 ++++++++---------- .../SecurityIndexAccessEvaluatorTest.java | 2 +- 10 files changed, 186 insertions(+), 180 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 8d6503e6c1..c7703651ef 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1142,7 +1142,7 @@ public Collection createComponents( principalExtractor = ReflectionHelper.instantiatePrincipalExtractor(principalExtractorClass); } - restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool); + restLayerEvaluator = new RestLayerPrivilegesEvaluator(evaluator); securityRestHandler = new SecurityRestFilter( backendRegistry, @@ -1161,7 +1161,6 @@ public Collection createComponents( dcf.registerDCFListener(xffResolver); dcf.registerDCFListener(evaluator); dcf.registerDCFListener(restLayerEvaluator); - dcf.registerDCFListener(securityRestHandler); dcf.registerDCFListener(tokenManager); if (!(auditLog instanceof NullAuditLog)) { // Don't register if advanced modules is disabled in which case auditlog is instance of NullAuditLog diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 420211f29b..dbe2527410 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -27,12 +27,14 @@ package org.opensearch.security.filter; import java.nio.file.Path; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.regex.Pattern; import javax.net.ssl.SSLPeerUnverifiedException; +import com.google.common.collect.ImmutableSet; import org.apache.http.HttpStatus; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -225,17 +227,12 @@ void authorizeRequest(RestHandler original, SecurityRequestChannel request, User if (routeSupportsRestAuthorization) { PrivilegesEvaluatorResponse pres = new PrivilegesEvaluatorResponse(); NamedRoute route = ((NamedRoute) handler.get()); - // if actionNames are present evaluate those first - Set actionNames = route.actionNames(); - if (actionNames != null && !actionNames.isEmpty()) { - pres = evaluator.evaluate(user, actionNames); - } - - // now if pres.allowed is still false check for the NamedRoute name as a permission - if (!pres.isAllowed()) { - String action = route.name(); - pres = evaluator.evaluate(user, Set.of(action)); - } + // Check both route.actionNames() and route.name(). The presence of either is sufficient. + Set actionNames = ImmutableSet.builder() + .addAll(route.actionNames() != null ? route.actionNames() : Collections.emptySet()) + .add(route.name()) + .build(); + pres = evaluator.evaluate(user, route.name(), actionNames); if (log.isDebugEnabled()) { log.debug(pres.toString()); diff --git a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java index ef40d3096b..3f6dc61fca 100644 --- a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java @@ -111,6 +111,10 @@ public PrivilegesEvaluatorResponse hasClusterPrivilege(PrivilegesEvaluationConte return cluster.providesPrivilege(context, action, context.getMappedRoles()); } + public PrivilegesEvaluatorResponse hasAnyClusterPrivilege(PrivilegesEvaluationContext context, Set actions) { + return cluster.providesAnyPrivilege(context, actions, context.getMappedRoles()); + } + public PrivilegesEvaluatorResponse hasIndexPrivilege( PrivilegesEvaluationContext context, Set actions, @@ -311,6 +315,46 @@ PrivilegesEvaluatorResponse providesPrivilege(PrivilegesEvaluationContext contex 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. + * Otherwise, allowed will be false and missingPrivileges will contain the name of the given action. + */ + PrivilegesEvaluatorResponse providesAnyPrivilege(PrivilegesEvaluationContext context, Set actions, Set roles) { + // 1: Check roles with wildcards + if (CollectionUtils.containsAny(roles, this.rolesWithWildcardPermissions)) { + return PrivilegesEvaluatorResponse.ok(); + } + + // 2: Check well-known actions - this should cover most cases + for (String action : actions) { + ImmutableSet rolesWithPrivileges = this.actionToRoles.get(action); + + if (rolesWithPrivileges != null && CollectionUtils.containsAny(roles, rolesWithPrivileges)) { + return PrivilegesEvaluatorResponse.ok(); + } + } + + // 3: Only if everything else fails: Check the matchers in case we have a non-well-known action + for (String action : actions) { + if (!this.wellKnownClusterActions.contains(action)) { + for (String role : roles) { + WildcardMatcher matcher = this.rolesToActionMatcher.get(role); + + if (matcher != null && matcher.test(action)) { + return PrivilegesEvaluatorResponse.ok(); + } + } + } + } + + if (actions.size() == 1) { + return PrivilegesEvaluatorResponse.insufficient(actions.iterator().next(), context); + } else { + return PrivilegesEvaluatorResponse.insufficient("any of " + actions, context); + } + } } /** diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java index d9690054bf..6c96dea488 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java @@ -51,7 +51,7 @@ public class PrivilegesEvaluationContext { */ private final Map renderedPatternTemplateCache = new HashMap<>(); - public PrivilegesEvaluationContext( + PrivilegesEvaluationContext( User user, ImmutableSet mappedRoles, String action, diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index a2c25317c5..47e0786c6c 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -188,18 +188,22 @@ public PrivilegesEvaluator( pitPrivilegesEvaluator = new PitPrivilegesEvaluator(); this.namedXContentRegistry = namedXContentRegistry; - configurationRepository.subscribeOnChange(configMap -> { - try { - // TODO: It is not nice that we cannot use a concrete generic parameter for SecurityDynamicConfiguration - // TODO: At the moment, the implementations only support the V7 config objects - 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); - } - }); + if (configurationRepository != null) { + configurationRepository.subscribeOnChange(configMap -> { + try { + // TODO: It is not nice that we cannot use a concrete generic parameter for SecurityDynamicConfiguration + // TODO: At the moment, the implementations only support the V7 config objects + 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); + } + }); + } if (clusterService != null) { clusterService.addListener(new ClusterStateListener() { @@ -226,11 +230,11 @@ void updateConfiguration( if (rolesConfiguration != null) { @SuppressWarnings("unchecked") SecurityDynamicConfiguration actionGroupsWithStatics = actionGroupsConfiguration != null - ? (SecurityDynamicConfiguration) DynamicConfigFactory.addStatics(actionGroupsConfiguration.deepClone()) + ? (SecurityDynamicConfiguration) DynamicConfigFactory.addStatics(actionGroupsConfiguration.clone()) : (SecurityDynamicConfiguration) DynamicConfigFactory.addStatics(SecurityDynamicConfiguration.empty()); FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsWithStatics); ActionPrivileges actionPrivileges = new ActionPrivileges( - DynamicConfigFactory.addStatics(rolesConfiguration.deepClone()), + DynamicConfigFactory.addStatics(rolesConfiguration.clone()), flattenedActionGroups, () -> clusterStateSupplier.get().metadata().getIndicesLookup() ); @@ -249,6 +253,10 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { this.dcm = dcm; } + ActionPrivileges getActionPrivileges() { + return this.actionPrivileges; + } + public SecurityRoles getSecurityRoles(Set roles) { return configModel.getSecurityRoles().filter(roles); } @@ -264,7 +272,7 @@ private boolean hasRestAdminPermissions(final Set roles, String permissi } public boolean isInitialized() { - return configModel != null && configModel.getSecurityRoles() != null && dcm != null; + return configModel != null && dcm != null && actionPrivileges != null; } private void setUserInfoInThreadContext(User user) { @@ -281,6 +289,10 @@ private void setUserInfoInThreadContext(User user) { } } + public PrivilegesEvaluationContext createContext(User user, String action) { + return createContext(user, action, null, null, null); + } + public PrivilegesEvaluationContext createContext( User user, String action0, diff --git a/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java index 12183166d7..b1f994163c 100644 --- a/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java @@ -16,85 +16,38 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.OpenSearchSecurityException; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.core.common.transport.TransportAddress; -import org.opensearch.security.securityconf.ConfigModel; -import org.opensearch.security.securityconf.SecurityRoles; -import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; -import org.opensearch.threadpool.ThreadPool; - -import org.greenrobot.eventbus.Subscribe; public class RestLayerPrivilegesEvaluator { protected final Logger log = LogManager.getLogger(this.getClass()); - private final ClusterService clusterService; - private ThreadContext threadContext; - private ConfigModel configModel; - - public RestLayerPrivilegesEvaluator(final ClusterService clusterService, final ThreadPool threadPool) { - this.clusterService = clusterService; - this.threadContext = threadPool.getThreadContext(); - } - - @Subscribe - public void onConfigModelChanged(final ConfigModel configModel) { - this.configModel = configModel; - } - - SecurityRoles getSecurityRoles(final Set roles) { - return configModel.getSecurityRoles().filter(roles); - } + private final PrivilegesEvaluator privilegesEvaluator; - boolean isInitialized() { - return configModel != null && configModel.getSecurityRoles() != null; + public RestLayerPrivilegesEvaluator(PrivilegesEvaluator privilegesEvaluator) { + this.privilegesEvaluator = privilegesEvaluator; } - public PrivilegesEvaluatorResponse evaluate(final User user, final Set actions) { - if (!isInitialized()) { - throw new OpenSearchSecurityException("OpenSearch Security is not initialized."); - } - - final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); - - final Set mappedRoles = mapRoles(user, caller); - - final SecurityRoles securityRoles = getSecurityRoles(mappedRoles); + public PrivilegesEvaluatorResponse evaluate(final User user, final String routeName, final Set actions) { + PrivilegesEvaluationContext context = privilegesEvaluator.createContext(user, routeName); final boolean isDebugEnabled = log.isDebugEnabled(); if (isDebugEnabled) { - log.debug("Evaluate permissions for {} on {}", user, clusterService.localNode().getName()); + log.debug("Evaluate permissions for {}", user); log.debug("Action: {}", actions); - log.debug("Mapped roles: {}", mappedRoles.toString()); + log.debug("Mapped roles: {}", context.getMappedRoles().toString()); } - for (final String action : actions) { - if (!securityRoles.impliesClusterPermissionPermission(action)) { - // TODO This will exhibit a weird behaviour when a REST action specifies two permissions, and - // if the user has no permissions for the first one, but has permissions for the second one: - // First, the information "No permission match" will be logged, but then the action will be - // allowed nevertheless. - log.info( - "No permission match for {} [Action [{}]] [RolesChecked {}]. No permissions for {}", - user, - action, - securityRoles.getRoleNames(), - action - ); - } else { - if (isDebugEnabled) { - log.debug("Allowed because we have permissions for {}", actions); - } - return PrivilegesEvaluatorResponse.ok(); - } - } + PrivilegesEvaluatorResponse result = privilegesEvaluator.getActionPrivileges().hasAnyClusterPrivilege(context, actions); - return PrivilegesEvaluatorResponse.insufficient(actions).resolvedSecurityRoles(mappedRoles); - } + if (!result.allowed) { + log.info( + "No permission match for {} [Action [{}]] [RolesChecked {}]. No permissions for {}", + user, + routeName, + context.getMappedRoles(), + result.getMissingPrivileges() + ); + } - Set mapRoles(final User user, final TransportAddress caller) { - return this.configModel.mapSecurityRoles(user, caller); + return result; } } 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 403572c57c..37192e4a24 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/SecurityDynamicConfiguration.java @@ -169,6 +169,7 @@ public static SecurityDynamicConfiguration fromYaml(String yaml, CType ct DefaultObjectMapper.getTypeFactory().constructParametricType(SecurityDynamicConfiguration.class, implementationClass) ); result.ctype = ctype; + result.version = 2; return result; } @@ -327,6 +328,18 @@ public Class getImplementingClass() { return getCType() == null ? null : getCType().getImplementationClass().get(getVersion()); } + @JsonIgnore + public SecurityDynamicConfiguration clone() { + SecurityDynamicConfiguration result = new SecurityDynamicConfiguration(); + result.version = this.version; + result.ctype = this.ctype; + result.primaryTerm = this.primaryTerm; + result.seqNo = this.seqNo; + result._meta = this._meta; + result.centries.putAll(this.centries); + return result; + } + @JsonIgnore public SecurityDynamicConfiguration deepClone() { try { diff --git a/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index 811aba4abb..8fedc05683 100644 --- a/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -571,8 +571,8 @@ static PrivilegesEvaluationContext ctx(String... roles) { null, null, null, - new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), - null + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + null ); } } diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index c374a10c24..25f398f66a 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -11,28 +11,33 @@ package org.opensearch.security.privileges; -import java.util.Collections; import java.util.Set; +import java.util.TreeMap; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.Configurator; -import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.opensearch.OpenSearchSecurityException; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.security.auditlog.NullAuditLog; import org.opensearch.security.securityconf.ConfigModel; -import org.opensearch.security.securityconf.SecurityRoles; +import org.opensearch.security.securityconf.DynamicConfigModel; +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.user.User; -import org.opensearch.threadpool.ThreadPool; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -41,12 +46,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThrows; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; @@ -56,11 +56,9 @@ public class RestLayerPrivilegesEvaluatorTest { @Mock(strictness = Mock.Strictness.LENIENT) private ClusterService clusterService; @Mock - private ThreadPool threadPool; - @Mock private ConfigModel configModel; - - private RestLayerPrivilegesEvaluator privilegesEvaluator; + @Mock + private DynamicConfigModel dynamicConfigModel; private static final User TEST_USER = new User("test_user"); @@ -71,16 +69,14 @@ private void setLoggingLevel(final Level level) { @Before public void setUp() { - when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); - when(clusterService.localNode()).thenReturn(mock(DiscoveryNode.class, withSettings().strictness(Strictness.LENIENT))); - privilegesEvaluator = new RestLayerPrivilegesEvaluator( - clusterService, - threadPool - ); - privilegesEvaluator.onConfigModelChanged(configModel); // Defaults to the mocked config model - verify(threadPool).getThreadContext(); // Called during construction of RestLayerPrivilegesEvaluator + when(configModel.mapSecurityRoles(TEST_USER, null)).thenReturn(Set.of("test_role")); setLoggingLevel(Level.DEBUG); // Enable debug logging scenarios for verification + ClusterState clusterState = mock(ClusterState.class); + when(clusterService.state()).thenReturn(clusterState); + Metadata metadata = mock(Metadata.class); + when(clusterState.metadata()).thenReturn(metadata); + when(metadata.getIndicesLookup()).thenReturn(new TreeMap<>()); } @After @@ -89,96 +85,88 @@ public void after() { } @Test - public void testEvaluate_Initialized_Success() { + public void testEvaluate_Initialized_Success() throws Exception { String action = "action"; - SecurityRoles securityRoles = mock(SecurityRoles.class); - when(configModel.getSecurityRoles()).thenReturn(securityRoles); - when(configModel.getSecurityRoles().filter(Collections.emptySet())).thenReturn(securityRoles); - when(securityRoles.impliesClusterPermissionPermission(action)).thenReturn(false); + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - any", CType.ROLES); + + PrivilegesEvaluator privilegesEvaluator = createPrivilegesEvaluator(roles); + RestLayerPrivilegesEvaluator restPrivilegesEvaluator = new RestLayerPrivilegesEvaluator(privilegesEvaluator); - PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); + PrivilegesEvaluatorResponse response = restPrivilegesEvaluator.evaluate(TEST_USER, "route_name", Set.of(action)); assertThat(response.isAllowed(), equalTo(false)); assertThat(response.getMissingPrivileges(), equalTo(Set.of(action))); - assertThat(response.getResolvedSecurityRoles(), Matchers.empty()); - verify(configModel, times(3)).getSecurityRoles(); } @Test public void testEvaluate_NotInitialized_NullModel_ExceptionThrown() { - // Null out the config model - privilegesEvaluator.onConfigModelChanged(null); - final OpenSearchSecurityException exception = assertThrows( - OpenSearchSecurityException.class, - () -> privilegesEvaluator.evaluate(TEST_USER, null) - ); - assertThat(exception.getMessage(), equalTo("OpenSearch Security is not initialized.")); - verify(configModel, never()).getSecurityRoles(); - } - - @Test - public void testEvaluate_NotInitialized_NoSecurityRoles_ExceptionThrown() { + PrivilegesEvaluator privilegesEvaluator = createPrivilegesEvaluator(null); + RestLayerPrivilegesEvaluator restPrivilegesEvaluator = new RestLayerPrivilegesEvaluator(privilegesEvaluator); final OpenSearchSecurityException exception = assertThrows( OpenSearchSecurityException.class, - () -> privilegesEvaluator.evaluate(TEST_USER, null) + () -> restPrivilegesEvaluator.evaluate(TEST_USER, "route_name", null) ); assertThat(exception.getMessage(), equalTo("OpenSearch Security is not initialized.")); - verify(configModel).getSecurityRoles(); } @Test - public void testMapRoles_ReturnsMappedRoles() { - final User user = mock(User.class); - final Set mappedRoles = Collections.singleton("role1"); - when(configModel.mapSecurityRoles(any(), any())).thenReturn(mappedRoles); - - final Set result = privilegesEvaluator.mapRoles(user, null); - - assertThat(result, equalTo(mappedRoles)); - verifyNoInteractions(user); - verify(configModel).mapSecurityRoles(user, null); - } - - @Test - public void testEvaluate_Successful_NewPermission() { + public void testEvaluate_Successful_NewPermission() throws Exception { String action = "hw:greet"; - SecurityRoles securityRoles = mock(SecurityRoles.class); - when(configModel.getSecurityRoles()).thenReturn(securityRoles); - when(configModel.getSecurityRoles().filter(Collections.emptySet())).thenReturn(securityRoles); - when(securityRoles.impliesClusterPermissionPermission(action)).thenReturn(true); - - PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - hw:greet", CType.ROLES); + PrivilegesEvaluator privilegesEvaluator = createPrivilegesEvaluator(roles); + RestLayerPrivilegesEvaluator restPrivilegesEvaluator = new RestLayerPrivilegesEvaluator(privilegesEvaluator); + PrivilegesEvaluatorResponse response = restPrivilegesEvaluator.evaluate(TEST_USER, "route_name", Set.of(action)); assertThat(response.allowed, equalTo(true)); - verify(securityRoles).impliesClusterPermissionPermission(action); } @Test - public void testEvaluate_Successful_LegacyPermission() { + public void testEvaluate_Successful_LegacyPermission() throws Exception { String action = "cluster:admin/opensearch/hw/greet"; - SecurityRoles securityRoles = mock(SecurityRoles.class); - when(configModel.getSecurityRoles()).thenReturn(securityRoles); - when(configModel.getSecurityRoles().filter(Collections.emptySet())).thenReturn(securityRoles); - when(securityRoles.impliesClusterPermissionPermission(action)).thenReturn(true); - - PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - cluster:admin/opensearch/hw/greet", CType.ROLES); + PrivilegesEvaluator privilegesEvaluator = createPrivilegesEvaluator(roles); + RestLayerPrivilegesEvaluator restPrivilegesEvaluator = new RestLayerPrivilegesEvaluator(privilegesEvaluator); + PrivilegesEvaluatorResponse response = restPrivilegesEvaluator.evaluate(TEST_USER, "route_name", Set.of(action)); assertThat(response.allowed, equalTo(true)); - verify(securityRoles).impliesClusterPermissionPermission(action); - verify(configModel, times(3)).getSecurityRoles(); } @Test - public void testEvaluate_Unsuccessful() { + public void testEvaluate_Unsuccessful() throws Exception { String action = "action"; - SecurityRoles securityRoles = mock(SecurityRoles.class); - when(configModel.getSecurityRoles()).thenReturn(securityRoles); - when(configModel.getSecurityRoles().filter(Collections.emptySet())).thenReturn(securityRoles); - when(securityRoles.impliesClusterPermissionPermission(action)).thenReturn(false); + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - other_action", CType.ROLES); + PrivilegesEvaluator privilegesEvaluator = createPrivilegesEvaluator(roles); + RestLayerPrivilegesEvaluator restPrivilegesEvaluator = new RestLayerPrivilegesEvaluator(privilegesEvaluator); + PrivilegesEvaluatorResponse response = restPrivilegesEvaluator.evaluate(TEST_USER, "route_name", Set.of(action)); + assertThat(response.allowed, equalTo(false)); + } - PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); + PrivilegesEvaluator createPrivilegesEvaluator(SecurityDynamicConfiguration roles) { + PrivilegesEvaluator privilegesEvaluator = new PrivilegesEvaluator( + clusterService, + () -> clusterService.state(), + new ThreadContext(Settings.EMPTY), + null, + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + new NullAuditLog(), + Settings.EMPTY, + null, + null, + null, + null + ); + privilegesEvaluator.onConfigModelChanged(configModel); // Defaults to the mocked config model + privilegesEvaluator.onDynamicConfigModelChanged(dynamicConfigModel); - assertThat(response.allowed, equalTo(false)); - verify(securityRoles).impliesClusterPermissionPermission(action); + if (roles != null) { + privilegesEvaluator.updateConfiguration(SecurityDynamicConfiguration.empty(), roles); + } + return privilegesEvaluator; } } diff --git a/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java index 5e066899d3..52283e0dd6 100644 --- a/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java @@ -163,7 +163,7 @@ PrivilegesEvaluationContext ctx(String action) { request, null, null, - indexNameExpressionResolver, + indexNameExpressionResolver, null ); }