Skip to content

Commit

Permalink
Cluster permissions evaluation logic will now include `index_template…
Browse files Browse the repository at this point in the history
…` type action (opensearch-project#1885)

* Updates the cluster permission check function to include index_template permission type

Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura authored Jun 15, 2022
1 parent 9e5dbcf commit c1838bf
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ private Set<String> evaluateAdditionalIndexPermissions(final ActionRequest reque
public static boolean isClusterPerm(String action0) {
return ( action0.startsWith("cluster:")
|| action0.startsWith("indices:admin/template/")

|| action0.startsWith("indices:admin/index_template/")
|| action0.startsWith(SearchScrollAction.NAME)
|| (action0.equals(BulkAction.NAME))
|| (action0.equals(MultiGetAction.NAME))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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.template.put.PutComponentTemplateAction;
import org.opensearch.action.bulk.BulkRequest;
import org.opensearch.action.bulk.BulkShardRequest;
import org.opensearch.action.delete.DeleteRequest;
Expand Down Expand Up @@ -697,6 +698,8 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid
//do nothing
} else if (request instanceof SearchScrollRequest) {
//do nothing
} else if (request instanceof PutComponentTemplateAction.Request) {
// do nothing
} else {
if (isDebugEnabled) {
log.debug(request.getClass() + " not supported (It is likely not a indices related request)");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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.Before;
import org.junit.Test;

import org.opensearch.security.test.SingleClusterTest;
import org.opensearch.security.test.helper.rest.RestHelper;
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

public class IndexTemplateClusterPermissionsCheckTest extends SingleClusterTest{
private RestHelper rh;

final static String indexTemplateBody = "{ \"index_patterns\": [\"sem1234*\"], \"template\": { \"settings\": { \"number_of_shards\": 2, \"number_of_replicas\": 1 }, \"mappings\": { \"properties\": { \"timestamp\": { \"type\": \"date\", \"format\": \"yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis\" }, \"value\": { \"type\": \"double\" } } } } }";

private String getFailureResponseReason(String user) {
return "no permissions for [indices:admin/index_template/put] and User [name=" + user + ", backend_roles=[], requestedTenant=null]";
}

@Before
public void setupRestHelper() throws Exception{
setup();
rh = nonSslRestHelper();
}
@Test
public void testPutIndexTemplateByNonPrivilegedUser() throws Exception {
String expectedFailureResponse = getFailureResponseReason("ds3");

// should fail, as user `ds3` doesn't have correct permissions
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds3", "nagilum"));
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason"));
}

@Test
public void testPutIndexTemplateByPrivilegedUser() throws Exception {
// should pass, as user `sem-user` has correct permissions
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("sem-user", "nagilum"));
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
}

@Test
public void testPutIndexTemplateAsIndexLevelPermission() throws Exception {
String expectedFailureResponse = getFailureResponseReason("sem-user2");

// should fail, as user `sem-user2` is assigned `put-template` permission as index-level, not cluster-level
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("sem-user2", "nagilum"));
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.net.ssl.SSLContext;

Expand Down Expand Up @@ -358,6 +360,16 @@ public String toString() {
+ ", statusReason=" + statusReason + "]";
}

private static void findArrayAccessor(String input) {
final Pattern r = Pattern.compile("(.+?)\\[(\\d+)\\]");
final Matcher m = r.matcher(input);
if(m.find()) {
System.out.println("'" + input + "'\t Name was: " + m.group(1) + ",\t index position: " + m.group(2));
} else {
System.out.println("'" + input + "'\t No Match");
}
}

/**
* Given a json path with dots delimiated returns the object at the leaf
*/
Expand All @@ -379,11 +391,34 @@ public String findValueInJson(final String jsonDotPath) {
fail("Invalid json dot path '" + jsonDotPath + "', rewrite with '.' characters between path elements.");
}
do {
final String pathEntry = jsonPathScanner.next();
String pathEntry = jsonPathScanner.next();
// if pathEntry is an array lookup
int arrayEntryIdx = -1;

// Looks for an array-lookup pattern in the path
// e.g. root_cause[1] -> will match
// e.g. root_cause[2aasd] -> won't match
final Pattern r = Pattern.compile("(.+?)\\[(\\d+)\\]");
final Matcher m = r.matcher(pathEntry);
if(m.find()) {
pathEntry = m.group(1);
arrayEntryIdx = Integer.parseInt(m.group(2));
}

if (!currentNode.has(pathEntry)) {
fail("Unable to resolve '" + jsonDotPath + "', on path entry '" + pathEntry + "' from available fields " + currentNode.toPrettyString());
}
currentNode = currentNode.get(pathEntry);

// if it's an Array lookup we get the requested index item
if (arrayEntryIdx > -1) {
if(!currentNode.isArray()) {
fail("Unable to resolve '" + jsonDotPath + "', the '" + pathEntry + "' field is not an array " + currentNode.toPrettyString());
} else if (!currentNode.has(arrayEntryIdx)) {
fail("Unable to resolve '" + jsonDotPath + "', index '" + arrayEntryIdx + "' is out of bounds for array '" + pathEntry + "' \n" + currentNode.toPrettyString());
}
currentNode = currentNode.get(arrayEntryIdx);
}
} while (jsonPathScanner.hasNext());

if (!currentNode.isValueNode()) {
Expand Down
6 changes: 6 additions & 0 deletions src/test/resources/internal_users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,9 @@ hidden_test:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
opendistro_security_roles:
- hidden_test
sem-user:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
sem-user2:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
24 changes: 23 additions & 1 deletion src/test/resources/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ data_stream_1:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
cluster_permissions: [ "indices:admin/index_template/put" ]
index_permissions:
- index_patterns:
- "my-data-stream*"
Expand Down Expand Up @@ -1137,3 +1137,25 @@ hidden_test:
- hidden_test_not_hidden
allowed_actions:
- "*"

sem-role:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: [ "cluster_monitor", "indices:admin/index_template/put" ]
index_permissions:
- index_patterns:
- "sem*"
allowed_actions:
- "*"

sem-role2:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: [ "cluster_monitor" ]
index_permissions:
- index_patterns:
- "sem*"
allowed_actions:
- "indices:admin/index_template/put"
10 changes: 10 additions & 0 deletions src/test/resources/roles_mapping.yml
Original file line number Diff line number Diff line change
Expand Up @@ -413,3 +413,13 @@ data_stream_3:
hidden: false
users:
- "ds3"
sem-role:
reserved: false
hidden: false
users:
- "sem-user"
sem-role2:
reserved: false
hidden: false
users:
- "sem-user2"

0 comments on commit c1838bf

Please sign in to comment.