Skip to content

Commit

Permalink
Removed role cache from Permission Service.
Browse files Browse the repository at this point in the history 
Changed read/write permission checks to use permissions instead of roles.
Permission checks are using Subject.isPermitted() which honors wildcard semantics.
Altered JwtAuthRealm to filter user permissions to either * or first element of permission to check for speed.
Changed permission index from JsonNode to Map<>.  Serializes same way, but map semantics are simpler to navigate.
Altered AuthrizationInfo to contain index of Permissions and store Wildcard perms.

General cleanup of unused imports and removed unused dependencies (ie: Autowired fields were removed if no longer needed).

Fixes #2353.
  • Loading branch information
@chrisknoll
chrisknoll committed Feb 27, 2024
1 parent f258186 commit 379d73b
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 157 deletions.
149 changes: 28 additions & 121 deletions src/main/java/org/ohdsi/webapi/security/PermissionService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.cosium.spring.data.jpa.entity.graph.domain.EntityGraphUtils;
import org.apache.shiro.authz.UnauthorizedException;
import org.ohdsi.webapi.model.CommonEntity;
import org.ohdsi.webapi.security.dto.RoleDTO;
import org.ohdsi.webapi.security.model.EntityPermissionSchema;
import org.ohdsi.webapi.security.model.EntityPermissionSchemaResolver;
import org.ohdsi.webapi.security.model.EntityType;
Expand Down Expand Up @@ -34,16 +33,15 @@
import javax.annotation.PostConstruct;
import java.io.Serializable;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.authz.Permission;
import org.apache.shiro.authz.permission.WildcardPermission;
import org.apache.shiro.subject.Subject;

