Skip to content

Commit f258186

Browse files
authored
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.
1 parent 3f9e90c commit f258186

7 files changed

Lines changed: 97 additions & 22 deletions

File tree

pom.xml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,11 +1024,6 @@
10241024
<artifactId>HikariCP</artifactId>
10251025
<version>4.0.3</version>
10261026
</dependency>
1027-
<dependency>
1028-
<groupId>com.google.code.gson</groupId>
1029-
<artifactId>gson</artifactId>
1030-
<version>2.9.0</version>
1031-
</dependency>
10321027
<dependency>
10331028
<groupId>org.jasypt</groupId>
10341029
<artifactId>jasypt-hibernate4</artifactId>

src/main/java/org/ohdsi/webapi/service/UserService.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.ohdsi.webapi.service;
22

3+
import com.fasterxml.jackson.databind.JsonNode;
34
import com.odysseusinc.logging.event.*;
45
import org.eclipse.collections.impl.block.factory.Comparators;
56
import org.ohdsi.webapi.shiro.Entities.PermissionEntity;
@@ -50,6 +51,7 @@ public static class User implements Comparable<User> {
5051
public String login;
5152
public String name;
5253
public List<Permission> permissions;
54+
public JsonNode permissionIdx;
5355

5456
public User() {}
5557

@@ -114,6 +116,8 @@ public User getCurrentUser() throws Exception {
114116
user.login = currentUser.getLogin();
115117
user.name = currentUser.getName();
116118
user.permissions = convertPermissions(permissions);
119+
user.permissionIdx = authorizer.queryUserPermissions(currentUser.getLogin()).permissions;
120+
117121

118122
return user;
119123
}

src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package org.ohdsi.webapi.shiro;
22

3+
import com.fasterxml.jackson.databind.JsonNode;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import com.fasterxml.jackson.databind.node.ArrayNode;
36
import com.odysseusinc.logging.event.AddUserEvent;
47
import com.odysseusinc.logging.event.DeleteRoleEvent;
58
import org.apache.shiro.SecurityUtils;
@@ -25,10 +28,16 @@
2528
import org.springframework.transaction.annotation.Transactional;
2629

2730
import java.security.Principal;
31+
import java.util.HashMap;
2832
import java.util.LinkedHashSet;
33+
import java.util.List;
2934
import java.util.Map;
3035
import java.util.Set;
3136
import java.util.concurrent.ConcurrentHashMap;
37+
import org.apache.commons.lang3.StringUtils;
38+
import org.ohdsi.circe.helper.ResourceHelper;
39+
import org.springframework.beans.factory.annotation.Value;
40+
import org.springframework.jdbc.core.JdbcTemplate;
3241

3342
/**
3443
*
@@ -38,6 +47,9 @@
3847
@Transactional
3948
public class PermissionManager {
4049

50+
@Value("${datasource.ohdsi.schema}")
51+
private String ohdsiSchema;
52+
4153
@Autowired
4254
private UserRepository userRepository;
4355

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

5668
@Autowired
5769
private ApplicationEventPublisher eventPublisher;
58-
70+
71+
@Autowired
72+
private JdbcTemplate jdbcTemplate;
73+
74+
@Autowired
75+
private ObjectMapper objectMapper;
76+
5977
private ThreadLocal<ConcurrentHashMap<String, UserSimpleAuthorizationInfo>> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new);
6078

79+
public static class PermissionsDTO {
80+
81+
public JsonNode permissions = null;
82+
}
83+
6184
public RoleEntity addRole(String roleName, boolean isSystem) {
6285
Guard.checkNotEmpty(roleName);
6386

@@ -305,7 +328,51 @@ public Set<PermissionEntity> getUserPermissions(UserEntity user) {
305328

306329
return permissions;
307330
}
331+
332+
public PermissionsDTO queryUserPermissions(final String login) {
333+
String permQuery = StringUtils.replace(
334+
ResourceHelper.GetResourceAsString("/resources/security/getPermissionsForUser.sql"),
335+
"@ohdsi_schema",
336+
this.ohdsiSchema);
337+
final UserEntity user = userRepository.findByLogin(login);
338+
339+
List<String> permissions = this.jdbcTemplate.query(
340+
permQuery,
341+
(ps) -> {
342+
ps.setLong(1, user.getId());
343+
},
344+
(rs, rowNum) -> {
345+
return rs.getString("value");
346+
});
347+
PermissionsDTO permDto = new PermissionsDTO();
348+
permDto.permissions = permsToJsonNode(permissions);
349+
return permDto;
350+
}
351+
352+
/**
353+
* This method takes a list of strings and returns a JSObject representing
354+
* the first element of each permission as a key, and the List<String> of
355+
* permissions that start with the key as the value
356+
*/
357+
private JsonNode permsToJsonNode(List<String> permissions) {
358+
359+
Map<String, ArrayNode> resultMap = new HashMap<>();
360+
361+
// Process each input string
362+
for (String inputString : permissions) {
363+
String[] parts = inputString.split(":");
364+
String key = parts[0];
365+
// Create a new JsonArray for the key if it doesn't exist
366+
resultMap.putIfAbsent(key, objectMapper.createArrayNode());
367+
// Add the value to the JsonArray
368+
resultMap.get(key).add(inputString);
369+
}
308370

371+
// Convert the resultMap to a JsonNode
372+
373+
return objectMapper.valueToTree(resultMap);
374+
}
375+
309376
private Set<PermissionEntity> getRolePermissions(RoleEntity role) {
310377
Set<PermissionEntity> permissions = new LinkedHashSet<>();
311378

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package org.ohdsi.webapi.shiro.filters;
22

3+
import com.fasterxml.jackson.databind.ObjectMapper;
34
import static org.ohdsi.webapi.shiro.management.AtlasSecurity.PERMISSIONS_ATTRIBUTE;
45
import static org.ohdsi.webapi.shiro.management.AtlasSecurity.TOKEN_ATTRIBUTE;
56

6-
import com.google.gson.Gson;
77
import java.io.IOException;
88
import java.io.PrintWriter;
99
import javax.servlet.ServletRequest;
1010
import javax.servlet.ServletResponse;
1111
import javax.servlet.http.HttpServletResponse;
1212
import org.apache.shiro.web.servlet.AdviceFilter;
1313
import org.apache.shiro.web.util.WebUtils;
14+
import org.ohdsi.webapi.shiro.PermissionManager;
1415
import org.slf4j.Logger;
1516
import org.slf4j.LoggerFactory;
1617
import org.springframework.http.MediaType;
@@ -25,31 +26,29 @@ public class SendTokenInHeaderFilter extends AdviceFilter {
2526
private static final String ERROR_WRITING_PERMISSIONS_TO_RESPONSE_LOG = "Error writing permissions to response";
2627
private static final String TOKEN_HEADER_NAME = "Bearer";
2728

29+
private final ObjectMapper objectMapper;
30+
31+
public SendTokenInHeaderFilter(ObjectMapper objectMapper) {
32+
this.objectMapper = objectMapper;
33+
}
34+
35+
36+
2837
@Override
2938
protected boolean preHandle(ServletRequest request, ServletResponse response) {
3039
String jwt = (String)request.getAttribute(TOKEN_ATTRIBUTE);
31-
String permissions = (String)request.getAttribute(PERMISSIONS_ATTRIBUTE);
40+
PermissionManager.PermissionsDTO permissions = (PermissionManager.PermissionsDTO)request.getAttribute(PERMISSIONS_ATTRIBUTE);
3241

3342
HttpServletResponse httpResponse = WebUtils.toHttp(response);
3443
httpResponse.setHeader(TOKEN_HEADER_NAME, jwt);
3544
httpResponse.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
3645
httpResponse.setStatus(HttpServletResponse.SC_OK);
3746

3847
try (final PrintWriter responseWriter = response.getWriter()) {
39-
responseWriter.print(new Gson().toJson(new PermissionsDTO(permissions)));
48+
responseWriter.print(objectMapper.writeValueAsString(permissions));
4049
} catch (IOException e) {
4150
LOGGER.error(ERROR_WRITING_PERMISSIONS_TO_RESPONSE_LOG, e);
4251
}
4352
return false;
4453
}
45-
46-
public static class PermissionsDTO {
47-
48-
private PermissionsDTO(String permissions) {
49-
50-
this.permissions = permissions;
51-
}
52-
53-
public final String permissions;
54-
}
5554
}

src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th
145145
}
146146

147147
request.setAttribute(TOKEN_ATTRIBUTE, jwt);
148-
Collection<String> permissions = this.authorizer.getAuthorizationInfo(login).getStringPermissions();
149-
request.setAttribute(PERMISSIONS_ATTRIBUTE, StringUtils.join(permissions, "|"));
148+
PermissionManager.PermissionsDTO permissions = this.authorizer.queryUserPermissions(login);
149+
request.setAttribute(PERMISSIONS_ATTRIBUTE, permissions);
150150
return true;
151151
}
152152

