Skip to content

Commit

Permalink
Shrink operation privileges evaluation (#3716)
Browse files Browse the repository at this point in the history 
### Description
Bug fix. Shrink, or resize operations weren't properly evaluated. More
in the task: #2141

### Issues Resolved
#2141

Is this a backport? If so, please add backport PR # and/or commits #

### Testing
[Please provide details of testing done: unit testing, integration
testing and manual testing]

### Check List
- [x] New functionality includes testing
- [x] New functionality has been documented
- [x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Maciej Mierzwa <[email protected]>
(cherry picked from commit 3c01fde)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@github-actions
github-actions[bot] committed Nov 22, 2023
1 parent 71ffbdc commit 0add5ec
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 26 deletions.
68 changes: 42 additions & 26 deletions src/integrationTest/java/org/opensearch/security/SearchOperationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
import org.opensearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest;
import org.opensearch.action.admin.cluster.repositories.put.PutRepositoryRequest;
import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
Expand Down Expand Up @@ -83,6 +85,7 @@
import org.opensearch.client.indices.PutMappingRequest;
import org.opensearch.client.indices.ResizeRequest;
import org.opensearch.client.indices.ResizeResponse;
import org.opensearch.cluster.health.ClusterHealthStatus;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.IndexTemplateMetadata;
import org.opensearch.common.settings.Settings;
Expand Down Expand Up @@ -119,6 +122,7 @@
import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
import static org.opensearch.client.RequestOptions.DEFAULT;
import static org.opensearch.core.rest.RestStatus.ACCEPTED;
import static org.opensearch.core.rest.RestStatus.BAD_REQUEST;
import static org.opensearch.core.rest.RestStatus.FORBIDDEN;
import static org.opensearch.core.rest.RestStatus.INTERNAL_SERVER_ERROR;
import static org.opensearch.rest.RestRequest.Method.DELETE;
Expand Down Expand Up @@ -335,22 +339,24 @@ public class SearchOperationTest {
* indices with names prefixed by the {@link #INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX}
*/
static final User USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES = new User("index-operation-tester").roles(
new Role("index-manager").indexPermissions(
"indices:admin/create",
"indices:admin/get",
"indices:admin/delete",
"indices:admin/close",
"indices:admin/close*",
"indices:admin/open",
"indices:admin/resize",
"indices:monitor/stats",
"indices:monitor/settings/get",
"indices:admin/settings/update",
"indices:admin/mapping/put",
"indices:admin/mappings/get",
"indices:admin/cache/clear",
"indices:admin/aliases"
).on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*"))
new Role("index-manager").clusterPermissions("cluster:monitor/health")
.indexPermissions(
"indices:admin/create",
"indices:admin/get",
"indices:admin/delete",
"indices:admin/close",
"indices:admin/close*",
"indices:admin/open",
"indices:admin/resize",
"indices:monitor/stats",
"indices:monitor/settings/get",
"indices:admin/settings/update",
"indices:admin/mapping/put",
"indices:admin/mappings/get",
"indices:admin/cache/clear",
"indices:admin/aliases"
)
.on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*"))
);

private static User USER_ALLOWED_TO_CREATE_INDEX = new User("user-allowed-to-create-index").roles(
Expand Down Expand Up @@ -2274,21 +2280,33 @@ public void openIndex_negative() throws IOException {
}

@Test
@Ignore
// required permissions: "indices:admin/resize", "indices:monitor/stats
// todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails.
// Issue: https://github.com/opensearch-project/security/issues/2141
public void shrinkIndex_positive() throws IOException {
String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_source");
Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).put("index.number_of_shards", 2).build();
String targetIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_target");
Settings sourceIndexSettings = Settings.builder()
.put("index.number_of_replicas", 1)
.put("index.blocks.write", true)
.put("index.number_of_shards", 4)
.build();
IndexOperationsHelper.createIndex(cluster, sourceIndexName, sourceIndexSettings);

try (
RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(
USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES
)
) {
ClusterHealthResponse healthResponse = restHighLevelClient.cluster()
.health(
new ClusterHealthRequest(sourceIndexName).waitForNoRelocatingShards(true)
.waitForActiveShards(4)
.waitForNoInitializingShards(true)
.waitForGreenStatus(),
DEFAULT
);

assertThat(healthResponse.getStatus(), is(ClusterHealthStatus.GREEN));

ResizeRequest resizeRequest = new ResizeRequest(targetIndexName, sourceIndexName);
ResizeResponse response = restHighLevelClient.indices().shrink(resizeRequest, DEFAULT);

Expand Down Expand Up @@ -2331,10 +2349,7 @@ public void shrinkIndex_negative() throws IOException {
}

@Test
@Ignore
// required permissions: "indices:admin/resize", "indices:monitor/stats
// todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails.
// Issue: https://github.com/opensearch-project/security/issues/2141
public void cloneIndex_positive() throws IOException {
String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("clone_index_positive_source");
Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build();
Expand All @@ -2351,6 +2366,10 @@ public void cloneIndex_positive() throws IOException {

assertThat(response, isSuccessfulResizeResponse(targetIndexName));
assertThat(cluster, indexExists(targetIndexName));

// can't clone the same index twice, target already exists
ResizeRequest repeatResizeRequest = new ResizeRequest(targetIndexName, sourceIndexName);
assertThatThrownBy(() -> restHighLevelClient.indices().clone(repeatResizeRequest, DEFAULT), statusException(BAD_REQUEST));
}
}

Expand Down Expand Up @@ -2388,10 +2407,7 @@ public void cloneIndex_negative() throws IOException {
}

@Test
@Ignore
// required permissions: "indices:admin/resize", "indices:monitor/stats
// todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails.
// Issue: https://github.com/opensearch-project/security/issues/2141
public void splitIndex_positive() throws IOException {
String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("split_index_positive_source");
Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build();
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.opensearch.action.admin.indices.datastream.CreateDataStreamAction;
import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.opensearch.action.admin.indices.resolve.ResolveIndexAction;
import org.opensearch.action.admin.indices.shrink.ResizeRequest;
import org.opensearch.action.admin.indices.template.put.PutComponentTemplateAction;
import org.opensearch.action.bulk.BulkRequest;
import org.opensearch.action.bulk.BulkShardRequest;
Expand Down Expand Up @@ -777,6 +778,10 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid
return false;
}
((CreateIndexRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
} else if (request instanceof ResizeRequest) {
// clone or shrink operations
provider.provide(((ResizeRequest) request).indices(), request, true);
provider.provide(((ResizeRequest) request).getTargetIndexRequest().indices(), request, true);

Check warning on line 784 in src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java#L783-L784 

Added lines #L783 - L784 were not covered by tests
} else if (request instanceof CreateDataStreamAction.Request) {
provider.provide(((CreateDataStreamAction.Request) request).indices(), request, false);
} else if (request instanceof ReindexRequest) {
Expand Down

0 comments on commit 0add5ec

Please sign in to comment.