feat!: Remove "includeDisabled" request param from GET user endpoints

Breaking Change [#OCD-4734]
chpladmin · Dec 16, 2024 · 1afb8dd · 1afb8dd
1 parent 368c7a2
commit 1afb8dd
Show file tree

Hide file tree

Showing 13 changed files with 28 additions and 142 deletions.
diff --git a/.../chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java b/.../chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertificationBodyController.java
@@ -2,9 +2,7 @@
 
 import java.util.List;
 
-import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.ObjectUtils;
-import org.apache.commons.lang3.StringUtils;
 import org.quartz.SchedulerException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.MediaType;
@@ -35,8 +33,6 @@
 import gov.healthit.chpl.util.SwaggerSecurityRequirement;
 import gov.healthit.chpl.web.controller.results.CertificationBodyResults;
 import io.swagger.v3.oas.annotations.Operation;
-import io.swagger.v3.oas.annotations.Parameter;
-import io.swagger.v3.oas.annotations.enums.ParameterIn;
 import io.swagger.v3.oas.annotations.media.Content;
 import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.responses.ApiResponse;
@@ -245,15 +241,8 @@ public String deleteUserFromAcb(@PathVariable final Long acbId, @PathVariable fi
     })
     @RequestMapping(value = "/{acbId}/users", method = RequestMethod.GET,
             produces = "application/json; charset=utf-8")
