Skip to content

Commit

Permalink
Permission Performance Optimizations (#2341)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
chrisknoll committed Feb 14, 2024
1 parent 3f9e90c commit f258186
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 22 deletions.
5 changes: 0 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1024,11 +1024,6 @@
<artifactId>HikariCP</artifactId>
<version>4.0.3</version>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.9.0</version>
</dependency>
<dependency>
<groupId>org.jasypt</groupId>
<artifactId>jasypt-hibernate4</artifactId>
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/ohdsi/webapi/service/UserService.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -50,6 +51,7 @@ public static class User implements Comparable<User> {
public String login;
public String name;
public List<Permission> permissions;
public JsonNode permissionIdx;

public User() {}

Expand Down Expand Up @@ -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;
}
Expand Down
69 changes: 68 additions & 1 deletion src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

/**
*
Expand All @@ -38,6 +47,9 @@
@Transactional
public class PermissionManager {

@Value("${datasource.ohdsi.schema}")
private String ohdsiSchema;

@Autowired
private UserRepository userRepository;

Expand All @@ -55,9 +67,20 @@ public class PermissionManager {

@Autowired
private ApplicationEventPublisher eventPublisher;


@Autowired
private JdbcTemplate jdbcTemplate;

@Autowired
private ObjectMapper objectMapper;

private ThreadLocal<ConcurrentHashMap<String, UserSimpleAuthorizationInfo>> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new);

public static class PermissionsDTO {

public JsonNode permissions = null;
}

public RoleEntity addRole(String roleName, boolean isSystem) {
Guard.checkNotEmpty(roleName);

Expand Down Expand Up @@ -305,7 +328,51 @@ public Set<PermissionEntity> 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<String> 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<String> of
* permissions that start with the key as the value
*/
private JsonNode permsToJsonNode(List<String> permissions) {

Map<String, ArrayNode> 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<PermissionEntity> getRolePermissions(RoleEntity role) {
Set<PermissionEntity> permissions = new LinkedHashSet<>();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
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;
import javax.servlet.ServletResponse;
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;
Expand All @@ -25,31 +26,29 @@ 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);
httpResponse.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th
}

request.setAttribute(TOKEN_ATTRIBUTE, jwt);
Collection<String> 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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -259,6 +260,9 @@ public class AtlasRegularSecurity extends AtlasSecurity {

@Autowired
private PermissionManager permissionManager;

@Autowired
private ObjectMapper objectMapper;

public AtlasRegularSecurity(EntityPermissionSchemaResolver permissionSchemaResolver) {

Expand Down Expand Up @@ -293,7 +297,7 @@ public Map<FilterTemplates, Filter> 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));

Expand Down
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit f258186

Please sign in to comment.