From c5f994b3238ac11bf66a4617f6f0a7637adeb8f8 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Thu, 12 Dec 2024 12:48:16 -0500 Subject: [PATCH 01/12] feat!: Support revoking org access from acb/developer users [#OCD-4734] --- .../controller/UserManagementController.java | 6 +-- .../chpl/user/cognito/CognitoApiWrapper.java | 43 ++++++++++++--- .../chpl/user/cognito/CognitoUserManager.java | 54 +++++++++++++++++-- 3 files changed, 91 insertions(+), 12 deletions(-) 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 503a094154..463ce547e7 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 @@ -227,16 +227,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); } 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 99f5522098..aadf09c8a5 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; @@ -223,7 +225,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) @@ -364,7 +366,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) @@ -388,7 +390,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(","))) @@ -496,7 +527,7 @@ 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) @@ -515,7 +546,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) @@ -525,7 +556,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/CognitoUserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java index 6d0ef52a7e..3acd040c74 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,10 @@ 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.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -14,6 +17,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 +101,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 +120,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 { @@ -184,7 +233,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)") From 30f1e693621af6212c19a9d1a39e1bafdb76e34d Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Thu, 12 Dec 2024 14:54:16 -0500 Subject: [PATCH 02/12] fix: Put back environment filtering in AdminGetUser requests to Cognito [#OCD-4734] --- .../chpl/user/cognito/CognitoApiWrapper.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) 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 aadf09c8a5..78e89f754a 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 @@ -181,7 +181,13 @@ 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; } @Cacheable(value = CacheNames.COGNITO_USERS_BY_EMAIL, unless = "#result == null") @@ -195,7 +201,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; } @@ -212,7 +224,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 { @@ -295,7 +313,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) From bbfd77bbe2700d824d291159024bfcdb1dbf3460 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Thu, 12 Dec 2024 14:54:51 -0500 Subject: [PATCH 03/12] feat: Allow disabled users to be re-invited through create acct workflow [#OCD-4734] --- .../chpl/user/cognito/CognitoApiWrapper.java | 18 ++++++- .../chpl/user/cognito/CognitoUserManager.java | 50 +++++++++++++------ 2 files changed, 52 insertions(+), 16 deletions(-) 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 78e89f754a..8736c4e7cd 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 @@ -457,11 +457,25 @@ public void removeOrgsFromUser(User user, List orgIdsToRemove) throws User @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); + + List attributes = new ArrayList(); + attributes.add(AttributeType.builder().name(FORCE_PASSWORD_RESET_ATTRIBUTE_NAME).value("1").build()); + + AdminUpdateUserAttributesRequest setForcePasswordResetRequest = AdminUpdateUserAttributesRequest.builder() + .userPoolId(userPoolId) + .username(user.getCognitoId().toString()) + .userAttributes(attributes) + .build(); + + cognitoClient.adminUpdateUserAttributes(setForcePasswordResetRequest); } @Caching(evict = { 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 3acd040c74..b77c7c85d0 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 @@ -7,6 +7,7 @@ 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; @@ -172,23 +173,44 @@ 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 should enable them, add the organization in the request, + //and put them in FORCE_CHANGE_PASSWORD state, else we create a new user in the regular way + 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())) { + cognitoApiWrapper.enableUser(existingUser); + if (invitation.getOrganizationId() != null) { + cognitoApiWrapper.addOrgToUser(existingUser, invitation.getOrganizationId()); + } + User reenabledUser = cognitoApiWrapper.getUserNoCache(existingUser.getCognitoId()); + activityManager.addUserActivity(reenabledUser.getCognitoId(), + String.format("User %s was re-enabled", reenabledUser.getEmail()), + null, reenabledUser); } else { - cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), groupNameForEnvironment); - } - cognitoInvitationManager.deleteToken(UUID.fromString(userInfo.getHash())); - cognitoConfirmEmailEmailer.sendConfirmationEmail(credentials); + if (invitation.getOrganizationId() != null) { + userInfo.getUser().setOrganizationId(invitation.getOrganizationId()); + } + 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.deleteToken(UUID.fromString(userInfo.getHash())); + cognitoConfirmEmailEmailer.sendConfirmationEmail(credentials); - User createdUser = cognitoApiWrapper.getUserNoCache(credentials.getCognitoId()); - activityManager.addUserActivity(createdUser.getCognitoId(), - String.format("User %s was created", createdUser.getEmail()), - null, createdUser); + User createdUser = cognitoApiWrapper.getUserNoCache(credentials.getCognitoId()); + activityManager.addUserActivity(createdUser.getCognitoId(), + String.format("User %s was created", createdUser.getEmail()), + null, createdUser); + } } catch (Exception e) { //Invitation deletion should roll back due to @Transactional if (credentials != null) { From c715b91de08ce82c8212f248f9298131796e221c Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Fri, 13 Dec 2024 14:30:53 -0500 Subject: [PATCH 04/12] fix: Prevent failures getting activity if user does not exist [#OCD-4734] --- .../gov/healthit/chpl/util/ChplUserToCognitoUserUtil.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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); } } From 668e47594a11dc2b43c1f78b4293ad99129da3c4 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Fri, 13 Dec 2024 14:32:23 -0500 Subject: [PATCH 05/12] feat: Enable logging to debug issues logging in [#OCD-4734] --- .../healthit/chpl/auth/authentication/ChplJWTUserConverter.java | 1 + 1 file changed, 1 insertion(+) 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."); } From 12bf22ff93d5a65e7366c8a2c78d9a38ea46e4f7 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Fri, 13 Dec 2024 14:34:43 -0500 Subject: [PATCH 06/12] feat!: Add ability to re-enable a disable user via invitation [#OCD-4734] --- .../chpl/user/cognito/CognitoApiWrapper.java | 49 +++++++----- .../cognito/CognitoUserCreationValidator.java | 9 ++- .../chpl/user/cognito/CognitoUserManager.java | 80 +++++++++++++------ 3 files changed, 91 insertions(+), 47 deletions(-) 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 8736c4e7cd..f9d9fd85df 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 @@ -261,6 +261,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); @@ -279,7 +301,7 @@ public AuthenticationResultType refreshToken(String refreshToken, UUID cognitoId } catch (Exception e) { //This is cluttering the logs when the SSO flag is on, and the user logs in using CHPL creds //We might want to uncomment it when we move to only using Cognito creds - //LOGGER.error("Error refreshing token", e); + LOGGER.error("Error refreshing token", e); return null; } } @@ -294,12 +316,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); + } } } @@ -465,17 +489,6 @@ public void enableUser(User user) { .username(user.getCognitoId().toString()) .build(); cognitoClient.adminEnableUser(enableUserRequest); - - List attributes = new ArrayList(); - attributes.add(AttributeType.builder().name(FORCE_PASSWORD_RESET_ATTRIBUTE_NAME).value("1").build()); - - AdminUpdateUserAttributesRequest setForcePasswordResetRequest = AdminUpdateUserAttributesRequest.builder() - .userPoolId(userPoolId) - .username(user.getCognitoId().toString()) - .userAttributes(attributes) - .build(); - - cognitoClient.adminUpdateUserAttributes(setForcePasswordResetRequest); } @Caching(evict = { 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 6af4c66a27..f341115d8a 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 b77c7c85d0..1255e1061d 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 @@ -174,8 +174,8 @@ public UUID createUser(CreateUserFromInvitationRequest userInfo) try { CognitoUserInvitation invitation = cognitoInvitationManager.getByToken(UUID.fromString(userInfo.getHash())); - //if the user exists for this environment and is disabled, we should enable them, add the organization in the request, - //and put them in FORCE_CHANGE_PASSWORD state, else we create a new user in the regular way + //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()); @@ -184,32 +184,16 @@ public UUID createUser(CreateUserFromInvitationRequest userInfo) } if (existingUser != null && BooleanUtils.isFalse(existingUser.getAccountEnabled())) { - cognitoApiWrapper.enableUser(existingUser); - if (invitation.getOrganizationId() != null) { - cognitoApiWrapper.addOrgToUser(existingUser, invitation.getOrganizationId()); - } - User reenabledUser = cognitoApiWrapper.getUserNoCache(existingUser.getCognitoId()); - activityManager.addUserActivity(reenabledUser.getCognitoId(), - String.format("User %s was re-enabled", reenabledUser.getEmail()), - null, reenabledUser); + credentials = reenableUser(userInfo, invitation, existingUser); + } else if (existingUser == null) { + credentials = createNewUser(userInfo, invitation); } else { - if (invitation.getOrganizationId() != null) { - userInfo.getUser().setOrganizationId(invitation.getOrganizationId()); - } - 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.deleteToken(UUID.fromString(userInfo.getHash())); - cognitoConfirmEmailEmailer.sendConfirmationEmail(credentials); + LOGGER.warn("The user with email address " + userInfo.getUser().getEmail() + " already exists and is enabled. " + + "A new account cannot be created."); + } - 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 @@ -221,6 +205,50 @@ 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.deleteToken(UUID.fromString(userInfo.getHash())); + + 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.deleteToken(UUID.fromString(userInfo.getHash())); + + 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)") From 79c4eed23413c230a4140cbde03964b0c5d255f0 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 09:39:06 -0500 Subject: [PATCH 07/12] fix: Attempt to make retreiving users not fail if deb/acb doesn't exist [#OCD-4734] --- .../gov/healthit/chpl/user/cognito/CognitoApiWrapper.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 f9d9fd85df..d85016dc63 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 @@ -532,6 +532,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(); } @@ -540,7 +541,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; } } @@ -548,15 +549,16 @@ 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, e); return null; } } From d3cafba6764e5cd6b4b3373e57585eb6103f3e9d Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 09:39:37 -0500 Subject: [PATCH 08/12] refactor: Add some logging to debug jwt issues [#OCD-4734] --- .../chpl/auth/authentication/CognitoJwtUserConverter.java | 2 ++ 1 file changed, 2 insertions(+) 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 67cf818a9d..985dadd5fe 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; } } From d65c081ea5f3dd2ceaf9c8151b06847c95ce3274 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 12:16:27 -0500 Subject: [PATCH 09/12] feat: Change leeway value to 30 seconds to match CHPL jwt offset [#OCD-4734] --- .../chpl/auth/authentication/CognitoJwtUserConverter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 985dadd5fe..286a14b0d0 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 @@ -76,8 +76,8 @@ private DecodedJWT decodeJwt(String jwt) { RSAKeyProvider keyProvider = new CognitoRsaKeyProvider(region, userPoolId, tokenizeRsaKeyUrl); Algorithm algorithm = Algorithm.RSA256(keyProvider); JWTVerifier jwtVerifier = JWT.require(algorithm) - //.withAudience(clientId) - .build(); + .acceptLeeway(30000) //allows for the CHPL server clock and AWS server clock to be off by 30 seconds + .build(); DecodedJWT decodedJwt = jwtVerifier.verify(jwt); From 368c7a2fdf6eec0e59aac07d72e7ee42bc789742 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 12:20:12 -0500 Subject: [PATCH 10/12] feat: Make less db calls when loading cognito users - speeding it up? [#OCD-4734] --- .../chpl/user/cognito/CognitoApiWrapper.java | 55 +++++++++++++++---- 1 file changed, 43 insertions(+), 12 deletions(-) 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 1dfcdef506..2bcc75f21f 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 @@ -91,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; @@ -105,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 { @@ -355,10 +358,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) @@ -368,7 +370,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) { @@ -381,12 +383,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()); } @@ -556,12 +558,32 @@ private Developer getDeveloper(Long developerId) { try { return developerDAO.getSimpleDeveloperById(developerId, false); } catch (EntityRetrievalException e) { - LOGGER.error("A user exists with reference to developer organization {} which doees not exist.", 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) { +// User user = new User(); +// user.setCognitoId(UUID.fromString(userType.username())); +// user.setSubjectName(getUserAttribute(userType.attributes(), "email").value()); +// user.setFullName(getUserAttribute(userType.attributes(), "name").value()); +// user.setEmail(getUserAttribute(userType.attributes(), "email").value()); +// user.setAccountEnabled(userType.enabled()); +// user.setStatus(userType.userStatusAsString()); +// user.setPasswordResetRequired(getForcePasswordReset(userType.attributes())); +// user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); +// +// 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())); +// } +// return user; +// } + + 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()); @@ -574,9 +596,18 @@ private User createUserFromUserType(UserType userType) { 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; } From 1afb8ddc6c7028b6899326fd77a05bc5d8cbce47 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 12:21:05 -0500 Subject: [PATCH 11/12] feat!: Remove "includeDisabled" request param from GET user endpoints Breaking Change [#OCD-4734] --- .../CertificationBodyController.java | 15 ++------- .../web/controller/DeveloperController.java | 17 ++-------- .../controller/UserManagementController.java | 24 ++++---------- .../manager/CertificationBodyManager.java | 9 ++--- .../chpl/manager/DeveloperManager.java | 8 ++--- .../chpl/manager/auth/UserManager.java | 14 +------- .../permissions/ChplResourcePermissions.java | 33 +------------------ .../CognitoResourcePermissions.java | 26 ++------------- .../chpl/permissions/ResourcePermissions.java | 8 ----- .../job/MassRequirePasswordChangeJob.java | 2 +- ...nChangeRequestDeveloperEmailGenerator.java | 2 +- .../messaging/MessageDevelopersJob.java | 4 ++- .../chpl/user/cognito/CognitoUserManager.java | 8 ++--- 13 files changed, 28 insertions(+), 142 deletions(-) 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 229f2613b6..2ef5ad67cd 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; @@ -525,18 +521,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(); @@ -564,8 +554,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) { @@ -575,8 +565,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/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/CognitoUserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/user/cognito/CognitoUserManager.java index 6a197f91bf..dfc592bf08 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 @@ -252,12 +252,8 @@ private CognitoCredentials createNewUser(CreateUserFromInvitationRequest userInf @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) { From 7620526fd5c1ab4b6fc491d416bd8a91cca8ba74 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 16 Dec 2024 14:00:35 -0500 Subject: [PATCH 12/12] refactor: Remove commented code [#OCD-4734] --- .../chpl/user/cognito/CognitoApiWrapper.java | 20 ------------------- 1 file changed, 20 deletions(-) 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 2bcc75f21f..833a545ea8 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 @@ -563,26 +563,6 @@ private Developer getDeveloper(Long developerId) { } } -// private User createUserFromUserType(UserType userType) { -// User user = new User(); -// user.setCognitoId(UUID.fromString(userType.username())); -// user.setSubjectName(getUserAttribute(userType.attributes(), "email").value()); -// user.setFullName(getUserAttribute(userType.attributes(), "name").value()); -// user.setEmail(getUserAttribute(userType.attributes(), "email").value()); -// user.setAccountEnabled(userType.enabled()); -// user.setStatus(userType.userStatusAsString()); -// user.setPasswordResetRequired(getForcePasswordReset(userType.attributes())); -// user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); -// -// 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())); -// } -// return user; -// } - private User createUserFromUserType(UserType userType, List developers, List acbs) { User user = new User(); user.setCognitoId(UUID.fromString(userType.username()));