From 641822660162ffb781324bbf8ae7871f206596a7 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 31 Dec 2024 16:49:42 -0500 Subject: [PATCH] Add testing for cluster and index priv Signed-off-by: Derek Ho --- .../privileges/ActionPrivilegesTest.java | 116 +++++++++++++++++- .../security/privileges/ActionPrivileges.java | 31 +++-- 2 files changed, 136 insertions(+), 11 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index 7807dae748..ecd76b127c 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -38,15 +38,19 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.common.unit.ByteSizeValue; +import org.opensearch.security.action.apitokens.ApiToken; +import org.opensearch.security.action.apitokens.Permissions; import org.opensearch.security.resolver.IndexResolverReplacer; 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.user.User; import org.opensearch.security.util.MockIndexMetadataBuilder; import static org.hamcrest.MatcherAssert.assertThat; +import static org.opensearch.security.privileges.ActionPrivilegesTest.IndexPrivileges.IndicesAndAliases.resolved; import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isAllowed; import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isForbidden; import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isPartiallyOk; @@ -258,6 +262,69 @@ public void hasAny_wildcard() throws Exception { isForbidden(missingPrivileges("cluster:whatever")) ); } + + @Test + public void apiToken_explicit_failsWithWildcard() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - '*'", CType.ROLES); + ActionPrivileges subject = new ActionPrivileges(roles, FlattenedActionGroups.EMPTY, null, Settings.EMPTY); + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache().getJtis().put(token, new Permissions(List.of("*"), List.of())); + // Explicit fails + assertThat( + subject.hasExplicitClusterPrivilege(context, "cluster:whatever"), + isForbidden(missingPrivileges("cluster:whatever")) + ); + // Not explicit succeeds + assertThat(subject.hasClusterPrivilege(context, "cluster:whatever"), isAllowed()); + // Any succeeds + assertThat(subject.hasAnyClusterPrivilege(context, ImmutableSet.of("cluster:whatever")), isAllowed()); + } + + @Test + public void apiToken_succeedsWithExactMatch() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - '*'", CType.ROLES); + ActionPrivileges subject = new ActionPrivileges(roles, FlattenedActionGroups.EMPTY, null, Settings.EMPTY); + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache().getJtis().put(token, new Permissions(List.of("cluster:whatever"), List.of())); + // Explicit succeeds + assertThat(subject.hasExplicitClusterPrivilege(context, "cluster:whatever"), isAllowed()); + // Not explicit succeeds + assertThat(subject.hasClusterPrivilege(context, "cluster:whatever"), isAllowed()); + // Any succeeds + assertThat(subject.hasAnyClusterPrivilege(context, ImmutableSet.of("cluster:whatever")), isAllowed()); + // Any succeeds + assertThat(subject.hasAnyClusterPrivilege(context, ImmutableSet.of("cluster:whatever", "cluster:other")), isAllowed()); + } + + @Test + public void apiToken_succeedsWithActionGroupsExapnded() throws Exception { + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + // + " cluster_permissions:\n" + // + " - '*'", CType.ROLES); + + SecurityDynamicConfiguration config = SecurityDynamicConfiguration.fromYaml( + "CLUSTER_ALL:\n allowed_actions:\n - \"cluster:*\"", + CType.ACTIONGROUPS + ); + + FlattenedActionGroups actionGroups = new FlattenedActionGroups(config); + ActionPrivileges subject = new ActionPrivileges(roles, actionGroups, null, Settings.EMPTY); + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache().getJtis().put(token, new Permissions(List.of("CLUSTER_ALL"), List.of())); + // Explicit succeeds + assertThat(subject.hasExplicitClusterPrivilege(context, "cluster:whatever"), isAllowed()); + // Not explicit succeeds + assertThat(subject.hasClusterPrivilege(context, "cluster:whatever"), isAllowed()); + // Any succeeds + assertThat(subject.hasClusterPrivilege(context, "cluster:monitor/main"), isAllowed()); + } } /** @@ -292,6 +359,20 @@ public void positive_full() throws Exception { assertThat(result, isAllowed()); } + @Test + public void apiTokens_positive_full() throws Exception { + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache() + .getJtis() + .put( + token, + new Permissions(List.of("index_a11"), List.of(new ApiToken.IndexPermission(List.of("index_a11"), List.of("*")))) + ); + PrivilegesEvaluatorResponse result = subject.hasIndexPrivilege(context, requiredActions, resolved("index_a11")); + assertThat(result, isAllowed()); + } + @Test public void positive_partial() throws Exception { PrivilegesEvaluationContext ctx = ctx("test_role"); @@ -346,6 +427,18 @@ public void negative_wrongRole() throws Exception { assertThat(result, isForbidden(missingPrivileges(requiredActions))); } + @Test + public void apiToken_negative_noPermissions() throws Exception { + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache() + .getJtis() + .put(token, new Permissions(List.of(), List.of(new ApiToken.IndexPermission(List.of(), List.of())))); + + PrivilegesEvaluatorResponse result = subject.hasIndexPrivilege(context, requiredActions, resolved("index_a11")); + assertThat(result, isForbidden(missingPrivileges(requiredActions))); + } + @Test public void negative_wrongAction() throws Exception { PrivilegesEvaluationContext ctx = ctx("test_role"); @@ -375,6 +468,23 @@ public void positive_hasExplicit_full() { } } + @Test + public void apiTokens_positive_hasExplicit_full() { + String token = "blah"; + PrivilegesEvaluationContext context = ctxWithUserName("apitoken_test:" + token); + context.getApiTokenIndexListenerCache() + .getJtis() + .put( + token, + new Permissions(List.of("index_a11"), List.of(new ApiToken.IndexPermission(List.of("index_a11"), List.of("*")))) + ); + + PrivilegesEvaluatorResponse result = subject.hasExplicitIndexPrivilege(context, requiredActions, resolved("index_a11")); + + assertThat(result, isForbidden(missingPrivileges(requiredActions))); + + } + private boolean covers(PrivilegesEvaluationContext ctx, String... indices) { for (String index : indices) { if (!indexSpec.covers(ctx.getUser(), index)) { @@ -1017,7 +1127,11 @@ static SecurityDynamicConfiguration createRoles(int numberOfRoles, int n } static PrivilegesEvaluationContext ctx(String... roles) { - User user = new User("test_user"); + return ctxWithUserName("test-user", roles); + } + + static PrivilegesEvaluationContext ctxWithUserName(String userName, String... roles) { + User user = new User(userName); user.addAttributes(ImmutableMap.of("attrs.dept_no", "a11")); return new PrivilegesEvaluationContext( user, diff --git a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java index 08b2d6aa5c..d722231796 100644 --- a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java @@ -431,7 +431,7 @@ PrivilegesEvaluatorResponse apiTokenProvidesClusterPrivilege( String jti = context.getUser().getName().split(":")[1]; if (context.getApiTokenIndexListenerCache().getJtis().get(jti) != null) { // Expand the action groups - ImmutableSet resolvedClusterPermissions = actionGroups.resolve( + Set resolvedClusterPermissions = actionGroups.resolve( context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm() ); @@ -449,15 +449,18 @@ PrivilegesEvaluatorResponse apiTokenProvidesClusterPrivilege( // Check for pattern matches (like "cluster:*") for (String permission : resolvedClusterPermissions) { - // Skip exact matches as we already checked those - if (!permission.contains("*")) { - continue; - } + // skip pure *, which was evaluated above + if (permission != "*") { + // Skip exact matches as we already checked those + if (!permission.contains("*")) { + continue; + } - WildcardMatcher permissionMatcher = WildcardMatcher.from(permission); - for (String action : actions) { - if (permissionMatcher.test(action)) { - return PrivilegesEvaluatorResponse.ok(); + WildcardMatcher permissionMatcher = WildcardMatcher.from(permission); + for (String action : actions) { + if (permissionMatcher.test(action)) { + return PrivilegesEvaluatorResponse.ok(); + } } } } @@ -967,7 +970,15 @@ PrivilegesEvaluatorResponse apiTokenProvidesIndexPrivilege( } if (!indexHasAllPermissions) { - return PrivilegesEvaluatorResponse.insufficient("Insufficient permissions for index"); + return PrivilegesEvaluatorResponse.insufficient(checkTable) + .reason( + resolvedIndices.getAllIndices().size() == 1 + ? "Insufficient permissions for the referenced index" + : "None of " + + resolvedIndices.getAllIndices().size() + + " referenced indices has sufficient permissions" + ) + .evaluationExceptions(exceptions); } } // If we get here, all indices had sufficient permissions