Skip to content

Commit

Permalink
Fixed SecurityIndexAccessEvaluatorTest
Browse files Browse the repository at this point in the history
Signed-off-by: Nils Bandener <[email protected]>
  • Loading branch information
nibix committed May 31, 2024
1 parent 8c89263 commit 433e5cd
Show file tree
Hide file tree
Showing 6 changed files with 350 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,25 @@ public PrivilegesEvaluatorResponse hasIndexPrivilege(
Set<String> actions,
IndexResolverReplacer.Resolved resolvedIndices
) {
if (resolvedIndices.isLocalAll()) {
PrivilegesEvaluatorResponse response = this.index.providesWildcardPrivilege(context, actions);

if (response != null) {
return response;
}
}

if (resolvedIndices.getAllIndices().isEmpty()) {
log.debug("No local indices; grant the request");
return PrivilegesEvaluatorResponse.ok();
}

// TODO one might want to consider to create a semantic wrapper for action in order to be better tell apart
// what's the action and what's the index in the generic parameters of CheckTable.
CheckTable<String, String> checkTable = CheckTable.create(resolvedIndices.getAllIndices(), actions);
CheckTable<String, String> checkTable = CheckTable.create(
resolvedIndices.getAllIndicesResolved(context.getClusterStateSupplier(), context.getIndexNameExpressionResolver()),
actions
);

StatefulIndexPrivileges statefulIndex = this.statefulIndex;
PrivilegesEvaluatorResponse resultFromStatefulIndex = null;
Expand Down Expand Up @@ -568,6 +579,29 @@ PrivilegesEvaluatorResponse providesPrivilege(
);
}

/**
* Returns PrivilegesEvaluatorResponse.ok() if the user identified in the context object has privileges for all
* indices (using *) for the given actions. Returns null otherwise. Then, further checks must be done to check
* the user's privileges.
*/
PrivilegesEvaluatorResponse providesWildcardPrivilege(PrivilegesEvaluationContext context, Set<String> actions) {
ImmutableSet<String> effectiveRoles = context.getMappedRoles();
CheckTable<String, String> checkTable = CheckTable.create(ImmutableSet.of("*"), actions);

for (String action : actions) {
ImmutableSet<String> rolesWithWildcardIndexPrivileges = this.actionToRolesWithWildcardIndexPrivileges.get(action);

if (rolesWithWildcardIndexPrivileges != null
&& CollectionUtils.containsAny(rolesWithWildcardIndexPrivileges, effectiveRoles)) {
if (checkTable.check("*", action)) {
return PrivilegesEvaluatorResponse.ok();
}
}
}

return null;
}

PrivilegesEvaluatorResponse providesExplicitPrivilege(
PrivilegesEvaluationContext context,
Set<String> actions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@

import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

import com.google.common.collect.ImmutableSet;

import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.security.support.WildcardMatcher;
import org.opensearch.security.user.User;
Expand Down Expand Up @@ -42,19 +44,22 @@ public class PrivilegesEvaluationContext {
private final Map<String, WildcardMatcher> renderedPatternTemplateCache = new HashMap<>();
private final ImmutableSet<String> mappedRoles;

private final Supplier<ClusterState> clusterStateSupplier;
private final IndexNameExpressionResolver indexNameExpressionResolver;

public PrivilegesEvaluationContext(
User user,
ImmutableSet<String> mappedRoles,
String action,
Object request,
Supplier<ClusterState> clusterStateSupplier,
IndexNameExpressionResolver indexNameExpressionResolver
) {
this.user = user;
this.mappedRoles = mappedRoles;
this.action = action;
this.request = request;
this.clusterStateSupplier = clusterStateSupplier;
this.indexNameExpressionResolver = indexNameExpressionResolver;
}

Expand Down Expand Up @@ -95,12 +100,16 @@ public PrivilegesEvaluationContext mappedRoles(ImmutableSet<String> mappedRoles)
if (this.mappedRoles != null && this.mappedRoles.equals(mappedRoles)) {
return this;
} else {
return new PrivilegesEvaluationContext(user, mappedRoles, action, request, indexNameExpressionResolver);
return new PrivilegesEvaluationContext(user, mappedRoles, action, request, clusterStateSupplier, indexNameExpressionResolver);
}
}

public IndexNameExpressionResolver getIndexNameExpressionResolver() {
return indexNameExpressionResolver;
}

public Supplier<ClusterState> getClusterStateSupplier() {
return clusterStateSupplier;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,14 @@ public PrivilegesEvaluatorResponse evaluate(
throw new OpenSearchSecurityException("OpenSearch Security is not initialized: roles configuration is missing");
}

PrivilegesEvaluationContext context = new PrivilegesEvaluationContext(user, mappedRoles, action0, request, resolver);
PrivilegesEvaluationContext context = new PrivilegesEvaluationContext(
user,
mappedRoles,
action0,
request,
clusterService::state,
resolver
);

if (request instanceof BulkRequest && (Strings.isNullOrEmpty(user.getRequestedTenant()))) {
// Shortcut for bulk actions. The details are checked on the lower level of the BulkShardRequests (Action
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -489,8 +490,12 @@ public Set<String> getAllIndices() {
}

public Set<String> getAllIndicesResolved(ClusterService clusterService, IndexNameExpressionResolver resolver) {
return getAllIndicesResolved(clusterService::state, resolver);
}

public Set<String> getAllIndicesResolved(Supplier<ClusterState> clusterStateSupplier, IndexNameExpressionResolver resolver) {
if (isLocalAll) {
return new HashSet<>(Arrays.asList(resolver.concreteIndexNames(clusterService.state(), indicesOptions, "*")));
return new HashSet<>(Arrays.asList(resolver.concreteIndexNames(clusterStateSupplier.get(), indicesOptions, "*")));
} else {
return allIndices;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ static IndexResolverReplacer.Resolved resolved(String... indices) {
return new IndexResolverReplacer.Resolved(
ImmutableSet.of(),
ImmutableSet.copyOf(indices),
ImmutableSet.of(),
ImmutableSet.copyOf(indices),
ImmutableSet.of(),
IndicesOptions.LENIENT_EXPAND_OPEN
);
Expand Down Expand Up @@ -444,7 +444,7 @@ static IndexResolverReplacer.Resolved resolved(String... indices) {
return new IndexResolverReplacer.Resolved(
ImmutableSet.of(),
allIndices.build(),
ImmutableSet.of(),
ImmutableSet.copyOf(indices),
ImmutableSet.of(),
IndicesOptions.LENIENT_EXPAND_OPEN
);
Expand Down Expand Up @@ -582,6 +582,7 @@ static PrivilegesEvaluationContext ctx(String... roles) {
ImmutableSet.copyOf(roles),
null,
null,
null,
new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY))
);
}
Expand Down
Loading

0 comments on commit 433e5cd

Please sign in to comment.