@Service
public class PermissionService {
Expand All @@ -64,9 +62,6 @@ public class PermissionService {
@Value("#{!'${security.provider}'.equals('DisabledSecurity')}")
private boolean securityEnabled;

private ThreadLocal<ConcurrentHashMap<EntityType, ConcurrentHashMap<String, Set<RoleDTO>>>> permissionCache =
ThreadLocal.withInitial(ConcurrentHashMap::new);

private final EntityGraph PERMISSION_ENTITY_GRAPH = EntityGraphUtils.fromAttributePaths("rolePermissions", "rolePermissions.role");

public PermissionService(
Expand Down Expand Up @@ -133,14 +128,14 @@ public void checkCommonEntityOwnership(EntityType entityType, Integer entityId)

public Map<String, String> getPermissionTemplates(EntityPermissionSchema permissionSchema, AccessType accessType) {

switch (accessType) {
case WRITE:
return permissionSchema.getWritePermissions();
case READ:
return permissionSchema.getReadPermissions();
default:
throw new UnsupportedOperationException();
}
switch (accessType) {
case WRITE:
return permissionSchema.getWritePermissions();
case READ:
return permissionSchema.getReadPermissions();
default:
throw new UnsupportedOperationException();
}
}

public List<RoleEntity> finaAllRolesHavingPermissions(List<String> permissions) {
Expand Down Expand Up @@ -178,97 +173,28 @@ private boolean isCurrentUserOwnerOf(CommonEntity entity) {
return Objects.equals(owner.getLogin(), loggedInUsername);
}

public List<Permission> getEntityPermissions(EntityType entityType, Number id, AccessType accessType) {
Set<String> permissionTemplates = getTemplatesForType(entityType, accessType).keySet();

public void preparePermissionCache(EntityType entityType, Set<String> permissionTemplates) {
if (permissionCache.get().get(entityType) == null) {
final ConcurrentHashMap<String, Set<RoleDTO>> rolesForEntity = new ConcurrentHashMap<>();
permissionCache.get().put(entityType, rolesForEntity);

List<String> permissionsSQLTemplates = permissionTemplates.stream()
.map(pt -> getPermissionSqlTemplate(pt))
.collect(Collectors.toList());

Map<Long, RoleDTO> roleDTOMap = new HashMap<>();
permissionsSQLTemplates.forEach(p -> {
Iterable<PermissionEntity> permissionEntities = permissionRepository.findByValueLike(p, PERMISSION_ENTITY_GRAPH);
for (PermissionEntity permissionEntity : permissionEntities) {
Set<RoleDTO> roles = rolesForEntity.get(permissionEntity.getValue());
if (roles == null) {
rolesForEntity.put(permissionEntity.getValue(), new HashSet<>());
}
Set<RoleDTO> cachedRoles = rolesForEntity.get(permissionEntity.getValue());
permissionEntity.getRolePermissions().forEach(rp -> {
RoleDTO roleDTO = roleDTOMap.get(rp.getRole().getId());
if (roleDTO == null) {
roleDTO = conversionService.convert(rp.getRole(), RoleDTO.class);
roleDTOMap.put(roleDTO.getId(), roleDTO);
}
cachedRoles.add(roleDTO);
});
}
});
}
}

public List<RoleDTO> getRolesHavingPermissions(EntityType entityType, Number id) {
Set<String> permissionTemplates = getTemplatesForType(entityType, AccessType.WRITE).keySet();
preparePermissionCache(entityType, permissionTemplates);

List<String> permissions = permissionTemplates.stream()
.map(pt -> getPermission(pt, id))
List<Permission> permissions = permissionTemplates.stream()
.map(pt -> new WildcardPermission(getPermission(pt, id)))
.collect(Collectors.toList());
int fitCount = permissions.size();
Map<RoleDTO, Long> roleMap = permissions.stream()
.filter(p -> permissionCache.get().get(entityType).get(p) != null)
.flatMap(p -> permissionCache.get().get(entityType).get(p).stream())
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
List<RoleDTO> roles = roleMap.entrySet().stream()
.filter(es -> es.getValue() == fitCount)
.map(es -> es.getKey())
.collect(Collectors.toList());
return roles;
}

public List<RoleDTO> getRolesHavingReadPermissions(EntityType entityType, Number id) {
Set<String> permissionTemplates = getTemplatesForType(entityType, AccessType.READ).keySet();
preparePermissionCache(entityType, permissionTemplates);

List<String> permissions = permissionTemplates.stream()
.map(pt -> getPermission(pt, id))
.collect(Collectors.toList());
int fitCount = permissions.size();
Map<RoleDTO, Long> roleMap = permissions.stream()
.filter(p -> permissionCache.get().get(entityType).get(p) != null)
.flatMap(p -> permissionCache.get().get(entityType).get(p).stream())
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
List<RoleDTO> roles = roleMap.entrySet().stream()
.filter(es -> es.getValue() == fitCount)
.map(es -> es.getKey())
.collect(Collectors.toList());
return roles;
}

public void clearPermissionCache() {
this.permissionCache.set(new ConcurrentHashMap<>());
return permissions;
}

public boolean hasWriteAccess(CommonEntity entity) {
public boolean hasAccess(CommonEntity entity, AccessType accessType) {
boolean hasAccess = false;
if (securityEnabled && entity.getCreatedBy() != null) {
try {
Subject subject = SecurityUtils.getSubject();
String login = this.permissionManager.getSubjectName();
UserSimpleAuthorizationInfo authorizationInfo = this.permissionManager.getAuthorizationInfo(login);
if (Objects.equals(authorizationInfo.getUserId(), entity.getCreatedBy().getId())) {
hasAccess = true; // the role is the one that created the artifact
} else {
EntityType entityType = entityPermissionSchemaResolver.getEntityType(entity.getClass());

List<RoleDTO> roles = getRolesHavingPermissions(entityType, entity.getId());

Collection<String> userRoles = authorizationInfo.getRoles();
hasAccess = roles.stream()
.anyMatch(r -> userRoles.stream()
.anyMatch(re -> re.equals(r.getName())));
List<Permission> permsToCheck = getEntityPermissions(entityType, entity.getId(), accessType);
hasAccess = permsToCheck.stream().allMatch(p -> subject.isPermitted(p));
}
} catch (Exception e) {
logger.error("Error getting user roles and permissions", e);
Expand All @@ -277,43 +203,24 @@ public boolean hasWriteAccess(CommonEntity entity) {
}
return hasAccess;
}


public boolean hasWriteAccess(CommonEntity entity) {
return hasAccess(entity, AccessType.WRITE);
}

public boolean hasReadAccess(CommonEntity entity) {
boolean hasAccess = false;
if (securityEnabled && entity.getCreatedBy() != null) {
try {
String login = this.permissionManager.getSubjectName();
UserSimpleAuthorizationInfo authorizationInfo = this.permissionManager.getAuthorizationInfo(login);
if (Objects.equals(authorizationInfo.getUserId(), entity.getCreatedBy().getId())){
hasAccess = true; // the role is the one that created the artifact
} else {
EntityType entityType = entityPermissionSchemaResolver.getEntityType(entity.getClass());

List<RoleDTO> roles = getRolesHavingReadPermissions(entityType, entity.getId());

Collection<String> userRoles = authorizationInfo.getRoles();
hasAccess = roles.stream()
.anyMatch(r -> userRoles.stream()
.anyMatch(re -> re.equals(r.getName())));
}
} catch (Exception e) {
logger.error("Error getting user roles and permissions", e);
throw new RuntimeException(e);
}
}
return hasAccess;
return hasAccess(entity, AccessType.READ);
}

public void fillWriteAccess(CommonEntity entity, CommonEntityDTO entityDTO) {
if (securityEnabled && entity.getCreatedBy() != null) {
entityDTO.setHasWriteAccess(hasWriteAccess(entity));
entityDTO.setHasWriteAccess(hasAccess(entity, AccessType.WRITE));
}
}

public void fillReadAccess(CommonEntity entity, CommonEntityDTO entityDTO) {
if (securityEnabled && entity.getCreatedBy() != null) {
entityDTO.setHasReadAccess(hasReadAccess(entity));
entityDTO.setHasReadAccess(hasAccess(entity, AccessType.READ));
}
}

Expand Down
42 changes: 28 additions & 14 deletions src/main/java/org/ohdsi/webapi/security/model/UserSimpleAuthorizationInfo.java
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,39 @@
package org.ohdsi.webapi.security.model;

import java.util.List;
import java.util.Map;
import org.apache.shiro.authz.Permission;
import org.apache.shiro.authz.SimpleAuthorizationInfo;

public class UserSimpleAuthorizationInfo extends SimpleAuthorizationInfo {
private Long userId;

private String login;
private Long userId;
private String login;
private Map<String,List<Permission>> permissionIdx;

public Long getUserId() {
return userId;
}

public Long getUserId() {
return userId;
}

public void setUserId(Long userId) {
this.userId = userId;
}
public void setUserId(Long userId) {
this.userId = userId;
}

public String getLogin() {
return login;
}
public String getLogin() {
return login;
}

public void setLogin(String login) {
this.login = login;
}

public Map<String, List<Permission>> getPermissionIdx() {
return permissionIdx;
}

public void setPermissionIdx(Map<String, List<Permission>> permissionIdx) {
this.permissionIdx = permissionIdx;
}

public void setLogin(String login) {
this.login = login;
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/ohdsi/webapi/service/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static class User implements Comparable<User> {
public String login;
public String name;
public List<Permission> permissions;
public JsonNode permissionIdx;
public Map<String, List<String>> permissionIdx;

public User() {}

Expand Down
Loading

0 comments on commit 379d73b

Please sign in to comment.