Merge pull request #462 from hmcts/PUB-2681-Update-System-Admin

PUB-2681 - Update System Admin Alert

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'

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

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,6 +611,7 @@ void testDeleteAccountNotFound() throws Exception {
     }
 
     @Test
+    @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT)
     void testV2SystemAdminDeletesVerifiedUser() throws Exception {
         validUser.setUserProvenance(UserProvenances.CFT_IDAM);
         validUser.setRoles(Roles.VERIFIED);
@@ -643,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();
@@ -675,6 +677,7 @@ void testV2SystemAdminDeletesThirdPartyUser() throws Exception {
     }
 
     @Test
+    @Sql(executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, scripts = ADD_USERS_SCRIPT)
     void testV2SystemAdminDeletesSuperAdminUser() throws Exception {
         superAdminUser.setUserProvenance(UserProvenances.CFT_IDAM);
         String superAdminUserId = getSuperAdminUserId(superAdminUser);
@@ -895,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
@@ -918,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

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;
@@ -54,6 +55,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";
@@ -222,7 +224,29 @@ 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)
+            .andExpect(status().isForbidden()).andReturn();
+
+        assertEquals(FORBIDDEN.value(), responseCreateSystemAdminUser.getResponse().getStatus(),
+                     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)

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');
-

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 lombok.AllArgsConstructor;
 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;
@@ -44,6 +45,7 @@ public class SystemAdminB2CAccountController {
     @ApiResponse(responseCode = OK_CODE, description = PI_USER)
     @ApiResponse(responseCode = BAD_REQUEST_CODE, description = "{ErroredSystemAdminAccount}")
     @PostMapping("/add/system-admin")
+    @PreAuthorize("@authorisationService.userCanCreateSystemAdmin(#issuerId)")
     public ResponseEntity<? extends PiUser> createSystemAdminAccount(//NOSONAR
         @RequestHeader(ISSUER_ID) String issuerId, @RequestBody SystemAdminAccount account) {
         return ResponseEntity.ok(systemAdminB2CAccountService.addSystemAdminAccount(account, issuerId));

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<PiUser> 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())) {

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;
 
@@ -38,20 +37,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 +60,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 +76,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 +99,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 +109,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<ConstraintViolation<SystemAdminAccount>> constraintViolationSet = validator.validate(account);
 
         if (!constraintViolationSet.isEmpty()) {
@@ -126,14 +120,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 +138,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<PiUser> adminUser = userRepository.findByUserId(UUID.fromString(issuerId));
-        if (adminUser.isPresent() && adminUser.get().getRoles().equals(Roles.SYSTEM_ADMIN)) {
-            return adminUser.get().getProvenanceUserId();
-        }
-
-        return "";
-    }
 }