Dan/6338 org user list bug (#6802)

* paginate okta response on backend, add scrollbar to frontend * updating snapshot * updating unit tests * update mock to ensure it doesn't page * moving 'magic number' to constant
CDCgov · Oct 27, 2023 · 7828a38 · 7828a38
1 parent 8ddf077
commit 7828a38
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 12 deletions.
diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java
@@ -6,6 +6,7 @@
 import com.okta.sdk.resource.api.GroupApi;
 import com.okta.sdk.resource.api.UserApi;
 import com.okta.sdk.resource.client.ApiException;
+import com.okta.sdk.resource.common.PagedList;
 import com.okta.sdk.resource.group.GroupBuilder;
 import com.okta.sdk.resource.model.Application;
 import com.okta.sdk.resource.model.Error;
@@ -31,6 +32,7 @@
 import gov.cdc.usds.simplereport.db.model.Facility;
 import gov.cdc.usds.simplereport.db.model.Organization;
 import gov.cdc.usds.simplereport.service.model.IdentityAttributes;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashSet;
@@ -71,6 +73,7 @@ public class LiveOktaRepository implements OktaRepository {
   private final String adminGroupName;
 
   private static final String OKTA_ORG_PROFILE_MATCHER = "profile.name sw \"";
+  private static final int OKTA_PAGE_SIZE = 500;
 
   public LiveOktaRepository(
       AuthorizationProperties authorizationProperties,
@@ -211,21 +214,31 @@ private static void validateRequiredFields(IdentityAttributes userIdentity) {
 
   @Override
   public Set<String> getAllUsersForOrganization(Organization org) {
-    Group orgDefaultOktaGroup = getDefaultOktaGroup(org);
-    var groupUsers = groupApi.listGroupUsers(orgDefaultOktaGroup.getId(), null, null);
-    return groupUsers.stream()
+    return getAllUsersForOrg(org).stream()
         .map(u -> u.getProfile().getLogin())
         .collect(Collectors.toUnmodifiableSet());
   }
 
   @Override
   public Map<String, UserStatus> getAllUsersWithStatusForOrganization(Organization org) {
-    Group orgDefaultOktaGroup = getDefaultOktaGroup(org);
-    var groupUsers = groupApi.listGroupUsers(orgDefaultOktaGroup.getId(), null, null);
-    return groupUsers.stream()
+    return getAllUsersForOrg(org).stream()
         .collect(Collectors.toMap(u -> u.getProfile().getLogin(), User::getStatus));
   }
 
+  private List<User> getAllUsersForOrg(Organization org) {
+    PagedList<User> pagedUserList = new PagedList<>();
+    List<User> allUsers = new ArrayList<>();
+    Group orgDefaultOktaGroup = getDefaultOktaGroup(org);
+    do {
+      pagedUserList =
+          (PagedList<User>)
+              groupApi.listGroupUsers(
+                  orgDefaultOktaGroup.getId(), pagedUserList.getAfter(), OKTA_PAGE_SIZE);
+      allUsers.addAll(pagedUserList);
+    } while (pagedUserList.hasMoreItems());
+    return allUsers;
+  }
+
   private Group getDefaultOktaGroup(Organization org) {
     final String orgDefaultGroupName =
         generateRoleGroupName(org.getExternalId(), OrganizationRole.getDefault());

diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepositoryTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepositoryTest.java
@@ -20,6 +20,7 @@
 import com.okta.sdk.resource.api.GroupApi;
 import com.okta.sdk.resource.api.UserApi;
 import com.okta.sdk.resource.client.ApiException;
+import com.okta.sdk.resource.common.PagedList;
 import com.okta.sdk.resource.group.GroupBuilder;
 import com.okta.sdk.resource.model.Application;
 import com.okta.sdk.resource.model.Group;
@@ -586,7 +587,7 @@ void getAllUsersForOrganization() {
     var mockGroupList = List.of(mockGroup);
     var mockUser = mock(User.class);
     var mockUserProfile = mock(UserProfile.class);
-    var mockUserList = List.of(mockUser);
+    PagedList<User> mockedUserList = new PagedList<>(List.of(mockUser), "", "", null);
     when(groupApi.listGroups(
             eq(groupProfilePrefix),
             isNull(),
@@ -599,7 +600,7 @@ void getAllUsersForOrganization() {
         .thenReturn(mockGroupList);
     when(mockGroup.getProfile()).thenReturn(mockGroupProfile);
     when(mockGroupProfile.getName()).thenReturn(groupProfilePrefix);
-    when(groupApi.listGroupUsers(any(), isNull(), isNull())).thenReturn(mockUserList);
+    when(groupApi.listGroupUsers(any(), any(), any())).thenReturn(mockedUserList);
     when(mockUser.getProfile()).thenReturn(mockUserProfile);
     when(mockUserProfile.getLogin()).thenReturn("[email protected]");
 
@@ -633,7 +634,7 @@ void getAllUsersWithStatusForOrganization() {
     var mockGroupList = List.of(mockGroup);
     var mockGroupProfile = mock(GroupProfile.class);
     var mockUser = mock(User.class);
-    var mockUserList = List.of(mockUser);
+    PagedList<User> mockUserList = new PagedList<>(List.of(mockUser), "", "", null);
     var mockUserProfile = mock(UserProfile.class);
     when(groupApi.listGroups(
             eq(groupProfilePrefix),
@@ -648,7 +649,7 @@ void getAllUsersWithStatusForOrganization() {
     when(mockGroup.getProfile()).thenReturn(mockGroupProfile);
     when(mockGroupProfile.getName()).thenReturn(groupProfilePrefix);
     when(mockGroup.getId()).thenReturn("1234");
-    when(groupApi.listGroupUsers(eq("1234"), isNull(), isNull())).thenReturn(mockUserList);
+    when(groupApi.listGroupUsers(eq("1234"), any(), any())).thenReturn(mockUserList);
     when(mockUser.getProfile()).thenReturn(mockUserProfile);
     when(mockUserProfile.getLogin()).thenReturn("[email protected]");
     when(mockUser.getStatus()).thenReturn(UserStatus.ACTIVE);

diff --git a/frontend/src/app/Settings/Users/ManageUsers.scss b/frontend/src/app/Settings/Users/ManageUsers.scss
@@ -16,6 +16,11 @@
       margin-block-start: 1em;
       margin-block-end: 1em;
     }
+
+    .users-secondary-nav {
+      max-height: 44rem;
+      overflow-y: scroll;
+    }
   }
 
   .users-sidenav-item:hover {

diff --git a/frontend/src/app/Settings/Users/UsersSideNav.tsx b/frontend/src/app/Settings/Users/UsersSideNav.tsx
@@ -29,7 +29,10 @@ const UsersSideNav: React.FC<Props> = ({
   return (
     <div className="display-block users-sidenav">
       <h2 className="users-sidenav-header">Users</h2>
-      <nav className="prime-secondary-nav" aria-label="Tertiary navigation">
+      <nav
+        className="prime-secondary-nav users-secondary-nav"
+        aria-label="Tertiary navigation"
+      >
         <div
           role="tablist"
           aria-owns={getIdsAsString(users)}

diff --git a/frontend/src/app/Settings/Users/__snapshots__/ManageUsersContainer.test.tsx.snap b/frontend/src/app/Settings/Users/__snapshots__/ManageUsersContainer.test.tsx.snap
@@ -34,7 +34,7 @@ exports[`ManageUsersContainer loads the component and displays users successfull
           </h2>
           <nav
             aria-label="Tertiary navigation"
-            class="prime-secondary-nav"
+            class="prime-secondary-nav users-secondary-nav"
           >
             <div
               aria-owns="user-tab-3a4a221d-a8ca-42b3-aa03-37b93266025b user-tab-1029653e-24d9-428e-83b0-468319948902 user-tab-17656bad-08b6-4fd4-bb9a-ccac54e5ea0a user-tab-60bb9e3a-fe8a-4b81-b894-b01649c95e70 user-tab-0d3fa224-3d56-4382-b89c-9d8e415e59b3 user-tab-17656bad-07b6-4fd4-bb9a-ccbc54e5ea0a"