Skip to content

Commit

Permalink
Merge pull request #116 from companieshouse/idva6-834-bugfix
Browse files Browse the repository at this point in the history 
idva6-834
  • Loading branch information
@kkonuganti-ch
kkonuganti-ch authored Jun 17, 2024
2 parents f90c387 + 2200a1a commit 35bf1fe
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 74 deletions.
42 changes: 24 additions & 18 deletions ...n/java/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ public ResponseEntity<ResponseBodyPost> inviteUser(final String xRequestId, fina

@Override
public ResponseEntity<Void> 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);
Expand All @@ -282,20 +283,36 @@ public ResponseEntity<Void> 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<User> 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<User> 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.
Expand All @@ -312,20 +329,9 @@ public ResponseEntity<Void> 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());
}
}


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/uk/gov/companieshouse/accounts/association/service/EmailService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
90 changes: 86 additions & 4 deletions ...va/uk/gov/companieshouse/accounts/association/controller/UserCompanyAssociationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Supplier<User>> requestsToFetchAssociatedUsers = List.of( () -> new User().email( "the.joker@gotham.city" ) );
Mockito.doReturn( requestsToFetchAssociatedUsers ).when( emailService ).createRequestsToFetchAssociatedUsers( "x999999" );
Expand All @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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();
Expand All @@ -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());
Expand Down
64 changes: 20 additions & 44 deletions ...a/uk/gov/companieshouse/accounts/association/integration/UserCompanyAssociationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -638,10 +636,6 @@ public void setup() throws ApiErrorResponseException, URIValidationException {







final var invitationFortySevenOldest = new InvitationDao();
invitationFortySevenOldest.setInvitedBy("666");
invitationFortySevenOldest.setInvitedAt(now.minusDays(9));
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());
Expand All @@ -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")
Expand Down
Loading

0 comments on commit 35bf1fe

Please sign in to comment.