From 2d0d34432c1de17c899e34d7ee6bcf4ccfc6401f Mon Sep 17 00:00:00 2001 From: Kian Kwa Date: Mon, 7 Oct 2024 16:33:30 +0100 Subject: [PATCH 1/3] Remove inactive SSO accounts --- .../management/database/UserRepository.java | 9 +++++--- .../InactiveAccountManagementService.java | 11 ++++++++-- src/main/resources/application.yaml | 1 + .../service/AccountServiceTest.java | 21 +++++++++++++++++++ .../InactiveAccountManagementServiceTest.java | 21 ++++++++++++------- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java index 7f33c28b..16c2119c 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java @@ -34,9 +34,12 @@ List findExistingByProvenanceId(@Param("provUserId") String provenanceUs + " * :daysAgo AND user_provenance = 'PI_AAD' AND roles = 'VERIFIED'", nativeQuery = true) List findVerifiedUsersByLastVerifiedDate(@Param("daysAgo") int daysSinceLastVerified); - @Query(value = "SELECT * FROM pi_user WHERE CAST(last_signed_in_date AS DATE) = CURRENT_DATE - (interval '1' day)" - + " * :daysAgo AND roles <> 'VERIFIED' AND user_provenance = 'PI_AAD'", nativeQuery = true) - List findAadAdminUsersByLastSignedInDate(@Param("daysAgo") int daysSinceLastSignedIn); + @Query(value = "SELECT * FROM pi_user WHERE (user_provenance = 'PI_AAD' AND roles <> 'VERIFIED' AND " + + "CAST(last_signed_in_date AS DATE) <= CURRENT_DATE - (interval '1' day) * :aadDays) OR " + + "(user_provenance = 'SSO' AND :ssoDays > 0 AND " + + "CAST(last_signed_in_date AS DATE) <= CURRENT_DATE - (interval '1' day) * :ssoDays)", nativeQuery = true) + List findAdminUsersByLastSignedInDate(@Param("aadDays") int aadNumberOfDays, + @Param("ssoDays") int ssoNumberOfDays); @Query(value = "SELECT * FROM pi_user WHERE (CAST(last_signed_in_date AS DATE) = CURRENT_DATE - (interval '1' day)" + " * :cftDaysAgo AND user_provenance = 'CFT_IDAM') " diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java index b1ffe547..6778afd4 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java @@ -14,6 +14,8 @@ @Service public class InactiveAccountManagementService { + private static final int SSO_ADMIN_ACCOUNT_SIGN_IN_NOTIFICATION_DAYS = 0; + private final UserRepository userRepository; private final AzureUserService azureUserService; private final PublicationService publicationService; @@ -31,6 +33,9 @@ public class InactiveAccountManagementService { @Value("${verification.aad-admin-account-deletion-days}") private int aadAdminAccountDeletionDays; + @Value("${verification.sso-admin-account-deletion-days}") + private int ssoAdminAccountDeletionDays; + @Value("${verification.cft-idam-account-sign-in-notification-days}") private int cftIdamAccountSignInNotificationDays; @@ -74,7 +79,9 @@ public void sendMediaUsersForVerification() { * Then send their details on to publication services to send them a notification email. */ public void notifyAdminUsersToSignIn() { - userRepository.findAadAdminUsersByLastSignedInDate(aadAdminAccountSignInNotificationDays) + userRepository.findAdminUsersByLastSignedInDate(aadAdminAccountSignInNotificationDays, + SSO_ADMIN_ACCOUNT_SIGN_IN_NOTIFICATION_DAYS + ) .forEach(user -> { try { publicationService.sendInactiveAccountSignInNotificationEmail( @@ -119,7 +126,7 @@ public void findMediaAccountsForDeletion() { * Account service handles the deletion of their AAD, P&I user and subscriptions. */ public void findAdminAccountsForDeletion() { - userRepository.findAadAdminUsersByLastSignedInDate(aadAdminAccountDeletionDays) + userRepository.findAdminUsersByLastSignedInDate(aadAdminAccountDeletionDays, ssoAdminAccountDeletionDays) .forEach(user -> accountService.deleteAccount(user.getUserId())); } diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 471bb92a..4aaa2007 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -111,6 +111,7 @@ verification: media-account-deletion-days: 365 aad-admin-account-sign-in-notification-days: 76 aad-admin-account-deletion-days: 90 + sso-admin-account-deletion-days: 90 cft-idam-account-sign-in-notification-days: 118 cft-idam-account-deletion-days: 132 crime-idam-account-sign-in-notification-days: 180 diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountServiceTest.java index aa7b4303..807f33a1 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountServiceTest.java @@ -102,9 +102,11 @@ class AccountServiceTest { private static final String USER_NOT_FOUND_EXCEPTION_MESSAGE = "The exception when a user has not been found has been thrown"; private static final UUID VALID_USER_ID = UUID.randomUUID(); + private static final UUID VALID_USER_ID_SSO = UUID.randomUUID(); private static final UUID VALID_USER_ID_IDAM = UUID.randomUUID(); private static final PiUser PI_USER = new PiUser(); + private static final PiUser PI_USER_SSO = new PiUser(); private static final PiUser PI_USER_IDAM = new PiUser(); private static final AzureAccount AZURE_ACCOUNT = new AzureAccount(); private static final User EXPECTED_USER = new User(); @@ -118,6 +120,10 @@ void setup() { PI_USER.setProvenanceUserId(ID); PI_USER.setEmail(EMAIL); + PI_USER_SSO.setUserId(VALID_USER_ID_SSO); + PI_USER_SSO.setUserProvenance(UserProvenances.SSO); + PI_USER_SSO.setProvenanceUserId(ID); + PI_USER_IDAM.setUserId(VALID_USER_ID_IDAM); PI_USER_IDAM.setUserProvenance(UserProvenances.CFT_IDAM); PI_USER_IDAM.setProvenanceUserId(ID); @@ -289,6 +295,21 @@ void testDeleteAadAccount() throws AzureCustomException { verify(userRepository, times(1)).delete(PI_USER); } + @Test + void testDeleteSsoAccount() { + when(userRepository.findByUserId(VALID_USER_ID_SSO)).thenReturn(Optional.of(PI_USER_SSO)); + + doNothing().when(userRepository).delete(PI_USER_SSO); + when(subscriptionService.sendSubscriptionDeletionRequest(VALID_USER_ID_SSO.toString())) + .thenReturn(SUBSCRIPTIONS_DELETED); + + accountService.deleteAccount(VALID_USER_ID_SSO); + + verifyNoInteractions(azureUserService); + verify(subscriptionService).sendSubscriptionDeletionRequest(VALID_USER_ID_SSO.toString()); + verify(userRepository).delete(PI_USER_SSO); + } + @Test void testDeleteIdamAccount() { when(userRepository.findByUserId(VALID_USER_ID_IDAM)).thenReturn(Optional.of(PI_USER_IDAM)); diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java index 0a66f230..368d7eb6 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.UUID; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -30,6 +31,8 @@ class InactiveAccountManagementServiceTest { private static final String MEDIA_USER_EMAIL = "media@test.com"; private static final UUID AAD_ADMIN_UUID = UUID.randomUUID(); private static final String AAD_ADMIN_USER_EMAIL = "aad_admin@test.com"; + private static final UUID SSO_ADMIN_UUID = UUID.randomUUID(); + private static final String SSO_ADMIN_USER_EMAIL = "sso_admin@test.com"; private static final UUID CFT_IDAM_UUID = UUID.randomUUID(); private static final String CFT_IDAM_USER_EMAIL = "cft_idam@test.com"; private static final UUID CRIME_IDAM_UUID = UUID.randomUUID(); @@ -47,11 +50,14 @@ class InactiveAccountManagementServiceTest { private static final PiUser AAD_ADMIN_USER = new PiUser(AAD_ADMIN_UUID, UserProvenances.PI_AAD, "2", AAD_ADMIN_USER_EMAIL, Roles.INTERNAL_SUPER_ADMIN_CTSC, FORENAME, SURNAME, null, null, LAST_SIGNED_IN_DATE); + private static final PiUser SSO_ADMIN_USER = new PiUser(SSO_ADMIN_UUID, UserProvenances.SSO, + "3", SSO_ADMIN_USER_EMAIL, Roles.INTERNAL_SUPER_ADMIN_CTSC, + FORENAME, SURNAME, null, null, LAST_SIGNED_IN_DATE); private static final PiUser CFT_IDAM_USER = new PiUser(CFT_IDAM_UUID, UserProvenances.CFT_IDAM, - "3", CFT_IDAM_USER_EMAIL, Roles.INTERNAL_ADMIN_CTSC, + "4", CFT_IDAM_USER_EMAIL, Roles.INTERNAL_ADMIN_CTSC, FORENAME, SURNAME, null, null, LAST_SIGNED_IN_DATE); private static final PiUser CRIME_IDAM_USER = new PiUser(CRIME_IDAM_UUID, UserProvenances.CRIME_IDAM, - "4", CRIME_IDAM_USER_EMAIL, Roles.INTERNAL_ADMIN_CTSC, + "5", CRIME_IDAM_USER_EMAIL, Roles.INTERNAL_ADMIN_CTSC, FORENAME, SURNAME, null, null, LAST_SIGNED_IN_DATE); private static User azureMediaUser = new User(); @@ -100,7 +106,7 @@ void testNoMediaUsersForVerification() { @Test void testNotifyAdminUsersToSignIn() throws AzureCustomException { - when(userRepository.findAadAdminUsersByLastSignedInDate(anyInt())) + when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), eq(0))) .thenReturn(Collections.singletonList(AAD_ADMIN_USER)); when(azureUserService.getUser(AAD_ADMIN_USER_EMAIL)).thenReturn(azureAdminUser); @@ -112,7 +118,7 @@ void testNotifyAdminUsersToSignIn() throws AzureCustomException { @Test void testNoNotificationOfAdminUsersToSignIn() { - when(userRepository.findAadAdminUsersByLastSignedInDate(anyInt())) + when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) .thenReturn(Collections.emptyList()); inactiveAccountManagementService.notifyAdminUsersToSignIn(); @@ -163,16 +169,17 @@ void testNoMediaAccountDeletion() { @Test void testAdminAccountDeletion() { - when(userRepository.findAadAdminUsersByLastSignedInDate(anyInt())) - .thenReturn(Collections.singletonList(AAD_ADMIN_USER)); + when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) + .thenReturn(List.of(AAD_ADMIN_USER, SSO_ADMIN_USER)); inactiveAccountManagementService.findAdminAccountsForDeletion(); verify(accountService).deleteAccount(AAD_ADMIN_UUID); + verify(accountService).deleteAccount(SSO_ADMIN_UUID); } @Test void testNoAdminAccountDeletion() { - when(userRepository.findAadAdminUsersByLastSignedInDate(anyInt())) + when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) .thenReturn(Collections.emptyList()); inactiveAccountManagementService.findAdminAccountsForDeletion(); From 8097549ae8d6bebef983d1b154591ae3da9d27f5 Mon Sep 17 00:00:00 2001 From: Kian Kwa Date: Thu, 10 Oct 2024 10:42:44 +0100 Subject: [PATCH 2/3] Split out admin repository queries for notification and deletion --- .../account/management/database/UserRepository.java | 11 +++++++---- .../service/InactiveAccountManagementService.java | 10 +++------- .../service/InactiveAccountManagementServiceTest.java | 8 ++++---- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java index 16c2119c..22e3705b 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java @@ -29,17 +29,20 @@ List findExistingByProvenanceId(@Param("provUserId") String provenanceUs nativeQuery = true) List getAccManDataForMI(); - @Query(value = "SELECT * FROM pi_user WHERE CAST(last_verified_date AS DATE) = CURRENT_DATE - (interval '1' day)" + " * :daysAgo AND user_provenance = 'PI_AAD' AND roles = 'VERIFIED'", nativeQuery = true) List findVerifiedUsersByLastVerifiedDate(@Param("daysAgo") int daysSinceLastVerified); + @Query(value = "SELECT * FROM pi_user WHERE user_provenance = 'PI_AAD' AND roles <> 'VERIFIED' AND " + + "CAST(last_signed_in_date AS DATE) = CURRENT_DATE - (interval '1' day) * :aadDays", nativeQuery = true) + List findAdminUsersFortNotificationByLastSignedInDate(@Param("aadDays") int aadNumberOfDays); + @Query(value = "SELECT * FROM pi_user WHERE (user_provenance = 'PI_AAD' AND roles <> 'VERIFIED' AND " + "CAST(last_signed_in_date AS DATE) <= CURRENT_DATE - (interval '1' day) * :aadDays) OR " - + "(user_provenance = 'SSO' AND :ssoDays > 0 AND " + + "(user_provenance = 'SSO' AND " + "CAST(last_signed_in_date AS DATE) <= CURRENT_DATE - (interval '1' day) * :ssoDays)", nativeQuery = true) - List findAdminUsersByLastSignedInDate(@Param("aadDays") int aadNumberOfDays, - @Param("ssoDays") int ssoNumberOfDays); + List findAdminUsersForDeletionByLastSignedInDate(@Param("aadDays") int aadNumberOfDays, + @Param("ssoDays") int ssoNumberOfDays); @Query(value = "SELECT * FROM pi_user WHERE (CAST(last_signed_in_date AS DATE) = CURRENT_DATE - (interval '1' day)" + " * :cftDaysAgo AND user_provenance = 'CFT_IDAM') " diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java index 6778afd4..852a49e5 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementService.java @@ -13,9 +13,6 @@ @Slf4j @Service public class InactiveAccountManagementService { - - private static final int SSO_ADMIN_ACCOUNT_SIGN_IN_NOTIFICATION_DAYS = 0; - private final UserRepository userRepository; private final AzureUserService azureUserService; private final PublicationService publicationService; @@ -79,9 +76,7 @@ public void sendMediaUsersForVerification() { * Then send their details on to publication services to send them a notification email. */ public void notifyAdminUsersToSignIn() { - userRepository.findAdminUsersByLastSignedInDate(aadAdminAccountSignInNotificationDays, - SSO_ADMIN_ACCOUNT_SIGN_IN_NOTIFICATION_DAYS - ) + userRepository.findAdminUsersFortNotificationByLastSignedInDate(aadAdminAccountSignInNotificationDays) .forEach(user -> { try { publicationService.sendInactiveAccountSignInNotificationEmail( @@ -126,7 +121,8 @@ public void findMediaAccountsForDeletion() { * Account service handles the deletion of their AAD, P&I user and subscriptions. */ public void findAdminAccountsForDeletion() { - userRepository.findAdminUsersByLastSignedInDate(aadAdminAccountDeletionDays, ssoAdminAccountDeletionDays) + userRepository.findAdminUsersForDeletionByLastSignedInDate(aadAdminAccountDeletionDays, + ssoAdminAccountDeletionDays) .forEach(user -> accountService.deleteAccount(user.getUserId())); } diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java index 368d7eb6..ce81e6cb 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java @@ -106,7 +106,7 @@ void testNoMediaUsersForVerification() { @Test void testNotifyAdminUsersToSignIn() throws AzureCustomException { - when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), eq(0))) + when(userRepository.findAdminUsersFortNotificationByLastSignedInDate(anyInt())) .thenReturn(Collections.singletonList(AAD_ADMIN_USER)); when(azureUserService.getUser(AAD_ADMIN_USER_EMAIL)).thenReturn(azureAdminUser); @@ -118,7 +118,7 @@ void testNotifyAdminUsersToSignIn() throws AzureCustomException { @Test void testNoNotificationOfAdminUsersToSignIn() { - when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) + when(userRepository.findAdminUsersFortNotificationByLastSignedInDate(anyInt())) .thenReturn(Collections.emptyList()); inactiveAccountManagementService.notifyAdminUsersToSignIn(); @@ -169,7 +169,7 @@ void testNoMediaAccountDeletion() { @Test void testAdminAccountDeletion() { - when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) + when(userRepository.findAdminUsersForDeletionByLastSignedInDate(anyInt(), anyInt())) .thenReturn(List.of(AAD_ADMIN_USER, SSO_ADMIN_USER)); inactiveAccountManagementService.findAdminAccountsForDeletion(); @@ -179,7 +179,7 @@ void testAdminAccountDeletion() { @Test void testNoAdminAccountDeletion() { - when(userRepository.findAdminUsersByLastSignedInDate(anyInt(), anyInt())) + when(userRepository.findAdminUsersForDeletionByLastSignedInDate(anyInt(), anyInt())) .thenReturn(Collections.emptyList()); inactiveAccountManagementService.findAdminAccountsForDeletion(); From 6ef5b232fb1eabb1de00e9b74966ac508e92b619 Mon Sep 17 00:00:00 2001 From: Kian Kwa Date: Thu, 10 Oct 2024 13:48:01 +0100 Subject: [PATCH 3/3] Fixed checkstyle failures --- .../management/service/InactiveAccountManagementServiceTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java index ce81e6cb..5adb2916 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/InactiveAccountManagementServiceTest.java @@ -18,7 +18,6 @@ import java.util.List; import java.util.UUID; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions;