From d5c482e123abcd10da1140043b5525527bc32c38 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Sun, 28 Aug 2022 22:39:43 +0530 Subject: [PATCH 1/8] Changes to make PIT security model granular Signed-off-by: Bharathwaj G --- .../security/OpenSearchSecurityPlugin.java | 9 +- .../privileges/PitAccessEvaluator.java | 344 ++++++++++++++++++ .../privileges/PrivilegesEvaluator.java | 8 + .../resolver/IndexResolverReplacer.java | 10 +- .../static_config/static_action_groups.yml | 8 +- .../security/PitIntegrationTests.java | 179 +++++++++ src/test/resources/internal_users.yml | 6 + src/test/resources/roles.yml | 30 ++ src/test/resources/roles_mapping.yml | 10 + 9 files changed, 595 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java create mode 100644 src/test/java/org/opensearch/security/PitIntegrationTests.java diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 66530cfaed..5d6ab306b2 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -66,6 +66,7 @@ import org.opensearch.Version; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionResponse; +import org.opensearch.action.search.PitService; import org.opensearch.action.search.SearchScrollAction; import org.opensearch.action.support.ActionFilter; import org.opensearch.client.Client; @@ -1160,13 +1161,15 @@ public static class GuiceHolder implements LifecycleComponent { private static RepositoriesService repositoriesService; private static RemoteClusterService remoteClusterService; private static IndicesService indicesService; + private static PitService pitService; @Inject public GuiceHolder(final RepositoriesService repositoriesService, - final TransportService remoteClusterService, IndicesService indicesService) { + final TransportService remoteClusterService, IndicesService indicesService, PitService pitService) { GuiceHolder.repositoriesService = repositoriesService; GuiceHolder.remoteClusterService = remoteClusterService.getRemoteClusterService(); GuiceHolder.indicesService = indicesService; + GuiceHolder.pitService = pitService; } public static RepositoriesService getRepositoriesService() { @@ -1180,6 +1183,10 @@ public static RemoteClusterService getRemoteClusterService() { public static IndicesService getIndicesService() { return indicesService; } + + public static PitService getPitService() { + return pitService; + } @Override public void close() { diff --git a/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java new file mode 100644 index 0000000000..4708e064ae --- /dev/null +++ b/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java @@ -0,0 +1,344 @@ +/* + * Copyright 2015-2018 _floragunn_ GmbH + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ +package org.opensearch.security.privileges; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import com.google.common.collect.ImmutableSet; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.OpenSearchException; +import org.opensearch.action.ActionListener; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.action.admin.indices.segments.PitSegmentsRequest; +import org.opensearch.action.search.DeletePitRequest; +import org.opensearch.action.search.GetAllPitNodesRequest; +import org.opensearch.action.search.GetAllPitNodesResponse; +import org.opensearch.action.search.ListPitInfo; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.security.resolver.IndexResolverReplacer; +import org.opensearch.security.securityconf.SecurityRoles; +import org.opensearch.security.user.User; + +/** + * This class evaluates privileges for point in time (Delete and List all) operations + */ +public class PitAccessEvaluator { + + protected final Logger log = LogManager.getLogger(this.getClass()); + private boolean isDebugEnabled = log.isDebugEnabled(); + + public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final ClusterService clusterService, + final User user, final SecurityRoles securityRoles, final String action, + final IndexNameExpressionResolver resolver, + final PrivilegesEvaluatorResponse presponse) { + + // Skip pit evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster + // for privilege evaluation + if(action.startsWith("cluster:admin")) { + return presponse; + } + try { + if (request instanceof GetAllPitNodesRequest) { + if (((GetAllPitNodesRequest) request).getGetAllPitNodesResponse() != null) { + return presponse; + } + return handleGetAllPitsAccess(request, clusterService, user, securityRoles, + action, resolver, presponse); + } else if (request instanceof DeletePitRequest) { + DeletePitRequest deletePitRequest = (DeletePitRequest) request; + List pitIds = deletePitRequest.getPitIds(); + if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { + return handleDeleteAllPitAccess(deletePitRequest, clusterService, user, securityRoles, + action, resolver, presponse); + } else { + return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, presponse); + } + } else if (request instanceof PitSegmentsRequest) { + PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; + List pitIds = pitSegmentsRequest.getPitIds(); + if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { + return handleGetAllPitSegmentsAccess(pitSegmentsRequest, clusterService, user, securityRoles, + action, resolver, presponse); + } else { + return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, presponse); + } + } + } catch(InterruptedException e) { + Thread.currentThread().interrupt(); + log.error(e.toString()); + } + return presponse; + } + + /** + * Handle access for Get All PITs access + */ + private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest request, final ClusterService clusterService, + final User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + PrivilegesEvaluatorResponse presponse) throws InterruptedException { + List pitInfos = getAllPitInfos((GetAllPitNodesRequest) request); + // if cluster has no PITs, then allow the operation to pass with empty response + if(pitInfos.isEmpty()) { + presponse.allowed = true; + presponse.markComplete(); + } + List pitIds = new ArrayList<>(); + pitIds.addAll(pitInfos.stream().map(ListPitInfo::getPitId).collect(Collectors.toList())); + Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); + Map pitToPitInfoMap = new HashMap<>(); + + for(ListPitInfo pitInfo : pitInfos) { + pitToPitInfoMap.put(pitInfo.getPitId(), pitInfo); + } + List permittedPits = new ArrayList<>(); + for (String pitId : pitIds) { + String[] indices = pitToIndicesMap.get(pitId); + HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); + + final ImmutableSet INDICES_SET = ImmutableSet.copyOf(indicesSet); + final IndexResolverReplacer.Resolved pitResolved = + new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET, + ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS); + + final Set allPermittedIndices = securityRoles.reduce(pitResolved, + user, new String[]{action}, resolver, clusterService); + if(isDebugEnabled) { + log.debug("Evaluating PIT ID : " + pitId ); + } + if (allPermittedIndices.size() == INDICES_SET.size()) { + if(isDebugEnabled) { + log.debug(" Permitting PIT ID : " + pitId); + } + permittedPits.add(pitToPitInfoMap.get(pitId)); + } + } + if (permittedPits.size() > 0) { + ((GetAllPitNodesRequest) request).setGetAllPitNodesResponse(new GetAllPitNodesResponse(permittedPits, + ((GetAllPitNodesRequest) request).getGetAllPitNodesResponse())); + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; + } + + /** + * Handle access for 'delete all PITs' operation + */ + private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest deletePitRequest, ClusterService clusterService, + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + PrivilegesEvaluatorResponse presponse) throws InterruptedException { + List permittedPits = new ArrayList<>(); + List pitIds = getAllPitIds(); + // allow delete pit operation if there are no pits in the cluster ( response should be empty ) + if(pitIds.isEmpty()) { + deletePitRequest.clearAndSetPitIds(pitIds); + presponse.allowed = true; + presponse.markComplete(); + } + Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); + for (String pitId : pitIds) { + String[] indices = pitToIndicesMap.get(pitId); + HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); + Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, + securityRoles, action, resolver); + // user should have permissions for all indices associated with PIT, only then add PIT ID as permitted PIT + if(isDebugEnabled) { + log.debug("Evaluating PIT : " + pitId ); + } + if (allPermittedIndices.size() == indicesSet.size()) { + if(isDebugEnabled) { + log.debug(" Permitting PIT : " + pitId); + } + permittedPits.add(pitId); + } + } + // If there are any PITs for which the user has access to, then allow operation otherwise fail. + if(permittedPits.size() > 0) { + deletePitRequest.clearAndSetPitIds(permittedPits); + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; + } + + /** + * Handle access for PIT segments API + */ + private PrivilegesEvaluatorResponse handleGetAllPitSegmentsAccess(PitSegmentsRequest pitSegmentsRequest, ClusterService clusterService, + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + PrivilegesEvaluatorResponse presponse) throws InterruptedException { + List permittedPits = new ArrayList<>(); + List pitIds = getAllPitIds(); + // allow pit segments operation if there are no pits in the cluster ( response should be empty ) + if(pitIds.isEmpty()) { + pitSegmentsRequest.clearAndSetPitIds(pitIds); + presponse.allowed = true; + presponse.markComplete(); + } + Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); + for (String pitId : pitIds) { + String[] indices = pitToIndicesMap.get(pitId); + HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); + Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, + securityRoles, action, resolver); + // user should have permissions for all indices associated with PIT, only then add PIT ID as permitted PIT + if(isDebugEnabled) { + log.debug("Evaluating PIT : " + pitId ); + } + if (allPermittedIndices.size() == indicesSet.size()) { + if(isDebugEnabled) { + log.debug(" Permitting PIT : " + pitId); + } + permittedPits.add(pitId); + } + } + // If there are any PITs for which the user has access to, then allow operation otherwise fail. + if(permittedPits.size() > 0) { + pitSegmentsRequest.clearAndSetPitIds(permittedPits); + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; + } + + /** + * Handle access for delete operation / pit segments operation where PIT IDs are explicitly passed + */ + private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List pitIds, ClusterService clusterService, + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + PrivilegesEvaluatorResponse presponse) { + Map pitToIndicesMap = OpenSearchSecurityPlugin. + GuiceHolder.getPitService().getIndicesForPits(pitIds); + Set 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 allPermittedIndices = getPermittedIndices(pitIndices, clusterService, user, + securityRoles, action, resolver); + // In this case, PIT IDs are explicitly passed. + // So, only if user has access to all PIT's indices, allow delete operation, otherwise fail. + if(pitIndices.size() == allPermittedIndices.size()) { + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; + } + + /** + * This method returns list of permitted indices for the PIT indices passed + */ + private Set getPermittedIndices(Set pitIndices, ClusterService clusterService, + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver) { + final ImmutableSet INDICES_SET = ImmutableSet.copyOf(pitIndices); + final IndexResolverReplacer.Resolved pitResolved = + new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET, + ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS); + return securityRoles.reduce(pitResolved, + user, new String[]{action}, resolver, clusterService); + } + + /** + * Get all active PITs + */ + private List getAllPitInfos(GetAllPitNodesRequest request) throws InterruptedException { + final List pitInfos = new ArrayList<>(); + final CountDownLatch latch = new CountDownLatch(1); + ActionListener listener = new ActionListener() { + @Override + public void onResponse(GetAllPitNodesResponse response) { + pitInfos.addAll(response.getPitInfos()); + request.setGetAllPitNodesResponse(response); + } + + @Override + public void onFailure(Exception e) { + throw new OpenSearchException("List all PITs failed", e); + } + }; + LatchedActionListener latchedActionListener = new LatchedActionListener<>(listener, latch); + + OpenSearchSecurityPlugin.GuiceHolder.getPitService().getAllPits(latchedActionListener); + + if(!latch.await(15, TimeUnit.SECONDS)) { + log.warn("Failed to get all PITs information within the timeout {}", new TimeValue(15, TimeUnit.SECONDS)); + } + return pitInfos; + } + + /** + * Get all active PIT IDs + */ + private List getAllPitIds() throws InterruptedException { + + final List pitInfos = new ArrayList<>(); + final CountDownLatch latch = new CountDownLatch(1); + + ActionListener listener = new ActionListener() { + @Override + public void onResponse(GetAllPitNodesResponse response) { + pitInfos.addAll(response.getPitInfos()); + } + + @Override + public void onFailure(Exception e) { + throw new OpenSearchException("List all PITs failed", e); + + } + }; + LatchedActionListener latchedActionListener = new LatchedActionListener<>(listener, latch); + + OpenSearchSecurityPlugin.GuiceHolder.getPitService().getAllPits(latchedActionListener); + + if(!latch.await(15, TimeUnit.SECONDS)) { + log.warn("Failed to get all PITs information within the timeout {}", new TimeValue(15, TimeUnit.SECONDS)); + } + return pitInfos.stream().map(r -> r.getPitId()).collect(Collectors.toList()); + } +} diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index fd1b26d388..169c7b0510 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -130,6 +130,7 @@ public class PrivilegesEvaluator { private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator; private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator; private final TermsAggregationEvaluator termsAggregationEvaluator; + private final PitAccessEvaluator pitAccessEvaluator; private final boolean dlsFlsEnabled; private final boolean dfmEmptyOverwritesAll; private DynamicConfigModel dcm; @@ -158,6 +159,7 @@ public PrivilegesEvaluator(final ClusterService clusterService, final ThreadPool securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr); protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog); termsAggregationEvaluator = new TermsAggregationEvaluator(); + pitAccessEvaluator = new PitAccessEvaluator(); this.namedXContentRegistry = namedXContentRegistry; this.dlsFlsEnabled = dlsFlsEnabled; this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false); @@ -282,6 +284,12 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin return presponse; } + // check access for point in time requests + if(pitAccessEvaluator.evaluate(request, clusterService, user, securityRoles, + action0, resolver, presponse).isComplete()) { + return presponse; + } + final boolean dnfofEnabled = dcm.isDnfofEnabled(); final boolean isTraceEnabled = log.isTraceEnabled(); diff --git a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java index e0eddf9993..d2d0685860 100644 --- a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java @@ -370,11 +370,11 @@ public final static class Resolved { private final boolean isLocalAll; private final IndicesOptions indicesOptions; - private Resolved(final ImmutableSet aliases, - final ImmutableSet allIndices, - final ImmutableSet originalRequested, - final ImmutableSet remoteIndices, - IndicesOptions indicesOptions) { + public Resolved(final ImmutableSet aliases, + final ImmutableSet allIndices, + final ImmutableSet originalRequested, + final ImmutableSet remoteIndices, + IndicesOptions indicesOptions) { this.aliases = aliases; this.allIndices = allIndices; this.originalRequested = originalRequested; diff --git a/src/main/resources/static_config/static_action_groups.yml b/src/main/resources/static_config/static_action_groups.yml index d0ce7613a2..85af25e9fc 100644 --- a/src/main/resources/static_config/static_action_groups.yml +++ b/src/main/resources/static_config/static_action_groups.yml @@ -233,8 +233,10 @@ manage_point_in_time: static: true allowed_actions: - "indices:data/read/point_in_time/create" - - "cluster:admin/point_in_time/delete" - - "cluster:admin/point_in_time/read*" + - "indices:data/read/point_in_time/delete" + - "indices:data/read/point_in_time/readall" + - "indices:data/read/search" + - "cluster:admin/point_in_time/read_from_nodes" - "indices:monitor/point_in_time/segments" - type: "cluster" + type: "index" description: "Manage point in time actions" diff --git a/src/test/java/org/opensearch/security/PitIntegrationTests.java b/src/test/java/org/opensearch/security/PitIntegrationTests.java new file mode 100644 index 0000000000..b9fc4537c7 --- /dev/null +++ b/src/test/java/org/opensearch/security/PitIntegrationTests.java @@ -0,0 +1,179 @@ +/* + * Copyright 2015-2018 _floragunn_ GmbH + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ +package org.opensearch.security; + +import org.apache.http.HttpStatus; +import org.junit.Assert; +import org.junit.Test; + +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.support.WriteRequest; +import org.opensearch.client.Client; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.security.test.SingleClusterTest; +import org.opensearch.security.test.helper.rest.RestHelper; + +/** + * Integration tests to test point in time APIs permission model + */ +public class PitIntegrationTests extends SingleClusterTest { + + @Test + public void pitCreateWithDeleteAll() throws Exception { + setup(); + RestHelper rh = nonSslRestHelper(); + + // Create two indices + try (Client tc = getClient()) { + tc.index(new IndexRequest("pit_1").id("1").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE). + source("{\"content\":1}", XContentType.JSON)).actionGet(); + tc.index(new IndexRequest("pit_2").id("2").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE). + source("{\"content\":2}", XContentType.JSON)).actionGet(); + } + + RestHelper.HttpResponse resc; + + // Create point in time in index should be successful since the user has permission for index + resc = rh.executePostRequest("/pit_1/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Create point in time in index for which the user does not have permission + resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); + + // Create point in time in index for which the user has permission for + resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Delete all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Delete all PITs should throw error since there are no PITs for which the user has access for + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); + + // Delete all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Delete all PITs should work since there are no PITs in the cluster + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_NOT_FOUND, resc.getStatusCode()); + } + + @Test + public void pitCreateWithGetAll() throws Exception { + setup(); + RestHelper rh = nonSslRestHelper(); + + // Create two indices + try (Client tc = getClient()) { + tc.index(new IndexRequest("pit_1").id("1").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE). + source("{\"content\":1}", XContentType.JSON)).actionGet(); + tc.index(new IndexRequest("pit_2").id("2").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE). + source("{\"content\":2}", XContentType.JSON)).actionGet(); + } + + RestHelper.HttpResponse resc; + + // Create point in time in index should be successful since the user has permission for index + resc = rh.executePostRequest("/pit_1/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Create point in time in index for which the user does not have permission + resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); + + // Create point in time in index for which the user has permission for + resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // List all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeGetRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // PIT segments should work since there is atleast one PIT for which user has access for + resc = rh.executeGetRequest("/_cat/pit_segments/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + Thread.sleep(500); + + // Delete all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + + // List all PITs should throw error since there are no PITs for which the user has access for + resc = rh.executeGetRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); + + // PIT segments should throw error since there are PITs in system but no PIT for which user has access for + resc = rh.executeGetRequest("/_cat/pit_segments/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); + + // List all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeGetRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // PIT segments should work since there is atleast one PIT for which user has access for + resc = rh.executeGetRequest("/_cat/pit_segments/_all", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // Delete all PITs should work since there is atleast one PIT for which user has access for + resc = rh.executeDeleteRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + + // List all PITs should work since there are no PITs in the cluster + resc = rh.executeGetRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_NOT_FOUND, resc.getStatusCode()); + + + // PIT segments should work since there is atleast one PIT for which user has access for + resc = rh.executeGetRequest("/_cat/pit_segments/_all", + encodeBasicHeader("pit-2", "nagilum")); + Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + } +} \ No newline at end of file diff --git a/src/test/resources/internal_users.yml b/src/test/resources/internal_users.yml index 99d821ce33..3344ecdad0 100644 --- a/src/test/resources/internal_users.yml +++ b/src/test/resources/internal_users.yml @@ -346,6 +346,12 @@ ds2: ds3: hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m #password is: nagilum +pit-1: + hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m + #password is: nagilum +pit-2: + hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m + #password is: nagilum hidden_test: hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m opendistro_security_roles: diff --git a/src/test/resources/roles.yml b/src/test/resources/roles.yml index 20e7c38cdb..a66ee62485 100644 --- a/src/test/resources/roles.yml +++ b/src/test/resources/roles.yml @@ -1128,7 +1128,37 @@ data_stream_3: - "*" allowed_actions: - "DATASTREAM_ALL" +point_in_time_1: + reserved: true + hidden: false + description: "Migrated from v6 (all types mapped)" + cluster_permissions: + - "cluster:admin/point_in_time/read_from_nodes" + - "cluster_monitor" + index_permissions: + - index_patterns: + - "pit_1" + dls: null + fls: null + masked_fields: null + allowed_actions: + - "manage_point_in_time" +point_in_time_2: + reserved: true + hidden: false + description: "Migrated from v6 (all types mapped)" + cluster_permissions: + - "cluster:admin/point_in_time/read_from_nodes" + - "cluster_monitor" + index_permissions: + - index_patterns: + - "pit_2" + dls: null + fls: null + masked_fields: null + allowed_actions: + - "manage_point_in_time" hidden_test: cluster_permissions: - SGS_CLUSTER_COMPOSITE_OPS diff --git a/src/test/resources/roles_mapping.yml b/src/test/resources/roles_mapping.yml index 9253b0c970..c5ce58af9b 100644 --- a/src/test/resources/roles_mapping.yml +++ b/src/test/resources/roles_mapping.yml @@ -413,6 +413,16 @@ data_stream_3: hidden: false users: - "ds3" +point_in_time_1: + reserved: false + hidden: false + users: + - "pit-1" +point_in_time_2: + reserved: false + hidden: false + users: + - "pit-2" sem-role: reserved: false hidden: false From 953748f802be2680a4300059531c18fbd929f187 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Mon, 5 Sep 2022 18:36:25 +0530 Subject: [PATCH 2/8] Addressing comments , using dnfOfEnabled Signed-off-by: Bharathwaj G --- .../privileges/PitAccessEvaluator.java | 55 +++++++++++-------- .../privileges/PrivilegesEvaluator.java | 2 +- .../security/PitIntegrationTests.java | 19 +++---- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java index 4708e064ae..48e7c753c2 100644 --- a/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java @@ -69,7 +69,7 @@ public class PitAccessEvaluator { public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final ClusterService clusterService, final User user, final SecurityRoles securityRoles, final String action, final IndexNameExpressionResolver resolver, - final PrivilegesEvaluatorResponse presponse) { + boolean dnfofEnabled, final PrivilegesEvaluatorResponse presponse) { // Skip pit evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster // for privilege evaluation @@ -82,26 +82,26 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C return presponse; } return handleGetAllPitsAccess(request, clusterService, user, securityRoles, - action, resolver, presponse); + action, resolver, dnfofEnabled, presponse); } else if (request instanceof DeletePitRequest) { DeletePitRequest deletePitRequest = (DeletePitRequest) request; List pitIds = deletePitRequest.getPitIds(); if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { return handleDeleteAllPitAccess(deletePitRequest, clusterService, user, securityRoles, - action, resolver, presponse); + action, resolver, dnfofEnabled, presponse); } else { return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, presponse); + action, resolver, dnfofEnabled, presponse); } } else if (request instanceof PitSegmentsRequest) { PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; List pitIds = pitSegmentsRequest.getPitIds(); if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { return handleGetAllPitSegmentsAccess(pitSegmentsRequest, clusterService, user, securityRoles, - action, resolver, presponse); + action, resolver, dnfofEnabled, presponse); } else { return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, presponse); + action, resolver, dnfofEnabled, presponse); } } } catch(InterruptedException e) { @@ -115,14 +115,17 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C * Handle access for Get All PITs access */ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest request, final ClusterService clusterService, - final User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - PrivilegesEvaluatorResponse presponse) throws InterruptedException { + final User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List pitInfos = getAllPitInfos((GetAllPitNodesRequest) request); // if cluster has no PITs, then allow the operation to pass with empty response if(pitInfos.isEmpty()) { - presponse.allowed = true; - presponse.markComplete(); + if(dnfofEnabled) { + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; } List pitIds = new ArrayList<>(); pitIds.addAll(pitInfos.stream().map(ListPitInfo::getPitId).collect(Collectors.toList())); @@ -167,16 +170,19 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r * Handle access for 'delete all PITs' operation */ private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest deletePitRequest, ClusterService clusterService, - User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - PrivilegesEvaluatorResponse presponse) throws InterruptedException { + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List permittedPits = new ArrayList<>(); List pitIds = getAllPitIds(); // allow delete pit operation if there are no pits in the cluster ( response should be empty ) if(pitIds.isEmpty()) { - deletePitRequest.clearAndSetPitIds(pitIds); - presponse.allowed = true; - presponse.markComplete(); + if(dnfofEnabled) { + deletePitRequest.clearAndSetPitIds(pitIds); + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; } Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); for (String pitId : pitIds) { @@ -208,16 +214,19 @@ private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest de * Handle access for PIT segments API */ private PrivilegesEvaluatorResponse handleGetAllPitSegmentsAccess(PitSegmentsRequest pitSegmentsRequest, ClusterService clusterService, - User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - PrivilegesEvaluatorResponse presponse) throws InterruptedException { + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List permittedPits = new ArrayList<>(); List pitIds = getAllPitIds(); // allow pit segments operation if there are no pits in the cluster ( response should be empty ) if(pitIds.isEmpty()) { - pitSegmentsRequest.clearAndSetPitIds(pitIds); - presponse.allowed = true; - presponse.markComplete(); + if(dnfofEnabled) { + pitSegmentsRequest.clearAndSetPitIds(pitIds); + presponse.allowed = true; + presponse.markComplete(); + } + return presponse; } Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); for (String pitId : pitIds) { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 169c7b0510..0cc850ecc6 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -286,7 +286,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin // check access for point in time requests if(pitAccessEvaluator.evaluate(request, clusterService, user, securityRoles, - action0, resolver, presponse).isComplete()) { + action0, resolver, dcm.isDnfofEnabled(), presponse).isComplete()) { return presponse; } diff --git a/src/test/java/org/opensearch/security/PitIntegrationTests.java b/src/test/java/org/opensearch/security/PitIntegrationTests.java index b9fc4537c7..68a949f882 100644 --- a/src/test/java/org/opensearch/security/PitIntegrationTests.java +++ b/src/test/java/org/opensearch/security/PitIntegrationTests.java @@ -86,10 +86,10 @@ public void pitCreateWithDeleteAll() throws Exception { encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); - // Delete all PITs should work since there are no PITs in the cluster + // Delete all PITs throws forbidden error since there are no PITs in the cluster resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, resc.getStatusCode()); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); } @Test @@ -132,8 +132,6 @@ public void pitCreateWithGetAll() throws Exception { encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); - Thread.sleep(500); - // Delete all PITs should work since there is atleast one PIT for which user has access for resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); @@ -165,15 +163,14 @@ public void pitCreateWithGetAll() throws Exception { encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); - // List all PITs should work since there are no PITs in the cluster + // List all PITs throws forbidden error since there are no PITs in the cluster resc = rh.executeGetRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, resc.getStatusCode()); - + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); - // PIT segments should work since there is atleast one PIT for which user has access for - resc = rh.executeGetRequest("/_cat/pit_segments/_all", - encodeBasicHeader("pit-2", "nagilum")); - Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + // PIT segments API throws forbidden error since there are no PITs in the cluster + resc = rh.executeGetRequest("/_search/point_in_time/_all", + encodeBasicHeader("pit-1", "nagilum")); + Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode()); } } \ No newline at end of file From dd830b4104bffeabc0c985076c0657ef04f5917a Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Mon, 5 Sep 2022 20:24:24 +0530 Subject: [PATCH 3/8] Addressing comments , using dnfOfEmptyResultsEnabled Signed-off-by: Bharathwaj G --- ...uator.java => PitPrivilegesEvaluator.java} | 35 ++++++++++--------- .../privileges/PrivilegesEvaluator.java | 8 ++--- 2 files changed, 23 insertions(+), 20 deletions(-) rename src/main/java/org/opensearch/security/privileges/{PitAccessEvaluator.java => PitPrivilegesEvaluator.java} (91%) diff --git a/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java similarity index 91% rename from src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java rename to src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java index 48e7c753c2..c817b9529a 100644 --- a/src/main/java/org/opensearch/security/privileges/PitAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java @@ -61,7 +61,7 @@ /** * This class evaluates privileges for point in time (Delete and List all) operations */ -public class PitAccessEvaluator { +public class PitPrivilegesEvaluator { protected final Logger log = LogManager.getLogger(this.getClass()); private boolean isDebugEnabled = log.isDebugEnabled(); @@ -69,7 +69,7 @@ public class PitAccessEvaluator { public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final ClusterService clusterService, final User user, final SecurityRoles securityRoles, final String action, final IndexNameExpressionResolver resolver, - boolean dnfofEnabled, final PrivilegesEvaluatorResponse presponse) { + boolean dnfOfEmptyResultsEnabled, final PrivilegesEvaluatorResponse presponse) { // Skip pit evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster // for privilege evaluation @@ -82,26 +82,26 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C return presponse; } return handleGetAllPitsAccess(request, clusterService, user, securityRoles, - action, resolver, dnfofEnabled, presponse); + action, resolver, dnfOfEmptyResultsEnabled, presponse); } else if (request instanceof DeletePitRequest) { DeletePitRequest deletePitRequest = (DeletePitRequest) request; List pitIds = deletePitRequest.getPitIds(); if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { return handleDeleteAllPitAccess(deletePitRequest, clusterService, user, securityRoles, - action, resolver, dnfofEnabled, presponse); + action, resolver, dnfOfEmptyResultsEnabled, presponse); } else { return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfofEnabled, presponse); + action, resolver, dnfOfEmptyResultsEnabled, presponse); } } else if (request instanceof PitSegmentsRequest) { PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; List pitIds = pitSegmentsRequest.getPitIds(); if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { return handleGetAllPitSegmentsAccess(pitSegmentsRequest, clusterService, user, securityRoles, - action, resolver, dnfofEnabled, presponse); + action, resolver, dnfOfEmptyResultsEnabled, presponse); } else { return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfofEnabled, presponse); + action, resolver, dnfOfEmptyResultsEnabled, presponse); } } } catch(InterruptedException e) { @@ -117,11 +117,12 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest request, final ClusterService clusterService, final User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver, - boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { + boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List pitInfos = getAllPitInfos((GetAllPitNodesRequest) request); - // if cluster has no PITs, then allow the operation to pass with empty response + // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled + // config property is true, otherwise fail the operation if(pitInfos.isEmpty()) { - if(dnfofEnabled) { + if(dnfOfEmptyResultsEnabled) { presponse.allowed = true; presponse.markComplete(); } @@ -172,12 +173,13 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest deletePitRequest, ClusterService clusterService, User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver, - boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { + boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List permittedPits = new ArrayList<>(); List pitIds = getAllPitIds(); - // allow delete pit operation if there are no pits in the cluster ( response should be empty ) + // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled + // config property is true, otherwise fail the operation if(pitIds.isEmpty()) { - if(dnfofEnabled) { + if(dnfOfEmptyResultsEnabled) { deletePitRequest.clearAndSetPitIds(pitIds); presponse.allowed = true; presponse.markComplete(); @@ -216,12 +218,13 @@ private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest de private PrivilegesEvaluatorResponse handleGetAllPitSegmentsAccess(PitSegmentsRequest pitSegmentsRequest, ClusterService clusterService, User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver, - boolean dnfofEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { + boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { List permittedPits = new ArrayList<>(); List pitIds = getAllPitIds(); - // allow pit segments operation if there are no pits in the cluster ( response should be empty ) + // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled + // config property is true, otherwise fail the operation if(pitIds.isEmpty()) { - if(dnfofEnabled) { + if(dnfOfEmptyResultsEnabled) { pitSegmentsRequest.clearAndSetPitIds(pitIds); presponse.allowed = true; presponse.markComplete(); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 0cc850ecc6..10662e6f40 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -130,7 +130,7 @@ public class PrivilegesEvaluator { private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator; private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator; private final TermsAggregationEvaluator termsAggregationEvaluator; - private final PitAccessEvaluator pitAccessEvaluator; + private final PitPrivilegesEvaluator pitPrivilegesEvaluator; private final boolean dlsFlsEnabled; private final boolean dfmEmptyOverwritesAll; private DynamicConfigModel dcm; @@ -159,7 +159,7 @@ public PrivilegesEvaluator(final ClusterService clusterService, final ThreadPool securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr); protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog); termsAggregationEvaluator = new TermsAggregationEvaluator(); - pitAccessEvaluator = new PitAccessEvaluator(); + pitPrivilegesEvaluator = new PitPrivilegesEvaluator(); this.namedXContentRegistry = namedXContentRegistry; this.dlsFlsEnabled = dlsFlsEnabled; this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false); @@ -285,8 +285,8 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin } // check access for point in time requests - if(pitAccessEvaluator.evaluate(request, clusterService, user, securityRoles, - action0, resolver, dcm.isDnfofEnabled(), presponse).isComplete()) { + if(pitPrivilegesEvaluator.evaluate(request, clusterService, user, securityRoles, + action0, resolver, dcm.isDnfofForEmptyResultsEnabled(), presponse).isComplete()) { return presponse; } From eeff45e6ad9fd494942cf5850c4afd52116b707c Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 6 Sep 2022 16:35:35 +0530 Subject: [PATCH 4/8] Changes to separate list all pits call from security plugin Signed-off-by: Bharathwaj G --- .../privileges/PitPrivilegesEvaluator.java | 179 +----------------- 1 file changed, 8 insertions(+), 171 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java index c817b9529a..e68c73d408 100644 --- a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java @@ -78,31 +78,16 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C } try { if (request instanceof GetAllPitNodesRequest) { - if (((GetAllPitNodesRequest) request).getGetAllPitNodesResponse() != null) { - return presponse; - } return handleGetAllPitsAccess(request, clusterService, user, securityRoles, action, resolver, dnfOfEmptyResultsEnabled, presponse); } else if (request instanceof DeletePitRequest) { DeletePitRequest deletePitRequest = (DeletePitRequest) request; - List pitIds = deletePitRequest.getPitIds(); - if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { - return handleDeleteAllPitAccess(deletePitRequest, clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } else { - return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } + return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, dnfOfEmptyResultsEnabled, presponse); } else if (request instanceof PitSegmentsRequest) { PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; - List pitIds = pitSegmentsRequest.getPitIds(); - if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) { - return handleGetAllPitSegmentsAccess(pitSegmentsRequest, clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } else { - return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } + return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, dnfOfEmptyResultsEnabled, presponse); } } catch(InterruptedException e) { Thread.currentThread().interrupt(); @@ -118,7 +103,7 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r final User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver, boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { - List pitInfos = getAllPitInfos((GetAllPitNodesRequest) request); + List pitInfos = ((GetAllPitNodesRequest) request).getGetAllPitNodesResponse().getPitInfos(); // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled // config property is true, otherwise fail the operation if(pitInfos.isEmpty()) { @@ -167,103 +152,13 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r return presponse; } - /** - * Handle access for 'delete all PITs' operation - */ - private PrivilegesEvaluatorResponse handleDeleteAllPitAccess(DeletePitRequest deletePitRequest, ClusterService clusterService, - User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { - List permittedPits = new ArrayList<>(); - List pitIds = getAllPitIds(); - // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled - // config property is true, otherwise fail the operation - if(pitIds.isEmpty()) { - if(dnfOfEmptyResultsEnabled) { - deletePitRequest.clearAndSetPitIds(pitIds); - presponse.allowed = true; - presponse.markComplete(); - } - return presponse; - } - Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); - for (String pitId : pitIds) { - String[] indices = pitToIndicesMap.get(pitId); - HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); - Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, - securityRoles, action, resolver); - // user should have permissions for all indices associated with PIT, only then add PIT ID as permitted PIT - if(isDebugEnabled) { - log.debug("Evaluating PIT : " + pitId ); - } - if (allPermittedIndices.size() == indicesSet.size()) { - if(isDebugEnabled) { - log.debug(" Permitting PIT : " + pitId); - } - permittedPits.add(pitId); - } - } - // If there are any PITs for which the user has access to, then allow operation otherwise fail. - if(permittedPits.size() > 0) { - deletePitRequest.clearAndSetPitIds(permittedPits); - presponse.allowed = true; - presponse.markComplete(); - } - return presponse; - } - - /** - * Handle access for PIT segments API - */ - private PrivilegesEvaluatorResponse handleGetAllPitSegmentsAccess(PitSegmentsRequest pitSegmentsRequest, ClusterService clusterService, - User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { - List permittedPits = new ArrayList<>(); - List pitIds = getAllPitIds(); - // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled - // config property is true, otherwise fail the operation - if(pitIds.isEmpty()) { - if(dnfOfEmptyResultsEnabled) { - pitSegmentsRequest.clearAndSetPitIds(pitIds); - presponse.allowed = true; - presponse.markComplete(); - } - return presponse; - } - Map pitToIndicesMap = OpenSearchSecurityPlugin.GuiceHolder.getPitService().getIndicesForPits(pitIds); - for (String pitId : pitIds) { - String[] indices = pitToIndicesMap.get(pitId); - HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); - Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, - securityRoles, action, resolver); - // user should have permissions for all indices associated with PIT, only then add PIT ID as permitted PIT - if(isDebugEnabled) { - log.debug("Evaluating PIT : " + pitId ); - } - if (allPermittedIndices.size() == indicesSet.size()) { - if(isDebugEnabled) { - log.debug(" Permitting PIT : " + pitId); - } - permittedPits.add(pitId); - } - } - // If there are any PITs for which the user has access to, then allow operation otherwise fail. - if(permittedPits.size() > 0) { - pitSegmentsRequest.clearAndSetPitIds(permittedPits); - presponse.allowed = true; - presponse.markComplete(); - } - return presponse; - } - /** * Handle access for delete operation / pit segments operation where PIT IDs are explicitly passed */ private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List pitIds, ClusterService clusterService, - User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - PrivilegesEvaluatorResponse presponse) { + User user, SecurityRoles securityRoles, final String action, + IndexNameExpressionResolver resolver, + boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) { Map pitToIndicesMap = OpenSearchSecurityPlugin. GuiceHolder.getPitService().getIndicesForPits(pitIds); Set pitIndices = new HashSet<>(); @@ -295,62 +190,4 @@ private Set getPermittedIndices(Set pitIndices, ClusterService c return securityRoles.reduce(pitResolved, user, new String[]{action}, resolver, clusterService); } - - /** - * Get all active PITs - */ - private List getAllPitInfos(GetAllPitNodesRequest request) throws InterruptedException { - final List pitInfos = new ArrayList<>(); - final CountDownLatch latch = new CountDownLatch(1); - ActionListener listener = new ActionListener() { - @Override - public void onResponse(GetAllPitNodesResponse response) { - pitInfos.addAll(response.getPitInfos()); - request.setGetAllPitNodesResponse(response); - } - - @Override - public void onFailure(Exception e) { - throw new OpenSearchException("List all PITs failed", e); - } - }; - LatchedActionListener latchedActionListener = new LatchedActionListener<>(listener, latch); - - OpenSearchSecurityPlugin.GuiceHolder.getPitService().getAllPits(latchedActionListener); - - if(!latch.await(15, TimeUnit.SECONDS)) { - log.warn("Failed to get all PITs information within the timeout {}", new TimeValue(15, TimeUnit.SECONDS)); - } - return pitInfos; - } - - /** - * Get all active PIT IDs - */ - private List getAllPitIds() throws InterruptedException { - - final List pitInfos = new ArrayList<>(); - final CountDownLatch latch = new CountDownLatch(1); - - ActionListener listener = new ActionListener() { - @Override - public void onResponse(GetAllPitNodesResponse response) { - pitInfos.addAll(response.getPitInfos()); - } - - @Override - public void onFailure(Exception e) { - throw new OpenSearchException("List all PITs failed", e); - - } - }; - LatchedActionListener latchedActionListener = new LatchedActionListener<>(listener, latch); - - OpenSearchSecurityPlugin.GuiceHolder.getPitService().getAllPits(latchedActionListener); - - if(!latch.await(15, TimeUnit.SECONDS)) { - log.warn("Failed to get all PITs information within the timeout {}", new TimeValue(15, TimeUnit.SECONDS)); - } - return pitInfos.stream().map(r -> r.getPitId()).collect(Collectors.toList()); - } } From 4a8ad4ced39d1d60845ebe84b989d488880f69a1 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 6 Sep 2022 21:00:07 +0530 Subject: [PATCH 5/8] Addressing comments Signed-off-by: Bharathwaj G --- .../privileges/PitPrivilegesEvaluator.java | 74 ++++++------------- .../security/PitIntegrationTests.java | 15 ---- 2 files changed, 21 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java index e68c73d408..626a6c7447 100644 --- a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java @@ -1,18 +1,3 @@ -/* - * Copyright 2015-2018 _floragunn_ GmbH - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - /* * SPDX-License-Identifier: Apache-2.0 * @@ -32,18 +17,13 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import com.google.common.collect.ImmutableSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.OpenSearchException; -import org.opensearch.action.ActionListener; import org.opensearch.action.ActionRequest; -import org.opensearch.action.LatchedActionListener; import org.opensearch.action.admin.indices.segments.PitSegmentsRequest; import org.opensearch.action.search.DeletePitRequest; import org.opensearch.action.search.GetAllPitNodesRequest; @@ -52,7 +32,6 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.unit.TimeValue; import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.securityconf.SecurityRoles; @@ -71,27 +50,22 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C final IndexNameExpressionResolver resolver, boolean dnfOfEmptyResultsEnabled, final PrivilegesEvaluatorResponse presponse) { - // Skip pit evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster - // for privilege evaluation - if(action.startsWith("cluster:admin")) { + // Skip custom evaluation for "NodesGetAllPITs" action, since it fetches all PITs across the cluster + // for privilege evaluation - still this action will be evaluated in the generic PrivilegesEvaluator flow + if(action.startsWith("cluster:admin/point_in_time")) { return presponse; } - try { - if (request instanceof GetAllPitNodesRequest) { - return handleGetAllPitsAccess(request, clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } else if (request instanceof DeletePitRequest) { - DeletePitRequest deletePitRequest = (DeletePitRequest) request; - return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } else if (request instanceof PitSegmentsRequest) { - PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; - return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, - action, resolver, dnfOfEmptyResultsEnabled, presponse); - } - } catch(InterruptedException e) { - Thread.currentThread().interrupt(); - log.error(e.toString()); + if (request instanceof GetAllPitNodesRequest) { + return handleGetAllPitsAccess(request, clusterService, user, securityRoles, + action, resolver, dnfOfEmptyResultsEnabled, presponse); + } else if (request instanceof DeletePitRequest) { + DeletePitRequest deletePitRequest = (DeletePitRequest) request; + return handleExplicitPitsAccess(deletePitRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, presponse); + } else if (request instanceof PitSegmentsRequest) { + PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request; + return handleExplicitPitsAccess(pitSegmentsRequest.getPitIds(), clusterService, user, securityRoles, + action, resolver, presponse); } return presponse; } @@ -102,7 +76,7 @@ public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final C private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest request, final ClusterService clusterService, final User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver, - boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) throws InterruptedException { + boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) { List pitInfos = ((GetAllPitNodesRequest) request).getGetAllPitNodesResponse().getPitInfos(); // if cluster has no PITs, then allow the operation to pass with empty response if dnfOfEmptyResultsEnabled // config property is true, otherwise fail the operation @@ -126,17 +100,12 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r String[] indices = pitToIndicesMap.get(pitId); HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); - final ImmutableSet INDICES_SET = ImmutableSet.copyOf(indicesSet); - final IndexResolverReplacer.Resolved pitResolved = - new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET, - ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS); - - final Set allPermittedIndices = securityRoles.reduce(pitResolved, - user, new String[]{action}, resolver, clusterService); + final Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, + securityRoles, action, resolver); if(isDebugEnabled) { log.debug("Evaluating PIT ID : " + pitId ); } - if (allPermittedIndices.size() == INDICES_SET.size()) { + if (allPermittedIndices.size() == indicesSet.size()) { if(isDebugEnabled) { log.debug(" Permitting PIT ID : " + pitId); } @@ -157,8 +126,7 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r */ private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List pitIds, ClusterService clusterService, User user, SecurityRoles securityRoles, final String action, - IndexNameExpressionResolver resolver, - boolean dnfOfEmptyResultsEnabled, PrivilegesEvaluatorResponse presponse) { + IndexNameExpressionResolver resolver, PrivilegesEvaluatorResponse presponse) { Map pitToIndicesMap = OpenSearchSecurityPlugin. GuiceHolder.getPitService().getIndicesForPits(pitIds); Set pitIndices = new HashSet<>(); @@ -183,9 +151,9 @@ private PrivilegesEvaluatorResponse handleExplicitPitsAccess(List pitIds private Set getPermittedIndices(Set pitIndices, ClusterService clusterService, User user, SecurityRoles securityRoles, final String action, IndexNameExpressionResolver resolver) { - final ImmutableSet INDICES_SET = ImmutableSet.copyOf(pitIndices); + final ImmutableSet pitImmutableIndices = ImmutableSet.copyOf(pitIndices); final IndexResolverReplacer.Resolved pitResolved = - new IndexResolverReplacer.Resolved(INDICES_SET, INDICES_SET, INDICES_SET, + new IndexResolverReplacer.Resolved(pitImmutableIndices, pitImmutableIndices, pitImmutableIndices, ImmutableSet.of(), SearchRequest.DEFAULT_INDICES_OPTIONS); return securityRoles.reduce(pitResolved, user, new String[]{action}, resolver, clusterService); diff --git a/src/test/java/org/opensearch/security/PitIntegrationTests.java b/src/test/java/org/opensearch/security/PitIntegrationTests.java index 68a949f882..91673f5836 100644 --- a/src/test/java/org/opensearch/security/PitIntegrationTests.java +++ b/src/test/java/org/opensearch/security/PitIntegrationTests.java @@ -1,18 +1,3 @@ -/* - * Copyright 2015-2018 _floragunn_ GmbH - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - /* * SPDX-License-Identifier: Apache-2.0 * From 4380e7746025dcc1a01b0a1783db1dc3748e8cfa Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 6 Sep 2022 22:10:30 +0530 Subject: [PATCH 6/8] Addressing comments Signed-off-by: Bharathwaj G --- .../privileges/PitPrivilegesEvaluator.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java index 626a6c7447..9a31b73c4b 100644 --- a/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PitPrivilegesEvaluator.java @@ -96,16 +96,22 @@ private PrivilegesEvaluatorResponse handleGetAllPitsAccess(final ActionRequest r pitToPitInfoMap.put(pitInfo.getPitId(), pitInfo); } List permittedPits = new ArrayList<>(); - for (String pitId : pitIds) { - String[] indices = pitToIndicesMap.get(pitId); - HashSet indicesSet = new HashSet<>(Arrays.asList(indices)); - final Set allPermittedIndices = getPermittedIndices(indicesSet, clusterService, user, - securityRoles, action, resolver); + Set allPitIndices = new HashSet<>(); + for(String[] indices: pitToIndicesMap.values()) { + allPitIndices.addAll(Arrays.asList(indices)); + } + final Set allPermittedPitIndices = getPermittedIndices(allPitIndices, clusterService, user, + securityRoles, action, resolver); + + for (String pitId : pitIds) { + final String[] indices = pitToIndicesMap.get(pitId); + final HashSet pitIndicesSet = new HashSet<>(Arrays.asList(indices)); if(isDebugEnabled) { log.debug("Evaluating PIT ID : " + pitId ); } - if (allPermittedIndices.size() == indicesSet.size()) { + + if (allPermittedPitIndices.containsAll(pitIndicesSet)) { if(isDebugEnabled) { log.debug(" Permitting PIT ID : " + pitId); } From e440c35cc9464f5ff05b978ef40658835b50ef71 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 6 Sep 2022 22:31:13 +0530 Subject: [PATCH 7/8] Addressing comments - enhancing integ tests Signed-off-by: Bharathwaj G --- .../security/PitIntegrationTests.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/PitIntegrationTests.java b/src/test/java/org/opensearch/security/PitIntegrationTests.java index 91673f5836..0c8b5aa86b 100644 --- a/src/test/java/org/opensearch/security/PitIntegrationTests.java +++ b/src/test/java/org/opensearch/security/PitIntegrationTests.java @@ -45,6 +45,7 @@ public void pitCreateWithDeleteAll() throws Exception { resc = rh.executePostRequest("/pit_1/_search/point_in_time?keep_alive=100m", "", encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + String pitId1 = resc.findValueInJson("pit_id"); // Create point in time in index for which the user does not have permission resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", @@ -56,10 +57,14 @@ public void pitCreateWithDeleteAll() throws Exception { encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + String pitId2 = resc.findValueInJson("pit_id"); + // Delete all PITs should work since there is atleast one PIT for which user has access for resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + Assert.assertEquals(pitId1, resc.findValueInJson("pits[0].pit_id")); + Assert.assertEquals("true", resc.findValueInJson("pits[0].successful")); // Delete all PITs should throw error since there are no PITs for which the user has access for resc = rh.executeDeleteRequest("/_search/point_in_time/_all", @@ -70,6 +75,9 @@ public void pitCreateWithDeleteAll() throws Exception { resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + Assert.assertEquals(pitId2, resc.findValueInJson("pits[0].pit_id")); + Assert.assertEquals("true", resc.findValueInJson("pits[0].successful")); + // Delete all PITs throws forbidden error since there are no PITs in the cluster resc = rh.executeDeleteRequest("/_search/point_in_time/_all", @@ -97,6 +105,8 @@ public void pitCreateWithGetAll() throws Exception { encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + String pitId1 = resc.findValueInJson("pit_id"); + // Create point in time in index for which the user does not have permission resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", encodeBasicHeader("pit-1", "nagilum")); @@ -106,11 +116,13 @@ public void pitCreateWithGetAll() throws Exception { resc = rh.executePostRequest("/pit_2/_search/point_in_time?keep_alive=100m", "", encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + String pitId2 = resc.findValueInJson("pit_id"); // List all PITs should work since there is atleast one PIT for which user has access for resc = rh.executeGetRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + Assert.assertEquals(pitId1, resc.findValueInJson("pits[0].pit_id")); // PIT segments should work since there is atleast one PIT for which user has access for resc = rh.executeGetRequest("/_cat/pit_segments/_all", @@ -121,7 +133,8 @@ public void pitCreateWithGetAll() throws Exception { resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-1", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); - + Assert.assertEquals(pitId1, resc.findValueInJson("pits[0].pit_id")); + Assert.assertEquals("true", resc.findValueInJson("pits[0].successful")); // List all PITs should throw error since there are no PITs for which the user has access for resc = rh.executeGetRequest("/_search/point_in_time/_all", @@ -137,6 +150,7 @@ public void pitCreateWithGetAll() throws Exception { resc = rh.executeGetRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + Assert.assertEquals(pitId2, resc.findValueInJson("pits[0].pit_id")); // PIT segments should work since there is atleast one PIT for which user has access for resc = rh.executeGetRequest("/_cat/pit_segments/_all", @@ -147,6 +161,8 @@ public void pitCreateWithGetAll() throws Exception { resc = rh.executeDeleteRequest("/_search/point_in_time/_all", encodeBasicHeader("pit-2", "nagilum")); Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode()); + Assert.assertEquals(pitId2, resc.findValueInJson("pits[0].pit_id")); + Assert.assertEquals("true", resc.findValueInJson("pits[0].successful")); // List all PITs throws forbidden error since there are no PITs in the cluster resc = rh.executeGetRequest("/_search/point_in_time/_all", From 54d80cff433a0b42c023442b1cab746a64dd89a7 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 6 Sep 2022 22:37:25 +0530 Subject: [PATCH 8/8] Addressing comments - correcting roles permissions Signed-off-by: Bharathwaj G --- src/test/resources/roles.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/resources/roles.yml b/src/test/resources/roles.yml index a66ee62485..c943613d2a 100644 --- a/src/test/resources/roles.yml +++ b/src/test/resources/roles.yml @@ -1134,7 +1134,6 @@ point_in_time_1: description: "Migrated from v6 (all types mapped)" cluster_permissions: - "cluster:admin/point_in_time/read_from_nodes" - - "cluster_monitor" index_permissions: - index_patterns: - "pit_1" @@ -1150,7 +1149,6 @@ point_in_time_2: description: "Migrated from v6 (all types mapped)" cluster_permissions: - "cluster:admin/point_in_time/read_from_nodes" - - "cluster_monitor" index_permissions: - index_patterns: - "pit_2"