Fix Jakarta Issue (#248)

* Enable JSR-250 annotations in SecurityConfig

JSR-250 annotations (like @RolesAllowed) are now enabled in the SecurityConfig by setting `jsr250Enabled` to true. This enhances the flexibility of role-based access control within the application.

* Refactor role-based authority handling in CustomUserDetails

Updated the constructor to properly map user roles and dynamically add top admin privileges when applicable. Introduced new imports for improved role management and logging for debugging purposes. These changes enhance clarity and ensure more robust role-based access control.

* Add UUID initialization for Role and Privilege

Prevents test failing when using logging statements. The toString requires a uuid.

* Prevent security context setup after error responses

Added early return statements in JWTFilter to ensure the security context is not set up after sending error responses for unauthorized or forbidden access. This change avoids potential security issues and ensures proper request handling.

* Force eager loading of Privilege.accessRules to avoid LazyInitializationException

After switching granted authorities from privileges to roles, privilege.accessRules are no longer loaded into the Hibernate session when building CustomUserDetails. This change forces the fetch type to EAGER so that accessRules are preloaded with each Privilege entity and prevents LazyInitializationException. This will likely have negative consequences in our BioDataCatalyst environment and require a technical debt ticket to revert to lazy loading.

* Replace @secured with @RolesAllowed annotations for authorization

* Refactor role handling and remove default "ROLE_" prefix.

Updated `CustomUserDetails` to use privileges instead of roles and removed redundant logic for adding admin roles. Added `GrantedAuthorityDefaults` bean in `SecurityConfig` to eliminate the default "ROLE_" prefix for better alignment with custom authorities.

* Refactor role naming to use centralized AuthNaming constants

Removed the SecurityRoles enum and replaced its usage with centralized constants from AuthNaming.AuthRoleNaming.

* Revert EAGER fetch

* Remove SecurityRoles import across test files

File has been deleted

* Remove "ROLE_" prefix from role names and update usage

Changed role constants to remove the "ROLE_" prefix and updated the role naming convention. Added a GrantedAuthorityDefaults bean in SecurityConfig to ensure role-based security annotations match the updated role names.

* Update AuthRoleNaming documentation and remove unused constant

Refined the JavaDoc for AuthRoleNaming to clarify its purpose within @RolesAllowed annotations. Removed the unused constant `PIC_SURE_TOP_ADMIN`.

* Simplify role deletion logic

Removed security context checks in `RoleService` to streamline role deletion. The removeById RoleController endpoint already verifies the user is a SUPER_ADMIN. The privilege is only assigned to PIC-SURE Top Admins.

* Refactor RoleService tests and remove unnecessary methods

Consolidated RoleService test cases by removing duplicate methods and irrelevant tests.

* Cleaned up JWTFilter comments

Author: Gcolon021

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/config/SecurityConfig.java

@@ -12,13 +12,14 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
+import org.springframework.security.config.core.GrantedAuthorityDefaults;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
 
 import static org.springframework.security.config.http.SessionCreationPolicy.STATELESS;
 
 @Configuration
-@EnableMethodSecurity(prePostEnabled = false)
+@EnableMethodSecurity(prePostEnabled = false, jsr250Enabled = true)
 @EnableWebSecurity
 public class SecurityConfig {
 
@@ -75,4 +76,13 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
         return http.build();
     }
 
+    /**
+     * Remove the default "ROLE_" prefix so that hasRole("ADMIN")
+     * and @RolesAllowed("ADMIN") match a GrantedAuthority("ADMIN").
+     */
+    @Bean
+    public GrantedAuthorityDefaults grantedAuthorityDefaults() {
+        return new GrantedAuthorityDefaults("");
+    }
+
 }

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/enums/SecurityRoles.java

