Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter cohorts and concepts by permission #2245

Merged
merged 24 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
610af40
helpful build steps
rkboyce Feb 13, 2023
20e985d
Merge branch 'OHDSI:master' into filter_cohorts_and_concepts
rkboyce Feb 23, 2023
b792094
This is the first working filtration of the conceptset lists so that …
rkboyce Mar 5, 2023
db76dfe
Merge branch 'filter_cohorts_and_concepts' of https://github.com/vinc…
rkboyce Mar 5, 2023
feab759
Merge branch 'OHDSI:master' into filter_cohorts_and_concepts
rkboyce Mar 7, 2023
1762c70
fixed issue where the creator of an artifact was skipped as having RE…
rkboyce Mar 24, 2023
61f30fb
Merge branch 'filter_cohorts_and_concepts' of https://github.com/vinc…
rkboyce Mar 24, 2023
e41524a
Merge branch 'OHDSI:master' into filter_cohorts_and_concepts
rkboyce Apr 4, 2023
f001a2d
Merge pull request #1 from OHDSI/master
rkboyce Jun 19, 2023
bcaa822
Merge branch 'master' into filter_cohorts_and_concepts
rkboyce Jun 19, 2023
b8f5220
changed the name, data type, and location for the configuration optio…
rkboyce Jun 20, 2023
c9b63ea
changed the name, data type, and location for the configuration optio…
rkboyce Jun 20, 2023
477c46c
added draft READ permissions to the schemas and made partial progress…
rkboyce Jun 25, 2023
317cf3e
partial progress through the list of schema files that need edited to…
rkboyce Jun 25, 2023
e26efba
what appears to be the appropriate READ permission schema for cohortd…
rkboyce Jul 9, 2023
83befff
the appropriate READ permission schema for cohortdefinitions. Tests OK
rkboyce Jul 10, 2023
3d80d5c
added necessary code to filter cohort-characterizations based on read…
rkboyce Jul 15, 2023
0d0690f
added filtering of cohort pathway analyses based on READ permissions
rkboyce Jul 16, 2023
247effd
added filtering of IR analyses returned from Atlas based on READ perm…
rkboyce Jul 16, 2023
efa3557
Implemented filtering of prediction entities depending on a users REA…
rkboyce Jul 16, 2023
28d6480
slight change to add permissions that Atlas checks for for PLP and Es…
rkboyce Jul 23, 2023
23a86bc
changed parameter for listAccessesForEntity from the overloaded 'role…
rkboyce Jul 23, 2023
8c7a54f
changed parameter for listAccessesForEntity from the overloaded 'role…
rkboyce Jul 23, 2023
04ac2c8
Added the Makefile to the .gitignore
rkboyce Jul 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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/
/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>false</security.defaultGlobalReadPermissions>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should default this to true, and rely on settings.xml to override this setting. This will maintain current behavior between versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected now and tested that setting the variable false in the WebAPIConfig/settings.xml works


<!-- 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 All @@ -86,6 +89,9 @@ public class CcController {
private CharacterizationChecker checker;
private PermissionService permissionService;

@Value("#{'${security.defaultglobalreadpermissions}'.equals(false)}")
rkboyce marked this conversation as resolved.
Show resolved Hide resolved
private boolean defaultglobalreadpermissions;

public CcController(
final CcService service,
final FeAnalysisService feAnalysisService,
Expand Down Expand Up @@ -151,11 +157,28 @@ 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;
});
if (defaultglobalreadpermissions == true) { // don't filter based on read permissions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a place wehre we want to have proper stayle: defaultGlobalReadPermissions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

return service.getPage(pageable).map(entity -> {
CcShortDTO dto = convertCcToShortDto(entity);
permissionService.fillWriteAccess(entity, dto);
permissionService.fillReadAccess(entity, dto);
return dto;
});
} else { // filter out what the user does not have read access to
List<CcShortDTO> dtolist = new ArrayList<CcShortDTO>();

Page<CohortCharacterizationEntity> newpage = service.getPage(pageable);

for (CohortCharacterizationEntity entity : newpage) {
if(permissionService.hasReadAccess(entity)){
CcShortDTO dto = convertCcToShortDto(entity);
permissionService.fillWriteAccess(entity, dto);
permissionService.fillReadAccess(entity, dto);
dtolist.add(dto);
}
}
return new PageImpl<CcShortDTO>(dtolist, pageable, dtolist.size());
}
}

