Skip to content

Commit

Permalink
Add alias support
Browse files Browse the repository at this point in the history 
Signed-off-by: Derek Ho <[email protected]>
  • Loading branch information
@derek-ho
derek-ho committed Feb 6, 2025
1 parent b6aa196 commit bf31791
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 26 deletions.
4 changes: 3 additions & 1 deletion src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,9 @@ public List<RestHandler> getRestHandlers(
auditLog,
configPath,
principalExtractor,
apiTokenRepository
apiTokenRepository,
cs,
indexNameExpressionResolver
)
);
handlers.addAll(
Expand Down
27 changes: 21 additions & 6 deletions src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

import org.opensearch.OpenSearchException;
import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.action.ActionListener;
Expand All @@ -47,6 +49,8 @@
import org.opensearch.security.dlic.rest.api.RestApiPrivilegesEvaluator;
import org.opensearch.security.dlic.rest.api.SecurityApiDependencies;
import org.opensearch.security.dlic.rest.support.Utils;
import org.opensearch.security.privileges.IndexPattern;
import org.opensearch.security.privileges.PrivilegesEvaluationException;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.securityconf.DynamicConfigFactory;
import org.opensearch.security.securityconf.FlattenedActionGroups;
Expand Down Expand Up @@ -82,6 +86,8 @@ public class ApiTokenAction extends BaseRestHandler {
private final ConfigurationRepository configurationRepository;
private final PrivilegesEvaluator privilegesEvaluator;
private final SecurityApiDependencies securityApiDependencies;
private final ClusterService clusterService;
private final IndexNameExpressionResolver indexNameExpressionResolver;

private static final List<Route> ROUTES = addRoutesPrefix(
ImmutableList.of(new Route(POST, "/apitokens"), new Route(DELETE, "/apitokens"), new Route(GET, "/apitokens"))
Expand All @@ -96,7 +102,9 @@ public ApiTokenAction(
AuditLog auditLog,
Path configPath,
PrincipalExtractor principalExtractor,
ApiTokenRepository apiTokenRepository
ApiTokenRepository apiTokenRepository,
ClusterService clusterService,
IndexNameExpressionResolver indexNameExpressionResolver
) {
this.apiTokenRepository = apiTokenRepository;
this.threadPool = threadpool;
Expand All @@ -116,6 +124,8 @@ public ApiTokenAction(
auditLog,
settings
);
this.clusterService = clusterService;
this.indexNameExpressionResolver = indexNameExpressionResolver;
}

@Override
Expand Down Expand Up @@ -358,17 +368,15 @@ private void sendErrorResponse(RestChannel channel, RestStatus status, String er
* Validates that the user has the required permissions to create an API token (must be a subset of their own permissions)
* */
@SuppressWarnings("unchecked")
void validateUserPermissions(List<String> clusterPermissions, List<ApiToken.IndexPermission> indexPermissions) {
void validateUserPermissions(List<String> clusterPermissions, List<ApiToken.IndexPermission> indexPermissions)
throws PrivilegesEvaluationException {
final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
final TransportAddress caller = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
final Set<String> roles = privilegesEvaluator.mapRoles(user, caller);

// Early return conditions
if (roles.isEmpty()) {
throw new OpenSearchException("User does not have any roles");
} else if (roles.contains("all_access")) {
// all_access == *
return;
}

// Verify user has all requested cluster permissions
Expand Down Expand Up @@ -418,8 +426,15 @@ void validateUserPermissions(List<String> clusterPermissions, List<ApiToken.Inde
List<String> rolePatterns = indexPermission.getIndex_patterns();
List<String> roleIndexPerms = indexPermission.getAllowed_actions();

IndexPattern indexPattern = IndexPattern.from(rolePatterns);

// Check if this role's pattern covers the requested pattern
if (WildcardMatcher.from(rolePatterns).test(requestedPattern)) {
if (indexPattern.matches(
requestedPattern,
indexNameExpressionResolver,
(String template) -> WildcardMatcher.NONE,
clusterService.state().metadata().getIndicesLookup()
)) {
// Get resolved actions for this role's index permissions
Set<String> roleActions = flattenedActionGroups.resolve(roleIndexPerms);
WildcardMatcher matcher = WildcardMatcher.from(roleActions);
Expand Down
46 changes: 33 additions & 13 deletions src/main/java/org/opensearch/security/privileges/IndexPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,33 @@ private IndexPattern(WildcardMatcher staticPattern, ImmutableList<String> patter
this.hashCode = staticPattern.hashCode() + patternTemplates.hashCode() + dateMathExpressions.hashCode();
}

public boolean matches(String index, PrivilegesEvaluationContext context, Map<String, IndexAbstraction> indexMetadata)
throws PrivilegesEvaluationException {
@FunctionalInterface
public interface MatcherGenerator {
WildcardMatcher apply(String pattern) throws PrivilegesEvaluationException;
}

public boolean matches(
String index,
IndexNameExpressionResolver indexNameExpressionResolver,
MatcherGenerator matcherGenerator,
Map<String, IndexAbstraction> indexMetadata
) throws PrivilegesEvaluationException {
if (staticPattern != WildcardMatcher.NONE && staticPattern.test(index)) {
return true;
}

if (!patternTemplates.isEmpty()) {
for (String patternTemplate : this.patternTemplates) {
try {
WildcardMatcher matcher = context.getRenderedMatcher(patternTemplate);

if (matcher.test(index)) {
return true;
}
} catch (ExpressionEvaluationException e) {
throw new PrivilegesEvaluationException("Error while evaluating dynamic index pattern: " + patternTemplate, e);
WildcardMatcher matcher = matcherGenerator.apply(patternTemplate);

if (matcher.test(index)) {
return true;
}
}
}

if (!dateMathExpressions.isEmpty()) {
IndexNameExpressionResolver indexNameExpressionResolver = context.getIndexNameExpressionResolver();

// Note: The use of date math expressions in privileges is a bit odd, as it only provides a very limited
// solution for the potential user case. A different approach might be nice.

Expand All @@ -108,15 +112,20 @@ public boolean matches(String index, PrivilegesEvaluationContext context, Map<St
// Check for the privilege for aliases or data streams containing this index

if (indexAbstraction.getParentDataStream() != null) {
if (matches(indexAbstraction.getParentDataStream().getName(), context, indexMetadata)) {
if (matches(
indexAbstraction.getParentDataStream().getName(),
indexNameExpressionResolver,
matcherGenerator,
indexMetadata
)) {
return true;
}
}

// Retrieve aliases: The use of getWriteIndex() is a bit messy, but it is the only way to access
// alias metadata from here.
for (String alias : indexAbstraction.getWriteIndex().getAliases().keySet()) {
if (matches(alias, context, indexMetadata)) {
if (matches(alias, indexNameExpressionResolver, matcherGenerator, indexMetadata)) {
return true;
}
}
Expand All @@ -125,6 +134,17 @@ public boolean matches(String index, PrivilegesEvaluationContext context, Map<St
return false;
}

public boolean matches(String index, PrivilegesEvaluationContext context, Map<String, IndexAbstraction> indexMetadata)
throws PrivilegesEvaluationException {
return matches(index, context.getIndexNameExpressionResolver(), (String patternTemplate) -> {
try {
return context.getRenderedMatcher(patternTemplate);
} catch (ExpressionEvaluationException e) {
throw new PrivilegesEvaluationException("Error while evaluating dynamic index pattern: " + patternTemplate, e);
}
}, indexMetadata);
}

@Override
public String toString() {
if (patternTemplates.size() == 0 && dateMathExpressions.size() == 0) {
Expand Down
60 changes: 54 additions & 6 deletions src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
import org.junit.runner.RunWith;

import org.opensearch.OpenSearchException;
import org.opensearch.Version;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.security.configuration.ConfigurationRepository;
Expand Down Expand Up @@ -57,6 +63,14 @@ public class ApiTokenActionTest {
@Mock
private ConfigurationRepository configurationRepository;

@Mock
private ClusterService clusterService;
@Mock
private ClusterState clusterState;

@Mock
private Metadata metadata;

private SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfig;
private SecurityDynamicConfiguration<RoleV7> rolesConfig;
private FlattenedActionGroups flattenedActionGroups;
Expand Down Expand Up @@ -119,6 +133,13 @@ public void setUp() throws JsonProcessingException {
Arrays.asList(ImmutableMap.of("index_patterns", List.of("lo*"), "allowed_actions", List.of("write_group"))),
"cluster_permissions",
Arrays.asList("cluster_monitor")
),
"alias_group",
ImmutableMap.of(
"index_permissions",
Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs"), "allowed_actions", List.of("read"))),
"cluster_permissions",
Arrays.asList("cluster_monitor")
)

),
Expand All @@ -129,6 +150,19 @@ public void setUp() throws JsonProcessingException {

when(configurationRepository.getConfiguration(CType.ROLES)).thenReturn(rolesConfig);
when(configurationRepository.getConfiguration(CType.ACTIONGROUPS)).thenReturn(actionGroupsConfig);
when(clusterService.state()).thenReturn(clusterState);

when(clusterState.metadata()).thenReturn(
Metadata.builder()
.put(
IndexMetadata.builder("my-index")
.settings(Settings.builder().put("index.version.created", Version.CURRENT).build())
.numberOfShards(1)
.numberOfReplicas(0)
.putAlias(AliasMetadata.builder("logs").build())
)
.build()
);

apiTokenAction = new ApiTokenAction(

Expand All @@ -140,6 +174,8 @@ public void setUp() throws JsonProcessingException {
null,
null,
null,
null,
clusterService,
null
);

Expand Down Expand Up @@ -219,7 +255,7 @@ public void testExtractClusterPermissions() {
}

@Test
public void testExactMatchPermissionsWithActionGroups() {
public void testExactMatchPermissionsWithActionGroups() throws Exception {
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123"));

apiTokenAction.validateUserPermissions(List.of(), List.of(new ApiToken.IndexPermission(
Expand All @@ -243,7 +279,7 @@ public void testCreateMorePermissableWildcardPermissionWhenNoAccessThrowsExcepti
}

@Test
public void testMultipleRolesCoveringPermissions() {
public void testMultipleRolesCoveringPermissions() throws Exception {
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123"));

ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(List.of("logs-123"), List.of("read", "write"));
Expand All @@ -270,7 +306,7 @@ public void testSeparateIndexPatternThrowsException() {
}

@Test
public void testActionGroupResolution() {
public void testActionGroupResolution() throws Exception{
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123"));

ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(
Expand All @@ -282,14 +318,14 @@ public void testActionGroupResolution() {
}

@Test
public void testEmptyIndexPermissions() {
public void testEmptyIndexPermissions() throws Exception {
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123"));

apiTokenAction.validateUserPermissions(List.of("cluster:monitor"), List.of());
}

@Test
public void testClusterPermissionsEvaluation() {
public void testClusterPermissionsEvaluation() throws Exception {
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("cluster_monitor"));

apiTokenAction.validateUserPermissions(List.of("cluster_monitor"), List.of());
Expand All @@ -303,9 +339,21 @@ public void testClusterPermissionsMorePermissableRegexThrowsException() {
}

@Test
public void testClusterPermissionsFromMultipleRoles() {
public void testClusterPermissionsFromMultipleRoles() throws Exception{
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("cluster_monitor", "read_group_logs-123"));

apiTokenAction.validateUserPermissions(List.of("cluster_monitor", "cluster_health"), List.of());
}

@Test
public void testAliasAllowsAccessOnUnderlyingIndices() throws Exception {
when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("alias_group"));

ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(
List.of("my-index"),
List.of("read")
);

apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm));
}
}

0 comments on commit bf31791

Please sign in to comment.