From e5855ab4bb31ea86b74bd35ab27d03309a6751d4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 15:16:45 -0400 Subject: [PATCH] Change calls to isPluginUser and create InMemorySecurityRoles Signed-off-by: Craig Perkins --- .../privileges/PrivilegesEvaluator.java | 12 ++++++--- .../SystemIndexAccessEvaluator.java | 3 ++- .../security/securityconf/ConfigModelV6.java | 19 -------------- .../security/securityconf/ConfigModelV7.java | 24 +++-------------- .../securityconf/InMemorySecurityRoles.java | 20 ++++++++++++++ .../securityconf/InMemorySecurityRolesV7.java | 26 +++++++++++++++++++ .../security/securityconf/SecurityRoles.java | 3 --- .../transport/SecurityInterceptor.java | 3 ++- .../org/opensearch/security/user/User.java | 7 ----- .../SecurityRolesPermissionsTest.java | 4 +-- .../SecurityRolesPermissionsV6Test.java | 9 ------- 11 files changed, 63 insertions(+), 67 deletions(-) create mode 100644 src/main/java/org/opensearch/security/securityconf/InMemorySecurityRoles.java create mode 100644 src/main/java/org/opensearch/security/securityconf/InMemorySecurityRolesV7.java diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 238284f747..c984360cd6 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -90,10 +90,13 @@ import org.opensearch.security.resolver.IndexResolverReplacer.Resolved; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.DynamicConfigModel; +import org.opensearch.security.securityconf.InMemorySecurityRoles; +import org.opensearch.security.securityconf.InMemorySecurityRolesV7; import org.opensearch.security.securityconf.SecurityRoles; import org.opensearch.security.securityconf.impl.DashboardSignInOption; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.user.PluginUser; import org.opensearch.security.user.User; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; @@ -143,7 +146,7 @@ public class PrivilegesEvaluator { private final PitPrivilegesEvaluator pitPrivilegesEvaluator; private DynamicConfigModel dcm; private final NamedXContentRegistry namedXContentRegistry; - private final Map pluginRoles; + private final Map pluginRoles; public PrivilegesEvaluator( final ClusterService clusterService, @@ -197,9 +200,10 @@ public SecurityRoles getSecurityRoles(Set roles) { } public SecurityRoles getSecurityRoleForPlugin(String pluginIdentifier) { - SecurityRoles pluginRole = pluginRoles.get(pluginIdentifier); + InMemorySecurityRoles pluginRole = pluginRoles.get(pluginIdentifier); if (pluginRole == null) { - pluginRole = configModel.getSecurityRoles().createSecurityRole(pluginIdentifier, Set.of(BulkAction.NAME), Map.of()); + pluginRole = new InMemorySecurityRolesV7(1); + pluginRole.addSecurityRole(pluginIdentifier, Set.of(BulkAction.NAME), Map.of()); pluginRoles.put(pluginIdentifier, pluginRole); } return pluginRole; @@ -292,7 +296,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) } presponse.resolvedSecurityRoles.addAll(mappedRoles); final SecurityRoles securityRoles; - if (user.isPluginUser()) { + if (user instanceof PluginUser) { securityRoles = getSecurityRoleForPlugin(user.getName()); } else { securityRoles = getSecurityRoles(mappedRoles); diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java index 6b58f60d56..6e1168b214 100644 --- a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java @@ -48,6 +48,7 @@ import org.opensearch.security.securityconf.SecurityRoles; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.user.PluginUser; import org.opensearch.security.user.User; import org.opensearch.tasks.Task; @@ -316,7 +317,7 @@ private void evaluateSystemIndicesAccess( } } - if (user.isPluginUser()) { + if (user instanceof PluginUser) { Set matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( user.getName(), requestedResolved.getAllIndices() diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java index d859b75319..e35fb40a24 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java @@ -359,25 +359,6 @@ public SecurityRoles filter(Set keep) { return retVal; } - @Override - public SecurityRoles createSecurityRole( - String roleName, - Set clusterPerms, - Map> indexPatternToAllowedActions - ) { - SecurityRole role = new SecurityRole(roleName); - role.addClusterPerms(clusterPerms); - for (Map.Entry> entry : indexPatternToAllowedActions.entrySet()) { - IndexPattern idxPattern = new IndexPattern(entry.getKey()); - TypePerm perms = new TypePerm(""); - perms.addPerms(entry.getValue()); - idxPattern.addTypePerms(perms); - } - SecurityRoles roles = new SecurityRoles(1); - roles.addSecurityRole(role); - return roles; - } - @Override public EvaluatedDlsFlsConfig getDlsFls( User user, diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index 1ab0ba8edf..e41bbf9400 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -219,11 +219,11 @@ public static class SecurityRoles implements org.opensearch.security.securitycon final Set roles; - private SecurityRoles(int roleCount) { + protected SecurityRoles(int roleCount) { roles = new HashSet<>(roleCount); } - private SecurityRoles addSecurityRole(SecurityRole securityRole) { + protected SecurityRoles addSecurityRole(SecurityRole securityRole) { if (securityRole != null) { this.roles.add(securityRole); } @@ -273,24 +273,6 @@ public SecurityRoles filter(Set keep) { return retVal; } - @Override - public SecurityRoles createSecurityRole( - String roleName, - Set clusterPerms, - Map> indexPatternToAllowedActions - ) { - Set ipatterns = new HashSet<>(); - for (Map.Entry> entry : indexPatternToAllowedActions.entrySet()) { - IndexPattern idxPattern = new IndexPattern(entry.getKey()); - idxPattern.addPerm(entry.getValue()); - ipatterns.add(idxPattern); - } - SecurityRole role = new SecurityRole(roleName, ipatterns, WildcardMatcher.from(clusterPerms)); - SecurityRoles roles = new SecurityRoles(1); - roles.addSecurityRole(role); - return roles; - } - @Override public EvaluatedDlsFlsConfig getDlsFls( User user, @@ -554,7 +536,7 @@ public SecurityRole build() { } } - private SecurityRole(String name, Set ipatterns, WildcardMatcher clusterPerms) { + SecurityRole(String name, Set ipatterns, WildcardMatcher clusterPerms) { this.name = Objects.requireNonNull(name); this.ipatterns = ipatterns; this.clusterPerms = clusterPerms; diff --git a/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRoles.java b/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRoles.java new file mode 100644 index 0000000000..934533788f --- /dev/null +++ b/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRoles.java @@ -0,0 +1,20 @@ +/* + * 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.securityconf; + +import java.util.Map; +import java.util.Set; + +public interface InMemorySecurityRoles extends SecurityRoles { + + void addSecurityRole(String roleName, Set clusterPerms, Map> indexPatternToAllowedActions); +} diff --git a/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRolesV7.java b/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRolesV7.java new file mode 100644 index 0000000000..9c160a3bd7 --- /dev/null +++ b/src/main/java/org/opensearch/security/securityconf/InMemorySecurityRolesV7.java @@ -0,0 +1,26 @@ +package org.opensearch.security.securityconf; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.opensearch.security.support.WildcardMatcher; + +public class InMemorySecurityRolesV7 extends ConfigModelV7.SecurityRoles implements InMemorySecurityRoles { + + public InMemorySecurityRolesV7(int roleCount) { + super(roleCount); + } + + @Override + public void addSecurityRole(String roleName, Set clusterPerms, Map> indexPatternToAllowedActions) { + Set ipatterns = new HashSet<>(); + for (Map.Entry> entry : indexPatternToAllowedActions.entrySet()) { + ConfigModelV7.IndexPattern idxPattern = new ConfigModelV7.IndexPattern(entry.getKey()); + idxPattern.addPerm(entry.getValue()); + ipatterns.add(idxPattern); + } + ConfigModelV7.SecurityRole role = new ConfigModelV7.SecurityRole(roleName, ipatterns, WildcardMatcher.from(clusterPerms)); + roles.add(role); + } +} diff --git a/src/main/java/org/opensearch/security/securityconf/SecurityRoles.java b/src/main/java/org/opensearch/security/securityconf/SecurityRoles.java index 7bf4ee81fd..fb25e1a21f 100644 --- a/src/main/java/org/opensearch/security/securityconf/SecurityRoles.java +++ b/src/main/java/org/opensearch/security/securityconf/SecurityRoles.java @@ -27,7 +27,6 @@ package org.opensearch.security.securityconf; -import java.util.Map; import java.util.Set; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; @@ -98,7 +97,5 @@ Set getAllPermittedIndicesForDashboards( SecurityRoles filter(Set roles); - SecurityRoles createSecurityRole(String roleName, Set clusterPerms, Map> indexPatternToAllowedActions); - boolean isPermittedOnSystemIndex(String indexName); } diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 099e025c31..f32908a0a7 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -64,6 +64,7 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.HeaderHelper; import org.opensearch.security.support.SerializationFormat; +import org.opensearch.security.user.PluginUser; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.Transport.Connection; @@ -132,7 +133,7 @@ public SecurityRequestHandler getHandler(String private User determineUser(Connection connection) { User user0 = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); // pluginUser did not exist prior to 2.18.0 - if (user0 != null && user0.isPluginUser() && connection.getVersion().before(Version.V_2_18_0)) { + if (user0 != null && user0 instanceof PluginUser && connection.getVersion().before(Version.V_2_18_0)) { user0 = null; } return user0; diff --git a/src/main/java/org/opensearch/security/user/User.java b/src/main/java/org/opensearch/security/user/User.java index 67e557bf77..ed48ab7356 100644 --- a/src/main/java/org/opensearch/security/user/User.java +++ b/src/main/java/org/opensearch/security/user/User.java @@ -296,11 +296,4 @@ public boolean isServiceAccount() { Map userAttributesMap = this.getCustomAttributesMap(); return userAttributesMap != null && "true".equals(userAttributesMap.get("attr.internal.service")); } - - /** - * @return true if this instance is of the type PluginUser - */ - public boolean isPluginUser() { - return this instanceof PluginUser; - } } diff --git a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java index e9c10da015..b783cfb40d 100644 --- a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java +++ b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java @@ -221,8 +221,8 @@ public void hasExplicitClusterPermissionPermissionForRestAdmin() { @Test public void testCreateSecurityRole() { - SecurityRoles securityRoles = configModel.getSecurityRoles() - .createSecurityRole("testRole", Set.of("cluster:monitor/health"), Map.of("*", Set.of("indices:data/read/search"))); + InMemorySecurityRoles securityRoles = new InMemorySecurityRolesV7(1); + securityRoles.addSecurityRole("testRole", Set.of("cluster:monitor/health"), Map.of("*", Set.of("indices:data/read/search"))); assertTrue(securityRoles.getRoleNames().contains("testRole")); assertTrue(securityRoles.hasExplicitClusterPermissionPermission("cluster:monitor/health")); } diff --git a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java index 914f8750a0..281704a4ed 100644 --- a/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java +++ b/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java @@ -15,7 +15,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; import com.google.common.collect.ImmutableMap; @@ -128,14 +127,6 @@ public void hasExplicitIndexPermission() { ); } - @Test - public void testCreateSecurityRole() { - SecurityRoles securityRoles = configModel.getSecurityRoles() - .createSecurityRole("testRole", Set.of("cluster:monitor/health"), Map.of("*", Set.of("indices:data/read/search"))); - assertTrue(securityRoles.getRoleNames().contains("testRole")); - assertTrue(securityRoles.hasExplicitClusterPermissionPermission("cluster:monitor/health")); - } - @Test public void isPermittedOnSystemIndex() { final SecurityRoles securityRoleWithExplicitAccess = configModel.getSecurityRoles()