From ec82bc42df3373054ec2b9b0434cdbfaf255bf1a Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Sat, 21 Dec 2024 07:18:28 -0500 Subject: [PATCH 1/3] feat!: Delete users from cognito on developer join When developer A joins developer B, developer A gets deleted and here we are making sure that all users assigned to developer A lose permissions on that organization. [#OCD-4712] --- .../chpl/manager/DeveloperManager.java | 57 ++++++++++++++++++- .../domains/DeveloperDomainPermissions.java | 4 ++ .../developer/DeleteActionPermissions.java | 19 +++++++ .../job/developer/JoinDeveloperJob.java | 5 +- .../TransactionalJoinDeveloperManager.java | 6 +- 5 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/developer/DeleteActionPermissions.java diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java index 2663faface..e914d236ed 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java @@ -12,6 +12,7 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; +import org.ff4j.FF4j; import org.quartz.JobDataMap; import org.quartz.SchedulerException; import org.springframework.beans.factory.annotation.Autowired; @@ -25,6 +26,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; +import gov.healthit.chpl.FeatureList; import gov.healthit.chpl.caching.CacheNames; import gov.healthit.chpl.caching.ListingSearchCacheRefresh; import gov.healthit.chpl.dao.CertifiedProductDAO; @@ -39,6 +41,7 @@ import gov.healthit.chpl.domain.Developer; import gov.healthit.chpl.domain.DeveloperStatusEvent; import gov.healthit.chpl.domain.IdNamePair; +import gov.healthit.chpl.domain.Organization; import gov.healthit.chpl.domain.Product; import gov.healthit.chpl.domain.activity.ActivityConcept; import gov.healthit.chpl.domain.auth.User; @@ -70,6 +73,7 @@ import gov.healthit.chpl.scheduler.job.developer.messaging.MessageDevelopersJob; import gov.healthit.chpl.sharedstore.listing.ListingStoreRemove; import gov.healthit.chpl.sharedstore.listing.RemoveBy; +import gov.healthit.chpl.user.cognito.CognitoUserManager; import gov.healthit.chpl.util.AuthUtil; import gov.healthit.chpl.util.ChplProductNumberUtil; import gov.healthit.chpl.util.ErrorMessageUtil; @@ -94,7 +98,9 @@ public class DeveloperManager extends SecuredManager { private DeveloperValidationFactory developerValidationFactory; private SearchRequestValidator developerSearchRequestValidator; private SearchRequestNormalizer developerSearchRequestNormalizer; + private CognitoUserManager cognitoUserManager; private SchedulerManager schedulerManager; + private FF4j ff4j; @Autowired @SuppressWarnings("checkstyle:parameternumber") @@ -104,7 +110,9 @@ public DeveloperManager(DeveloperDAO developerDao, ProductManager productManager ActivityManager activityManager, ErrorMessageUtil msgUtil, ResourcePermissionsFactory resourcePermissionsFactory, DeveloperValidationFactory developerValidationFactory, @Qualifier("developerSearchRequestValidator") SearchRequestValidator developerSearchRequestValidator, - SchedulerManager schedulerManager) { + CognitoUserManager cognitoUserManager, + SchedulerManager schedulerManager, + FF4j ff4j) { this.developerDao = developerDao; this.productManager = productManager; this.versionManager = versionManager; @@ -118,7 +126,9 @@ public DeveloperManager(DeveloperDAO developerDao, ProductManager productManager this.developerValidationFactory = developerValidationFactory; this.developerSearchRequestValidator = developerSearchRequestValidator; this.developerSearchRequestNormalizer = new SearchRequestNormalizer(); + this.cognitoUserManager = cognitoUserManager; this.schedulerManager = schedulerManager; + this.ff4j = ff4j; } @Transactional(readOnly = true) @@ -340,6 +350,47 @@ public Long create(Developer developer) throws ValidationException, EntityCreati return developerId; } + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).DELETE)") + @Transactional(readOnly = false) + @CacheEvict(value = { + CacheNames.ALL_DEVELOPERS, + CacheNames.ALL_DEVELOPERS_INCLUDING_DELETED, + CacheNames.COLLECTIONS_DEVELOPERS, + CacheNames.GET_DECERTIFIED_DEVELOPERS, + CacheNames.QUESTIONABLE_ACTIVITIES, + CacheNames.COLLECTIONS_LISTINGS, + CacheNames.COGNITO_USERS_BY_EMAIL, + CacheNames.COGNITO_USERS_BY_UUID + }, allEntries = true) + public void delete(Long developerId) throws EntityRetrievalException { + developerDao.delete(developerId); + + //The soft-delete should take care of any relationship between user-developer in the CHPL db + //when the developer is deleted. + //The below code is to remove permissions to the developer from any users who might have had them in Cognito + if (ff4j.check(FeatureList.SSO)) { + List usersOnDeveloper = resourcePermissionsFactory.get().getAllUsersOnDeveloper(Developer.builder().id(developerId).build()); + + usersOnDeveloper.stream() + .forEach(user -> { + Organization developerOrg = user.getOrganizations().stream() + .filter(org -> org.getId().equals(developerId)) + .findAny().orElse(null); + if (developerOrg != null) { + user.getOrganizations().remove(developerOrg); + try { + cognitoUserManager.updateUser(user); + } catch (Exception ex) { + LOGGER.error("Error removing user's permissions on developer organization ID " + developerId + " in Cognito", ex); + } + } else { + LOGGER.error("User " + user.getEmail() + " did not have permissions to developer organization " + developerId + ". No user update will take place."); + } + }); + } + } + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).JOIN)") @Transactional(readOnly = false) @@ -349,7 +400,9 @@ public Long create(Developer developer) throws ValidationException, EntityCreati CacheNames.COLLECTIONS_DEVELOPERS, CacheNames.GET_DECERTIFIED_DEVELOPERS, CacheNames.QUESTIONABLE_ACTIVITIES, - CacheNames.COLLECTIONS_LISTINGS + CacheNames.COLLECTIONS_LISTINGS, + CacheNames.COGNITO_USERS_BY_EMAIL, + CacheNames.COGNITO_USERS_BY_UUID }, allEntries = true) public ChplOneTimeTrigger join(Long owningDeveloperId, List joiningDeveloperIds) throws EntityRetrievalException, JsonProcessingException, EntityCreationException, diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/DeveloperDomainPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/DeveloperDomainPermissions.java index 9650e26ef0..1bc06eb03b 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/DeveloperDomainPermissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/DeveloperDomainPermissions.java @@ -5,6 +5,7 @@ import org.springframework.stereotype.Component; import gov.healthit.chpl.permissions.domains.developer.CreateActionPermissions; +import gov.healthit.chpl.permissions.domains.developer.DeleteActionPermissions; import gov.healthit.chpl.permissions.domains.developer.GetAllUsersActionPermissions; import gov.healthit.chpl.permissions.domains.developer.JoinActionPermissions; import gov.healthit.chpl.permissions.domains.developer.MessageActionPermissions; @@ -15,6 +16,7 @@ public class DeveloperDomainPermissions extends DomainPermissions { public static final String UPDATE = "UPDATE"; public static final String CREATE = "CREATE"; + public static final String DELETE = "DELETE"; public static final String JOIN = "JOIN"; public static final String SPLIT = "SPLIT"; public static final String GET_ALL_USERS = "GET_ALL_USERS"; @@ -24,6 +26,7 @@ public class DeveloperDomainPermissions extends DomainPermissions { public DeveloperDomainPermissions( @Qualifier("developerUpdateActionPermissions") UpdateActionPermissions updateActionPermissions, @Qualifier("developerCreateActionPermissions") CreateActionPermissions createActionPermissions, + @Qualifier("developerDeleteActionPermissions") DeleteActionPermissions deleteActionPermissions, @Qualifier("developerJoinActionPermissions") JoinActionPermissions joinActionPermissions, @Qualifier("developerSplitActionPermissions") SplitActionPermissions splitActionPermissions, @Qualifier("developerGetAllUsersActionPermissions") GetAllUsersActionPermissions getUsersActionPermissions, @@ -31,6 +34,7 @@ public DeveloperDomainPermissions( getActionPermissions().put(UPDATE, updateActionPermissions); getActionPermissions().put(CREATE, createActionPermissions); + getActionPermissions().put(DELETE, deleteActionPermissions); getActionPermissions().put(JOIN, joinActionPermissions); getActionPermissions().put(SPLIT, splitActionPermissions); getActionPermissions().put(GET_ALL_USERS, getUsersActionPermissions); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/developer/DeleteActionPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/developer/DeleteActionPermissions.java new file mode 100644 index 0000000000..42137acdf9 --- /dev/null +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/developer/DeleteActionPermissions.java @@ -0,0 +1,19 @@ +package gov.healthit.chpl.permissions.domains.developer; + +import org.springframework.stereotype.Component; + +import gov.healthit.chpl.permissions.domains.ActionPermissions; + +@Component("developerDeleteActionPermissions") +public class DeleteActionPermissions extends ActionPermissions { + + @Override + public boolean hasAccess() { + return getResourcePermissions().isUserRoleAdmin() || getResourcePermissions().isUserRoleOnc(); + } + + @Override + public boolean hasAccess(Object obj) { + return getResourcePermissions().isUserRoleAdmin() || getResourcePermissions().isUserRoleOnc(); + } +} diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/JoinDeveloperJob.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/JoinDeveloperJob.java index 25fb4e362b..0eaa86c53c 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/JoinDeveloperJob.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/JoinDeveloperJob.java @@ -128,8 +128,11 @@ private void clearCachesRelatedToDevelopers() { cacheManager.getCache(CacheNames.ALL_DEVELOPERS).invalidate(); cacheManager.getCache(CacheNames.ALL_DEVELOPERS_INCLUDING_DELETED).invalidate(); cacheManager.getCache(CacheNames.COLLECTIONS_DEVELOPERS).invalidate(); - cacheManager.getCache(CacheNames.COLLECTIONS_LISTINGS).invalidate(); cacheManager.getCache(CacheNames.GET_DECERTIFIED_DEVELOPERS).invalidate(); + cacheManager.getCache(CacheNames.QUESTIONABLE_ACTIVITIES).invalidate(); + cacheManager.getCache(CacheNames.COLLECTIONS_LISTINGS).invalidate(); + cacheManager.getCache(CacheNames.COGNITO_USERS_BY_EMAIL).invalidate(); + cacheManager.getCache(CacheNames.COGNITO_USERS_BY_UUID).invalidate(); } private void sendJobCompletionEmails(Developer developerJoined, List developersJoining, diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java index 87f44e014e..5b463c711f 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java @@ -19,7 +19,6 @@ import gov.healthit.chpl.certifiedproduct.CertifiedProductDetailsManager; import gov.healthit.chpl.compliance.directreview.DirectReviewUpdateEmailService; -import gov.healthit.chpl.dao.DeveloperDAO; import gov.healthit.chpl.domain.CertifiedProductSearchDetails; import gov.healthit.chpl.domain.Developer; import gov.healthit.chpl.domain.Product; @@ -32,6 +31,7 @@ import gov.healthit.chpl.exception.ValidationException; import gov.healthit.chpl.manager.ActivityManager; import gov.healthit.chpl.manager.CertifiedProductManager; +import gov.healthit.chpl.manager.DeveloperManager; import gov.healthit.chpl.manager.ProductManager; import gov.healthit.chpl.sharedstore.listing.ListingStoreRemove; import gov.healthit.chpl.sharedstore.listing.RemoveBy; @@ -56,7 +56,7 @@ public class TransactionalJoinDeveloperManager { private ActivityManager activityManager; @Autowired - private DeveloperDAO devDao; + private DeveloperManager developerManager; @Autowired private DirectReviewUpdateEmailService directReviewEmailService; @@ -107,7 +107,7 @@ public void join(List developersJoining, Developer developerToJoin) // mark the passed in developers as deleted for (Long developerId : developerIdsJoining) { - devDao.delete(developerId); + developerManager.delete(developerId); } logListingActivities(preJoinListingDetails, postJoinListingDetails); From 1108062844d610a1edbb759e356876d51d7bbd6c Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Tue, 24 Dec 2024 09:53:54 -0500 Subject: [PATCH 2/3] fix: Correctly add/remove orgs from SSO users on developer join [#OCD-4712] --- .../healthit/chpl/domain/Organization.java | 2 + .../gov/healthit/chpl/domain/auth/User.java | 4 ++ .../chpl/manager/DeveloperManager.java | 39 ++++++++++-------- .../CognitoResourcePermissions.java | 4 +- .../TransactionalJoinDeveloperManager.java | 2 +- .../chpl/user/cognito/CognitoUserManager.java | 40 ++++++++++++++++--- 6 files changed, 67 insertions(+), 24 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/Organization.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/Organization.java index cdc381c966..b86a07f9c6 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/Organization.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/Organization.java @@ -3,10 +3,12 @@ import java.io.Serializable; import gov.healthit.chpl.dto.OrganizationDTO; +import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; @NoArgsConstructor +@Builder @Data public class Organization implements Serializable { private static final long serialVersionUID = -5910873076481736684L; diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/User.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/User.java index d473c49638..3e14d6ce98 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/User.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/User.java @@ -40,4 +40,8 @@ public class User extends Person implements Serializable { private String hash; private String status; + public boolean hasRole(String authority) { + return role.equals(authority); + } + } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java index e914d236ed..fa91f5225e 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java @@ -363,32 +363,39 @@ public Long create(Developer developer) throws ValidationException, EntityCreati CacheNames.COGNITO_USERS_BY_EMAIL, CacheNames.COGNITO_USERS_BY_UUID }, allEntries = true) - public void delete(Long developerId) throws EntityRetrievalException { - developerDao.delete(developerId); - - //The soft-delete should take care of any relationship between user-developer in the CHPL db - //when the developer is deleted. + public void deleteDeveloperForJoin(Long developerIdToDelete, Developer developerToJoin) throws EntityRetrievalException { //The below code is to remove permissions to the developer from any users who might have had them in Cognito + //and add permissions for the users to belong to the joined developer if (ff4j.check(FeatureList.SSO)) { - List usersOnDeveloper = resourcePermissionsFactory.get().getAllUsersOnDeveloper(Developer.builder().id(developerId).build()); + List usersOnDeveloper = resourcePermissionsFactory.get().getAllUsersOnDeveloper( + Developer.builder().id(developerIdToDelete).build()); usersOnDeveloper.stream() .forEach(user -> { - Organization developerOrg = user.getOrganizations().stream() - .filter(org -> org.getId().equals(developerId)) + Organization developerOrgToDelete = user.getOrganizations().stream() + .filter(org -> org.getId().equals(developerIdToDelete)) .findAny().orElse(null); - if (developerOrg != null) { - user.getOrganizations().remove(developerOrg); - try { - cognitoUserManager.updateUser(user); - } catch (Exception ex) { - LOGGER.error("Error removing user's permissions on developer organization ID " + developerId + " in Cognito", ex); - } + if (developerOrgToDelete != null) { + user.getOrganizations().remove(developerOrgToDelete); } else { - LOGGER.error("User " + user.getEmail() + " did not have permissions to developer organization " + developerId + ". No user update will take place."); + LOGGER.error("User " + user.getEmail() + " did not have permissions to developer organization " + developerIdToDelete); + } + Organization developerOrgToJoin = Organization.builder() + .id(developerToJoin.getId()) + .name(developerToJoin.getName()) + .build(); + user.getOrganizations().add(developerOrgToJoin); + try { + cognitoUserManager.updateUser(user); + } catch (Exception ex) { + LOGGER.error("Error removing user's permissions on developer organization ID " + developerIdToDelete + " in Cognito", ex); } }); } + + //The delete is last because if the developer is marked as deleted + //we have trouble finding it's users. + developerDao.delete(developerIdToDelete); } @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java index e116506917..6611f969d0 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java @@ -254,7 +254,7 @@ private CertificationBody getCertifcationBody(Long certificationBodyId) { try { return certificationBodyDAO.getById(certificationBodyId); } catch (EntityRetrievalException e) { - LOGGER.error("Could not retrieve Certification Body: {}", certificationBodyId); + LOGGER.error("Could not retrieve Certification Body: {}", certificationBodyId, e); return null; } } @@ -263,7 +263,7 @@ private Developer getDeveloper(Long developerId) { try { return developerDAO.getById(developerId); } catch (EntityRetrievalException e) { - LOGGER.error("Could not retrieve Developer: {}", developerId); + LOGGER.error("Could not retrieve Developer: {}", developerId, e); return null; } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java index 5b463c711f..3cc0878839 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/TransactionalJoinDeveloperManager.java @@ -107,7 +107,7 @@ public void join(List developersJoining, Developer developerToJoin) // mark the passed in developers as deleted for (Long developerId : developerIdsJoining) { - developerManager.delete(developerId); + developerManager.deleteDeveloperForJoin(developerId, developerToJoin); } logListingActivities(preJoinListingDetails, postJoinListingDetails); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java index 3812d3d0a6..9fd83a4b9e 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java @@ -1,5 +1,7 @@ package gov.healthit.chpl.user.cognito; +import static gov.healthit.chpl.util.LambdaExceptionUtil.rethrowConsumer; + import java.util.List; import java.util.Set; import java.util.UUID; @@ -102,15 +104,31 @@ public User updateUser(User user) throws ValidationException, UserRetrievalExcep User originalUser = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); cognitoApiWrapper.updateUser(user); - //check for organizations to remove (organizations are never added here, only removed) + //check for organizations to remove List removedOrganizationIds = getRemovedOrganizationIds(originalUser, user); if (!CollectionUtils.isEmpty(removedOrganizationIds)) { + LOGGER.info("Removing " + removedOrganizationIds + " access from user " + originalUser.getEmail()); cognitoApiWrapper.removeOrgsFromUser(originalUser, removedOrganizationIds); } - if (userShouldBeDisabled(originalUser, user)) { + //check for organizations to add (this only happens in the case of a join): + //Developer A is joining Developer B. All the users of Developer A + //have access removed from Developer A in the code block above but now will be given Developer B. + //Developer A gets deleted. + User userAfterRemovingOrgs = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); + List addedOrganizationIds = getAddedOrganizationIds(userAfterRemovingOrgs, user); + if (!CollectionUtils.isEmpty(addedOrganizationIds)) { + addedOrganizationIds.stream() + .forEach(rethrowConsumer(orgId -> { + LOGGER.info("Adding " + orgId + " access to user " + userAfterRemovingOrgs.getEmail()); + cognitoApiWrapper.addOrgToUser(userAfterRemovingOrgs, orgId); + })); + } + + User userAfterRemovingAndAddingOrgs = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); + if (userShouldBeDisabled(userAfterRemovingAndAddingOrgs, user)) { cognitoApiWrapper.disableUser(user); - } else if (userShouldBeEnabled(originalUser, user)) { + } else if (userShouldBeEnabled(userAfterRemovingAndAddingOrgs, user)) { cognitoApiWrapper.enableUser(user); } @@ -131,6 +149,16 @@ private List getRemovedOrganizationIds(User originalUser, User updatedUser return null; } + private List getAddedOrganizationIds(User originalUser, User updatedUser) { + List addedOrgs = subtractLists(updatedUser.getOrganizations(), originalUser.getOrganizations()); + if (!CollectionUtils.isEmpty(addedOrgs)) { + return addedOrgs.stream() + .map(org -> org.getId()) + .collect(Collectors.toList()); + } + return null; + } + private List subtractLists(List listA, List listB) { Predicate notInListB = orgFromA -> !listB.stream() .anyMatch(orgFromB -> orgFromA.getId().equals(orgFromB.getId())); @@ -146,10 +174,12 @@ private boolean userShouldBeDisabled(User originalUser, User updatedUser) { //They should also be disabled if the update to the specifically went from enabled to disabled. if (originalUser.getAccountEnabled() && !updatedUser.getAccountEnabled()) { return true; - } else if (!CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(originalUser)) + } else if (originalUser.hasRole(CognitoGroups.CHPL_ACB) + && !CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(originalUser)) && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(updatedUser))) { return true; - } else if (!CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(originalUser)) + } else if (originalUser.hasRole(CognitoGroups.CHPL_DEVELOPER) + && !CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(originalUser)) && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(updatedUser))) { return true; } From f9005f5a3e82313af77b76e4d090f351be018a55 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Thu, 26 Dec 2024 16:19:30 -0500 Subject: [PATCH 3/3] feat!: Disable all users from orig developer on a split The logic we decided to implement is to disable all the users from Developer A when Developer A is split into Developer A (remains, unchanged) and Developer B. Any users that need to have permissions on either Developer A or Developer B need to be manually (re)invited. [#OCD-4712] --- .../src/main/resources/jobs.xml | 4 +- .../chpl/manager/DeveloperManager.java | 38 +++++++++++++++++++ .../chpl/scheduler/job/SplitDeveloperJob.java | 24 +++++++++++- .../chpl/user/cognito/CognitoUserManager.java | 16 ++++---- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/chpl/chpl-resources/src/main/resources/jobs.xml b/chpl/chpl-resources/src/main/resources/jobs.xml index 8e4c099580..fe80369309 100644 --- a/chpl/chpl-resources/src/main/resources/jobs.xml +++ b/chpl/chpl-resources/src/main/resources/jobs.xml @@ -903,7 +903,7 @@ true false - + cognitoMassRequirePasswordChangeJob systemJobs @@ -912,7 +912,7 @@ true false - + updateSedFriendlyIdsJob systemJobs diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java index fa91f5225e..08fe9cce06 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java @@ -442,6 +442,44 @@ public ChplOneTimeTrigger join(Long owningDeveloperId, List joiningDevelop return joinDevelopersTrigger; } + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).SPLIT, #oldDeveloper)") + @Transactional(readOnly = false) + @CacheEvict(value = { + CacheNames.ALL_DEVELOPERS, + CacheNames.ALL_DEVELOPERS_INCLUDING_DELETED, + CacheNames.COLLECTIONS_DEVELOPERS, + CacheNames.GET_DECERTIFIED_DEVELOPERS, + CacheNames.QUESTIONABLE_ACTIVITIES, + CacheNames.COLLECTIONS_LISTINGS, + CacheNames.COGNITO_USERS_BY_EMAIL, + CacheNames.COGNITO_USERS_BY_UUID + }, allEntries = true) + public void removeUsersForDeveloperSplit(Developer oldDeveloper) throws EntityRetrievalException { + //The below code is to remove permissions to the developer from any users who might have had them in Cognito + if (ff4j.check(FeatureList.SSO)) { + List usersOnDeveloper = resourcePermissionsFactory.get().getAllUsersOnDeveloper( + Developer.builder().id(oldDeveloper.getId()).build()); + + usersOnDeveloper.stream() + .forEach(user -> { + Organization developerOrgToDelete = user.getOrganizations().stream() + .filter(org -> org.getId().equals(oldDeveloper.getId())) + .findAny().orElse(null); + if (developerOrgToDelete != null) { + user.getOrganizations().remove(developerOrgToDelete); + } else { + LOGGER.error("User " + user.getEmail() + " did not have permissions to developer organization " + oldDeveloper.getId()); + } + try { + cognitoUserManager.updateUser(user); + } catch (Exception ex) { + LOGGER.error("Error removing user's permissions on developer organization ID " + oldDeveloper.getId() + " in Cognito", ex); + } + }); + } + } + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).SPLIT, #oldDeveloper)") @CacheEvict(value = { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/SplitDeveloperJob.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/SplitDeveloperJob.java index 7d9f95ad62..7c58ddc1ba 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/SplitDeveloperJob.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/SplitDeveloperJob.java @@ -13,6 +13,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.ff4j.FF4j; import org.quartz.DisallowConcurrentExecution; import org.quartz.JobDataMap; import org.quartz.JobExecutionContext; @@ -27,6 +28,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; +import gov.healthit.chpl.FeatureList; import gov.healthit.chpl.auth.user.JWTAuthenticatedUser; import gov.healthit.chpl.caching.CacheNames; import gov.healthit.chpl.caching.ListingSearchCacheRefresh; @@ -90,6 +92,9 @@ public class SplitDeveloperJob extends QuartzJob { @Autowired private ChplHtmlEmailBuilder emailBuilder; + @Autowired + private FF4j ff4j; + @Value("${internalErrorEmailRecipients}") private String internalErrorEmailRecipients; @@ -117,6 +122,7 @@ public void execute(JobExecutionContext jobContext) throws JobExecutionException Exception splitException = null; try { postSplitDeveloper = splitDeveloper(newDeveloper, productIdsToMove); + devManager.removeUsersForDeveloperSplit(preSplitDeveloper); } catch (Exception e) { LOGGER.error("Error completing split of old developer '" + preSplitDeveloper.getName() + "' to new developer '" + newDeveloper.getName() + "'.", e); @@ -251,8 +257,11 @@ private void clearCachesRelatedToDevelopers() { cacheManager.getCache(CacheNames.ALL_DEVELOPERS).invalidate(); cacheManager.getCache(CacheNames.ALL_DEVELOPERS_INCLUDING_DELETED).invalidate(); cacheManager.getCache(CacheNames.COLLECTIONS_DEVELOPERS).invalidate(); - cacheManager.getCache(CacheNames.COLLECTIONS_LISTINGS).invalidate(); cacheManager.getCache(CacheNames.GET_DECERTIFIED_DEVELOPERS).invalidate(); + cacheManager.getCache(CacheNames.QUESTIONABLE_ACTIVITIES).invalidate(); + cacheManager.getCache(CacheNames.COLLECTIONS_LISTINGS).invalidate(); + cacheManager.getCache(CacheNames.COGNITO_USERS_BY_EMAIL).invalidate(); + cacheManager.getCache(CacheNames.COGNITO_USERS_BY_UUID).invalidate(); } private void sendJobCompletionEmails(Developer newDeveloper, List productIds, @@ -325,10 +334,23 @@ private String createHtmlEmailBodySuccess(String title, Developer createdDevelop } productList += ""; + String userReminder = ""; + if (ff4j.check(FeatureList.SSO)) { + userReminder = String.format("All users associated with %s have had their access removed " + + "and may have been disabled. Users will need to be manually re-invited with the " + + "correct organization access.", + preSplitDeveloper.getName()); + } else { + userReminder = String.format("User access to the developers %s and %s must be manually updated.", + preSplitDeveloper.getName(), + createdDeveloper.getName()); + } + String htmlMessage = emailBuilder.initialize() .heading(title) .paragraph(null, summaryText) .paragraph(null, productList) + .paragraph(null, userReminder) .footer(AdminFooter.class) .build(); return htmlMessage; diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java index 9fd83a4b9e..b042e4fffb 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java @@ -126,10 +126,10 @@ public User updateUser(User user) throws ValidationException, UserRetrievalExcep } User userAfterRemovingAndAddingOrgs = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); - if (userShouldBeDisabled(userAfterRemovingAndAddingOrgs, user)) { - cognitoApiWrapper.disableUser(user); - } else if (userShouldBeEnabled(userAfterRemovingAndAddingOrgs, user)) { - cognitoApiWrapper.enableUser(user); + if (userShouldBeDisabled(originalUser, userAfterRemovingAndAddingOrgs)) { + cognitoApiWrapper.disableUser(userAfterRemovingAndAddingOrgs); + } else if (userShouldBeEnabled(originalUser, userAfterRemovingAndAddingOrgs)) { + cognitoApiWrapper.enableUser(userAfterRemovingAndAddingOrgs); } User updatedUser = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); @@ -175,12 +175,12 @@ private boolean userShouldBeDisabled(User originalUser, User updatedUser) { if (originalUser.getAccountEnabled() && !updatedUser.getAccountEnabled()) { return true; } else if (originalUser.hasRole(CognitoGroups.CHPL_ACB) - && !CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(originalUser)) - && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(updatedUser))) { + && !CollectionUtils.isEmpty(originalUser.getOrganizations()) + && CollectionUtils.isEmpty(updatedUser.getOrganizations())) { return true; } else if (originalUser.hasRole(CognitoGroups.CHPL_DEVELOPER) - && !CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(originalUser)) - && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(updatedUser))) { + && !CollectionUtils.isEmpty(originalUser.getOrganizations()) + && CollectionUtils.isEmpty(updatedUser.getOrganizations())) { return true; } return false;