Skip to content

Commit

Permalink
Add testing for cluster and index priv
Browse files Browse the repository at this point in the history
Signed-off-by: Derek Ho <[email protected]>
  • Loading branch information
derek-ho committed Dec 31, 2024
1 parent 73eb2ab commit 6418226
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -258,6 +262,69 @@ public void hasAny_wildcard() throws Exception {
isForbidden(missingPrivileges("cluster:whatever"))
);
}

@Test
public void apiToken_explicit_failsWithWildcard() throws Exception {
SecurityDynamicConfiguration<RoleV7> 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<RoleV7> 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<RoleV7> roles = SecurityDynamicConfiguration.fromYaml("test_role:\n" + //
" cluster_permissions:\n" + //
" - '*'", CType.ROLES);

SecurityDynamicConfiguration<ActionGroupsV7> 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());
}
}

/**
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -1017,7 +1127,11 @@ static SecurityDynamicConfiguration<RoleV7> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> resolvedClusterPermissions = actionGroups.resolve(
Set<String> resolvedClusterPermissions = actionGroups.resolve(
context.getApiTokenIndexListenerCache().getJtis().get(jti).getClusterPerm()
);

Expand All @@ -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();
}
}

Check warning on line 464 in src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/ActionPrivileges.java#L464

Added line #L464 was not covered by tests
}
}
Expand Down Expand Up @@ -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"

Check warning on line 979 in src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/ActionPrivileges.java#L978-L979

Added lines #L978 - L979 were not covered by tests
)
.evaluationExceptions(exceptions);
}
}
// If we get here, all indices had sufficient permissions
Expand Down

0 comments on commit 6418226

Please sign in to comment.