Release (#238)

* Refactor controller mappings and annotations for clarity (#236)
* Refactor role retrieval to return Set instead of Optional (#237)
Replaced Optional with direct Set return for dbgap role retrieval in `RoleService` to simplify logic and usage. Updated corresponding methods, validations, and test cases to align with the new return type. This also ensures users who have no permissions have their permissions updated.

Author: Gcolon021

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/rest/ConnectionWebController.java

@@ -49,7 +49,7 @@ public ResponseEntity<?> getConnectionById(
     }
 
     @Operation(description = "GET a list of existing Connection, requires SUPER_ADMIN or ADMIN role")
-    @GetMapping(value = "/", produces = "application/json")
+    @GetMapping
     @Secured({SUPER_ADMIN, ADMIN})
     public ResponseEntity<List<Connection>> getAllConnections() {
         List<Connection> allConnections = connectionWebService.getAllConnections();

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/rest/RoleController.java

@@ -25,7 +25,7 @@
  */
 @Tag(name = "Role Management")
 @Controller
-@RequestMapping(value = "/role")
+@RequestMapping("/role")
 public class RoleController {
 
     private final RoleService roleService;
@@ -49,8 +49,8 @@ public ResponseEntity<?> getRoleById(
     }
 
     @Operation(description = "GET a list of existing Roles, requires ADMIN or SUPER_ADMIN role")
-    @GetMapping(produces = "application/json")
     @RolesAllowed({ADMIN, SUPER_ADMIN})
+    @GetMapping
     public ResponseEntity<List<Role>> getRoleAll() {
         List<Role> allRoles = this.roleService.getAllRoles();
         return PICSUREResponse.success(allRoles);

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleService.java

@@ -285,21 +285,21 @@ public boolean upsertRole(User u, String roleName, String roleDescription) {
         return status;
     }
 
-    public Optional<Set<String>> getRoleNamesForDbgapPermissions(Set<RasDbgapPermission> dbgapPermissions) {
+    public Set<String> getRoleNamesForDbgapPermissions(Set<RasDbgapPermission> dbgapPermissions) {
+        Set<String> roles = new HashSet<>();
         if (dbgapPermissions == null || dbgapPermissions.isEmpty()) {
             logger.info("getRoleNamesForDbgapPermissions() dbgapPermissions is empty");
-            return Optional.empty();
+            return roles;
         }
 
-        Set<String> roles = new HashSet<>();
         dbgapPermissions.forEach(dbgapPermission -> {
             String roleName = StringUtils.isNotBlank(dbgapPermission.getConsentGroup()) ?
                     "MANAGED_" + dbgapPermission.getPhsId() + "_" + dbgapPermission.getConsentGroup() :
                     "MANAGED_" + dbgapPermission.getPhsId();
             roles.add(roleName);
         });
 
-        return Optional.of(roles);
+        return roles;
     }
 
     public Set<Role> findByNameIn(Set<String> roleNames) {

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/RASAuthenticationService.java

@@ -118,22 +118,20 @@ public HashMap<String, String> authenticate(Map<String, String> authRequest, Str
 
         if (!rasPassport.get().getIss().equals(this.rasPassportIssuer)) {
             logger.error("validateRASPassport() LOGIN FAILED ___ PASSPORT ISSUER IS NOT CORRECT ___ USER: {} ___ " +
-                    "EXPECTED ISSUER {} ___ ACTUAL ISSUER {} ___ CODE {}",
+                         "EXPECTED ISSUER {} ___ ACTUAL ISSUER {} ___ CODE {}",
                     user.getSubject(), this.rasPassportIssuer, rasPassport.get().getIss(), authRequest.get("code"));
             return null;
         }
 
         logger.info("RAS PASSPORT FOUND ___ USER: {} ___ PASSPORT: {} ___ CODE {}", user.getSubject(), rasPassport.get(), authRequest.get("code"));
 
         Set<RasDbgapPermission> dbgapPermissions = this.rasPassPortService.ga4gpPassportToRasDbgapPermissions(rasPassport.get().getGa4ghPassportV1());
-        Optional<Set<String>> dbgapRoleNames = this.roleService.getRoleNamesForDbgapPermissions(dbgapPermissions);
-        if (dbgapRoleNames.isPresent()) {
-            user = userService.updateUserRoles(user, dbgapRoleNames.get());
-            logger.debug("USER {} ROLES UPDATED {} ___ CODE {}",
-                    user.getSubject(),
-                    user.getRoles().stream().map(role -> role.getName().replace("MANAGED_", "")).toArray(),
-                    authRequest.get("code"));
-        }
+        Set<String> dbgapRoleNames = this.roleService.getRoleNamesForDbgapPermissions(dbgapPermissions);
+        user = userService.updateUserRoles(user, dbgapRoleNames);
+        logger.debug("USER {} ROLES UPDATED {} ___ CODE {}",
+                user.getSubject(),
+                user.getRoles().stream().map(role -> role.getName().replace("MANAGED_", "")).toArray(),
+                authRequest.get("code"));
 
         String passport = introspectResponse.get("passport_jwt_v11").toString();
         user.setPassport(passport);
@@ -183,7 +181,7 @@ private HashMap<String, String> createUserClaims(User user, String idToken) {
      * Generate the user metadata that will be stored in the database. This metadata is used to determine the user's
      * role and other information.
      *
-     * @param user               The user
+     * @param user The user
      * @return The user metadata as an ObjectNode
      */
     protected ObjectNode generateRasUserMetadata(User user) {

pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleServiceTest.java

@@ -290,16 +290,15 @@ private Role createTopAdminRoleWithoutPrivs() {
 
     @Test
     public void getRoleNamesForDbgapPermissions_PermissionsAreNull() {
-        Optional<Set<String>> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(null);
+        Set<String> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(null);
         assertNotNull(rolesForDbgapPermissions);
         assertTrue(rolesForDbgapPermissions.isEmpty());
     }
 
     @Test
     public void getRoleNamesForDbgapPermissions_PermissionsEmpty() {
         Set<RasDbgapPermission> rasDbgapPermissions = new HashSet<>();
-
-        Optional<Set<String>> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(rasDbgapPermissions);
+        Set<String> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(rasDbgapPermissions);
         assertNotNull(rolesForDbgapPermissions);
         assertTrue(rolesForDbgapPermissions.isEmpty());
     }
@@ -330,7 +329,7 @@ public void getRoleNamesForDbgapPermissions() {
         rasDbgapPermissions.add(rasDbgapPermissionV1);
         rasDbgapPermissions.add(rasDbgapPermissionV2);
 
-        Optional<Set<String>> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(rasDbgapPermissions);
+        Set<String> rolesForDbgapPermissions = roleService.getRoleNamesForDbgapPermissions(rasDbgapPermissions);
         assertNotNull(rolesForDbgapPermissions);
         System.out.println(rolesForDbgapPermissions);
     }