From 0decb330350782ac83c2fc070ddea80756e0c6c0 Mon Sep 17 00:00:00 2001 From: devplant Date: Fri, 31 Mar 2023 16:19:42 +0300 Subject: [PATCH 1/4] RED-5504 - Prevent using exact same database schema/buckets/index for multiple tenants - add unique constraints to tenants table --- .../service/TenantManagementService.java | 19 +++++ .../db/changelog/db.changelog-master.yaml | 2 + ...d-unique-constraint-for-tenants-table.yaml | 17 +++++ .../server/integration/tests/TenantsTest.java | 71 +++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/master/4-add-unique-constraint-for-tenants-table.yaml create mode 100644 persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java index 04d1106ea..bbdeae8e2 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java @@ -12,6 +12,7 @@ import java.sql.DriverManager; import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -86,6 +87,7 @@ public class TenantManagementService { private final GeneralConfigurationService generalConfigurationService; private final KeyCloakRoleManagerService keyCloakRoleManagerService; private final KeyCloakAdminClientService keycloak; + private final Map tenantsHostAndSchemaMap = new HashMap<>(); public TenantManagementService(EncryptionDecryptionService encryptionService, @@ -201,8 +203,16 @@ public class TenantManagementService { } + private void checkDuplicateHostAndSchema(String hostAndSchemaName) { + + if (tenantsHostAndSchemaMap.containsValue(hostAndSchemaName)) { + throw ConflictException.withObjectName("host and schema"); + } + } + private void createSchema(TenantRequest tenantRequest) { + checkDuplicateHostAndSchema(buildHostAndSchemaName(tenantRequest.getDatabaseConnection())); var jdbcUrl = JDBCUtils.buildJdbcUrl(tenantRequest.getDatabaseConnection()); try (Connection connection = DriverManager.getConnection(jdbcUrl, tenantRequest.getDatabaseConnection().getUsername(), @@ -212,11 +222,20 @@ public class TenantManagementService { jdbcTemplate.execute((StatementCallback) stmt -> stmt.execute("CREATE SCHEMA " + tenantRequest.getDatabaseConnection().getSchema())); jdbcTemplate.execute((StatementCallback) stmt -> stmt.execute("GRANT USAGE ON SCHEMA " + tenantRequest.getDatabaseConnection() .getSchema() + " TO " + tenantRequest.getDatabaseConnection().getUsername())); + + tenantsHostAndSchemaMap.put(tenantRequest.getTenantId(), buildHostAndSchemaName(tenantRequest.getDatabaseConnection())); } catch (Exception e) { log.info("Could not create schema, ignoring"); } } + private String buildHostAndSchemaName(DatabaseConnection databaseConnection) { + StringBuilder sb = new StringBuilder(databaseConnection.getHost()) + .append("currentSchema=") + .append(databaseConnection.getSchema()); + return sb.toString(); + } + private boolean tryToAccessRealm(String tenantId) { diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/db.changelog-master.yaml b/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/db.changelog-master.yaml index 8a632ee7b..d1fa43356 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/db.changelog-master.yaml @@ -5,3 +5,5 @@ databaseChangeLog: file: db/changelog/master/2-quartz.changelog.yaml - include: file: db/changelog/master/3-detailed-db-connection.changelog.yaml + - include: + file: db/changelog/master/4-add-unique-constraint-for-tenants-table.yaml diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/master/4-add-unique-constraint-for-tenants-table.yaml b/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/master/4-add-unique-constraint-for-tenants-table.yaml new file mode 100644 index 000000000..27baab89c --- /dev/null +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/resources/db/changelog/master/4-add-unique-constraint-for-tenants-table.yaml @@ -0,0 +1,17 @@ +databaseChangeLog: + - changeSet: + id: add-unique-constraint-for-tenants-table + author: corinaolariu + changes: + - addUniqueConstraint: + columnNames: db_host, db_schema + constraintName: unique_constraint_tenant_host_shema + tableName: tenant + - addUniqueConstraint: + columnNames: storage_s3_endpoint, storage_s3_region, storage_s3_bucket_name + constraintName: unique_constraint_tenant_s3_storage + tableName: tenant + - addUniqueConstraint: + columnNames: storage_azure_connection_string, storage_azure_container_name + constraintName: unique_constraint_tenant_azure_storage + tableName: tenant diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java new file mode 100644 index 000000000..7c5564a79 --- /dev/null +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java @@ -0,0 +1,71 @@ +package com.iqser.red.service.peristence.v1.server.integration.tests; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.Set; +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import com.iqser.red.keycloak.commons.roles.ApplicationRoles; +import com.iqser.red.service.peristence.v1.server.integration.client.TenantsClient; +import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.DatabaseConnection; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.RedUser; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.S3StorageConnection; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.SearchConnection; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.TenantRequest; + +import feign.FeignException; + +public class TenantsTest extends AbstractPersistenceServerServiceTest { + + @Autowired + private TenantsClient tenantsClient; + + @Test + public void testCreateTenantWithSameHostAndSchema() { + + var tenantRequest = TenantRequest.builder() + .tenantId("redaction2") + .displayName("Redaction default2") + .guid(UUID.randomUUID().toString()) + .databaseConnection(DatabaseConnection.builder() + .driver("postgresql") + .host("localhost") + .port("port") + .database("redaction") + .schema("myschema") + .username("redaction") + .password("redaction") + .build()) + .searchConnection(SearchConnection.builder() + .hosts(Set.of("elasticsearchHost2")) + .port(9200) + .scheme("https2") + .username("elastic") + .numberOfShards("1") + .numberOfReplicas("5") + .build()) + .s3StorageConnection(S3StorageConnection.builder() + .key("key") + .secret("secret") + .signerType("signerType") + .bucketName("bucketName2") + .region("eu") + .endpoint("endpoint2") + .build()) + .redUsers(List.of(RedUser.builder().username("user").password("password").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin1@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) + .build(); + + try { + tenantsClient.createTenant(tenantRequest); + } catch(FeignException e) { + assertThat(e.status()).isEqualTo(409); + } + } +} From 308811f9422e322e596f5be2bbe3e44f0af4d771 Mon Sep 17 00:00:00 2001 From: devplant Date: Mon, 3 Apr 2023 09:56:39 +0300 Subject: [PATCH 2/4] RED-5504 - Prevent using exact same database schema/buckets/index for multiple tenants - add junit test --- .../server/integration/tests/TenantsTest.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java index 7c5564a79..0842ced06 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java @@ -68,4 +68,48 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { assertThat(e.status()).isEqualTo(409); } } + + @Test + public void testCreateTenantWithDuplicateStorageS3() { + + var tenantRequest = TenantRequest.builder() + .tenantId("redaction2") + .displayName("Redaction default2") + .guid(UUID.randomUUID().toString()) + .databaseConnection(DatabaseConnection.builder() + .driver("postgresql") + .host("localhost") + .port("port") + .database("redaction") + .schema("myschema2") + .username("redaction") + .password("redaction") + .build()) + .searchConnection(SearchConnection.builder() + .hosts(Set.of("elasticsearchHost2")) + .port(9200) + .scheme("https2") + .username("elastic") + .numberOfShards("1") + .numberOfReplicas("5") + .build()) + .s3StorageConnection(S3StorageConnection.builder() + .key("key") + .secret("secret") + .signerType("signerType") + .bucketName("bucketName") + .region("eu") + .endpoint("endpoint") + .build()) + .redUsers(List.of(RedUser.builder().username("user").password("password").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin1@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) + .build(); + + try { + tenantsClient.createTenant(tenantRequest); + } catch(FeignException e) { + assertThat(e.status()).isEqualTo(409); + } + } } From de8d1178e743ae6e07df398781b7692c5d5971fc Mon Sep 17 00:00:00 2001 From: devplant Date: Mon, 3 Apr 2023 12:04:57 +0300 Subject: [PATCH 3/4] RED-5504 - Prevent using exact same database schema/buckets/index for multiple tenants - update junit test --- .../server/integration/tests/TenantsTest.java | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java index 0842ced06..3dcfe9642 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java @@ -12,11 +12,13 @@ import org.springframework.beans.factory.annotation.Autowired; import com.iqser.red.keycloak.commons.roles.ApplicationRoles; import com.iqser.red.service.peristence.v1.server.integration.client.TenantsClient; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.AzureStorageConnection; import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.DatabaseConnection; import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.RedUser; import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.S3StorageConnection; import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.SearchConnection; import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.TenantRequest; +import com.iqser.red.service.persistence.service.v1.api.shared.model.multitenancy.TenantResponse; import feign.FeignException; @@ -72,6 +74,8 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { @Test public void testCreateTenantWithDuplicateStorageS3() { + TenantResponse firstTenant = tenantsClient.getTenants().get(0); + var tenantRequest = TenantRequest.builder() .tenantId("redaction2") .displayName("Redaction default2") @@ -79,7 +83,7 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { .databaseConnection(DatabaseConnection.builder() .driver("postgresql") .host("localhost") - .port("port") + .port(firstTenant.getDatabaseConnection().getPort()) .database("redaction") .schema("myschema2") .username("redaction") @@ -112,4 +116,79 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { assertThat(e.status()).isEqualTo(409); } } + + @Test + public void testCreateTenantWithDuplicateAzure() { + + TenantResponse firstTenant = tenantsClient.getTenants().get(0); + + var tenantRequest = TenantRequest.builder() + .tenantId("redaction2") + .displayName("Redaction default2") + .guid(UUID.randomUUID().toString()) + .databaseConnection(DatabaseConnection.builder() + .driver("postgresql") + .host("localhost") + .port(firstTenant.getDatabaseConnection().getPort()) + .database("redaction") + .schema("myschema2") + .username("redaction") + .password("redaction") + .build()) + .searchConnection(SearchConnection.builder() + .hosts(Set.of("elasticsearchHost2")) + .port(9200) + .scheme("https2") + .username("elastic") + .numberOfShards("1") + .numberOfReplicas("5") + .build()) + .azureStorageConnection(AzureStorageConnection.builder() + .connectionString("connection") + .containerName("container") + .build()) + .redUsers(List.of(RedUser.builder().username("user").password("password").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin1@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) + .build(); + + tenantsClient.createTenant(tenantRequest); + assertThat(tenantsClient.getTenants().size()).isEqualTo(2); + + var tenantRequest2 = TenantRequest.builder() + .tenantId("redaction2") + .displayName("Redaction default2") + .guid(UUID.randomUUID().toString()) + .databaseConnection(DatabaseConnection.builder() + .driver("postgresql") + .host("localhost") + .port(firstTenant.getDatabaseConnection().getPort()) + .database("redaction") + .schema("myschema3") + .username("redaction") + .password("redaction") + .build()) + .searchConnection(SearchConnection.builder() + .hosts(Set.of("elasticsearchHost2")) + .port(9200) + .scheme("https2") + .username("elastic") + .numberOfShards("1") + .numberOfReplicas("5") + .build()) + .azureStorageConnection(AzureStorageConnection.builder() + .connectionString("connection") + .containerName("container") + .build()) + .redUsers(List.of(RedUser.builder().username("user").password("password").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin1@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build(), + RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) + .build(); + + try { + tenantsClient.createTenant(tenantRequest2); + } catch(FeignException e) { + assertThat(e.status()).isEqualTo(409); + } + } } From 663770bf9b262c30af3f5c9ccf034bea65100dc2 Mon Sep 17 00:00:00 2001 From: devplant Date: Tue, 4 Apr 2023 14:43:20 +0300 Subject: [PATCH 4/4] RED-5504 - Prevent using exact same database schema/buckets/index for multiple tenants - remove tenantsHostAndSchemaMap - update junit test --- .../service/TenantManagementService.java | 20 --------- .../server/integration/tests/TenantsTest.java | 41 ++++++++++++------- 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java index bbdeae8e2..f3f846edc 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/TenantManagementService.java @@ -12,7 +12,6 @@ import java.sql.DriverManager; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -87,8 +86,6 @@ public class TenantManagementService { private final GeneralConfigurationService generalConfigurationService; private final KeyCloakRoleManagerService keyCloakRoleManagerService; private final KeyCloakAdminClientService keycloak; - private final Map tenantsHostAndSchemaMap = new HashMap<>(); - public TenantManagementService(EncryptionDecryptionService encryptionService, @Qualifier("tenantLiquibaseProperties") LiquibaseProperties liquibaseProperties, @@ -203,16 +200,8 @@ public class TenantManagementService { } - private void checkDuplicateHostAndSchema(String hostAndSchemaName) { - - if (tenantsHostAndSchemaMap.containsValue(hostAndSchemaName)) { - throw ConflictException.withObjectName("host and schema"); - } - } - private void createSchema(TenantRequest tenantRequest) { - checkDuplicateHostAndSchema(buildHostAndSchemaName(tenantRequest.getDatabaseConnection())); var jdbcUrl = JDBCUtils.buildJdbcUrl(tenantRequest.getDatabaseConnection()); try (Connection connection = DriverManager.getConnection(jdbcUrl, tenantRequest.getDatabaseConnection().getUsername(), @@ -222,20 +211,11 @@ public class TenantManagementService { jdbcTemplate.execute((StatementCallback) stmt -> stmt.execute("CREATE SCHEMA " + tenantRequest.getDatabaseConnection().getSchema())); jdbcTemplate.execute((StatementCallback) stmt -> stmt.execute("GRANT USAGE ON SCHEMA " + tenantRequest.getDatabaseConnection() .getSchema() + " TO " + tenantRequest.getDatabaseConnection().getUsername())); - - tenantsHostAndSchemaMap.put(tenantRequest.getTenantId(), buildHostAndSchemaName(tenantRequest.getDatabaseConnection())); } catch (Exception e) { log.info("Could not create schema, ignoring"); } } - private String buildHostAndSchemaName(DatabaseConnection databaseConnection) { - StringBuilder sb = new StringBuilder(databaseConnection.getHost()) - .append("currentSchema=") - .append(databaseConnection.getSchema()); - return sb.toString(); - } - private boolean tryToAccessRealm(String tenantId) { diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java index 3dcfe9642..229f1bba1 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/TenantsTest.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Set; import java.util.UUID; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -30,6 +31,8 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { @Test public void testCreateTenantWithSameHostAndSchema() { + TenantResponse firstTenant = tenantsClient.getTenants().get(0); + var tenantRequest = TenantRequest.builder() .tenantId("redaction2") .displayName("Redaction default2") @@ -37,7 +40,7 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { .databaseConnection(DatabaseConnection.builder() .driver("postgresql") .host("localhost") - .port("port") + .port(firstTenant.getDatabaseConnection().getPort()) .database("redaction") .schema("myschema") .username("redaction") @@ -64,11 +67,15 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) .build(); - try { + Exception exception = Assertions.assertThrows(FeignException.Conflict.class, () -> { tenantsClient.createTenant(tenantRequest); - } catch(FeignException e) { - assertThat(e.status()).isEqualTo(409); - } + }); + + String expectedMessage = "An object of type tenant already exists"; + String actualMessage = exception.getMessage(); + + assertThat(actualMessage).contains(expectedMessage); + } @Test @@ -110,11 +117,14 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) .build(); - try { + Exception exception = Assertions.assertThrows(FeignException.Conflict.class, () -> { tenantsClient.createTenant(tenantRequest); - } catch(FeignException e) { - assertThat(e.status()).isEqualTo(409); - } + }); + + String expectedMessage = "An object of type tenant already exists"; + String actualMessage = exception.getMessage(); + + assertThat(actualMessage).contains(expectedMessage); } @Test @@ -185,10 +195,13 @@ public class TenantsTest extends AbstractPersistenceServerServiceTest { RedUser.builder().username("manageradmin2@test.com").password("secret").redRoles(ApplicationRoles.ROLE_DATA.keySet()).build())) .build(); - try { - tenantsClient.createTenant(tenantRequest2); - } catch(FeignException e) { - assertThat(e.status()).isEqualTo(409); - } + Exception exception = Assertions.assertThrows(FeignException.Conflict.class, () -> { + tenantsClient.createTenant(tenantRequest); + }); + + String expectedMessage = "An object of type tenant already exists"; + String actualMessage = exception.getMessage(); + + assertThat(actualMessage).contains(expectedMessage); } }