diff --git a/chpl/chpl-api/e2e/collections/acb-controller.postman_collection.json b/chpl/chpl-api/e2e/collections/acb-controller.postman_collection.json index fa143402cf..0ece99d8ba 100644 --- a/chpl/chpl-api/e2e/collections/acb-controller.postman_collection.json +++ b/chpl/chpl-api/e2e/collections/acb-controller.postman_collection.json @@ -172,14 +172,14 @@ "raw": "{\"id\":1,\"acbCode\":\"02\",\"name\":\"UL LLC\",\"website\":\"https://testwebsite.com\",\"address\":{\"addressId\":1,\"line1\":\"address\",\"line2\":null,\"city\":\"city\",\"state\":\"state\",\"zipcode\":\"111111\",\"country\":\"country\"},\"retired\":false,\"retirementDay\":null}" }, "url": { - "raw": "{{url}}/rest/acbs/1", + "raw": "{{url}}/rest/acbs/3", "host": [ "{{url}}" ], "path": [ "rest", "acbs", - "1" + "3" ] } }, @@ -254,14 +254,14 @@ } ], "url": { - "raw": "{{url}}/rest/acbs/1/users", + "raw": "{{url}}/rest/acbs/3/users", "host": [ "{{url}}" ], "path": [ "rest", "acbs", - "1", + "3", "users" ] } @@ -513,14 +513,14 @@ } ], "url": { - "raw": "{{url}}/rest/acbs/1/users", + "raw": "{{url}}/rest/acbs/3/users", "host": [ "{{url}}" ], "path": [ "rest", "acbs", - "1", + "3", "users" ] } 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 d65142cbc5..1b0b0d2f28 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 @@ -1,12 +1,9 @@ package gov.healthit.chpl.auth.authentication; import java.util.UUID; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.apache.commons.lang3.NotImplementedException; import org.springframework.beans.factory.annotation.Value; -import org.springframework.security.core.authority.SimpleGrantedAuthority; import com.auth0.jwt.JWT; import com.auth0.jwt.JWTVerifier; @@ -25,39 +22,25 @@ public class CognitoJwtUserConverter implements JWTUserConverter { private String region; private String userPoolId; - private String clientId; private String tokenizeRsaKeyUrl; - public CognitoJwtUserConverter(@Value("${cognito.region}") String region, @Value("${cognito.userPoolId}") String userPoolId, - @Value("${cognito.clientId}") String clientId, @Value("${cognito.tokenizezRsaKeyUrl}") String tokenizeRsaKeyUrl) { + public CognitoJwtUserConverter(@Value("${cognito.region}") String region, + @Value("${cognito.userPoolId}") String userPoolId, + @Value("${cognito.tokenizezRsaKeyUrl}") String tokenizeRsaKeyUrl) { this.region = region; this.userPoolId = userPoolId; - this.clientId = clientId; this.tokenizeRsaKeyUrl = tokenizeRsaKeyUrl; } @Override public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidationException, MultipleUserAccountsException { try { - DecodedJWT decodeJwt = decodeJwt(jwt); - if (decodeJwt.getClaims().size() != 0) { + DecodedJWT decodedJwt = decodeJwt(jwt); + if (decodedJwt.getClaims().size() != 0) { return JWTAuthenticatedUser.builder() .authenticationSystem(AuthenticationSystem.COGNITO) .authenticated(true) - .cognitoId(UUID.fromString(decodeJwt.getSubject())) - .subjectName(decodeJwt.getClaim("email").asString()) - .fullName(decodeJwt.getClaim("name").asString()) - .email(decodeJwt.getClaim("email").asString()) - .organizationIds( - decodeJwt.getClaims().containsKey("custom:organizations") - ? Stream.of(decodeJwt.getClaim("custom:organizations").asString().split(",")) - .map(Long::valueOf) - .toList() - : null) - .authorities(decodeJwt.getClaim("cognito:groups").asList(String.class).stream() - .filter(group -> !group.endsWith("-env")) //Remove environment related groups - .map(group -> new SimpleGrantedAuthority(group)) - .collect(Collectors.toSet())) + .cognitoId(UUID.fromString(decodedJwt.getSubject())) .build(); } else { throw new JWTValidationException("Invalid authentication token."); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java index 9db515712d..747812095b 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java @@ -1,7 +1,13 @@ package gov.healthit.chpl.auth.authentication; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.ff4j.FF4j; import org.springframework.beans.factory.annotation.Value; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.stereotype.Component; import gov.healthit.chpl.FeatureList; @@ -25,11 +31,15 @@ public class JWTUserConverterFacade implements JWTUserConverter { private FF4j ff4j; - public JWTUserConverterFacade(JWTConsumer jwtConsumer, UserDAO userDAO, @Value("${cognito.region}") String region, - @Value("${cognito.userPoolId}") String userPoolId, @Value("${cognito.clientId}") String clientId, - @Value("${cognito.tokenizezRsaKeyUrl}") String tokenizeRsaKeyUrl, FF4j ff4j, CognitoApiWrapper cognitoApiWrapper) { + public JWTUserConverterFacade(JWTConsumer jwtConsumer, + UserDAO userDAO, + @Value("${cognito.region}") String region, + @Value("${cognito.userPoolId}") String userPoolId, + @Value("${cognito.tokenizezRsaKeyUrl}") String tokenizeRsaKeyUrl, + FF4j ff4j, + CognitoApiWrapper cognitoApiWrapper) { chplJwtUserConverter = new ChplJWTUserConverter(jwtConsumer, userDAO); - cognitoJwtUserConverter = new CognitoJwtUserConverter(region, userPoolId, clientId, tokenizeRsaKeyUrl); + cognitoJwtUserConverter = new CognitoJwtUserConverter(region, userPoolId, tokenizeRsaKeyUrl); this.ff4j = ff4j; this.cognitoApiWrapper = cognitoApiWrapper; } @@ -42,11 +52,20 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio user = cognitoJwtUserConverter.getAuthenticatedUser(jwt); if (user != null) { try { - //Set some values not avail in the Cognito Access Token that were avail in the CHPL token + //Many values are not available from the Cognito Access Token so we have to set them + //manually here from the Cognito user data User cognitoUser = cognitoApiWrapper.getUserInfo(user.getCognitoId()); user.setEmail(cognitoUser.getEmail()); user.setSubjectName(cognitoUser.getEmail()); user.setFullName(cognitoUser.getFullName()); + if (!StringUtils.isEmpty(cognitoUser.getRole())) { + user.setAuthorities(Stream.of(new SimpleGrantedAuthority(cognitoUser.getRole())).collect(Collectors.toSet())); + } + if (!CollectionUtils.isEmpty(cognitoUser.getOrganizations())) { + user.setOrganizationIds(cognitoUser.getOrganizations().stream() + .map(org -> org.getId()) + .toList()); + } } catch (UserRetrievalException e) { throw new JWTValidationException("Could not locate the Cognito user id"); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/CognitoGroups.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/CognitoGroups.java index 2153cfe70e..99dfa8b6c0 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/CognitoGroups.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/domain/auth/CognitoGroups.java @@ -6,12 +6,9 @@ public final class CognitoGroups { private CognitoGroups() {} public static final String CHPL_ADMIN = "chpl-admin"; public static final String CHPL_ONC = "chpl-onc"; - public static final String CHPL_USER_CREATOR = "chpl-user-creator"; public static final String CHPL_ACB = "chpl-onc-acb"; public static final String CHPL_CMS_STAFF = "chpl-cms-staff"; public static final String CHPL_DEVELOPER = "chpl-developer"; - public static final String CHPL_USER_AUTHENTICATOR = "chpl-user-authenticator"; - public static final String CHPL_INVITED_USER_CREATOR = "chpl-invited-user-creator"; public static final String CHPL_STARTUP = "chpl-startup"; public static final String CHPL_SYSTEM = "chpl-system"; @@ -19,12 +16,9 @@ public static List getAll() { return List.of( CHPL_ADMIN, CHPL_ONC, - CHPL_USER_CREATOR, CHPL_ACB, CHPL_CMS_STAFF, CHPL_DEVELOPER, - CHPL_USER_AUTHENTICATOR, - CHPL_INVITED_USER_CREATOR, CHPL_STARTUP, CHPL_SYSTEM); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/insight/InsightService.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/insight/InsightService.java index 34e2425875..ddbcdabfbe 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/insight/InsightService.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/insight/InsightService.java @@ -6,6 +6,7 @@ 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; import org.springframework.http.HttpStatusCode; @@ -61,6 +62,9 @@ private JsonNode fetchInsightSubmissionsForDeveloper(Long developerId) throws In try { response = insightRestTemplate.getForEntity(url, String.class); LOGGER.debug("Response: " + response.getBody()); + if (response == null || StringUtils.isEmpty(response.getBody())) { + LOGGER.warn("A null or empty response was received from the Insights API."); + } } catch (Exception ex) { HttpStatusCode statusCode = (response != null ? response.getStatusCode() : null); if (statusCode == null && ex instanceof RestClientResponseException) { @@ -69,7 +73,7 @@ private JsonNode fetchInsightSubmissionsForDeveloper(Long developerId) throws In LOGGER.error("Unable to connect to the URL " + url + ". Got response status code " + statusCode); throw new InsightRequestFailedException(ex.getMessage(), ex, statusCode); } - String responseBody = response == null ? "" : response.getBody(); + String responseBody = ((response == null || StringUtils.isEmpty(response.getBody())) ? "{}" : response.getBody()); JsonNode root = null; try { root = objectMapper.readTree(responseBody); 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 e5d4c3e8e3..8b7ef99123 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 @@ -86,14 +86,14 @@ public CertificationBody create(CertificationBody acb) throws EntityCreationExce @Transactional @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).CERTIFICATION_BODY, " - + "T(gov.healthit.chpl.permissions.domains.CertificationBodyDomainPermissions).UPDATE, #acb)") + + "T(gov.healthit.chpl.permissions.domains.CertificationBodyDomainPermissions).UPDATE, #acbToUpdate)") @CacheEvict(value = { CacheNames.GET_DECERTIFIED_DEVELOPERS, CacheNames.COLLECTIONS_DEVELOPERS, CacheNames.COLLECTIONS_LISTINGS, CacheNames.COMPLAINTS }, allEntries = true) - @ListingStoreRemove(removeBy = RemoveBy.ACB_ID, id = "#acb.id") + @ListingStoreRemove(removeBy = RemoveBy.ACB_ID, id = "#acbToUpdate.id") @ListingSearchCacheRefresh // no other caches have ACB data so we do not need to clear all public CertificationBody update(CertificationBody acbToUpdate) throws EntityRetrievalException, SchedulerException, ValidationException, ActivityException, InvalidArgumentsException { 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 6611f969d0..04588c49a9 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 @@ -200,17 +200,17 @@ public boolean isUserRoleDeveloperAdmin() { @Override public boolean isUserRoleUserCreator() { - return doesAuditUserHaveRole(CognitoGroups.CHPL_USER_CREATOR); + return false; } @Override public boolean isUserRoleUserAuthenticator() { - return doesAuditUserHaveRole(CognitoGroups.CHPL_USER_AUTHENTICATOR); + return false; } @Override public boolean isUserRoleInvitedUserCreator() { - return doesAuditUserHaveRole(CognitoGroups.CHPL_INVITED_USER_CREATOR); + return false; } @Override 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 ff0050edc0..a7ecb5ae29 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 @@ -46,10 +46,16 @@ public interface ResourcePermissions { boolean isUserRoleDeveloperAdmin(); + //Not used with Cognito users + @Deprecated boolean isUserRoleUserCreator(); + //Not used with Cognito users + @Deprecated boolean isUserRoleUserAuthenticator(); + //Not used with Cognito users + @Deprecated boolean isUserRoleInvitedUserCreator(); boolean isUserRoleStartup(); 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 9e36e313a1..cf79c4551b 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,8 +81,9 @@ @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"; + public static final String ORGANIZATIONS_ATTRIBUTE_NAME = "custom:organizations"; + public static final String ROLES_ATTRIBUTE_NAME = "custom:roles"; + public static final String FORCE_PASSWORD_RESET_ATTRIBUTE_NAME = "custom:forcePasswordReset"; private String clientId; private String userPoolId; @@ -94,10 +95,15 @@ public class CognitoApiWrapper { 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, + 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 acbDao) { cognitoClient = createCognitoClient(accessKey, secretKey, region); @@ -141,13 +147,12 @@ public AuthenticationResultType authenticate(LoginCredentials credentials) throw } catch (CognitoAuthenticationChallengeException e) { throw e; } 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("Authentication error: {}", e.getMessage(), e); + LOGGER.error("Authentication error for user {}: {}", credentials.getUserName(), e.getMessage(), e); return null; } } + //@Cacheable(value = CacheNames.COGNITO_USERS_BY_EMAIL, unless = "#result == null") public AuthenticationResultType respondToNewPasswordRequiredChallenge(CognitoNewPasswordRequiredRequest newPassworRequiredRequest) { AdminRespondToAuthChallengeRequest request = AdminRespondToAuthChallengeRequest.builder() .userPoolId(userPoolId) @@ -240,7 +245,7 @@ public User getUserNoCache(UUID cognitoId) throws UserRetrievalException { return user; } - public CognitoCredentials createUser(CreateUserRequest userRequest) throws UserCreationException { + public CognitoCredentials createUser(CreateUserRequest userRequest, String roleName) throws UserCreationException { try { String tempPassword = PasswordUtil.generatePassword(); @@ -250,6 +255,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(ROLES_ATTRIBUTE_NAME).value(roleName != null ? roleName : "").build(), AttributeType.builder().name(ORGANIZATIONS_ATTRIBUTE_NAME).value( userRequest.getOrganizationId() != null ? userRequest.getOrganizationId().toString() : "").build()) .temporaryPassword(tempPassword) @@ -311,7 +317,11 @@ public AuthenticationResultType refreshToken(String refreshToken, UUID cognitoId } } - public void setUserPassword(String userName, String password, Boolean permanent) { + @Caching(evict = { + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_UUID, key = "#result.cognitoId"), + @CacheEvict(value = CacheNames.COGNITO_USERS_BY_EMAIL, key = "#result.email") + }) + public User setUserPassword(String userName, String password, Boolean permanent) { AdminSetUserPasswordRequest request = AdminSetUserPasswordRequest.builder() .username(userName) .password(password) @@ -330,6 +340,14 @@ public void setUserPassword(String userName, String password, Boolean permanent) LOGGER.error("Could not retrieve user: {}", userName, e); } } + + User user = null; + try { + user = getUserInfo(userName); + } catch (UserRetrievalException e) { + LOGGER.error("Could not retrieve user: {}", userName, e); + } + return user; } public AdminAddUserToGroupResponse addUserToGroup(String email, String groupName) { @@ -389,8 +407,8 @@ public List getAllUsers() { users.addAll(response.users().stream() .map(userType -> createUserFromUserType(userType, allDevIdsAndNames, allAcbs)) .toList()); - } + return users.stream() .filter(currUser -> currUser.getAccountEnabled()) .collect(Collectors.toList()); @@ -576,7 +594,7 @@ private User createUserFromUserType(UserType userType, List developer user.setAccountEnabled(userType.enabled()); user.setStatus(userType.userStatusAsString()); user.setPasswordResetRequired(getForcePasswordReset(userType.attributes())); - user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); + user.setRole(getRole(userType.attributes())); AttributeType orgIdsAttribute = getUserAttribute(userType.attributes(), ORGANIZATIONS_ATTRIBUTE_NAME); if (orgIdsAttribute != null && StringUtils.isNotEmpty(orgIdsAttribute.value())) { @@ -605,7 +623,7 @@ private User createUserFromGetUserResponse(AdminGetUserResponse response) { user.setAccountEnabled(response.enabled()); user.setStatus(response.userStatusAsString()); user.setPasswordResetRequired(getForcePasswordReset(response.userAttributes())); - user.setRole(getRoleBasedOnFilteredGroups(getGroupsForUser(user.getEmail()))); + user.setRole(getRole(response.userAttributes())); 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(",")) @@ -623,12 +641,13 @@ private Boolean getForcePasswordReset(List attributes) { return false; } - private String getRoleBasedOnFilteredGroups(List groups) { - return groups.stream() - .map(groupType -> groupType.groupName()) - .filter(group -> !group.endsWith("-env")) - .findAny() - .orElseThrow(() -> new RuntimeException("Could not determine user's role")); + private String getRole(List attributes) { + String role = null; + String delimitedRoleNames = getUserAttribute(attributes, ROLES_ATTRIBUTE_NAME).value(); + if (delimitedRoleNames != null && StringUtils.isNotEmpty(delimitedRoleNames)) { + role = Stream.of(delimitedRoleNames.split(",")).toList().get(0); + } + return role; } private List getGroupsForUser(String email) { 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 b042e4fffb..59c1786806 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 @@ -262,8 +262,7 @@ private CognitoCredentials createNewUser(CreateUserFromInvitationRequest userInf if (invitation.getOrganizationId() != null) { userInfo.getUser().setOrganizationId(invitation.getOrganizationId()); } - CognitoCredentials credentials = cognitoApiWrapper.createUser(userInfo.getUser()); - cognitoApiWrapper.addUserToGroup(userInfo.getUser().getEmail(), invitation.getGroupName()); + CognitoCredentials credentials = cognitoApiWrapper.createUser(userInfo.getUser(), invitation.getGroupName()); if (isProdEnvironment) { addUserToAppropriateEnvironments(userInfo.getUser().getEmail(), invitation.getGroupName()); } else {