Skip to content

Commit

Permalink
Tenant Permissions : added the possibility to specify tenants via par…
Browse files Browse the repository at this point in the history
…ameter (#1813)

* Issue #817 : Tenant Permissions, added the possibility to specify tenants via parameter substitution.

Signed-off-by: Christophe Chazeau <[email protected]>
Co-authored-by: Peter Nied <[email protected]>
  • Loading branch information
peternied authored Jun 7, 2022
1 parent 07637c4 commit ce59944
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -97,13 +98,13 @@ public ConfigModelV7(
log.error("Cannot apply roles mapping resolution", e);
rolesMappingResolution = ConfigConstants.RolesMappingResolution.MAPPING_ONLY;
}

agr = reloadActionGroups(actiongroups);
securityRoles = reload(roles);
tenantHolder = new TenantHolder(roles, tenants);
roleMappingHolder = new RoleMappingHolder(rolemappings, dcm.getHostsResolverMode());
}

public Set<String> getAllConfiguredTenantNames() {
return Collections.unmodifiableSet(tenants.getCEntries().keySet());
}
Expand All @@ -115,7 +116,7 @@ public SecurityRoles getSecurityRoles() {
private static interface ActionGroupResolver {
Set<String> resolvedActions(final List<String> actions);
}

private ActionGroupResolver reloadActionGroups(SecurityDynamicConfiguration<ActionGroupsV7> actionGroups) {
return new ActionGroupResolver() {

Expand Down Expand Up @@ -1014,10 +1015,6 @@ private static boolean impliesTypePerm(Set<IndexPattern> ipatterns, Resolved res
);
}



//#######

private class TenantHolder {

private SetMultimap<String, Tuple<String, Boolean>> tenantsMM = null;
Expand All @@ -1028,7 +1025,7 @@ public TenantHolder(SecurityDynamicConfiguration<RoleV7> roles, SecurityDynamicC
final ExecutorService execs = Executors.newFixedThreadPool(10);

for(Entry<String, RoleV7> securityRole: roles.getCEntries().entrySet()) {

if(securityRole.getValue() == null) {
continue;
}
Expand All @@ -1038,14 +1035,21 @@ public TenantHolder(SecurityDynamicConfiguration<RoleV7> roles, SecurityDynamicC
public Tuple<String, Set<Tuple<String, Boolean>>> call() throws Exception {
final Set<Tuple<String, Boolean>> tuples = new HashSet<>();
final List<RoleV7.Tenant> tenants = securityRole.getValue().getTenant_permissions();

if (tenants != null) {

for (RoleV7.Tenant tenant : tenants) {

for(String matchingTenant: WildcardMatcher.from(tenant.getTenant_patterns()).getMatchAny(definedTenants.getCEntries().keySet(), Collectors.toList())) {

// find Wildcarded tenant patterns
List<String> matchingTenants = WildcardMatcher.from(tenant.getTenant_patterns()).getMatchAny(definedTenants.getCEntries().keySet(), Collectors.toList()) ;
for(String matchingTenant: matchingTenants ) {
tuples.add(new Tuple<String, Boolean>(matchingTenant, agr.resolvedActions(tenant.getAllowed_actions()).contains("kibana:saved_objects/*/write")));
}
// find parameter substitution specified tenant
Pattern parameterPattern = Pattern.compile("^\\$\\{attr");
List<String> matchingParameterTenantList = tenant.getTenant_patterns().stream().filter(parameterPattern.asPredicate()).collect(Collectors.toList());
for(String matchingParameterTenant : matchingParameterTenantList ) {
tuples.add(new Tuple<String, Boolean>(matchingParameterTenant,agr.resolvedActions(tenant.getAllowed_actions()).contains("kibana:saved_objects/*/write"))) ;
}
}
}

Expand Down Expand Up @@ -1096,11 +1100,22 @@ public Map<String, Boolean> mapTenants(final User user, Set<String> roles) {
result.put(user.getName(), true);

tenantsMM.entries().stream().filter(e -> roles.contains(e.getKey())).filter(e -> !user.getName().equals(e.getValue().v1())).forEach(e -> {
final String tenant = e.getValue().v1();

// replaceProperties for tenant name because
// at this point e.getValue().v1() can be in this form : "${attr.[internal|jwt|proxy|ldap].*}"
// let's substitute it with the eventual value of the user's attribute
final String tenant = replaceProperties(e.getValue().v1(),user);
final boolean rw = e.getValue().v2();

if (rw || !result.containsKey(tenant)) { //RW outperforms RO
result.put(tenant, rw);

// We want to make sure that we add a tenant that exissts
// Indeed, because we don't have control over what will be
// passed on as values of users' attributes, we have to make
// sure that we don't allow them to select tenants that do not exist.
if(ConfigModelV7.this.tenants.getCEntries().keySet().contains(tenant)) {
result.put(tenant, rw);
}
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.HashMap;
import java.util.Map;

import org.apache.http.Header;
import org.apache.http.HttpStatus;
import org.apache.http.message.BasicHeader;
import org.junit.Assert;
Expand All @@ -40,6 +41,9 @@
import org.opensearch.security.test.helper.rest.RestHelper;
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;

public class MultitenancyTests extends SingleClusterTest {

@Override
Expand Down Expand Up @@ -387,4 +391,54 @@ public void testDashboardsAlias65() throws Exception {
Assert.assertTrue(res.getBody().contains(".kibana_-900636979_kibanaro"));
}


@Test
public void testTenantParametersSubstitution() throws Exception {
final Settings settings = Settings.builder()
.build();
setup(settings);
final RestHelper rh = nonSslRestHelper();

HttpResponse res;
final String url = ".kibana/_doc/5.6.0?pretty";

final String tenantName = "tenant_parameters_substitution";
final String createTenantBody = "{\"buildNum\": 15460, \"defaultIndex\": \"plop\", \"tenant\": \"" + tenantName + "\"}";
final Header asNoAccessUser = encodeBasicHeader("hr_employee", "hr_employee");
final Header asUser = encodeBasicHeader("user_tenant_parameters_substitution", "user_tenant_parameters_substitution");

final Header actOnNoAccessTenant = new BasicHeader("securitytenant", "blafasel");
final Header actOnUserTenant = new BasicHeader("securitytenant", tenantName);

res = rh.executePutRequest(url, createTenantBody, asUser, actOnNoAccessTenant);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));

res = rh.executePutRequest(url, createTenantBody, asNoAccessUser, actOnUserTenant);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));

res = rh.executePutRequest(url, createTenantBody, asUser, actOnUserTenant);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_CREATED));

res = rh.executeGetRequest(url, asUser, actOnUserTenant);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(res.findValueInJson("_source.tenant"), equalTo(tenantName));


final String tenantNameAppended = "tenant_parameters_substitution_1";
final String createTenantAppendedBody = "{\"buildNum\": 15460, \"defaultIndex\": \"plop\", \"tenant\": \"" + tenantNameAppended + "\"}";
final Header userTenantAppended = new BasicHeader("securitytenant", tenantNameAppended);

res = rh.executeGetRequest(url, asNoAccessUser, userTenantAppended);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));

res = rh.executeGetRequest(url, asUser, userTenantAppended);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_NOT_FOUND));

res = rh.executePutRequest(url, createTenantAppendedBody, asUser, userTenantAppended);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_CREATED));

res = rh.executeGetRequest(url, asUser, userTenantAppended);
assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(res.findValueInJson("_source.tenant"), equalTo(tenantNameAppended));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import javax.net.ssl.SSLContext;

import com.fasterxml.jackson.databind.JsonNode;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
Expand Down Expand Up @@ -72,9 +74,12 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.test.helper.cluster.ClusterInfo;
import org.opensearch.security.test.helper.file.FileHelper;

import static org.junit.jupiter.api.Assertions.fail;

public class RestHelper {

protected final Logger log = LogManager.getLogger(RestHelper.class);
Expand Down Expand Up @@ -357,6 +362,41 @@ public String toString() {
+ ", statusReason=" + statusReason + "]";
}

/**
* Given a json path with dots delimiated returns the object at the leaf
*/
public String findValueInJson(final String jsonDotPath) {
// Make sure its json / then parse it
if (!isJsonContentType()) {
fail("Response was expected to be JSON, body was: \n" + body);
}
JsonNode currentNode = null;
try {
currentNode = DefaultObjectMapper.readTree(body);
} catch (final Exception e) {
throw new RuntimeException(e);
}

// Break the path into parts, and scan into the json object
try (final Scanner jsonPathScanner = new Scanner(jsonDotPath).useDelimiter("\\.")) {
if (!jsonPathScanner.hasNext()) {
fail("Invalid json dot path '" + jsonDotPath + "', rewrite with '.' characters between path elements.");
}
do {
final String pathEntry = jsonPathScanner.next();
if (!currentNode.has(pathEntry)) {
fail("Unable to resolve '" + jsonDotPath + "', on path entry '" + pathEntry + "' from available fields " + currentNode.toPrettyString());
}
currentNode = currentNode.get(pathEntry);
} while (jsonPathScanner.hasNext());

if (!currentNode.isValueNode()) {
fail("Unexpected value note, index directly to the object to reference, object\n" + currentNode.toPrettyString());
}
return currentNode.asText();
}
}

private static HttpResponse from(Future<HttpResponse> future) {
try {
return future.get();
Expand Down
8 changes: 8 additions & 0 deletions src/test/resources/multitenancy/internal_users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,11 @@ kibanaro:
backend_roles: []
attributes: {}
description: "Migrated from v6"
user_tenant_parameters_substitution:
hash: "$2y$12$xgJfGiHpYOkRpF9W9dXYZOpJJ4bHz3VTwdv7ZZYTwlvx7NbH62qUi"
reserved: false
hidden: false
backend_roles: []
attributes:
attribute1: "tenant_parameters_substitution"
description: "PR# 819 / Issue#817"
17 changes: 17 additions & 0 deletions src/test/resources/multitenancy/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,20 @@ opendistro_security_role_starfleet_captains:
- "command_tenant"
allowed_actions:
- "kibana_all_write"
opendistro_security_role_tenant_parameters_substitution:
reserved: false
hidden: false
description: "PR#819 / Issue#817"
cluster_permissions:
- "OPENDISTRO_SECURITY_CLUSTER_COMPOSITE_OPS"
index_permissions:
- index_patterns:
- "?kibana"
allowed_actions:
- "ALL"
tenant_permissions:
- tenant_patterns:
- "${attr.internal.attribute1}"
- "${attr.internal.attribute1}_1"
allowed_actions:
- "kibana_all_write"
9 changes: 9 additions & 0 deletions src/test/resources/multitenancy/roles_mapping.yml
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,12 @@ opendistro_security_kibana4_testindex:
- "test"
and_backend_roles: []
description: "Migrated from v6"
opendistro_security_role_tenant_parameters_substitution:
reserved: false
hidden: false
backend_roles: []
hosts: []
users:
- "user_tenant_parameters_substitution"
and_backend_roles: []
description: "PR#819 / Issue#817"
8 changes: 8 additions & 0 deletions src/test/resources/multitenancy/roles_tenants.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,11 @@ human_resources:
reserved: false
hidden: false
description: "Migrated from v6"
tenant_parameters_substitution:
reserved: false
hidden: false
description: "PR#819 / Issue#817"
tenant_parameters_substitution_1:
reserved: false
hidden: false
description: "PR#819 / Issue#817"

0 comments on commit ce59944

Please sign in to comment.