From a79165bb09e128710b08a14e840fd2dc6d55d995 Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Fri, 18 Aug 2023 15:14:10 +0200 Subject: [PATCH 1/8] updated pmd checks --- config/pmd/test_pmd.xml | 3 ++- .../tenantusermanagement/service/TenantManagementService.java | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/config/pmd/test_pmd.xml b/config/pmd/test_pmd.xml index ac03639..d302c54 100644 --- a/config/pmd/test_pmd.xml +++ b/config/pmd/test_pmd.xml @@ -9,8 +9,9 @@ - + + diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index 24bc30b..b3c2933 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -297,6 +297,10 @@ public class TenantManagementService implements TenantProvider { realm.setUsers(users.stream().map(this::toUserRepresentation).collect(Collectors.toList())); + var policyString = "digits and length and lowerCase and notEmail and notUsername and specialChars and upperCase"; +// PasswordPolicy passwordPolicy = PasswordPolicy.parse(session, policyString); + realm.setPasswordPolicy(policyString); + keycloak.getAdminClient().realms().create(realm); } From c904e5947cd8226c41455ae88ee0665337cd4a86 Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Mon, 21 Aug 2023 09:06:20 +0200 Subject: [PATCH 2/8] added password policy and caught exception --- build.gradle.kts | 1 + .../service/TenantManagementService.java | 15 +++++++++++---- .../com/knecon/fforesight/tests/TenantsTest.java | 5 +++++ .../fforesight/utils/TestTenantService.java | 4 ++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 1e244ab..07c114e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -88,6 +88,7 @@ dependencies { implementation("com.google.guava:guava:31.1-jre") implementation("org.liquibase:liquibase-core:4.17.2") implementation("org.keycloak:keycloak-admin-client:21.0.1") + implementation("org.keycloak:keycloak-model-jpa:21.0.1") implementation("org.springframework.boot:spring-boot-starter-amqp") implementation("org.springframework.boot:spring-boot-starter-validation") implementation("org.springframework.retry:spring-retry") diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index b3c2933..d016aaa 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -14,6 +14,7 @@ import java.util.stream.Collectors; import javax.sql.DataSource; +import org.keycloak.policy.PasswordPolicyNotMetException; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -192,6 +193,8 @@ public class TenantManagementService implements TenantProvider { if (!realmReady) { throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to create KC realm"); } + keycloak.getAdminClient().realms(); + System.out.println(keycloak.getAdminClient().realm("test-tenant-fforesight").toRepresentation()); generalConfigurationService.initGeneralConfiguration(tenantRequest.getTenantId()); keyCloakRoleManagerService.updateRoles(tenantRequest.getTenantId()); @@ -297,9 +300,12 @@ public class TenantManagementService implements TenantProvider { realm.setUsers(users.stream().map(this::toUserRepresentation).collect(Collectors.toList())); - var policyString = "digits and length and lowerCase and notEmail and notUsername and specialChars and upperCase"; -// PasswordPolicy passwordPolicy = PasswordPolicy.parse(session, policyString); - realm.setPasswordPolicy(policyString); + try { + realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); + } catch (PasswordPolicyNotMetException e) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + } + keycloak.getAdminClient().realms().create(realm); } @@ -449,6 +455,7 @@ public class TenantManagementService implements TenantProvider { return tenantRepository.findAll().stream().map(this::convert).collect(Collectors.toList()); } + public TenantResponse removePasswords(TenantResponse tenantResponse) { if (tenantResponse.getDatabaseConnection() != null) { @@ -463,7 +470,7 @@ public class TenantManagementService implements TenantProvider { tenantResponse.getAzureStorageConnection().setConnectionString(PASSWORD); } - if (tenantResponse.getS3StorageConnection() != null){ + if (tenantResponse.getS3StorageConnection() != null) { tenantResponse.getS3StorageConnection().setSecret(PASSWORD); } diff --git a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java index 0e4195d..3fe7071 100644 --- a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java +++ b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java @@ -21,6 +21,7 @@ import com.knecon.fforesight.tenantusermanagement.model.TenantRequest; import com.knecon.fforesight.utils.TestTenantService; import feign.FeignException; +import lombok.SneakyThrows; public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { @@ -37,10 +38,13 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { @Test + @SneakyThrows public void testCreateNewTenant() { testTenantService.createTestTenantIfNotExists("new_tenant", minioPort); + Thread.sleep(100000); + TenantContext.setTenantId("new_tenant"); var deploymentKey = tenantsClient.getDeploymentKey("new_tenant"); @@ -52,6 +56,7 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { assertThat(tenantsClient.getTenants().stream().anyMatch(t -> t.getTenantId().equals("new_tenant"))).isTrue(); TenantContext.clear(); + } @Test diff --git a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java index 3f0e171..c1f2009 100644 --- a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java +++ b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java @@ -60,7 +60,7 @@ public class TestTenantService { .tenantId(testTenantId) .displayName(testTenantId) .guid(UUID.randomUUID().toString()) - .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password("secret").email("test@fforesight.com").build())) + .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password("secret1234!OH").email("test@fforesight.com").build())) .databaseConnection(DatabaseConnection.builder() .driver("postgresql") .host(SpringPostgreSQLTestContainer.getInstance().getHost()) @@ -84,7 +84,7 @@ public class TestTenantService { assertThat(response.getGuid()).isNotBlank(); TenantContext.setTenantId(testTenantId); - tokenService.setUser("test@fforesight.com", "secret"); + tokenService.setUser("test@fforesight.com", "secret1234!OH"); var tenant = tenantsClient.getTenant(testTenantId); assertThat(tenant.getGuid()).isNotBlank(); From 2d10b2b5c63afe7ae3784498b3d73d39b5961bf5 Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Mon, 21 Aug 2023 20:48:52 +0200 Subject: [PATCH 3/8] added password policy and caught exception --- build.gradle.kts | 1 - .../service/TenantManagementService.java | 39 +++++++++++++++---- .../knecon/fforesight/tests/TenantsTest.java | 20 +++++++++- .../fforesight/utils/TestTenantService.java | 38 +++++++++++++++--- 4 files changed, 82 insertions(+), 16 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 07c114e..1e244ab 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -88,7 +88,6 @@ dependencies { implementation("com.google.guava:guava:31.1-jre") implementation("org.liquibase:liquibase-core:4.17.2") implementation("org.keycloak:keycloak-admin-client:21.0.1") - implementation("org.keycloak:keycloak-model-jpa:21.0.1") implementation("org.springframework.boot:spring-boot-starter-amqp") implementation("org.springframework.boot:spring-boot-starter-validation") implementation("org.springframework.retry:spring-retry") diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index d016aaa..b903ef9 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -10,11 +10,12 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.sql.DataSource; -import org.keycloak.policy.PasswordPolicyNotMetException; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -194,7 +195,6 @@ public class TenantManagementService implements TenantProvider { throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to create KC realm"); } keycloak.getAdminClient().realms(); - System.out.println(keycloak.getAdminClient().realm("test-tenant-fforesight").toRepresentation()); generalConfigurationService.initGeneralConfiguration(tenantRequest.getTenantId()); keyCloakRoleManagerService.updateRoles(tenantRequest.getTenantId()); @@ -300,12 +300,7 @@ public class TenantManagementService implements TenantProvider { realm.setUsers(users.stream().map(this::toUserRepresentation).collect(Collectors.toList())); - try { - realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); - } catch (PasswordPolicyNotMetException e) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage(), e); - } - + realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); keycloak.getAdminClient().realms().create(realm); } @@ -325,6 +320,7 @@ public class TenantManagementService implements TenantProvider { var credentialRepresentation = new CredentialRepresentation(); credentialRepresentation.setType("password"); + validatePasswordWithPolicy(redUser.getPassword(), redUser.getUsername(), redUser.getEmail()); credentialRepresentation.setValue(redUser.getPassword()); var user = new UserRepresentation(); @@ -351,6 +347,33 @@ public class TenantManagementService implements TenantProvider { } + private void validatePasswordWithPolicy(String password, String username, String email) { + if (password.contains(username) || password.contains(email) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password does not match password policy."); + } + } + + + private boolean checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(String str) { + + if (str.length() >= 12) { + Pattern lowerCaseLetter = Pattern.compile("[a-z]"); + Pattern upperCaseLetter = Pattern.compile("[A-Z]"); + Pattern digit = Pattern.compile("[0-9]"); + Pattern special = Pattern.compile("[^a-zA-Z0-9 ]"); + + Matcher hasLowerCaseLetter = lowerCaseLetter.matcher(str); + Matcher hasUpperCaseLetter = upperCaseLetter.matcher(str); + Matcher hasDigit = digit.matcher(str); + Matcher hasSpecialChar = special.matcher(str); + + return hasLowerCaseLetter.find() && hasUpperCaseLetter.find() && hasDigit.find() && hasSpecialChar.find(); + } + + return false; + } + + @Override @Transactional public void updateDetails(String tenantId, com.knecon.fforesight.tenantcommons.model.UpdateDetailsRequest request) { diff --git a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java index 3fe7071..8b6a67c 100644 --- a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java +++ b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java @@ -1,11 +1,15 @@ package com.knecon.fforesight.tests; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.List; import java.util.Map; import java.util.Set; +import javax.ws.rs.BadRequestException; + import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -18,6 +22,7 @@ import com.knecon.fforesight.tenantcommons.model.DatabaseConnection; import com.knecon.fforesight.tenantcommons.model.S3StorageConnection; import com.knecon.fforesight.tenantcommons.model.SearchConnection; import com.knecon.fforesight.tenantusermanagement.model.TenantRequest; +import com.knecon.fforesight.tenantusermanagement.model.TenantUser; import com.knecon.fforesight.utils.TestTenantService; import feign.FeignException; @@ -43,8 +48,6 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { testTenantService.createTestTenantIfNotExists("new_tenant", minioPort); - Thread.sleep(100000); - TenantContext.setTenantId("new_tenant"); var deploymentKey = tenantsClient.getDeploymentKey("new_tenant"); @@ -226,4 +229,17 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { TenantContext.clear(); } + + @Test + public void testPasswordPolicy() { + testTenantService.createTestTenantIfNotExists("new_tenant", minioPort); + + TenantContext.setTenantId("new_tenant"); + + assertThat(tenantsClient.getTenant("new_tenant")).isNotNull(); + + assertThatThrownBy(() -> testTenantService.createTestTenantIfNotExists("new_tenant_2", minioPort, "passwordDoesNotMatchPolicy")).isInstanceOf(FeignException.class); + + TenantContext.clear(); + } } diff --git a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java index c1f2009..ef1b956 100644 --- a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java +++ b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java @@ -29,6 +29,7 @@ public class TestTenantService { private final TokenService tokenService; + public void createTestTenantIfNotExists(String testTenantId, int actualPort) { try { @@ -42,7 +43,9 @@ public class TestTenantService { } + public void createTestTenantWithoutStorageIfNotExist(String testTenantId) { + try { var tenantExists = internalTenantsResource.getTenant(testTenantId); assertThat(tenantExists.getGuid()).isNotBlank(); @@ -53,14 +56,36 @@ public class TestTenantService { } } + + public void createTestTenantIfNotExists(String testTenantId, int actualPort, String userPassword) { + + try { + var tenantExists = internalTenantsResource.getTenant(testTenantId); + assertThat(tenantExists.getGuid()).isNotBlank(); + + } catch (Exception e) { + // not found + createUser(testTenantId, actualPort, true, userPassword); + } + + } + + private void createUser(String testTenantId, int actualPort, boolean withStorage) { + + createUser(testTenantId, actualPort, withStorage, "secret1234!OH"); + } + + + private void createUser(String testTenantId, int actualPort, boolean withStorage, String userPassword) { + // not found TenantRequest tenantRequest; var tenantRequestBuilder = TenantRequest.builder() .tenantId(testTenantId) .displayName(testTenantId) .guid(UUID.randomUUID().toString()) - .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password("secret1234!OH").email("test@fforesight.com").build())) + .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password(userPassword).email("test@fforesight.com").build())) .databaseConnection(DatabaseConnection.builder() .driver("postgresql") .host(SpringPostgreSQLTestContainer.getInstance().getHost()) @@ -73,9 +98,12 @@ public class TestTenantService { .searchConnection(SearchConnection.builder().hosts(Set.of("localhost")).port(9200).scheme("http").numberOfShards("1").numberOfReplicas("5").build()); if (withStorage) { - tenantRequest = tenantRequestBuilder - .s3StorageConnection(S3StorageConnection.builder().key("minioadmin").secret("minioadmin").bucketName("redaction").endpoint("http://localhost:" + actualPort).build()) - .build(); + tenantRequest = tenantRequestBuilder.s3StorageConnection(S3StorageConnection.builder() + .key("minioadmin") + .secret("minioadmin") + .bucketName("redaction") + .endpoint("http://localhost:" + actualPort) + .build()).build(); } else { tenantRequest = tenantRequestBuilder.build(); } @@ -84,7 +112,7 @@ public class TestTenantService { assertThat(response.getGuid()).isNotBlank(); TenantContext.setTenantId(testTenantId); - tokenService.setUser("test@fforesight.com", "secret1234!OH"); + tokenService.setUser("test@fforesight.com", userPassword); var tenant = tenantsClient.getTenant(testTenantId); assertThat(tenant.getGuid()).isNotBlank(); From 791b6b76da04e195f5a91391a34bb4915b9894bc Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Tue, 22 Aug 2023 09:32:14 +0200 Subject: [PATCH 4/8] RED-7292 fixed email is null error --- .../service/TenantManagementService.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index b903ef9..2160602 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -348,11 +348,18 @@ public class TenantManagementService implements TenantProvider { private void validatePasswordWithPolicy(String password, String username, String email) { - if (password.contains(username) || password.contains(email) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { + + if (password.equals(username) || !validateEmailIsNotPassword(email, password) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password does not match password policy."); } } + private boolean validateEmailIsNotPassword(String email, String password) { + if(email == null) { + return true; + } + return !email.equals(password); + } private boolean checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(String str) { From d74dab8fef17e6e50677e1b592032589c5bd7e7b Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Wed, 23 Aug 2023 14:11:04 +0200 Subject: [PATCH 5/8] RED-7292: removed unneded sneaky throws annotation --- src/test/java/com/knecon/fforesight/tests/TenantsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java index 8b6a67c..bbab702 100644 --- a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java +++ b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java @@ -43,7 +43,6 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { @Test - @SneakyThrows public void testCreateNewTenant() { testTenantService.createTestTenantIfNotExists("new_tenant", minioPort); From c0b897112aad0cd6ed49466267256ee5626a4437 Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Thu, 24 Aug 2023 08:17:18 +0200 Subject: [PATCH 6/8] RED-7292: removed unintentionally added line --- .../tenantusermanagement/service/TenantManagementService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index 2160602..3f50796 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -194,7 +194,6 @@ public class TenantManagementService implements TenantProvider { if (!realmReady) { throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to create KC realm"); } - keycloak.getAdminClient().realms(); generalConfigurationService.initGeneralConfiguration(tenantRequest.getTenantId()); keyCloakRoleManagerService.updateRoles(tenantRequest.getTenantId()); From 98c6f271907dccf8e1318e8cc76dcc58a0c2ea56 Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Thu, 24 Aug 2023 09:11:30 +0200 Subject: [PATCH 7/8] RED-7292: added check, if KC password policy exists --- .../service/TenantManagementService.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index 3f50796..6049042 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -85,7 +85,7 @@ public class TenantManagementService implements TenantProvider { @SneakyThrows public TenantResponse createTenant(TenantRequest tenantRequest) { - log.info("Tenants are: {}", tenantRepository.findAll().stream().map(TenantEntity::getTenantId).collect(Collectors.toList())); + log.info("Tenants are: {}", tenantRepository.findAll().stream().map(TenantEntity::getTenantId).toList()); log.info("Requested to create tenant for: {}", tenantRequest.getTenantId()); try { @@ -297,7 +297,7 @@ public class TenantManagementService implements TenantProvider { rolesRepresentation.setRealm(roles); realm.setRoles(rolesRepresentation); - realm.setUsers(users.stream().map(this::toUserRepresentation).collect(Collectors.toList())); + realm.setUsers(users.stream().map(tenantUser -> toUserRepresentation(tenantId, tenantUser)).toList()); realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); @@ -315,11 +315,11 @@ public class TenantManagementService implements TenantProvider { } - private UserRepresentation toUserRepresentation(TenantUser redUser) { + private UserRepresentation toUserRepresentation(String tenantId, TenantUser redUser) { var credentialRepresentation = new CredentialRepresentation(); - credentialRepresentation.setType("password"); - validatePasswordWithPolicy(redUser.getPassword(), redUser.getUsername(), redUser.getEmail()); + credentialRepresentation.setType(CredentialRepresentation.PASSWORD); + validatePasswordWithPolicy(tenantId, redUser.getPassword(), redUser.getUsername(), redUser.getEmail()); credentialRepresentation.setValue(redUser.getPassword()); var user = new UserRepresentation(); @@ -346,10 +346,14 @@ public class TenantManagementService implements TenantProvider { } - private void validatePasswordWithPolicy(String password, String username, String email) { + private void validatePasswordWithPolicy(String tenantId, String password, String username, String email) { - if (password.equals(username) || !validateEmailIsNotPassword(email, password) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password does not match password policy."); + var realmPasswordPolicy = realmService.realm(tenantId).toRepresentation().getPasswordPolicy(); + if(realmPasswordPolicy == null || realmPasswordPolicy.isEmpty()) { + // KC has no policy set, thus we validate it manually + if (password.equals(username) || !validateEmailIsNotPassword(email, password) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password does not match password policy."); + } } } @@ -481,7 +485,7 @@ public class TenantManagementService implements TenantProvider { public List getTenants() { - return tenantRepository.findAll().stream().map(this::convert).collect(Collectors.toList()); + return tenantRepository.findAll().stream().map(this::convert).toList(); } From d7f0899c3b5cc52fc94082217ce9f56e1c21ca0d Mon Sep 17 00:00:00 2001 From: Ali Oezyetimoglu Date: Thu, 24 Aug 2023 13:16:58 +0200 Subject: [PATCH 8/8] RED-7292: removed hard coded part of password policy and extracted set of password policy to after creation and availability of realm; now it is possible for the first users not to match the policy --- .../service/TenantManagementService.java | 58 ++++--------------- .../knecon/fforesight/tests/TenantsTest.java | 15 ++--- .../fforesight/utils/TestTenantService.java | 38 ++---------- 3 files changed, 25 insertions(+), 86 deletions(-) diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java index 6049042..0ecb211 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/service/TenantManagementService.java @@ -10,9 +10,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; import javax.sql.DataSource; @@ -195,6 +192,8 @@ public class TenantManagementService implements TenantProvider { throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to create KC realm"); } + setPasswordPolicyForRealm(tenantRequest.getTenantId()); + generalConfigurationService.initGeneralConfiguration(tenantRequest.getTenantId()); keyCloakRoleManagerService.updateRoles(tenantRequest.getTenantId()); } @@ -297,14 +296,20 @@ public class TenantManagementService implements TenantProvider { rolesRepresentation.setRealm(roles); realm.setRoles(rolesRepresentation); - realm.setUsers(users.stream().map(tenantUser -> toUserRepresentation(tenantId, tenantUser)).toList()); - - realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); + realm.setUsers(users.stream().map(this::toUserRepresentation).toList()); keycloak.getAdminClient().realms().create(realm); } + private void setPasswordPolicyForRealm(String tenantId) { + + var realm = realmService.realm(tenantId).toRepresentation(); + realm.setPasswordPolicy("digits(1) and length(12) and lowerCase(1) and notEmail and notUsername and specialChars(1) and upperCase(1)"); + realmService.realm(tenantId).update(realm); + } + + private boolean tryToAccessRealm(String tenantId) { try { @@ -315,11 +320,10 @@ public class TenantManagementService implements TenantProvider { } - private UserRepresentation toUserRepresentation(String tenantId, TenantUser redUser) { + private UserRepresentation toUserRepresentation(TenantUser redUser) { var credentialRepresentation = new CredentialRepresentation(); credentialRepresentation.setType(CredentialRepresentation.PASSWORD); - validatePasswordWithPolicy(tenantId, redUser.getPassword(), redUser.getUsername(), redUser.getEmail()); credentialRepresentation.setValue(redUser.getPassword()); var user = new UserRepresentation(); @@ -346,44 +350,6 @@ public class TenantManagementService implements TenantProvider { } - private void validatePasswordWithPolicy(String tenantId, String password, String username, String email) { - - var realmPasswordPolicy = realmService.realm(tenantId).toRepresentation().getPasswordPolicy(); - if(realmPasswordPolicy == null || realmPasswordPolicy.isEmpty()) { - // KC has no policy set, thus we validate it manually - if (password.equals(username) || !validateEmailIsNotPassword(email, password) || !checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(password)) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password does not match password policy."); - } - } - } - - private boolean validateEmailIsNotPassword(String email, String password) { - if(email == null) { - return true; - } - return !email.equals(password); - } - - private boolean checkStringContainsUpperCaseAndLowerCaseAndDigitsAndSpecialCharsAndHasLength(String str) { - - if (str.length() >= 12) { - Pattern lowerCaseLetter = Pattern.compile("[a-z]"); - Pattern upperCaseLetter = Pattern.compile("[A-Z]"); - Pattern digit = Pattern.compile("[0-9]"); - Pattern special = Pattern.compile("[^a-zA-Z0-9 ]"); - - Matcher hasLowerCaseLetter = lowerCaseLetter.matcher(str); - Matcher hasUpperCaseLetter = upperCaseLetter.matcher(str); - Matcher hasDigit = digit.matcher(str); - Matcher hasSpecialChar = special.matcher(str); - - return hasLowerCaseLetter.find() && hasUpperCaseLetter.find() && hasDigit.find() && hasSpecialChar.find(); - } - - return false; - } - - @Override @Transactional public void updateDetails(String tenantId, com.knecon.fforesight.tenantcommons.model.UpdateDetailsRequest request) { diff --git a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java index bbab702..f2c3cfc 100644 --- a/src/test/java/com/knecon/fforesight/tests/TenantsTest.java +++ b/src/test/java/com/knecon/fforesight/tests/TenantsTest.java @@ -1,15 +1,11 @@ package com.knecon.fforesight.tests; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.List; import java.util.Map; import java.util.Set; -import javax.ws.rs.BadRequestException; - import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -22,11 +18,10 @@ import com.knecon.fforesight.tenantcommons.model.DatabaseConnection; import com.knecon.fforesight.tenantcommons.model.S3StorageConnection; import com.knecon.fforesight.tenantcommons.model.SearchConnection; import com.knecon.fforesight.tenantusermanagement.model.TenantRequest; -import com.knecon.fforesight.tenantusermanagement.model.TenantUser; +import com.knecon.fforesight.tenantusermanagement.service.RealmService; import com.knecon.fforesight.utils.TestTenantService; import feign.FeignException; -import lombok.SneakyThrows; public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { @@ -39,6 +34,10 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { @Autowired private EncryptionDecryptionService encryptionService; + @Autowired + private RealmService realmService; + + private static final String PASSWORD = "**********"; @@ -237,7 +236,9 @@ public class TenantsTest extends AbstractTenantUserManagementIntegrationTest { assertThat(tenantsClient.getTenant("new_tenant")).isNotNull(); - assertThatThrownBy(() -> testTenantService.createTestTenantIfNotExists("new_tenant_2", minioPort, "passwordDoesNotMatchPolicy")).isInstanceOf(FeignException.class); + var passwordPolicy = realmService.realm("new_tenant").toRepresentation().getPasswordPolicy(); + + assertThat(passwordPolicy).isNotEmpty(); TenantContext.clear(); } diff --git a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java index ef1b956..3f0e171 100644 --- a/src/test/java/com/knecon/fforesight/utils/TestTenantService.java +++ b/src/test/java/com/knecon/fforesight/utils/TestTenantService.java @@ -29,7 +29,6 @@ public class TestTenantService { private final TokenService tokenService; - public void createTestTenantIfNotExists(String testTenantId, int actualPort) { try { @@ -43,9 +42,7 @@ public class TestTenantService { } - public void createTestTenantWithoutStorageIfNotExist(String testTenantId) { - try { var tenantExists = internalTenantsResource.getTenant(testTenantId); assertThat(tenantExists.getGuid()).isNotBlank(); @@ -56,36 +53,14 @@ public class TestTenantService { } } - - public void createTestTenantIfNotExists(String testTenantId, int actualPort, String userPassword) { - - try { - var tenantExists = internalTenantsResource.getTenant(testTenantId); - assertThat(tenantExists.getGuid()).isNotBlank(); - - } catch (Exception e) { - // not found - createUser(testTenantId, actualPort, true, userPassword); - } - - } - - private void createUser(String testTenantId, int actualPort, boolean withStorage) { - - createUser(testTenantId, actualPort, withStorage, "secret1234!OH"); - } - - - private void createUser(String testTenantId, int actualPort, boolean withStorage, String userPassword) { - // not found TenantRequest tenantRequest; var tenantRequestBuilder = TenantRequest.builder() .tenantId(testTenantId) .displayName(testTenantId) .guid(UUID.randomUUID().toString()) - .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password(userPassword).email("test@fforesight.com").build())) + .defaultUsers(List.of(TenantUser.builder().roles(Set.of("SUPER_USER")).username("test@fforesight.com").password("secret").email("test@fforesight.com").build())) .databaseConnection(DatabaseConnection.builder() .driver("postgresql") .host(SpringPostgreSQLTestContainer.getInstance().getHost()) @@ -98,12 +73,9 @@ public class TestTenantService { .searchConnection(SearchConnection.builder().hosts(Set.of("localhost")).port(9200).scheme("http").numberOfShards("1").numberOfReplicas("5").build()); if (withStorage) { - tenantRequest = tenantRequestBuilder.s3StorageConnection(S3StorageConnection.builder() - .key("minioadmin") - .secret("minioadmin") - .bucketName("redaction") - .endpoint("http://localhost:" + actualPort) - .build()).build(); + tenantRequest = tenantRequestBuilder + .s3StorageConnection(S3StorageConnection.builder().key("minioadmin").secret("minioadmin").bucketName("redaction").endpoint("http://localhost:" + actualPort).build()) + .build(); } else { tenantRequest = tenantRequestBuilder.build(); } @@ -112,7 +84,7 @@ public class TestTenantService { assertThat(response.getGuid()).isNotBlank(); TenantContext.setTenantId(testTenantId); - tokenService.setUser("test@fforesight.com", userPassword); + tokenService.setUser("test@fforesight.com", "secret"); var tenant = tenantsClient.getTenant(testTenantId); assertThat(tenant.getGuid()).isNotBlank();