Skip to content

Commit

Permalink
Filter cohorts and concepts by permission (#2245) (#2301)
Browse files Browse the repository at this point in the history
The new property is called security.defaultGlobalReadPermissions which is true by default.

Co-authored-by: Richard D Boyce, PhD <rdb20@pitt.edu>
  • Loading branch information
chrisknoll and rkboyce committed Aug 14, 2023
1 parent c6be6ce commit 5393ed2
Show file tree
Hide file tree
Showing 26 changed files with 319 additions and 87 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Makefile
WebAPIConfig/
*application.properties
.idea/
Expand All @@ -12,6 +13,7 @@ sandbox/
/nbactions*.xml
*~
.DS_Store
.factorypath

### Developer's personal properties ###
**/resources/config/application*-dev-*.properties
Expand Down
55 changes: 10 additions & 45 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,56 +1,21 @@
compile:
mvn clean
mvn compile -Pwebapi-postgresql-laertes
mvn clean compile -DskipUnitTests -DskipITtests -s WebAPIConfig/settings.xml -P webapi-postgresql

package: compile
mvn package -Pwebapi-postgresql-laertes
mvn package -DskipUnitTests -DskipITtests -s WebAPIConfig/settings.xml -P webapi-postgresql

deploy: package
sudo service tomcat7 stop
sleep 4
sudo rm -rf /var/lib/tomcat7/webapps/WebAPI*
sudo cp -r target/WebAPI.war /var/lib/tomcat7/webapps/
sudo chown tomcat7 /var/lib/tomcat7/webapps/WebAPI.war
sudo chgrp tomcat7 /var/lib/tomcat7/webapps/WebAPI.war
sudo service tomcat7 start
deploy: package
/home/ubuntu/Downloads/apache-tomcat-8.5.84-DEV/bin/shutdown.sh
mv /home/ubuntu/Downloads/apache-tomcat-8.5.84-DEV/webapps/WebAPI /mnt/disk1/webapi-dev-tmp/WebAPI-FOLDER-`date +%m%d%H%S`
mv /home/ubuntu/Downloads/apache-tomcat-8.5.84-DEV/webapps/WebAPI.war /mnt/disk1/webapi-dev-tmp/WebAPI.war-`date +%m%d%H%S`
mv target/WebAPI.war /home/ubuntu/Downloads/apache-tomcat-8.5.84-DEV/webapps/
echo "Now run /home/ubuntu/Downloads/apache-tomcat-8.5.84-DEV/bin/startup.sh"

git-push:
git push myfork master
git push

test:
wget -O tests/test-general-evidence.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/752061"
wget -O tests/test-drug-hoi.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/752061-374013"
wget -O tests/test-drug.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drug/752061"
wget -O tests/test-hoi.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/hoi/320073"
wget -O tests/test-info.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/info"
wget -O tests/test-drug-hoi-eu-spc.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/904351-4190045"
wget -O tests/test-drug-hoi-splicer.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/19133853-195588"
wget -O tests/test-drug-hoi-faers-counts-and-signals.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/1154343-433031"
wget -O tests/test-drug-hoi-pubmed-mesh-cr.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/1154343-433031"
wget -O tests/test-drug-hoi-pubmed-mesh-clin-trial.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/789578-378144"
wget -O tests/test-drug-hoi-pubmed-mesh-other.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/19010482-316866"
wget -O tests/test-drug-hoi-semmed-cr.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/1112807-441202"
wget -O tests/test-drug-hoi-semmed-clin-trial.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drughoi/19059744-381591"
wget -O tests/test-drug-rollup-ingredient.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drugrollup/ingredient/1000632"
wget -O tests/test-drug-rollup-clin-drug.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drugrollup/clinicaldrug/19074181"
wget -O tests/test-drug-rollup-branded-drug.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/drugrollup/brandeddrug/1000640"
wget -O tests/test-rdf-evidencesummary.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/evidencesummary?conditionID=139900&drugID=1115008&evidenceGroup=Literature"
wget -O tests/test-rdf-evidencedetails.json "http://localhost:8080/WebAPI/LAERTES_CDM/evidence/evidencedetails?conditionID=24134&drugID=1115008&evidenceType=SPL_SPLICER_ADR"
wget -O /tmp/tests/test-drug-rollup-branded-drug.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drugrollup/brandeddrug/1000640"

test-public:
wget -O tests/test-general-evidence.json "http://api.ohdsi.org/WebAPI/CS1/evidence/1000640"
wget -O /tmp/tests/test-drug-hoi.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/1000640-137682"
wget -O /tmp/tests/test-drug.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drug/1000640"
wget -O /tmp/tests/test-hoi.json "http://api.ohdsi.org/WebAPI/CS1/evidence/hoi/320073"
wget -O /tmp/tests/test-info.json "http://api.ohdsi.org/WebAPI/CS1/evidence/info"
wget -O /tmp/tests/test-drug-hoi-eu-spc.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/40239056-75053"
wget -O /tmp/tests/test-drug-hoi-splicer.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/19133853-195588"
wget -O /tmp/tests/test-drug-hoi-faers-counts-and-signals.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/1154343-433031"
wget -O /tmp/tests/test-drug-hoi-pubmed-mesh-cr.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/1154343-433031"
wget -O /tmp/tests/test-drug-hoi-pubmed-mesh-clin-trial.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/789578-378144"
wget -O /tmp/tests/test-drug-hoi-pubmed-mesh-other.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/19010482-316866"
wget -O /tmp/tests/test-drug-hoi-semmed-cr.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/1782521-45612000"
wget -O /tmp/tests/test-drug-hoi-semmed-clin-trial.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drughoi/1303425-45616736"
wget -O /tmp/tests/test-drug-rollup-ingredient.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drugrollup/ingredient/1000632"
wget -O /tmp/tests/test-drug-rollup-clin-drug.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drugrollup/clinicaldrug/19074181"
wget -O /tmp/tests/test-drug-rollup-branded-drug.json "http://api.ohdsi.org/WebAPI/CS1/evidence/drugrollup/brandeddrug/1000640"
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@
<spring.datasource.hikari.register-mbeans>true</spring.datasource.hikari.register-mbeans>
<spring.datasource.hikari.mbean-name>authDataSource</spring.datasource.hikari.mbean-name>

<!-- If defaultGlobalReadPermissions is set to true (default), then all users can see every artifact. -->
<!-- If it is set to false, WebAPI will filter out the artifacts that a user does not explicitly have -->
<!-- read permissions to -->
<security.defaultGlobalReadPermissions>true</security.defaultGlobalReadPermissions>

<!-- EMBEDDED SERVER CONFIGURATION (ServerProperties) -->
<server.port>8080</server.port>
<server.ssl.key-store></server.ssl.key-store>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@
import org.ohdsi.webapi.versioning.dto.VersionUpdateDTO;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Controller;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.web.bind.annotation.RequestBody;

import javax.ws.rs.Consumes;
Expand All @@ -63,6 +65,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -151,11 +154,12 @@ public CohortCharacterizationDTO copy(@PathParam("id") final Long id) {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public Page<CcShortDTO> list(@Pagination Pageable pageable) {
return service.getPage(pageable).map(entity -> {
CcShortDTO dto = convertCcToShortDto(entity);
permissionService.fillWriteAccess(entity, dto);
return dto;
});
return service.getPage(pageable).map(entity -> {
CcShortDTO dto = convertCcToShortDto(entity);
permissionService.fillWriteAccess(entity, dto);
permissionService.fillReadAccess(entity, dto);
return dto;
});
}

/**
Expand All @@ -168,7 +172,12 @@ public Page<CcShortDTO> list(@Pagination Pageable pageable) {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public Page<CohortCharacterizationDTO> listDesign(@Pagination Pageable pageable) {
return service.getPageWithLinkedEntities(pageable).map(this::convertCcToDto);
return service.getPageWithLinkedEntities(pageable).map(entity -> {
CohortCharacterizationDTO dto = convertCcToDto(entity);
permissionService.fillWriteAccess(entity, dto);
permissionService.fillReadAccess(entity, dto);
return dto;
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@
import static org.ohdsi.webapi.Constants.Params.JOB_AUTHOR;
import static org.ohdsi.webapi.Constants.Params.JOB_NAME;
import static org.ohdsi.webapi.Constants.Params.SOURCE_ID;
import org.ohdsi.webapi.security.PermissionService;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.domain.PageImpl;

@Service
@Transactional
Expand Down Expand Up @@ -207,7 +210,12 @@ public class CcServiceImpl extends AbstractDaoService implements CcService, Gene
private final GenericConversionService genericConversionService;
private final VocabularyService vocabularyService;
private VersionService<CharacterizationVersion> versionService;

private PermissionService permissionService;

@Value("#{'${security.defaultGlobalReadPermissions}'.equals(false)}")
private boolean defaultGlobalReadPermissions;

private final Environment env;

public CcServiceImpl(
Expand All @@ -231,6 +239,7 @@ public CcServiceImpl(
final JobInvalidator jobInvalidator,
final VocabularyService vocabularyService,
final VersionService<CharacterizationVersion> versionService,
final PermissionService permissionService,
@Qualifier("conversionService") final GenericConversionService genericConversionService,
Environment env) {
this.repository = ccRepository;
Expand All @@ -251,6 +260,7 @@ public CcServiceImpl(
this.eventPublisher = eventPublisher;
this.jobInvalidator = jobInvalidator;
this.vocabularyService = vocabularyService;
this.permissionService = permissionService;
this.genericConversionService = genericConversionService;
this.versionService = versionService;
this.env = env;
Expand Down Expand Up @@ -531,12 +541,24 @@ public CohortCharacterization findDesignByGenerationId(@CcGenerationId final Lon

@Override
public Page<CohortCharacterizationEntity> getPageWithLinkedEntities(final Pageable pageable) {
return repository.findAll(pageable, defaultEntityGraph);
return repository.findAll(pageable, defaultEntityGraph);

}

@Override
public Page<CohortCharacterizationEntity> getPage(final Pageable pageable) {
return repository.findAll(pageable);
List<CohortCharacterizationEntity> ccList = repository.findAll()
.stream().filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true)
.collect(Collectors.toList());
return getPageFromResults(pageable, ccList);
}

private Page<CohortCharacterizationEntity> getPageFromResults(Pageable pageable, List<CohortCharacterizationEntity> results) {
// Calculate the start and end indices for the current page
int startIndex = pageable.getPageNumber() * pageable.getPageSize();
int endIndex = Math.min(startIndex + pageable.getPageSize(), results.size());

return new PageImpl<>(results.subList(startIndex, endIndex), pageable, results.size());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.convert.support.GenericConversionService;
import org.springframework.stereotype.Controller;

Expand Down Expand Up @@ -70,7 +71,10 @@ public class EstimationController {
private final ScriptExecutionService executionService;
private EstimationChecker checker;
private PermissionService permissionService;


@Value("#{'${security.defaultGlobalReadPermissions}'.equals(false)}")
private boolean defaultGlobalReadPermissions;

public EstimationController(EstimationService service,
GenericConversionService conversionService,
CommonGenerationSensitiveInfoService sensitiveInfoService,
Expand All @@ -97,11 +101,12 @@ public EstimationController(EstimationService service,
@Path("/")
@Produces(MediaType.APPLICATION_JSON)
public List<EstimationShortDTO> getAnalysisList() {

return StreamSupport.stream(service.getAnalysisList().spliterator(), false)
.filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true)
.map(analysis -> {
EstimationShortDTO dto = conversionService.convert(analysis, EstimationShortDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@
import org.ohdsi.webapi.versioning.dto.VersionDTO;
import org.ohdsi.webapi.versioning.dto.VersionUpdateDTO;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestBody;

import javax.transaction.Transactional;
import javax.ws.rs.*;
import javax.ws.rs.core.MediaType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -156,13 +159,14 @@ public PathwayAnalysisDTO importAnalysis(final PathwayAnalysisExportDTO dto) {
@Consumes(MediaType.APPLICATION_JSON)
@Transactional
public Page<PathwayAnalysisDTO> list(@Pagination Pageable pageable) {

return pathwayService.getPage(pageable).map(pa -> {
PathwayAnalysisDTO dto = conversionService.convert(pa, PathwayAnalysisDTO.class);
permissionService.fillWriteAccess(pa, dto);
permissionService.fillReadAccess(pa, dto);
return dto;
});
}


/**
* Check that a pathway analysis name exists.
Expand Down
25 changes: 23 additions & 2 deletions src/main/java/org/ohdsi/webapi/pathway/PathwayServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@
import static org.ohdsi.webapi.Constants.Params.JOB_NAME;
import static org.ohdsi.webapi.Constants.Params.PATHWAY_ANALYSIS_ID;
import static org.ohdsi.webapi.Constants.Params.SOURCE_ID;
import org.ohdsi.webapi.cohortcharacterization.domain.CohortCharacterizationEntity;
import org.ohdsi.webapi.security.PermissionService;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.domain.PageImpl;

@Service
@Transactional
Expand All @@ -116,6 +120,11 @@ public class PathwayServiceImpl extends AbstractDaoService implements PathwaySer
private final CohortDefinitionService cohortDefinitionService;
private final VersionService<PathwayVersion> versionService;

private PermissionService permissionService;

@Value("#{'${security.defaultGlobalReadPermissions}'.equals(false)}")
private boolean defaultGlobalReadPermissions;

private final List<String> STEP_COLUMNS = Arrays.asList(new String[]{"step_1", "step_2", "step_3", "step_4", "step_5", "step_6", "step_7", "step_8", "step_9", "step_10"});

private final EntityGraph defaultEntityGraph = EntityUtils.fromAttributePaths(
Expand All @@ -142,7 +151,8 @@ public PathwayServiceImpl(
@Qualifier("conversionService") GenericConversionService genericConversionService,
StepBuilderFactory stepBuilderFactory,
CohortDefinitionService cohortDefinitionService,
VersionService<PathwayVersion> versionService) {
VersionService<PathwayVersion> versionService,
PermissionService permissionService) {

this.pathwayAnalysisRepository = pathwayAnalysisRepository;
this.pathwayAnalysisGenerationRepository = pathwayAnalysisGenerationRepository;
Expand All @@ -159,6 +169,7 @@ public PathwayServiceImpl(
this.stepBuilderFactory = stepBuilderFactory;
this.cohortDefinitionService = cohortDefinitionService;
this.versionService = versionService;
this.permissionService = permissionService;

SerializedPathwayAnalysisToPathwayAnalysisConverter.setConversionService(conversionService);
}
Expand Down Expand Up @@ -218,8 +229,18 @@ public PathwayAnalysisEntity importAnalysis(PathwayAnalysisEntity toImport) {

@Override
public Page<PathwayAnalysisEntity> getPage(final Pageable pageable) {
List<PathwayAnalysisEntity> pathwayList = pathwayAnalysisRepository.findAll(defaultEntityGraph)
.stream().filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true)
.collect(Collectors.toList());
return getPageFromResults(pageable, pathwayList);
}

private Page<PathwayAnalysisEntity> getPageFromResults(Pageable pageable, List<PathwayAnalysisEntity> results) {
// Calculate the start and end indices for the current page
int startIndex = pageable.getPageNumber() * pageable.getPageSize();
int endIndex = Math.min(startIndex + pageable.getPageSize(), results.size());

return pathwayAnalysisRepository.findAll(pageable, defaultEntityGraph);
return new PageImpl<>(results.subList(startIndex, endIndex), pageable, results.size());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.convert.support.GenericConversionService;
import org.springframework.stereotype.Controller;

Expand Down Expand Up @@ -67,6 +68,9 @@ public class PredictionController {

private PermissionService permissionService;

@Value("#{'${security.defaultGlobalReadPermissions}'.equals(false)}")
private boolean defaultGlobalReadPermissions;

@Autowired
public PredictionController(PredictionService service,
GenericConversionService conversionService,
Expand Down Expand Up @@ -94,24 +98,26 @@ public PredictionController(PredictionService service,
@Path("/")
@Produces(MediaType.APPLICATION_JSON)
public List<CommonAnalysisDTO> getAnalysisList() {

return StreamSupport
.stream(service.getAnalysisList().spliterator(), false)
.filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true)
.map(analysis -> {
CommonAnalysisDTO dto = conversionService.convert(analysis, CommonAnalysisDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
}

/**
* Check to see if a prediction design exists by name
*
*
* @summary Prediction design exists by name
* @param id The prediction design id
* @param name The prediction design name
* @return 1 if a prediction design with the given name and id exist in WebAPI and 0 otherwise
* @return 1 if a prediction design with the given name and id exist in WebAPI
* and 0 otherwise
*/
@GET
@Path("/{id}/exists")
Expand Down

0 comments on commit 5393ed2

Please sign in to comment.