diff --git a/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java b/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java index 3f7b4737bf..e4d75c5611 100644 --- a/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java +++ b/src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java @@ -15,6 +15,7 @@ import java.util.Set; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -62,6 +63,15 @@ public class PrivilegesInterceptorImpl extends PrivilegesInterceptor { "0-1" ); + private static final ImmutableSet READ_ONLY_ALLOWED_ACTIONS = ImmutableSet.of( + "indices:admin/get", + "indices:data/read/get", + "indices:data/read/search", + "indices:data/read/msearch", + "indices:data/read/mget", + "indices:data/read/mget[shard]" + ); + protected final Logger log = LogManager.getLogger(this.getClass()); public PrivilegesInterceptorImpl( @@ -90,7 +100,7 @@ private boolean isTenantAllowed( log.debug("request " + request.getClass()); } - if (tenants.get(requestedTenant) == Boolean.FALSE && action.startsWith("indices:data/write")) { + if (tenants.get(requestedTenant) == Boolean.FALSE && !READ_ONLY_ALLOWED_ACTIONS.contains(action)) { log.warn("Tenant {} is not allowed to write (user: {})", requestedTenant, user.getName()); return false; } diff --git a/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java b/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java index 598d1662d7..e31e4de425 100644 --- a/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java +++ b/src/test/java/org/opensearch/security/multitenancy/test/MultitenancyTests.java @@ -623,4 +623,129 @@ public void testMultitenancyAnonymousUser() throws Exception { res = rh.executeGetRequest(url, new BasicHeader("securitytenant", "human_resources")); Assert.assertEquals(HttpStatus.SC_FORBIDDEN, res.getStatusCode()); } + + @Test + public void testMultitenancyUserReadOnlyActions() throws Exception { + setup(Settings.EMPTY); + + /* Create the tenant for the anonymous user to run the tests */ + final String tenant = "test_tenant_ro"; + + final TenantExpectation tenantExpectation = new TenantExpectation(); + tenantExpectation.isTenantWritable = "false"; + tenantExpectation.createDocStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.updateDocStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.updateIndexStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.deleteIndexStatuCode = HttpStatus.SC_FORBIDDEN; + + verifyTenantActions(nonSslRestHelper(), tenant, tenantExpectation, encodeBasicHeader("user_a", "user_a")); + } + + @Test + public void testMultitenancyUserReadWriteActions() throws Exception { + setup(Settings.EMPTY); + + /* Create the tenant for the anonymous user to run the tests */ + final String tenant = "opendistro_security_anonymous"; + + final TenantExpectation tenantExpectation = new TenantExpectation(); + tenantExpectation.isTenantWritable = "true"; + tenantExpectation.createDocStatusCode = HttpStatus.SC_CREATED; + tenantExpectation.updateDocStatusCode = HttpStatus.SC_OK; + tenantExpectation.updateIndexStatusCode = HttpStatus.SC_OK; + tenantExpectation.deleteIndexStatuCode = HttpStatus.SC_BAD_REQUEST; // tenant index cannot be deleted because its an alias + + verifyTenantActions(nonSslRestHelper(), tenant, tenantExpectation, encodeBasicHeader("admin", "admin")); + } + + @Test + public void testMultitenancyAnonymousUserReadOnlyActions() throws Exception { + setup(Settings.EMPTY, new DynamicSecurityConfig().setConfig("config_anonymous.yml"), Settings.EMPTY); + + /* Create the tenant for the anonymous user to run the tests */ + final String tenant = "anonymous_tenant"; + + final TenantExpectation tenantExpectation = new TenantExpectation(); + tenantExpectation.isTenantWritable = "false"; + tenantExpectation.createDocStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.updateDocStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.updateIndexStatusCode = HttpStatus.SC_FORBIDDEN; + tenantExpectation.deleteIndexStatuCode = HttpStatus.SC_FORBIDDEN; + + verifyTenantActions(nonSslRestHelper(), tenant, tenantExpectation, /* Anonymous user*/ null); + } + + @Test + public void testMultitenancyAnonymousUserWriteActionAllowed() throws Exception { + setup(Settings.EMPTY, new DynamicSecurityConfig().setConfig("config_anonymous.yml"), Settings.EMPTY); + + /* Create the tenant for the anonymous user to run the tests */ + final String tenant = "opendistro_security_anonymous"; + + final TenantExpectation tenantExpectation = new TenantExpectation(); + tenantExpectation.isTenantWritable = "true"; + tenantExpectation.createDocStatusCode = HttpStatus.SC_FORBIDDEN; // tenant write permissions != index write permissions + tenantExpectation.updateDocStatusCode = HttpStatus.SC_FORBIDDEN; // tenant write permissions != index write permissions + tenantExpectation.updateIndexStatusCode = HttpStatus.SC_OK; + tenantExpectation.deleteIndexStatuCode = HttpStatus.SC_BAD_REQUEST; // tenant index cannot be deleted because its an alias + + verifyTenantActions(nonSslRestHelper(), tenant, tenantExpectation, /* Anonymous user*/ null); + } + + private static void verifyTenantActions( + final RestHelper rh, + final String tenant, + final TenantExpectation tenantExpectation, + final Header asUser + ) { + final BasicHeader inTenant = new BasicHeader("securitytenant", tenant); + final HttpResponse adminIndexDocToCreateTenant = rh.executePutRequest( + ".kibana/_doc/5.6.0", + "{\"buildNum\": 15460, \"defaultIndex\": \"anon\", \"tenant\": \"" + tenant + "\"}", + encodeBasicHeader("admin", "admin"), + inTenant + ); + assertThat(adminIndexDocToCreateTenant.getBody(), adminIndexDocToCreateTenant.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); + + final HttpResponse authInfo = rh.executeGetRequest("/_opendistro/_security/authinfo?pretty", inTenant, asUser); + assertThat(authInfo.getBody(), authInfo.findValueInJson("tenants." + tenant), equalTo(tenantExpectation.isTenantWritable)); + + final HttpResponse search = rh.executeGetRequest(".kibana/_search", inTenant, asUser); + assertThat(search.getBody(), search.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse msearch = rh.executePostRequest(".kibana/_msearch", "{}\n{\"query\":{\"match_all\":{}}}\n", inTenant, asUser); + assertThat(msearch.getBody(), msearch.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse mget = rh.executePostRequest(".kibana/_mget", "{\"docs\":[{\"_id\":\"5.6.0\"}]}", inTenant, asUser); + assertThat(mget.getBody(), mget.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse getDoc = rh.executeGetRequest(".kibana/_doc/5.6.0", inTenant, asUser); + assertThat(getDoc.getBody(), getDoc.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse createDoc = rh.executePostRequest(".kibana/_doc", "{}", inTenant, asUser); + assertThat(createDoc.getBody(), createDoc.getStatusCode(), equalTo(tenantExpectation.createDocStatusCode)); + + final HttpResponse updateDoc = rh.executePutRequest(".kibana/_doc/5.6.0", "{}", inTenant, asUser); + assertThat(updateDoc.getBody(), updateDoc.getStatusCode(), equalTo(tenantExpectation.updateDocStatusCode)); + + final HttpResponse deleteDoc = rh.executeDeleteRequest(".kibana/_doc/5.6.0", inTenant, asUser); + assertThat(deleteDoc.getBody(), deleteDoc.getStatusCode(), equalTo(tenantExpectation.updateDocStatusCode)); + + final HttpResponse getKibana = rh.executeGetRequest(".kibana", inTenant, asUser); + assertThat(getKibana.getBody(), getKibana.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse closeKibana = rh.executePostRequest(".kibana/_close", "{}", inTenant, asUser); + assertThat(closeKibana.getBody(), closeKibana.getStatusCode(), equalTo(tenantExpectation.updateIndexStatusCode)); + + final HttpResponse deleteKibana = rh.executeDeleteRequest(".kibana", inTenant, asUser); + assertThat(deleteKibana.getBody(), deleteKibana.getStatusCode(), equalTo(tenantExpectation.deleteIndexStatuCode)); + } + + private static class TenantExpectation { + private String isTenantWritable; + private int createDocStatusCode; + private int updateDocStatusCode; + private int updateIndexStatusCode; + private int deleteIndexStatuCode; + } } diff --git a/src/test/resources/multitenancy/roles_mapping.yml b/src/test/resources/multitenancy/roles_mapping.yml index 397171a360..d1fb92b471 100644 --- a/src/test/resources/multitenancy/roles_mapping.yml +++ b/src/test/resources/multitenancy/roles_mapping.yml @@ -184,3 +184,21 @@ opendistro_security_anonymous_multitenancy: - "opendistro_security_anonymous" and_backend_roles: [] description: "PR#2459" +opendistro_security_kibana_testindex: + reserved: false + hidden: false + backend_roles: + hosts: [] + users: + - "user_a" + and_backend_roles: [] + description: "" +all_access: + reserved: false + hidden: false + backend_roles: + hosts: [] + users: + - "admin" + and_backend_roles: [] + description: "admin user can do everything" diff --git a/src/test/resources/multitenancy/roles_tenants.yml b/src/test/resources/multitenancy/roles_tenants.yml index a4025e94da..8402a6f301 100644 --- a/src/test/resources/multitenancy/roles_tenants.yml +++ b/src/test/resources/multitenancy/roles_tenants.yml @@ -66,3 +66,7 @@ anonymous_tenant: reserved: false hidden: false description: "PR#2459" +opendistro_security_anonymous: + reserved: false + hidden: false + description: "tenant that is writable for anonymous users"