From c62254667692c0b6419de9a2133cfd3778604151 Mon Sep 17 00:00:00 2001 From: elisa lee Date: Thu, 1 Aug 2024 10:26:28 -0500 Subject: [PATCH 1/2] Add mutation to clear user roles and facilities --- .../api/apiuser/UserMutationResolver.java | 6 +++ .../usds/simplereport/db/model/ApiUser.java | 10 +++++ .../simplereport/db/model/ApiUserRole.java | 2 + .../simplereport/service/ApiUserService.java | 15 +++++++ .../src/main/resources/graphql/admin.graphqls | 3 ++ .../api/apiuser/UserMutationResolverTest.java | 40 ++++++++++++++++- .../service/ApiUserServiceTest.java | 45 +++++++++++++++++++ frontend/src/generated/graphql.tsx | 5 +++ 8 files changed, 124 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java b/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java index 6d10b8da8f..1c74662e38 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java @@ -174,4 +174,10 @@ public User updateUserPrivilegesAndGroupAccess( username, orgExternalId, accessAllFacilities, facilityIdsToAssign, role); return new User(_us.getUserByLoginEmail(username)); } + + @AuthorizationConfiguration.RequireGlobalAdminUser + @MutationMapping + public ApiUser markUserRolesAndFacilitiesAsDeleted(@Argument String username) { + return _us.markUserRolesAndFacilitiesAsDeleted(username); + } } diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUser.java b/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUser.java index a3130e18ba..a44d85eb8f 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUser.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUser.java @@ -71,6 +71,10 @@ public void setFacilities(Set facilities) { } } + public Set getRoles() { + return this.roleAssignments.stream().map(ApiUserRole::getRole).collect(Collectors.toSet()); + } + public void setRoles(Set newOrgRoles, Organization org) { this.roleAssignments.clear(); for (OrganizationRole orgRole : newOrgRoles) { @@ -82,4 +86,10 @@ public void setRoles(Set newOrgRoles, Organization org) { this.roleAssignments.add(new ApiUserRole(this, org, orgRole)); } } + + public ApiUser clearRolesAndFacilities() { + this.roleAssignments.clear(); + this.facilityAssignments.clear(); + return this; + } } diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUserRole.java b/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUserRole.java index 7c02843b32..59e118184b 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUserRole.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/db/model/ApiUserRole.java @@ -7,6 +7,7 @@ import jakarta.persistence.Enumerated; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; +import lombok.Getter; import org.hibernate.annotations.JdbcTypeCode; import org.hibernate.type.SqlTypes; @@ -24,6 +25,7 @@ public class ApiUserRole extends AuditedEntity { @Column(nullable = false, columnDefinition = "organization_role") @JdbcTypeCode(SqlTypes.NAMED_ENUM) @Enumerated(EnumType.STRING) + @Getter private OrganizationRole role; protected ApiUserRole() { diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java b/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java index 84a7e80407..5df3e95d56 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java @@ -376,6 +376,21 @@ public UserInfo resendActivationEmail(UUID userId) { return new UserInfo(apiUser, Optional.of(orgRoles), false); } + /** + * Clear a user's roles and facilities - intended to be used on site admin users only, + * non-site-admin users will be in a misconfigured state without roles and facilities + * + * @param username + * @return ApiUser + */ + @AuthorizationConfiguration.RequireGlobalAdminUser + public ApiUser markUserRolesAndFacilitiesAsDeleted(String username) { + ApiUser foundUser = + _apiUserRepo.findByLoginEmail(username).orElseThrow(NonexistentUserException::new); + foundUser.clearRolesAndFacilities(); + return foundUser; + } + private ApiUser getApiUser(UUID id) { return getApiUser(id, false); } diff --git a/backend/src/main/resources/graphql/admin.graphqls b/backend/src/main/resources/graphql/admin.graphqls index a220082d86..d2fdf478ab 100644 --- a/backend/src/main/resources/graphql/admin.graphqls +++ b/backend/src/main/resources/graphql/admin.graphqls @@ -64,6 +64,9 @@ extend type Mutation { accessAllFacilities: Boolean = false, facilities: [ID] = [], role: Role!): User! + markUserRolesAndFacilitiesAsDeleted( + username: String! + ): ApiUser updateFeatureFlag(name: String!, value: Boolean!):FeatureFlag deleteE2EOktaOrganizations(orgExternalId: String!): Organization sendOrgAdminEmailCSV(type: String!, state: String!): Boolean diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java index 05091cca0c..54f9142398 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java @@ -1,12 +1,17 @@ package gov.cdc.usds.simplereport.api.apiuser; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import gov.cdc.usds.simplereport.api.model.Role; import gov.cdc.usds.simplereport.api.model.User; +import gov.cdc.usds.simplereport.api.model.errors.NonexistentUserException; +import gov.cdc.usds.simplereport.db.model.ApiUser; import gov.cdc.usds.simplereport.db.model.Organization; +import gov.cdc.usds.simplereport.db.repository.ApiUserRepository; import gov.cdc.usds.simplereport.service.ApiUserService; import gov.cdc.usds.simplereport.service.BaseServiceTest; import gov.cdc.usds.simplereport.service.model.UserInfo; @@ -21,6 +26,7 @@ @WithSimpleReportOrgAdminUser class UserMutationResolverTest extends BaseServiceTest { + @Autowired ApiUserRepository apiUserRepository; @Mock ApiUserService mockedApiUserService; @Autowired private TestDataFactory _dataFactory; @@ -32,11 +38,11 @@ class UserMutationResolverTest extends BaseServiceTest { @BeforeEach void setup() { Organization org = _dataFactory.saveValidOrganization(); - orgUserInfo = _dataFactory.createValidApiUser("demo@example.com", org); + orgUserInfo = _dataFactory.createValidApiUser("demo@example.com", org, Role.USER); } @Test - void reactivateUserAndResetPassword_orgAdmin_success() { + void reactivateUserAndResetPassword_success() { UUID userInfoInternalId = orgUserInfo.getInternalId(); // GIVEN @@ -50,4 +56,34 @@ void reactivateUserAndResetPassword_orgAdmin_success() { assertThat(resetUser.getInternalId()).isEqualTo(userInfoInternalId); verify(mockedApiUserService, times(1)).reactivateUserAndResetPassword(userInfoInternalId); } + + @Test + void markUserRolesAndFacilitiesAsDeleted_nonExistentUser_throwException() { + String username = orgUserInfo.getEmail(); + + // GIVEN + ApiUser foundUser = apiUserRepository.findByLoginEmail(username).get(); + when(mockedApiUserService.markUserRolesAndFacilitiesAsDeleted(username)).thenReturn(foundUser); + + // WHEN + ApiUser resetUser = userMutationResolver.markUserRolesAndFacilitiesAsDeleted(username); + + // THEN + assertThat(resetUser.getLoginEmail()).isEqualTo(orgUserInfo.getEmail()); + verify(mockedApiUserService, times(1)).markUserRolesAndFacilitiesAsDeleted(username); + } + + @Test + void markUserRolesAndFacilitiesAsDeleted_failure() { + String username = "nonexistentuser@examplemail.com"; + + when(mockedApiUserService.markUserRolesAndFacilitiesAsDeleted(username)) + .thenThrow(new NonexistentUserException()); + + assertThrows( + NonexistentUserException.class, + () -> { + userMutationResolver.markUserRolesAndFacilitiesAsDeleted(username); + }); + } } diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java index d8c0257a04..156856b795 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java @@ -35,6 +35,7 @@ import gov.cdc.usds.simplereport.test_util.SliceTestConfiguration.WithSimpleReportOrgAdminUser; import gov.cdc.usds.simplereport.test_util.SliceTestConfiguration.WithSimpleReportSiteAdminUser; import gov.cdc.usds.simplereport.test_util.TestDataFactory; +import gov.cdc.usds.simplereport.test_util.TestUserIdentities; import java.util.Collections; import java.util.EnumSet; import java.util.List; @@ -47,7 +48,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.security.access.AccessDeniedException; +import org.springframework.test.context.TestPropertySource; +@TestPropertySource(properties = {"spring.jpa.properties.hibernate.enable_lazy_load_no_trans=true"}) class ApiUserServiceTest extends BaseServiceTest { @Autowired @SpyBean ApiUserRepository _apiUserRepo; @@ -564,6 +567,48 @@ void updateUserPrivilegesAndGroupAccess_facilityToMoveNotFoundInOrg_throwsExcept assertEquals(expectedError, caught.getMessage()); } + @Test + @WithSimpleReportOrgAdminUser + void orgAdminUser_markUserRolesAndFacilitiesAsDeleted_error() { + String username = "nonexistentuser@examplemail.com"; + assertThrows( + AccessDeniedException.class, () -> _service.markUserRolesAndFacilitiesAsDeleted(username)); + } + + @Test + @WithSimpleReportSiteAdminUser + void siteAdminUser_markUserRolesAndFacilitiesAsDeleted_nonExistentUser_throws() { + final String email = "nonexistentuser@examplemail.com"; + + assertThrows( + NonexistentUserException.class, + () -> { + _service.markUserRolesAndFacilitiesAsDeleted(email); + }); + } + + @Test + @WithSimpleReportSiteAdminUser + void siteAdminUser_markUserRolesAndFacilitiesAsDeleted_success() { + initSampleData(); + final String email = TestUserIdentities.STANDARD_USER; + ApiUser foundUser = _apiUserRepo.findByLoginEmail(email).get(); + + // check initial facilities and roles + int initialFacilitiesCount = foundUser.getFacilities().size(); + Set initialOrgRoles = foundUser.getRoles(); + assertEquals(1, initialFacilitiesCount); + assertEquals(1, initialOrgRoles.size()); + assertEquals("USER", initialOrgRoles.stream().findFirst().get().getName()); + + ApiUser updatedUser = _service.markUserRolesAndFacilitiesAsDeleted(email); + // check facilities and roles after deletion + int updatedFacilitiesCount = updatedUser.getFacilities().size(); + int updatedOrgRolesCount = updatedUser.getRoles().size(); + assertEquals(0, updatedFacilitiesCount); + assertEquals(0, updatedOrgRolesCount); + } + private void roleCheck(final UserInfo userInfo, final Set expected) { EnumSet actual = EnumSet.copyOf(userInfo.getRoles()); assertEquals(expected, actual); diff --git a/frontend/src/generated/graphql.tsx b/frontend/src/generated/graphql.tsx index e6fc4e3788..e358993c4c 100644 --- a/frontend/src/generated/graphql.tsx +++ b/frontend/src/generated/graphql.tsx @@ -209,6 +209,7 @@ export type Mutation = { markFacilityAsDeleted?: Maybe; markOrganizationAsDeleted?: Maybe; markPendingOrganizationAsDeleted?: Maybe; + markUserRolesAndFacilitiesAsDeleted?: Maybe; reactivateUser?: Maybe; reactivateUserAndResetPassword?: Maybe; removePatientFromQueue?: Maybe; @@ -382,6 +383,10 @@ export type MutationMarkPendingOrganizationAsDeletedArgs = { orgExternalId: Scalars["String"]["input"]; }; +export type MutationMarkUserRolesAndFacilitiesAsDeletedArgs = { + username: Scalars["String"]["input"]; +}; + export type MutationReactivateUserArgs = { id: Scalars["ID"]["input"]; }; From fec76806d6291011b3405069baf933c4f18ff443 Mon Sep 17 00:00:00 2001 From: elisa lee Date: Mon, 5 Aug 2024 09:59:21 -0500 Subject: [PATCH 2/2] rename method --- .../api/apiuser/UserMutationResolver.java | 4 ++-- .../usds/simplereport/service/ApiUserService.java | 2 +- backend/src/main/resources/graphql/admin.graphqls | 2 +- .../api/apiuser/UserMutationResolverTest.java | 14 +++++++------- .../simplereport/service/ApiUserServiceTest.java | 13 ++++++------- frontend/src/generated/graphql.tsx | 10 +++++----- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java b/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java index 1c74662e38..ca6da655b0 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolver.java @@ -177,7 +177,7 @@ public User updateUserPrivilegesAndGroupAccess( @AuthorizationConfiguration.RequireGlobalAdminUser @MutationMapping - public ApiUser markUserRolesAndFacilitiesAsDeleted(@Argument String username) { - return _us.markUserRolesAndFacilitiesAsDeleted(username); + public ApiUser clearUserRolesAndFacilities(@Argument String username) { + return _us.clearUserRolesAndFacilities(username); } } diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java b/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java index 5df3e95d56..1e2dc6b888 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/service/ApiUserService.java @@ -384,7 +384,7 @@ public UserInfo resendActivationEmail(UUID userId) { * @return ApiUser */ @AuthorizationConfiguration.RequireGlobalAdminUser - public ApiUser markUserRolesAndFacilitiesAsDeleted(String username) { + public ApiUser clearUserRolesAndFacilities(String username) { ApiUser foundUser = _apiUserRepo.findByLoginEmail(username).orElseThrow(NonexistentUserException::new); foundUser.clearRolesAndFacilities(); diff --git a/backend/src/main/resources/graphql/admin.graphqls b/backend/src/main/resources/graphql/admin.graphqls index d2fdf478ab..a0eaa9f90c 100644 --- a/backend/src/main/resources/graphql/admin.graphqls +++ b/backend/src/main/resources/graphql/admin.graphqls @@ -64,7 +64,7 @@ extend type Mutation { accessAllFacilities: Boolean = false, facilities: [ID] = [], role: Role!): User! - markUserRolesAndFacilitiesAsDeleted( + clearUserRolesAndFacilities( username: String! ): ApiUser updateFeatureFlag(name: String!, value: Boolean!):FeatureFlag diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java index 54f9142398..ac1bdb9fd3 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/apiuser/UserMutationResolverTest.java @@ -58,32 +58,32 @@ void reactivateUserAndResetPassword_success() { } @Test - void markUserRolesAndFacilitiesAsDeleted_nonExistentUser_throwException() { + void clearUserRolesAndFacilities_nonExistentUser_throwException() { String username = orgUserInfo.getEmail(); // GIVEN ApiUser foundUser = apiUserRepository.findByLoginEmail(username).get(); - when(mockedApiUserService.markUserRolesAndFacilitiesAsDeleted(username)).thenReturn(foundUser); + when(mockedApiUserService.clearUserRolesAndFacilities(username)).thenReturn(foundUser); // WHEN - ApiUser resetUser = userMutationResolver.markUserRolesAndFacilitiesAsDeleted(username); + ApiUser resetUser = userMutationResolver.clearUserRolesAndFacilities(username); // THEN assertThat(resetUser.getLoginEmail()).isEqualTo(orgUserInfo.getEmail()); - verify(mockedApiUserService, times(1)).markUserRolesAndFacilitiesAsDeleted(username); + verify(mockedApiUserService, times(1)).clearUserRolesAndFacilities(username); } @Test - void markUserRolesAndFacilitiesAsDeleted_failure() { + void clearUserRolesAndFacilities_failure() { String username = "nonexistentuser@examplemail.com"; - when(mockedApiUserService.markUserRolesAndFacilitiesAsDeleted(username)) + when(mockedApiUserService.clearUserRolesAndFacilities(username)) .thenThrow(new NonexistentUserException()); assertThrows( NonexistentUserException.class, () -> { - userMutationResolver.markUserRolesAndFacilitiesAsDeleted(username); + userMutationResolver.clearUserRolesAndFacilities(username); }); } } diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java index 156856b795..106e30a2f7 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/service/ApiUserServiceTest.java @@ -569,27 +569,26 @@ void updateUserPrivilegesAndGroupAccess_facilityToMoveNotFoundInOrg_throwsExcept @Test @WithSimpleReportOrgAdminUser - void orgAdminUser_markUserRolesAndFacilitiesAsDeleted_error() { + void orgAdminUser_clearUserRolesAndFacilities_error() { String username = "nonexistentuser@examplemail.com"; - assertThrows( - AccessDeniedException.class, () -> _service.markUserRolesAndFacilitiesAsDeleted(username)); + assertThrows(AccessDeniedException.class, () -> _service.clearUserRolesAndFacilities(username)); } @Test @WithSimpleReportSiteAdminUser - void siteAdminUser_markUserRolesAndFacilitiesAsDeleted_nonExistentUser_throws() { + void siteAdminUser_clearUserRolesAndFacilities_nonExistentUser_throws() { final String email = "nonexistentuser@examplemail.com"; assertThrows( NonexistentUserException.class, () -> { - _service.markUserRolesAndFacilitiesAsDeleted(email); + _service.clearUserRolesAndFacilities(email); }); } @Test @WithSimpleReportSiteAdminUser - void siteAdminUser_markUserRolesAndFacilitiesAsDeleted_success() { + void siteAdminUser_clearUserRolesAndFacilities_success() { initSampleData(); final String email = TestUserIdentities.STANDARD_USER; ApiUser foundUser = _apiUserRepo.findByLoginEmail(email).get(); @@ -601,7 +600,7 @@ void siteAdminUser_markUserRolesAndFacilitiesAsDeleted_success() { assertEquals(1, initialOrgRoles.size()); assertEquals("USER", initialOrgRoles.stream().findFirst().get().getName()); - ApiUser updatedUser = _service.markUserRolesAndFacilitiesAsDeleted(email); + ApiUser updatedUser = _service.clearUserRolesAndFacilities(email); // check facilities and roles after deletion int updatedFacilitiesCount = updatedUser.getFacilities().size(); int updatedOrgRolesCount = updatedUser.getRoles().size(); diff --git a/frontend/src/generated/graphql.tsx b/frontend/src/generated/graphql.tsx index e358993c4c..20f4d285cc 100644 --- a/frontend/src/generated/graphql.tsx +++ b/frontend/src/generated/graphql.tsx @@ -195,6 +195,7 @@ export type Mutation = { addUser?: Maybe; addUserToCurrentOrg?: Maybe; adminUpdateOrganization?: Maybe; + clearUserRolesAndFacilities?: Maybe; correctTestMarkAsCorrection?: Maybe; correctTestMarkAsError?: Maybe; createApiUserNoOkta?: Maybe; @@ -209,7 +210,6 @@ export type Mutation = { markFacilityAsDeleted?: Maybe; markOrganizationAsDeleted?: Maybe; markPendingOrganizationAsDeleted?: Maybe; - markUserRolesAndFacilitiesAsDeleted?: Maybe; reactivateUser?: Maybe; reactivateUserAndResetPassword?: Maybe; removePatientFromQueue?: Maybe; @@ -304,6 +304,10 @@ export type MutationAdminUpdateOrganizationArgs = { type: Scalars["String"]["input"]; }; +export type MutationClearUserRolesAndFacilitiesArgs = { + username: Scalars["String"]["input"]; +}; + export type MutationCorrectTestMarkAsCorrectionArgs = { id: Scalars["ID"]["input"]; reason?: InputMaybe; @@ -383,10 +387,6 @@ export type MutationMarkPendingOrganizationAsDeletedArgs = { orgExternalId: Scalars["String"]["input"]; }; -export type MutationMarkUserRolesAndFacilitiesAsDeletedArgs = { - username: Scalars["String"]["input"]; -}; - export type MutationReactivateUserArgs = { id: Scalars["ID"]["input"]; };