diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java index 3596c32c8e..51b0d54f28 100644 --- a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java +++ b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java @@ -2,9 +2,7 @@ import java.util.List; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; -import org.apache.commons.lang3.StringUtils; import org.quartz.SchedulerException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; @@ -35,8 +33,6 @@ import gov.healthit.chpl.util.SwaggerSecurityRequirement; import gov.healthit.chpl.web.controller.results.CertificationBodyResults; import io.swagger.v3.oas.annotations.Operation; -import io.swagger.v3.oas.annotations.Parameter; -import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -245,15 +241,8 @@ public String deleteUserFromAcb(@PathVariable final Long acbId, @PathVariable fi }) @RequestMapping(value = "/{acbId}/users", method = RequestMethod.GET, produces = "application/json; charset=utf-8") - public @ResponseBody UsersResponse getUsers(@PathVariable("acbId") Long acbId, - @Parameter(description = "Whether to include users whose accounts have been marked as disabled. " - + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). " - + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.", - allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled") - @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabled) - throws InvalidArgumentsException, EntityRetrievalException { - List users = acbManager.getUsers(acbId, - StringUtils.isEmpty(includeDisabled) ? false : BooleanUtils.toBoolean(includeDisabled)); + public @ResponseBody UsersResponse getUsers(@PathVariable("acbId") Long acbId) throws InvalidArgumentsException, EntityRetrievalException { + List users = acbManager.getUsers(acbId); UsersResponse results = new UsersResponse(); results.setUsers(users); diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java index b4a900f71b..ae10044ac8 100644 --- a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java +++ b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java @@ -4,9 +4,7 @@ import java.util.List; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.NotImplementedException; -import org.apache.commons.lang3.StringUtils; import org.ff4j.FF4j; import org.quartz.SchedulerException; import org.springframework.beans.factory.annotation.Autowired; @@ -18,7 +16,6 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; @@ -60,8 +57,6 @@ import gov.healthit.chpl.web.controller.results.DeveloperAttestationSubmissionResults; import gov.healthit.chpl.web.controller.results.DeveloperResults; import io.swagger.v3.oas.annotations.Operation; -import io.swagger.v3.oas.annotations.Parameter; -import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; import lombok.extern.log4j.Log4j2; @@ -309,15 +304,9 @@ public PermissionDeletedResponse deleteUserFromDeveloper(@PathVariable Long deve }) @RequestMapping(value = "/{developerId}/users", method = RequestMethod.GET, produces = "application/json; charset=utf-8") - public @ResponseBody UsersResponse getUsers(@PathVariable("developerId") Long developerId, - @Parameter(description = "Whether to include users whose accounts have been marked as disabled. " - + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). " - + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.", - allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled") - @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabled) - throws InvalidArgumentsException, EntityRetrievalException { - List domainUsers = developerManager.getAllUsersOnDeveloper(developerId, - StringUtils.isEmpty(includeDisabled) ? false : BooleanUtils.toBoolean(includeDisabled)); + public @ResponseBody UsersResponse getUsers(@PathVariable("developerId") Long developerId) + throws InvalidArgumentsException, EntityRetrievalException { + List domainUsers = developerManager.getAllUsersOnDeveloper(developerId); UsersResponse results = new UsersResponse(); results.setUsers(domainUsers); return results; diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java index 6f5f5a8331..2ed0e742b7 100644 --- a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java +++ b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java @@ -7,7 +7,6 @@ import java.util.Set; import java.util.UUID; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.ff4j.FF4j; @@ -22,7 +21,6 @@ import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; @@ -64,8 +62,6 @@ import gov.healthit.chpl.web.controller.annotation.DeprecatedApi; import gov.healthit.chpl.web.controller.annotation.DeprecatedApiResponseFields; import io.swagger.v3.oas.annotations.Operation; -import io.swagger.v3.oas.annotations.Parameter; -import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; import lombok.Getter; @@ -248,16 +244,16 @@ public void addUser(@RequestBody CreateUserFromInvitationRequest userInfo) throw method = RequestMethod.PUT, consumes = MediaType.APPLICATION_JSON_VALUE, produces = "application/json; charset=utf-8") - public User updateUserDetails(@RequestBody User userInfo, @PathVariable("cognitoUserId") UUID cognitoUserId) + public User updateUserDetails(@RequestBody User user, @PathVariable("cognitoUserId") UUID cognitoUserId) throws ValidationException, UserRetrievalException, ActivityException { if (!ff4j.check(FeatureList.SSO)) { throw new NotImplementedException("This method has not been implemented"); } - if (!cognitoUserId.equals(userInfo.getCognitoId())) { + if (!cognitoUserId.equals(user.getCognitoId())) { throw new ValidationException(msgUtil.getMessage("url.body.notMatch")); } - return cognitoUserManager.updateUser(userInfo); + return cognitoUserManager.updateUser(user); } @@ -527,18 +523,12 @@ public DeletedUser deleteUser(@PathVariable("userId") Long userId) throws UserRe }) @RequestMapping(value = "", method = RequestMethod.GET, produces = "application/json; charset=utf-8") @PreAuthorize("isAuthenticated()") - public @ResponseBody UsersResponse getUsers( - @Parameter(description = "Whether to include users whose accounts have been marked as disabled. " - + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). " - + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.", - allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled") - @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabledStr) { - boolean includeDisabled = StringUtils.isEmpty(includeDisabledStr) ? false : BooleanUtils.toBoolean(includeDisabledStr); + public @ResponseBody UsersResponse getUsers() { List users = null; if (ff4j.check(FeatureList.SSO)) { - users = getAllCognitoUsers(includeDisabled); + users = getAllCognitoUsers(); } else { - users = getAllChplUsers(includeDisabled); + users = getAllChplUsers(); } UsersResponse response = new UsersResponse(); @@ -566,8 +556,8 @@ public DeletedUser deleteUser(@PathVariable("userId") Long userId) throws UserRe return userManager.getUserInfo(id); } - private List getAllChplUsers(Boolean includeDisabled) { - List userList = userManager.getAll(includeDisabled); + private List getAllChplUsers() { + List userList = userManager.getAll(); List users = new ArrayList(userList.size()); for (UserDTO userDto : userList) { @@ -577,8 +567,8 @@ private List getAllChplUsers(Boolean includeDisabled) { return users; } - private List getAllCognitoUsers(Boolean includeDisabled) { - return cognitoUserManager.getAll(includeDisabled); + private List getAllCognitoUsers() { + return cognitoUserManager.getAll(); } private class DeletedUser { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/ChplJWTUserConverter.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/ChplJWTUserConverter.java index ca52958a70..eda6cbf228 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/ChplJWTUserConverter.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/ChplJWTUserConverter.java @@ -45,6 +45,7 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio try { validatedClaims = jwtConsumer.consume(jwt); } catch (InvalidJwtException e) { + LOGGER.error("Invalid JWT Exception", e); throw new JWTValidationException("Invalid authentication token."); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/CognitoJwtUserConverter.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/CognitoJwtUserConverter.java index e21c1a9667..d65142cbc5 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/CognitoJwtUserConverter.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/CognitoJwtUserConverter.java @@ -63,9 +63,11 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio throw new JWTValidationException("Invalid authentication token."); } } catch (JWTValidationException e) { + LOGGER.error("JWT Validation Exception", e); throw e; } catch (Exception e) { + LOGGER.error("Error decoding JWT token", e); return null; } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertificationBodyManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertificationBodyManager.java index 04f57ae83f..e5d4c3e8e3 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertificationBodyManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertificationBodyManager.java @@ -210,18 +210,13 @@ public CertificationBody getById(Long id) throws EntityRetrievalException { @Transactional(readOnly = true) @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).CERTIFICATION_BODY, " + "T(gov.healthit.chpl.permissions.domains.CertificationBodyDomainPermissions).GET_USERS, #acbId)") - public List getUsers(Long acbId, boolean includeDisabled) - throws InvalidArgumentsException, EntityRetrievalException { + public List getUsers(Long acbId) throws InvalidArgumentsException, EntityRetrievalException { CertificationBody acb = resourcePermissionsFactory.get().getAcbIfPermissionById(acbId); if (acb == null) { throw new InvalidArgumentsException("Could not find the ACB specified."); } - if (!resourcePermissionsFactory.get().isUserRoleAdmin() - && !resourcePermissionsFactory.get().isUserRoleOnc()) { - includeDisabled = false; - } - List users = resourcePermissionsFactory.get().getAllUsersOnAcb(acb, includeDisabled); + List users = resourcePermissionsFactory.get().getAllUsersOnAcb(acb); return users; } 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 fe1b92635a..2663faface 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 @@ -245,13 +245,9 @@ private SimpleListing convertToSimpleListing(CertifiedProductDetailsDTO listingD @Transactional(readOnly = true) @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS, #devId)") - public List getAllUsersOnDeveloper(Long devId, boolean includeDisabled) throws EntityRetrievalException { + public List getAllUsersOnDeveloper(Long devId) throws EntityRetrievalException { Developer dev = getById(devId); - if (!resourcePermissionsFactory.get().isUserRoleAdmin() - && !resourcePermissionsFactory.get().isUserRoleOnc()) { - includeDisabled = false; - } - List users = resourcePermissionsFactory.get().getAllUsersOnDeveloper(dev, includeDisabled); + List users = resourcePermissionsFactory.get().getAllUsersOnDeveloper(dev); return users; } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java index 2b6985c300..552401d37d 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java @@ -9,9 +9,7 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; @@ -170,18 +168,8 @@ public void delete(UserDTO user) throws UserRetrievalException, ActivityExceptio @Transactional @PostFilter("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).SECURED_USER, " + "T(gov.healthit.chpl.permissions.domains.SecuredUserDomainPermissions).GET_ALL, filterObject)") - public List getAll(Boolean includeDisabled) { - if (includeDisabled == null - || (!resourcePermissionsFactory.get().isUserRoleAdmin() - && !resourcePermissionsFactory.get().isUserRoleOnc())) { - includeDisabled = false; - } + public List getAll() { List allUsers = userDAO.findAll(); - if (includeDisabled == null || BooleanUtils.isFalse(includeDisabled)) { - return allUsers.stream() - .filter(user -> user.isAccountEnabled()) - .collect(Collectors.toList()); - } return allUsers; } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java index f47a82c009..ea09c17972 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java @@ -68,34 +68,20 @@ public boolean isDeveloperNotBannedOrSuspended(Long developerId) { @Override @Transactional(readOnly = true) public List getAllUsersOnAcb(CertificationBody acb) { - return getAllUsersOnAcb(acb, false); - } - - @Override - @Transactional(readOnly = true) - public List getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled) { List dtos = userCertificationBodyMapDAO.getByAcbId(acb.getId()); return dtos.stream() .map(dto -> dto.getUser().toDomain()) - .filter(user -> includeDisabled ? true : user.getAccountEnabled()) .toList(); } @Override @Transactional(readOnly = true) public List getAllUsersOnDeveloper(Developer dev) { - return getAllUsersOnDeveloper(dev, false); - } - - @Override - @Transactional(readOnly = true) - public List getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) { List dtos = userDeveloperMapDAO.getByDeveloperId(dev.getId()); return dtos.stream() .map(udm -> udm.getUser().toDomain()) - .filter(user -> includeDisabled ? true : user.getAccountEnabled()) .toList(); } @@ -104,30 +90,15 @@ public List getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS)") public List getAllDeveloperUsers() { - return getAllDeveloperUsers(false); - } - - @Override - @Transactional(readOnly = true) - @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, " - + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS)") - public List getAllDeveloperUsers(boolean includeDisabled) { List dtos = userDeveloperMapDAO.getAllDeveloperUsers(); return dtos.stream() .map(udm -> udm.getUser().toDomain()) - .filter(user -> includeDisabled ? true : user.getAccountEnabled()) .toList(); } @Override @Transactional(readOnly = true) public List getAllUsersForCurrentUser() { - return getAllUsersForCurrentUser(false); - } - - @Override - @Transactional(readOnly = true) - public List getAllUsersForCurrentUser(boolean includeDisabled) { JWTAuthenticatedUser user = AuthUtil.getCurrentUser(); List users = new ArrayList(); @@ -155,9 +126,7 @@ public List getAllUsersForCurrentUser(boolean includeDisabled) { } catch (UserRetrievalException ex) { } } } - return users.stream() - .filter(currUser -> includeDisabled ? true : currUser.getAccountEnabled()) - .collect(Collectors.toList()); + return users; } @Override 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 4e75dc524c..e116506917 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 @@ -54,12 +54,7 @@ public boolean isDeveloperNotBannedOrSuspended(Long developerId) { @Override public List getAllUsersOnAcb(CertificationBody acb) { - return getAllUsersOnAcb(acb, false); - } - - @Override - public List getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled) { - List allUsersOnAcb = cognitoApiWrapper.getAllUsers(includeDisabled).stream() + List allUsersOnAcb = cognitoApiWrapper.getAllUsers().stream() .filter(user -> user.getRole() != null && user.getRole().equals(CognitoGroups.CHPL_ACB) && user.getOrganizations().stream() @@ -73,12 +68,7 @@ public List getAllUsersOnAcb(CertificationBody acb, boolean includeDisable @Override public List getAllUsersOnDeveloper(Developer dev) { - return getAllUsersOnDeveloper(dev, false); - } - - @Override - public List getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) { - List allUsersOnDeveloper = cognitoApiWrapper.getAllUsers(includeDisabled).stream() + List allUsersOnDeveloper = cognitoApiWrapper.getAllUsers().stream() .filter(user -> user.getRole() != null && user.getRole().equals(CognitoGroups.CHPL_DEVELOPER) && user.getOrganizations().stream() @@ -92,12 +82,7 @@ public List getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) @Override public List getAllDeveloperUsers() { - return getAllDeveloperUsers(false); - } - - @Override - public List getAllDeveloperUsers(boolean includeDisabled) { - List allDeveloperUsers = cognitoApiWrapper.getAllUsers(includeDisabled).stream() + List allDeveloperUsers = cognitoApiWrapper.getAllUsers().stream() .filter(user -> user.getRole() != null && user.getRole().equals(CognitoGroups.CHPL_DEVELOPER)) .collect(Collectors.toList()); @@ -107,11 +92,6 @@ public List getAllDeveloperUsers(boolean includeDisabled) { @Override public List getAllUsersForCurrentUser() { - return getAllUsersForCurrentUser(false); - } - - @Override - public List getAllUsersForCurrentUser(boolean includeDisabled) { LOGGER.error("Not implemented: getAllUsersForCurrentUser"); throw new NotImplementedException("Not implemented: getAllUsersForCurrentUser"); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ResourcePermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ResourcePermissions.java index 1e288f76e2..ff0050edc0 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ResourcePermissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ResourcePermissions.java @@ -14,20 +14,12 @@ public interface ResourcePermissions { List getAllUsersOnAcb(CertificationBody acb); - List getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled); - List getAllUsersOnDeveloper(Developer dev); - List getAllUsersOnDeveloper(Developer dev, boolean includeDisabled); - List getAllDeveloperUsers(); - List getAllDeveloperUsers(boolean includeDisabled); - List getAllUsersForCurrentUser(); - List getAllUsersForCurrentUser(boolean includeDisabled); - List getAllAcbsForCurrentUser(); List getAllAcbsForUser(User user); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java index cb03254132..b761ef4afe 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java @@ -57,7 +57,7 @@ public void execute(JobExecutionContext jobContext) throws JobExecutionException String jwt = authenticationManager.getJWT(actor); JWTAuthenticatedUser authenticatedUser = userConverterFacade.getAuthenticatedUser(jwt); SecurityContextHolder.getContext().setAuthentication(authenticatedUser); - List allUsers = userManager.getAll(false); + List allUsers = userManager.getAll(); for (UserDTO user : allUsers) { if (interrupted) { LOGGER.info("Interrupted while marking users as password change required"); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/attestation/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/attestation/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java index 298c8a1b3a..d60a85d906 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/attestation/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/attestation/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java @@ -48,7 +48,7 @@ public MissingAttestationChangeRequestDeveloperEmailGenerator(DeveloperManager d public DeveloperEmail getDeveloperEmail(DeveloperSearchResult developer, User submittedUser) { try { - List developerUsers = developerManager.getAllUsersOnDeveloper(developer.getId(), false); + List developerUsers = developerManager.getAllUsersOnDeveloper(developer.getId()); return DeveloperEmail.builder() .developer(developer) .recipients(getRecipients(developerUsers)) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/messaging/MessageDevelopersJob.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/messaging/MessageDevelopersJob.java index 9f64ad2924..132cd947fa 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/messaging/MessageDevelopersJob.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/developer/messaging/MessageDevelopersJob.java @@ -85,7 +85,9 @@ public void execute(JobExecutionContext context) throws JobExecutionException { LOGGER); LOGGER.info("Messaging " + developersToMessage.size() + " developers."); - List enabledDeveloperUsers = resourcePermissionsFactory.get().getAllDeveloperUsers(); + List enabledDeveloperUsers = resourcePermissionsFactory.get().getAllDeveloperUsers().stream() + .filter(devUser -> devUser.getAccountEnabled() != null && devUser.getAccountEnabled()) + .collect(Collectors.toList()); List developersWithoutUsers = getDevelopersWithoutUsers(developersToMessage, enabledDeveloperUsers); List developerEmails = developersToMessage.stream() .map(developer -> messageGenerator.getDeveloperEmail(developer, developerMessageRequest, enabledDeveloperUsers)) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoApiWrapper.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoApiWrapper.java index 4c4f5c8efd..9e36e313a1 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoApiWrapper.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoApiWrapper.java @@ -81,6 +81,8 @@ @Component public class CognitoApiWrapper { private static final String HMAC_SHA256_ALGORITHM = "HmacSHA256"; + private static final String ORGANIZATIONS_ATTRIBUTE_NAME = "custom:organizations"; + private static final String FORCE_PASSWORD_RESET_ATTRIBUTE_NAME = "custom:forcePasswordReset"; private String clientId; private String userPoolId; @@ -89,12 +91,14 @@ public class CognitoApiWrapper { private CognitoIdentityProviderClient cognitoClient; private CertificationBodyDAO certificationBodyDAO; private DeveloperDAO developerDAO; + private CertificationBodyDAO acbDao; @Autowired public CognitoApiWrapper(@Value("${cognito.accessKey}") String accessKey, @Value("${cognito.secretKey}") String secretKey, @Value("${cognito.region}") String region, @Value("${cognito.clientId}") String clientId, @Value("${cognito.userPoolId}") String userPoolId, @Value("${cognito.userPoolClientSecret}") String userPoolClientSecret, @Value("${cognito.environment.groupName}") String environmentGroupName, - CertificationBodyDAO certificationBodyDAO, DeveloperDAO developerDAO) { + CertificationBodyDAO certificationBodyDAO, DeveloperDAO developerDAO, + CertificationBodyDAO acbDao) { cognitoClient = createCognitoClient(accessKey, secretKey, region); this.clientId = clientId; @@ -103,6 +107,7 @@ public CognitoApiWrapper(@Value("${cognito.accessKey}") String accessKey, @Value this.userPoolClientSecret = userPoolClientSecret; this.certificationBodyDAO = certificationBodyDAO; this.developerDAO = developerDAO; + this.acbDao = acbDao; } public AuthenticationResultType authenticate(LoginCredentials credentials) throws CognitoAuthenticationChallengeException { @@ -180,8 +185,14 @@ public User getUserInfo(UUID cognitoId) throws UserRetrievalException { if (response == null || response.sdkHttpResponse() == null || !response.sdkHttpResponse().isSuccessful()) { return null; } - return createUserFromGetUserResponse(response); + User user = createUserFromGetUserResponse(response); + List groupsForUser = getGroupsForUser(user.getEmail()); + if (!doesGroupMatchCurrentEnvironment(groupsForUser)) { + return null; + } + return user; } catch (Exception e) { + LOGGER.error("Unable to get user " + cognitoId.toString() + " with AdminGetUserRequest", e); return null; } } @@ -197,7 +208,13 @@ public User getUserInfo(String email) throws UserRetrievalException { if (response == null || response.sdkHttpResponse() == null || !response.sdkHttpResponse().isSuccessful()) { return null; } - return createUserFromGetUserResponse(response); + + User user = createUserFromGetUserResponse(response); + List groupsForUser = getGroupsForUser(user.getEmail()); + if (!doesGroupMatchCurrentEnvironment(groupsForUser)) { + return null; + } + return user; } catch (Exception e) { return null; } @@ -214,7 +231,13 @@ public User getUserNoCache(UUID cognitoId) throws UserRetrievalException { if (response == null || response.sdkHttpResponse() == null || !response.sdkHttpResponse().isSuccessful()) { return null; } - return createUserFromGetUserResponse(response); + + User user = createUserFromGetUserResponse(response); + List groupsForUser = getGroupsForUser(user.getEmail()); + if (!doesGroupMatchCurrentEnvironment(groupsForUser)) { + return null; + } + return user; } public CognitoCredentials createUser(CreateUserRequest userRequest) throws UserCreationException { @@ -227,7 +250,7 @@ public CognitoCredentials createUser(CreateUserRequest userRequest) throws UserC .userAttributes( AttributeType.builder().name("name").value(userRequest.getFullName()).build(), AttributeType.builder().name("email").value(userRequest.getEmail()).build(), - AttributeType.builder().name("custom:organizations").value( + AttributeType.builder().name(ORGANIZATIONS_ATTRIBUTE_NAME).value( userRequest.getOrganizationId() != null ? userRequest.getOrganizationId().toString() : "").build()) .temporaryPassword(tempPassword) .messageAction(MessageActionType.SUPPRESS) @@ -245,6 +268,28 @@ public CognitoCredentials createUser(CreateUserRequest userRequest) throws UserC } } + @Caching(evict = { + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_UUID, key = "#existingUser.cognitoId"), + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_EMAIL, key = "#existingUser.email") + }) + public CognitoCredentials reenableUser(User existingUser) throws UserCreationException { + try { + enableUser(existingUser); + updateUser(existingUser); + + String tempPassword = PasswordUtil.generatePassword(); + setUserPassword(existingUser.getEmail(), tempPassword, false); + + return CognitoCredentials.builder() + .cognitoId(existingUser.getCognitoId()) + .userName(existingUser.getEmail()) + .password(tempPassword) + .build(); + } catch (Exception e) { + throw new UserCreationException(String.format("Error re-enabling user with email %s in store.", existingUser.getEmail()), e); + } + } + public AuthenticationResultType refreshToken(String refreshToken, UUID cognitoId) { Map authParams = new LinkedHashMap(); authParams.put("REFRESH_TOKEN", refreshToken); @@ -276,12 +321,14 @@ public void setUserPassword(String userName, String password, Boolean permanent) cognitoClient.adminSetUserPassword(request); - try { - User user = getUserInfo(userName); - user.setPasswordResetRequired(false); - updateUser(user); - } catch (UserRetrievalException e) { - LOGGER.error("Could not retrieve user: {}", userName, e); + if (permanent) { + try { + User user = getUserInfo(userName); + user.setPasswordResetRequired(false); + updateUser(user); + } catch (UserRetrievalException e) { + LOGGER.error("Could not retrieve user: {}", userName, e); + } } } @@ -295,7 +342,7 @@ public AdminAddUserToGroupResponse addUserToGroup(String email, String groupName return cognitoClient.adminAddUserToGroup(request); } - // 'beforeInvocation = false allows us to use the return value to get the key to use for the eviction + // 'beforeInvocation = false' allows us to use the return value to get the key to use for the eviction @Caching(evict = { @CacheEvict(value = CacheNames.COGNITO_USERS_BY_UUID, key = "#cognitoId"), @CacheEvict(value = CacheNames.COGNITO_USERS_BY_EMAIL, key = "#result.email", beforeInvocation = false) @@ -315,10 +362,9 @@ public User deleteUser(UUID cognitoId) { } public List getAllUsers() { - return getAllUsers(false); - } + List allDevIdsAndNames = developerDAO.findAllIdsAndNames(); + List allAcbs = acbDao.findAll(); - public List getAllUsers(boolean includeDisabled) { ListUsersInGroupRequest request = ListUsersInGroupRequest.builder() .userPoolId(userPoolId) .groupName(environmentGroupName) @@ -328,7 +374,7 @@ public List getAllUsers(boolean includeDisabled) { ListUsersInGroupResponse response = cognitoClient.listUsersInGroup(request); users.addAll(response.users().stream() - .map(userType -> createUserFromUserType(userType)) + .map(userType -> createUserFromUserType(userType, allDevIdsAndNames, allAcbs)) .toList()); while (response.nextToken() != null) { @@ -341,12 +387,12 @@ public List getAllUsers(boolean includeDisabled) { response = cognitoClient.listUsersInGroup(request); users.addAll(response.users().stream() - .map(userType -> createUserFromUserType(userType)) + .map(userType -> createUserFromUserType(userType, allDevIdsAndNames, allAcbs)) .toList()); } return users.stream() - .filter(currUser -> includeDisabled ? true : currUser.getAccountEnabled()) + .filter(currUser -> currUser.getAccountEnabled()) .collect(Collectors.toList()); } @@ -366,7 +412,7 @@ public void updateUser(User user) throws UserRetrievalException { List attributes = new ArrayList(); attributes.add(AttributeType.builder().name("name").value(user.getFullName()).build()); attributes.add(AttributeType.builder().name("email_verified").value("true").build()); - attributes.add(AttributeType.builder().name("custom:forcePasswordReset").value(user.getPasswordResetRequired() ? "1" : "0").build()); + attributes.add(AttributeType.builder().name(FORCE_PASSWORD_RESET_ATTRIBUTE_NAME).value(user.getPasswordResetRequired() ? "1" : "0").build()); AdminUpdateUserAttributesRequest request = AdminUpdateUserAttributesRequest.builder() .userPoolId(userPoolId) @@ -390,7 +436,36 @@ public void addOrgToUser(User user, Long orgId) throws UserRetrievalException { .collect(Collectors.toSet()); orgIds.add(orgId); - attributes.add(AttributeType.builder().name("custom:organizations").value( + attributes.add(AttributeType.builder().name(ORGANIZATIONS_ATTRIBUTE_NAME).value( + orgIds.stream() + .map(o -> o.toString()) + .collect(Collectors.joining(","))) + .build()); + + AdminUpdateUserAttributesRequest request = AdminUpdateUserAttributesRequest.builder() + .userPoolId(userPoolId) + .username(user.getCognitoId().toString()) + .userAttributes(attributes) + .build(); + + cognitoClient.adminUpdateUserAttributes(request); + } + + @Caching(evict = { + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_UUID, key = "#user.cognitoId"), + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_EMAIL, key = "#user.email") + }) + public void removeOrgsFromUser(User user, List orgIdsToRemove) throws UserRetrievalException { + Set orgIds = CollectionUtils.isEmpty(user.getOrganizations()) + ? new HashSet() + : user.getOrganizations().stream() + .map(org -> org.getId()) + .collect(Collectors.toSet()); + orgIdsToRemove.stream() + .forEach(orgIdToRemove -> orgIds.remove(orgIdToRemove)); + + List attributes = new ArrayList(); + attributes.add(AttributeType.builder().name(ORGANIZATIONS_ATTRIBUTE_NAME).value( orgIds.stream() .map(o -> o.toString()) .collect(Collectors.joining(","))) @@ -410,11 +485,14 @@ public void addOrgToUser(User user, Long orgId) throws UserRetrievalException { @CacheEvict(value = CacheNames.COGNITO_USERS_BY_EMAIL, key = "#user.email") }) public void enableUser(User user) { - AdminEnableUserRequest request = AdminEnableUserRequest.builder() + //If a user is getting enabled, it's because they were at one time disabled + //and our workflow is that they can only become re-enabled by receiving a new invitation. + //As part of the new invitation process, we want to force the user to reset their password as well. + AdminEnableUserRequest enableUserRequest = AdminEnableUserRequest.builder() .userPoolId(userPoolId) .username(user.getCognitoId().toString()) .build(); - cognitoClient.adminEnableUser(request); + cognitoClient.adminEnableUser(enableUserRequest); } @Caching(evict = { @@ -458,6 +536,7 @@ private List getOrganizations(String role, List orgIds) { private List getCerificationBodyOrganizations(String role, List orgIds) { return orgIds.stream() .map(acbId -> getCertificationBody(acbId)) + .filter(acb -> acb != null) .map(acb -> new Organization(acb.getId(), acb.getName())) .toList(); } @@ -466,7 +545,7 @@ private CertificationBody getCertificationBody(Long certificationBodyId) { try { return certificationBodyDAO.getById(certificationBodyId); } catch (EntityRetrievalException e) { - LOGGER.error("Could not retrieve ACB: {}", certificationBodyId, e); + LOGGER.error("A user exists with reference to ACB organization {} which doees not exist.", certificationBodyId, e); return null; } } @@ -474,20 +553,21 @@ private CertificationBody getCertificationBody(Long certificationBodyId) { private List getDeveloperOrganizations(String role, List orgIds) { return orgIds.stream() .map(developerId -> getDeveloper(developerId)) + .filter(dev -> dev != null) .map(dev -> new Organization(dev.getId(), dev.getName())) .toList(); } private Developer getDeveloper(Long developerId) { try { - return developerDAO.getById(developerId); + return developerDAO.getSimpleDeveloperById(developerId, false); } catch (EntityRetrievalException e) { - LOGGER.error("Could not retrieve Developer: {}", developerId, e); + LOGGER.error("A user exists with reference to developer organization {} which doees not exist.", developerId); return null; } } - private User createUserFromUserType(UserType userType) { + private User createUserFromUserType(UserType userType, List developers, List acbs) { User user = new User(); user.setCognitoId(UUID.fromString(userType.username())); user.setSubjectName(getUserAttribute(userType.attributes(), "email").value()); @@ -498,11 +578,20 @@ private User createUserFromUserType(UserType userType) { user.setPasswordResetRequired(getForcePasswordReset(userType.attributes())); user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); - AttributeType orgIdsAttribute = getUserAttribute(userType.attributes(), "custom:organizations"); + AttributeType orgIdsAttribute = getUserAttribute(userType.attributes(), ORGANIZATIONS_ATTRIBUTE_NAME); if (orgIdsAttribute != null && StringUtils.isNotEmpty(orgIdsAttribute.value())) { - user.setOrganizations(getOrganizations(user.getRole(), Stream.of(orgIdsAttribute.value().split(",")) - .map(Long::valueOf) - .toList())); + List organizationIds = Stream.of(orgIdsAttribute.value().split(",")).map(Long::valueOf).toList(); + if (user.getRole().equalsIgnoreCase(CognitoGroups.CHPL_ACB)) { + user.setOrganizations(acbs.stream() + .filter(acb -> organizationIds.contains(acb.getId())) + .map(acb -> new Organization(acb.getId(), acb.getName())) + .collect(Collectors.toList())); + } else if (user.getRole().equalsIgnoreCase(CognitoGroups.CHPL_DEVELOPER)) { + user.setOrganizations(developers.stream() + .filter(dev -> organizationIds.contains(dev.getId())) + .map(dev -> new Organization(dev.getId(), dev.getName())) + .collect(Collectors.toList())); + } } return user; } @@ -517,7 +606,7 @@ private User createUserFromGetUserResponse(AdminGetUserResponse response) { user.setStatus(response.userStatusAsString()); user.setPasswordResetRequired(getForcePasswordReset(response.userAttributes())); user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); - AttributeType orgIdsAttribute = getUserAttribute(response.userAttributes(), "custom:organizations"); + AttributeType orgIdsAttribute = getUserAttribute(response.userAttributes(), ORGANIZATIONS_ATTRIBUTE_NAME); if (orgIdsAttribute != null && StringUtils.isNotEmpty(orgIdsAttribute.value())) { user.setOrganizations(getOrganizations(user.getRole(), Stream.of(orgIdsAttribute.value().split(",")) .map(Long::valueOf) @@ -527,7 +616,7 @@ private User createUserFromGetUserResponse(AdminGetUserResponse response) { } private Boolean getForcePasswordReset(List attributes) { - String forcePasswordReset = getUserAttribute(attributes, "custom:forcePasswordReset").value(); + String forcePasswordReset = getUserAttribute(attributes, FORCE_PASSWORD_RESET_ATTRIBUTE_NAME).value(); if (!StringUtils.isEmpty(forcePasswordReset)) { return forcePasswordReset.equals("1"); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserCreationValidator.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserCreationValidator.java index 5404e5965a..67b0e8849e 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserCreationValidator.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserCreationValidator.java @@ -4,12 +4,14 @@ import java.util.Set; import java.util.UUID; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; import gov.healthit.chpl.domain.CreateUserFromInvitationRequest; +import gov.healthit.chpl.domain.auth.User; import gov.healthit.chpl.exception.UserRetrievalException; import gov.healthit.chpl.user.cognito.invitation.CognitoUserInvitation; import gov.healthit.chpl.user.cognito.invitation.CognitoUserInvitationDAO; @@ -52,7 +54,7 @@ public Set validate(CreateUserFromInvitationRequest userInfo) { } try { - if (doesUserExistInCognito(userInfo.getUser().getEmail())) { + if (doesEnabledUserExistInCognito(userInfo.getUser().getEmail())) { messages.add(msgUtil.getMessage("user.accountAlreadyExists", userInfo.getUser().getEmail())); } } catch (UserRetrievalException e) { @@ -95,7 +97,8 @@ private Set validateCreateUserFromInvitationRequest(CreateUserFromInvita return validationErrors; } - private Boolean doesUserExistInCognito(String email) throws UserRetrievalException { - return cognitoApiWrapper.getUserInfo(email) != null; + private Boolean doesEnabledUserExistInCognito(String email) throws UserRetrievalException { + User existingUser = cognitoApiWrapper.getUserInfo(email); + return existingUser != null && BooleanUtils.isTrue(existingUser.getAccountEnabled()); } } \ No newline at end of file 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 2cfa71d0a7..3812d3d0a6 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 @@ -3,7 +3,11 @@ import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -14,6 +18,7 @@ import org.springframework.transaction.annotation.Transactional; import gov.healthit.chpl.domain.CreateUserFromInvitationRequest; +import gov.healthit.chpl.domain.Organization; import gov.healthit.chpl.domain.auth.CognitoEnvironments; import gov.healthit.chpl.domain.auth.CognitoGroups; import gov.healthit.chpl.domain.auth.User; @@ -97,9 +102,15 @@ public User updateUser(User user) throws ValidationException, UserRetrievalExcep User originalUser = cognitoApiWrapper.getUserNoCache(user.getCognitoId()); cognitoApiWrapper.updateUser(user); - if (originalUser.getAccountEnabled() && !user.getAccountEnabled()) { + //check for organizations to remove (organizations are never added here, only removed) + List removedOrganizationIds = getRemovedOrganizationIds(originalUser, user); + if (!CollectionUtils.isEmpty(removedOrganizationIds)) { + cognitoApiWrapper.removeOrgsFromUser(originalUser, removedOrganizationIds); + } + + if (userShouldBeDisabled(originalUser, user)) { cognitoApiWrapper.disableUser(user); - } else if (!originalUser.getAccountEnabled() && user.getAccountEnabled()) { + } else if (userShouldBeEnabled(originalUser, user)) { cognitoApiWrapper.enableUser(user); } @@ -110,6 +121,45 @@ public User updateUser(User user) throws ValidationException, UserRetrievalExcep return updatedUser; } + private List getRemovedOrganizationIds(User originalUser, User updatedUser) { + List removedOrgs = subtractLists(originalUser.getOrganizations(), updatedUser.getOrganizations()); + if (!CollectionUtils.isEmpty(removedOrgs)) { + return removedOrgs.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())); + + return listA.stream() + .filter(notInListB) + .collect(Collectors.toList()); + } + + private boolean userShouldBeDisabled(User originalUser, User updatedUser) { + //If there are no organizations remaining for this user and the user is an acb or developer + //then they should be disabled. + //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)) + && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllAcbsForUser(updatedUser))) { + return true; + } else if (!CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(originalUser)) + && CollectionUtils.isEmpty(resourcePermissionsFactory.get().getAllDevelopersForUser(updatedUser))) { + return true; + } + return false; + } + + private boolean userShouldBeEnabled(User originalUser, User updatedUser) { + return !originalUser.getAccountEnabled() && updatedUser.getAccountEnabled(); + } + @Transactional public UUID createUser(CreateUserFromInvitationRequest userInfo) throws ValidationException, UserCreationException, UserRetrievalException, ActivityException, EmailNotSentException { @@ -123,23 +173,28 @@ public UUID createUser(CreateUserFromInvitationRequest userInfo) CognitoCredentials credentials = null; try { CognitoUserInvitation invitation = cognitoInvitationManager.getByToken(UUID.fromString(userInfo.getHash())); - if (invitation.getOrganizationId() != null) { - userInfo.getUser().setOrganizationId(invitation.getOrganizationId()); + + //if the user exists for this environment and is disabled, we will re-enable them + //otherwise we will create a brand new user + User existingUser = null; + try { + existingUser = cognitoApiWrapper.getUserInfo(userInfo.getUser().getEmail()); + } catch (Exception ex) { + LOGGER.warn("Unable to look up user with email address " + userInfo.getUser().getEmail()); } - credentials = cognitoApiWrapper.createUser(userInfo.getUser()); - cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), invitation.getGroupName()); - if (isProdEnvironment) { - addUserToAppropriateEnvironments(userInfo.getUser().getEmail(), invitation.getGroupName()); + + if (existingUser != null && BooleanUtils.isFalse(existingUser.getAccountEnabled())) { + credentials = reenableUser(userInfo, invitation, existingUser); + } else if (existingUser == null) { + credentials = createNewUser(userInfo, invitation); } else { - cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), groupNameForEnvironment); + LOGGER.warn("The user with email address " + userInfo.getUser().getEmail() + " already exists and is enabled. " + + "A new account cannot be created."); } - cognitoInvitationManager.deleteInvitation(invitation); - cognitoConfirmEmailEmailer.sendConfirmationEmail(credentials); - User createdUser = cognitoApiWrapper.getUserNoCache(credentials.getCognitoId()); - activityManager.addUserActivity(createdUser.getCognitoId(), - String.format("User %s was created", createdUser.getEmail()), - null, createdUser); + if (credentials != null) { + cognitoConfirmEmailEmailer.sendConfirmationEmail(credentials); + } } catch (Exception e) { //Invitation deletion should roll back due to @Transactional if (credentials != null) { @@ -150,15 +205,55 @@ public UUID createUser(CreateUserFromInvitationRequest userInfo) return credentials == null ? null : credentials.getCognitoId(); } + private CognitoCredentials reenableUser(CreateUserFromInvitationRequest userInfo, CognitoUserInvitation invitation, + User existingUser) throws UserRetrievalException, UserCreationException, EmailNotSentException, ActivityException { + LOGGER.info("Re-enabling user " + existingUser.getEmail() + " from invitation " + userInfo.getHash()); + + existingUser.setFullName(userInfo.getUser().getFullName()); + CognitoCredentials credentials = cognitoApiWrapper.reenableUser(existingUser); + if (invitation.getOrganizationId() != null) { + cognitoApiWrapper.addOrgToUser(existingUser, invitation.getOrganizationId()); + } + + cognitoApiWrapper.updateUser(existingUser); + cognitoInvitationManager.deleteInvitation(invitation); + + User reenabledUser = cognitoApiWrapper.getUserNoCache(existingUser.getCognitoId()); + activityManager.addUserActivity(reenabledUser.getCognitoId(), + String.format("User %s was re-enabled", reenabledUser.getEmail()), + existingUser, reenabledUser); + + return credentials; + } + + private CognitoCredentials createNewUser(CreateUserFromInvitationRequest userInfo, CognitoUserInvitation invitation) + throws UserRetrievalException, UserCreationException, EmailNotSentException, ActivityException { + LOGGER.info("Creating new user " + userInfo.getUser().getEmail() + " from invitation " + userInfo.getHash()); + if (invitation.getOrganizationId() != null) { + userInfo.getUser().setOrganizationId(invitation.getOrganizationId()); + } + CognitoCredentials credentials = cognitoApiWrapper.createUser(userInfo.getUser()); + cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), invitation.getGroupName()); + if (isProdEnvironment) { + addUserToAppropriateEnvironments(userInfo.getUser().getEmail(), invitation.getGroupName()); + } else { + cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), groupNameForEnvironment); + } + cognitoInvitationManager.deleteInvitation(invitation); + + User createdUser = cognitoApiWrapper.getUserNoCache(credentials.getCognitoId()); + activityManager.addUserActivity(createdUser.getCognitoId(), + String.format("User %s was created", createdUser.getEmail()), + null, createdUser); + + return credentials; + } + @Transactional @PostFilter("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).SECURED_USER, " + "T(gov.healthit.chpl.permissions.domains.SecuredUserDomainPermissions).GET_ALL, filterObject)") - public List getAll(boolean includeDisabled) { - if (!resourcePermissionsFactory.get().isUserRoleAdmin() - && !resourcePermissionsFactory.get().isUserRoleOnc()) { - includeDisabled = false; - } - return cognitoApiWrapper.getAllUsers(includeDisabled); + public List getAll() { + return cognitoApiWrapper.getAllUsers(); } private void addUserToAppropriateEnvironments(String userEmail, String userRole) { @@ -184,7 +279,6 @@ private void addUserToAppropriateEnvironments(String userEmail, String userRole) } } - @Transactional @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).SECURED_USER, " + "T(gov.healthit.chpl.permissions.domains.SecuredUserDomainPermissions).ADD_ORG_TO_USER)") diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/util/ChplUserToCognitoUserUtil.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/util/ChplUserToCognitoUserUtil.java index 98ab1f1177..8780403511 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/util/ChplUserToCognitoUserUtil.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/util/ChplUserToCognitoUserUtil.java @@ -7,7 +7,6 @@ import gov.healthit.chpl.dao.auth.UserDAO; import gov.healthit.chpl.domain.auth.User; -import gov.healthit.chpl.exception.UserRetrievalException; import gov.healthit.chpl.user.cognito.CognitoApiWrapper; import lombok.extern.log4j.Log4j2; @@ -29,13 +28,13 @@ public User getUser(Long chplUserId, UUID cognitoUserId) { if (chplUserId != null) { try { currentUser = userDAO.getById(chplUserId, true).toDomain(); - } catch (UserRetrievalException e) { + } catch (Exception e) { LOGGER.error("Could not retreive user with ID: {}", chplUserId, e); } } else if (cognitoUserId != null) { try { currentUser = cognitoApiWrapper.getUserInfo(cognitoUserId); - } catch (UserRetrievalException e) { + } catch (Exception e) { LOGGER.error("Could not retreive user with ID: {}", cognitoUserId, e); } }