Skip to content

Commit

Permalink
Removed need to mockito in perf test
Browse files Browse the repository at this point in the history 
Signed-off-by: Nils Bandener <[email protected]>
  • Loading branch information
@nibix
nibix committed Jul 2, 2024
1 parent 04d8a24 commit 7a2a7d3
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 2,589 deletions.
1 change: 0 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 @@ -1121,7 +1121,6 @@ public Collection<Object> createComponents(
// DLS-FLS is enabled if not client and not disabled and not SSL only.
final boolean dlsFlsEnabled = !SSLConfig.isSslOnlyMode();
evaluator = new PrivilegesEvaluator(
clusterService,
clusterService::state,
threadPool.getThreadContext(),
cr,
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public static boolean handle(
this.threadContext = threadContext;
this.resolver = resolver;

this.requiresIndexScoping = resolved.isLocalAll() || resolved.getAllIndicesResolved(clusterService, resolver).size() != 1;
this.requiresIndexScoping = resolved.isLocalAll() || resolved.getAllIndicesResolved(clusterService::state, resolver).size() != 1;
}

private boolean handle() {
Expand Down Expand Up @@ -473,7 +473,7 @@ private boolean modifyQuery(String localClusterAlias) throws IOException {

int queryCount = 0;

Set<String> indices = resolved.getAllIndicesResolved(clusterService, resolver);
Set<String> indices = resolved.getAllIndicesResolved(clusterService::state, resolver);

for (String index : indices) {
String dlsEval = SecurityUtils.evalMap(filterLevelQueries, index);
Expand Down
20 changes: 14 additions & 6 deletions src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

import org.opensearch.action.ActionRequest;
import org.opensearch.action.admin.indices.segments.PitSegmentsRequest;
import org.opensearch.action.search.CreatePitRequest;
import org.opensearch.action.search.DeletePitRequest;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.resolver.IndexResolverReplacer;
Expand All @@ -41,7 +44,9 @@ public PrivilegesEvaluatorResponse evaluate(
final SecurityRoles securityRoles,
final String action,
final PrivilegesEvaluatorResponse presponse,
final IndexResolverReplacer irr
final IndexResolverReplacer irr,
IndexNameExpressionResolver resolver,
Supplier<ClusterState> cs
) {

if (!(request instanceof DeletePitRequest || request instanceof PitSegmentsRequest)) {
Expand All @@ -60,7 +65,7 @@ public PrivilegesEvaluatorResponse evaluate(
if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) {
return presponse;
} else {
return handlePitsAccess(pitIds, user, securityRoles, action, presponse, irr);
return handlePitsAccess(pitIds, user, securityRoles, action, presponse, irr, resolver, cs);
}
}

Expand All @@ -73,15 +78,17 @@ private PrivilegesEvaluatorResponse handlePitsAccess(
SecurityRoles securityRoles,
final String action,
PrivilegesEvaluatorResponse presponse,
final IndexResolverReplacer irr
final IndexResolverReplacer irr,
IndexNameExpressionResolver resolver,
Supplier<ClusterState> cs
) {
Map<String, String[]> pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds);
Set<String> pitIndices = new HashSet<>();
// add indices across all PITs to a set and evaluate if user has access to all indices
for (String[] indices : pitToIndicesMap.values()) {
pitIndices.addAll(Arrays.asList(indices));
}
Set<String> allPermittedIndices = getPermittedIndices(pitIndices, clusterService, user, securityRoles, action, resolver, irr);
Set<String> allPermittedIndices = getPermittedIndices(pitIndices, user, securityRoles, action, resolver, irr, cs);
// Only if user has access to all PIT's indices, allow operation, otherwise continue evaluation in PrivilegesEvaluator.
if (allPermittedIndices.containsAll(pitIndices)) {
presponse.allowed = true;
Expand All @@ -99,11 +106,12 @@ private Set<String> getPermittedIndices(
SecurityRoles securityRoles,
final String action,
IndexNameExpressionResolver resolver,
final IndexResolverReplacer irr
final IndexResolverReplacer irr,
Supplier<ClusterState> cs
) {
String[] indicesArr = new String[pitIndices.size()];
CreatePitRequest req = new CreatePitRequest(new TimeValue(1, TimeUnit.DAYS), true, pitIndices.toArray(indicesArr));
final IndexResolverReplacer.Resolved pitResolved = irr.resolveRequest(req);
return securityRoles.reduce(pitResolved, user, new String[] { action }, resolver, clusterService);
return securityRoles.reduce(pitResolved, user, new String[] { action }, resolver, cs);
}
}
17 changes: 9 additions & 8 deletions src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import org.opensearch.action.termvectors.MultiTermVectorsAction;
import org.opensearch.action.update.UpdateAction;
import org.opensearch.cluster.ClusterChangedEvent;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateListener;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexMetadata;
Expand Down Expand Up @@ -148,7 +149,6 @@ public class PrivilegesEvaluator {
private final NamedXContentRegistry namedXContentRegistry;

public PrivilegesEvaluator(
final ClusterService clusterService,
Supplier<ClusterState> clusterStateSupplier,
final ThreadContext threadContext,
final ConfigurationRepository configurationRepository,
Expand Down Expand Up @@ -329,7 +329,8 @@ public PrivilegesEvaluatorResponse evaluate(
presponse,
securityRoles,
user,
resolver
resolver,
clusterStateSupplier
).isComplete()) {
return presponse;
}
Expand All @@ -340,7 +341,7 @@ public PrivilegesEvaluatorResponse evaluate(
}

// check access for point in time requests
if (pitPrivilegesEvaluator.evaluate(request, clusterService, user, securityRoles, action0, resolver, presponse, irr).isComplete()) {
if (pitPrivilegesEvaluator.evaluate(request, user, securityRoles, action0, presponse, irr, resolver, clusterStateSupplier).isComplete()) {
return presponse;
}

Expand All @@ -355,7 +356,7 @@ public PrivilegesEvaluatorResponse evaluate(
user,
dfmEmptyOverwritesAll,
resolver,
clusterService,
clusterStateSupplier,
namedXContentRegistry
);

Expand Down Expand Up @@ -428,7 +429,7 @@ public PrivilegesEvaluatorResponse evaluate(
}

// term aggregations
if (termsAggregationEvaluator.evaluate(requestedResolved, request, clusterService, user, securityRoles, resolver, presponse)
if (termsAggregationEvaluator.evaluate(requestedResolved, request, user, securityRoles, presponse, resolver, clusterStateSupplier)
.isComplete()) {
return presponse;
}
Expand Down Expand Up @@ -484,7 +485,7 @@ public PrivilegesEvaluatorResponse evaluate(
return presponse;
}

Set<String> reduced = securityRoles.reduce(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterService);
Set<String> reduced = securityRoles.reduce(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterStateSupplier);

if (reduced.isEmpty()) {
if (dcm.isDnfofForEmptyResultsEnabled() && request instanceof IndicesRequest.Replaceable) {
Expand Down Expand Up @@ -522,9 +523,9 @@ public PrivilegesEvaluatorResponse evaluate(
}

if (dcm.isMultiRolespanEnabled()) {
permGiven = securityRoles.impliesTypePermGlobal(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterService);
permGiven = securityRoles.impliesTypePermGlobal(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterStateSupplier);
} else {
permGiven = securityRoles.get(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterService);
permGiven = securityRoles.get(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterStateSupplier);

}

Expand Down
15 changes: 11 additions & 4 deletions src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.apache.logging.log4j.LogManager;
Expand All @@ -38,6 +39,8 @@
import org.opensearch.action.ActionRequest;
import org.opensearch.action.RealtimeRequest;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.common.settings.Settings;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.resolver.IndexResolverReplacer;
Expand Down Expand Up @@ -125,9 +128,11 @@ public PrivilegesEvaluatorResponse evaluate(
final Resolved requestedResolved,
final PrivilegesEvaluatorResponse presponse,
final SecurityRoles securityRoles,
final User user
final User user,
IndexNameExpressionResolver resolver,
Supplier<ClusterState> cs
) {
evaluateSystemIndicesAccess(action, requestedResolved, request, task, presponse, securityRoles, user, resolver, clusterService);
evaluateSystemIndicesAccess(action, requestedResolved, request, task, presponse, securityRoles, user, resolver, cs);

if (requestedResolved.isLocalAll()
|| requestedResolved.getAllIndices().contains(securityIndex)
Expand Down Expand Up @@ -238,7 +243,9 @@ private void evaluateSystemIndicesAccess(
final Task task,
final PrivilegesEvaluatorResponse presponse,
SecurityRoles securityRoles,
final User user
final User user,
IndexNameExpressionResolver resolver,
Supplier<ClusterState> cs
) {
// Perform access check is system index permissions are enabled
boolean containsSystemIndex = requestContainsAnySystemIndices(requestedResolved);
Expand Down Expand Up @@ -284,7 +291,7 @@ private void evaluateSystemIndicesAccess(
user,
new String[] { ConfigConstants.SYSTEM_INDEX_PERMISSION },
resolver,
clusterService
cs
)) {
auditLog.logSecurityIndexAttempt(request, action, task);
if (log.isInfoEnabled()) {
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@
package org.opensearch.security.privileges;

import java.util.Set;
import java.util.function.Supplier;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.action.ActionRequest;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.index.query.MatchNoneQueryBuilder;
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.index.query.TermsQueryBuilder;
Expand Down Expand Up @@ -64,7 +67,9 @@ public PrivilegesEvaluatorResponse evaluate(
final ActionRequest request,
User user,
SecurityRoles securityRoles,
PrivilegesEvaluatorResponse presponse
PrivilegesEvaluatorResponse presponse,
IndexNameExpressionResolver resolver,
Supplier<ClusterState> cs
) {
try {
if (request instanceof SearchRequest) {
Expand All @@ -85,7 +90,9 @@ public PrivilegesEvaluatorResponse evaluate(
final Set<String> allPermittedIndices = securityRoles.getAllPermittedIndicesForDashboards(
resolved,
user,
READ_ACTIONS
READ_ACTIONS,
resolver,
cs
);
if (allPermittedIndices == null || allPermittedIndices.isEmpty()) {
sr.source().query(NONE_QUERY);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java
View file
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 @@ -488,9 +489,9 @@ public Set<String> getAllIndices() {
return allIndices;
}

public Set<String> getAllIndicesResolved(ClusterService clusterService, IndexNameExpressionResolver resolver) {
public Set<String> getAllIndicesResolved(Supplier<ClusterState> clusterService, IndexNameExpressionResolver resolver) {
if (isLocalAll) {
return new HashSet<>(Arrays.asList(resolver.concreteIndexNames(clusterService.state(), indicesOptions, "*")));
return new HashSet<>(Arrays.asList(resolver.concreteIndexNames(clusterService.get(), indicesOptions, "*")));
} else {
return allIndices;
}
Expand Down
Loading

0 comments on commit 7a2a7d3

Please sign in to comment.