@@ -70,9 +70,9 @@ public JWTFilter(TOSService tosService,
     }
 
     /**
-     * Filter implementation that performs authentication and authorization checks based on the provided request headers.
+     * Filter that performs authentication and authorization checks based on the provided request headers.
      * The filter checks for the presence of the "Authorization" header and validates the token.
-     * It sets the appropriate security context based on the type of token (long term token or PSAMA application token) and
+     * It sets the appropriate security context based on the type of token (long-term token or PSAMA application token) and
      * performs the necessary checks to ensure that the user or application is authorized to access the requested resource.
      * This filter is called by the configured security filter chain in the SecurityConfig class.
      *
@@ -91,16 +91,12 @@ protected void doFilterInternal(HttpServletRequest request, @NonNull HttpServlet
             // without any authentication or authorization checks
             filterChain.doFilter(request, response);
         } else {
-            // If the header is present, we need to check the token
             String token = authorizationHeader.substring(6).trim();
-
-            // Parse the token
             Jws<Claims> jws = this.jwtUtil.parseToken(token);
             String userId = jws.getPayload().get(this.userClaimId, String.class);
 
             if (userId.startsWith(AuthNaming.LONG_TERM_TOKEN_PREFIX)) {
-                // For profile information, we do indeed allow long term token
-                // to be a valid token.
+                // For profile information, we do indeed allow a long-term token to be a valid token.
                 if (request.getRequestURI().startsWith("/auth/user/me")) {
                     String realClaimsSubject = jws.getPayload().getSubject().substring(AuthNaming.LONG_TERM_TOKEN_PREFIX.length() + 1);
 
@@ -112,7 +108,7 @@ protected void doFilterInternal(HttpServletRequest request, @NonNull HttpServlet
             } else if (userId.startsWith(AuthNaming.PSAMA_APPLICATION_TOKEN_PREFIX)) {
                 logger.info("User Authentication Starts with {}", AuthNaming.PSAMA_APPLICATION_TOKEN_PREFIX);
 
-                // Check if user is attempting to access the correct introspect endpoint. If not reject the request
+                // Check if the user is attempting to access the correct introspection endpoint. If not reject the request,
                 // log an error indicating the user's token may be being used by a malicious actor.
                 if (!request.getRequestURI().endsWith("token/inspect") && !request.getRequestURI().endsWith("open/validate")) {
                     logger.error("{} attempted to perform request {} token may be compromised.", userId, request.getRequestURI());
@@ -122,7 +118,7 @@ protected void doFilterInternal(HttpServletRequest request, @NonNull HttpServlet
                 String applicationId = userId.split("\\|")[1];
                 logger.info("Application ID: {}", applicationId);
 
-                // Authenticate as Application
+                // Create custom application details. Will be used to set the correct security context.
                 CustomApplicationDetails customApplicationDetails = (CustomApplicationDetails) this.customUserDetailService.loadUserByUsername("application:" + applicationId);
                 if (customApplicationDetails.getApplication() == null) {
                     logger.error("Cannot find an application by userId: {}", applicationId);
@@ -160,9 +156,9 @@ private void setSecurityContextForApplication(HttpServletRequest request, Custom
 
     /**
      * Sets the security context for the given user.
-     * This method is responsible for validating the user claims, checking if the user is active,
-     * ensuring that the user has accepted the terms of service (if enabled), validating user roles and privileges,
-     * and setting the user object as an attribute in the request.
+     * This method is responsible for validating the user claims. It checks if the user is active,
+     * ensuring the user has accepted the terms of service (if enabled), validating user roles and privileges,
+     * and setting the user object as an attribute in the security context.
      *
      * @param request           the HttpServletRequest object
      * @param response          the HttpServletResponse object
@@ -192,6 +188,8 @@ private void setSecurityContextForUser(HttpServletRequest request, HttpServletRe
             //If user has not accepted terms of service and is attempted to get information other than the terms of service, don't authenticate
             try {
                 response.sendError(HttpServletResponse.SC_FORBIDDEN, "User must accept terms of service");
+                // Return early to prevent setting up security context
+                return;
             } catch (IOException e) {
                 logger.error("Failed to send response.", e);
             }
@@ -202,6 +200,8 @@ private void setSecurityContextForUser(HttpServletRequest request, HttpServletRe
             logger.error("User doesn't have any roles or privileges.");
             try {
                 response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User doesn't have any roles or privileges.");
+                // Return early to prevent setting up security context
+                return;
             } catch (IOException e) {
                 logger.error("Failed to send response.", e);
             }

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/filter/JWTFilter.java

@@ -2,6 +2,7 @@
 
 import edu.harvard.hms.dbmi.avillach.auth.entity.User;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 
 import java.util.ArrayList;
@@ -15,9 +16,9 @@ public class CustomUserDetails implements UserDetails {
     public CustomUserDetails(User user) {
         this.user = user;
         if (user != null && user.getRoles() != null) {
-            this.authorities = user.getTotalPrivilege().stream()
-                    .map(privilege -> (GrantedAuthority) privilege::getName)
-                    .toList();
+            this.authorities = new ArrayList<>(user.getTotalPrivilege().stream()
+                    .map(privilege-> new SimpleGrantedAuthority(privilege.getName()))
+                    .toList());
         } else {
             this.authorities = new ArrayList<>();
         }

pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/model/CustomUserDetails.java

@@ -9,7 +9,6 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
-import org.springframework.security.access.annotation.Secured;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.*;
 
@@ -39,7 +38,7 @@ public AccessRuleController(AccessRuleService accessRuleService) {
     }
 
     @Operation(description = "GET information of one AccessRule with the UUID, requires ADMIN or SUPER_ADMIN role")
-    @Secured(value = {ADMIN, SUPER_ADMIN})
+    @RolesAllowed({ADMIN, SUPER_ADMIN})
     @GetMapping(value = "/{accessRuleId}")
     public ResponseEntity<?> getAccessRuleById(
             @Parameter(description = "The UUID of the accessRule to fetch information about")
@@ -54,7 +53,7 @@ public ResponseEntity<?> getAccessRuleById(
     }
 
     @Operation(description = "GET a list of existing AccessRules, requires ADMIN or SUPER_ADMIN role")
-    @Secured({ADMIN, SUPER_ADMIN})
+    @RolesAllowed({ADMIN, SUPER_ADMIN})
     @GetMapping("")
     public ResponseEntity<List<AccessRule>> getAccessRuleAll() {
         List<AccessRule> allAccessRules = this.accessRuleService.getAllAccessRules();

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

@@ -6,9 +6,9 @@
 import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.Parameter;
 import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.annotation.security.RolesAllowed;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.ResponseEntity;
-import org.springframework.security.access.annotation.Secured;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.*;
 
@@ -36,7 +36,7 @@ public ConnectionWebController(ConnectionWebService connectionWebSerivce) {
 
     @Operation(description = "GET information of one Connection with the UUID, requires ADMIN or SUPER_ADMIN role")
     @GetMapping(path = "/{connectionId}", produces = "application/json")
-    @Secured({SUPER_ADMIN, ADMIN})
+    @RolesAllowed({SUPER_ADMIN, ADMIN})
     public ResponseEntity<?> getConnectionById(
             @Parameter(required = true, description = "The UUID of the Connection to fetch information about")
             @PathVariable("connectionId") String connectionId) {
@@ -50,14 +50,14 @@ public ResponseEntity<?> getConnectionById(
 
     @Operation(description = "GET a list of existing Connection, requires SUPER_ADMIN or ADMIN role")
     @GetMapping
-    @Secured({SUPER_ADMIN, ADMIN})
+    @RolesAllowed({SUPER_ADMIN, ADMIN})
     public ResponseEntity<List<Connection>> getAllConnections() {
         List<Connection> allConnections = connectionWebService.getAllConnections();
         return ResponseEntity.ok(allConnections);
     }
 
     @Operation(description = "POST a list of Connections, requires SUPER_ADMIN role")
-    @Secured({SUPER_ADMIN})
+    @RolesAllowed({SUPER_ADMIN})
     @PostMapping(produces = "application/json", consumes = "application/json")
     public ResponseEntity<?> addConnection(
             @Parameter(required = true, description = "A list of Connections in JSON format")
@@ -72,7 +72,7 @@ public ResponseEntity<?> addConnection(
     }
 
     @Operation(description = "Update a list of Connections, will only update the fields listed, requires SUPER_ADMIN role")
-    @Secured({SUPER_ADMIN})
+    @RolesAllowed({SUPER_ADMIN})
     @PutMapping(produces = "application/json", consumes = "application/json")
     public ResponseEntity<List<Connection>> updateConnection(
             @Parameter(required = true, description = "A list of Connection with fields to be updated in JSON format")
@@ -82,7 +82,7 @@ public ResponseEntity<List<Connection>> updateConnection(
     }
 
     @Operation(description = "DELETE an Connection by Id only if the Connection is not associated by others, requires SUPER_ADMIN role")
-    @Secured({SUPER_ADMIN})
+    @RolesAllowed({SUPER_ADMIN})
     @DeleteMapping(path = "/{connectionId}", produces = "application/json")
     public ResponseEntity<List<Connection>> removeById(
             @Parameter(required = true, description = "A valid connection Id")

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

@@ -3,8 +3,6 @@
 import edu.harvard.hms.dbmi.avillach.auth.entity.Privilege;
 import edu.harvard.hms.dbmi.avillach.auth.entity.Role;
 import edu.harvard.hms.dbmi.avillach.auth.entity.User;
-import edu.harvard.hms.dbmi.avillach.auth.enums.SecurityRoles;
-import edu.harvard.hms.dbmi.avillach.auth.model.CustomUserDetails;
 import edu.harvard.hms.dbmi.avillach.auth.model.fenceMapping.StudyMetaData;
 import edu.harvard.hms.dbmi.avillach.auth.model.ras.RasDbgapPermission;
 import edu.harvard.hms.dbmi.avillach.auth.repository.RoleRepository;
@@ -15,8 +13,6 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.event.ContextRefreshedEvent;
 import org.springframework.context.event.EventListener;
-import org.springframework.security.core.context.SecurityContext;
-import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -139,20 +135,10 @@ public List<Role> updateRoles(List<Role> roles) {
     @Transactional
     public Optional<List<Role>> removeRoleById(String roleId) {
         Optional<Role> optionalRole = roleRepository.findById(UUID.fromString(roleId));
-
         if (optionalRole.isEmpty()) {
             return Optional.empty();
         }
 
-        SecurityContext context = SecurityContextHolder.getContext();
-        CustomUserDetails current_user = (CustomUserDetails) context.getAuthentication().getPrincipal();
-        Set<String> roleNames = current_user.getUser().getRoles().stream().map(Role::getName).collect(Collectors.toSet());
-
-        if (!roleNames.contains(SecurityRoles.PIC_SURE_TOP_ADMIN.getRole())){
-            logger.info("User doesn't have PIC-SURE Top Admin role, can't remove any role");
-            return Optional.empty();
-        }
-
         roleRepository.deleteById(optionalRole.get().getUuid());
         return Optional.of(roleRepository.findAll());
     }

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

@@ -13,8 +13,7 @@ public class AuthNaming {
     public static final String PSAMA_APPLICATION_TOKEN_PREFIX = "PSAMA_APPLICATION";
 
     /**
-     * <p>Contains all preset PSAMA role naming conventions.</p>
-     * <p>Note: Only users with super admin access can edit these role names.</p>
+     * <p>Constants used to in @RolesAllowed() annotations.</p>
      */
     public static class AuthRoleNaming {
         public static final String ADMIN = "ADMIN";