From 20771cb0e244d99183113efd055a2582ed49e5ff Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Mon, 25 Nov 2024 17:04:52 +0000 Subject: [PATCH 01/30] View Audit Logs --- .../management/controllers/AuditTest.java | 4 ++- .../controllers/AuditController.java | 18 ++++++++-- .../management/database/AuditRepository.java | 10 +++++- .../management/service/AuditService.java | 35 +++++++++++++++++-- .../controllers/AuditControllerTest.java | 12 +++++-- .../management/service/AuditServiceTest.java | 15 ++++++-- 6 files changed, 83 insertions(+), 11 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java index eae6f253..426d9dc9 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java @@ -52,6 +52,8 @@ class AuditTest { private static final String UNAUTHORIZED_USERNAME = "unauthorized_isAuthorized"; private static final String FORBIDDEN_STATUS_CODE = "Status code does not match forbidden"; private static final String GET_AUDIT_LOG_FAILED = "Failed to retrieve audit log"; + private static final String GET_ALL_AUDIT_PARAMS = + "?filterStartDate=2020-01-01T00:00:00.000Z&filterEndDate=2100-01-01T00:00:00.000Z"; private static final AuditAction AUDIT_ACTION = AuditAction.MANAGE_THIRD_PARTY_USER_VIEW; private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @@ -94,7 +96,7 @@ void testGetAllAuditLogs() throws Exception { .contentType(MediaType.APPLICATION_JSON); mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); - MvcResult mvcResult = mockMvc.perform(get(ROOT_URL)) + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + GET_ALL_AUDIT_PARAMS)) .andExpect(status().isOk()) .andReturn(); diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java index 236c6fe6..d2ab0c1d 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java @@ -9,6 +9,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; +import org.springframework.format.annotation.DateTimeFormat; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -21,7 +22,10 @@ import uk.gov.hmcts.reform.pip.account.management.model.AuditLog; import uk.gov.hmcts.reform.pip.account.management.service.AuditService; import uk.gov.hmcts.reform.pip.model.authentication.roles.IsAdmin; +import uk.gov.hmcts.reform.pip.model.enums.AuditAction; +import java.time.LocalDateTime; +import java.util.List; import java.util.UUID; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; @@ -45,14 +49,22 @@ public AuditController(AuditService auditService) { this.auditService = auditService; } - @ApiResponse(responseCode = OK_ERROR_CODE, description = "All audit logs returned as a page.") + @ApiResponse(responseCode = OK_ERROR_CODE, description = "All audit logs returned as a page with filtering.") @Operation(summary = "Get all audit logs returned as a page") @GetMapping public ResponseEntity> getAllAuditLogs( @RequestParam(name = "pageNumber", defaultValue = "0") int pageNumber, - @RequestParam(name = "pageSize", defaultValue = "25") int pageSize) { + @RequestParam(name = "pageSize", defaultValue = "25") int pageSize, + @RequestParam(name = "email", defaultValue = "", required = false) String email, + @RequestParam(name = "userId", defaultValue = "", required = false) String userId, + @RequestParam(name = "actions", defaultValue = "", required = false) List auditActions, + @RequestParam(name = "filterStartDate", defaultValue = "") + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime filterStartDate, + @RequestParam(name = "filterEndDate", defaultValue = "") + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime filterEndDate) { Pageable pageable = PageRequest.of(pageNumber, pageSize); - return ResponseEntity.ok(auditService.getAllAuditLogs(pageable)); + return ResponseEntity.ok(auditService.getAllAuditLogs(pageable, email, userId, + auditActions, filterStartDate, filterEndDate)); } @ApiResponse(responseCode = OK_ERROR_CODE, description = "Audit log with id {id} returned.") diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java index b1cb9d9a..a9e1bc41 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java @@ -5,8 +5,10 @@ import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.transaction.annotation.Transactional; import uk.gov.hmcts.reform.pip.account.management.model.AuditLog; +import uk.gov.hmcts.reform.pip.model.enums.AuditAction; import java.time.LocalDateTime; +import java.util.List; import java.util.UUID; public interface AuditRepository extends JpaRepository { @@ -14,5 +16,11 @@ public interface AuditRepository extends JpaRepository { @Transactional void deleteAllByTimestampBefore(LocalDateTime timestamp); - Page findAllByOrderByTimestampDesc(Pageable pageable); + Page findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( + String email, + String userId, + List auditAction, + LocalDateTime timeStampFrom, + LocalDateTime timeStampTo, + Pageable pageable); } diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java index b9b37eee..6ef4a296 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java @@ -8,8 +8,12 @@ import uk.gov.hmcts.reform.pip.account.management.database.AuditRepository; import uk.gov.hmcts.reform.pip.account.management.errorhandling.exceptions.NotFoundException; import uk.gov.hmcts.reform.pip.account.management.model.AuditLog; +import uk.gov.hmcts.reform.pip.model.enums.AuditAction; import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; import java.util.UUID; /** @@ -33,8 +37,35 @@ public AuditService(AuditRepository auditRepository) { * @param pageable The pageable object to query by. * @return Returns the audit logs in a page. */ - public Page getAllAuditLogs(Pageable pageable) { - return auditRepository.findAllByOrderByTimestampDesc(pageable); + public Page getAllAuditLogs(Pageable pageable, String email, String userId, + List auditActions, LocalDateTime filterStartDate, LocalDateTime filterEndDate) { + + // If email address is supplied then find by an exact match + String userEmailAddressToQuery = "%%"; + if (!email.isBlank()) { + userEmailAddressToQuery = email; + } + + // If user provenance id is supplied then find by an exact match + String userIdToQuery = "%%"; + if (!userId.isBlank()) { + userIdToQuery = userId; + } + + // If user audit action is supplied then find by an exact match + List auditActionsToQuery = new ArrayList<>(EnumSet.allOf(AuditAction.class)); + if (!auditActions.isEmpty()) { + auditActionsToQuery = auditActions; + } + + return auditRepository + .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( + userEmailAddressToQuery, + userIdToQuery, + auditActionsToQuery, + filterStartDate, + filterEndDate, + pageable); } public AuditLog getAuditLogById(UUID id) { diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java index bc1eb5ad..157d78ae 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java @@ -14,6 +14,9 @@ import uk.gov.hmcts.reform.pip.model.account.UserProvenances; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -29,11 +32,16 @@ class AuditControllerTest { private AuditController auditController; private static final String STATUS_CODE_MATCH = "Status code responses should match"; + private static final String EMAIL = "a@b.com"; + private static final String USER_ID = "123"; + private static final List AUDIT_ACTIONS = new ArrayList<>(); + private static final LocalDateTime FILTER_START_DATE = LocalDateTime.now(); + private static final LocalDateTime FILTER_END_DATE = LocalDateTime.now(); @Test void testGetAllAuditLogs() { - ResponseEntity> response = auditController.getAllAuditLogs(0, 25); - + ResponseEntity> response = auditController.getAllAuditLogs(0, 25, + EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE); assertEquals(HttpStatus.OK, response.getStatusCode(), STATUS_CODE_MATCH); } diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java index 906e4687..3255d7dc 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java @@ -16,6 +16,7 @@ import uk.gov.hmcts.reform.pip.model.enums.AuditAction; import java.time.LocalDateTime; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -38,6 +39,11 @@ class AuditServiceTest { @InjectMocks private AuditService auditService; + private static final String EMAIL = "a@b.com"; + private static final String USER_ID = "123"; + private static final List AUDIT_ACTIONS = new ArrayList<>(); + private static final LocalDateTime FILTER_START_DATE = LocalDateTime.now(); + private static final LocalDateTime FILTER_END_DATE = LocalDateTime.now(); private final AuditLog auditLogExample = new AuditLog(); @BeforeEach @@ -48,15 +54,20 @@ void setup() { auditLogExample.setAction(AuditAction.MANAGE_USER); auditLogExample.setDetails("Test details for manage user"); auditLogExample.setTimestamp(LocalDateTime.now()); + + AUDIT_ACTIONS.add(AuditAction.ADMIN_CREATION); } @Test void testGetAllAuditLogs() { Pageable pageable = PageRequest.of(0, 25); Page page = new PageImpl<>(List.of(auditLogExample), pageable, List.of(auditLogExample).size()); - when(auditRepository.findAllByOrderByTimestampDesc(pageable)).thenReturn(page); + when(auditRepository + .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( + EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE, pageable)).thenReturn(page); - Page returnedAuditLogs = auditService.getAllAuditLogs(pageable); + Page returnedAuditLogs = auditService.getAllAuditLogs(pageable, EMAIL, USER_ID, AUDIT_ACTIONS, + FILTER_START_DATE, FILTER_END_DATE); assertEquals(auditLogExample, returnedAuditLogs.getContent().get(0), "Returned audit log does not match the expected"); From 2045fc86075fe6e6b9821920283fe1ef34f70786 Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Wed, 27 Nov 2024 09:43:42 +0000 Subject: [PATCH 02/30] fix code review comment --- .../pip/account/management/service/AuditService.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java index 6ef4a296..0daf5c97 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java @@ -40,12 +40,6 @@ public AuditService(AuditRepository auditRepository) { public Page getAllAuditLogs(Pageable pageable, String email, String userId, List auditActions, LocalDateTime filterStartDate, LocalDateTime filterEndDate) { - // If email address is supplied then find by an exact match - String userEmailAddressToQuery = "%%"; - if (!email.isBlank()) { - userEmailAddressToQuery = email; - } - // If user provenance id is supplied then find by an exact match String userIdToQuery = "%%"; if (!userId.isBlank()) { @@ -60,7 +54,7 @@ public Page getAllAuditLogs(Pageable pageable, String email, String us return auditRepository .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( - userEmailAddressToQuery, + "%" + email + "%", userIdToQuery, auditActionsToQuery, filterStartDate, From 8b1f4825408571e06af76f0b12b45cbe4d66e5c0 Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Wed, 27 Nov 2024 09:57:58 +0000 Subject: [PATCH 03/30] fix test --- .../pip/account/management/service/AuditServiceTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java index 3255d7dc..366c5f08 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java @@ -64,7 +64,8 @@ void testGetAllAuditLogs() { Page page = new PageImpl<>(List.of(auditLogExample), pageable, List.of(auditLogExample).size()); when(auditRepository .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( - EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE, pageable)).thenReturn(page); + "%" + EMAIL + "%", USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE, pageable)) + .thenReturn(page); Page returnedAuditLogs = auditService.getAllAuditLogs(pageable, EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE); From a0c97394964a22b0c916437232de1028bef6c180 Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Thu, 28 Nov 2024 10:51:46 +0000 Subject: [PATCH 04/30] Make filter date param as optional --- .../management/controllers/AuditTest.java | 4 +-- .../controllers/AuditController.java | 9 ++---- .../management/database/AuditRepository.java | 6 ++++ .../management/service/AuditService.java | 30 +++++++++++++---- .../controllers/AuditControllerTest.java | 6 ++-- .../management/service/AuditServiceTest.java | 32 ++++++++++++++++--- 6 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java index 426d9dc9..eae6f253 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java @@ -52,8 +52,6 @@ class AuditTest { private static final String UNAUTHORIZED_USERNAME = "unauthorized_isAuthorized"; private static final String FORBIDDEN_STATUS_CODE = "Status code does not match forbidden"; private static final String GET_AUDIT_LOG_FAILED = "Failed to retrieve audit log"; - private static final String GET_ALL_AUDIT_PARAMS = - "?filterStartDate=2020-01-01T00:00:00.000Z&filterEndDate=2100-01-01T00:00:00.000Z"; private static final AuditAction AUDIT_ACTION = AuditAction.MANAGE_THIRD_PARTY_USER_VIEW; private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @@ -96,7 +94,7 @@ void testGetAllAuditLogs() throws Exception { .contentType(MediaType.APPLICATION_JSON); mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); - MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + GET_ALL_AUDIT_PARAMS)) + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL)) .andExpect(status().isOk()) .andReturn(); diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java index d2ab0c1d..2750e959 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditController.java @@ -9,7 +9,6 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; -import org.springframework.format.annotation.DateTimeFormat; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -24,7 +23,6 @@ import uk.gov.hmcts.reform.pip.model.authentication.roles.IsAdmin; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; -import java.time.LocalDateTime; import java.util.List; import java.util.UUID; @@ -58,13 +56,10 @@ public ResponseEntity> getAllAuditLogs( @RequestParam(name = "email", defaultValue = "", required = false) String email, @RequestParam(name = "userId", defaultValue = "", required = false) String userId, @RequestParam(name = "actions", defaultValue = "", required = false) List auditActions, - @RequestParam(name = "filterStartDate", defaultValue = "") - @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime filterStartDate, - @RequestParam(name = "filterEndDate", defaultValue = "") - @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime filterEndDate) { + @RequestParam(name = "filterDate", defaultValue = "", required = false) String filterDate) { Pageable pageable = PageRequest.of(pageNumber, pageSize); return ResponseEntity.ok(auditService.getAllAuditLogs(pageable, email, userId, - auditActions, filterStartDate, filterEndDate)); + auditActions, filterDate)); } @ApiResponse(responseCode = OK_ERROR_CODE, description = "Audit log with id {id} returned.") diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java index a9e1bc41..69ba59ac 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/AuditRepository.java @@ -16,6 +16,12 @@ public interface AuditRepository extends JpaRepository { @Transactional void deleteAllByTimestampBefore(LocalDateTime timestamp); + Page findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInOrderByTimestampDesc( + String email, + String userId, + List auditAction, + Pageable pageable); + Page findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( String email, String userId, diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java index 0daf5c97..364b98a8 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java @@ -10,7 +10,10 @@ import uk.gov.hmcts.reform.pip.account.management.model.AuditLog; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; +import java.time.LocalDate; import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; @@ -38,7 +41,7 @@ public AuditService(AuditRepository auditRepository) { * @return Returns the audit logs in a page. */ public Page getAllAuditLogs(Pageable pageable, String email, String userId, - List auditActions, LocalDateTime filterStartDate, LocalDateTime filterEndDate) { + List auditActions, String filterDate) { // If user provenance id is supplied then find by an exact match String userIdToQuery = "%%"; @@ -52,14 +55,27 @@ public Page getAllAuditLogs(Pageable pageable, String email, String us auditActionsToQuery = auditActions; } + if (filterDate.isBlank()) { + return auditRepository + .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInOrderByTimestampDesc( + "%" + email + "%", + userIdToQuery, + auditActionsToQuery, + pageable); + } + + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDateTime filterStartDate = LocalDate.parse(filterDate, formatter).atTime(LocalTime.MIN); + LocalDateTime filterEndDate = LocalDate.parse(filterDate, formatter).atTime(LocalTime.MAX); + return auditRepository .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( - "%" + email + "%", - userIdToQuery, - auditActionsToQuery, - filterStartDate, - filterEndDate, - pageable); + "%" + email + "%", + userIdToQuery, + auditActionsToQuery, + filterStartDate, + filterEndDate, + pageable); } public AuditLog getAuditLogById(UUID id) { diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java index 157d78ae..ba34841b 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditControllerTest.java @@ -14,7 +14,6 @@ import uk.gov.hmcts.reform.pip.model.account.UserProvenances; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; -import java.time.LocalDateTime; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -35,13 +34,12 @@ class AuditControllerTest { private static final String EMAIL = "a@b.com"; private static final String USER_ID = "123"; private static final List AUDIT_ACTIONS = new ArrayList<>(); - private static final LocalDateTime FILTER_START_DATE = LocalDateTime.now(); - private static final LocalDateTime FILTER_END_DATE = LocalDateTime.now(); + private static final String FILTER_DATE = "2023-11-01"; @Test void testGetAllAuditLogs() { ResponseEntity> response = auditController.getAllAuditLogs(0, 25, - EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE); + EMAIL, USER_ID, AUDIT_ACTIONS, FILTER_DATE); assertEquals(HttpStatus.OK, response.getStatusCode(), STATUS_CODE_MATCH); } diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java index 366c5f08..da267176 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuditServiceTest.java @@ -15,7 +15,10 @@ import uk.gov.hmcts.reform.pip.account.management.model.AuditLog; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; +import java.time.LocalDate; import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -42,8 +45,7 @@ class AuditServiceTest { private static final String EMAIL = "a@b.com"; private static final String USER_ID = "123"; private static final List AUDIT_ACTIONS = new ArrayList<>(); - private static final LocalDateTime FILTER_START_DATE = LocalDateTime.now(); - private static final LocalDateTime FILTER_END_DATE = LocalDateTime.now(); + private static final String FILTER_DATE = "2024-11-01"; private final AuditLog auditLogExample = new AuditLog(); @BeforeEach @@ -63,12 +65,32 @@ void testGetAllAuditLogs() { Pageable pageable = PageRequest.of(0, 25); Page page = new PageImpl<>(List.of(auditLogExample), pageable, List.of(auditLogExample).size()); when(auditRepository - .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( - "%" + EMAIL + "%", USER_ID, AUDIT_ACTIONS, FILTER_START_DATE, FILTER_END_DATE, pageable)) + .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInOrderByTimestampDesc( + "%" + EMAIL + "%", USER_ID, AUDIT_ACTIONS, pageable)) .thenReturn(page); Page returnedAuditLogs = auditService.getAllAuditLogs(pageable, EMAIL, USER_ID, AUDIT_ACTIONS, - FILTER_START_DATE, FILTER_END_DATE); + ""); + + assertEquals(auditLogExample, returnedAuditLogs.getContent().get(0), + "Returned audit log does not match the expected"); + } + + @Test + void testGetAllAuditLogsWithFilterDate() { + Pageable pageable = PageRequest.of(0, 25); + Page page = new PageImpl<>(List.of(auditLogExample), pageable, List.of(auditLogExample).size()); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDateTime filterStartDate = LocalDate.parse(FILTER_DATE, formatter).atTime(LocalTime.MIN); + LocalDateTime filterEndDate = LocalDate.parse(FILTER_DATE, formatter).atTime(LocalTime.MAX); + + when(auditRepository + .findAllByUserEmailLikeIgnoreCaseAndUserIdLikeAndActionInAndTimestampBetweenOrderByTimestampDesc( + "%" + EMAIL + "%", USER_ID, AUDIT_ACTIONS, filterStartDate, filterEndDate, pageable)) + .thenReturn(page); + + Page returnedAuditLogs = auditService.getAllAuditLogs(pageable, EMAIL, USER_ID, AUDIT_ACTIONS, + FILTER_DATE); assertEquals(auditLogExample, returnedAuditLogs.getContent().get(0), "Returned audit log does not match the expected"); From d1d87eaf9574db14844f8ea540c79324d1f1dd01 Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Thu, 28 Nov 2024 13:31:48 +0000 Subject: [PATCH 05/30] fix typo --- .../reform/pip/account/management/service/AuditService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java index 364b98a8..be70fd6a 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuditService.java @@ -43,7 +43,7 @@ public AuditService(AuditRepository auditRepository) { public Page getAllAuditLogs(Pageable pageable, String email, String userId, List auditActions, String filterDate) { - // If user provenance id is supplied then find by an exact match + // If user id is supplied then find by an exact match String userIdToQuery = "%%"; if (!userId.isBlank()) { userIdToQuery = userId; From be2cac9ff3aacf8898c42ae40bd4bab0d10f3480 Mon Sep 17 00:00:00 2001 From: junaidiqbalmoj Date: Tue, 3 Dec 2024 12:21:08 +0000 Subject: [PATCH 06/30] add integration tests --- .../management/controllers/AuditTest.java | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java index eae6f253..0455a583 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AuditTest.java @@ -24,6 +24,9 @@ import uk.gov.hmcts.reform.pip.model.account.UserProvenances; import uk.gov.hmcts.reform.pip.model.enums.AuditAction; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.springframework.http.HttpStatus.FORBIDDEN; @@ -43,6 +46,7 @@ class AuditTest { private static final String ROOT_URL = "/audit"; private static final String EMAIL = "test_account_admin@hmcts.net"; + private static final String ADDITIONAL_USER_EMAIL = "test_account_admin_2@hmcts.net"; private static final Roles ROLES = Roles.SYSTEM_ADMIN; private static final UserProvenances USER_PROVENANCE = UserProvenances.PI_AAD; private static final String AUDIT_DETAILS = "User requested to view all third party users"; @@ -53,6 +57,8 @@ class AuditTest { private static final String FORBIDDEN_STATUS_CODE = "Status code does not match forbidden"; private static final String GET_AUDIT_LOG_FAILED = "Failed to retrieve audit log"; private static final AuditAction AUDIT_ACTION = AuditAction.MANAGE_THIRD_PARTY_USER_VIEW; + private static final AuditAction ADDITIONAL_USER_AUDIT_ACTION = AuditAction.MANAGE_USER; + private static final String ADDITIONAL_USER_AUDIT_DETAILS = "Manage user"; private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @@ -117,6 +123,264 @@ void testGetAllAuditLogs() throws Exception { assertEquals(AUDIT_DETAILS, auditLog2.getDetails(), GET_AUDIT_LOG_FAILED); } + @Test + void testGetAllAuditLogsFilterByEmail() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + ADDITIONAL_USER_EMAIL, + ROLES, + USER_PROVENANCE, + AUDIT_ACTION, + AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?email=" + EMAIL)) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + AuditLog auditLog1 = pageResponse.getContent().get(0); + + assertEquals(EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + } + + @Test + void testGetAllAuditLogsPartialEmailFilter() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + EMAIL, + ROLES, + USER_PROVENANCE, + AUDIT_ACTION, + AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?email=test_account_admin")) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + + AuditLog auditLog1 = pageResponse.getContent().get(0); + + assertEquals(EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + + AuditLog auditLog2 = pageResponse.getContent().get(1); + + assertEquals(EMAIL, auditLog2.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(USER_ID, auditLog2.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(AUDIT_DETAILS, auditLog2.getDetails(), GET_AUDIT_LOG_FAILED); + } + + @Test + void testGetAllAuditLogsFilterByUserId() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + ADDITIONAL_USER_EMAIL, + ROLES, + USER_PROVENANCE, + AUDIT_ACTION, + AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?userId=" + ADDITIONAL_USER_ID)) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + AuditLog auditLog1 = pageResponse.getContent().get(0); + + assertEquals(ADDITIONAL_USER_EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + } + + @Test + void testGetAllAuditLogsFilterByAuditAction() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + ADDITIONAL_USER_EMAIL, + ROLES, + USER_PROVENANCE, + ADDITIONAL_USER_AUDIT_ACTION, + ADDITIONAL_USER_AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?actions=" + ADDITIONAL_USER_AUDIT_ACTION)) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + AuditLog auditLog1 = pageResponse.getContent().get(0); + + + assertEquals(pageResponse.getContent().size(), 1, GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + } + + @Test + void testGetAllAuditLogsFilterByDate() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + ADDITIONAL_USER_EMAIL, + ROLES, + USER_PROVENANCE, + ADDITIONAL_USER_AUDIT_ACTION, + ADDITIONAL_USER_AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDate filterDate = LocalDate.parse(LocalDate.now().toString(), formatter); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?filterDate=" + filterDate)) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + + AuditLog auditLog1 = pageResponse.getContent().get(0); + + assertEquals(ADDITIONAL_USER_EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + + AuditLog auditLog2 = pageResponse.getContent().get(1); + + assertEquals(EMAIL, auditLog2.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(USER_ID, auditLog2.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(AUDIT_DETAILS, auditLog2.getDetails(), GET_AUDIT_LOG_FAILED); + } + + @Test + void testGetAllAuditLogsFilterByEmailAndUserIdAndAuditActionAndDate() throws Exception { + MockHttpServletRequestBuilder mockHttpServletRequestBuilder1 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(createAuditLog())) + .contentType(MediaType.APPLICATION_JSON); + + mockMvc.perform(mockHttpServletRequestBuilder1).andExpect(status().isOk()); + + MockHttpServletRequestBuilder mockHttpServletRequestBuilder2 = MockMvcRequestBuilders + .post(ROOT_URL) + .content(OBJECT_MAPPER.writeValueAsString(new AuditLog( + ADDITIONAL_USER_ID, + ADDITIONAL_USER_EMAIL, + ROLES, + USER_PROVENANCE, + ADDITIONAL_USER_AUDIT_ACTION, + ADDITIONAL_USER_AUDIT_DETAILS + ))) + .contentType(MediaType.APPLICATION_JSON); + mockMvc.perform(mockHttpServletRequestBuilder2).andExpect(status().isOk()); + + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + LocalDate filterDate = LocalDate.parse(LocalDate.now().toString(), formatter); + + MvcResult mvcResult = mockMvc.perform(get(ROOT_URL + "?email=" + ADDITIONAL_USER_EMAIL + + "&userId=" + ADDITIONAL_USER_ID + "&actions=" + ADDITIONAL_USER_AUDIT_ACTION + + "&filterDate=" + filterDate)) + .andExpect(status().isOk()) + .andReturn(); + + CustomPageImpl pageResponse = + OBJECT_MAPPER.readValue( + mvcResult.getResponse().getContentAsString(), + new TypeReference<>() { + } + ); + AuditLog auditLog1 = pageResponse.getContent().get(0); + + assertEquals(ADDITIONAL_USER_EMAIL, auditLog1.getUserEmail(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_ID, auditLog1.getUserId(), GET_AUDIT_LOG_FAILED); + assertEquals(ADDITIONAL_USER_AUDIT_DETAILS, auditLog1.getDetails(), GET_AUDIT_LOG_FAILED); + } + @Test @WithMockUser(username = UNAUTHORIZED_USERNAME, authorities = {UNAUTHORIZED_ROLE}) void testUnauthorizedGetAllAuditLogs() throws Exception { From 6bbed03e56db2565e429f6db16b00fe871ee0e19 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Wed, 4 Dec 2024 09:19:30 +0000 Subject: [PATCH 07/30] PUB-2681 - Updated System Admin --- build.gradle | 2 +- .../SystemAdminB2CAccountController.java | 2 + .../service/AuthorisationService.java | 13 +++++ .../service/SystemAdminB2CAccountService.java | 51 ++++++------------- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/build.gradle b/build.gradle index fb9ba4b0..dfcfc6b1 100644 --- a/build.gradle +++ b/build.gradle @@ -221,7 +221,7 @@ dependencies { implementation group: 'com.opencsv', name: 'opencsv', version: '5.9' implementation group: 'commons-validator', name: 'commons-validator', version: '1.9.0' - implementation group: 'com.github.hmcts', name: 'pip-data-models', version: '2.1.32', { + implementation group: 'com.github.hmcts', name: 'pip-data-models', version: '2.1.34', { exclude group: 'org.springframework.boot', module: 'spring-boot-starter-data-jpa' } implementation group: 'io.hypersistence', name: 'hypersistence-utils-hibernate-63', version: '3.8.3' diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java index 5af1b895..dfdea391 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java @@ -5,6 +5,7 @@ import io.swagger.v3.oas.annotations.tags.Tag; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -48,6 +49,7 @@ public SystemAdminB2CAccountController(SystemAdminB2CAccountService systemAdminB @ApiResponse(responseCode = OK_CODE, description = PI_USER) @ApiResponse(responseCode = BAD_REQUEST_CODE, description = "{ErroredSystemAdminAccount}") @PostMapping("/add/system-admin") + @PreAuthorize("@authorisationService.userCanCreateSystemAdmin(#issuerId)") public ResponseEntity createSystemAdminAccount(//NOSONAR @RequestHeader(ISSUER_ID) String issuerId, @RequestBody SystemAdminAccount account) { return ResponseEntity.ok(systemAdminB2CAccountService.addSystemAdminAccount(account, issuerId)); diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationService.java index 0907d9f2..4026ee82 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationService.java @@ -10,6 +10,7 @@ import uk.gov.hmcts.reform.pip.model.account.UserProvenances; import java.util.List; +import java.util.Optional; import java.util.UUID; import static uk.gov.hmcts.reform.pip.model.LogBuilder.writeLog; @@ -70,6 +71,18 @@ public boolean userCanUpdateAccount(UUID userId, UUID adminUserId) { return isAuthorised; } + public boolean userCanCreateSystemAdmin(UUID userId) { + Optional adminUser = userRepository.findByUserId(userId); + boolean isSystemAdmin = adminUser.isPresent() && adminUser.get().getRoles().equals(Roles.SYSTEM_ADMIN); + + if (!isSystemAdmin) { + log.error(writeLog( + String.format("User with ID %s is forbidden to create a B2C system admin", userId) + )); + } + return isSystemAdmin; + } + private boolean isAuthorisedRole(UUID userId, UUID adminUserId) { PiUser user = getUser(userId); if (UserProvenances.SSO.equals(user.getUserProvenance())) { diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java index 7961ada8..61eccda9 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java @@ -38,20 +38,20 @@ public class SystemAdminB2CAccountService { private final AzureUserService azureUserService; private final UserRepository userRepository; private final PublicationService publicationService; - private final AzureAccountService azureAccountService; + private final AccountService accountService; private final Integer maxSystemAdminValue; @Autowired public SystemAdminB2CAccountService(Validator validator, AzureUserService azureUserService, UserRepository userRepository, PublicationService publicationService, @Value("${admin.max-system-admin}")Integer maxSystemAdminValue, - AzureAccountService azureAccountService) { + AccountService accountService) { this.validator = validator; this.azureUserService = azureUserService; this.userRepository = userRepository; this.publicationService = publicationService; this.maxSystemAdminValue = maxSystemAdminValue; - this.azureAccountService = azureAccountService; + this.accountService = accountService; } /** @@ -61,18 +61,12 @@ public SystemAdminB2CAccountService(Validator validator, AzureUserService azureU * @return The PiUser of the created system admin account. */ public PiUser addSystemAdminAccount(SystemAdminAccount account, String issuerId) { - - String displayName = ""; - String provenanceUserId = verifyAdminUser(issuerId); - if (!provenanceUserId.isEmpty()) { - displayName = azureAccountService.retrieveAzureAccount(provenanceUserId).getDisplayName(); - } - - validateSystemAdminAccount(account, issuerId, displayName); + PiUser piUser = accountService.getUserById(UUID.fromString(issuerId)); + validateSystemAdminAccount(account, issuerId, piUser.getEmail()); try { User user = azureUserService.createUser(account.convertToAzureAccount(), false); PiUser createdUser = userRepository.save(account.convertToPiUser(user.getId())); - handleNewSystemAdminAccountAction(account, issuerId, ActionResult.SUCCEEDED, displayName); + handleNewSystemAdminAccountAction(account, issuerId, ActionResult.SUCCEEDED, piUser.getEmail()); publicationService.sendNotificationEmail( account.getEmail(), @@ -83,19 +77,20 @@ public PiUser addSystemAdminAccount(SystemAdminAccount account, String issuerId) } catch (AzureCustomException e) { ErroredSystemAdminAccount erroredSystemAdminAccount = new ErroredSystemAdminAccount(account); erroredSystemAdminAccount.setErrorMessages(List.of(e.getLocalizedMessage())); - handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, displayName); + handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, piUser.getEmail()); throw new SystemAdminAccountException(erroredSystemAdminAccount); } + } /** * This method handles the logging and publishing that a new system admin account has been created. * @param systemAdminAccount The system admin account that has been created * @param adminId The ID of the admin user who is creating the account. - * @param name The name of the admin user who is creating the account + * @param email The email of the admin user who is creating the account */ public void handleNewSystemAdminAccountAction(SystemAdminAccount systemAdminAccount, String adminId, - ActionResult result, String name) { + ActionResult result, String email) { log.info(writeLog(UUID.fromString(adminId), "has attempted to create a System Admin account, which has: " + result.toString())); @@ -105,7 +100,7 @@ public void handleNewSystemAdminAccountAction(SystemAdminAccount systemAdminAcco CreateSystemAdminAction createSystemAdminAction = new CreateSystemAdminAction(); createSystemAdminAction.setAccountEmail(systemAdminAccount.getEmail()); createSystemAdminAction.setEmailList(existingAdminEmails); - createSystemAdminAction.setRequesterName(name); + createSystemAdminAction.setRequesterEmail(email); createSystemAdminAction.setActionResult(result); publicationService.sendSystemAdminAccountAction(createSystemAdminAction); @@ -115,9 +110,9 @@ public void handleNewSystemAdminAccountAction(SystemAdminAccount systemAdminAcco * A helper method which specifically handles validation failures on the system admin account. * @param account The system admin account to validate. * @param issuerId The ID of the admin user that is issuing the account. - * @param name The name of the admin user requesting the account. + * @param email The email of the admin user requesting the account. */ - private void validateSystemAdminAccount(SystemAdminAccount account, String issuerId, String name) { + private void validateSystemAdminAccount(SystemAdminAccount account, String issuerId, String email) { Set> constraintViolationSet = validator.validate(account); if (!constraintViolationSet.isEmpty()) { @@ -126,14 +121,14 @@ private void validateSystemAdminAccount(SystemAdminAccount account, String issue .stream().map(constraint -> constraint.getPropertyPath() + ": " + constraint.getMessage()).toList()); - handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, name); + handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, email); throw new SystemAdminAccountException(erroredSystemAdminAccount); } if (userRepository.findByEmailAndUserProvenance(account.getEmail(), UserProvenances.PI_AAD).isPresent()) { ErroredSystemAdminAccount erroredSystemAdminAccount = new ErroredSystemAdminAccount(account); erroredSystemAdminAccount.setDuplicate(true); - handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, name); + handleNewSystemAdminAccountAction(account, issuerId, ActionResult.FAILED, email); throw new SystemAdminAccountException(erroredSystemAdminAccount); } @@ -144,22 +139,8 @@ private void validateSystemAdminAccount(SystemAdminAccount account, String issue if (systemAdminUsers.size() >= maxSystemAdminValue) { ErroredSystemAdminAccount erroredSystemAdminAccount = new ErroredSystemAdminAccount(account); erroredSystemAdminAccount.setAboveMaxSystemAdmin(true); - handleNewSystemAdminAccountAction(account, issuerId, ActionResult.ATTEMPTED, name); + handleNewSystemAdminAccountAction(account, issuerId, ActionResult.ATTEMPTED, email); throw new SystemAdminAccountException(erroredSystemAdminAccount); } } - - /** - * Method to find whether user is SYSTEM_ADMIN or not. - * @param issuerId The ID of the admin user - * @return Boolean user is SYSTEM_ADMIN or not - */ - private String verifyAdminUser(String issuerId) { - Optional adminUser = userRepository.findByUserId(UUID.fromString(issuerId)); - if (adminUser.isPresent() && adminUser.get().getRoles().equals(Roles.SYSTEM_ADMIN)) { - return adminUser.get().getProvenanceUserId(); - } - - return ""; - } } From aecbd63aef03df69a04235d70eaa2bb895870ef2 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:41:41 +0000 Subject: [PATCH 08/30] PUB-2681 - Add unit and integration tests --- .../SystemAdminB2CAccountTest.java | 23 ++++ .../resources/add-admin-users.sql | 3 +- .../service/SystemAdminB2CAccountService.java | 1 - .../service/AuthorisationServiceTest.java | 42 +++++++ .../SystemAdminB2CAccountServiceTest.java | 107 ++++++++---------- 5 files changed, 115 insertions(+), 61 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java index 21089fb0..30f83303 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java @@ -54,6 +54,7 @@ class SystemAdminB2CAccountTest { private static final String ISSUER_ID = "1234-1234-1234-1234"; private static final String SYSTEM_ADMIN_ISSUER_ID = "87f907d2-eb28-42cc-b6e1-ae2b03f7bba2"; + private static final String SUPER_ADMIN_ISSUER_ID = "87f907d2-eb28-42cc-b6e1-ae2b03f7bba3"; private static final String ISSUER_HEADER = "x-issuer-id"; private static final String GIVEN_NAME = "Given Name"; private static final String ID = "1234"; @@ -232,4 +233,26 @@ void testUnauthorizedCreateSystemAdminAccount() throws Exception { FORBIDDEN_STATUS_CODE ); } + + @Test + void testCreateSystemAdminUserWhenNotSystemAdmin() throws Exception { + SystemAdminAccount systemAdmin = new SystemAdminAccount(); + systemAdmin.setFirstName(TEST_SYS_ADMIN_FIRSTNAME); + systemAdmin.setSurname(TEST_SYS_ADMIN_SURNAME); + systemAdmin.setEmail(TEST_SYS_ADMIN_EMAIL); + + MockHttpServletRequestBuilder createRequest = + MockMvcRequestBuilders + .post(CREATE_SYSTEM_ADMIN_URL) + .content(OBJECT_MAPPER.writeValueAsString(systemAdmin)) + .header(ISSUER_HEADER, SUPER_ADMIN_ISSUER_ID) + .contentType(MediaType.APPLICATION_JSON); + + MvcResult responseCreateSystemAdminUser = mockMvc.perform(createRequest) + .andExpect(status().isForbidden()).andReturn(); + + assertEquals(FORBIDDEN.value(), responseCreateSystemAdminUser.getResponse().getStatus(), + FORBIDDEN_STATUS_CODE + ); + } } diff --git a/src/integrationTest/resources/add-admin-users.sql b/src/integrationTest/resources/add-admin-users.sql index d978c131..4a548321 100644 --- a/src/integrationTest/resources/add-admin-users.sql +++ b/src/integrationTest/resources/add-admin-users.sql @@ -1,5 +1,4 @@ INSERT INTO pi_user (user_id, email, provenance_user_id,user_provenance,roles,forenames,surname) VALUES -('87f907d2-eb28-42cc-b6e1-ae2b03f7bba2', 'SyestemAdmin@justice.gov.uk', 'e5f1cc77-6e9a-40ab-8da0-a9666b328464','PI_AAD','SYSTEM_ADMIN','System','Admin'), +('87f907d2-eb28-42cc-b6e1-ae2b03f7bba2', 'SystemAdmin@justice.gov.uk', 'e5f1cc77-6e9a-40ab-8da0-a9666b328464','PI_AAD','SYSTEM_ADMIN','System','Admin'), ('87f907d2-eb28-42cc-b6e1-ae2b03f7bba3', 'SuperAdminCtsc@justice.gov.uk', 'e5f1cc77-6e9a-40ab-8da0-a9666b328465','PI_AAD','INTERNAL_SUPER_ADMIN_CTSC','Super','Admin'), ('87f907d2-eb28-42cc-b6e1-ae2b03f7bba4', 'SyestemAdminSso@justice.gov.uk', 'e5f1cc77-6e9a-40ab-8da0-a9666b328466','SSO','SYSTEM_ADMIN','System','Admin'); - diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java index 61eccda9..ff5e8303 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountService.java @@ -19,7 +19,6 @@ import uk.gov.hmcts.reform.pip.model.system.admin.CreateSystemAdminAction; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.UUID; diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationServiceTest.java index abbd93e8..41f25e43 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AuthorisationServiceTest.java @@ -20,6 +20,7 @@ import java.util.Optional; import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.verifyNoInteractions; @@ -667,4 +668,45 @@ void testExceptionThrowIfAdminUserNotFound() { .isInstanceOf(NotFoundException.class) .hasMessage(String.format("User with supplied user id: %s could not be found", ADMIN_USER_ID)); } + + @Test + void testUserCanCreateSystemAdmin() { + UUID userId = UUID.randomUUID(); + PiUser user = new PiUser(); + user.setRoles(Roles.SYSTEM_ADMIN); + + when(userRepository.findByUserId(userId)).thenReturn(Optional.of(user)); + + assertThat(authorisationService.userCanCreateSystemAdmin(userId)).isTrue(); + } + + @Test + void testUserCannotCreateSystemAdminIfAccountNotFound() { + UUID userId = UUID.randomUUID(); + + when(userRepository.findByUserId(userId)).thenReturn(Optional.empty()); + + try (LogCaptor logCaptor = LogCaptor.forClass(AuthorisationService.class)) { + assertThat(authorisationService.userCanCreateSystemAdmin(userId)).isFalse(); + + assertThat(logCaptor.getErrorLogs().get(0)).contains( + String.format("User with ID %s is forbidden to create a B2C system admin", userId)); + } + } + + @Test + void testUserCannotCreateSystemAdminIfUserIsNotSystemAdmin() { + UUID userId = UUID.randomUUID(); + PiUser user = new PiUser(); + user.setRoles(Roles.INTERNAL_ADMIN_LOCAL); + + when(userRepository.findByUserId(userId)).thenReturn(Optional.of(user)); + + try (LogCaptor logCaptor = LogCaptor.forClass(AuthorisationService.class)) { + assertThat(authorisationService.userCanCreateSystemAdmin(userId)).isFalse(); + + assertThat(logCaptor.getErrorLogs().get(0)).contains( + String.format("User with ID %s is forbidden to create a B2C system admin", userId)); + } + } } diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountServiceTest.java index 38bbf390..8cdec77e 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/SystemAdminB2CAccountServiceTest.java @@ -46,7 +46,7 @@ class SystemAdminB2CAccountServiceTest { private PublicationService publicationService; @Mock - private AzureAccountService azureAccountService; + private AccountService accountService; @Mock private UserRepository userRepository; @@ -62,7 +62,7 @@ class SystemAdminB2CAccountServiceTest { private SystemAdminB2CAccountService systemAdminAccountService; - private static final String ID = UUID.randomUUID().toString(); + private static final UUID ID = UUID.randomUUID(); private static final String EMAIL = "test@email.com"; private static final String FORENAME = "Test"; private static final String SURNAME = "Surname"; @@ -74,16 +74,17 @@ class SystemAdminB2CAccountServiceTest { private final User expectedUser = new User(); private final PiUser expectedPiUser = new PiUser(); private final PiUser ssoUser = new PiUser(); + private final PiUser systemAdminUser = new PiUser(); @BeforeEach void setup() { expectedUser.setGivenName(FORENAME); - expectedUser.setId(ID); + expectedUser.setId(ID.toString()); expectedUser.setSurname(SURNAME); expectedPiUser.setUserId(UUID.randomUUID()); expectedPiUser.setEmail(EMAIL); - expectedPiUser.setProvenanceUserId(ID); + expectedPiUser.setProvenanceUserId(ID.toString()); expectedPiUser.setRoles(Roles.SYSTEM_ADMIN); expectedPiUser.setUserProvenance(UserProvenances.PI_AAD); @@ -93,9 +94,13 @@ void setup() { ssoUser.setRoles(Roles.SYSTEM_ADMIN); ssoUser.setUserProvenance(UserProvenances.SSO); + systemAdminUser.setRoles(Roles.SYSTEM_ADMIN); + systemAdminUser.setUserId(ID); + systemAdminUser.setEmail(EMAIL); + systemAdminAccountService = new SystemAdminB2CAccountService(validator, azureUserService, userRepository, publicationService, 4, - azureAccountService); + accountService); } @@ -107,14 +112,16 @@ void testAddSystemAdminAccount() throws AzureCustomException { when(azureUserService.createUser(argThat(user -> EMAIL.equals(user.getEmail())), anyBoolean())) .thenReturn(expectedUser); when(userRepository.save(any())).thenReturn(expectedPiUser); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); when(publicationService.sendNotificationEmail(EMAIL, FORENAME, SURNAME)).thenReturn(Boolean.TRUE); - when(userRepository.findByUserId(any())).thenReturn(Optional.ofNullable(expectedPiUser)); + + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> EMAIL.equals(arg.getRequesterEmail()))); + + when(accountService.getUserById(ID)).thenReturn(systemAdminUser); when(validator.validate(SYSTEM_ADMIN_ACCOUNT)).thenReturn(Set.of()); when(userRepository.findByRoles(Roles.SYSTEM_ADMIN)).thenReturn(List.of(expectedPiUser)); - PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID); + PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString()); assertEquals(expectedPiUser, returnedUser, USER_MESSAGE); } @@ -124,16 +131,13 @@ void testAddSystemAdminAccountThrowsException() throws AzureCustomException { AzureAccount azUser = new AzureAccount(); azUser.setDisplayName(FORENAME); - when(userRepository.findByUserId(any())) - .thenReturn(Optional.ofNullable(expectedPiUser)); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); + when(accountService.getUserById(ID)).thenReturn(systemAdminUser); when(azureUserService.createUser(argThat(user -> EMAIL.equals(user.getEmail())), anyBoolean())) .thenThrow(new AzureCustomException("Test error")); SystemAdminAccountException systemAdminAccountException = assertThrows(SystemAdminAccountException.class, () -> - systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID)); + systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString())); assertEquals("Test error", @@ -146,41 +150,22 @@ void testConstraintViolationException() { AzureAccount azUser = new AzureAccount(); azUser.setDisplayName(FORENAME); - when(userRepository.findByUserId(any())) - .thenReturn(Optional.ofNullable(expectedPiUser)); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); + when(accountService.getUserById(ID)).thenReturn(systemAdminUser); when(validator.validate(any())).thenReturn(Set.of(constraintViolation)); when(constraintViolation.getMessage()).thenReturn("This is a message"); when(constraintViolation.getPropertyPath()).thenReturn(path); + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> EMAIL.equals(arg.getRequesterEmail()))); + SystemAdminAccountException systemAdminAccountException = assertThrows(SystemAdminAccountException.class, () -> - systemAdminAccountService.addSystemAdminAccount(ERRORED_SYSTEM_ADMIN_ACCOUNT, ID)); + systemAdminAccountService.addSystemAdminAccount(ERRORED_SYSTEM_ADMIN_ACCOUNT, ID.toString())); assertNotEquals(0, systemAdminAccountException.getErroredSystemAdminAccount().getErrorMessages().size(), "Constraint violation error messages not displayed"); } - @Test - void testAddSystemAdminAccountNotVerified() throws AzureCustomException { - AzureAccount azUser = new AzureAccount(); - azUser.setDisplayName(FORENAME); - - expectedPiUser.setRoles(Roles.VERIFIED); - when(azureUserService.createUser(argThat(user -> EMAIL.equals(user.getEmail())), anyBoolean())) - .thenReturn(expectedUser); - when(userRepository.save(any())).thenReturn(expectedPiUser); - when(publicationService.sendNotificationEmail(EMAIL, FORENAME, SURNAME)).thenReturn(Boolean.FALSE); - when(userRepository.findByUserId(any())).thenReturn(Optional.ofNullable(expectedPiUser)); - when(validator.validate(SYSTEM_ADMIN_ACCOUNT)).thenReturn(Set.of()); - when(userRepository.findByRoles(Roles.SYSTEM_ADMIN)).thenReturn(List.of(expectedPiUser)); - - PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID); - - assertEquals(expectedPiUser, returnedUser, USER_MESSAGE); - } - @Test void testAddSystemAdminAccountNotExists() throws AzureCustomException { AzureAccount azUser = new AzureAccount(); @@ -191,11 +176,13 @@ void testAddSystemAdminAccountNotExists() throws AzureCustomException { .thenReturn(expectedUser); when(userRepository.save(any())).thenReturn(expectedPiUser); when(publicationService.sendNotificationEmail(EMAIL, FORENAME, SURNAME)).thenReturn(Boolean.FALSE); - when(userRepository.findByUserId(any())).thenReturn(Optional.empty()); + when(accountService.getUserById(ID)).thenReturn(new PiUser()); when(validator.validate(SYSTEM_ADMIN_ACCOUNT)).thenReturn(Set.of()); when(userRepository.findByRoles(Roles.SYSTEM_ADMIN)).thenReturn(List.of(expectedPiUser)); + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> arg.getRequesterEmail() == null)); - PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID); + PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString()); assertEquals(expectedPiUser, returnedUser, USER_MESSAGE); } @@ -206,14 +193,14 @@ void testUserAlreadyExists() { azUser.setDisplayName(FORENAME); when(userRepository.findByEmailAndUserProvenance(EMAIL, UserProvenances.PI_AAD)) .thenReturn(Optional.of(expectedPiUser)); - when(userRepository.findByUserId(any())) - .thenReturn(Optional.ofNullable(expectedPiUser)); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> EMAIL.equals(arg.getRequesterEmail()))); + when(accountService.getUserById(ID)) + .thenReturn(systemAdminUser); SystemAdminAccountException systemAdminAccountException = assertThrows(SystemAdminAccountException.class, () -> - systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID)); + systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString())); assertTrue(systemAdminAccountException.getErroredSystemAdminAccount().isDuplicate(), "Duplicate account flag " + "not set"); @@ -225,16 +212,17 @@ void testAboveMaxAllowsUsersWithAllAadUsers() { azUser.setDisplayName(FORENAME); when(userRepository.findByEmailAndUserProvenance(EMAIL, UserProvenances.PI_AAD)) .thenReturn(Optional.empty()); - when(userRepository.findByUserId(any())) - .thenReturn(Optional.ofNullable(expectedPiUser)); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); + when(accountService.getUserById(ID)) + .thenReturn(systemAdminUser); when(userRepository.findByRoles(Roles.SYSTEM_ADMIN)).thenReturn(List.of(expectedPiUser, expectedPiUser, expectedPiUser, expectedPiUser)); + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> EMAIL.equals(arg.getRequesterEmail()))); + SystemAdminAccountException systemAdminAccountException = assertThrows(SystemAdminAccountException.class, () -> - systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID)); + systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString())); assertTrue(systemAdminAccountException.getErroredSystemAdminAccount().isAboveMaxSystemAdmin(), "Max system " + "admin flag not set"); @@ -246,17 +234,18 @@ void testAboveMaxAllowsUsersNotIncludingSsoUser() throws AzureCustomException { azUser.setDisplayName(FORENAME); when(userRepository.findByEmailAndUserProvenance(EMAIL, UserProvenances.PI_AAD)) .thenReturn(Optional.empty()); - when(userRepository.findByUserId(any())) - .thenReturn(Optional.ofNullable(expectedPiUser)); - when(azureAccountService.retrieveAzureAccount(any())) - .thenReturn(azUser); + when(accountService.getUserById(ID)) + .thenReturn(systemAdminUser); when(userRepository.findByRoles(Roles.SYSTEM_ADMIN)).thenReturn(List.of(expectedPiUser, expectedPiUser, expectedPiUser, ssoUser)); + doNothing().when(publicationService) + .sendSystemAdminAccountAction(argThat(arg -> EMAIL.equals(arg.getRequesterEmail()))); + when(azureUserService.createUser(argThat(user -> EMAIL.equals(user.getEmail())), anyBoolean())) .thenReturn(expectedUser); when(userRepository.save(any())).thenReturn(expectedPiUser); - PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID); + PiUser returnedUser = systemAdminAccountService.addSystemAdminAccount(SYSTEM_ADMIN_ACCOUNT, ID.toString()); assertEquals(expectedPiUser, returnedUser, USER_MESSAGE); } @@ -270,13 +259,15 @@ void testHandleNewSystemAdminAccountAction() { doNothing().when(publicationService).sendSystemAdminAccountAction(systemAdminAccountArgumentCaptor.capture()); - systemAdminAccountService.handleNewSystemAdminAccountAction(SYSTEM_ADMIN_ACCOUNT, ID, ActionResult.ATTEMPTED, - FORENAME); + systemAdminAccountService.handleNewSystemAdminAccountAction(SYSTEM_ADMIN_ACCOUNT, + ID.toString(), + ActionResult.ATTEMPTED, + EMAIL); CreateSystemAdminAction createSystemAdminAction = systemAdminAccountArgumentCaptor.getValue(); assertEquals(EMAIL, createSystemAdminAction.getAccountEmail(), "Unknown email retrieved"); - assertEquals(FORENAME, createSystemAdminAction.getRequesterName(), "Unknown requester name retrieved"); + assertEquals(EMAIL, createSystemAdminAction.getRequesterEmail(), "Unknown requester name retrieved"); assertEquals(List.of(EMAIL, EMAIL), createSystemAdminAction.getEmailList(), "Unknown email list retrieved"); assertEquals(ActionResult.ATTEMPTED, createSystemAdminAction.getActionResult(), "Action result not as expected"); From 180b67b2a7e958c0be12b5858429c92703ec4b16 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:40:27 +0000 Subject: [PATCH 09/30] PUB-2681 - Updated integration tests --- .../reform/pip/account/management/controllers/AccountTest.java | 2 ++ .../management/controllers/SystemAdminB2CAccountTest.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java index f6913d0e..701ab8ea 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java @@ -610,6 +610,7 @@ void testDeleteAccountNotFound() throws Exception { } @Test + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") void testV2SystemAdminDeletesVerifiedUser() throws Exception { validUser.setUserProvenance(UserProvenances.CFT_IDAM); validUser.setRoles(Roles.VERIFIED); @@ -675,6 +676,7 @@ void testV2SystemAdminDeletesThirdPartyUser() throws Exception { } @Test + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") void testV2SystemAdminDeletesSuperAdminUser() throws Exception { superAdminUser.setUserProvenance(UserProvenances.CFT_IDAM); String superAdminUserId = getSuperAdminUserId(superAdminUser); diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java index 30f83303..f347f36f 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountTest.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; @@ -223,7 +224,7 @@ void testUnauthorizedCreateSystemAdminAccount() throws Exception { MockMvcRequestBuilders .post(CREATE_SYSTEM_ADMIN_URL) .content(OBJECT_MAPPER.writeValueAsString(systemAdmin)) - .header(ISSUER_HEADER, ISSUER_ID) + .header(ISSUER_HEADER, UUID.randomUUID().toString()) .contentType(MediaType.APPLICATION_JSON); MvcResult responseCreateSystemAdminUser = mockMvc.perform(createRequest) From e6ee71b5179d798e00191548d88bbd1cc6314300 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Mon, 9 Dec 2024 16:42:42 +0000 Subject: [PATCH 10/30] Replace constructor with lombok annotation --- .../controllers/BulkAccountCreationController.java | 8 ++------ .../controllers/SystemAdminAccountController.java | 8 ++------ .../controllers/SystemAdminB2CAccountController.java | 8 ++------ 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountCreationController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountCreationController.java index 1318d2f3..3c9e0bf0 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountCreationController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountCreationController.java @@ -4,7 +4,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; -import org.springframework.beans.factory.annotation.Autowired; +import lombok.AllArgsConstructor; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.PostMapping; @@ -27,6 +27,7 @@ @ApiResponse(responseCode = "403", description = "User has not been authorized") @Validated @IsAdmin +@AllArgsConstructor @SecurityRequirement(name = "bearerAuth") public class BulkAccountCreationController { private static final String ISSUER_ID = "x-issuer-id"; @@ -34,11 +35,6 @@ public class BulkAccountCreationController { private final BulkAccountCreationService bulkAccountCreationService; - @Autowired - public BulkAccountCreationController(BulkAccountCreationService bulkAccountCreationService) { - this.bulkAccountCreationService = bulkAccountCreationService; - } - @ApiResponse(responseCode = OK_CODE, description = "CREATED_ACCOUNTS:[{Created user ids}], ERRORED_ACCOUNTS: [{failed accounts}]") @ApiResponse(responseCode = "400", description = "Bad request") diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountController.java index 1d2f3807..93384f84 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountController.java @@ -3,7 +3,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; -import org.springframework.beans.factory.annotation.Autowired; +import lombok.AllArgsConstructor; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.PostMapping; @@ -20,17 +20,13 @@ @RequestMapping("/account") @Validated @IsAdmin +@AllArgsConstructor @SecurityRequirement(name = "bearerAuth") public class SystemAdminAccountController { private static final String PI_USER = "{piUser}"; private final SystemAdminAccountService systemAdminAccountService; - @Autowired - public SystemAdminAccountController(SystemAdminAccountService systemAdminAccountService) { - this.systemAdminAccountService = systemAdminAccountService; - } - /** * Create a system admin account for SSO user on the user table. * diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java index 5af1b895..42e60e97 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountController.java @@ -3,7 +3,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; -import org.springframework.beans.factory.annotation.Autowired; +import lombok.AllArgsConstructor; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.PostMapping; @@ -23,6 +23,7 @@ @ApiResponse(responseCode = "403", description = "User has not been authorized") @Validated @IsAdmin +@AllArgsConstructor @SecurityRequirement(name = "bearerAuth") public class SystemAdminB2CAccountController { private static final String ISSUER_ID = "x-issuer-id"; @@ -32,11 +33,6 @@ public class SystemAdminB2CAccountController { private final SystemAdminB2CAccountService systemAdminB2CAccountService; - @Autowired - public SystemAdminB2CAccountController(SystemAdminB2CAccountService systemAdminB2CAccountService) { - this.systemAdminB2CAccountService = systemAdminB2CAccountService; - } - /** * POST endpoint that deals with creating a new System Admin Account (including PI and Azure) * This will also trigger any welcome emails. From d0d7854b0d3e221a86a61579c4e4d11d5ca13168 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Mon, 9 Dec 2024 18:01:37 +0000 Subject: [PATCH 11/30] Add functional test for creating a system admin account endpoint --- .../SystemAdminAccountCreationTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java new file mode 100644 index 00000000..12589757 --- /dev/null +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -0,0 +1,56 @@ +package uk.gov.hmcts.reform.pip.account.management.controllers; + +import com.github.dockerjava.zerodep.shaded.org.apache.hc.core5.http.HttpHeaders; +import io.restassured.response.Response; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import uk.gov.hmcts.reform.pip.account.management.utils.FunctionalTestBase; +import uk.gov.hmcts.reform.pip.account.management.utils.OAuthClient; + +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +@ExtendWith(SpringExtension.class) +@ActiveProfiles(profiles = "functional") +@SpringBootTest(classes = {OAuthClient.class}) +class SystemAdminAccountCreationTest extends FunctionalTestBase { + private static final String TEST_EMAIL_PREFIX = String.format( + "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_EMAIL = String.format(TEST_EMAIL_PREFIX, "@justice.gov.uk"); + + private static final String ACCOUNT_URL = "/account"; + private static final String SYSTEM_ADMIN_URL = ACCOUNT_URL + "/system-admin"; + private static final String TEST_PROVENANCE_ID = UUID.randomUUID().toString(); + private static final String BEARER = "Bearer "; + + private Map bearer; + + @BeforeAll + public void startUp() { + bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); + } + + @Test + public void createSystemAdminAccount() { + String requestBody = """ + { + "email": "%s", + "provenanceUserId": "%s" + } + """.formatted(TEST_EMAIL, TEST_PROVENANCE_ID); + + Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); + assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_EMAIL); + assertThat(response.jsonPath().getString("provenanceUserId")).isEqualTo(TEST_PROVENANCE_ID); + } +} From a5ae3407f633b224613f822a395110770c7b2258 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Tue, 10 Dec 2024 12:06:28 +0000 Subject: [PATCH 12/30] Remove whitespace --- .../reform/pip/account/management/service/AccountService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountService.java index 8ec302cc..9dd8f6f3 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountService.java @@ -139,7 +139,6 @@ public Map> addUsers(List users, String issuerId) public boolean isUserAuthorisedForPublication(UUID userId, ListType listType, Sensitivity sensitivity) { PiUser userToCheck = checkUserReturned(userRepository.findByUserId(userId), userId); return sensitivityService.checkAuthorisation(userToCheck, listType, sensitivity); - } /** From 5df8416d1bfe6bb2ec340a3f8c91168285f88eda Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Tue, 10 Dec 2024 12:19:02 +0000 Subject: [PATCH 13/30] Remove String.format as we're concatenating strings --- .../management/controllers/MediaApplicationCreationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/MediaApplicationCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/MediaApplicationCreationTest.java index 9583034a..3755f757 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/MediaApplicationCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/MediaApplicationCreationTest.java @@ -37,7 +37,7 @@ class MediaApplicationCreationTest extends FunctionalTestBase { private static final String TEST_EMAIL_PREFIX = String.format( "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); - private static final String TEST_EMAIL = String.format(TEST_EMAIL_PREFIX + "@justice.gov.uk"); + private static final String TEST_EMAIL = TEST_EMAIL_PREFIX + "@justice.gov.uk"; private static final String STATUS = "PENDING"; private static final String TESTING_SUPPORT_APPLICATION_URL = "/testing-support/application/"; From 333090ae9f4af3e5c02ffe1bf641f53e623c6d37 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Tue, 10 Dec 2024 12:25:53 +0000 Subject: [PATCH 14/30] Add teardown method and update constant variable names --- .../SystemAdminAccountCreationTest.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java index 12589757..e8e60ef4 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -2,6 +2,7 @@ import com.github.dockerjava.zerodep.shaded.org.apache.hc.core5.http.HttpHeaders; import io.restassured.response.Response; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -22,13 +23,14 @@ @ActiveProfiles(profiles = "functional") @SpringBootTest(classes = {OAuthClient.class}) class SystemAdminAccountCreationTest extends FunctionalTestBase { - private static final String TEST_EMAIL_PREFIX = String.format( + private static final String TEST_USER_EMAIL_PREFIX = String.format( "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); - private static final String TEST_EMAIL = String.format(TEST_EMAIL_PREFIX, "@justice.gov.uk"); + private static final String TEST_USER_EMAIL = TEST_USER_EMAIL_PREFIX + "@justice.gov.uk"; + private static final String TEST_USER_PROVENANCE_ID = UUID.randomUUID().toString(); + private static final String TESTING_SUPPORT_APPLICATION_URL = "/testing-support/account/"; private static final String ACCOUNT_URL = "/account"; private static final String SYSTEM_ADMIN_URL = ACCOUNT_URL + "/system-admin"; - private static final String TEST_PROVENANCE_ID = UUID.randomUUID().toString(); private static final String BEARER = "Bearer "; private Map bearer; @@ -38,6 +40,11 @@ public void startUp() { bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); } + @AfterAll + public void teardown() { + doDeleteRequest(TESTING_SUPPORT_APPLICATION_URL + TEST_USER_EMAIL, bearer); + } + @Test public void createSystemAdminAccount() { String requestBody = """ @@ -45,12 +52,12 @@ public void createSystemAdminAccount() { "email": "%s", "provenanceUserId": "%s" } - """.formatted(TEST_EMAIL, TEST_PROVENANCE_ID); + """.formatted(TEST_USER_EMAIL, TEST_USER_PROVENANCE_ID); Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); - assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_EMAIL); - assertThat(response.jsonPath().getString("provenanceUserId")).isEqualTo(TEST_PROVENANCE_ID); + assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_USER_EMAIL); + assertThat(response.jsonPath().getString("provenanceUserId")).isEqualTo(TEST_USER_PROVENANCE_ID); } } From 675ec2a69a4f16198d4d004a3d314debe9512c5f Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Tue, 10 Dec 2024 12:34:52 +0000 Subject: [PATCH 15/30] Add tests for no email and no user provenance id --- .../SystemAdminAccountCreationTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java index e8e60ef4..11b42e84 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -60,4 +60,32 @@ public void createSystemAdminAccount() { assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_USER_EMAIL); assertThat(response.jsonPath().getString("provenanceUserId")).isEqualTo(TEST_USER_PROVENANCE_ID); } + + @Test + public void shouldFailToCreateSystemAdminAccountWithoutEmail() { + String requestBody = """ + { + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_PROVENANCE_ID); + + Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + } + + @Test + public void shouldFailToCreateSystemAdminAccountWithoutProvenanceUserId() { + String requestBody = """ + { + "email": "%s" + } + """.formatted(TEST_USER_EMAIL); + + Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + } + + } From 55eedf8a0c0304b721c02ef622c583654c4e49cc Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Tue, 10 Dec 2024 15:21:56 +0000 Subject: [PATCH 16/30] Update url --- .../controllers/SystemAdminAccountCreationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java index 11b42e84..84645f0b 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -28,7 +28,7 @@ class SystemAdminAccountCreationTest extends FunctionalTestBase { private static final String TEST_USER_EMAIL = TEST_USER_EMAIL_PREFIX + "@justice.gov.uk"; private static final String TEST_USER_PROVENANCE_ID = UUID.randomUUID().toString(); - private static final String TESTING_SUPPORT_APPLICATION_URL = "/testing-support/account/"; + private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; private static final String ACCOUNT_URL = "/account"; private static final String SYSTEM_ADMIN_URL = ACCOUNT_URL + "/system-admin"; private static final String BEARER = "Bearer "; @@ -42,7 +42,7 @@ public void startUp() { @AfterAll public void teardown() { - doDeleteRequest(TESTING_SUPPORT_APPLICATION_URL + TEST_USER_EMAIL, bearer); + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL, bearer); } @Test From f96ffa5798b9ab42423b7d4779fa0ae8a4db3a90 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Wed, 11 Dec 2024 13:53:11 +0000 Subject: [PATCH 17/30] Add functional tests for system admin b2c account creation endpoint --- .../SystemAdminB2CAccountCreationTest.java | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java new file mode 100644 index 00000000..af99c5df --- /dev/null +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java @@ -0,0 +1,113 @@ +package uk.gov.hmcts.reform.pip.account.management.controllers; + +import com.github.dockerjava.zerodep.shaded.org.apache.hc.core5.http.HttpHeaders; +import io.restassured.response.Response; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import uk.gov.hmcts.reform.pip.account.management.utils.FunctionalTestBase; +import uk.gov.hmcts.reform.pip.account.management.utils.OAuthClient; + +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +@ExtendWith(SpringExtension.class) +@ActiveProfiles(profiles = "functional") +@SpringBootTest(classes = {OAuthClient.class}) +class SystemAdminB2CAccountCreationTest extends FunctionalTestBase { + private static final String TEST_USER_EMAIL_PREFIX_1 = String.format( + "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_USER_EMAIL_PREFIX_2 = String.format( + "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_USER_EMAIL_1 = TEST_USER_EMAIL_PREFIX_1 + "@justice.gov.uk"; + private static final String TEST_USER_EMAIL_2 = TEST_USER_EMAIL_PREFIX_2 + "@justice.gov.uk"; + private static final String TEST_USER_PROVENANCE_ID = UUID.randomUUID().toString(); + private static final String USER_ID = UUID.randomUUID().toString(); + + private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; + private static final String ACCOUNT_URL = "/account"; + private static final String SYSTEM_ADMIN_B2C_URL = ACCOUNT_URL + "/add/system-admin"; + private static final String BEARER = "Bearer "; + private static final String ISSUER_ID = "x-issuer-id"; + + private Map bearer; + private Map issuerId; + + @BeforeAll + public void startUp() { + bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); + issuerId = Map.of(ISSUER_ID, USER_ID); + } + + @AfterAll + public void teardown() { + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL_1, bearer); + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL_2, bearer); + } + + @Test + public void createSystemAdminB2CAccount() { + String requestBody = """ + { + "email": "%s", + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_EMAIL_1, TEST_USER_PROVENANCE_ID); + + Response response = doPostRequestForB2C(SYSTEM_ADMIN_B2C_URL, bearer, issuerId, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); + assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_USER_EMAIL_1); + assertThat(response.jsonPath().getString("provenanceUserId")).isNotNull(); + } + + @Test + public void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { + String requestBody = """ + { + "email": "%s" + } + """.formatted(TEST_USER_EMAIL_2); + + Response response = doPostRequestForB2C(SYSTEM_ADMIN_B2C_URL, bearer, issuerId, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); + assertThat(response.jsonPath().getString("email")).isEqualTo(TEST_USER_EMAIL_2); + assertThat(response.jsonPath().getString("provenanceUserId")).isNotNull(); + } + + @Test + public void shouldFailToCreateSystemAdminB2CAccountWithoutEmail() { + String requestBody = """ + { + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_PROVENANCE_ID); + + Response response = doPostRequestForB2C(SYSTEM_ADMIN_B2C_URL, bearer, issuerId, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + } + + @Test + public void shouldFailToCreateSystemAdminB2CAccountWithoutIssuerId() { + String requestBody = """ + { + "email": "%s", + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_EMAIL_1, TEST_USER_PROVENANCE_ID); + + Response response = doPostRequest(SYSTEM_ADMIN_B2C_URL, bearer, requestBody); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + } +} From 6ce456ff839c0fe1559e9f2d7c8e0e4073fa9e84 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Wed, 11 Dec 2024 13:54:28 +0000 Subject: [PATCH 18/30] Add post method for system admin b2c functional tests --- .../account/management/utils/FunctionalTestBase.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java index 4595cdfa..1f445a1a 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java @@ -58,6 +58,18 @@ protected Response doPostRequest(final String path, final Map ad .thenReturn(); } + protected Response doPostRequestForB2C(final String path, final Map additionalHeaders, + final Map issuerId, final String body) { + return given() + .relaxedHTTPSValidation() + .headers(getRequestHeaders(additionalHeaders)) + .headers(getRequestHeaders(issuerId)) + .body(body) + .when() + .post(path) + .thenReturn(); + } + protected Response doPostMultipartForApplication(final String path, final Map additionalHeaders, final File file, String name, String email, String employer, String status) { return given() From 0cfd0a1e0be1dc46732745aacefdd15411d2bd2c Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:15:22 +0000 Subject: [PATCH 19/30] PUB-2681 - Fixed PMD issue --- .../account/management/controllers/AccountTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java index 701ab8ea..100c98ee 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountTest.java @@ -119,6 +119,7 @@ class AccountTest { private static final String FORBIDDEN_STATUS_CODE = "Status code does not match forbidden"; private static final String DELETE_USER_FAILURE = "Failed to delete user account"; private static final String DELETE_USER_SUCCESS = "User deleted"; + private static final String ADD_USERS_SCRIPT = "classpath:add-admin-users.sql"; private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @@ -610,7 +611,7 @@ void testDeleteAccountNotFound() throws Exception { } @Test - @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT) void testV2SystemAdminDeletesVerifiedUser() throws Exception { validUser.setUserProvenance(UserProvenances.CFT_IDAM); validUser.setRoles(Roles.VERIFIED); @@ -644,7 +645,7 @@ void testV2SystemAdminDeletesVerifiedUser() throws Exception { } @Test - @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT) void testV2SystemAdminDeletesThirdPartyUser() throws Exception { PiUser thirdPartyUser = createThirdPartyUser(); @@ -676,7 +677,7 @@ void testV2SystemAdminDeletesThirdPartyUser() throws Exception { } @Test - @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT) void testV2SystemAdminDeletesSuperAdminUser() throws Exception { superAdminUser.setUserProvenance(UserProvenances.CFT_IDAM); String superAdminUserId = getSuperAdminUserId(superAdminUser); @@ -897,7 +898,7 @@ void testUpdateAccountRoleByIdNotFound() throws Exception { } @Test - @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT) void testCreateThirdPartyUser() throws Exception { PiUser thirdPartyUser = createThirdPartyUser(); MockHttpServletRequestBuilder mockHttpServletRequestBuilder = MockMvcRequestBuilders @@ -920,7 +921,7 @@ void testCreateThirdPartyUser() throws Exception { } @Test - @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:add-admin-users.sql") + @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT) void testUnauthorizedCreateThirdPartyUser() throws Exception { PiUser thirdPartyUser = createUser(true, Roles.GENERAL_THIRD_PARTY); MockHttpServletRequestBuilder mockHttpServletRequestBuilder = MockMvcRequestBuilders From 092b8c8864e6ed5399adfd8ec4cb4488539ed0f2 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 11:28:52 +0000 Subject: [PATCH 20/30] Add functional tests for bulk account creation endpoint --- .../controllers/BulkAccountTest.java | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java new file mode 100644 index 00000000..0d6f13e4 --- /dev/null +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -0,0 +1,113 @@ +package uk.gov.hmcts.reform.pip.account.management.controllers; + +import com.github.dockerjava.zerodep.shaded.org.apache.hc.core5.http.HttpHeaders; +import io.restassured.response.Response; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import uk.gov.hmcts.reform.pip.account.management.utils.FunctionalTestBase; +import uk.gov.hmcts.reform.pip.account.management.utils.OAuthClient; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +@ExtendWith(SpringExtension.class) +@ActiveProfiles(profiles = "functional") +@SpringBootTest(classes = {OAuthClient.class}) +class BulkAccountTest extends FunctionalTestBase { + private static final String USER_ID = UUID.randomUUID().toString(); + private static final String MOCK_FILE = "files/test-csv.csv"; + private static final String BULK_UPLOAD_URL = "account/media-bulk-upload"; + private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; + private static final String BEARER = "Bearer "; + private static final String ISSUER_ID = "x-issuer-id"; + + private Map bearer; + private Map issuerId; + + @BeforeAll + void startUp() throws IOException { + bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); + issuerId = Map.of(ISSUER_ID, USER_ID); + + StringBuilder csvContent = new StringBuilder("email,firstName,surname\n"); + + for (int i = 0; i < 5; i++) { + String email = generateTestEmail(); + csvContent.append(email).append(",testBulkFirstName,testBulkSurname\n"); + } + + var mockFilePath = Files.createTempFile("mock-bulk-upload", ".csv"); + try (FileWriter writer = new FileWriter(mockFilePath.toFile())) { + writer.write(csvContent.toString()); + } + + System.setProperty(MOCK_FILE, mockFilePath.toString()); + } + + private String generateTestEmail() { + String prefix = String.format("pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + return prefix + "@justice.gov.uk"; + } + + @AfterAll + public void teardown() throws IOException { + File mockFile = new File(System.getProperty(MOCK_FILE)); + + List emails = Files.lines(mockFile.toPath(), StandardCharsets.UTF_8) + .skip(1) + .map(line -> line.split(",")[0]) + .toList(); + + for (String email : emails) { + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + email, bearer); + } + + Files.deleteIfExists(mockFile.toPath()); + } + + @Test + public void createAccountsInBulk() { + File mockFile = new File(System.getProperty(MOCK_FILE)); + + Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); + assertThat(response.jsonPath().getList("CREATED_ACCOUNTS").size()).isEqualTo(5); + assertThat(response.jsonPath().getList("ERRORED_ACCOUNTS").isEmpty()).isTrue(); + } + + @Test + public void shouldReturnOKButNotCreateAccountsForDuplicates() { + File mockFile = new File(System.getProperty(MOCK_FILE)); + + Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); + assertThat(response.jsonPath().getList("CREATED_ACCOUNTS").isEmpty()).isTrue(); + assertThat(response.jsonPath().getList("ERRORED_ACCOUNTS").isEmpty()).isTrue(); + } + +} + + + + + + + + From 9460dc66f3eb8f88cb32df651d9197da95803e2e Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 11:30:48 +0000 Subject: [PATCH 21/30] Add post method for bulk account creation functional tests --- .../management/utils/FunctionalTestBase.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java index 1f445a1a..23a442c3 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/utils/FunctionalTestBase.java @@ -70,8 +70,21 @@ protected Response doPostRequestForB2C(final String path, final Map additionalHeaders, + final Map issuerId, final File mediaList) { + return given() + .relaxedHTTPSValidation() + .headers(additionalHeaders) + .headers(getRequestHeaders(issuerId)) + .contentType("multipart/form-data") + .multiPart("mediaList", mediaList) + .when() + .post(path) + .thenReturn(); + } + protected Response doPostMultipartForApplication(final String path, final Map additionalHeaders, - final File file, String name, String email, String employer, String status) { + final File file, String name, String email, String employer, String status) { return given() .relaxedHTTPSValidation() .headers(additionalHeaders) From 7151da1417896d87a12ed9cf79d06d7e28e844b4 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 11:50:32 +0000 Subject: [PATCH 22/30] Fix checkstyle issues --- .../management/controllers/BulkAccountTest.java | 2 +- .../SystemAdminAccountCreationTest.java | 16 ++++++++-------- .../SystemAdminB2CAccountCreationTest.java | 16 ++++++++-------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index 0d6f13e4..7802c1db 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -92,7 +92,7 @@ public void createAccountsInBulk() { } @Test - public void shouldReturnOKButNotCreateAccountsForDuplicates() { + public void shouldReturnOkButNotCreateAccountsForDuplicates() { File mockFile = new File(System.getProperty(MOCK_FILE)); Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java index 84645f0b..0ca88024 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -64,10 +64,10 @@ public void createSystemAdminAccount() { @Test public void shouldFailToCreateSystemAdminAccountWithoutEmail() { String requestBody = """ - { - "provenanceUserId": "%s" - } - """.formatted(TEST_USER_PROVENANCE_ID); + { + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_PROVENANCE_ID); Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); @@ -77,10 +77,10 @@ public void shouldFailToCreateSystemAdminAccountWithoutEmail() { @Test public void shouldFailToCreateSystemAdminAccountWithoutProvenanceUserId() { String requestBody = """ - { - "email": "%s" - } - """.formatted(TEST_USER_EMAIL); + { + "email": "%s" + } + """.formatted(TEST_USER_EMAIL); Response response = doPostRequest(SYSTEM_ADMIN_URL, bearer, requestBody); diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java index af99c5df..71d6287c 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java @@ -72,10 +72,10 @@ public void createSystemAdminB2CAccount() { @Test public void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { String requestBody = """ - { - "email": "%s" - } - """.formatted(TEST_USER_EMAIL_2); + { + "email": "%s" + } + """.formatted(TEST_USER_EMAIL_2); Response response = doPostRequestForB2C(SYSTEM_ADMIN_B2C_URL, bearer, issuerId, requestBody); @@ -87,10 +87,10 @@ public void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { @Test public void shouldFailToCreateSystemAdminB2CAccountWithoutEmail() { String requestBody = """ - { - "provenanceUserId": "%s" - } - """.formatted(TEST_USER_PROVENANCE_ID); + { + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_PROVENANCE_ID); Response response = doPostRequestForB2C(SYSTEM_ADMIN_B2C_URL, bearer, issuerId, requestBody); From 09319eb2d347b2d3c97605a279c0532e640574dd Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 12:20:04 +0000 Subject: [PATCH 23/30] Fix pmd issues --- .../management/controllers/BulkAccountTest.java | 14 ++++++++------ .../SystemAdminAccountCreationTest.java | 7 +++---- .../SystemAdminB2CAccountCreationTest.java | 8 ++++---- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index 7802c1db..e67a12fb 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -13,11 +13,12 @@ import uk.gov.hmcts.reform.pip.account.management.utils.FunctionalTestBase; import uk.gov.hmcts.reform.pip.account.management.utils.OAuthClient; +import java.io.BufferedWriter; import java.io.File; -import java.io.FileWriter; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.Map; import java.util.UUID; @@ -44,15 +45,16 @@ void startUp() throws IOException { bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); issuerId = Map.of(ISSUER_ID, USER_ID); - StringBuilder csvContent = new StringBuilder("email,firstName,surname\n"); + StringBuilder csvContent = new StringBuilder(400); + csvContent.append("email,firstName,surname\n"); for (int i = 0; i < 5; i++) { String email = generateTestEmail(); csvContent.append(email).append(",testBulkFirstName,testBulkSurname\n"); } - var mockFilePath = Files.createTempFile("mock-bulk-upload", ".csv"); - try (FileWriter writer = new FileWriter(mockFilePath.toFile())) { + Path mockFilePath = Files.createTempFile("mock-bulk-upload", ".csv"); + try (BufferedWriter writer = Files.newBufferedWriter(mockFilePath)) { writer.write(csvContent.toString()); } @@ -81,7 +83,7 @@ public void teardown() throws IOException { } @Test - public void createAccountsInBulk() { + void createAccountsInBulk() { File mockFile = new File(System.getProperty(MOCK_FILE)); Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); @@ -92,7 +94,7 @@ public void createAccountsInBulk() { } @Test - public void shouldReturnOkButNotCreateAccountsForDuplicates() { + void shouldReturnOkButNotCreateAccountsForDuplicates() { File mockFile = new File(System.getProperty(MOCK_FILE)); Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java index 0ca88024..78014050 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminAccountCreationTest.java @@ -46,7 +46,7 @@ public void teardown() { } @Test - public void createSystemAdminAccount() { + void createSystemAdminAccount() { String requestBody = """ { "email": "%s", @@ -62,7 +62,7 @@ public void createSystemAdminAccount() { } @Test - public void shouldFailToCreateSystemAdminAccountWithoutEmail() { + void shouldFailToCreateSystemAdminAccountWithoutEmail() { String requestBody = """ { "provenanceUserId": "%s" @@ -75,7 +75,7 @@ public void shouldFailToCreateSystemAdminAccountWithoutEmail() { } @Test - public void shouldFailToCreateSystemAdminAccountWithoutProvenanceUserId() { + void shouldFailToCreateSystemAdminAccountWithoutProvenanceUserId() { String requestBody = """ { "email": "%s" @@ -87,5 +87,4 @@ public void shouldFailToCreateSystemAdminAccountWithoutProvenanceUserId() { assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } - } diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java index 71d6287c..0143962b 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java @@ -54,7 +54,7 @@ public void teardown() { } @Test - public void createSystemAdminB2CAccount() { + void createSystemAdminB2CAccount() { String requestBody = """ { "email": "%s", @@ -70,7 +70,7 @@ public void createSystemAdminB2CAccount() { } @Test - public void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { + void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { String requestBody = """ { "email": "%s" @@ -85,7 +85,7 @@ public void shouldCreateSystemAdminB2CAccountWithoutProvenanceUserId() { } @Test - public void shouldFailToCreateSystemAdminB2CAccountWithoutEmail() { + void shouldFailToCreateSystemAdminB2CAccountWithoutEmail() { String requestBody = """ { "provenanceUserId": "%s" @@ -98,7 +98,7 @@ public void shouldFailToCreateSystemAdminB2CAccountWithoutEmail() { } @Test - public void shouldFailToCreateSystemAdminB2CAccountWithoutIssuerId() { + void shouldFailToCreateSystemAdminB2CAccountWithoutIssuerId() { String requestBody = """ { "email": "%s", From 981319a02a5a7b6b86e8ed67289d688d4f515d32 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 15:01:43 +0000 Subject: [PATCH 24/30] Use common email prefix to delete all accounts at once --- .../controllers/BulkAccountTest.java | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index e67a12fb..3772de2e 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -31,6 +31,7 @@ @SpringBootTest(classes = {OAuthClient.class}) class BulkAccountTest extends FunctionalTestBase { private static final String USER_ID = UUID.randomUUID().toString(); + private static final String COMMON_PREFIX = "pip-am-test-email-bulk-"; private static final String MOCK_FILE = "files/test-csv.csv"; private static final String BULK_UPLOAD_URL = "account/media-bulk-upload"; private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; @@ -62,24 +63,14 @@ void startUp() throws IOException { } private String generateTestEmail() { - String prefix = String.format("pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + String prefix = String.format(COMMON_PREFIX + "%s", ThreadLocalRandom.current().nextInt(1000, 9999)); return prefix + "@justice.gov.uk"; } @AfterAll public void teardown() throws IOException { - File mockFile = new File(System.getProperty(MOCK_FILE)); - - List emails = Files.lines(mockFile.toPath(), StandardCharsets.UTF_8) - .skip(1) - .map(line -> line.split(",")[0]) - .toList(); - - for (String email : emails) { - doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + email, bearer); - } - - Files.deleteIfExists(mockFile.toPath()); + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + COMMON_PREFIX, bearer); + Files.deleteIfExists(Path.of(MOCK_FILE)); } @Test @@ -103,13 +94,4 @@ void shouldReturnOkButNotCreateAccountsForDuplicates() { assertThat(response.jsonPath().getList("CREATED_ACCOUNTS").isEmpty()).isTrue(); assertThat(response.jsonPath().getList("ERRORED_ACCOUNTS").isEmpty()).isTrue(); } - } - - - - - - - - From e78fdd0484c094b7a25d66ce31a373fba4ceecf6 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 15:09:28 +0000 Subject: [PATCH 25/30] Set mock csv as variable --- .../management/controllers/BulkAccountTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index 3772de2e..c7e5e978 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -32,12 +32,12 @@ class BulkAccountTest extends FunctionalTestBase { private static final String USER_ID = UUID.randomUUID().toString(); private static final String COMMON_PREFIX = "pip-am-test-email-bulk-"; - private static final String MOCK_FILE = "files/test-csv.csv"; private static final String BULK_UPLOAD_URL = "account/media-bulk-upload"; private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; private static final String BEARER = "Bearer "; private static final String ISSUER_ID = "x-issuer-id"; + private String mockFile; private Map bearer; private Map issuerId; @@ -59,7 +59,7 @@ void startUp() throws IOException { writer.write(csvContent.toString()); } - System.setProperty(MOCK_FILE, mockFilePath.toString()); + mockFile = mockFilePath.toString(); } private String generateTestEmail() { @@ -70,14 +70,14 @@ private String generateTestEmail() { @AfterAll public void teardown() throws IOException { doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + COMMON_PREFIX, bearer); - Files.deleteIfExists(Path.of(MOCK_FILE)); + Files.deleteIfExists(Path.of(mockFile)); } @Test void createAccountsInBulk() { - File mockFile = new File(System.getProperty(MOCK_FILE)); + File mockBulkUploadFile = new File(mockFile); - Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); + Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockBulkUploadFile); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); assertThat(response.jsonPath().getList("CREATED_ACCOUNTS").size()).isEqualTo(5); @@ -86,9 +86,9 @@ void createAccountsInBulk() { @Test void shouldReturnOkButNotCreateAccountsForDuplicates() { - File mockFile = new File(System.getProperty(MOCK_FILE)); + File mockBulkUploadFile = new File(mockFile); - Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockFile); + Response response = doPostMultipartForBulk(BULK_UPLOAD_URL, bearer, issuerId, mockBulkUploadFile); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK.value()); assertThat(response.jsonPath().getList("CREATED_ACCOUNTS").isEmpty()).isTrue(); From 1231a49cfbfa3ad592f3326f1a52e388a174fa91 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 15:17:21 +0000 Subject: [PATCH 26/30] Remove unused imports --- .../pip/account/management/controllers/BulkAccountTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index c7e5e978..763b46b4 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -16,10 +16,8 @@ import java.io.BufferedWriter; import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; From 878c90ad8ea233aab3803077042ec45b7889c011 Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 16:09:29 +0000 Subject: [PATCH 27/30] Add test suite specific email prefix --- .../management/controllers/BulkAccountTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index 763b46b4..ffb2e138 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -29,7 +29,9 @@ @SpringBootTest(classes = {OAuthClient.class}) class BulkAccountTest extends FunctionalTestBase { private static final String USER_ID = UUID.randomUUID().toString(); - private static final String COMMON_PREFIX = "pip-am-test-email-bulk-"; + private static final String EMAIL_PREFIX = "pip-am-test-email-"; + private static final String TEST_SUITE_PREFIX = String.format("%s-", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_SUITE_EMAIL_PREFIX = EMAIL_PREFIX + TEST_SUITE_PREFIX; private static final String BULK_UPLOAD_URL = "account/media-bulk-upload"; private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; private static final String BEARER = "Bearer "; @@ -61,13 +63,13 @@ void startUp() throws IOException { } private String generateTestEmail() { - String prefix = String.format(COMMON_PREFIX + "%s", ThreadLocalRandom.current().nextInt(1000, 9999)); - return prefix + "@justice.gov.uk"; + String prefix = String.format("%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + return TEST_SUITE_EMAIL_PREFIX + prefix + "@justice.gov.uk"; } @AfterAll public void teardown() throws IOException { - doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + COMMON_PREFIX, bearer); + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_SUITE_EMAIL_PREFIX, bearer); Files.deleteIfExists(Path.of(mockFile)); } From 7320303c8655389bf37adb2f5cf7b6b9d901ae4b Mon Sep 17 00:00:00 2001 From: Natasha Alker Date: Fri, 13 Dec 2024 16:15:00 +0000 Subject: [PATCH 28/30] Fix checkstyle issue --- .../pip/account/management/controllers/BulkAccountTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java index ffb2e138..896ed553 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/BulkAccountTest.java @@ -30,7 +30,8 @@ class BulkAccountTest extends FunctionalTestBase { private static final String USER_ID = UUID.randomUUID().toString(); private static final String EMAIL_PREFIX = "pip-am-test-email-"; - private static final String TEST_SUITE_PREFIX = String.format("%s-", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_SUITE_PREFIX = String.format("%s-", + ThreadLocalRandom.current().nextInt(1000, 9999)); private static final String TEST_SUITE_EMAIL_PREFIX = EMAIL_PREFIX + TEST_SUITE_PREFIX; private static final String BULK_UPLOAD_URL = "account/media-bulk-upload"; private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; From d2856ce4f3696ad331e87558bcfe25407eb29300 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:06:02 +0000 Subject: [PATCH 29/30] Updated max system admin accounts staging --- charts/pip-account-management/values.stg.template.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/charts/pip-account-management/values.stg.template.yaml b/charts/pip-account-management/values.stg.template.yaml index 5d49432c..1bb8dca6 100644 --- a/charts/pip-account-management/values.stg.template.yaml +++ b/charts/pip-account-management/values.stg.template.yaml @@ -5,3 +5,4 @@ java: environment: ENABLE_TESTING_SUPPORT_API: true MANAGED_IDENTITY_CLIENT_ID: 8d0ead51-3b31-44df-a78e-ada4eea9fe87 + MAX_SYSTEM_ADMIN_ACCOUNTS: 20 From fbd9ae94759c7cd626dcc22fc5fd06ee9d4b9bd3 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:14:46 +0000 Subject: [PATCH 30/30] PUB-2681 - Updated functional tests --- .../SystemAdminB2CAccountCreationTest.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java index 0143962b..6063802b 100644 --- a/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java +++ b/src/functionalTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/SystemAdminB2CAccountCreationTest.java @@ -27,14 +27,17 @@ class SystemAdminB2CAccountCreationTest extends FunctionalTestBase { "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); private static final String TEST_USER_EMAIL_PREFIX_2 = String.format( "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); + private static final String TEST_USER_EMAIL_PREFIX_3 = String.format( + "pip-am-test-email-%s", ThreadLocalRandom.current().nextInt(1000, 9999)); private static final String TEST_USER_EMAIL_1 = TEST_USER_EMAIL_PREFIX_1 + "@justice.gov.uk"; private static final String TEST_USER_EMAIL_2 = TEST_USER_EMAIL_PREFIX_2 + "@justice.gov.uk"; + private static final String TEST_USER_EMAIL_3 = TEST_USER_EMAIL_PREFIX_3 + "@justice.gov.uk"; private static final String TEST_USER_PROVENANCE_ID = UUID.randomUUID().toString(); - private static final String USER_ID = UUID.randomUUID().toString(); private static final String TESTING_SUPPORT_ACCOUNT_URL = "/testing-support/account/"; private static final String ACCOUNT_URL = "/account"; private static final String SYSTEM_ADMIN_B2C_URL = ACCOUNT_URL + "/add/system-admin"; + private static final String SYSTEM_ADMIN_SSO_URL = ACCOUNT_URL + "/system-admin"; private static final String BEARER = "Bearer "; private static final String ISSUER_ID = "x-issuer-id"; @@ -44,13 +47,25 @@ class SystemAdminB2CAccountCreationTest extends FunctionalTestBase { @BeforeAll public void startUp() { bearer = Map.of(HttpHeaders.AUTHORIZATION, BEARER + accessToken); - issuerId = Map.of(ISSUER_ID, USER_ID); + + String requestBody = """ + { + "email": "%s", + "provenanceUserId": "%s" + } + """.formatted(TEST_USER_EMAIL_3, UUID.randomUUID().toString()); + + String userId = doPostRequest(SYSTEM_ADMIN_SSO_URL, bearer, requestBody) + .jsonPath().getString("userId"); + + issuerId = Map.of(ISSUER_ID, userId); } @AfterAll public void teardown() { doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL_1, bearer); doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL_2, bearer); + doDeleteRequest(TESTING_SUPPORT_ACCOUNT_URL + TEST_USER_EMAIL_3, bearer); } @Test