diff --git a/src/main/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociations.java b/src/main/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociations.java index 322cde7..983de85 100644 --- a/src/main/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociations.java +++ b/src/main/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociations.java @@ -267,6 +267,7 @@ public ResponseEntity inviteUser(final String xRequestId, fina @Override public ResponseEntity updateAssociationStatusForId(final String xRequestId, final String associationId, final String requestingUserId, final RequestBodyPut requestBody) { + final var newStatus = requestBody.getStatus(); LOG.infoContext(xRequestId, String.format("Attempting to update association status for association %s to %s.", associationId, newStatus.getValue()), null); @@ -282,20 +283,36 @@ public ResponseEntity updateAssociationStatusForId(final String xRequestId } final AssociationDao associationDao = associationOptional.get(); + + final var oldStatus = associationDao.getStatus(); + + final var timestampKey = newStatus.equals(CONFIRMED) ? "approved_at" : "removed_at"; var update = new Update() .set("status", newStatus.getValue()) .set(timestampKey, LocalDateTime.now().toString()); + final boolean requestingAndTargetUserEmailMatches = Objects.nonNull(associationDao.getUserEmail()) && associationDao.getUserEmail().equals(requestingUserDetails.getEmail()); + final boolean requestingAndTargetUserIdMatches = Objects.nonNull(associationDao.getUserId()) && associationDao.getUserId().equals(requestingUserId); + final boolean requestingAndTargetUserMatches = requestingAndTargetUserIdMatches || requestingAndTargetUserEmailMatches; + final boolean associationWithUserEmailExists = Objects.nonNull(associationDao.getUserEmail()); + + + String targetUserDisplayValue = requestingUserDisplayValue; - boolean requestingAndTargetUserMatches; - String targetUserDisplayValue; - Optional targetUserOptional; - if (Objects.isNull(associationDao.getUserId())) { + if( !requestingAndTargetUserMatches ){ + //if requesting user and target user is different, requesting user is only allowed to remove and should have active association with the company. + if (newStatus.equals(CONFIRMED) || !associationsService.confirmedAssociationExists(associationDao.getCompanyNumber(), requestingUserId)) { + LOG.error(String.format("%s: requesting %s user does not have access to perform the action", xRequestId, requestingUserId)); + throw new BadRequestRuntimeException("requesting user does not have access to perform the action"); + } + } + + if (associationWithUserEmailExists) { var targetUserEmail = associationDao.getUserEmail(); - requestingAndTargetUserMatches = requestingUserDetails.getEmail().equals(targetUserEmail); - targetUserOptional = + + Optional targetUserOptional = Optional.ofNullable(usersService.searchUserDetails(List.of(targetUserEmail))) .flatMap(list -> list.stream().findFirst()); // do not allow user to accept invitation until one login account is created. @@ -312,20 +329,9 @@ public ResponseEntity updateAssociationStatusForId(final String xRequestId } else { targetUserDisplayValue = targetUserEmail; } - } else { - requestingAndTargetUserMatches = requestingUserDetails.getUserId().equals(associationDao.getUserId()); - if (requestingAndTargetUserMatches) { - targetUserDisplayValue=requestingUserDisplayValue; - } else { - //if requesting user and target user is different, requesting user is only allowed to remove and should have active association with the company. - if (newStatus.equals(CONFIRMED) || !associationsService.confirmedAssociationExists(associationDao.getCompanyNumber(), requestingUserId)) { - LOG.error(String.format("%s: requesting %s user does not have access to perform the action", xRequestId, requestingUserId)); - throw new BadRequestRuntimeException("requesting user does not have access to perform the action"); - } - + } else if(!requestingAndTargetUserMatches){ var tempUser = usersService.fetchUserDetails(associationDao.getUserId()); targetUserDisplayValue=Optional.ofNullable(tempUser.getDisplayName()).orElse(tempUser.getEmail()); - } } diff --git a/src/main/java/uk/gov/companieshouse/accounts/association/service/EmailService.java b/src/main/java/uk/gov/companieshouse/accounts/association/service/EmailService.java index 50f0872..076425d 100644 --- a/src/main/java/uk/gov/companieshouse/accounts/association/service/EmailService.java +++ b/src/main/java/uk/gov/companieshouse/accounts/association/service/EmailService.java @@ -111,7 +111,7 @@ public void sendAuthorisationRemovedEmailToRemovedUser(final String xRequestId, LOG.infoContext(xRequestId, new EmailNotification( MessageType.YOUR_AUTHORISATION_REMOVED_MESSAGE_TYPE, - StaticPropertyUtil.APPLICATION_NAMESPACE, + removedByDisplayName, user.getEmail(), companyDetails.getCompanyNumber()).toMessage(), null); } diff --git a/src/test/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociationsTest.java b/src/test/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociationsTest.java index 6df6d51..6ceb997 100644 --- a/src/test/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociationsTest.java +++ b/src/test/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociationsTest.java @@ -1176,7 +1176,9 @@ void updateAssociationStatusForIdNotificationsUsesDisplayNamesWhenAvailable() th .userId("444") .email("robin@gotham.city") .displayName("Robin"); - Mockito.doReturn( targetUser ).when( usersService ).fetchUserDetails( "444" ); + UsersList list = new UsersList(); + list.add(targetUser); + Mockito.doReturn( list ).when( usersService ).searchUserDetails( List.of("robin@gotham.city")); List> requestsToFetchAssociatedUsers = List.of( () -> new User().email( "the.joker@gotham.city" ) ); Mockito.doReturn( requestsToFetchAssociatedUsers ).when( emailService ).createRequestsToFetchAssociatedUsers( "x999999" ); @@ -1190,7 +1192,9 @@ void updateAssociationStatusForIdNotificationsUsesDisplayNamesWhenAvailable() th .content( "{\"status\":\"removed\"}" ) ) .andExpect( status().isOk() ); - Mockito.verify( emailService ).sendAuthorisationRemovedEmailToAssociatedUsers( eq( "theId123" ), eq( companyDetails ), eq( "Harley Quinn" ), eq( "Robin" ), any() ); + Mockito.verify( emailService ).sendAuthorisationRemovedEmailToRemovedUser( eq( "theId123" ), eq( companyDetails ), eq( "Harley Quinn" ), any() ); + + Mockito.verify( emailService ).sendAuthorisationRemovedEmailToAssociatedUsers( eq( "theId123" ), eq( companyDetails ), eq( "Harley Quinn" ),eq("Robin") ,any() ); } @Test @@ -1207,7 +1211,7 @@ void updateAssociationStatusForIdUserAcceptedInvitationNotificationsSendsNotific final var associationFortyTwo = new AssociationDao(); associationFortyTwo.setCompanyNumber("x888888"); associationFortyTwo.setUserId("333"); - associationFortyTwo.setUserEmail("harley.quinn@gotham.city"); + associationFortyTwo.setStatus(StatusEnum.AWAITING_APPROVAL.getValue()); associationFortyTwo.setId("42"); associationFortyTwo.setApprovalRoute(ApprovalRouteEnum.INVITATION.getValue()); @@ -1243,6 +1247,84 @@ void updateAssociationStatusForIdUserAcceptedInvitationNotificationsSendsNotific Mockito.verify( emailService ).sendInvitationAcceptedEmailToAssociatedUsers( eq( "theId123" ), eq( companyDetails ), eq( "the.joker@gotham.city" ), eq( "harley.quinn@gotham.city" ), any() ); } + @Test + void confirmUserStatusForExistingAssociationWithoutOneLoginUserShouldThrow400BadRequest() throws Exception { + final var invitationFourtyTwoA = new InvitationDao(); + invitationFourtyTwoA.setInvitedBy("222"); + invitationFourtyTwoA.setInvitedAt( now.plusDays(16) ); + + final var invitationFourtyTwoB = new InvitationDao(); + invitationFourtyTwoB.setInvitedBy("444"); + invitationFourtyTwoB.setInvitedAt( now.plusDays(14) ); + + final var associationFourtyTwo = new AssociationDao(); + associationFourtyTwo.setCompanyNumber("x888888"); + associationFourtyTwo.setUserEmail("harley.quinn@gotham.city"); + associationFourtyTwo.setStatus(StatusEnum.AWAITING_APPROVAL.getValue()); + associationFourtyTwo.setId("42"); + associationFourtyTwo.setApprovalRoute(ApprovalRouteEnum.INVITATION.getValue()); + associationFourtyTwo.setInvitations( List.of( invitationFourtyTwoA, invitationFourtyTwoB ) ); + + Mockito.doReturn(Optional.of(associationFourtyTwo)).when(associationsService).findAssociationDaoById("42"); + + + final var companyDetails = new CompanyDetails().companyName( "Twitter" ); + Mockito.doReturn( companyDetails ).when( companyService ).fetchCompanyProfile( "x888888" ); + + var result = mockMvc.perform( patch( "/associations/{associationId}", "42" ) + .header("X-Request-Id", "theId123") + .header("Eric-identity", "333") + .header("ERIC-Identity-Type", "oauth2") + .header("ERIC-Authorised-Key-Roles", "*") + .contentType( MediaType.APPLICATION_JSON ) + .content( "{\"status\":\"confirmed\"}" ) ) + .andExpect( status().isBadRequest() ) + .andReturn(); + Assertions.assertEquals("{\"errors\":[{\"error\":\"Could not find data for user harley.quinn@gotham.city\",\"type\":\"ch:service\"}]}", result.getResponse().getContentAsString()); + + } + + @Test + void confirmUserStatusRequestForExistingAssociationWithoutOneLoginUserAndDifferentRequestingUserShouldThrow400BadRequest() throws Exception { + final var invitationFourtyTwoA = new InvitationDao(); + invitationFourtyTwoA.setInvitedBy("222"); + invitationFourtyTwoA.setInvitedAt( now.plusDays(16) ); + + final var invitationFourtyTwoB = new InvitationDao(); + invitationFourtyTwoB.setInvitedBy("444"); + invitationFourtyTwoB.setInvitedAt( now.plusDays(14) ); + + final var associationFourtyTwo = new AssociationDao(); + associationFourtyTwo.setCompanyNumber("x888888"); + associationFourtyTwo.setUserEmail("harley.quinn@gotham.city"); + associationFourtyTwo.setStatus(StatusEnum.AWAITING_APPROVAL.getValue()); + associationFourtyTwo.setId("42"); + associationFourtyTwo.setApprovalRoute(ApprovalRouteEnum.INVITATION.getValue()); + associationFourtyTwo.setInvitations( List.of( invitationFourtyTwoA, invitationFourtyTwoB ) ); + + Mockito.doReturn(Optional.of(associationFourtyTwo)).when(associationsService).findAssociationDaoById("42"); + + final var invitedByUser = new User() + .userId("222") + .email("the.joker@gotham.city"); + Mockito.doReturn( invitedByUser ).when( usersService ).fetchUserDetails( "222" ); + + final var companyDetails = new CompanyDetails().companyName( "Twitter" ); + Mockito.doReturn( companyDetails ).when( companyService ).fetchCompanyProfile( "x888888" ); + + var result = mockMvc.perform( patch( "/associations/{associationId}", "42" ) + .header("X-Request-Id", "theId123") + .header("Eric-identity", "222") + .header("ERIC-Identity-Type", "oauth2") + .header("ERIC-Authorised-Key-Roles", "*") + .contentType( MediaType.APPLICATION_JSON ) + .content( "{\"status\":\"confirmed\"}" ) ) + .andExpect( status().isBadRequest() ) + .andReturn(); + Assertions.assertEquals("{\"errors\":[{\"error\":\"requesting user does not have access to perform the action\",\"type\":\"ch:service\"}]}", result.getResponse().getContentAsString()); + + } + @Test void updateAssociationStatusForIdUserAcceptedInvitationNotificationsUsesDisplayNamesWhenAvailable() throws Exception { final var invitationFourtyTwoA = new InvitationDao(); @@ -1256,7 +1338,7 @@ void updateAssociationStatusForIdUserAcceptedInvitationNotificationsUsesDisplayN final var associationFourtyTwo = new AssociationDao(); associationFourtyTwo.setCompanyNumber("x888888"); associationFourtyTwo.setUserId("333"); - associationFourtyTwo.setUserEmail("harley.quinn@gotham.city"); + associationFourtyTwo.setStatus(StatusEnum.AWAITING_APPROVAL.getValue()); associationFourtyTwo.setId("42"); associationFourtyTwo.setApprovalRoute(ApprovalRouteEnum.INVITATION.getValue()); diff --git a/src/test/java/uk/gov/companieshouse/accounts/association/integration/UserCompanyAssociationsTest.java b/src/test/java/uk/gov/companieshouse/accounts/association/integration/UserCompanyAssociationsTest.java index b3e9b43..a82992d 100644 --- a/src/test/java/uk/gov/companieshouse/accounts/association/integration/UserCompanyAssociationsTest.java +++ b/src/test/java/uk/gov/companieshouse/accounts/association/integration/UserCompanyAssociationsTest.java @@ -9,7 +9,6 @@ import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.internal.verification.Times; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -48,7 +47,6 @@ import uk.gov.companieshouse.email_producer.EmailProducer; import uk.gov.companieshouse.email_producer.EmailSendingException; import uk.gov.companieshouse.email_producer.factory.KafkaProducerFactory; -import uk.gov.companieshouse.email_producer.model.EmailData; import java.time.LocalDateTime; import java.util.List; @@ -185,7 +183,7 @@ public void setup() throws ApiErrorResponseException, URIValidationException { final var associationEighteen = new AssociationDao(); associationEighteen.setCompanyNumber("333333"); associationEighteen.setUserId("9999"); - associationEighteen.setUserEmail("scrooge.mcduck@disney.land"); + //associationEighteen.setUserEmail("scrooge.mcduck@disney.land"); associationEighteen.setStatus(StatusEnum.CONFIRMED.getValue()); associationEighteen.setId("18"); associationEighteen.setApprovedAt(now.plusDays(1)); @@ -638,10 +636,6 @@ public void setup() throws ApiErrorResponseException, URIValidationException { - - - - final var invitationFortySevenOldest = new InvitationDao(); invitationFortySevenOldest.setInvitedBy("666"); invitationFortySevenOldest.setInvitedAt(now.minusDays(9)); @@ -739,9 +733,10 @@ public void setup() throws ApiErrorResponseException, URIValidationException { Mockito.lenient().doReturn( toGetUserDetailsApiResponse( "99999", "scrooge.mcduck1@disney.land", null ) ).when( privateAccountsUserUserGet99999 ).execute(); Mockito.doReturn( toCompanyDetailsApiResponse( "000000", "Boston Dynamics" ) ).when( companyProfileEndpoint ).fetchCompanyProfile( "000000" ); - Mockito.doReturn( toSearchUserDetailsApiResponse( "bruce.wayne@gotham.city", "111" ) ).when( accountsUserEndpoint ).searchUserDetails( eq( List.of( "bruce.wayne@gotham.city" ) ) ); + Mockito.doReturn( toSearchUserDetailsApiResponse( "robin@gotham.city", "444" ) ).when( accountsUserEndpoint ).searchUserDetails( eq( List.of( "robin@gotham.city" ) ) ); + latch = new CountDownLatch(1); doAnswer( invocation -> { latch.countDown(); @@ -753,6 +748,9 @@ public void setup() throws ApiErrorResponseException, URIValidationException { @Test @DirtiesContext( methodMode = MethodMode.BEFORE_METHOD ) void updateAssociationStatusForIdUserAcceptedInvitationNotificationsSendsNotification() throws Exception { + Mockito.lenient().doReturn( toSearchUserDetailsApiResponse( "harley.quinn@gotham.city", "333" ) ).when( accountsUserEndpoint ).searchUserDetails( eq( List.of( "harley.quinn@gotham.city" ) ) ); + + latch = new CountDownLatch(3); doAnswer( invocation -> { latch.countDown(); @@ -1399,41 +1397,6 @@ void updateAssociationStatusForIdWithConfirmedUpdatesAssociationStatus() throws Assertions.assertEquals( oldAssociationData.getUserId(), newAssociationData.getUserId() ); } - @Test - void updateAssociationStatusForIdWithNullUserIdAndExistingUserAndConfirmedUpdatesAssociationStatus() throws Exception { - Mockito.doReturn( toSearchUserDetailsApiResponse( "light.yagami@death.note", "000" ) ).when( accountsUserEndpoint ).searchUserDetails( any() ); - - final var associationZero = new AssociationDao(); - associationZero.setCompanyNumber("000000"); - associationZero.setUserEmail("light.yagami@death.note"); - associationZero.setStatus(StatusEnum.CONFIRMED.getValue()); - associationZero.setId("0"); - associationZero.setApprovedAt(now.plusDays(1)); - associationZero.setRemovedAt(now.plusDays(2)); - associationZero.setApprovalRoute(ApprovalRouteEnum.AUTH_CODE.getValue()); - associationZero.setApprovalExpiryAt(now.plusDays(3)); - associationZero.setInvitations( List.of() ); - associationZero.setEtag( "aa" ); - - associationsRepository.insert( associationZero ); - - mockMvc.perform( patch( "/associations/{associationId}", "0" ) - .header("X-Request-Id", "theId123") - .header("Eric-identity", "000") - .header("ERIC-Identity-Type", "oauth2") - .header("ERIC-Authorised-Key-Roles", "*") - .contentType( MediaType.APPLICATION_JSON ) - .content( "{\"status\":\"confirmed\"}" ) ) - .andExpect( status().isOk() ); - - final var newAssociationData = associationsRepository.findById("0").get(); - Assertions.assertEquals( RequestBodyPut.StatusEnum.CONFIRMED.getValue(), newAssociationData.getStatus() ); - Assertions.assertNotEquals( associationZero.getApprovedAt(), newAssociationData.getApprovedAt() ); - Assertions.assertEquals( localDateTimeToNormalisedString( associationZero.getRemovedAt() ), localDateTimeToNormalisedString( newAssociationData.getRemovedAt() ) ); - Assertions.assertNotEquals( associationZero.getEtag(), newAssociationData.getEtag() ); - Assertions.assertNotEquals( associationZero.getUserEmail(), newAssociationData.getUserEmail() ); - Assertions.assertNotEquals( associationZero.getUserId(), newAssociationData.getUserId() ); - } @Test void updateAssociationStatusForIdWithNullUserIdAndNonexistentUserAndConfirmedReturnsBadRequest() throws Exception { @@ -1467,11 +1430,23 @@ void updateAssociationStatusForIdWithNullUserIdAndNonexistentUserAndConfirmedRet void updateAssociationStatusForIdWithNullUserIdAndExistingUserAndRemovedUpdatesAssociationStatus() throws Exception { Mockito.doReturn( toSearchUserDetailsApiResponse( "light.yagami@death.note", "000" ) ).when( accountsUserEndpoint ).searchUserDetails( any() ); + final var association1 = new AssociationDao(); + association1.setCompanyNumber("000000"); + association1.setUserId("000"); + association1.setStatus(StatusEnum.CONFIRMED.getValue()); + association1.setId("0"); + association1.setApprovedAt(now.plusDays(1)); + association1.setRemovedAt(now.plusDays(2)); + association1.setApprovalRoute(ApprovalRouteEnum.AUTH_CODE.getValue()); + association1.setApprovalExpiryAt(now.plusDays(3)); + association1.setInvitations( List.of() ); + association1.setEtag( "aa" ); + final var associationZero = new AssociationDao(); associationZero.setCompanyNumber("000000"); associationZero.setUserEmail("light.yagami@death.note"); associationZero.setStatus(StatusEnum.CONFIRMED.getValue()); - associationZero.setId("0"); + associationZero.setApprovedAt(now.plusDays(1)); associationZero.setRemovedAt(now.plusDays(2)); associationZero.setApprovalRoute(ApprovalRouteEnum.AUTH_CODE.getValue()); @@ -1480,6 +1455,7 @@ void updateAssociationStatusForIdWithNullUserIdAndExistingUserAndRemovedUpdatesA associationZero.setEtag( "aa" ); associationsRepository.insert( associationZero ); + associationsRepository.insert( association1 ); mockMvc.perform( patch( "/associations/{associationId}", "0" ) .header("X-Request-Id", "theId123") diff --git a/src/test/java/uk/gov/companieshouse/accounts/association/models/email/builders/InviteEmailBuilderTest.java b/src/test/java/uk/gov/companieshouse/accounts/association/models/email/builders/InviteEmailBuilderTest.java index 7a1bb7b..a6a534d 100644 --- a/src/test/java/uk/gov/companieshouse/accounts/association/models/email/builders/InviteEmailBuilderTest.java +++ b/src/test/java/uk/gov/companieshouse/accounts/association/models/email/builders/InviteEmailBuilderTest.java @@ -1,6 +1,7 @@ package uk.gov.companieshouse.accounts.association.models.email.builders; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -17,13 +18,7 @@ class InviteEmailBuilderTest { @Test void buildInstantiatesEmailData() { - final var expectedEmailData = new InviteEmailData(); - expectedEmailData.setTo( "kpatel@companieshouse.gov.uk" ); - expectedEmailData.setSubject( "Companies House: invitation to be authorised to file online for Tesla" ); - expectedEmailData.setInviterDisplayName( "Krishna Patel" ); - expectedEmailData.setCompanyName( "Tesla" ); - expectedEmailData.setInvitationExpiryTimestamp( "1992-05-01T10:30:00.000000" ); - expectedEmailData.setInvitationLink( COMPANY_INVITATIONS_URL ); + final var expectedEmailData = getInviteEmailData(); var actualEmailBuilder = new InviteEmailBuilder(); ReflectionTestUtils.setField(actualEmailBuilder, "invitationLink", COMPANY_INVITATIONS_URL); @@ -38,6 +33,17 @@ void buildInstantiatesEmailData() { Assertions.assertEquals( expectedEmailData, actualEmailData ); } + private @NotNull InviteEmailData getInviteEmailData() { + final var expectedEmailData = new InviteEmailData(); + expectedEmailData.setTo( "kpatel@companieshouse.gov.uk" ); + expectedEmailData.setSubject( "Companies House: invitation to be authorised to file online for Tesla" ); + expectedEmailData.setInviterDisplayName( "Krishna Patel" ); + expectedEmailData.setCompanyName( "Tesla" ); + expectedEmailData.setInvitationExpiryTimestamp( "1992-05-01T10:30:00.000000" ); + expectedEmailData.setInvitationLink( COMPANY_INVITATIONS_URL ); + return expectedEmailData; + } + @Test void buildWithNullsThrowsNullPointerException(){ final var inviteEmailBuilder = new InviteEmailBuilder();