Update AuthorizationService (#187)

* Update AuthorizationService Open access does not currently assign a connection to users. We should change this in the future, but for now we will handle the case. * Refactor open access role handling in UserService The revision refactors how open access roles are handled in UserService.java. A condition was added to check if the user's roles are null before assigning a new HashSet, thus improving code robustness. Instead of setting the roles directly, the role is now added to the user's existing roles, ensuring preservation of any pre-existing roles.
hms-dbmi · Jul 15, 2024 · f19aa8e · f19aa8e
1 parent f5e237e
commit f19aa8e
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 9 deletions.
diff --git a/config/psama/psama.env b/config/psama/psama.env
@@ -6,6 +6,7 @@
 # after initial login.
 APPLICATION_CLIENT_SECRET=
 APPLICATION_CLIENT_SECRET_IS_BASE_64=false
+STACK_SPECIFIC_APPLICATION_ID=
 
 # Fence IDP Configuration
 FENCE_IDP_PROVIDER_IS_ENABLED=false

diff --git a/...h-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java b/...h-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java
@@ -563,13 +563,15 @@ public User createOpenAccessUser(Role openAccessRole) {
         // Save the user to get a UUID
         user = save(user);
         user.setSubject("open_access|" + user.getUuid().toString());
-        if (openAccessRole != null) {
-            user.setRoles(Set.of(openAccessRole));
-        } else {
-            logger.error("createOpenAccessUser() openAccessRole is null");
+
+        if (user.getRoles() == null) {
             user.setRoles(new HashSet<>());
         }
 
+        if (openAccessRole != null) {
+            user.getRoles().add(openAccessRole);
+        }
+
         user.setEmail(user.getUuid() + "@open_access.com");
         user = save(user);
 

diff --git a/...a/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java b/...a/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java
@@ -87,7 +87,6 @@ public AuthorizationService(AccessRuleService accessRuleService, @Value("${stric
      */
     public boolean isAuthorized(Application application, Object requestBody, User user) {
         // create timer
-        long startTime = System.currentTimeMillis();
         String applicationName = application.getName();
         String resourceId = "null";
         String targetService = "null";
@@ -98,7 +97,6 @@ public boolean isAuthorized(Application application, Object requestBody, User us
             return true;
         }
 
-        long parseTimeFrame = System.currentTimeMillis();
         try {
             Map requestBodyMap = (Map) requestBody;
             Map queryMap = (Map) requestBodyMap.get("query");
@@ -122,10 +120,14 @@ public boolean isAuthorized(Application application, Object requestBody, User us
             logger.debug("isAuthorized() Stack Trace: ", e1);
             return false;
         }
-        logger.info("Parse timeframe {} ms", (System.currentTimeMillis() - parseTimeFrame));
 
         Set<AccessRule> accessRules;
-        String label = user.getConnection().getLabel();
+        String label = "";
+        if (user.getConnection() != null) {
+            // Open Access doesn't currently use a connection
+            label = user.getConnection().getLabel();
+        }
+
         if (!this.strictConnections.contains(label)) {
             Set<Privilege> privileges = user.getPrivilegesByApplication(application);
             if (privileges == null || privileges.isEmpty()) {
@@ -174,7 +176,6 @@ public boolean isAuthorized(Application application, Object requestBody, User us
                 .map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName()))
                 .collect(Collectors.joining(", ")) + "]");
 
-        logger.info("Login time: {}ms", System.currentTimeMillis() - startTime);
         return result;
     }