From f258186ce9955ff5ee1cef220de88da7a9049de2 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Tue, 13 Feb 2024 19:21:49 -0500 Subject: [PATCH] Permission Performance Optimizations (#2341) * Implemented permission lookup to bypass hibernate. * Moved PermissionsDTO to PermissionManager * Removed GSON to use the app-wide ObjectMapper from Jackson * Removed unused imports to GSON. --- pom.xml | 5 -- .../org/ohdsi/webapi/service/UserService.java | 4 ++ .../ohdsi/webapi/shiro/PermissionManager.java | 69 ++++++++++++++++++- .../filters/SendTokenInHeaderFilter.java | 25 ++++--- .../filters/UpdateAccessTokenFilter.java | 4 +- .../management/AtlasRegularSecurity.java | 6 +- .../security/getPermissionsForUser.sql | 6 ++ 7 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 src/main/resources/resources/security/getPermissionsForUser.sql diff --git a/pom.xml b/pom.xml index a152507cac..bf5b8deeeb 100644 --- a/pom.xml +++ b/pom.xml @@ -1024,11 +1024,6 @@ HikariCP 4.0.3 - - com.google.code.gson - gson - 2.9.0 - org.jasypt jasypt-hibernate4 diff --git a/src/main/java/org/ohdsi/webapi/service/UserService.java b/src/main/java/org/ohdsi/webapi/service/UserService.java index fc19c071a8..2c786a8440 100644 --- a/src/main/java/org/ohdsi/webapi/service/UserService.java +++ b/src/main/java/org/ohdsi/webapi/service/UserService.java @@ -1,5 +1,6 @@ package org.ohdsi.webapi.service; +import com.fasterxml.jackson.databind.JsonNode; import com.odysseusinc.logging.event.*; import org.eclipse.collections.impl.block.factory.Comparators; import org.ohdsi.webapi.shiro.Entities.PermissionEntity; @@ -50,6 +51,7 @@ public static class User implements Comparable { public String login; public String name; public List permissions; + public JsonNode permissionIdx; public User() {} @@ -114,6 +116,8 @@ public User getCurrentUser() throws Exception { user.login = currentUser.getLogin(); user.name = currentUser.getName(); user.permissions = convertPermissions(permissions); + user.permissionIdx = authorizer.queryUserPermissions(currentUser.getLogin()).permissions; + return user; } diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 4e0eeec5b7..96c6d13c55 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -1,5 +1,8 @@ package org.ohdsi.webapi.shiro; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; import com.odysseusinc.logging.event.AddUserEvent; import com.odysseusinc.logging.event.DeleteRoleEvent; import org.apache.shiro.SecurityUtils; @@ -25,10 +28,16 @@ import org.springframework.transaction.annotation.Transactional; import java.security.Principal; +import java.util.HashMap; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.apache.commons.lang3.StringUtils; +import org.ohdsi.circe.helper.ResourceHelper; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.jdbc.core.JdbcTemplate; /** * @@ -38,6 +47,9 @@ @Transactional public class PermissionManager { + @Value("${datasource.ohdsi.schema}") + private String ohdsiSchema; + @Autowired private UserRepository userRepository; @@ -55,9 +67,20 @@ public class PermissionManager { @Autowired private ApplicationEventPublisher eventPublisher; - + + @Autowired + private JdbcTemplate jdbcTemplate; + + @Autowired + private ObjectMapper objectMapper; + private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); + public static class PermissionsDTO { + + public JsonNode permissions = null; + } + public RoleEntity addRole(String roleName, boolean isSystem) { Guard.checkNotEmpty(roleName); @@ -305,7 +328,51 @@ public Set getUserPermissions(UserEntity user) { return permissions; } + + public PermissionsDTO queryUserPermissions(final String login) { + String permQuery = StringUtils.replace( + ResourceHelper.GetResourceAsString("/resources/security/getPermissionsForUser.sql"), + "@ohdsi_schema", + this.ohdsiSchema); + final UserEntity user = userRepository.findByLogin(login); + + List permissions = this.jdbcTemplate.query( + permQuery, + (ps) -> { + ps.setLong(1, user.getId()); + }, + (rs, rowNum) -> { + return rs.getString("value"); + }); + PermissionsDTO permDto = new PermissionsDTO(); + permDto.permissions = permsToJsonNode(permissions); + return permDto; + } + + /** + * This method takes a list of strings and returns a JSObject representing + * the first element of each permission as a key, and the List of + * permissions that start with the key as the value + */ + private JsonNode permsToJsonNode(List permissions) { + + Map resultMap = new HashMap<>(); + + // Process each input string + for (String inputString : permissions) { + String[] parts = inputString.split(":"); + String key = parts[0]; + // Create a new JsonArray for the key if it doesn't exist + resultMap.putIfAbsent(key, objectMapper.createArrayNode()); + // Add the value to the JsonArray + resultMap.get(key).add(inputString); + } + // Convert the resultMap to a JsonNode + + return objectMapper.valueToTree(resultMap); + } + private Set getRolePermissions(RoleEntity role) { Set permissions = new LinkedHashSet<>(); diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/SendTokenInHeaderFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/SendTokenInHeaderFilter.java index a8c7a68d08..08b253f9b5 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/SendTokenInHeaderFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/SendTokenInHeaderFilter.java @@ -1,9 +1,9 @@ package org.ohdsi.webapi.shiro.filters; +import com.fasterxml.jackson.databind.ObjectMapper; import static org.ohdsi.webapi.shiro.management.AtlasSecurity.PERMISSIONS_ATTRIBUTE; import static org.ohdsi.webapi.shiro.management.AtlasSecurity.TOKEN_ATTRIBUTE; -import com.google.gson.Gson; import java.io.IOException; import java.io.PrintWriter; import javax.servlet.ServletRequest; @@ -11,6 +11,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.shiro.web.servlet.AdviceFilter; import org.apache.shiro.web.util.WebUtils; +import org.ohdsi.webapi.shiro.PermissionManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.MediaType; @@ -25,10 +26,18 @@ public class SendTokenInHeaderFilter extends AdviceFilter { private static final String ERROR_WRITING_PERMISSIONS_TO_RESPONSE_LOG = "Error writing permissions to response"; private static final String TOKEN_HEADER_NAME = "Bearer"; + private final ObjectMapper objectMapper; + + public SendTokenInHeaderFilter(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + + @Override protected boolean preHandle(ServletRequest request, ServletResponse response) { String jwt = (String)request.getAttribute(TOKEN_ATTRIBUTE); - String permissions = (String)request.getAttribute(PERMISSIONS_ATTRIBUTE); + PermissionManager.PermissionsDTO permissions = (PermissionManager.PermissionsDTO)request.getAttribute(PERMISSIONS_ATTRIBUTE); HttpServletResponse httpResponse = WebUtils.toHttp(response); httpResponse.setHeader(TOKEN_HEADER_NAME, jwt); @@ -36,20 +45,10 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) { httpResponse.setStatus(HttpServletResponse.SC_OK); try (final PrintWriter responseWriter = response.getWriter()) { - responseWriter.print(new Gson().toJson(new PermissionsDTO(permissions))); + responseWriter.print(objectMapper.writeValueAsString(permissions)); } catch (IOException e) { LOGGER.error(ERROR_WRITING_PERMISSIONS_TO_RESPONSE_LOG, e); } return false; } - - public static class PermissionsDTO { - - private PermissionsDTO(String permissions) { - - this.permissions = permissions; - } - - public final String permissions; - } } diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java index bfbb06eae1..f5597058e8 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java @@ -145,8 +145,8 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th } request.setAttribute(TOKEN_ATTRIBUTE, jwt); - Collection permissions = this.authorizer.getAuthorizationInfo(login).getStringPermissions(); - request.setAttribute(PERMISSIONS_ATTRIBUTE, StringUtils.join(permissions, "|")); + PermissionManager.PermissionsDTO permissions = this.authorizer.queryUserPermissions(login); + request.setAttribute(PERMISSIONS_ATTRIBUTE, permissions); return true; } diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java index 67e27df70b..e19a1ebc8b 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java @@ -1,5 +1,6 @@ package org.ohdsi.webapi.shiro.management; +import com.fasterxml.jackson.databind.ObjectMapper; import io.buji.pac4j.filter.CallbackFilter; import io.buji.pac4j.filter.SecurityFilter; import io.buji.pac4j.realm.Pac4jRealm; @@ -259,6 +260,9 @@ public class AtlasRegularSecurity extends AtlasSecurity { @Autowired private PermissionManager permissionManager; + + @Autowired + private ObjectMapper objectMapper; public AtlasRegularSecurity(EntityPermissionSchemaResolver permissionSchemaResolver) { @@ -293,7 +297,7 @@ public Map getFilters() { } filters.put(SEND_TOKEN_IN_URL, new SendTokenInUrlFilter(this.oauthUiCallback)); - filters.put(SEND_TOKEN_IN_HEADER, new SendTokenInHeaderFilter()); + filters.put(SEND_TOKEN_IN_HEADER, new SendTokenInHeaderFilter(this.objectMapper)); filters.put(RUN_AS, new RunAsFilter(userRepository)); diff --git a/src/main/resources/resources/security/getPermissionsForUser.sql b/src/main/resources/resources/security/getPermissionsForUser.sql new file mode 100644 index 0000000000..f64fc6c60f --- /dev/null +++ b/src/main/resources/resources/security/getPermissionsForUser.sql @@ -0,0 +1,6 @@ +select distinct sp.value +from @ohdsi_schema.sec_user_role sur +join @ohdsi_schema.sec_role_permission srp on sur.role_id = srp.role_id +join @ohdsi_schema.sec_permission sp on sp.id = srp.permission_id +where sur.user_id = ? +order by value;