From 25b34aee782edcd06ebdf86b6644960f027121fc Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Fri, 7 Jun 2024 16:01:06 +0200 Subject: [PATCH 01/28] Implement endpoints for public virtual studies --- .../web/PublicVirtualStudiesController.java | 169 ++++++++++++++++++ .../web/SessionServiceController.java | 7 +- 2 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java new file mode 100644 index 00000000000..c4d42fff492 --- /dev/null +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -0,0 +1,169 @@ +package org.cbioportal.web; + +import com.mongodb.BasicDBObject; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import org.cbioportal.service.util.SessionServiceRequestHandler; +import org.cbioportal.web.parameter.VirtualStudy; +import org.cbioportal.web.parameter.VirtualStudyData; +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.ParameterizedTypeReference; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestHeader; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.client.RestTemplate; + +import java.util.Collections; +import java.util.List; +import java.util.Set; + +@Controller +@RequestMapping("/api/public_virtual_studies") +public class PublicVirtualStudiesController { + + private static final Logger LOG = LoggerFactory.getLogger(PublicVirtualStudiesController.class); + + @Value("${session.endpoint.publisher-api-key:}") + private String requiredPublisherApiKey; + + public static final String ALL_USERS = "*"; + @Autowired + SessionServiceRequestHandler sessionServiceRequestHandler; + + @Value("${session.service.url:}") + private String sessionServiceURL; + + @GetMapping + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) + public ResponseEntity> getPublicVirtualStudies() { + //TODO move this logic to sessionServiceRequestHandler? + BasicDBObject basicDBObject = new BasicDBObject(); + basicDBObject.put("data.users", ALL_USERS); + ResponseEntity> responseEntity = new RestTemplate().exchange( + sessionServiceURL + "/virtual_study/query/fetch", + HttpMethod.POST, + new HttpEntity<>(basicDBObject.toString(), sessionServiceRequestHandler.getHttpHeaders()), + new ParameterizedTypeReference<>() { + }); + + return new ResponseEntity<>(responseEntity.getBody(), HttpStatus.OK); + } + + @PostMapping + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) + public ResponseEntity publishVirtualStudyData( + @RequestBody VirtualStudyData virtualStudyData, + @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey + ) { + if (requiredPublisherApiKey == null + || requiredPublisherApiKey.isBlank() + || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { + return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + } + VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); + //TODO move this logic to sessionServiceRequestHandler? + ResponseEntity responseEntity = new RestTemplate().exchange( + sessionServiceURL + "/virtual_study", + HttpMethod.POST, + new HttpEntity<>(virtualStudyDataToPublish, sessionServiceRequestHandler.getHttpHeaders()), + new ParameterizedTypeReference<>() { + }); + + return new ResponseEntity<>(responseEntity.getBody(), HttpStatus.OK); + } + + @PostMapping("/{id}") + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) + public ResponseEntity publishVirtualStudy( + @PathVariable String id, + @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey + ) { + if (requiredPublisherApiKey == null + || requiredPublisherApiKey.isBlank() + || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { + return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + } + ResponseEntity responseEntity = getVirtualStudyById(id); + HttpStatusCode statusCode = responseEntity.getStatusCode(); + if (!statusCode.is2xxSuccessful()) { + LOG.error("The downstream server replied with statusCode={} and body={}." + + " Replying with the same status code to the client.", + statusCode, responseEntity.getBody()); + return new ResponseEntity<>(null, statusCode); + } + if (responseEntity.getBody() == null) { + LOG.error("The downstream server replied without body and statusCode={}." + + " Replying with internal server error status code to the client.", + statusCode); + return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); + } + return publishVirtualStudyData(responseEntity.getBody().getData(), providedPublisherApiKey); + } + + private ResponseEntity getVirtualStudyById(String id) { + return new RestTemplate() + .exchange(sessionServiceURL + "/virtual_study/" + id, + HttpMethod.GET, + new HttpEntity<>(sessionServiceRequestHandler.getHttpHeaders()), + VirtualStudy.class); + } + + //TODO Removing does not work. Is there caching? + @DeleteMapping("/{id}") + @ApiResponse(responseCode = "200", description = "OK") + public ResponseEntity retractVirtualStudy( + @PathVariable String id, + @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey + ) { + if (requiredPublisherApiKey == null + || requiredPublisherApiKey.isBlank() + || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { + return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + } + ResponseEntity responseEntity = getVirtualStudyById(id); + HttpStatusCode statusCode = responseEntity.getStatusCode(); + if (!statusCode.is2xxSuccessful()) { + LOG.error("The downstream server replied with statusCode={} and body={}." + + " Replying with the same status code to the client.", + statusCode, responseEntity.getBody()); + return new ResponseEntity<>(null, statusCode); + } + if (responseEntity.getBody() == null) { + LOG.error("The downstream server replied without body and statusCode={}." + + " Replying with internal server error status code to the client.", + statusCode); + return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); + } + VirtualStudy virtualStudy = responseEntity.getBody(); + VirtualStudyData data = virtualStudy.getData(); + data.setUsers(Collections.emptySet()); + new RestTemplate() + .put(sessionServiceURL + "/virtual_study/" + id, + new HttpEntity<>(data, sessionServiceRequestHandler.getHttpHeaders())); + return new ResponseEntity(HttpStatus.OK); + } + + private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { + VirtualStudyData virtualStudyDataToPublish = new VirtualStudyData(); + virtualStudyDataToPublish.setName(virtualStudyData.getName()); + virtualStudyDataToPublish.setDescription(virtualStudyData.getDescription()); + virtualStudyDataToPublish.setStudies(virtualStudyData.getStudies()); + virtualStudyDataToPublish.setStudyViewFilter(virtualStudyData.getStudyViewFilter()); + virtualStudyDataToPublish.setUsers(Set.of(ALL_USERS)); + return virtualStudyDataToPublish; + } +} diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 6a2395cd469..9576393b33e 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -132,11 +132,13 @@ private ResponseEntity addSession( if (type.equals(Session.SessionType.virtual_study) || type.equals(Session.SessionType.group)) { // JSON from file to Object VirtualStudyData virtualStudyData = sessionServiceObjectMapper.readValue(body.toString(), VirtualStudyData.class); + //TODO sanitize what's supplied. e.g. anonymous user should not specify the users field! if (isAuthorized()) { virtualStudyData.setOwner(userName()); if ((operation.isPresent() && operation.get().equals(SessionOperation.save)) || type.equals(Session.SessionType.group)) { + //TODO userName could not be ALL_USERS (*) here virtualStudyData.setUsers(Collections.singleton(userName())); } } @@ -260,7 +262,7 @@ public ResponseEntity> getUserStudies() throws JsonProcessing content = @Content(schema = @Schema(implementation = Session.class))) public ResponseEntity addSession(@PathVariable Session.SessionType type, @RequestBody JSONObject body) throws IOException { - + //FIXME? anonymous user can create sessions. Do we really want that? return addSession(type, Optional.empty(), body); } @@ -268,7 +270,7 @@ public ResponseEntity addSession(@PathVariable Session.SessionType type @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = Session.class))) public ResponseEntity addUserSavedVirtualStudy(@RequestBody JSONObject body) throws IOException { - + //FIXME? anonymous user can create virtual studies. Do we really want that? return addSession(Session.SessionType.virtual_study, Optional.of(SessionOperation.save), body); } @@ -300,6 +302,7 @@ public void updateUsersInSession(@PathVariable Session.SessionType type, @PathVa VirtualStudyData virtualStudyData = virtualStudy.getData(); Set users = virtualStudyData.getUsers(); updateUserList(operation, users); + //TODO userName could not be ALL_USERS (*) here virtualStudyData.setUsers(users); httpEntity = new HttpEntity<>(virtualStudyData, sessionServiceRequestHandler.getHttpHeaders()); } From 23bba70ef83e07384b75b06f33db6892be0fb1d8 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 12 Jun 2024 18:06:43 +0200 Subject: [PATCH 02/28] Add possibility to specify cancer type id and pmi for virt. study during publishing --- .../web/PublicVirtualStudiesController.java | 30 +++++++++++++++++-- .../web/parameter/VirtualStudyData.java | 18 +++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index c4d42fff492..b73d2403039 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -4,6 +4,8 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; +import org.cbioportal.service.CancerTypeService; +import org.cbioportal.service.exception.CancerTypeNotFoundException; import org.cbioportal.service.util.SessionServiceRequestHandler; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; @@ -25,6 +27,7 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.client.RestTemplate; import java.util.Collections; @@ -46,6 +49,9 @@ public class PublicVirtualStudiesController { @Value("${session.service.url:}") private String sessionServiceURL; + + @Autowired + private CancerTypeService cancerTypeService; @GetMapping @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) @@ -67,7 +73,9 @@ public ResponseEntity> getPublicVirtualStudies() { @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) public ResponseEntity publishVirtualStudyData( @RequestBody VirtualStudyData virtualStudyData, - @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey + @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey, + @RequestParam(required = false) String typeOfCancerId, + @RequestParam(required = false) String pmid ) { if (requiredPublisherApiKey == null || requiredPublisherApiKey.isBlank() @@ -75,6 +83,18 @@ public ResponseEntity publishVirtualStudyData( return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); } VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); + if (typeOfCancerId != null) { + try { + cancerTypeService.getCancerType(typeOfCancerId); + virtualStudyDataToPublish.setTypeOfCancerId(typeOfCancerId); + } catch (CancerTypeNotFoundException e) { + LOG.error("No cancer type with id={} were found.", typeOfCancerId); + return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST); + } + } + if (pmid != null) { + virtualStudyDataToPublish.setPmid(pmid); + } //TODO move this logic to sessionServiceRequestHandler? ResponseEntity responseEntity = new RestTemplate().exchange( sessionServiceURL + "/virtual_study", @@ -90,7 +110,9 @@ public ResponseEntity publishVirtualStudyData( @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) public ResponseEntity publishVirtualStudy( @PathVariable String id, - @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey + @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey, + @RequestParam(required = false) String typeOfCancerId, + @RequestParam(required = false) String pmid ) { if (requiredPublisherApiKey == null || requiredPublisherApiKey.isBlank() @@ -111,7 +133,7 @@ public ResponseEntity publishVirtualStudy( statusCode); return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); } - return publishVirtualStudyData(responseEntity.getBody().getData(), providedPublisherApiKey); + return publishVirtualStudyData(responseEntity.getBody().getData(), providedPublisherApiKey, typeOfCancerId, pmid); } private ResponseEntity getVirtualStudyById(String id) { @@ -163,6 +185,8 @@ private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData virtualStudyDataToPublish.setDescription(virtualStudyData.getDescription()); virtualStudyDataToPublish.setStudies(virtualStudyData.getStudies()); virtualStudyDataToPublish.setStudyViewFilter(virtualStudyData.getStudyViewFilter()); + virtualStudyDataToPublish.setTypeOfCancerId(virtualStudyData.getTypeOfCancerId()); + virtualStudyDataToPublish.setPmid(virtualStudyData.getPmid()); virtualStudyDataToPublish.setUsers(Set.of(ALL_USERS)); return virtualStudyDataToPublish; } diff --git a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java index 98cbfa9bd2d..54a5a88a1d6 100644 --- a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java +++ b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java @@ -21,6 +21,9 @@ public class VirtualStudyData implements Serializable { private Long lastUpdated = System.currentTimeMillis(); private Set users = new HashSet<>(); + private String typeOfCancerId; + private String pmid; + public String getOwner() { return owner; } @@ -104,4 +107,19 @@ public void setStudyViewFilter(StudyViewFilter studyViewFilter) { this.studyViewFilter = studyViewFilter; } + public String getTypeOfCancerId() { + return typeOfCancerId; + } + + public void setTypeOfCancerId(String typeOfCancerId) { + this.typeOfCancerId = typeOfCancerId; + } + + public String getPmid() { + return pmid; + } + + public void setPmid(String pmid) { + this.pmid = pmid; + } } From d3c2f3caede6a4848aefc442de9b7fb683470c4f Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 13 Jun 2024 16:21:55 +0200 Subject: [PATCH 03/28] Filter out forbidden study ids from virtual studies --- .../VirtualStudyPermissionService.java | 52 +++++++++++++++++++ .../security/config/MethodSecurityConfig.java | 32 +++++------- .../web/PublicVirtualStudiesController.java | 16 +++++- .../web/SessionServiceController.java | 17 +++++- 4 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java diff --git a/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java b/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java new file mode 100644 index 00000000000..c8860af4e75 --- /dev/null +++ b/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java @@ -0,0 +1,52 @@ +package org.cbioportal.security; + +import org.cbioportal.utils.security.AccessLevel; +import org.cbioportal.web.parameter.StudyViewFilter; +import org.cbioportal.web.parameter.VirtualStudy; +import org.cbioportal.web.parameter.VirtualStudyData; +import org.cbioportal.web.parameter.VirtualStudySamples; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Component; + +import java.util.Iterator; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +@Component +public class VirtualStudyPermissionService { + @Autowired(required = false) + private CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator; + + public void filterOutForbiddenStudies(List virtualStudies) { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null || cancerStudyPermissionEvaluator == null) { + return; + } + Iterator virtualStudyIterator = virtualStudies.iterator(); + while (virtualStudyIterator.hasNext()) { + VirtualStudy virtualStudy = virtualStudyIterator.next(); + VirtualStudyData virtualStudyData = virtualStudy.getData(); + + Set filteredStudies = virtualStudyData.getStudies().stream() + .filter(study -> + cancerStudyPermissionEvaluator.hasPermission(authentication, study.getId(), "CancerStudyId", AccessLevel.READ)) + .collect(Collectors.toSet()); + if (filteredStudies.isEmpty()) { + virtualStudyIterator.remove(); + continue; + } + virtualStudyData.setStudies(filteredStudies); + + StudyViewFilter studyViewFilter = virtualStudyData.getStudyViewFilter(); + List filteredStudyIds = studyViewFilter.getStudyIds().stream() + .filter(studyId -> + cancerStudyPermissionEvaluator.hasPermission(authentication, studyId, "CancerStudyId", AccessLevel.READ)) + .toList(); + virtualStudyData.getStudyViewFilter().setStudyIds(filteredStudyIds); + } + + } +} diff --git a/src/main/java/org/cbioportal/security/config/MethodSecurityConfig.java b/src/main/java/org/cbioportal/security/config/MethodSecurityConfig.java index f1171fd48db..c6bd8d04645 100644 --- a/src/main/java/org/cbioportal/security/config/MethodSecurityConfig.java +++ b/src/main/java/org/cbioportal/security/config/MethodSecurityConfig.java @@ -2,12 +2,10 @@ import org.cbioportal.persistence.cachemaputil.CacheMapUtil; import org.cbioportal.security.CancerStudyPermissionEvaluator; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; @@ -17,28 +15,22 @@ // We are allowing users to enable method_authorization if optional_oauth2 is selected @ConditionalOnExpression("{'oauth2','saml', 'saml_plus_basic'}.contains('${authenticate}') or ('optional_oauth2' eq '${authenticate}' and 'true' eq '${security.method_authorization_enabled}')") public class MethodSecurityConfig { - @Value("${app.name:}") - private String appName; - - @Value("${filter_groups_by_appname:true}") - private String doFilterGroupsByAppName; - - @Value("${always_show_study_group:}") - private String alwaysShowCancerStudyGroup; - - @Autowired - private CacheMapUtil cacheMapUtil; @Bean - public MethodSecurityExpressionHandler createExpressionHandler() { + public CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator( + @Value("${app.name:}") String appName, + @Value("${filter_groups_by_appname:true}") String doFilterGroupsByAppName, + @Value("${always_show_study_group:}") String alwaysShowCancerStudyGroup, + CacheMapUtil cacheMapUtil + ) { + return new CancerStudyPermissionEvaluator(appName, doFilterGroupsByAppName, alwaysShowCancerStudyGroup, cacheMapUtil); + } + + @Bean + public MethodSecurityExpressionHandler createExpressionHandler(CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator) { DefaultMethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler(); - expressionHandler.setPermissionEvaluator(cancerStudyPermissionEvaluator()); + expressionHandler.setPermissionEvaluator(cancerStudyPermissionEvaluator); return expressionHandler; } - - @Bean - public CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator() { - return new CancerStudyPermissionEvaluator(appName, doFilterGroupsByAppName, alwaysShowCancerStudyGroup, cacheMapUtil); - } } \ No newline at end of file diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index b73d2403039..e6c26515c42 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -4,11 +4,16 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; +import org.cbioportal.security.CancerStudyPermissionEvaluator; +import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.CancerTypeService; import org.cbioportal.service.exception.CancerTypeNotFoundException; import org.cbioportal.service.util.SessionServiceRequestHandler; +import org.cbioportal.utils.security.AccessLevel; +import org.cbioportal.web.parameter.StudyViewFilter; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; +import org.cbioportal.web.parameter.VirtualStudySamples; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -19,6 +24,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -30,9 +36,12 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.client.RestTemplate; +import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; @Controller @RequestMapping("/api/public_virtual_studies") @@ -53,6 +62,9 @@ public class PublicVirtualStudiesController { @Autowired private CancerTypeService cancerTypeService; + @Autowired + private VirtualStudyPermissionService virtualStudyPermissionService; + @GetMapping @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) public ResponseEntity> getPublicVirtualStudies() { @@ -66,7 +78,9 @@ public ResponseEntity> getPublicVirtualStudies() { new ParameterizedTypeReference<>() { }); - return new ResponseEntity<>(responseEntity.getBody(), HttpStatus.OK); + List virtualStudies = responseEntity.getBody(); + virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudies); + return new ResponseEntity<>(virtualStudies, HttpStatus.OK); } @PostMapping diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 9576393b33e..f4ab63b2b6b 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -12,6 +12,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import jakarta.servlet.http.HttpServletResponse; import jakarta.validation.constraints.Size; +import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.util.CustomAttributeWithData; import org.cbioportal.service.util.CustomDataSession; import org.cbioportal.service.util.SessionServiceRequestHandler; @@ -77,6 +78,9 @@ public class SessionServiceController { @Value("${session.service.url:}") private String sessionServiceURL; + @Autowired + private VirtualStudyPermissionService virtualStudyPermissionService; + private static Map> pageToSettingsDataClass = ImmutableMap.of( SessionPage.study_view, StudyPageSettings.class, SessionPage.results_view, ResultsPageSettings.class @@ -206,7 +210,14 @@ public ResponseEntity getSession(@PathVariable Session.SessionType type Session session; switch (type) { case virtual_study: - session = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); + VirtualStudy virtualStudy = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); + List virtualStudies = new ArrayList<>(); + virtualStudies.add(virtualStudy); + virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudies); + if (virtualStudies.isEmpty()) { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); + } + session = virtualStudies.getFirst(); break; case settings: session = sessionServiceObjectMapper.readValue(sessionDataJson, PageSettings.class); @@ -248,7 +259,9 @@ public ResponseEntity> getUserStudies() throws JsonProcessing httpEntity, new ParameterizedTypeReference>() {}); - return new ResponseEntity<>(responseEntity.getBody(), HttpStatus.OK); + List virtualStudyList = responseEntity.getBody(); + virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudyList); + return new ResponseEntity<>(virtualStudyList, HttpStatus.OK); } catch (Exception exception) { LOG.error("Error occurred", exception); return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); From f102f61f7fdf83676ddcd788ff8288a52120dbc8 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 13 Jun 2024 16:29:00 +0200 Subject: [PATCH 04/28] Do not allow set * user for user virtual study To prevent missuse of the request to publish virtual study for everyone! --- .../org/cbioportal/web/SessionServiceController.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index f4ab63b2b6b..245566fcfaa 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -59,6 +59,8 @@ import java.util.Set; import java.util.regex.Pattern; +import static org.cbioportal.web.PublicVirtualStudiesController.ALL_USERS; + @Controller @RequestMapping("/api/session") public class SessionServiceController { @@ -139,11 +141,14 @@ private ResponseEntity addSession( //TODO sanitize what's supplied. e.g. anonymous user should not specify the users field! if (isAuthorized()) { - virtualStudyData.setOwner(userName()); + String userName = userName(); + if (userName.equals(ALL_USERS)) { + throw new IllegalStateException("Illegal username " + ALL_USERS + " for assigning virtual studies."); + } + virtualStudyData.setOwner(userName); if ((operation.isPresent() && operation.get().equals(SessionOperation.save)) || type.equals(Session.SessionType.group)) { - //TODO userName could not be ALL_USERS (*) here - virtualStudyData.setUsers(Collections.singleton(userName())); + virtualStudyData.setUsers(Collections.singleton(userName)); } } From fc78963cc4f9cc8b8bebc3712f641aaa5dc4c2ff Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 19 Jun 2024 17:46:20 +0200 Subject: [PATCH 05/28] Add integration tests for (un)publishing virtual study --- .../PublicVirtualStudiesIntegrationTest.java | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java diff --git a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java new file mode 100644 index 00000000000..06b8923ead6 --- /dev/null +++ b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java @@ -0,0 +1,207 @@ +package org.cbioportal.test.integration; + +import org.cbioportal.test.integration.security.ContainerConfig; +import org.cbioportal.utils.removeme.Session; +import org.cbioportal.web.parameter.StudyViewFilter; +import org.cbioportal.web.parameter.VirtualStudy; +import org.cbioportal.web.parameter.VirtualStudyData; +import org.cbioportal.web.parameter.VirtualStudySamples; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import java.util.List; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.cbioportal.test.integration.security.ContainerConfig.MyMysqlInitializer; +import static org.cbioportal.test.integration.security.ContainerConfig.PortInitializer; +import static org.cbioportal.test.integration.security.ContainerConfig.SESSION_SERVICE_PORT; + +@RunWith(SpringRunner.class) +@SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT +) +@TestPropertySource( + properties = { + "authenticate=false", + "session.endpoint.publisher-api-key=this-is-a-secret", + "session.service.url=http://localhost:" + SESSION_SERVICE_PORT + "/api/sessions/public_portal/", + // DB settings (also see MysqlInitializer) + "spring.datasource.driverClassName=com.mysql.jdbc.Driver", + "spring.jpa.database-platform=org.hibernate.dialect.MySQL5Dialect", + } +) +@ContextConfiguration(initializers = { + MyMysqlInitializer.class, + PortInitializer.class +}) +@DirtiesContext +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +public class PublicVirtualStudiesIntegrationTest extends ContainerConfig { + + final static String CBIO_URL = String.format("http://localhost:%d", CBIO_PORT); + + final static HttpHeaders jsonContentType = new HttpHeaders() { + { + set("Content-Type", "application/json"); + } + }; + + final static HttpHeaders invalidKeyContainingHeaders = new HttpHeaders() { + { + set("X-PUBLISHER-API-KEY", "this-is-not-valid-key"); + } + }; + + final static HttpHeaders validKeyContainingHeaders = new HttpHeaders() { + { + set("X-PUBLISHER-API-KEY", "this-is-a-secret"); + } + }; + + final static ParameterizedTypeReference> typeRef = new ParameterizedTypeReference<>() { + }; + + static String createdVsId; + static String publishedVsId; + + @Autowired + private TestRestTemplate restTemplate; + + @Test + public void test1NoPublicVirtualStudiesAtTheBeginning() { + ResponseEntity> response1 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies", + HttpMethod.GET, + null, + typeRef); + + assertThat(response1.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response1.getBody()).hasSize(0); + } + + @Test + public void test2CreateVirtualStudy() { + VirtualStudyData vsDataToSave = createTestVsData(); + + ResponseEntity response2 = restTemplate.exchange( + CBIO_URL + "/api/session/virtual_study", + HttpMethod.POST, + new HttpEntity<>(vsDataToSave, jsonContentType), + VirtualStudy.class); + assertThat(response2.getStatusCode().is2xxSuccessful()).isTrue(); + VirtualStudy savedVs = response2.getBody(); + assertThat(savedVs).isNotNull().hasFieldOrProperty("id").isNotNull(); + createdVsId = savedVs.getId(); + } + + @Test + public void test3PublishVirtualStudy() { + ResponseEntity response3 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + HttpMethod.POST, + null, + VirtualStudy.class); + assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + + response3 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + HttpMethod.POST, + new HttpEntity<>(null, invalidKeyContainingHeaders), + VirtualStudy.class); + assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + + response3 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + HttpMethod.POST, + new HttpEntity<>(null, validKeyContainingHeaders), + VirtualStudy.class); + assertThat(response3.getStatusCode().is2xxSuccessful()).isTrue(); + VirtualStudy publishedVs = response3.getBody(); + assertThat(publishedVs).isNotNull().hasFieldOrProperty("id").isNotNull(); + publishedVsId = publishedVs.getId(); + } + + @Test + public void test4ListJustPublishedStudy() { + ResponseEntity> response4 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies", + HttpMethod.GET, + null, + typeRef); + + assertThat(response4.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response4.getBody()).hasSize(1); + } + + @Test + public void test5UnpublishVirtualStudy() { + ResponseEntity response5 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + HttpMethod.DELETE, + null, + Object.class); + assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + + response5 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + HttpMethod.DELETE, + new HttpEntity<>(null, invalidKeyContainingHeaders), + Object.class); + assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + + response5 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + HttpMethod.DELETE, + new HttpEntity<>(null, validKeyContainingHeaders), + Object.class); + assertThat(response5.getStatusCode().is2xxSuccessful()).isTrue(); + } + + @Test + public void test6NoPublicVirtualStudiesAfterRemoval() { + ResponseEntity> response6 = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies", + HttpMethod.GET, + null, + typeRef); + + assertThat(response6.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response6.getBody()).hasSize(0); + } + + private VirtualStudyData createTestVsData() { + VirtualStudyData data = new VirtualStudyData(); + VirtualStudySamples study1 = new VirtualStudySamples(); + study1.setId("study_tcga_pub"); + study1.setSamples(Set.of("TCGA-A1-A0SB-01", "TCGA-A1-A0SJ-01")); + VirtualStudySamples study2 = new VirtualStudySamples(); + study2.setId("acc_tcga"); + study2.setSamples(Set.of("TCGA-XX-0800-01")); + Set studies = Set.of( + study1, + study2 + ); + data.setStudies(studies); + StudyViewFilter studyViewFilter = new StudyViewFilter(); + studyViewFilter.setStudyIds(List.of("study_tcga_pub", "acc_tcga")); + data.setStudyViewFilter(studyViewFilter); + return data; + } + +} \ No newline at end of file From 7cc1c166cee8da93fdddd582a35347864381d0a1 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 19 Jun 2024 20:25:14 +0200 Subject: [PATCH 06/28] Assert fields fo published virtual studies --- .../PublicVirtualStudiesIntegrationTest.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java index 06b8923ead6..982fa29c964 100644 --- a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java +++ b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java @@ -1,7 +1,6 @@ package org.cbioportal.test.integration; import org.cbioportal.test.integration.security.ContainerConfig; -import org.cbioportal.utils.removeme.Session; import org.cbioportal.web.parameter.StudyViewFilter; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; @@ -80,6 +79,8 @@ public class PublicVirtualStudiesIntegrationTest extends ContainerConfig { static String createdVsId; static String publishedVsId; + final static VirtualStudyData virtualStudyDataToSave = createTestVsData(); + @Autowired private TestRestTemplate restTemplate; @@ -97,12 +98,11 @@ public void test1NoPublicVirtualStudiesAtTheBeginning() { @Test public void test2CreateVirtualStudy() { - VirtualStudyData vsDataToSave = createTestVsData(); ResponseEntity response2 = restTemplate.exchange( CBIO_URL + "/api/session/virtual_study", HttpMethod.POST, - new HttpEntity<>(vsDataToSave, jsonContentType), + new HttpEntity<>(virtualStudyDataToSave, jsonContentType), VirtualStudy.class); assertThat(response2.getStatusCode().is2xxSuccessful()).isTrue(); VirtualStudy savedVs = response2.getBody(); @@ -112,28 +112,29 @@ public void test2CreateVirtualStudy() { @Test public void test3PublishVirtualStudy() { + String url = CBIO_URL + "/api/public_virtual_studies/" + createdVsId + "?typeOfCancerId=acc&pmid=12345"; ResponseEntity response3 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + url, HttpMethod.POST, null, VirtualStudy.class); assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); response3 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + url, HttpMethod.POST, new HttpEntity<>(null, invalidKeyContainingHeaders), VirtualStudy.class); assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); response3 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + createdVsId, + url, HttpMethod.POST, new HttpEntity<>(null, validKeyContainingHeaders), VirtualStudy.class); assertThat(response3.getStatusCode().is2xxSuccessful()).isTrue(); VirtualStudy publishedVs = response3.getBody(); - assertThat(publishedVs).isNotNull().hasFieldOrProperty("id").isNotNull(); + assertThat(publishedVs).isNotNull(); publishedVsId = publishedVs.getId(); } @@ -146,7 +147,17 @@ public void test4ListJustPublishedStudy() { typeRef); assertThat(response4.getStatusCode().is2xxSuccessful()).isTrue(); - assertThat(response4.getBody()).hasSize(1); + List virtualStudies = response4.getBody(); + assertThat(virtualStudies).isNotNull().hasSize(1); + VirtualStudy virtualStudy = virtualStudies.get(0); + VirtualStudyData virtualStudyData = virtualStudy.getData(); + assertThat(virtualStudyData) + .hasFieldOrPropertyWithValue("name", virtualStudyDataToSave.getName()) + .hasFieldOrPropertyWithValue("description", virtualStudyDataToSave.getDescription()) + .hasFieldOrPropertyWithValue("typeOfCancerId", "acc") + .hasFieldOrPropertyWithValue("pmid", "12345"); + assertThat(virtualStudyData.getStudies()).hasSize(2); + assertThat(virtualStudyData.getStudyViewFilter()).isNotNull(); } @Test @@ -185,8 +196,10 @@ public void test6NoPublicVirtualStudiesAfterRemoval() { assertThat(response6.getBody()).hasSize(0); } - private VirtualStudyData createTestVsData() { + static VirtualStudyData createTestVsData() { VirtualStudyData data = new VirtualStudyData(); + data.setName("test virtual study name"); + data.setDescription("test virtual study description"); VirtualStudySamples study1 = new VirtualStudySamples(); study1.setId("study_tcga_pub"); study1.setSamples(Set.of("TCGA-A1-A0SB-01", "TCGA-A1-A0SJ-01")); From f231dd930dabb2e8e27c286c425e36cb39aa6b45 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 20 Jun 2024 16:46:59 +0200 Subject: [PATCH 07/28] Use recommended ways to inject dependencies in spring --- .../VirtualStudyPermissionService.java | 20 +++++--- .../web/PublicVirtualStudiesController.java | 51 ++++++++++--------- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java b/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java index c8860af4e75..40183545102 100644 --- a/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java +++ b/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java @@ -5,24 +5,28 @@ import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; import org.cbioportal.web.parameter.VirtualStudySamples; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.stereotype.Component; +import org.springframework.stereotype.Service; import java.util.Iterator; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -@Component +@Service public class VirtualStudyPermissionService { - @Autowired(required = false) - private CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator; + + private final Optional cancerStudyPermissionEvaluator; + + public VirtualStudyPermissionService(Optional cancerStudyPermissionEvaluator) { + this.cancerStudyPermissionEvaluator = cancerStudyPermissionEvaluator; + } public void filterOutForbiddenStudies(List virtualStudies) { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null || cancerStudyPermissionEvaluator == null) { + if (authentication == null || cancerStudyPermissionEvaluator.isEmpty()) { return; } Iterator virtualStudyIterator = virtualStudies.iterator(); @@ -32,7 +36,7 @@ public void filterOutForbiddenStudies(List virtualStudies) { Set filteredStudies = virtualStudyData.getStudies().stream() .filter(study -> - cancerStudyPermissionEvaluator.hasPermission(authentication, study.getId(), "CancerStudyId", AccessLevel.READ)) + cancerStudyPermissionEvaluator.get().hasPermission(authentication, study.getId(), "CancerStudyId", AccessLevel.READ)) .collect(Collectors.toSet()); if (filteredStudies.isEmpty()) { virtualStudyIterator.remove(); @@ -43,7 +47,7 @@ public void filterOutForbiddenStudies(List virtualStudies) { StudyViewFilter studyViewFilter = virtualStudyData.getStudyViewFilter(); List filteredStudyIds = studyViewFilter.getStudyIds().stream() .filter(studyId -> - cancerStudyPermissionEvaluator.hasPermission(authentication, studyId, "CancerStudyId", AccessLevel.READ)) + cancerStudyPermissionEvaluator.get().hasPermission(authentication, studyId, "CancerStudyId", AccessLevel.READ)) .toList(); virtualStudyData.getStudyViewFilter().setStudyIds(filteredStudyIds); } diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index e6c26515c42..04caf520aee 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -4,19 +4,14 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; -import org.cbioportal.security.CancerStudyPermissionEvaluator; import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.CancerTypeService; import org.cbioportal.service.exception.CancerTypeNotFoundException; import org.cbioportal.service.util.SessionServiceRequestHandler; -import org.cbioportal.utils.security.AccessLevel; -import org.cbioportal.web.parameter.StudyViewFilter; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; -import org.cbioportal.web.parameter.VirtualStudySamples; 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.ParameterizedTypeReference; import org.springframework.http.HttpEntity; @@ -24,7 +19,6 @@ import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; -import org.springframework.security.core.Authentication; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -36,34 +30,41 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.client.RestTemplate; -import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; @Controller @RequestMapping("/api/public_virtual_studies") public class PublicVirtualStudiesController { private static final Logger LOG = LoggerFactory.getLogger(PublicVirtualStudiesController.class); - - @Value("${session.endpoint.publisher-api-key:}") - private String requiredPublisherApiKey; - + public static final String ALL_USERS = "*"; - @Autowired - SessionServiceRequestHandler sessionServiceRequestHandler; - - @Value("${session.service.url:}") - private String sessionServiceURL; - - @Autowired - private CancerTypeService cancerTypeService; - - @Autowired - private VirtualStudyPermissionService virtualStudyPermissionService; + + private final String requiredPublisherApiKey; + + private final SessionServiceRequestHandler sessionServiceRequestHandler; + + private final String sessionServiceURL; + + private final CancerTypeService cancerTypeService; + + private final VirtualStudyPermissionService virtualStudyPermissionService; + + public PublicVirtualStudiesController( + @Value("${session.endpoint.publisher-api-key:}") String requiredPublisherApiKey, + SessionServiceRequestHandler sessionServiceRequestHandler, + @Value("${session.service.url:}") String sessionServiceURL, + CancerTypeService cancerTypeService, + VirtualStudyPermissionService virtualStudyPermissionService + ) { + this.requiredPublisherApiKey = requiredPublisherApiKey; + this.sessionServiceRequestHandler = sessionServiceRequestHandler; + this.sessionServiceURL = sessionServiceURL; + this.cancerTypeService = cancerTypeService; + this.virtualStudyPermissionService = virtualStudyPermissionService; + } @GetMapping @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) @@ -183,7 +184,7 @@ public ResponseEntity retractVirtualStudy( " Replying with internal server error status code to the client.", statusCode); return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); - } + } VirtualStudy virtualStudy = responseEntity.getBody(); VirtualStudyData data = virtualStudy.getData(); data.setUsers(Collections.emptySet()); From c247aa959ab44a39834ad038a9a47388edcbd12e Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 20 Jun 2024 17:16:07 +0200 Subject: [PATCH 08/28] Add issue link to session service FIXMEs --- .../java/org/cbioportal/web/SessionServiceController.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 245566fcfaa..2bb4634e4ed 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -280,7 +280,7 @@ public ResponseEntity> getUserStudies() throws JsonProcessing content = @Content(schema = @Schema(implementation = Session.class))) public ResponseEntity addSession(@PathVariable Session.SessionType type, @RequestBody JSONObject body) throws IOException { - //FIXME? anonymous user can create sessions. Do we really want that? + //FIXME? anonymous user can create sessions. Do we really want that? https://github.com/cBioPortal/cbioportal/issues/10843 return addSession(type, Optional.empty(), body); } @@ -288,7 +288,7 @@ public ResponseEntity addSession(@PathVariable Session.SessionType type @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = Session.class))) public ResponseEntity addUserSavedVirtualStudy(@RequestBody JSONObject body) throws IOException { - //FIXME? anonymous user can create virtual studies. Do we really want that? + //FIXME? anonymous user can create virtual studies. Do we really want that? https://github.com/cBioPortal/cbioportal/issues/10843 return addSession(Session.SessionType.virtual_study, Optional.of(SessionOperation.save), body); } From 522bd8aba34da73cddb3d45bf4056339320ccf84 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 20 Jun 2024 17:31:07 +0200 Subject: [PATCH 09/28] Fix sonar reported NPE bugs --- .../web/PublicVirtualStudiesController.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 04caf520aee..efc8bf19b28 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -136,19 +136,20 @@ public ResponseEntity publishVirtualStudy( } ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); + VirtualStudy virtualStudy = responseEntity.getBody(); if (!statusCode.is2xxSuccessful()) { LOG.error("The downstream server replied with statusCode={} and body={}." + " Replying with the same status code to the client.", - statusCode, responseEntity.getBody()); + statusCode, virtualStudy); return new ResponseEntity<>(null, statusCode); } - if (responseEntity.getBody() == null) { + if (virtualStudy == null) { LOG.error("The downstream server replied without body and statusCode={}." + " Replying with internal server error status code to the client.", statusCode); return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); } - return publishVirtualStudyData(responseEntity.getBody().getData(), providedPublisherApiKey, typeOfCancerId, pmid); + return publishVirtualStudyData(virtualStudy.getData(), providedPublisherApiKey, typeOfCancerId, pmid); } private ResponseEntity getVirtualStudyById(String id) { @@ -173,19 +174,19 @@ public ResponseEntity retractVirtualStudy( } ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); + VirtualStudy virtualStudy = responseEntity.getBody(); if (!statusCode.is2xxSuccessful()) { LOG.error("The downstream server replied with statusCode={} and body={}." + " Replying with the same status code to the client.", - statusCode, responseEntity.getBody()); + statusCode, virtualStudy); return new ResponseEntity<>(null, statusCode); } - if (responseEntity.getBody() == null) { + if (virtualStudy == null) { LOG.error("The downstream server replied without body and statusCode={}." + " Replying with internal server error status code to the client.", statusCode); return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); } - VirtualStudy virtualStudy = responseEntity.getBody(); VirtualStudyData data = virtualStudy.getData(); data.setUsers(Collections.emptySet()); new RestTemplate() From 7e86b34d4d9cdd248e99ba01cf1fe24d8c9b00d3 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 16:57:40 +0200 Subject: [PATCH 10/28] Remove unnecessary checks for null --- .../cbioportal/web/PublicVirtualStudiesController.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index efc8bf19b28..1305631b153 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -92,8 +92,7 @@ public ResponseEntity publishVirtualStudyData( @RequestParam(required = false) String typeOfCancerId, @RequestParam(required = false) String pmid ) { - if (requiredPublisherApiKey == null - || requiredPublisherApiKey.isBlank() + if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); } @@ -129,8 +128,7 @@ public ResponseEntity publishVirtualStudy( @RequestParam(required = false) String typeOfCancerId, @RequestParam(required = false) String pmid ) { - if (requiredPublisherApiKey == null - || requiredPublisherApiKey.isBlank() + if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); } @@ -167,8 +165,7 @@ public ResponseEntity retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { - if (requiredPublisherApiKey == null - || requiredPublisherApiKey.isBlank() + if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); } From 7f23ad0568b5f8cbc56674c9f8f4321f0c7f0e7a Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 16:59:07 +0200 Subject: [PATCH 11/28] Remove obsolete TODO comment --- .../java/org/cbioportal/web/PublicVirtualStudiesController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 1305631b153..ca9d416ded2 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -158,7 +158,6 @@ private ResponseEntity getVirtualStudyById(String id) { VirtualStudy.class); } - //TODO Removing does not work. Is there caching? @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") public ResponseEntity retractVirtualStudy( From d81832f28da8da4bc4027b38b0b6e33e2b1980aa Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 17:20:25 +0200 Subject: [PATCH 12/28] Throw AccessForbiddenException and use GlobalExceptionHadler instead --- .../service/exception/AccessForbiddenException.java | 7 +++++++ .../org/cbioportal/web/PublicVirtualStudiesController.java | 7 ++++--- .../org/cbioportal/web/error/GlobalExceptionHandler.java | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/cbioportal/service/exception/AccessForbiddenException.java diff --git a/src/main/java/org/cbioportal/service/exception/AccessForbiddenException.java b/src/main/java/org/cbioportal/service/exception/AccessForbiddenException.java new file mode 100644 index 00000000000..d9dd8b64bd5 --- /dev/null +++ b/src/main/java/org/cbioportal/service/exception/AccessForbiddenException.java @@ -0,0 +1,7 @@ +package org.cbioportal.service.exception; + +public class AccessForbiddenException extends RuntimeException { + public AccessForbiddenException(String message) { + super(message); + } +} diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index ca9d416ded2..f7d0eff1195 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -6,6 +6,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.CancerTypeService; +import org.cbioportal.service.exception.AccessForbiddenException; import org.cbioportal.service.exception.CancerTypeNotFoundException; import org.cbioportal.service.util.SessionServiceRequestHandler; import org.cbioportal.web.parameter.VirtualStudy; @@ -94,7 +95,7 @@ public ResponseEntity publishVirtualStudyData( ) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { - return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + throw new AccessForbiddenException("The provided publisher API key is not correct."); } VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); if (typeOfCancerId != null) { @@ -130,7 +131,7 @@ public ResponseEntity publishVirtualStudy( ) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { - return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + throw new AccessForbiddenException("The provided publisher API key is not correct."); } ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); @@ -166,7 +167,7 @@ public ResponseEntity retractVirtualStudy( ) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { - return new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + throw new AccessForbiddenException("The provided publisher API key is not correct."); } ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); diff --git a/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java b/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java index 75bc5c55e6e..9ddb93aaf70 100644 --- a/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java +++ b/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java @@ -162,6 +162,12 @@ public ResponseEntity handleDataAccessTokenProhibitedUserExceptio return new ResponseEntity<>(response, HttpStatus.UNAUTHORIZED); } + @ExceptionHandler(AccessForbiddenException.class) + public ResponseEntity handleAccessForbiddenException() { + ErrorResponse response = new ErrorResponse("The access is forbidden."); + return new ResponseEntity<>(response, HttpStatus.UNAUTHORIZED); + } + @ExceptionHandler(TokenNotFoundException.class) public ResponseEntity handleTokenNotFoundException() { ErrorResponse response = new ErrorResponse("Specified token cannot be found"); From 837435d4a3c143fd80e56afb3306d384a4360a1e Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 17:27:39 +0200 Subject: [PATCH 13/28] Throw IllegalStateException is downstream server return unsuccessful result --- .../web/PublicVirtualStudiesController.java | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index f7d0eff1195..cc7a817f499 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -136,17 +136,11 @@ public ResponseEntity publishVirtualStudy( ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); VirtualStudy virtualStudy = responseEntity.getBody(); - if (!statusCode.is2xxSuccessful()) { + if (!statusCode.is2xxSuccessful() || virtualStudy == null) { LOG.error("The downstream server replied with statusCode={} and body={}." + " Replying with the same status code to the client.", statusCode, virtualStudy); - return new ResponseEntity<>(null, statusCode); - } - if (virtualStudy == null) { - LOG.error("The downstream server replied without body and statusCode={}." + - " Replying with internal server error status code to the client.", - statusCode); - return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); + throw new IllegalStateException("The downstream server response is not successful"); } return publishVirtualStudyData(virtualStudy.getData(), providedPublisherApiKey, typeOfCancerId, pmid); } @@ -172,17 +166,11 @@ public ResponseEntity retractVirtualStudy( ResponseEntity responseEntity = getVirtualStudyById(id); HttpStatusCode statusCode = responseEntity.getStatusCode(); VirtualStudy virtualStudy = responseEntity.getBody(); - if (!statusCode.is2xxSuccessful()) { + if (!statusCode.is2xxSuccessful() || virtualStudy == null) { LOG.error("The downstream server replied with statusCode={} and body={}." + " Replying with the same status code to the client.", statusCode, virtualStudy); - return new ResponseEntity<>(null, statusCode); - } - if (virtualStudy == null) { - LOG.error("The downstream server replied without body and statusCode={}." + - " Replying with internal server error status code to the client.", - statusCode); - return new ResponseEntity<>(null, HttpStatus.INTERNAL_SERVER_ERROR); + throw new IllegalStateException("The downstream server response is not successful"); } VirtualStudyData data = virtualStudy.getData(); data.setUsers(Collections.emptySet()); From bde0e6244bc04176151226f3a9d208645a7650c0 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 17:31:50 +0200 Subject: [PATCH 14/28] Remove raw use of ResponseEntity --- .../org/cbioportal/web/PublicVirtualStudiesController.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index cc7a817f499..93a6adc6bc2 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -155,7 +155,7 @@ private ResponseEntity getVirtualStudyById(String id) { @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") - public ResponseEntity retractVirtualStudy( + public void retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { @@ -177,7 +177,6 @@ public ResponseEntity retractVirtualStudy( new RestTemplate() .put(sessionServiceURL + "/virtual_study/" + id, new HttpEntity<>(data, sessionServiceRequestHandler.getHttpHeaders())); - return new ResponseEntity(HttpStatus.OK); } private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { From 1f2dc38c9b973e26150b972ac628b14c83dc8cca Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 17:36:42 +0200 Subject: [PATCH 15/28] Throw bad request exception instead of returning ResponseEntity --- .../org/cbioportal/web/PublicVirtualStudiesController.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 93a6adc6bc2..3266530e896 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -30,6 +30,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.client.RestTemplate; +import org.springframework.web.server.ResponseStatusException; import java.util.Collections; import java.util.List; @@ -104,7 +105,7 @@ public ResponseEntity publishVirtualStudyData( virtualStudyDataToPublish.setTypeOfCancerId(typeOfCancerId); } catch (CancerTypeNotFoundException e) { LOG.error("No cancer type with id={} were found.", typeOfCancerId); - return new ResponseEntity<>(null, HttpStatus.BAD_REQUEST); + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "The cancer type is not valid.", e); } } if (pmid != null) { From 8d7052169d419249607381581f0145a1b467d4a1 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 26 Jun 2024 17:43:30 +0200 Subject: [PATCH 16/28] Do not filter out VS when user does not hava access to underlying study samples --- .../VirtualStudyPermissionService.java | 56 ------------------- .../web/PublicVirtualStudiesController.java | 8 +-- .../web/SessionServiceController.java | 14 +---- 3 files changed, 2 insertions(+), 76 deletions(-) delete mode 100644 src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java diff --git a/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java b/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java deleted file mode 100644 index 40183545102..00000000000 --- a/src/main/java/org/cbioportal/security/VirtualStudyPermissionService.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.cbioportal.security; - -import org.cbioportal.utils.security.AccessLevel; -import org.cbioportal.web.parameter.StudyViewFilter; -import org.cbioportal.web.parameter.VirtualStudy; -import org.cbioportal.web.parameter.VirtualStudyData; -import org.cbioportal.web.parameter.VirtualStudySamples; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.stereotype.Service; - -import java.util.Iterator; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -@Service -public class VirtualStudyPermissionService { - - private final Optional cancerStudyPermissionEvaluator; - - public VirtualStudyPermissionService(Optional cancerStudyPermissionEvaluator) { - this.cancerStudyPermissionEvaluator = cancerStudyPermissionEvaluator; - } - - public void filterOutForbiddenStudies(List virtualStudies) { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null || cancerStudyPermissionEvaluator.isEmpty()) { - return; - } - Iterator virtualStudyIterator = virtualStudies.iterator(); - while (virtualStudyIterator.hasNext()) { - VirtualStudy virtualStudy = virtualStudyIterator.next(); - VirtualStudyData virtualStudyData = virtualStudy.getData(); - - Set filteredStudies = virtualStudyData.getStudies().stream() - .filter(study -> - cancerStudyPermissionEvaluator.get().hasPermission(authentication, study.getId(), "CancerStudyId", AccessLevel.READ)) - .collect(Collectors.toSet()); - if (filteredStudies.isEmpty()) { - virtualStudyIterator.remove(); - continue; - } - virtualStudyData.setStudies(filteredStudies); - - StudyViewFilter studyViewFilter = virtualStudyData.getStudyViewFilter(); - List filteredStudyIds = studyViewFilter.getStudyIds().stream() - .filter(studyId -> - cancerStudyPermissionEvaluator.get().hasPermission(authentication, studyId, "CancerStudyId", AccessLevel.READ)) - .toList(); - virtualStudyData.getStudyViewFilter().setStudyIds(filteredStudyIds); - } - - } -} diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 3266530e896..65bafd7cfab 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -4,7 +4,6 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; -import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.CancerTypeService; import org.cbioportal.service.exception.AccessForbiddenException; import org.cbioportal.service.exception.CancerTypeNotFoundException; @@ -52,20 +51,16 @@ public class PublicVirtualStudiesController { private final CancerTypeService cancerTypeService; - private final VirtualStudyPermissionService virtualStudyPermissionService; - public PublicVirtualStudiesController( @Value("${session.endpoint.publisher-api-key:}") String requiredPublisherApiKey, SessionServiceRequestHandler sessionServiceRequestHandler, @Value("${session.service.url:}") String sessionServiceURL, - CancerTypeService cancerTypeService, - VirtualStudyPermissionService virtualStudyPermissionService + CancerTypeService cancerTypeService ) { this.requiredPublisherApiKey = requiredPublisherApiKey; this.sessionServiceRequestHandler = sessionServiceRequestHandler; this.sessionServiceURL = sessionServiceURL; this.cancerTypeService = cancerTypeService; - this.virtualStudyPermissionService = virtualStudyPermissionService; } @GetMapping @@ -82,7 +77,6 @@ public ResponseEntity> getPublicVirtualStudies() { }); List virtualStudies = responseEntity.getBody(); - virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudies); return new ResponseEntity<>(virtualStudies, HttpStatus.OK); } diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 2bb4634e4ed..8a9965d2b24 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -12,7 +12,6 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import jakarta.servlet.http.HttpServletResponse; import jakarta.validation.constraints.Size; -import org.cbioportal.security.VirtualStudyPermissionService; import org.cbioportal.service.util.CustomAttributeWithData; import org.cbioportal.service.util.CustomDataSession; import org.cbioportal.service.util.SessionServiceRequestHandler; @@ -80,9 +79,6 @@ public class SessionServiceController { @Value("${session.service.url:}") private String sessionServiceURL; - @Autowired - private VirtualStudyPermissionService virtualStudyPermissionService; - private static Map> pageToSettingsDataClass = ImmutableMap.of( SessionPage.study_view, StudyPageSettings.class, SessionPage.results_view, ResultsPageSettings.class @@ -215,14 +211,7 @@ public ResponseEntity getSession(@PathVariable Session.SessionType type Session session; switch (type) { case virtual_study: - VirtualStudy virtualStudy = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); - List virtualStudies = new ArrayList<>(); - virtualStudies.add(virtualStudy); - virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudies); - if (virtualStudies.isEmpty()) { - return new ResponseEntity<>(HttpStatus.NOT_FOUND); - } - session = virtualStudies.getFirst(); + session = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); break; case settings: session = sessionServiceObjectMapper.readValue(sessionDataJson, PageSettings.class); @@ -265,7 +254,6 @@ public ResponseEntity> getUserStudies() throws JsonProcessing new ParameterizedTypeReference>() {}); List virtualStudyList = responseEntity.getBody(); - virtualStudyPermissionService.filterOutForbiddenStudies(virtualStudyList); return new ResponseEntity<>(virtualStudyList, HttpStatus.OK); } catch (Exception exception) { LOG.error("Error occurred", exception); From cc3a7ee8ec5555c98c339cc281adeaeac2a269d5 Mon Sep 17 00:00:00 2001 From: Charles Haynes Date: Thu, 27 Jun 2024 11:58:21 -0400 Subject: [PATCH 17/28] Fix integration tests --- pom.xml | 10 +++++++++- .../cbioportal/web/PublicVirtualStudiesController.java | 3 ++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index b3866426742..20936fa44d7 100644 --- a/pom.xml +++ b/pom.xml @@ -98,6 +98,7 @@ 3.14.0 4.17.0 7.1.0 + 5.2.1 @@ -346,7 +347,14 @@ sentry-spring-boot-starter-jakarta ${sentry.version} - + + org.apache.httpcomponents.client5 + httpclient5 + ${apache_httpclient.version} + test + + + diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 65bafd7cfab..d88985a692c 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -150,7 +150,7 @@ private ResponseEntity getVirtualStudyById(String id) { @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") - public void retractVirtualStudy( + public ResponseEntity retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { @@ -172,6 +172,7 @@ public void retractVirtualStudy( new RestTemplate() .put(sessionServiceURL + "/virtual_study/" + id, new HttpEntity<>(data, sessionServiceRequestHandler.getHttpHeaders())); + return ResponseEntity.ok().build(); } private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { From faaf3fae524f6b5699e2c9f3970fbf372eee4dc1 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:03:04 +0200 Subject: [PATCH 18/28] Extract http calls to the session service to the handler --- .../util/SessionServiceRequestHandler.java | 81 +++++++++++++++++++ .../web/PublicVirtualStudiesController.java | 66 ++------------- 2 files changed, 86 insertions(+), 61 deletions(-) diff --git a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java index c408f2f2139..48322e367bd 100644 --- a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java +++ b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java @@ -4,13 +4,23 @@ import java.nio.charset.Charset; +import java.util.Collections; +import java.util.List; +import com.mongodb.BasicDBObject; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; +import org.cbioportal.web.PublicVirtualStudiesController; +import org.cbioportal.web.parameter.VirtualStudy; +import org.cbioportal.web.parameter.VirtualStudyData; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Component; import org.springframework.web.client.RestTemplate; @@ -18,6 +28,8 @@ @Component public class SessionServiceRequestHandler { + private static final Logger LOG = LoggerFactory.getLogger(SessionServiceRequestHandler.class); + @Value("${session.service.url:}") private String sessionServiceURL; @@ -62,4 +74,73 @@ public String getSessionDataJson(SessionType type, String id) throws Exception { return responseEntity.getBody(); } + /** + * Gets virtual study by id + * @param id - id of the virtual study to read + * @return virtual study + */ + public VirtualStudy getVirtualStudyById(String id) { + ResponseEntity responseEntity = new RestTemplate() + .exchange(sessionServiceURL + "/virtual_study/" + id, + HttpMethod.GET, + new HttpEntity<>(getHttpHeaders()), + VirtualStudy.class); + HttpStatusCode statusCode = responseEntity.getStatusCode(); + VirtualStudy virtualStudy = responseEntity.getBody(); + if (!statusCode.is2xxSuccessful() || virtualStudy == null) { + LOG.error("The downstream server replied with statusCode={} and body={}." + + " Replying with the same status code to the client.", + statusCode, virtualStudy); + throw new IllegalStateException("The downstream server response is not successful"); + } + return responseEntity.getBody(); + } + + /** + * Get list of virtual studies by username + * @param username - user assigned to the virtual study + * @return - list of virtual studies + */ + public List getVirtualStudiesForUser(String username) { + BasicDBObject basicDBObject = new BasicDBObject(); + basicDBObject.put("data.users", username); + ResponseEntity> responseEntity = new RestTemplate().exchange( + sessionServiceURL + "/virtual_study/query/fetch", + HttpMethod.POST, + new HttpEntity<>(basicDBObject.toString(), getHttpHeaders()), + new ParameterizedTypeReference<>() { + }); + + return responseEntity.getBody(); + } + + /** + * Creates a virtual study out of virtual study definition (aka virtualStudyData) + * @param virtualStudyData - definition of virtual study + * @return virtual study object with id and the virtualStudyData + */ + public VirtualStudy createVirtualStudy(VirtualStudyData virtualStudyData) { + ResponseEntity responseEntity = new RestTemplate().exchange( + sessionServiceURL + "/virtual_study", + HttpMethod.POST, + new HttpEntity<>(virtualStudyData, getHttpHeaders()), + new ParameterizedTypeReference<>() { + }); + + return responseEntity.getBody(); + } + + + /** + * Soft delete of the virtual study by de-associating all assigned users. + * @param id - id of virtual study to soft delete + */ + public void softRemoveVirtualStudy(String id) { + VirtualStudy virtualStudy = getVirtualStudyById(id); + VirtualStudyData data = virtualStudy.getData(); + data.setUsers(Collections.emptySet()); + new RestTemplate() + .put(sessionServiceURL + "/virtual_study/" + id, + new HttpEntity<>(data, getHttpHeaders())); + } } diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index d88985a692c..b39f3d8cffc 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -1,6 +1,5 @@ package org.cbioportal.web; -import com.mongodb.BasicDBObject; import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -13,11 +12,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; -import org.springframework.core.ParameterizedTypeReference; -import org.springframework.http.HttpEntity; -import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; -import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.DeleteMapping; @@ -28,10 +23,8 @@ import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.client.RestTemplate; import org.springframework.web.server.ResponseStatusException; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -47,36 +40,22 @@ public class PublicVirtualStudiesController { private final SessionServiceRequestHandler sessionServiceRequestHandler; - private final String sessionServiceURL; - private final CancerTypeService cancerTypeService; public PublicVirtualStudiesController( @Value("${session.endpoint.publisher-api-key:}") String requiredPublisherApiKey, SessionServiceRequestHandler sessionServiceRequestHandler, - @Value("${session.service.url:}") String sessionServiceURL, CancerTypeService cancerTypeService ) { this.requiredPublisherApiKey = requiredPublisherApiKey; this.sessionServiceRequestHandler = sessionServiceRequestHandler; - this.sessionServiceURL = sessionServiceURL; this.cancerTypeService = cancerTypeService; } @GetMapping @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) public ResponseEntity> getPublicVirtualStudies() { - //TODO move this logic to sessionServiceRequestHandler? - BasicDBObject basicDBObject = new BasicDBObject(); - basicDBObject.put("data.users", ALL_USERS); - ResponseEntity> responseEntity = new RestTemplate().exchange( - sessionServiceURL + "/virtual_study/query/fetch", - HttpMethod.POST, - new HttpEntity<>(basicDBObject.toString(), sessionServiceRequestHandler.getHttpHeaders()), - new ParameterizedTypeReference<>() { - }); - - List virtualStudies = responseEntity.getBody(); + List virtualStudies = sessionServiceRequestHandler.getVirtualStudiesForUser(ALL_USERS); return new ResponseEntity<>(virtualStudies, HttpStatus.OK); } @@ -105,15 +84,9 @@ public ResponseEntity publishVirtualStudyData( if (pmid != null) { virtualStudyDataToPublish.setPmid(pmid); } - //TODO move this logic to sessionServiceRequestHandler? - ResponseEntity responseEntity = new RestTemplate().exchange( - sessionServiceURL + "/virtual_study", - HttpMethod.POST, - new HttpEntity<>(virtualStudyDataToPublish, sessionServiceRequestHandler.getHttpHeaders()), - new ParameterizedTypeReference<>() { - }); + VirtualStudy virtualStudy = sessionServiceRequestHandler.createVirtualStudy(virtualStudyDataToPublish); - return new ResponseEntity<>(responseEntity.getBody(), HttpStatus.OK); + return new ResponseEntity<>(virtualStudy, HttpStatus.OK); } @PostMapping("/{id}") @@ -128,26 +101,10 @@ public ResponseEntity publishVirtualStudy( || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { throw new AccessForbiddenException("The provided publisher API key is not correct."); } - ResponseEntity responseEntity = getVirtualStudyById(id); - HttpStatusCode statusCode = responseEntity.getStatusCode(); - VirtualStudy virtualStudy = responseEntity.getBody(); - if (!statusCode.is2xxSuccessful() || virtualStudy == null) { - LOG.error("The downstream server replied with statusCode={} and body={}." + - " Replying with the same status code to the client.", - statusCode, virtualStudy); - throw new IllegalStateException("The downstream server response is not successful"); - } + VirtualStudy virtualStudy = sessionServiceRequestHandler.getVirtualStudyById(id); return publishVirtualStudyData(virtualStudy.getData(), providedPublisherApiKey, typeOfCancerId, pmid); } - private ResponseEntity getVirtualStudyById(String id) { - return new RestTemplate() - .exchange(sessionServiceURL + "/virtual_study/" + id, - HttpMethod.GET, - new HttpEntity<>(sessionServiceRequestHandler.getHttpHeaders()), - VirtualStudy.class); - } - @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") public ResponseEntity retractVirtualStudy( @@ -158,20 +115,7 @@ public ResponseEntity retractVirtualStudy( || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { throw new AccessForbiddenException("The provided publisher API key is not correct."); } - ResponseEntity responseEntity = getVirtualStudyById(id); - HttpStatusCode statusCode = responseEntity.getStatusCode(); - VirtualStudy virtualStudy = responseEntity.getBody(); - if (!statusCode.is2xxSuccessful() || virtualStudy == null) { - LOG.error("The downstream server replied with statusCode={} and body={}." + - " Replying with the same status code to the client.", - statusCode, virtualStudy); - throw new IllegalStateException("The downstream server response is not successful"); - } - VirtualStudyData data = virtualStudy.getData(); - data.setUsers(Collections.emptySet()); - new RestTemplate() - .put(sessionServiceURL + "/virtual_study/" + id, - new HttpEntity<>(data, sessionServiceRequestHandler.getHttpHeaders())); + sessionServiceRequestHandler.softRemoveVirtualStudy(id); return ResponseEntity.ok().build(); } From 0e3e59b0926066c3cddc6ccdaeea2109d0a45fdc Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:11:04 +0200 Subject: [PATCH 19/28] Remove todo comment User has to have * username witch is not likely --- src/main/java/org/cbioportal/web/SessionServiceController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 8a9965d2b24..83cf95c932a 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -308,7 +308,6 @@ public void updateUsersInSession(@PathVariable Session.SessionType type, @PathVa VirtualStudyData virtualStudyData = virtualStudy.getData(); Set users = virtualStudyData.getUsers(); updateUserList(operation, users); - //TODO userName could not be ALL_USERS (*) here virtualStudyData.setUsers(users); httpEntity = new HttpEntity<>(virtualStudyData, sessionServiceRequestHandler.getHttpHeaders()); } From 579367170505be214ccaa302dbea7ff46545a82b Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:23:07 +0200 Subject: [PATCH 20/28] Fix sonarcloud issues --- .../util/SessionServiceRequestHandler.java | 1 - .../web/PublicVirtualStudiesController.java | 2 +- .../PublicVirtualStudiesIntegrationTest.java | 22 +++++++++---------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java index 48322e367bd..f943eb93e19 100644 --- a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java +++ b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java @@ -10,7 +10,6 @@ import com.mongodb.BasicDBObject; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; -import org.cbioportal.web.PublicVirtualStudiesController; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; import org.slf4j.Logger; diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index b39f3d8cffc..d7360a1165e 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -107,7 +107,7 @@ public ResponseEntity publishVirtualStudy( @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") - public ResponseEntity retractVirtualStudy( + public ResponseEntity retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { diff --git a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java index 982fa29c964..ab831592bba 100644 --- a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java +++ b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java @@ -53,33 +53,33 @@ @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class PublicVirtualStudiesIntegrationTest extends ContainerConfig { - final static String CBIO_URL = String.format("http://localhost:%d", CBIO_PORT); + static final String CBIO_URL = String.format("http://localhost:%d", CBIO_PORT); - final static HttpHeaders jsonContentType = new HttpHeaders() { + static final HttpHeaders jsonContentType = new HttpHeaders() { { set("Content-Type", "application/json"); } }; - - final static HttpHeaders invalidKeyContainingHeaders = new HttpHeaders() { + + static final HttpHeaders invalidKeyContainingHeaders = new HttpHeaders() { { set("X-PUBLISHER-API-KEY", "this-is-not-valid-key"); } }; - - final static HttpHeaders validKeyContainingHeaders = new HttpHeaders() { + + static final HttpHeaders validKeyContainingHeaders = new HttpHeaders() { { set("X-PUBLISHER-API-KEY", "this-is-a-secret"); } }; - - final static ParameterizedTypeReference> typeRef = new ParameterizedTypeReference<>() { + + static final ParameterizedTypeReference> typeRef = new ParameterizedTypeReference<>() { }; static String createdVsId; static String publishedVsId; - final static VirtualStudyData virtualStudyDataToSave = createTestVsData(); + static final VirtualStudyData virtualStudyDataToSave = createTestVsData(); @Autowired private TestRestTemplate restTemplate; @@ -93,7 +93,7 @@ public void test1NoPublicVirtualStudiesAtTheBeginning() { typeRef); assertThat(response1.getStatusCode().is2xxSuccessful()).isTrue(); - assertThat(response1.getBody()).hasSize(0); + assertThat(response1.getBody()).isEmpty(); } @Test @@ -193,7 +193,7 @@ public void test6NoPublicVirtualStudiesAfterRemoval() { typeRef); assertThat(response6.getStatusCode().is2xxSuccessful()).isTrue(); - assertThat(response6.getBody()).hasSize(0); + assertThat(response6.getBody()).isEmpty(); } static VirtualStudyData createTestVsData() { From 67dcf90f885e1e1ce7c3a1563c1223eae215e8ea Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:31:49 +0200 Subject: [PATCH 21/28] Deduplicate ensuring publisher api key is correct --- .../web/PublicVirtualStudiesController.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index d7360a1165e..c7df0c56abe 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -67,10 +67,7 @@ public ResponseEntity publishVirtualStudyData( @RequestParam(required = false) String typeOfCancerId, @RequestParam(required = false) String pmid ) { - if (requiredPublisherApiKey.isBlank() - || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { - throw new AccessForbiddenException("The provided publisher API key is not correct."); - } + ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); if (typeOfCancerId != null) { try { @@ -97,10 +94,7 @@ public ResponseEntity publishVirtualStudy( @RequestParam(required = false) String typeOfCancerId, @RequestParam(required = false) String pmid ) { - if (requiredPublisherApiKey.isBlank() - || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { - throw new AccessForbiddenException("The provided publisher API key is not correct."); - } + ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); VirtualStudy virtualStudy = sessionServiceRequestHandler.getVirtualStudyById(id); return publishVirtualStudyData(virtualStudy.getData(), providedPublisherApiKey, typeOfCancerId, pmid); } @@ -111,12 +105,16 @@ public ResponseEntity retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { + ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); + sessionServiceRequestHandler.softRemoveVirtualStudy(id); + return ResponseEntity.ok().build(); + } + + private void ensureProvidedPublisherApiKeyCorrect(String providedPublisherApiKey) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { throw new AccessForbiddenException("The provided publisher API key is not correct."); } - sessionServiceRequestHandler.softRemoveVirtualStudy(id); - return ResponseEntity.ok().build(); } private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { From a72884afbe1f48df597167c2246a4c3740a86b55 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:35:17 +0200 Subject: [PATCH 22/28] Remove usage of generic wildcard type --- .../java/org/cbioportal/web/PublicVirtualStudiesController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index c7df0c56abe..41126b72ceb 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -101,7 +101,7 @@ public ResponseEntity publishVirtualStudy( @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") - public ResponseEntity retractVirtualStudy( + public ResponseEntity retractVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { From ca1b3a9433a22622a906367f2b3ff2149a72246e Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Thu, 27 Jun 2024 21:47:12 +0200 Subject: [PATCH 23/28] Extract logic to update VS metadata fields into a method --- .../web/PublicVirtualStudiesController.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 41126b72ceb..78998aa691a 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -23,7 +23,6 @@ import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.server.ResponseStatusException; import java.util.List; import java.util.Set; @@ -69,18 +68,7 @@ public ResponseEntity publishVirtualStudyData( ) { ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); - if (typeOfCancerId != null) { - try { - cancerTypeService.getCancerType(typeOfCancerId); - virtualStudyDataToPublish.setTypeOfCancerId(typeOfCancerId); - } catch (CancerTypeNotFoundException e) { - LOG.error("No cancer type with id={} were found.", typeOfCancerId); - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "The cancer type is not valid.", e); - } - } - if (pmid != null) { - virtualStudyDataToPublish.setPmid(pmid); - } + updateStudyMetadataFieldsIfSpecified(virtualStudyDataToPublish, typeOfCancerId, pmid); VirtualStudy virtualStudy = sessionServiceRequestHandler.createVirtualStudy(virtualStudyDataToPublish); return new ResponseEntity<>(virtualStudy, HttpStatus.OK); @@ -117,6 +105,21 @@ private void ensureProvidedPublisherApiKeyCorrect(String providedPublisherApiKey } } + private void updateStudyMetadataFieldsIfSpecified(VirtualStudyData virtualStudyData, String typeOfCancerId, String pmid) { + if (typeOfCancerId != null) { + try { + cancerTypeService.getCancerType(typeOfCancerId); + virtualStudyData.setTypeOfCancerId(typeOfCancerId); + } catch (CancerTypeNotFoundException e) { + LOG.error("No cancer type with id={} were found.", typeOfCancerId); + throw new IllegalArgumentException( "The cancer type is not valid: " + typeOfCancerId); + } + } + if (pmid != null) { + virtualStudyData.setPmid(pmid); + } + } + private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { VirtualStudyData virtualStudyDataToPublish = new VirtualStudyData(); virtualStudyDataToPublish.setName(virtualStudyData.getName()); From 86167371c0b0c8d1375912766df2e683edb51dd8 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Fri, 28 Jun 2024 17:49:28 +0200 Subject: [PATCH 24/28] Document publishing virtual study feature --- docs/Create-And-Publish-Virtual-Study.md | 64 +++++++++++++++++++ docs/Data-Loading.md | 3 + .../resources/application.properties.EXAMPLE | 3 + 3 files changed, 70 insertions(+) create mode 100644 docs/Create-And-Publish-Virtual-Study.md diff --git a/docs/Create-And-Publish-Virtual-Study.md b/docs/Create-And-Publish-Virtual-Study.md new file mode 100644 index 00000000000..f04ae388654 --- /dev/null +++ b/docs/Create-And-Publish-Virtual-Study.md @@ -0,0 +1,64 @@ +# Create and Publish Virtual Study + +A [Virtual Study](./user-guide/faq.md#what-is-a-virtual-study) defines a subset or a combination of samples from existing physical studies in the system. + +*Note*: To publish or un-publish a virtual study, your cBioPortal instance must be configured with `session.endpoint.publisher-api-key` in the `application.properties`. + +## Create Virtual Study + +To create a virtual study in cBioPortal, follow these steps: + +1. Define the desired filters on the study or multiple studies summary page. +2. Click the button with the bookmark icon () in the top right corner of the screen. +3. Provide a title and description, then click the Save button. You will see a link that looks like: + +``` +https:///study?id= +``` + +4. Save the virtual study link or ID if you want to publish it. + +If you are logged in, this virtual study will appear in the `My Virtual Studies` section on the landing page. +You can always find the ID of the virtual study from the URL of the page that opens after clicking on it. + +## Publish Virtual Study + +To publish a virtual study, you need to supply the publisher API key in the `X-PUBLISHER-API-KEY` header. + +Here is a curl command to publish a virtual study: +```shell +curl \ + -X POST \ + -H 'X-PUBLISHER-API-KEY: ' \ + -v 'http:///api/public_virtual_studies/' +``` +This call publishes a *copy* of the virtual study under a different ID. +The new ID hash will be printed by the command. +The published virtual study will appear under the `Public Virtual Studies` section (next to the `My Virtual Studies` section) on the landing page for all users of cBioPortal. + +While publishing, you can specify the PubMed ID (`pmid`) and `typeOfCancerId` of the virtual study using the following command: +```shell +curl \ + -X POST \ + -H 'X-PUBLISHER-API-KEY: ' \ + -v 'http:///api/public_virtual_studies/?pmid=&typeOfCancerId=' +``` + +The type of cancer code should match the known types of cancers in the cBioPortal database. +If the type of cancer is specified, the published virtual study will appear under the respective cancer section on the landing page. +Specifying the `pmid` enables a link to the PubMed page of the study. + +## Un-publish Virtual Study + +To un-publish a virtual study, you need to supply the publisher API key in the `X-PUBLISHER-API-KEY` header. +After un-publishing, the virtual study is still accessible by the link, but it will be unlisted from the list of public virtual studies and not shown on the landing page anymore. + +Here is the command to un-publish a virtual study: +```shell +curl \ + -X DELETE \ + -H 'X-PUBLISHER-API-KEY: ' \ + -v 'http:///api/public_virtual_studies/' +``` + +*Note*: You have to use the ID of the *published* virtual study, not the one it was copied from during the publishing process. \ No newline at end of file diff --git a/docs/Data-Loading.md b/docs/Data-Loading.md index 39f27d9b385..9465f051edb 100644 --- a/docs/Data-Loading.md +++ b/docs/Data-Loading.md @@ -58,3 +58,6 @@ To remove a study, the [cbioportalImporter script](/Data-Loading-Maintaining-Stu ## Example studies Examples for the different types of data are available on the [File Formats](/File-Formats.md) page. The Provisional TCGA studies, downloadable from the [Data Sets section](https://www.cbioportal.org/datasets) are complete studies that can be used as reference when creating data files. + +## Public Virtual Studies +If your new study data is a subset or a combination of existing studies in the system, consider using [Public Virtual Studies](./Create-And-Publish-Virtual-Study.md) instead of duplicating data. \ No newline at end of file diff --git a/src/main/resources/application.properties.EXAMPLE b/src/main/resources/application.properties.EXAMPLE index 50a440c0bca..82bfe4a07a5 100644 --- a/src/main/resources/application.properties.EXAMPLE +++ b/src/main/resources/application.properties.EXAMPLE @@ -239,6 +239,9 @@ session.service.url=https://cbioportal-session-service.herokuapp.com/session_ser #session.service.user= #session.service.password= +# Publishing Virtual Studies +#session.endpoint.publisher-api-key= + # disabled tabs, | delimited # possible values: cancer_types_summary, mutual_exclusivity, comparison, plots, mutations, co_expression, enrichments, survival, network, download, bookmark, IGV disabled_tabs= From 3347010bb91a2f5693ded827e5a32a8a3affd278 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Tue, 2 Jul 2024 14:49:42 +0200 Subject: [PATCH 25/28] Update docs/Create-And-Publish-Virtual-Study.md Co-authored-by: pieterlukasse --- docs/Create-And-Publish-Virtual-Study.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Create-And-Publish-Virtual-Study.md b/docs/Create-And-Publish-Virtual-Study.md index f04ae388654..6e7a12ccd5e 100644 --- a/docs/Create-And-Publish-Virtual-Study.md +++ b/docs/Create-And-Publish-Virtual-Study.md @@ -1,6 +1,6 @@ # Create and Publish Virtual Study -A [Virtual Study](./user-guide/faq.md#what-is-a-virtual-study) defines a subset or a combination of samples from existing physical studies in the system. +A [Virtual Study](./user-guide/faq.md#what-is-a-virtual-study) defines a subset or a combination of samples from one or more studies in the system. *Note*: To publish or un-publish a virtual study, your cBioPortal instance must be configured with `session.endpoint.publisher-api-key` in the `application.properties`. From 4cd40c40660f538723fba817ea365abdc9b403a9 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Tue, 2 Jul 2024 16:16:56 +0200 Subject: [PATCH 26/28] Publish virtual study by modifying it instead of making copy Also get rid of undocumented endpoint --- docs/Create-And-Publish-Virtual-Study.md | 8 +--- .../util/SessionServiceRequestHandler.java | 12 +++++- .../web/PublicVirtualStudiesController.java | 42 +++++-------------- .../PublicVirtualStudiesIntegrationTest.java | 24 +++++------ 4 files changed, 33 insertions(+), 53 deletions(-) diff --git a/docs/Create-And-Publish-Virtual-Study.md b/docs/Create-And-Publish-Virtual-Study.md index 6e7a12ccd5e..1a8cc0e2169 100644 --- a/docs/Create-And-Publish-Virtual-Study.md +++ b/docs/Create-And-Publish-Virtual-Study.md @@ -32,8 +32,6 @@ curl \ -H 'X-PUBLISHER-API-KEY: ' \ -v 'http:///api/public_virtual_studies/' ``` -This call publishes a *copy* of the virtual study under a different ID. -The new ID hash will be printed by the command. The published virtual study will appear under the `Public Virtual Studies` section (next to the `My Virtual Studies` section) on the landing page for all users of cBioPortal. While publishing, you can specify the PubMed ID (`pmid`) and `typeOfCancerId` of the virtual study using the following command: @@ -58,7 +56,5 @@ Here is the command to un-publish a virtual study: curl \ -X DELETE \ -H 'X-PUBLISHER-API-KEY: ' \ - -v 'http:///api/public_virtual_studies/' -``` - -*Note*: You have to use the ID of the *published* virtual study, not the one it was copied from during the publishing process. \ No newline at end of file + -v 'http:///api/public_virtual_studies/' +``` \ No newline at end of file diff --git a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java index f943eb93e19..a6e1c317234 100644 --- a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java +++ b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java @@ -138,8 +138,16 @@ public void softRemoveVirtualStudy(String id) { VirtualStudy virtualStudy = getVirtualStudyById(id); VirtualStudyData data = virtualStudy.getData(); data.setUsers(Collections.emptySet()); + updateVirtualStudy(virtualStudy); + } + + /** + * Updates virtual study + * @param virtualStudy - virtual study to update + */ + public void updateVirtualStudy(VirtualStudy virtualStudy) { new RestTemplate() - .put(sessionServiceURL + "/virtual_study/" + id, - new HttpEntity<>(data, getHttpHeaders())); + .put(sessionServiceURL + "/virtual_study/" + virtualStudy.getId(), + new HttpEntity<>(virtualStudy.getData(), getHttpHeaders())); } } diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 78998aa691a..8189b9ad845 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -19,7 +19,6 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; -import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -58,33 +57,17 @@ public ResponseEntity> getPublicVirtualStudies() { return new ResponseEntity<>(virtualStudies, HttpStatus.OK); } - @PostMapping - @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) - public ResponseEntity publishVirtualStudyData( - @RequestBody VirtualStudyData virtualStudyData, - @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey, - @RequestParam(required = false) String typeOfCancerId, - @RequestParam(required = false) String pmid - ) { - ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); - VirtualStudyData virtualStudyDataToPublish = makeCopyForPublishing(virtualStudyData); - updateStudyMetadataFieldsIfSpecified(virtualStudyDataToPublish, typeOfCancerId, pmid); - VirtualStudy virtualStudy = sessionServiceRequestHandler.createVirtualStudy(virtualStudyDataToPublish); - - return new ResponseEntity<>(virtualStudy, HttpStatus.OK); - } - @PostMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) - public ResponseEntity publishVirtualStudy( + public ResponseEntity publishVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey, @RequestParam(required = false) String typeOfCancerId, @RequestParam(required = false) String pmid ) { ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); - VirtualStudy virtualStudy = sessionServiceRequestHandler.getVirtualStudyById(id); - return publishVirtualStudyData(virtualStudy.getData(), providedPublisherApiKey, typeOfCancerId, pmid); + publishVirtualStudy(id, typeOfCancerId, pmid); + return ResponseEntity.ok().build(); } @DeleteMapping("/{id}") @@ -98,6 +81,14 @@ public ResponseEntity retractVirtualStudy( return ResponseEntity.ok().build(); } + private void publishVirtualStudy(String id, String typeOfCancerId, String pmid) { + VirtualStudy virtualStudyDataToPublish = sessionServiceRequestHandler.getVirtualStudyById(id); + VirtualStudyData virtualStudyData = virtualStudyDataToPublish.getData(); + updateStudyMetadataFieldsIfSpecified(virtualStudyData, typeOfCancerId, pmid); + virtualStudyData.setUsers(Set.of(ALL_USERS)); + sessionServiceRequestHandler.updateVirtualStudy(virtualStudyDataToPublish); + } + private void ensureProvidedPublisherApiKeyCorrect(String providedPublisherApiKey) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { @@ -120,15 +111,4 @@ private void updateStudyMetadataFieldsIfSpecified(VirtualStudyData virtualStudyD } } - private VirtualStudyData makeCopyForPublishing(VirtualStudyData virtualStudyData) { - VirtualStudyData virtualStudyDataToPublish = new VirtualStudyData(); - virtualStudyDataToPublish.setName(virtualStudyData.getName()); - virtualStudyDataToPublish.setDescription(virtualStudyData.getDescription()); - virtualStudyDataToPublish.setStudies(virtualStudyData.getStudies()); - virtualStudyDataToPublish.setStudyViewFilter(virtualStudyData.getStudyViewFilter()); - virtualStudyDataToPublish.setTypeOfCancerId(virtualStudyData.getTypeOfCancerId()); - virtualStudyDataToPublish.setPmid(virtualStudyData.getPmid()); - virtualStudyDataToPublish.setUsers(Set.of(ALL_USERS)); - return virtualStudyDataToPublish; - } } diff --git a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java index ab831592bba..01d54b41cee 100644 --- a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java +++ b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java @@ -76,8 +76,7 @@ public class PublicVirtualStudiesIntegrationTest extends ContainerConfig { static final ParameterizedTypeReference> typeRef = new ParameterizedTypeReference<>() { }; - static String createdVsId; - static String publishedVsId; + static String virtualStudyId; static final VirtualStudyData virtualStudyDataToSave = createTestVsData(); @@ -107,35 +106,32 @@ public void test2CreateVirtualStudy() { assertThat(response2.getStatusCode().is2xxSuccessful()).isTrue(); VirtualStudy savedVs = response2.getBody(); assertThat(savedVs).isNotNull().hasFieldOrProperty("id").isNotNull(); - createdVsId = savedVs.getId(); + virtualStudyId = savedVs.getId(); } @Test public void test3PublishVirtualStudy() { - String url = CBIO_URL + "/api/public_virtual_studies/" + createdVsId + "?typeOfCancerId=acc&pmid=12345"; - ResponseEntity response3 = restTemplate.exchange( + String url = CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId + "?typeOfCancerId=acc&pmid=12345"; + ResponseEntity response3 = restTemplate.exchange( url, HttpMethod.POST, null, - VirtualStudy.class); + Void.class); assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); response3 = restTemplate.exchange( url, HttpMethod.POST, new HttpEntity<>(null, invalidKeyContainingHeaders), - VirtualStudy.class); + Void.class); assertThat(response3.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); response3 = restTemplate.exchange( url, HttpMethod.POST, new HttpEntity<>(null, validKeyContainingHeaders), - VirtualStudy.class); + Void.class); assertThat(response3.getStatusCode().is2xxSuccessful()).isTrue(); - VirtualStudy publishedVs = response3.getBody(); - assertThat(publishedVs).isNotNull(); - publishedVsId = publishedVs.getId(); } @Test @@ -163,21 +159,21 @@ public void test4ListJustPublishedStudy() { @Test public void test5UnpublishVirtualStudy() { ResponseEntity response5 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, null, Object.class); assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); response5 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, new HttpEntity<>(null, invalidKeyContainingHeaders), Object.class); assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); response5 = restTemplate.exchange( - CBIO_URL + "/api/public_virtual_studies/" + publishedVsId, + CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, new HttpEntity<>(null, validKeyContainingHeaders), Object.class); From b99e106f6d915e1d9784d353f0b0c84d8247217f Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Tue, 2 Jul 2024 16:21:08 +0200 Subject: [PATCH 27/28] Improve name and docs of method to retrieve VS for user --- .../service/util/SessionServiceRequestHandler.java | 6 +++--- .../org/cbioportal/web/PublicVirtualStudiesController.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java index a6e1c317234..af9749ee052 100644 --- a/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java +++ b/src/main/java/org/cbioportal/service/util/SessionServiceRequestHandler.java @@ -96,11 +96,11 @@ public VirtualStudy getVirtualStudyById(String id) { } /** - * Get list of virtual studies by username - * @param username - user assigned to the virtual study + * Get list of virtual studies accessible to user + * @param username - user for whom get list of virtual studies * @return - list of virtual studies */ - public List getVirtualStudiesForUser(String username) { + public List getVirtualStudiesAccessibleToUser(String username) { BasicDBObject basicDBObject = new BasicDBObject(); basicDBObject.put("data.users", username); ResponseEntity> responseEntity = new RestTemplate().exchange( diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index 8189b9ad845..cc8fe356748 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -53,7 +53,7 @@ public PublicVirtualStudiesController( @GetMapping @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = VirtualStudy.class))) public ResponseEntity> getPublicVirtualStudies() { - List virtualStudies = sessionServiceRequestHandler.getVirtualStudiesForUser(ALL_USERS); + List virtualStudies = sessionServiceRequestHandler.getVirtualStudiesAccessibleToUser(ALL_USERS); return new ResponseEntity<>(virtualStudies, HttpStatus.OK); } From 84249a3f34e480ae61c90ae13659788a136ca75d Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Tue, 2 Jul 2024 18:19:15 +0200 Subject: [PATCH 28/28] Assign VM after un-publshing to the owner Make sure that we unpublish public virtual study. Fail otherwise. --- docs/Create-And-Publish-Virtual-Study.md | 3 +- .../web/PublicVirtualStudiesController.java | 29 +++++++++++++++-- .../web/error/GlobalExceptionHandler.java | 6 ++++ .../PublicVirtualStudiesIntegrationTest.java | 32 ++++++++++++++++--- 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/docs/Create-And-Publish-Virtual-Study.md b/docs/Create-And-Publish-Virtual-Study.md index 1a8cc0e2169..14ebbf9c438 100644 --- a/docs/Create-And-Publish-Virtual-Study.md +++ b/docs/Create-And-Publish-Virtual-Study.md @@ -49,7 +49,8 @@ Specifying the `pmid` enables a link to the PubMed page of the study. ## Un-publish Virtual Study To un-publish a virtual study, you need to supply the publisher API key in the `X-PUBLISHER-API-KEY` header. -After un-publishing, the virtual study is still accessible by the link, but it will be unlisted from the list of public virtual studies and not shown on the landing page anymore. +After un-publishing, virtual study will no longer be displayed in the `Public Virtual Studies` section on the landing page. +However, it reappears in the `My Virtual Studies` section for the owner. Here is the command to un-publish a virtual study: ```shell diff --git a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java index cc8fe356748..c48ac446041 100644 --- a/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java +++ b/src/main/java/org/cbioportal/web/PublicVirtualStudiesController.java @@ -24,6 +24,7 @@ import org.springframework.web.bind.annotation.RequestParam; import java.util.List; +import java.util.NoSuchElementException; import java.util.Set; @Controller @@ -72,15 +73,21 @@ public ResponseEntity publishVirtualStudy( @DeleteMapping("/{id}") @ApiResponse(responseCode = "200", description = "OK") - public ResponseEntity retractVirtualStudy( + public ResponseEntity unPublishVirtualStudy( @PathVariable String id, @RequestHeader(value = "X-PUBLISHER-API-KEY") String providedPublisherApiKey ) { ensureProvidedPublisherApiKeyCorrect(providedPublisherApiKey); - sessionServiceRequestHandler.softRemoveVirtualStudy(id); + unPublishVirtualStudy(id); return ResponseEntity.ok().build(); } + /** + * Publishes virtual study optionally updating metadata fields + * @param id - id of public virtual study to publish + * @param typeOfCancerId - if specified (not null) update type of cancer of published virtual study + * @param pmid - if specified (not null) update PubMed ID of published virtual study + */ private void publishVirtualStudy(String id, String typeOfCancerId, String pmid) { VirtualStudy virtualStudyDataToPublish = sessionServiceRequestHandler.getVirtualStudyById(id); VirtualStudyData virtualStudyData = virtualStudyDataToPublish.getData(); @@ -89,6 +96,24 @@ private void publishVirtualStudy(String id, String typeOfCancerId, String pmid) sessionServiceRequestHandler.updateVirtualStudy(virtualStudyDataToPublish); } + /** + * Un-publish virtual study + * @param id - id of public virtual study to un-publish + */ + private void unPublishVirtualStudy(String id) { + VirtualStudy virtualStudyToUnPublish = sessionServiceRequestHandler.getVirtualStudyById(id); + if (virtualStudyToUnPublish == null) { + throw new NoSuchElementException("The virtual study with id=" + id + " has not been found in the public list."); + } + VirtualStudyData virtualStudyData = virtualStudyToUnPublish.getData(); + Set users = virtualStudyData.getUsers(); + if (users == null || users.isEmpty() || !users.contains(ALL_USERS)) { + throw new NoSuchElementException("The virtual study with id=" + id + " has not been found in the public list."); + } + virtualStudyData.setUsers(Set.of(virtualStudyData.getOwner())); + sessionServiceRequestHandler.updateVirtualStudy(virtualStudyToUnPublish); + } + private void ensureProvidedPublisherApiKeyCorrect(String providedPublisherApiKey) { if (requiredPublisherApiKey.isBlank() || !requiredPublisherApiKey.equals(providedPublisherApiKey)) { diff --git a/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java b/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java index 9ddb93aaf70..1c24d60c431 100644 --- a/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java +++ b/src/main/java/org/cbioportal/web/error/GlobalExceptionHandler.java @@ -18,6 +18,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import java.util.Iterator; +import java.util.NoSuchElementException; // TODO @@ -207,4 +208,9 @@ public ResponseEntity handleBadSqlGrammar(BadSqlGrammarException HttpStatus.INTERNAL_SERVER_ERROR ); } + + @ExceptionHandler(NoSuchElementException.class) + public ResponseEntity handleNoSuchElementException(NoSuchElementException ex) { + return new ResponseEntity<>(new ErrorResponse(ex.getMessage()), HttpStatus.NOT_FOUND); + } } diff --git a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java index 01d54b41cee..e2c74996b75 100644 --- a/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java +++ b/src/test/java/org/cbioportal/test/integration/PublicVirtualStudiesIntegrationTest.java @@ -109,6 +109,16 @@ public void test2CreateVirtualStudy() { virtualStudyId = savedVs.getId(); } + @Test + public void test2_1UnPublishVirtualStudyFails() { + ResponseEntity response = restTemplate.exchange( + CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, + HttpMethod.DELETE, + new HttpEntity<>(null, validKeyContainingHeaders), + Object.class); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + } + @Test public void test3PublishVirtualStudy() { String url = CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId + "?typeOfCancerId=acc&pmid=12345"; @@ -158,28 +168,28 @@ public void test4ListJustPublishedStudy() { @Test public void test5UnpublishVirtualStudy() { - ResponseEntity response5 = restTemplate.exchange( + ResponseEntity response5 = restTemplate.exchange( CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, null, - Object.class); + Void.class); assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); response5 = restTemplate.exchange( CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, new HttpEntity<>(null, invalidKeyContainingHeaders), - Object.class); + Void.class); assertThat(response5.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); response5 = restTemplate.exchange( CBIO_URL + "/api/public_virtual_studies/" + virtualStudyId, HttpMethod.DELETE, new HttpEntity<>(null, validKeyContainingHeaders), - Object.class); + Void.class); assertThat(response5.getStatusCode().is2xxSuccessful()).isTrue(); } - + @Test public void test6NoPublicVirtualStudiesAfterRemoval() { ResponseEntity> response6 = restTemplate.exchange( @@ -192,6 +202,18 @@ public void test6NoPublicVirtualStudiesAfterRemoval() { assertThat(response6.getBody()).isEmpty(); } + @Test + public void test7UnpublishedVirtualStudyExists() { + ResponseEntity response = restTemplate.exchange( + CBIO_URL + "/api/session/virtual_study/" + virtualStudyId, + HttpMethod.GET, + null, + VirtualStudy.class); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + VirtualStudy body = response.getBody(); + assertThat(body).isNotNull(); + } + static VirtualStudyData createTestVsData() { VirtualStudyData data = new VirtualStudyData(); data.setName("test virtual study name");