Skip to content

Commit 4a8ad4c

Browse files
bharath-techiebharath-techie
committed
Addressing comments
Signed-off-by: Bharathwaj G <[email protected]>
1 parent eeff45e commit 4a8ad4c

File tree

2 files changed

+21
-68
lines changed

2 files changed

+21
-68
lines changed

‎src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java

Lines changed: 21 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,3 @@
1-
/*
2-
* Copyright 2015-2018 _floragunn_ GmbH
3-
* Licensed under the Apache License, Version 2.0 (the "License");
4-
* you may not use this file except in compliance with the License.
5-
* You may obtain a copy of the License at
6-
*
7-
* http://www.apache.org/licenses/LICENSE-2.0
8-
*
9-
* Unless required by applicable law or agreed to in writing, software
10-
* distributed under the License is distributed on an "AS IS" BASIS,
11-
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12-
* See the License for the specific language governing permissions and
13-
* limitations under the License.
14-
*/
15-
161
/*
172
* SPDX-License-Identifier: Apache-2.0
183
*
@@ -32,18 +17,13 @@
3217
import java.util.List;
3318
import java.util.Map;
3419
import java.util.Set;
35-
import java.util.concurrent.CountDownLatch;
36-
import java.util.concurrent.TimeUnit;
3720
import java.util.stream.Collectors;
3821

3922
import com.google.common.collect.ImmutableSet;
4023
import org.apache.logging.log4j.LogManager;
4124
import org.apache.logging.log4j.Logger;
4225

43-
import org.opensearch.OpenSearchException;
44-
import org.opensearch.action.ActionListener;
4526
import org.opensearch.action.ActionRequest;
46-
import org.opensearch.action.LatchedActionListener;
4727
import org.opensearch.action.admin.indices.segments.PitSegmentsRequest;
4828
import org.opensearch.action.search.DeletePitRequest;
4929
import org.opensearch.action.search.GetAllPitNodesRequest;
@@ -52,7 +32,6 @@
5232
import org.opensearch.action.search.SearchRequest;
5333
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
5434
import org.opensearch.cluster.service.ClusterService;
55-
import org.opensearch.common.unit.TimeValue;
5635
import org.opensearch.security.OpenSearchSecurityPlugin;
5736
import org.opensearch.security.resolver.IndexResolverReplacer;
5837
import org.opensearch.security.securityconf.SecurityRoles;
@@ -71,27 +50,22 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C
7150
final IndexNameExpressionResolver resolver,
7251
boolean dnfOfEmptyResultsEnabled, final PrivilegesEvaluatorResponse presponse) {
7352

74-
// Skip pit evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster
75-
// for privilege evaluation
76-
if(action.startsWith("cluster:admin")) {
53+
// Skip custom evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster
54+
// for privilege evaluation - still this action will be evaluated in the generic PrivilegesEvaluator flow
55+
if(action.startsWith("cluster:admin/point_in_time")) {
7756
return presponse;
7857
}
79-
try {
80-
if (request instanceof GetAllPitNodesRequest) {
81-
return handleGetAllPitsAccess(request, clusterService, user, securityRoles,
82-
action, resolver, dnfOfEmptyResultsEnabled, presponse);
83-
} else if (request instanceof DeletePitRequest) {
84-
DeletePitRequest deletePitRequest = (DeletePitRequest) request;
85-
return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles,
86-
action, resolver, dnfOfEmptyResultsEnabled, presponse);
87-
} else if (request instanceof PitSegmentsRequest) {
88-
PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request;
89-
return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles,
90-
action, resolver, dnfOfEmptyResultsEnabled, presponse);
91-
}
92-
} catch(InterruptedException e) {
93-
Thread.currentThread().interrupt();
94-
log.error(e.toString());
58+
if (request instanceof GetAllPitNodesRequest) {
59+
return handleGetAllPitsAccess(request, clusterService, user, securityRoles,
60+
action, resolver, dnfOfEmptyResultsEnabled, presponse);
61+
} else if (request instanceof DeletePitRequest) {
62+
DeletePitRequest deletePitRequest = (DeletePitRequest) request;
63+
return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles,
64+
action, resolver, presponse);
65+
} else if (request instanceof PitSegmentsRequest) {
66+
PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request;
67+
return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles,
68+
action, resolver, presponse);
9569
}
9670
return presponse;
9771
}
@@ -102,7 +76,7 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C
10276
private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest request, final ClusterService clusterService,
10377
final User user, SecurityRoles securityRoles, final String action,
10478
IndexNameExpressionResolver resolver,
105-
boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException {
79+
boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) {
10680
List<ListPitInfo> pitInfos = ((GetAllPitNodesRequest) request).getGetAllPitNodesResponse().getPitInfos();
10781
// if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled
10882
// config property is true, otherwise fail the operation
@@ -126,17 +100,12 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r
126100
String[] indices = pitToIndicesMap.get(pitId);
127101
HashSet<String> indicesSet = new HashSet<>(Arrays.asList(indices));
128102

129-
final ImmutableSet<String> INDICES_SET = ImmutableSet.copyOf(indicesSet);
130-
final IndexResolverReplacer.Resolved pitResolved =
131-
new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET,
132-
ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS);
133-
134-
final Set<String> allPermittedIndices = securityRoles.reduce(pitResolved,
135-
user, new String[]{action}, resolver, clusterService);
103+
final Set<String> allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user,
104+
securityRoles, action, resolver);
136105
if(isDebugEnabled) {
137106
log.debug("Evaluating PIT ID : " + pitId );
138107
}
139-
if (allPermittedIndices.size() == INDICES_SET.size()) {
108+
if (allPermittedIndices.size() == indicesSet.size()) {
140109
if(isDebugEnabled) {
141110
log.debug(" Permitting PIT ID : " + pitId);
142111
}
@@ -157,8 +126,7 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r
157126
*/
158127
private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List<String> pitIds, ClusterService clusterService,
159128
User user, SecurityRoles securityRoles, final String action,
160-
IndexNameExpressionResolver resolver,
161-
boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) {
129+
IndexNameExpressionResolver resolver, PrivilegesEvaluatorResponse presponse) {
162130
Map<String, String[]> pitToIndicesMap = OpenSearchSecurityPlugin.
163131
GuiceHolder.getPitService().getIndicesForPits(pitIds);
164132
Set<String> pitIndices = new HashSet<>();
@@ -183,9 +151,9 @@ private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List<String> pitIds
183151
private Set<String> getPermittedIndices(Set<String> pitIndices, ClusterService clusterService,
184152
User user, SecurityRoles securityRoles, final String action,
185153
IndexNameExpressionResolver resolver) {
186-
final ImmutableSet<String> INDICES_SET = ImmutableSet.copyOf(pitIndices);
154+
final ImmutableSet<String> pitImmutableIndices = ImmutableSet.copyOf(pitIndices);
187155
final IndexResolverReplacer.Resolved pitResolved =
188-
new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET,
156+
new IndexResolverReplacer.Resolved(pitImmutableIndices, pitImmutableIndices, pitImmutableIndices,
189157
ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS);
190158
return securityRoles.reduce(pitResolved,
191159
user, new String[]{action}, resolver, clusterService);

‎src/test/java/org/opensearch/security/PitIntegrationTests.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,3 @@
1-
/*
2-
* Copyright 2015-2018 _floragunn_ GmbH
3-
* Licensed under the Apache License, Version 2.0 (the "License");
4-
* you may not use this file except in compliance with the License.
5-
* You may obtain a copy of the License at
6-
*
7-
* http://www.apache.org/licenses/LICENSE-2.0
8-
*
9-
* Unless required by applicable law or agreed to in writing, software
10-
* distributed under the License is distributed on an "AS IS" BASIS,
11-
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12-
* See the License for the specific language governing permissions and
13-
* limitations under the License.
14-
*/
15-
161
/*
172
* SPDX-License-Identifier: Apache-2.0
183
*

0 commit comments

Comments
 (0)