DMP-3819 Update POST /admin/users to disallow duplicate email address (#3040)

Author: karen-hedges

src/integrationTest/java/uk/gov/hmcts/darts/common/repository/UserAccountRepositoryIntTest.java

@@ -0,0 +1,50 @@
+package uk.gov.hmcts.darts.common.repository;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
+import uk.gov.hmcts.darts.authorisation.api.AuthorisationApi;
+import uk.gov.hmcts.darts.authorisation.component.UserIdentity;
+import uk.gov.hmcts.darts.common.entity.UserAccountEntity;
+import uk.gov.hmcts.darts.testutils.PostgresIntegrationBase;
+import uk.gov.hmcts.darts.testutils.stubs.SuperAdminUserStub;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class UserAccountRepositoryIntTest extends PostgresIntegrationBase {
+
+    @Autowired
+    private UserAccountRepository userAccountRepository;
+
+    @Autowired
+    private SuperAdminUserStub superAdminUserStub;
+
+    @MockitoBean
+    private AuthorisationApi authorisationApi;
+
+    @MockitoBean
+    private UserIdentity userIdentity;
+
+    private UserAccountEntity integrationTestUser;
+
+    @BeforeEach
+    void setUp() {
+        integrationTestUser = dartsDatabase.getUserAccountStub().getIntegrationTestUserAccountEntity();
+        Mockito.when(authorisationApi.getCurrentUser()).thenReturn(integrationTestUser);
+    }
+
+    @Test
+    void findByEmailAddressIgnoreCase_shouldReturnExpectedUserAccount_whenItExistsIgnoringCase() {
+        superAdminUserStub.givenUserIsAuthorised(userIdentity);
+        String userEmail = "[email protected]";
+
+        List<UserAccountEntity> foundUsers = userAccountRepository.findByEmailAddressIgnoreCase(userEmail);
+
+        assertEquals(1, foundUsers.size());
+        assertEquals(integrationTestUser.getEmailAddress(), foundUsers.get(0).getEmailAddress());
+    }
+}

src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/PostUserIntTest.java

@@ -13,12 +13,14 @@
 import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.support.TransactionTemplate;
+import uk.gov.hmcts.darts.audio.model.Problem;
 import uk.gov.hmcts.darts.authorisation.api.AuthorisationApi;
 import uk.gov.hmcts.darts.authorisation.component.UserIdentity;
 import uk.gov.hmcts.darts.common.entity.SecurityGroupEntity;
 import uk.gov.hmcts.darts.common.entity.UserAccountEntity;
 import uk.gov.hmcts.darts.testutils.IntegrationBase;
 import uk.gov.hmcts.darts.testutils.stubs.SuperAdminUserStub;
+import uk.gov.hmcts.darts.usermanagement.exception.UserManagementError;
 
 import java.io.UnsupportedEncodingException;
 import java.util.List;
@@ -72,7 +74,7 @@ void setUp() {
     }
 
     @Test
-    void createUserShouldSucceedWhenProvidedWithValidValuesForMinimumRequiredFields() throws Exception {
+    void createUser_ShouldSucceed_WhenProvidedWithValidValuesForMinimumRequiredFields() throws Exception {
         superAdminUserStub.givenUserIsAuthorised(userIdentity);
 
         MockHttpServletRequestBuilder request = buildRequest()
@@ -118,7 +120,7 @@ void createUserShouldSucceedWhenProvidedWithValidValuesForMinimumRequiredFields(
     }
 
     @Test
-    void createUserShouldSucceedWhenProvidedWithValidValuesForAllFields() throws Exception {
+    void createUser_ShouldSucceed_WhenProvidedWithValidValuesForAllFields() throws Exception {
         superAdminUserStub.givenUserIsAuthorised(userIdentity);
 
         MockHttpServletRequestBuilder request = buildRequest()
@@ -174,7 +176,7 @@ void createUserShouldSucceedWhenProvidedWithValidValuesForAllFields() throws Exc
     }
 
     @Test
-    void createUserShouldFailWhenRequiredFieldsAreMissing() throws Exception {
+    void createUser_ShouldFail_WhenRequiredFieldsAreMissing() throws Exception {
         superAdminUserStub.givenUserIsAuthorised(userIdentity);
 
         MockHttpServletRequestBuilder request = buildRequest()
@@ -193,7 +195,7 @@ void createUserShouldFailWhenRequiredFieldsAreMissing() throws Exception {
     }
 
     @Test
-    void createUserShouldSucceedWhenProvidedWithAUserEmailAddressThatMatchesAnExistingDisabledAccount() throws Exception {
+    void createUser_ShouldFail_WhenProvidedWithAUserEmailAddressThatMatchesAnExistingDisabledAccount() throws Exception {
         superAdminUserStub.givenUserIsAuthorised(userIdentity);
 
         UserAccountEntity userAccountEntity = new UserAccountEntity();
@@ -211,12 +213,18 @@ void createUserShouldSucceedWhenProvidedWithAUserEmailAddressThatMatchesAnExisti
                            "email_address": "[email protected]"
                          }
                          """);
-        mockMvc.perform(request)
-            .andExpect(status().isCreated());
+        MvcResult mvcResult = mockMvc.perform(request)
+            .andExpect(status().isConflict()).andReturn();
+        String actualJson = mvcResult.getResponse().getContentAsString();
+
+        Problem problem = objectMapper.readValue(actualJson, Problem.class);
+        assertEquals(UserManagementError.DUPLICATE_EMAIL.getType(), problem.getType().toString());
+        assertEquals(UserManagementError.DUPLICATE_EMAIL.getTitle(), problem.getTitle());
+        assertEquals("User with email [email protected] already exists", problem.getDetail());
     }
 
     @Test
-    void createUserShouldFailWhenProvidedWithASecurityGroupThatDoesntExist() throws Exception {
+    void createUser_ShouldFail_WhenProvidedWithASecurityGroupThatDoesntExist() throws Exception {
         superAdminUserStub.givenUserIsAuthorised(userIdentity);
 
         MockHttpServletRequestBuilder request = buildRequest()

src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/UserControllerTest.java renamed to src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/UserControllerIntTest.java

@@ -43,7 +43,7 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 @AutoConfigureMockMvc
-class UserControllerTest extends IntegrationBase {
+class UserControllerIntTest extends IntegrationBase {
 
     private static final String ENDPOINT_URL = "/admin/users/";
 
@@ -325,8 +325,7 @@ void testActivateModifyUserWithSuperAdminAndFailWithNoFullNameAndNoEmailAddress(
         // add user to the super admin group
         Optional<SecurityGroupEntity> groupEntity
             = securityGroupRepository.findByGroupNameIgnoreCase(SecurityGroupEnum.SUPER_ADMIN.getName());
-
-
+        
         userAccountEntity.getSecurityGroupEntities().add(groupEntity.get());
         userAccountEntity = dartsDatabaseStub.save(userAccountEntity);
 

src/main/java/uk/gov/hmcts/darts/common/repository/UserAccountRepository.java

@@ -62,8 +62,6 @@ AND securityRole.id IN (:roleIds)
         """)
     Optional<UserAccountEntity> findByRoleAndUserId(Integer securityRoleId, Integer userId);
 
-    List<UserAccountEntity> findByEmailAddressIgnoreCaseAndActive(String emailAddress, Boolean active);
-
     List<UserAccountEntity> findByEmailAddressIgnoreCase(String emailAddress);
 
     @Modifying

src/main/java/uk/gov/hmcts/darts/usermanagement/service/impl/UserManagementServiceImpl.java

@@ -194,7 +194,7 @@ private List<Long> updateEntity(UserPatch userPatch, UserAccountEntity userAccou
         if (active != null) {
             userAccountEntity.setActive(active);
 
-            // if we are disabling the use then disable the transcriptions
+            // if we are disabling the user then disable the transcriptions
             // and remove user from security groups
             if (active.equals(Boolean.FALSE)) {
                 unassignUserFromGroupsTheyArePartOf(userAccountEntity);

src/main/java/uk/gov/hmcts/darts/usermanagement/service/validation/UserEmailValidator.java

@@ -24,7 +24,7 @@ public void validate(User user) {
                 String.format("Invalid email format {%s}", user.getEmailAddress()));
         }
 
-        userAccountRepository.findByEmailAddressIgnoreCaseAndActive(user.getEmailAddress(), true)
+        userAccountRepository.findByEmailAddressIgnoreCase(user.getEmailAddress())
             .stream().findFirst()
             .ifPresent(existingUser -> {
                 throw new DartsApiException(

src/test/java/uk/gov/hmcts/darts/usermanagement/service/impl/UserEmailValidatorTest.java

@@ -3,6 +3,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import uk.gov.hmcts.darts.common.entity.UserAccountEntity;
@@ -36,19 +38,22 @@ void setUp() {
     }
 
     @Test
-    void doesNotThrowExceptionIfEmailNotCurrentlyAssociatedWithActiveUser() {
-        when(userAccountRepository.findByEmailAddressIgnoreCaseAndActive(NEW_EMAIL_ADDRESS, true))
+    void validate_doesNotThrowException_whenEmailNotCurrentlyAssociatedWithAnyUser() {
+        when(userAccountRepository.findByEmailAddressIgnoreCase(NEW_EMAIL_ADDRESS))
             .thenReturn(Collections.emptyList());
 
         userEmailValidator.validate(someUserWithEmail(NEW_EMAIL_ADDRESS));
 
         assertThatNoException().isThrownBy(() -> userEmailValidator.validate(someUserWithEmail(NEW_EMAIL_ADDRESS)));
     }
 
-    @Test
-    void throwsExceptionIfEmailAlreadyAssociatedWithActiveUser() {
-        when(userAccountRepository.findByEmailAddressIgnoreCaseAndActive(EXISTING_EMAIL_ADDRESS, true))
-            .thenReturn(List.of(someUserAccountWithEmail(EXISTING_EMAIL_ADDRESS)));
+    @ParameterizedTest
+    @ValueSource(strings = {"true", "false"})
+    void validate_throwsException_whenEmailAlreadyAssociatedWithExistingUser(boolean isActive) {
+        UserAccountEntity existingUser = someUserAccountWithEmail(EXISTING_EMAIL_ADDRESS);
+        existingUser.setActive(isActive);
+        when(userAccountRepository.findByEmailAddressIgnoreCase(EXISTING_EMAIL_ADDRESS))
+            .thenReturn(List.of(existingUser));
 
         User user = someUserWithEmail(EXISTING_EMAIL_ADDRESS);
         assertThatThrownBy(() -> userEmailValidator.validate(user))
@@ -58,7 +63,7 @@ void throwsExceptionIfEmailAlreadyAssociatedWithActiveUser() {
     }
 
     @Test
-    void throwsExceptionIfEmailAddressHasNoDomain() {
+    void validate_throwsException_whenEmailAddressHasNoDomain() {
 
         User user = someUserWithEmail(EMAIL_ADDRESS_NO_DOMAIN);
         assertThatThrownBy(() -> userEmailValidator.validate(user))
@@ -68,7 +73,7 @@ void throwsExceptionIfEmailAddressHasNoDomain() {
     }
 
     @Test
-    void throwsExceptionIfEmailAddressHasNoUser() {
+    void validate_throwsException_whenEmailAddressHasNoUser() {
 
         User user = someUserWithEmail(EMAIL_ADDRESS_NO_USER);
         assertThatThrownBy(() -> userEmailValidator.validate(user))