-    public @ResponseBody UsersResponse getUsers(@PathVariable("acbId") Long acbId,
-            @Parameter(description = "Whether to include users whose accounts have been marked as disabled. "
-                    + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). "
-                    + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.",
-                allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled")
-            @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabled)
-            throws InvalidArgumentsException, EntityRetrievalException {
-        List<User> users = acbManager.getUsers(acbId,
-                StringUtils.isEmpty(includeDisabled) ? false : BooleanUtils.toBoolean(includeDisabled));
+    public @ResponseBody UsersResponse getUsers(@PathVariable("acbId") Long acbId) throws InvalidArgumentsException, EntityRetrievalException {
+        List<User> users = acbManager.getUsers(acbId);
 
         UsersResponse results = new UsersResponse();
         results.setUsers(users);

diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/DeveloperController.java
@@ -4,9 +4,7 @@
 import java.util.List;
 
 import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.NotImplementedException;
-import org.apache.commons.lang3.StringUtils;
 import org.ff4j.FF4j;
 import org.quartz.SchedulerException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -18,7 +16,6 @@
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
-import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -60,8 +57,6 @@
 import gov.healthit.chpl.web.controller.results.DeveloperAttestationSubmissionResults;
 import gov.healthit.chpl.web.controller.results.DeveloperResults;
 import io.swagger.v3.oas.annotations.Operation;
-import io.swagger.v3.oas.annotations.Parameter;
-import io.swagger.v3.oas.annotations.enums.ParameterIn;
 import io.swagger.v3.oas.annotations.security.SecurityRequirement;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import lombok.extern.log4j.Log4j2;
@@ -309,15 +304,9 @@ public PermissionDeletedResponse deleteUserFromDeveloper(@PathVariable Long deve
             })
     @RequestMapping(value = "/{developerId}/users", method = RequestMethod.GET,
             produces = "application/json; charset=utf-8")
-    public @ResponseBody UsersResponse getUsers(@PathVariable("developerId") Long developerId,
-        @Parameter(description = "Whether to include users whose accounts have been marked as disabled. "
-                + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). "
-                + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.",
-            allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled")
-        @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabled)
-    throws InvalidArgumentsException, EntityRetrievalException {
-        List<User> domainUsers = developerManager.getAllUsersOnDeveloper(developerId,
-                StringUtils.isEmpty(includeDisabled) ? false : BooleanUtils.toBoolean(includeDisabled));
+    public @ResponseBody UsersResponse getUsers(@PathVariable("developerId") Long developerId)
+            throws InvalidArgumentsException, EntityRetrievalException {
+        List<User> domainUsers = developerManager.getAllUsersOnDeveloper(developerId);
         UsersResponse results = new UsersResponse();
         results.setUsers(domainUsers);
         return results;

diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/UserManagementController.java
@@ -7,7 +7,6 @@
 import java.util.Set;
 import java.util.UUID;
 
-import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.NotImplementedException;
 import org.apache.commons.lang3.StringUtils;
 import org.ff4j.FF4j;
@@ -22,7 +21,6 @@
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
-import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -64,8 +62,6 @@
 import gov.healthit.chpl.web.controller.annotation.DeprecatedApi;
 import gov.healthit.chpl.web.controller.annotation.DeprecatedApiResponseFields;
 import io.swagger.v3.oas.annotations.Operation;
-import io.swagger.v3.oas.annotations.Parameter;
-import io.swagger.v3.oas.annotations.enums.ParameterIn;
 import io.swagger.v3.oas.annotations.security.SecurityRequirement;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import lombok.Getter;
@@ -525,18 +521,12 @@ public DeletedUser deleteUser(@PathVariable("userId") Long userId) throws UserRe
             })
     @RequestMapping(value = "", method = RequestMethod.GET, produces = "application/json; charset=utf-8")
     @PreAuthorize("isAuthenticated()")
-    public @ResponseBody UsersResponse getUsers(
-            @Parameter(description = "Whether to include users whose accounts have been marked as disabled. "
-                    + "Any string that can be evaluated as a boolean may be passed in (ex: true, false, off, on, yes, no). "
-                    + "The parameter only affects the response when called by an authenticated ADMIN or ONC user.",
-                allowEmptyValue = true, in = ParameterIn.QUERY, name = "includeDisabled")
-            @RequestParam(value = "includeDisabled", required = false, defaultValue = "false") String includeDisabledStr) {
-        boolean includeDisabled = StringUtils.isEmpty(includeDisabledStr) ? false : BooleanUtils.toBoolean(includeDisabledStr);
+    public @ResponseBody UsersResponse getUsers() {
         List<User> users = null;
         if (ff4j.check(FeatureList.SSO)) {
-            users = getAllCognitoUsers(includeDisabled);
+            users = getAllCognitoUsers();
         } else {
-            users = getAllChplUsers(includeDisabled);
+            users = getAllChplUsers();
         }
 
         UsersResponse response = new UsersResponse();
@@ -564,8 +554,8 @@ public DeletedUser deleteUser(@PathVariable("userId") Long userId) throws UserRe
         return userManager.getUserInfo(id);
     }
 
-    private List<User> getAllChplUsers(Boolean includeDisabled) {
-        List<UserDTO> userList = userManager.getAll(includeDisabled);
+    private List<User> getAllChplUsers() {
+        List<UserDTO> userList = userManager.getAll();
         List<User> users = new ArrayList<User>(userList.size());
 
         for (UserDTO userDto : userList) {
@@ -575,8 +565,8 @@ private List<User> getAllChplUsers(Boolean includeDisabled) {
         return users;
     }
 
-    private List<User> getAllCognitoUsers(Boolean includeDisabled) {
-        return cognitoUserManager.getAll(includeDisabled);
+    private List<User> getAllCognitoUsers() {
+        return cognitoUserManager.getAll();
     }
 
     private class DeletedUser {

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
@@ -210,18 +210,13 @@ public CertificationBody getById(Long id) throws EntityRetrievalException {
     @Transactional(readOnly = true)
     @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).CERTIFICATION_BODY, "
             + "T(gov.healthit.chpl.permissions.domains.CertificationBodyDomainPermissions).GET_USERS, #acbId)")
-    public List<User> getUsers(Long acbId, boolean includeDisabled)
-            throws InvalidArgumentsException, EntityRetrievalException {
+    public List<User> getUsers(Long acbId) throws InvalidArgumentsException, EntityRetrievalException {
         CertificationBody acb = resourcePermissionsFactory.get().getAcbIfPermissionById(acbId);
         if (acb == null) {
             throw new InvalidArgumentsException("Could not find the ACB specified.");
         }
 
-        if (!resourcePermissionsFactory.get().isUserRoleAdmin()
-                && !resourcePermissionsFactory.get().isUserRoleOnc()) {
-            includeDisabled = false;
-        }
-        List<User> users = resourcePermissionsFactory.get().getAllUsersOnAcb(acb, includeDisabled);
+        List<User> users = resourcePermissionsFactory.get().getAllUsersOnAcb(acb);
         return users;
     }
 

diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/DeveloperManager.java
@@ -245,13 +245,9 @@ private SimpleListing convertToSimpleListing(CertifiedProductDetailsDTO listingD
     @Transactional(readOnly = true)
     @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, "
             + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS, #devId)")
-    public List<User> getAllUsersOnDeveloper(Long devId, boolean includeDisabled) throws EntityRetrievalException {
+    public List<User> getAllUsersOnDeveloper(Long devId) throws EntityRetrievalException {
         Developer dev = getById(devId);
-        if (!resourcePermissionsFactory.get().isUserRoleAdmin()
-                && !resourcePermissionsFactory.get().isUserRoleOnc()) {
-            includeDisabled = false;
-        }
-        List<User> users = resourcePermissionsFactory.get().getAllUsersOnDeveloper(dev, includeDisabled);
+        List<User> users = resourcePermissionsFactory.get().getAllUsersOnDeveloper(dev);
         return users;
     }
 

diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/auth/UserManager.java
@@ -9,9 +9,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
-import java.util.stream.Collectors;
 
-import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -170,18 +168,8 @@ public void delete(UserDTO user) throws UserRetrievalException, ActivityExceptio
     @Transactional
     @PostFilter("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).SECURED_USER, "
             + "T(gov.healthit.chpl.permissions.domains.SecuredUserDomainPermissions).GET_ALL, filterObject)")
-    public List<UserDTO> getAll(Boolean includeDisabled) {
-        if (includeDisabled == null
-                ||  (!resourcePermissionsFactory.get().isUserRoleAdmin()
-                && !resourcePermissionsFactory.get().isUserRoleOnc())) {
-            includeDisabled = false;
-        }
+    public List<UserDTO> getAll() {
         List<UserDTO> allUsers = userDAO.findAll();
-        if (includeDisabled == null || BooleanUtils.isFalse(includeDisabled)) {
-            return allUsers.stream()
-                    .filter(user -> user.isAccountEnabled())
-                    .collect(Collectors.toList());
-        }
         return allUsers;
     }
 

diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/ChplResourcePermissions.java
@@ -68,34 +68,20 @@ public boolean isDeveloperNotBannedOrSuspended(Long developerId) {
     @Override
     @Transactional(readOnly = true)
     public List<User> getAllUsersOnAcb(CertificationBody acb) {
-        return getAllUsersOnAcb(acb, false);
-    }
-
-    @Override
-    @Transactional(readOnly = true)
-    public List<User> getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled) {
         List<UserCertificationBodyMapDTO> dtos = userCertificationBodyMapDAO.getByAcbId(acb.getId());
 
         return dtos.stream()
                 .map(dto -> dto.getUser().toDomain())
-                .filter(user -> includeDisabled ? true : user.getAccountEnabled())
                 .toList();
     }
 
     @Override
     @Transactional(readOnly = true)
     public List<User> getAllUsersOnDeveloper(Developer dev) {
-        return getAllUsersOnDeveloper(dev, false);
-    }
-
-    @Override
-    @Transactional(readOnly = true)
-    public List<User> getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) {
         List<UserDeveloperMapDTO> dtos = userDeveloperMapDAO.getByDeveloperId(dev.getId());
 
         return dtos.stream()
                 .map(udm -> udm.getUser().toDomain())
-                .filter(user -> includeDisabled ? true : user.getAccountEnabled())
                 .toList();
     }
 
@@ -104,30 +90,15 @@ public List<User> getAllUsersOnDeveloper(Developer dev, boolean includeDisabled)
     @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, "
             + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS)")
     public List<User> getAllDeveloperUsers() {
-        return getAllDeveloperUsers(false);
-    }
-
-    @Override
-    @Transactional(readOnly = true)
-    @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).DEVELOPER, "
-            + "T(gov.healthit.chpl.permissions.domains.DeveloperDomainPermissions).GET_ALL_USERS)")
-    public List<User> getAllDeveloperUsers(boolean includeDisabled) {
         List<UserDeveloperMapDTO> dtos = userDeveloperMapDAO.getAllDeveloperUsers();
         return dtos.stream()
                 .map(udm -> udm.getUser().toDomain())
-                .filter(user -> includeDisabled ? true : user.getAccountEnabled())
                 .toList();
     }
 
     @Override
     @Transactional(readOnly = true)
     public List<User> getAllUsersForCurrentUser() {
-        return getAllUsersForCurrentUser(false);
-    }
-
-    @Override
-    @Transactional(readOnly = true)
-    public List<User> getAllUsersForCurrentUser(boolean includeDisabled) {
         JWTAuthenticatedUser user = AuthUtil.getCurrentUser();
         List<User> users = new ArrayList<User>();
 
@@ -155,9 +126,7 @@ public List<User> getAllUsersForCurrentUser(boolean includeDisabled) {
                 } catch (UserRetrievalException ex) { }
             }
         }
-        return users.stream()
-                .filter(currUser -> includeDisabled ? true : currUser.getAccountEnabled())
-                .collect(Collectors.toList());
+        return users;
     }
 
     @Override

diff --git a/.../chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java b/.../chpl-service/src/main/java/gov/healthit/chpl/permissions/CognitoResourcePermissions.java
@@ -54,12 +54,7 @@ public boolean isDeveloperNotBannedOrSuspended(Long developerId) {
 
     @Override
     public List<User> getAllUsersOnAcb(CertificationBody acb) {
-        return getAllUsersOnAcb(acb, false);
-    }
-
-    @Override
-    public List<User> getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled) {
-        List<User> allUsersOnAcb = cognitoApiWrapper.getAllUsers(includeDisabled).stream()
+        List<User> allUsersOnAcb = cognitoApiWrapper.getAllUsers().stream()
                 .filter(user -> user.getRole() != null
                         && user.getRole().equals(CognitoGroups.CHPL_ACB)
                         && user.getOrganizations().stream()
@@ -73,12 +68,7 @@ public List<User> getAllUsersOnAcb(CertificationBody acb, boolean includeDisable
 
     @Override
     public List<User> getAllUsersOnDeveloper(Developer dev) {
-        return getAllUsersOnDeveloper(dev, false);
-    }
-
-    @Override
-    public List<User> getAllUsersOnDeveloper(Developer dev, boolean includeDisabled) {
-        List<User> allUsersOnDeveloper = cognitoApiWrapper.getAllUsers(includeDisabled).stream()
+        List<User> allUsersOnDeveloper = cognitoApiWrapper.getAllUsers().stream()
                 .filter(user -> user.getRole() != null
                         && user.getRole().equals(CognitoGroups.CHPL_DEVELOPER)
                         && user.getOrganizations().stream()
@@ -92,12 +82,7 @@ public List<User> getAllUsersOnDeveloper(Developer dev, boolean includeDisabled)
 
     @Override
     public List<User> getAllDeveloperUsers() {
-        return getAllDeveloperUsers(false);
-    }
-
-    @Override
-    public List<User> getAllDeveloperUsers(boolean includeDisabled) {
-        List<User> allDeveloperUsers = cognitoApiWrapper.getAllUsers(includeDisabled).stream()
+        List<User> allDeveloperUsers = cognitoApiWrapper.getAllUsers().stream()
                 .filter(user -> user.getRole() != null
                         && user.getRole().equals(CognitoGroups.CHPL_DEVELOPER))
                 .collect(Collectors.toList());
@@ -107,11 +92,6 @@ public List<User> getAllDeveloperUsers(boolean includeDisabled) {
 
     @Override
     public List<User> getAllUsersForCurrentUser() {
-        return getAllUsersForCurrentUser(false);
-    }
-
-    @Override
-    public List<User> getAllUsersForCurrentUser(boolean includeDisabled) {
         LOGGER.error("Not implemented: getAllUsersForCurrentUser");
         throw new NotImplementedException("Not implemented: getAllUsersForCurrentUser");
     }

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
@@ -14,20 +14,12 @@ public interface ResourcePermissions {
 
     List<User> getAllUsersOnAcb(CertificationBody acb);
 
-    List<User> getAllUsersOnAcb(CertificationBody acb, boolean includeDisabled);
-
     List<User> getAllUsersOnDeveloper(Developer dev);
 
-    List<User> getAllUsersOnDeveloper(Developer dev, boolean includeDisabled);
-
     List<User> getAllDeveloperUsers();
 
-    List<User> getAllDeveloperUsers(boolean includeDisabled);
-
     List<User> getAllUsersForCurrentUser();
 
-    List<User> getAllUsersForCurrentUser(boolean includeDisabled);
-
     List<CertificationBody> getAllAcbsForCurrentUser();
 
     List<CertificationBody> getAllAcbsForUser(User user);

diff --git a/...l-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java b/...l-service/src/main/java/gov/healthit/chpl/scheduler/job/MassRequirePasswordChangeJob.java
@@ -57,7 +57,7 @@ public void execute(JobExecutionContext jobContext) throws JobExecutionException
             String jwt = authenticationManager.getJWT(actor);
             JWTAuthenticatedUser authenticatedUser = userConverterFacade.getAuthenticatedUser(jwt);
             SecurityContextHolder.getContext().setAuthentication(authenticatedUser);
-            List<UserDTO> allUsers = userManager.getAll(false);
+            List<UserDTO> allUsers = userManager.getAll();
             for (UserDTO user : allUsers) {
                 if (interrupted) {
                     LOGGER.info("Interrupted while marking users as password change required");

diff --git a/...on/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java b/...on/email/missingchangerequest/MissingAttestationChangeRequestDeveloperEmailGenerator.java
@@ -48,7 +48,7 @@ public MissingAttestationChangeRequestDeveloperEmailGenerator(DeveloperManager d
 
     public DeveloperEmail getDeveloperEmail(DeveloperSearchResult developer, User submittedUser) {
         try {
-            List<User> developerUsers = developerManager.getAllUsersOnDeveloper(developer.getId(), false);
+            List<User> developerUsers = developerManager.getAllUsersOnDeveloper(developer.getId());
             return DeveloperEmail.builder()
                     .developer(developer)
                     .recipients(getRecipients(developerUsers))