/**
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;
rkboyce marked this conversation as resolved.
Show resolved Hide resolved

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

return StreamSupport.stream(service.getAnalysisList().spliterator(), false)
.map(analysis -> {
EstimationShortDTO dto = conversionService.convert(analysis, EstimationShortDTO.class);
permissionService.fillWriteAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
if (defaultglobalreadpermissions == true) { // don't filter based on read permissions
return StreamSupport.stream(service.getAnalysisList().spliterator(), false)
.map(analysis -> {
EstimationShortDTO dto = conversionService.convert(analysis, EstimationShortDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
} else {
return StreamSupport.stream(service.getAnalysisList().spliterator(), false)
.filter(candidateEstimation -> permissionService.hasReadAccess(candidateEstimation))
.map(analysis -> {
EstimationShortDTO dto = conversionService.convert(analysis, EstimationShortDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was mentioned before but you can put the filter conditional in the stream expression:

      return StreamSupport.stream(service.getAnalysisList().spliterator(), false)
	.filter(!defaultglobalreadpermissions  ? candidateEstimation -> permissionService.hasReadAccess(candidateEstimation) : candidateEstimation -> 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
34 changes: 28 additions & 6 deletions src/main/java/org/ohdsi/webapi/pathway/PathwayController.java
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 All @@ -53,6 +56,9 @@ public class PathwayController {
private PathwayChecker checker;
private PermissionService permissionService;

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

@Autowired
public PathwayController(ConversionService conversionService, ConverterUtils converterUtils, PathwayService pathwayService, SourceService sourceService, CommonGenerationSensitiveInfoService sensitiveInfoService, PathwayChecker checker, PermissionService permissionService, I18nService i18nService) {

Expand Down Expand Up @@ -156,13 +162,29 @@ 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);
return dto;
});
if (defaultglobalreadpermissions == true) { // don't filter based on read permissions
return pathwayService.getPage(pageable).map(pa -> {
PathwayAnalysisDTO dto = conversionService.convert(pa, PathwayAnalysisDTO.class);
permissionService.fillWriteAccess(pa, dto);
permissionService.fillReadAccess(pa, dto);
return dto;
});
} else { // filter out entities that the user does not have read permissions to view
List<PathwayAnalysisDTO> dtolist = new ArrayList<PathwayAnalysisDTO>();

Page<PathwayAnalysisEntity> newpage = pathwayService.getPage(pageable);
for (PathwayAnalysisEntity pa : newpage) {
if (permissionService.hasReadAccess(pa)) {
PathwayAnalysisDTO dto = conversionService.convert(pa, PathwayAnalysisDTO.class);
permissionService.fillWriteAccess(pa, dto);
permissionService.fillReadAccess(pa, dto);
dtolist.add(dto);
}
}
return new PageImpl<PathwayAnalysisDTO>(dtolist, pageable, dtolist.size());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this can be written same as in Estimation:

	return pathwayService.getPage(pageable)
		.filter(!defaultglobalreadpermissions  ? pa -> permissionService.hasReadAccess(pa) : pa -> true)
		.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
58 changes: 38 additions & 20 deletions src/main/java/org/ohdsi/webapi/prediction/PredictionController.java
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 All @@ -93,26 +97,40 @@ public PredictionController(PredictionService service,
@GET
@Path("/")
@Produces(MediaType.APPLICATION_JSON)
public List<CommonAnalysisDTO> getAnalysisList() {

return StreamSupport
.stream(service.getAnalysisList().spliterator(), false)
.map(analysis -> {
CommonAnalysisDTO dto = conversionService.convert(analysis, CommonAnalysisDTO.class);
permissionService.fillWriteAccess(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
*/
public List<CommonAnalysisDTO> getAnalysisList() {
if (defaultglobalreadpermissions == true) { // don't filter based on read permissions
return StreamSupport
.stream(service.getAnalysisList().spliterator(), false)
.map(analysis -> {
CommonAnalysisDTO dto = conversionService.convert(analysis, CommonAnalysisDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
} else {
return StreamSupport
.stream(service.getAnalysisList().spliterator(), false)
.filter(candidateAnalysis -> permissionService.hasReadAccess(candidateAnalysis))
.map(analysis -> {
CommonAnalysisDTO dto = conversionService.convert(analysis, CommonAnalysisDTO.class);
permissionService.fillWriteAccess(analysis, dto);
permissionService.fillReadAccess(analysis, dto);
return dto;
})
.collect(Collectors.toList());
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above: use conditional filter to reduce redundancy.



/**
* 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
*/
Comment on lines +126 to +133
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indent of comments, should be aligned to the following function.

@GET
@Path("/{id}/exists")
@Produces(MediaType.APPLICATION_JSON)
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/org/ohdsi/webapi/security/PermissionController.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,31 @@ public List<RoleDTO> listAccessesForEntity(@QueryParam("roleSearch") String role
}

/**
* Get entity role access information
* Get roles that have a permission type (READ/WRITE) to entity
*
* @summary Get entity role information
* @summary Get roles that have a specific permission (READ/WRITE) for the entity
* @param entityType The entity type
* @param entityId The entity ID
* @return The list of roles
* @return The list of permissions for the permission type
* @throws Exception
*/
@GET
@Path("/access/{entityType}/{entityId}")
@Path("/access/{entityType}/{entityId}/{permType}")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public List<RoleDTO> listAccessesForEntity(
public List<RoleDTO> listAccessesForEntityByPermType(
@PathParam("entityType") EntityType entityType,
@PathParam("entityId") Integer entityId
@PathParam("entityId") Integer entityId,
@PathParam("permType") String permType
) throws Exception {

permissionService.checkCommonEntityOwnership(entityType, entityId);

Set<String> permissionTemplates = permissionService.getTemplatesForType(entityType, AccessType.WRITE).keySet();
Set<String> permissionTemplates = null;
if (permType == "WRITE") {
permissionTemplates = permissionService.getTemplatesForType(entityType, AccessType.WRITE).keySet();
} else {
permissionTemplates = permissionService.getTemplatesForType(entityType, AccessType.READ).keySet();
}
Comment on lines -95 to +110
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backwards compatibility, you should maintain the endpoint /access/entityType/entityId, tho this will default to the perm type of 'WRITE'. So, revert this change to leave listAccessForEntity, but have this function call over to 'listAccessForEntityByPermType) with the perType paramater as AccessType.WRITE. In addition, i believe permType should be the enum, not the string, but will need to do some tests if the Java REST api will automatically serialize the ENUM input. The issue is that you could pass a perm type of 'BlahBlah' and it will assume that is 'READ' via the else statement.


List<String> permissions = permissionTemplates
.stream()
Expand Down