From e105e7a6b3c573e3978a1e3a801343afb337210c Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 6 Jan 2025 15:42:22 -0500 Subject: [PATCH 1/9] [#OCD-4782] [#OCD-4782-new] --- .../java/gov/healthit/chpl/domain/auth/CognitoGroups.java | 6 ------ .../chpl/permissions/CognitoResourcePermissions.java | 6 +++--- .../gov/healthit/chpl/permissions/ResourcePermissions.java | 6 ++++++ 3 files changed, 9 insertions(+), 9 deletions(-) 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/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(); From ba810188f34d20b0b32d52a0da447440f7e6a52f Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 6 Jan 2025 16:56:46 -0500 Subject: [PATCH 2/9] feat: Convert JWT user role from custom Cognito attribute [#OCD-4782-new] [#OCD-4782] --- .../auth/authentication/CognitoJwtUserConverter.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 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 d65142cbc5..1296f782e2 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 @@ -25,14 +25,13 @@ 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; } @@ -54,8 +53,7 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio .map(Long::valueOf) .toList() : null) - .authorities(decodeJwt.getClaim("cognito:groups").asList(String.class).stream() - .filter(group -> !group.endsWith("-env")) //Remove environment related groups + .authorities(decodeJwt.getClaim("custom:role").asList(String.class).stream() .map(group -> new SimpleGrantedAuthority(group)) .collect(Collectors.toSet())) .build(); From 9e3e395bf3ea925ec9fc066e6fc7a49a01df4391 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 6 Jan 2025 16:57:53 -0500 Subject: [PATCH 3/9] fix: Remove unused Cognito clientID in JWT converter [#OCD-4782-new] [#OCD-4782] --- .../auth/authentication/JWTUserConverterFacade.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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..a4d6bf929e 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 @@ -25,11 +25,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; } From 5ad9a737d57158414b69415602a1e8e52dd5e308 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Thu, 9 Jan 2025 13:05:20 -0500 Subject: [PATCH 4/9] feat!: Get use role from custom:roles Cognito attribute [#OCD-4782] --- .../CognitoJwtUserConverter.java | 15 ++++--- .../chpl/user/cognito/CognitoApiWrapper.java | 44 +++++++++++-------- .../chpl/user/cognito/CognitoUserManager.java | 3 +- 3 files changed, 36 insertions(+), 26 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 1296f782e2..9ab5489dc9 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 @@ -19,6 +19,7 @@ import gov.healthit.chpl.auth.user.JWTAuthenticatedUser; import gov.healthit.chpl.exception.JWTValidationException; import gov.healthit.chpl.exception.MultipleUserAccountsException; +import gov.healthit.chpl.user.cognito.CognitoApiWrapper; import lombok.extern.log4j.Log4j2; @Log4j2 @@ -48,14 +49,18 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio .fullName(decodeJwt.getClaim("name").asString()) .email(decodeJwt.getClaim("email").asString()) .organizationIds( - decodeJwt.getClaims().containsKey("custom:organizations") - ? Stream.of(decodeJwt.getClaim("custom:organizations").asString().split(",")) + decodeJwt.getClaims().containsKey(CognitoApiWrapper.ORGANIZATIONS_ATTRIBUTE_NAME) + ? Stream.of(decodeJwt.getClaim(CognitoApiWrapper.ORGANIZATIONS_ATTRIBUTE_NAME).asString().split(",")) .map(Long::valueOf) .toList() : null) - .authorities(decodeJwt.getClaim("custom:role").asList(String.class).stream() - .map(group -> new SimpleGrantedAuthority(group)) - .collect(Collectors.toSet())) + //in CHPL each user can only have a single role, but we are supporting the possibility that + //other applications could need something different + .authorities(decodeJwt.getClaims().containsKey(CognitoApiWrapper.ROLES_ATTRIBUTE_NAME) + ? Stream.of(decodeJwt.getClaim(CognitoApiWrapper.ROLES_ATTRIBUTE_NAME).asString().split(",")) + .map(attr -> new SimpleGrantedAuthority(attr)) + .collect(Collectors.toSet()) + : null) .build(); } else { throw new JWTValidationException("Invalid authentication token."); 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..1185c198b4 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,9 +147,7 @@ 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: {}", e.getMessage(), e); return null; } } @@ -240,7 +244,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 +254,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) @@ -389,8 +394,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 +581,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 +610,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 +628,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 { From 9f3eca0ac6b7264e294872f5ce83d01a142344d6 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Sun, 12 Jan 2025 16:28:32 -0500 Subject: [PATCH 5/9] fix: Add necessary CacheEvict annotation so user can sign up [#OCD-4782] --- .../chpl/user/cognito/CognitoApiWrapper.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 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 1185c198b4..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 @@ -147,11 +147,12 @@ public AuthenticationResultType authenticate(LoginCredentials credentials) throw } catch (CognitoAuthenticationChallengeException e) { throw e; } catch (Exception e) { - 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) @@ -316,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) @@ -335,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) { From 0cb969018bec9b03cd685ef33ec76ba051bb2e85 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Sun, 12 Jan 2025 16:29:02 -0500 Subject: [PATCH 6/9] feat: Clean up getting of user data from Cognito access token The JWT (token) does not have a bunch of user data that the code was trying to set from there. All those fields were null. The role, orgs, name, and email needs to be pulled from the User object that we get querying Cognito. [#OCD-4782] --- .../CognitoJwtUserConverter.java | 26 +++---------------- .../JWTUserConverterFacade.java | 17 +++++++++++- 2 files changed, 19 insertions(+), 24 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 9ab5489dc9..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; @@ -19,7 +16,6 @@ import gov.healthit.chpl.auth.user.JWTAuthenticatedUser; import gov.healthit.chpl.exception.JWTValidationException; import gov.healthit.chpl.exception.MultipleUserAccountsException; -import gov.healthit.chpl.user.cognito.CognitoApiWrapper; import lombok.extern.log4j.Log4j2; @Log4j2 @@ -39,28 +35,12 @@ public CognitoJwtUserConverter(@Value("${cognito.region}") String region, @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(CognitoApiWrapper.ORGANIZATIONS_ATTRIBUTE_NAME) - ? Stream.of(decodeJwt.getClaim(CognitoApiWrapper.ORGANIZATIONS_ATTRIBUTE_NAME).asString().split(",")) - .map(Long::valueOf) - .toList() - : null) - //in CHPL each user can only have a single role, but we are supporting the possibility that - //other applications could need something different - .authorities(decodeJwt.getClaims().containsKey(CognitoApiWrapper.ROLES_ATTRIBUTE_NAME) - ? Stream.of(decodeJwt.getClaim(CognitoApiWrapper.ROLES_ATTRIBUTE_NAME).asString().split(",")) - .map(attr -> new SimpleGrantedAuthority(attr)) - .collect(Collectors.toSet()) - : null) + .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 a4d6bf929e..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; @@ -46,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"); } From 2a4f2bb4ddf6c9719ca02c2354c481c1e26f7521 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 13 Jan 2025 09:39:55 -0500 Subject: [PATCH 7/9] fix: Better handle empty Insights response We were throwing an unexpected IOException if the Insights response was empty/null so this just tries to handle that better. Print a warning if it's empty (because I don't think it ever *should* be) but don't have unhandled exceptions. [#OCD-4782] --- .../main/java/gov/healthit/chpl/insight/InsightService.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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); From e12427a83264fe53f799efc468bc5a63c5943cca Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Tue, 14 Jan 2025 10:11:45 -0500 Subject: [PATCH 8/9] test[aqa]: Use non-retired ACB that has enabled users when appropriate [#OCD-4782] --- .../acb-controller.postman_collection.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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" ] } From 434c58468e4eb3e732aee2a69b88d31a5628a4ea Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 22 Jan 2025 13:36:23 -0500 Subject: [PATCH 9/9] fix: Use correct SpEL for ACB update [#OCD-4782] --- .../gov/healthit/chpl/manager/CertificationBodyManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 {