Tenant Permissions : added the possibility to specify tenants via parameter

src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java

@@ -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;
@@ -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());
     }
@@ -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() {
 
@@ -1014,10 +1015,6 @@ private static boolean impliesTypePerm(Set<IndexPattern> ipatterns, Resolved res
                 );
     }
 
-
-
-    //#######
-
     private class TenantHolder {
 
         private SetMultimap<String, Tuple<String, Boolean>> tenantsMM = null;
@@ -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;
                 }
@@ -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"))) ;
+                                }
                             }
                         }
 
@@ -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);
+                    }
                 }
             });
 

src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java

@@ -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;
@@ -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
@@ -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));
+    }
 }

src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java

@@ -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;
@@ -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);
@@ -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();

src/test/resources/multitenancy/internal_users.yml

@@ -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"

src/test/resources/multitenancy/roles.yml

@@ -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"

src/test/resources/multitenancy/roles_mapping.yml

@@ -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"

src/test/resources/multitenancy/roles_tenants.yml

@@ -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"