src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.ohdsi.webapi.shiro.management;
22

3+
import com.fasterxml.jackson.databind.ObjectMapper;
34
import io.buji.pac4j.filter.CallbackFilter;
45
import io.buji.pac4j.filter.SecurityFilter;
56
import io.buji.pac4j.realm.Pac4jRealm;
@@ -259,6 +260,9 @@ public class AtlasRegularSecurity extends AtlasSecurity {
259260

260261
@Autowired
261262
private PermissionManager permissionManager;
263+
264+
@Autowired
265+
private ObjectMapper objectMapper;
262266

263267
public AtlasRegularSecurity(EntityPermissionSchemaResolver permissionSchemaResolver) {
264268

@@ -293,7 +297,7 @@ public Map<FilterTemplates, Filter> getFilters() {
293297
}
294298

295299
filters.put(SEND_TOKEN_IN_URL, new SendTokenInUrlFilter(this.oauthUiCallback));
296-
filters.put(SEND_TOKEN_IN_HEADER, new SendTokenInHeaderFilter());
300+
filters.put(SEND_TOKEN_IN_HEADER, new SendTokenInHeaderFilter(this.objectMapper));
297301

298302
filters.put(RUN_AS, new RunAsFilter(userRepository));
299303

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
select distinct sp.value
2+
from @ohdsi_schema.sec_user_role sur
3+
join @ohdsi_schema.sec_role_permission srp on sur.role_id = srp.role_id
4+
join @ohdsi_schema.sec_permission sp on sp.id = srp.permission_id
5+
where sur.user_id = ?
6+
order by value;

0 commit comments

Comments
 (0)