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 1e2dc6b888..e94ed68d26 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 @@ -599,7 +599,13 @@ public UserInfo getCurrentUserInfoForWhoAmI() { Optional currentOrgRoles = _orgService.getCurrentOrganizationRoles(); boolean isAdmin = _authService.isSiteAdmin(); if (!_featureFlagsConfig.isOktaMigrationEnabled() && currentOrgRoles.isPresent() && !isAdmin) { - setRolesAndFacilities(currentOrgRoles.get(), currentUser); + try { + setRolesAndFacilities(currentOrgRoles.get(), currentUser); + } catch (PrivilegeUpdateFacilityAccessException e) { + log.warn( + "Could not migrate roles and facilities for user with id={} because facilities were invalid", + currentUser.getInternalId()); + } } return new UserInfo(currentUser, currentOrgRoles, isAdmin); } @@ -739,7 +745,13 @@ private UserInfo consolidateUser( OrganizationRoles orgRoles = new OrganizationRoles(org, accessibleFacilities, claims.getGrantedRoles()); if (!_featureFlagsConfig.isOktaMigrationEnabled() && !isSiteAdmin) { - setRolesAndFacilities(orgRoles, apiUser); + try { + setRolesAndFacilities(orgRoles, apiUser); + } catch (PrivilegeUpdateFacilityAccessException e) { + log.warn( + "Could not migrate roles and facilities for user with id={} because facilities were invalid", + apiUser.getInternalId()); + } } return new UserInfo(apiUser, Optional.of(orgRoles), isSiteAdmin, userStatus); } diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/BaseAuthenticatedFullStackTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/BaseAuthenticatedFullStackTest.java index c2828973a1..11eb5cb2a1 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/api/BaseAuthenticatedFullStackTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/BaseAuthenticatedFullStackTest.java @@ -65,6 +65,10 @@ protected void useBrokenUser() { username = TestUserIdentities.BROKEN_USER; } + protected void useInvalidFacilitiesUser() { + username = TestUserIdentities.INVALID_FACILITIES_USER; + } + @BeforeEach public void baseAuthenticatedFullStackTestSetup() { truncateDb(); diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/graphql/ApiUserManagementTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/graphql/ApiUserManagementTest.java index c6c8feb5ff..51197df8c7 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/api/graphql/ApiUserManagementTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/graphql/ApiUserManagementTest.java @@ -224,6 +224,16 @@ void whoami_nobody_okResponses() { assertUserHasNoOrg(who); } + @Test + void whoami_invalidFacilityAccess_okRolesFacilities() { + useInvalidFacilitiesUser(); + + ObjectNode who = (ObjectNode) runQuery("current-user-query").get("whoami"); + assertFalse(who.get("isAdmin").asBoolean()); + assertEquals(who.get("role").asText(), Role.USER.name()); + assertTrue(extractFacilitiesFromUser(who).isEmpty()); + } + @ParameterizedTest @ValueSource(strings = {"addUserOp", "addUserLegacy"}) void addUser_superUser_success(String operation) { 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 106e30a2f7..5702a91278 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 @@ -74,17 +74,19 @@ void cleanup() { void getUsersInCurrentOrg_adminUser_success() { initSampleData(); List users = _service.getUsersInCurrentOrg(); - assertEquals(5, users.size()); + assertEquals(6, users.size()); assertEquals("admin@example.com", users.get(0).getLoginEmail()); assertEquals("Andrews", users.get(0).getNameInfo().getLastName()); assertEquals("bobbity@example.com", users.get(1).getLoginEmail()); assertEquals("Bobberoo", users.get(1).getNameInfo().getLastName()); - assertEquals("nobody@example.com", users.get(2).getLoginEmail()); - assertEquals("Nixon", users.get(2).getNameInfo().getLastName()); - assertEquals("notruby@example.com", users.get(3).getLoginEmail()); - assertEquals("Reynolds", users.get(3).getNameInfo().getLastName()); - assertEquals("allfacilities@example.com", users.get(4).getLoginEmail()); - assertEquals("Williams", users.get(4).getNameInfo().getLastName()); + assertEquals("invalid@example.com", users.get(2).getLoginEmail()); + assertEquals("Irwin", users.get(2).getNameInfo().getLastName()); + assertEquals("nobody@example.com", users.get(3).getLoginEmail()); + assertEquals("Nixon", users.get(3).getNameInfo().getLastName()); + assertEquals("notruby@example.com", users.get(4).getLoginEmail()); + assertEquals("Reynolds", users.get(4).getNameInfo().getLastName()); + assertEquals("allfacilities@example.com", users.get(5).getLoginEmail()); + assertEquals("Williams", users.get(5).getNameInfo().getLastName()); } @Test @@ -103,14 +105,15 @@ void getUsersInCurrentOrg_standardUser_error() { void getUsersAndStatusInCurrentOrg_success() { initSampleData(); List users = _service.getUsersAndStatusInCurrentOrg(); - assertEquals(5, users.size()); + assertEquals(6, users.size()); checkApiUserWithStatus(users.get(0), "admin@example.com", "Andrews", UserStatus.ACTIVE); checkApiUserWithStatus(users.get(1), "bobbity@example.com", "Bobberoo", UserStatus.ACTIVE); - checkApiUserWithStatus(users.get(2), "nobody@example.com", "Nixon", UserStatus.ACTIVE); - checkApiUserWithStatus(users.get(3), "notruby@example.com", "Reynolds", UserStatus.ACTIVE); + checkApiUserWithStatus(users.get(2), "invalid@example.com", "Irwin", UserStatus.ACTIVE); + checkApiUserWithStatus(users.get(3), "nobody@example.com", "Nixon", UserStatus.ACTIVE); + checkApiUserWithStatus(users.get(4), "notruby@example.com", "Reynolds", UserStatus.ACTIVE); checkApiUserWithStatus( - users.get(4), "allfacilities@example.com", "Williams", UserStatus.ACTIVE); + users.get(5), "allfacilities@example.com", "Williams", UserStatus.ACTIVE); } @Test @@ -497,6 +500,18 @@ void getUserByLoginEmail_not_authorized() { assertEquals("Access Denied", caught.getMessage()); } + @Test + @WithSimpleReportSiteAdminUser + void getUserByLoginEmail_invalidClaims_success() { + initSampleData(); + String email = "invalid@example.com"; + + // we should be able to retrieve user info for a user with invalid claims (no facility access) + // without failing + UserInfo result = _service.getUserByLoginEmail(email); + assertThat(result.getFacilities()).isEmpty(); + } + @Test @WithSimpleReportSiteAdminUser void updateUserPrivilegesAndGroupAccess_assignToAllFacilities_success() { diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/test_util/TestUserIdentities.java b/backend/src/test/java/gov/cdc/usds/simplereport/test_util/TestUserIdentities.java index 9ff063cd6e..bcaea987c5 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/test_util/TestUserIdentities.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/test_util/TestUserIdentities.java @@ -29,6 +29,7 @@ public class TestUserIdentities { public static final String ENTRY_ONLY_USER = "nobody@example.com"; public static final String ORG_ADMIN_USER = "admin@example.com"; public static final String ALL_FACILITIES_USER = "allfacilities@example.com"; + public static final String INVALID_FACILITIES_USER = "invalid@example.com"; public static final String OTHER_ORG_USER = "intruder@pirate.com"; public static final String OTHER_ORG_ADMIN = "captain@pirate.com"; diff --git a/backend/src/test/resources/application-test.yaml b/backend/src/test/resources/application-test.yaml index 89c7937ab5..a16fad3904 100644 --- a/backend/src/test/resources/application-test.yaml +++ b/backend/src/test/resources/application-test.yaml @@ -123,6 +123,13 @@ simple-report: authorization: # USER_ALL_FACILITIES_ORG_ROLES organization-external-id: DIS_ORG granted-roles: NO_ACCESS, ADMIN + - identity: #standard user with invalid no facility access + username: invalid@example.com + first-name: Igor + last-name: Irwin + authorization: + organization-external-id: DIS_ORG + granted-roles: NO_ACCESS, USER - identity: # OUTSIDE_ORG_USER username: intruder@pirate.com first-name: Bootstrap