diff --git a/pom.xml b/pom.xml index f799a72a71..c3778b5502 100644 --- a/pom.xml +++ b/pom.xml @@ -761,18 +761,6 @@ xml-security-impl 1.0 - - org.springframework.boot - spring-boot-starter-test - ${spring.boot.version} - test - - - com.vaadin.external.google - android-json - - - @@ -1029,24 +1017,6 @@ jasypt-hibernate4 1.9.2 - - org.dbunit - dbunit - 2.7.0 - test - - - com.github.springtestdbunit - spring-test-dbunit - 1.3.0 - test - - - pl.pragmatists - JUnitParams - 1.1.0 - test - org.bouncycastle bcprov-jdk15on @@ -1208,6 +1178,16 @@ commonmark-ext-gfm-tables 0.15.2 + + org.dom4j + dom4j + ${dom4j.version} + + + com.cloudbees + syslog-java-client + 1.1.7 + com.opentable.components otj-pg-embedded @@ -1215,24 +1195,43 @@ test - com.github.mjeanroy - dbunit-plus - 2.0.1 - test + org.springframework.boot + spring-boot-starter-test + ${spring.boot.version} + test + + + com.vaadin.external.google + android-json + + - org.dom4j - dom4j - ${dom4j.version} + org.dbunit + dbunit + 2.7.0 + test - com.cloudbees - syslog-java-client - 1.1.7 + com.github.springtestdbunit + spring-test-dbunit + 1.3.0 + test + + + pl.pragmatists + JUnitParams + 1.1.0 + test + + + com.github.mjeanroy + dbunit-plus + 2.0.1 + test - webapi-oracle diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionService.java b/src/main/java/org/ohdsi/webapi/security/PermissionService.java index 5bb63729d3..cc510579dd 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionService.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionService.java @@ -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; @@ -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 { @@ -64,9 +62,6 @@ public class PermissionService { @Value("#{!'${security.provider}'.equals('DisabledSecurity')}") private boolean securityEnabled; - private ThreadLocal>>> permissionCache = - ThreadLocal.withInitial(ConcurrentHashMap::new); - private final EntityGraph PERMISSION_ENTITY_GRAPH = EntityGraphUtils.fromAttributePaths("rolePermissions", "rolePermissions.role"); public PermissionService( @@ -133,14 +128,14 @@ public void checkCommonEntityOwnership(EntityType entityType, Integer entityId) public Map 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 finaAllRolesHavingPermissions(List permissions) { @@ -178,97 +173,28 @@ private boolean isCurrentUserOwnerOf(CommonEntity entity) { return Objects.equals(owner.getLogin(), loggedInUsername); } + public List getEntityPermissions(EntityType entityType, Number id, AccessType accessType) { + Set permissionTemplates = getTemplatesForType(entityType, accessType).keySet(); - public void preparePermissionCache(EntityType entityType, Set permissionTemplates) { - if (permissionCache.get().get(entityType) == null) { - final ConcurrentHashMap> rolesForEntity = new ConcurrentHashMap<>(); - permissionCache.get().put(entityType, rolesForEntity); - - List permissionsSQLTemplates = permissionTemplates.stream() - .map(pt -> getPermissionSqlTemplate(pt)) - .collect(Collectors.toList()); - - Map roleDTOMap = new HashMap<>(); - permissionsSQLTemplates.forEach(p -> { - Iterable permissionEntities = permissionRepository.findByValueLike(p, PERMISSION_ENTITY_GRAPH); - for (PermissionEntity permissionEntity : permissionEntities) { - Set roles = rolesForEntity.get(permissionEntity.getValue()); - if (roles == null) { - rolesForEntity.put(permissionEntity.getValue(), new HashSet<>()); - } - Set 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 getRolesHavingPermissions(EntityType entityType, Number id) { - Set permissionTemplates = getTemplatesForType(entityType, AccessType.WRITE).keySet(); - preparePermissionCache(entityType, permissionTemplates); - - List permissions = permissionTemplates.stream() - .map(pt -> getPermission(pt, id)) + List permissions = permissionTemplates.stream() + .map(pt -> new WildcardPermission(getPermission(pt, id))) .collect(Collectors.toList()); - int fitCount = permissions.size(); - Map 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 roles = roleMap.entrySet().stream() - .filter(es -> es.getValue() == fitCount) - .map(es -> es.getKey()) - .collect(Collectors.toList()); - return roles; - } - - public List getRolesHavingReadPermissions(EntityType entityType, Number id) { - Set permissionTemplates = getTemplatesForType(entityType, AccessType.READ).keySet(); - preparePermissionCache(entityType, permissionTemplates); - - List permissions = permissionTemplates.stream() - .map(pt -> getPermission(pt, id)) - .collect(Collectors.toList()); - int fitCount = permissions.size(); - Map 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 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 roles = getRolesHavingPermissions(entityType, entity.getId()); - - Collection userRoles = authorizationInfo.getRoles(); - hasAccess = roles.stream() - .anyMatch(r -> userRoles.stream() - .anyMatch(re -> re.equals(r.getName()))); + List 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); @@ -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 roles = getRolesHavingReadPermissions(entityType, entity.getId()); - - Collection 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)); } } diff --git a/src/main/java/org/ohdsi/webapi/security/model/UserSimpleAuthorizationInfo.java b/src/main/java/org/ohdsi/webapi/security/model/UserSimpleAuthorizationInfo.java index e4d09e44a8..5db3519fa4 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/UserSimpleAuthorizationInfo.java +++ b/src/main/java/org/ohdsi/webapi/security/model/UserSimpleAuthorizationInfo.java @@ -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> 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> getPermissionIdx() { + return permissionIdx; + } + + public void setPermissionIdx(Map> permissionIdx) { + this.permissionIdx = permissionIdx; + } - public void setLogin(String login) { - this.login = login; - } } diff --git a/src/main/java/org/ohdsi/webapi/service/UserService.java b/src/main/java/org/ohdsi/webapi/service/UserService.java index 2c786a8440..5dd56bcc18 100644 --- a/src/main/java/org/ohdsi/webapi/service/UserService.java +++ b/src/main/java/org/ohdsi/webapi/service/UserService.java @@ -51,7 +51,7 @@ public static class User implements Comparable { public String login; public String name; public List permissions; - public JsonNode permissionIdx; + public Map> permissionIdx; public User() {} diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 96c6d13c55..43ff4ace64 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -1,8 +1,6 @@ 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; @@ -28,13 +26,18 @@ import org.springframework.transaction.annotation.Transactional; import java.security.Principal; +import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; +import org.apache.shiro.authz.Permission; +import org.apache.shiro.authz.permission.WildcardPermission; import org.ohdsi.circe.helper.ResourceHelper; import org.springframework.beans.factory.annotation.Value; import org.springframework.jdbc.core.JdbcTemplate; @@ -71,14 +74,11 @@ public class PermissionManager { @Autowired private JdbcTemplate jdbcTemplate; - @Autowired - private ObjectMapper objectMapper; - private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); public static class PermissionsDTO { - public JsonNode permissions = null; + public Map> permissions = null; } public RoleEntity addRole(String roleName, boolean isSystem) { @@ -132,6 +132,12 @@ public Iterable getRoles(boolean includePersonalRoles) { } } + /** + * Return the UserSimpleAuthorizastionInfo which contains the login, roles and permissions for the specified login + * + * @param login The login to fetch the authorization info + * @return A UserSimpleAuthorizationInfo containing roles and permissions. + */ public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) { return authorizationInfoCache.get().computeIfAbsent(login, newLogin -> { @@ -148,14 +154,21 @@ public UserSimpleAuthorizationInfo getAuthorizationInfo(final String login) { for (UserRoleEntity userRole: userEntity.getUserRoles()) { info.addRole(userRole.getRole().getName()); } - final Set permissionNames = new LinkedHashSet<>(); - final Set permissions = this.getUserPermissions(userEntity); - for (PermissionEntity permission : permissions) { - permissionNames.add(permission.getValue()); + // convert permission index from queryUserPermissions() into a map of WildcardPermissions + Map> permsIdx = this.queryUserPermissions(newLogin).permissions; + Map permissionMap = new HashMap>(); + Set permissionNames = new HashSet<>(); + + for(String permIdxKey : permsIdx.keySet()) { + List perms = permsIdx.get(permIdxKey); + permissionNames.addAll(perms); + // convert raw string permission into Wildcard perm for each element in this key's array. + permissionMap.put(permIdxKey, perms.stream().map(perm -> new WildcardPermission(perm)).collect(Collectors.toList())); } info.setStringPermissions(permissionNames); + info.setPermissionIdx(permissionMap); return info; }); } @@ -345,7 +358,7 @@ public PermissionsDTO queryUserPermissions(final String login) { return rs.getString("value"); }); PermissionsDTO permDto = new PermissionsDTO(); - permDto.permissions = permsToJsonNode(permissions); + permDto.permissions = permsToMap(permissions); return permDto; } @@ -354,23 +367,23 @@ public PermissionsDTO queryUserPermissions(final String login) { * 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) { + private Map> permsToMap(List permissions) { - Map resultMap = new HashMap<>(); + 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()); + resultMap.putIfAbsent(key, new ArrayList<>()); // Add the value to the JsonArray resultMap.get(key).add(inputString); } // Convert the resultMap to a JsonNode - return objectMapper.valueToTree(resultMap); + return resultMap; } private Set getRolePermissions(RoleEntity role) { @@ -474,7 +487,7 @@ private RolePermissionEntity addPermission(final RoleEntity role, final Permissi } private boolean isRelationAllowed(final String relationStatus) { - return relationStatus == null || relationStatus == RequestStatus.APPROVED; + return relationStatus == null || relationStatus.equals(RequestStatus.APPROVED); } private UserRoleEntity addUser(final UserEntity user, final RoleEntity role, diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java index ce12edbe45..d76aa8a921 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/CacheFilter.java @@ -19,9 +19,6 @@ public class CacheFilter implements Filter { @Autowired private PermissionManager permissionManager; - @Autowired - private PermissionService permissionService; - @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -31,7 +28,6 @@ public void init(FilterConfig filterConfig) throws ServletException { public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { permissionManager.clearAuthorizationInfoCache(); - permissionService.clearPermissionCache(); chain.doFilter(request, response); } diff --git a/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java b/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java index 2c1a4514db..e7245b1b49 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java +++ b/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java @@ -1,12 +1,17 @@ package org.ohdsi.webapi.shiro.realms; +import java.util.ArrayList; +import java.util.List; +import org.apache.commons.lang3.StringUtils; import org.apache.shiro.authc.AuthenticationException; import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.SimpleAuthenticationInfo; import org.apache.shiro.authz.AuthorizationInfo; +import org.apache.shiro.authz.Permission; import org.apache.shiro.realm.AuthorizingRealm; import org.apache.shiro.subject.PrincipalCollection; +import org.ohdsi.webapi.security.model.UserSimpleAuthorizationInfo; import org.ohdsi.webapi.shiro.PermissionManager; import org.ohdsi.webapi.shiro.tokens.JwtAuthToken; @@ -37,5 +42,20 @@ protected AuthorizationInfo doGetAuthorizationInfo(PrincipalCollection principal @Override protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken at) throws AuthenticationException { return new SimpleAuthenticationInfo(at.getPrincipal(), "", getName()); - } + } + + @Override + protected boolean isPermitted(Permission permission, AuthorizationInfo info) { + // for speed, we check the permission against the set of permissions found by the key '*' and the first element of the permission. + String permPrefix = StringUtils.split(permission.toString(), ":")[0]; + // we only need to check * perms and perms that begin with the same prefix (up to first :) as the requested permission + // starting with perms that start with * + List permsToCheck = new ArrayList(((UserSimpleAuthorizationInfo)info).getPermissionIdx().getOrDefault("*", new ArrayList<>())); + // adding those permissiosn that start with the permPrefix + permsToCheck.addAll(((UserSimpleAuthorizationInfo)info).getPermissionIdx().getOrDefault(permPrefix, new ArrayList<>())); + + // do check + return permsToCheck.stream().anyMatch(permToCheck -> permToCheck.implies(permission)); + + } } diff --git a/src/test/java/org/ohdsi/webapi/AbstractDatabaseTest.java b/src/test/java/org/ohdsi/webapi/AbstractDatabaseTest.java index afb076d75b..9b7e5417d5 100644 --- a/src/test/java/org/ohdsi/webapi/AbstractDatabaseTest.java +++ b/src/test/java/org/ohdsi/webapi/AbstractDatabaseTest.java @@ -1,5 +1,6 @@ package org.ohdsi.webapi; +import com.github.mjeanroy.dbunit.core.dataset.DataSetFactory; import org.junit.ClassRule; import org.junit.Ignore; import org.junit.rules.ExternalResource; @@ -16,63 +17,101 @@ import java.sql.Driver; import java.sql.DriverManager; import java.sql.SQLException; +import org.dbunit.DatabaseUnitException; +import org.dbunit.database.DatabaseConfig; +import org.dbunit.database.DatabaseDataSourceConnection; +import org.dbunit.database.IDatabaseConnection; +import org.dbunit.dataset.IDataSet; +import org.dbunit.ext.postgresql.PostgresqlDataTypeFactory; +import org.dbunit.operation.DatabaseOperation; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; @SpringBootTest @RunWith(SpringRunner.class) @TestPropertySource(locations = "/application-test.properties") public abstract class AbstractDatabaseTest { - static class JdbcTemplateTestWrapper extends ExternalResource { - @Override - protected void before() throws Throwable { - jdbcTemplate = new JdbcTemplate(getDataSource()); - try { - // note for future reference: should probably either define a TestContext DataSource with these params - // or make it so this proparty is only set once (during database initialization) since the below will run for each test class (but only be effective once) - System.setProperty("datasource.url", getDataSource().getConnection().getMetaData().getURL()); - System.setProperty("flyway.datasource.url", System.getProperty("datasource.url")); - } catch (Exception ex) { - throw new RuntimeException(ex); - } - } + + static class JdbcTemplateTestWrapper extends ExternalResource { + + @Override + protected void before() throws Throwable { + jdbcTemplate = new JdbcTemplate(getDataSource()); + try { + // note for future reference: should probably either define a TestContext DataSource with these params + // or make it so this proparty is only set once (during database initialization) since the below will run for each test class (but only be effective once) + System.setProperty("datasource.url", getDataSource().getConnection().getMetaData().getURL()); + System.setProperty("flyway.datasource.url", System.getProperty("datasource.url")); + } catch (Exception ex) { + throw new RuntimeException(ex); + } } + } - static class DriverExcludeTestWrapper extends ExternalResource { - @Override - protected void before() throws Throwable { - // Put the redshift driver at the end so that it doesn't - // conflict with postgres queries - java.util.Enumeration drivers = DriverManager.getDrivers(); - while (drivers.hasMoreElements()) { - Driver driver = drivers.nextElement(); - if (driver.getClass().getName().contains("com.amazon.redshift.jdbc")) { - try { - DriverManager.deregisterDriver(driver); - DriverManager.registerDriver(driver); - } catch (SQLException e) { - throw new RuntimeException("Could not deregister redshift driver", e); - } - } - } + static class DriverExcludeTestWrapper extends ExternalResource { + + @Override + protected void before() throws Throwable { + // Put the redshift driver at the end so that it doesn't + // conflict with postgres queries + java.util.Enumeration drivers = DriverManager.getDrivers(); + while (drivers.hasMoreElements()) { + Driver driver = drivers.nextElement(); + if (driver.getClass().getName().contains("com.amazon.redshift.jdbc")) { + try { + DriverManager.deregisterDriver(driver); + DriverManager.registerDriver(driver); + } catch (SQLException e) { + throw new RuntimeException("Could not deregister redshift driver", e); + } } + } } + } - @ClassRule - public static TestRule chain = RuleChain.outerRule(new DriverExcludeTestWrapper()) - .around(pg = new PostgresSingletonRule()) - .around(new JdbcTemplateTestWrapper()); + @ClassRule + public static TestRule chain = RuleChain.outerRule(new DriverExcludeTestWrapper()) + .around(pg = new PostgresSingletonRule()) + .around(new JdbcTemplateTestWrapper()); - protected static PostgresSingletonRule pg; + protected static PostgresSingletonRule pg; - protected static JdbcTemplate jdbcTemplate; + protected static JdbcTemplate jdbcTemplate; - protected static DataSource getDataSource() { - return pg.getEmbeddedPostgres().getPostgresDatabase(); - } - - protected void truncateTable (String tableName) { - jdbcTemplate.execute(String.format("TRUNCATE %s CASCADE",tableName)); - } - protected void resetSequence(String sequenceName) { - jdbcTemplate.execute(String.format("ALTER SEQUENCE %s RESTART WITH 1", sequenceName)); + protected static DataSource getDataSource() { + return pg.getEmbeddedPostgres().getPostgresDatabase(); + } + + protected void truncateTable(String tableName) { + jdbcTemplate.execute(String.format("TRUNCATE %s CASCADE", tableName)); + } + + protected void resetSequence(String sequenceName) { + jdbcTemplate.execute(String.format("ALTER SEQUENCE %s RESTART WITH 1", sequenceName)); + } + + protected static IDatabaseConnection getConnection() throws SQLException { + final IDatabaseConnection con = new DatabaseDataSourceConnection(getDataSource()); + con.getConfig().setProperty(DatabaseConfig.FEATURE_QUALIFIED_TABLE_NAMES, true); + con.getConfig().setProperty(DatabaseConfig.PROPERTY_DATATYPE_FACTORY, new PostgresqlDataTypeFactory()); + return con; + } + + protected void loadPrepData(String[] datasetPaths) throws Exception { + loadPrepData(datasetPaths, DatabaseOperation.CLEAN_INSERT); + } + protected void loadPrepData(String[] datasetPaths, DatabaseOperation dbOp) throws Exception { + final IDatabaseConnection dbUnitCon = getConnection(); + final IDataSet ds = DataSetFactory.createDataSet(datasetPaths); + + assertNotNull("No dataset found", ds); + + try { + dbOp.execute(dbUnitCon, ds); // clean load of the DB. Careful, clean means "delete the old stuff" + } catch (final DatabaseUnitException e) { + fail(e.getMessage()); + } finally { + dbUnitCon.close(); } + } } diff --git a/src/test/java/org/ohdsi/webapi/pathway/PathwayAnalysisTest.java b/src/test/java/org/ohdsi/webapi/pathway/PathwayAnalysisTest.java index 0c4aa72881..9ff81cecb8 100644 --- a/src/test/java/org/ohdsi/webapi/pathway/PathwayAnalysisTest.java +++ b/src/test/java/org/ohdsi/webapi/pathway/PathwayAnalysisTest.java @@ -24,16 +24,12 @@ import java.util.Collection; import org.dbunit.Assertion; import org.dbunit.DatabaseUnitException; -import org.dbunit.database.DatabaseConfig; -import org.dbunit.database.DatabaseDataSourceConnection; import org.dbunit.database.IDatabaseConnection; import org.dbunit.dataset.CompositeDataSet; import org.dbunit.dataset.IDataSet; import org.dbunit.dataset.ITable; -import org.dbunit.ext.postgresql.PostgresqlDataTypeFactory; import org.dbunit.operation.DatabaseOperation; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assert.assertEquals; @@ -96,12 +92,6 @@ public class PathwayAnalysisTest extends AbstractDatabaseTest { private static SerializedPathwayAnalysisToPathwayAnalysisConverter converter = new SerializedPathwayAnalysisToPathwayAnalysisConverter(); - private static IDatabaseConnection getConnection() throws SQLException { - final IDatabaseConnection con = new DatabaseDataSourceConnection(getDataSource()); - con.getConfig().setProperty(DatabaseConfig.FEATURE_QUALIFIED_TABLE_NAMES, true); - con.getConfig().setProperty(DatabaseConfig.PROPERTY_DATATYPE_FACTORY, new PostgresqlDataTypeFactory()); - return con; - } @Before public void setUp() throws Exception { @@ -183,21 +173,6 @@ private Source getCdmSource() throws SQLException { return source; } - private void loadPrepData(String[] datasetPaths) throws Exception { - final IDatabaseConnection dbUnitCon = getConnection(); - final IDataSet ds = DataSetFactory.createDataSet(datasetPaths); - - assertNotNull("No dataset found", ds); - - try { - DatabaseOperation.CLEAN_INSERT.execute(dbUnitCon, ds); // clean load of the DB. Careful, clean means "delete the old stuff" - } catch (final DatabaseUnitException e) { - fail(e.getMessage()); - } finally { - dbUnitCon.close(); - } - } - private void generateAnalysis(PathwayAnalysisEntity entity) throws Exception { JobExecutionResource executionResource = pathwayService.generatePathways(entity.getId(), SOURCE_ID); PathwayAnalysisGenerationEntity generationEntity; diff --git a/src/test/java/org/ohdsi/webapi/security/PermissionTest.java b/src/test/java/org/ohdsi/webapi/security/PermissionTest.java new file mode 100644 index 0000000000..c8a83be095 --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/security/PermissionTest.java @@ -0,0 +1,105 @@ +/* + * Copyright 2024 cknoll1. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.ohdsi.webapi.security; + +import java.util.Arrays; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.SimplePrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.web.mgt.DefaultWebSecurityManager; +import org.dbunit.operation.DatabaseOperation; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import org.junit.Before; +import org.junit.Test; +import org.ohdsi.webapi.AbstractDatabaseTest; +import org.ohdsi.webapi.shiro.PermissionManager; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.test.context.TestPropertySource; + +/** + * + * @author cknoll1 + */ +@TestPropertySource(properties = { + "security.provider=AtlasRegularSecurity" +}) +public class PermissionTest extends AbstractDatabaseTest { + + @Autowired + private PermissionManager permissionManager; + + @Value("${security.provider}") + String securityProvider; + + @Autowired + private DefaultWebSecurityManager securityManager; + + private Subject subject; + + @Before + public void setup() { + // Set the SecurityManager for the current thread + SimplePrincipalCollection principalCollection = new SimplePrincipalCollection(); + principalCollection.addAll(Arrays.asList("permsTest"),"testRealm"); + subject = new Subject.Builder(securityManager) + .authenticated(true) + .principals(principalCollection) + .buildSubject(); + ThreadContext.bind(subject); + } + + @Test + public void permsTest() throws Exception { + // need to clear authorization cache before each test + permissionManager.clearAuthorizationInfoCache(); + Subject s = SecurityUtils.getSubject(); + String subjetName = permissionManager.getSubjectName(); + + final String[] testDataSetsPaths = new String[] {"/permission/permsTest_PREP.json" }; + + loadPrepData(testDataSetsPaths, DatabaseOperation.REFRESH); + + // subject can manage printer1 and printer2, can do print and query on any printer. + assertTrue(s.isPermitted("printer:manage:printer1")); + assertTrue(s.isPermitted("printer:manage:printer2")); + assertFalse(s.isPermitted("printer:manage:printer3")); + assertTrue(s.isPermitted("printer:query:printer4")); + assertTrue(s.isPermitted("printer:print:printer5")); + + loadPrepData(testDataSetsPaths, DatabaseOperation.DELETE); + + } + + @Test + public void wildcardTest() throws Exception { + // need to clear authorization cache before each test + permissionManager.clearAuthorizationInfoCache(); + Subject s = SecurityUtils.getSubject(); + final String[] testDataSetsPaths = new String[] {"/permission/wildcardTest_PREP.json" }; + loadPrepData(testDataSetsPaths, DatabaseOperation.REFRESH); + + // subject has * permisison, so any permisison test is true + assertTrue(s.isPermitted("printer:manage:printer1")); + assertTrue(s.isPermitted("printer")); + + loadPrepData(testDataSetsPaths, DatabaseOperation.DELETE); + + } + +} diff --git a/src/test/resources/permission/permsTest_PREP.json b/src/test/resources/permission/permsTest_PREP.json new file mode 100644 index 0000000000..8ad261396e --- /dev/null +++ b/src/test/resources/permission/permsTest_PREP.json @@ -0,0 +1,65 @@ +{ + "public.sec_user": [ + { + "id":1001, + "login":"permsTest", + "name":"Perms Test User", + "origin":"SYSTEM" + } + ], + "public.sec_role": [ + { + "id": 1001, + "name":"permsTest", + "system_role":false + } + ], + "public.sec_permission" : [ + { + "id": 1001, + "value":"printer:manage:printer1" + }, + { + "id": 1002, + "value":"printer:manage:printer2" + }, + { + "id": 1003, + "value":"printer:query:*" + }, + { + "id": 1004, + "value":"printer:print" + } + ], + "public.sec_role_permission" : [ + { + "id":1001, + "role_id":1001, + "permission_id":1001 + }, + { + "id":1002, + "role_id":1001, + "permission_id":1002 + }, + { + "id":1003, + "role_id":1001, + "permission_id":1003 + }, + { + "id":1004, + "role_id":1001, + "permission_id":1004 + } + ], + "public.sec_user_role": [ + { + "id": 1001, + "user_id":1001, + "role_id":1001, + "origin":"SYSTEM" + } + ] +} diff --git a/src/test/resources/permission/wildcardTest_PREP.json b/src/test/resources/permission/wildcardTest_PREP.json new file mode 100644 index 0000000000..6f08990e8f --- /dev/null +++ b/src/test/resources/permission/wildcardTest_PREP.json @@ -0,0 +1,38 @@ +{ + "public.sec_user": [ + { + "id":1001, + "login":"permsTest", + "name":"Perms Test User", + "origin":"SYSTEM" + } + ], + "public.sec_role": [ + { + "id": 1001, + "name":"permsTest", + "system_role":false + } + ], + "public.sec_permission" : [ + { + "id": 1001, + "value":"*" + } + ], + "public.sec_role_permission" : [ + { + "id":1001, + "role_id":1001, + "permission_id":1001 + } + ], + "public.sec_user_role": [ + { + "id": 1001, + "user_id":1001, + "role_id":1001, + "origin":"SYSTEM" + } + ] +}