fix: restore E2E stability and address regressions (#11887)

.github/workflows/integration-test.yml

@@ -0,0 +1,99 @@
+name: Integration tests
+on: [push, pull_request]
+jobs:
+  build:
+    name: Integration tests
+    runs-on: ubuntu-latest
+    env:
+      PORTAL_SOURCE_DIR: /home/runner/work/cbioportal/cbioportal/cbioportal
+      PORTAL_COMPOSE_DIR: /home/runner/work/cbioportal/cbioportal/cbioportal-docker-compose
+      PORTAL_INFO_DIR: /home/runner/work/cbioportal/cbioportal/portalInfo
+    steps:
+      - name: 'Checkout cbioportal repo'
+        uses: actions/checkout@v4
+        with:
+          path: ./cbioportal
+      - name: 'Set up JDK 21'
+        uses: oracle-actions/setup-java@v1
+        with:
+          website: oracle.com
+          release: 21
+      - name: 'Cache Maven packages'
+        uses: actions/cache@v4
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: 'Copy application.properties'
+        working-directory: ./cbioportal
+        run: |
+          cp src/main/resources/application.properties.EXAMPLE src/main/resources/application.properties
+      - name: 'Build cbioportal'
+        working-directory: ./cbioportal
+        run: |
+          echo "<settings><servers><server><id>github</id><username>${{ github.actor }}</username><password>${{ secrets.GITHUB_TOKEN }}</password></server></servers></settings>" > settings.xml
+          mvn --settings settings.xml -DskipTests clean install
+      - name: 'Checkout cbioportal-docker-compose repo'
+        uses: actions/checkout@v4
+        with:
+          repository: cbioportal/cbioportal-docker-compose
+          path: ./cbioportal-docker-compose
+      - name: 'Initialize cbioportal-docker-compose'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          cd ./data && ./init.sh && rm -rf ./studies/* && cd ../config && \
+          cat $PORTAL_SOURCE_DIR/src/main/resources/application.properties | \
+              sed 's|spring.datasource.url=.*|spring.datasource.url=jdbc:mysql://cbioportal-database:3306/cbioportal?useSSL=false|' | \
+              sed 's|spring.datasource.username=.*|spring.datasource.username=cbio_user|' | \
+              sed 's|spring.datasource.password=.*|spring.datasource.password=somepassword|' | \
+              sed 's|session.service.url=.*|session.service.url=http://cbioportal-session:5001/api/sessions/my_portal/|' | \
+              sed 's|feature.study.export=.*|feature.study.export=true|' | \
+              sed 's|#\?session.endpoint.publisher-api-key=.*|session.endpoint.publisher-api-key=TEST|' \
+              > application.properties && \
+          DB_VERSION=$(mvn -q -DforceStdout -Dexpression=db.version -f $PORTAL_SOURCE_DIR/pom.xml help:evaluate || true) && \
+          if [ -n "$DB_VERSION" ]; then echo "db.version=${DB_VERSION}" > maven.properties; else : > maven.properties; fi && \
+          echo "maven.properties => $(cat maven.properties | tr -d '\n' || echo '(empty)')"
+      - name: 'Dump Properties'
+        working-directory: ./cbioportal-docker-compose
+        run: cat config/application.properties
+      - name: 'Start cbioportal-docker-compose'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          docker compose -f docker-compose.yml -f $PORTAL_SOURCE_DIR/test/integration/docker-compose-localbuild.yml up -d
+      - name: 'Wait for cbioportal to initialize ...'
+        id: startup
+        uses: nev7n/wait_for_response@v1
+        with:
+          url: 'http://localhost:8080/api/health'
+          responseCode: 200
+          timeout: 900000
+          interval: 30000
+      - name: Log cBioPortal Service
+        if: ${{ failure() && steps.startup.conclusion == 'failure' }}
+        working-directory: ./cbioportal-docker-compose
+        run: docker compose logs cbioportal
+      - name: 'TEST - Validate and load study_es_0'
+        if: steps.startup.conclusion == 'success'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          $PORTAL_SOURCE_DIR/test/integration/test_load_study.sh
+      - name: 'TEST - Import and Export of study_es_0_import_export'
+        if: steps.startup.conclusion == 'success'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          $PORTAL_SOURCE_DIR/test/integration/test_import_export.sh
+      - name: 'TEST - Publish virtual study'
+        if: steps.startup.conclusion == 'success'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          $PORTAL_SOURCE_DIR/test/integration/test_publish_vs.sh
+      - name: 'TEST - Add OncoKB annotations to study'
+        if: steps.startup.conclusion == 'success'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          $PORTAL_SOURCE_DIR/test/integration/test_integration_test_oncokb_import.sh
+      - name: 'TEST - Update OncoKB annotations'
+        if: steps.startup.conclusion == 'success'
+        working-directory: ./cbioportal-docker-compose
+        run: |
+          $PORTAL_SOURCE_DIR/test/integration/test_update_oncokb.sh

.github/workflows/security-integration-test.yml

@@ -0,0 +1,36 @@
+name: Security integration tests
+on: [push, pull_request]
+jobs:
+  build:
+    name: Security integration tests
+    runs-on: ubuntu-latest
+    env:
+      PORTAL_SOURCE_DIR: /home/runner/work/cbioportal/cbioportal/cbioportal
+      PORTAL_COMPOSE_DIR: /home/runner/work/cbioportal/cbioportal/cbioportal-docker-compose
+      PORTAL_INFO_DIR: /home/runner/work/cbioportal/cbioportal/portalInfo
+    steps:
+      - name: 'Checkout cbioportal repo'
+        uses: actions/checkout@v4
+        with:
+          path: ./cbioportal
+      - name: 'Set up JDK 21'
+        uses: oracle-actions/setup-java@v1
+        with:
+          website: oracle.com
+          release: 21
+      - name: 'Cache Maven packages'
+        uses: actions/cache@v4
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: 'Download Chrome'
+        uses: browser-actions/setup-chrome@v1
+      - name: 'Copy Application.Properties'
+        working-directory: ./cbioportal
+        run: |
+          cp src/main/resources/application.properties.EXAMPLE src/main/resources/application.properties
+      - name: 'Run integration tests'
+        working-directory: ./cbioportal
+        run: |
+          mvn verify -Pintegration-test

.gitignore

@@ -109,4 +109,17 @@ Dockerfile.local
 security.properties
 *.crt
 *.key
-*~
+*~
+*.log
+*.txt
+build_output*
+test_output*
+run_e2e*
+hs_err*
+verify_output.txt
+.gemini/
+.agents/
+.agent/
+_agents/
+_agent/
+

pom.xml

@@ -30,7 +30,7 @@
     <frontend.artifactId>frontend-cbioportal</frontend.artifactId>
     <frontend.version>v6.4.4</frontend.version>
     <!-- THIS SHOULD BE KEPT IN SYNC TO VERSION IN CGDS.SQL -->
-    <db.version>2.14.5</db.version>
+    <db.version>2.14.6</db.version>
     <derived_table.version>1.0.7</derived_table.version>
 
     <!-- Version properties for dependencies that should have same version. -->

src/main/java/org/cbioportal/application/security/CancerStudyPermissionEvaluator.java

@@ -97,6 +97,7 @@ public class CancerStudyPermissionEvaluator implements PermissionEvaluator {
   private final String FILTER_GROUPS_BY_APP_NAME;
 
   private final String PUBLIC_CANCER_STUDIES_GROUP;
+  private final String DOWNLOAD_GROUP;
 
   //    @Value("${always_show_study_group:}")
   //    private void setPublicCancerStudiesGroup(String property) {
@@ -114,10 +115,12 @@ public CancerStudyPermissionEvaluator(
       final String appName,
       final String doFilterGroupsByAppName,
       final String alwaysShowCancerStudyGroup,
+      final String downloadGroup,
       final CacheMapUtil cacheMapUtil) {
     this.APP_NAME = appName;
     this.FILTER_GROUPS_BY_APP_NAME = doFilterGroupsByAppName;
     this.PUBLIC_CANCER_STUDIES_GROUP = alwaysShowCancerStudyGroup;
+    this.DOWNLOAD_GROUP = downloadGroup;
     this.cacheMapUtil = cacheMapUtil;
   }
 
@@ -427,6 +430,53 @@ private boolean hasAccessToCancerStudy(
 
     Set<String> grantedAuthorities = getGrantedAuthorities(authentication);
     String stableStudyID = cancerStudy.getCancerStudyIdentifier();
+
+    if (AccessLevel.DOWNLOAD == permission) {
+      if (!hasAccessToCancerStudy(authentication, cancerStudy, AccessLevel.READ)) {
+        return false;
+      }
+
+      // 1. Superuser Check (Move this to the top)
+      if (grantedAuthorities.contains(ALL_CANCER_STUDIES_ID.toUpperCase())) {
+        return true;
+      }
+
+      // 2. TCGA/TARGET Superuser checks
+      if (grantedAuthorities.contains(ALL_TCGA_CANCER_STUDIES_ID.toUpperCase())
+          && (stableStudyID != null && stableStudyID.toUpperCase().endsWith("_TCGA"))) {
+        return true;
+      }
+      if (grantedAuthorities.contains(ALL_TARGET_CANCER_STUDIES_ID.toUpperCase())
+          && (stableStudyID != null
+              && (stableStudyID.toUpperCase().endsWith("_TARGET")
+                  || stableStudyID.equalsIgnoreCase("ALL_TARGET_PHASE1")
+                  || stableStudyID.equalsIgnoreCase("ALL_TARGET_PHASE2")))) {
+        return true;
+      }
+
+      // 3. Global DOWNLOAD_GROUP check (Strict Requirement if configured)
+      if (DOWNLOAD_GROUP != null
+          && !DOWNLOAD_GROUP.isEmpty()
+          && !grantedAuthorities.contains(DOWNLOAD_GROUP.toUpperCase())) {
+        return false;
+      }
+
+      // 4. Specific Download Groups check
+      String downloadGroups = cancerStudy.getDownloadGroups();
+      if (downloadGroups != null && !downloadGroups.isEmpty()) {
+        Set<String> dGroups =
+            Arrays.stream(downloadGroups.split(";"))
+                .map(String::trim)
+                .filter(g -> !g.isEmpty())
+                .map(String::toUpperCase)
+                .collect(Collectors.toSet());
+        return !Collections.disjoint(dGroups, grantedAuthorities);
+      }
+
+      // If no study-specific download_groups are set, user can download (provided they have READ)
+      return true;
+    }
+
     if (log.isDebugEnabled()) {
       log.debug("hasAccessToCancerStudy(), cancer study stable id: " + stableStudyID);
       log.debug("hasAccessToCancerStudy(), user: " + authentication.getPrincipal().toString());
@@ -471,8 +521,12 @@ private boolean hasAccessToCancerStudy(
     // groups)
     // need to filter out empty groups, this can cause issue if grantedAuthorities and groups both
     // contain empty string
+    String cancerStudyGroups = cancerStudy.getGroups();
+    if (cancerStudyGroups == null || cancerStudyGroups.isEmpty()) {
+      return false;
+    }
     Set<String> groups =
-        Arrays.stream(cancerStudy.getGroups().split(";"))
+        Arrays.stream(cancerStudyGroups.split(";"))
             .filter(g -> !g.isEmpty())
             .collect(Collectors.toSet());
     if (!Collections.disjoint(groups, grantedAuthorities)) {

src/main/java/org/cbioportal/application/security/config/MethodSecurityConfig.java

@@ -23,9 +23,10 @@ public CancerStudyPermissionEvaluator cancerStudyPermissionEvaluator(
       @Value("${app.name:}") String appName,
       @Value("${filter_groups_by_appname:true}") String doFilterGroupsByAppName,
       @Value("${always_show_study_group:}") String alwaysShowCancerStudyGroup,
+      @Value("${download_group:}") String downloadGroup,
       CacheMapUtil cacheMapUtil) {
     return new CancerStudyPermissionEvaluator(
-        appName, doFilterGroupsByAppName, alwaysShowCancerStudyGroup, cacheMapUtil);
+        appName, doFilterGroupsByAppName, alwaysShowCancerStudyGroup, downloadGroup, cacheMapUtil);
   }
 
   @Bean

src/main/java/org/cbioportal/legacy/model/CancerStudy.java

@@ -16,6 +16,7 @@ public class CancerStudy implements ReadPermission, Serializable {
   private String pmid;
   private String citation;
   private String groups;
+  private String downloadGroups;
   private Integer status;
   private Date importDate;
   private TypeOfCancer typeOfCancer;
@@ -108,6 +109,14 @@ public void setGroups(String groups) {
     this.groups = groups;
   }
 
+  public String getDownloadGroups() {
+    return downloadGroups;
+  }
+
+  public void setDownloadGroups(String downloadGroups) {
+    this.downloadGroups = downloadGroups;
+  }
+
   public Integer getStatus() {
     return status;
   }

src/main/java/org/cbioportal/legacy/service/impl/NamespaceAttributeServiceImpl.java

@@ -1,6 +1,8 @@
 package org.cbioportal.legacy.service.impl;
 
+import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Stream;
 import org.cbioportal.legacy.model.NamespaceAttribute;
 import org.cbioportal.legacy.model.NamespaceAttributeCount;
 import org.cbioportal.legacy.persistence.NamespaceRepository;
@@ -20,16 +22,25 @@ public NamespaceAttributeServiceImpl(NamespaceRepository namespaceRepository) {
 
   @Override
   public List<NamespaceAttribute> fetchNamespaceAttributes(List<String> studyIds) {
+    if (studyIds == null || studyIds.isEmpty()) {
+      return new ArrayList<>();
+    }
 
     List<NamespaceAttribute> outerNamespaceKeys =
         namespaceRepository.getNamespaceOuterKey(studyIds);
 
+    if (outerNamespaceKeys == null) {
+      return new ArrayList<>();
+    }
+
     return outerNamespaceKeys.stream()
         .flatMap(
-            outerNamespaceKey ->
-                namespaceRepository
-                    .getNamespaceInnerKey(outerNamespaceKey.getOuterKey(), studyIds)
-                    .stream())
+            outerNamespaceKey -> {
+              List<NamespaceAttribute> innerKeys =
+                  namespaceRepository.getNamespaceInnerKey(
+                      outerNamespaceKey.getOuterKey(), studyIds);
+              return innerKeys == null ? Stream.empty() : innerKeys.stream();
+            })
         .toList();
   }
 

src/main/java/org/cbioportal/legacy/utils/security/AccessLevel.java

@@ -2,5 +2,6 @@
 
 public enum AccessLevel {
   READ,
-  LIST
+  LIST,
+  DOWNLOAD
 }

src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java

@@ -885,13 +885,14 @@ private boolean extractAttributesFromMolecularProfileCasesGroupsAndAlterationTyp
           molecularProfileCasesGroupFilters);
       request.setAttribute(
           "interceptedMolecularProfileCasesGroupFilters", molecularProfileCasesGroupFilters);
-      if (molecularProfileCasesAndAlterationTypesGroupFilters.getAlterationEventTypes() != null) {
-        AlterationFilter alterationEnrichmentEventTypes =
-            molecularProfileCasesAndAlterationTypesGroupFilters.getAlterationEventTypes();
-        LOG.debug("extracted alterationEventTypes: {}", alterationEnrichmentEventTypes);
-        LOG.debug("setting alterationEventTypes to {}", alterationEnrichmentEventTypes);
-        request.setAttribute("alterationEventTypes", alterationEnrichmentEventTypes);
+      AlterationFilter alterationEnrichmentEventTypes =
+          molecularProfileCasesAndAlterationTypesGroupFilters.getAlterationEventTypes();
+      if (alterationEnrichmentEventTypes == null) {
+        alterationEnrichmentEventTypes = new AlterationFilter();
       }
+      LOG.debug("extracted alterationEventTypes: {}", alterationEnrichmentEventTypes);
+      LOG.debug("setting alterationEventTypes to {}", alterationEnrichmentEventTypes);
+      request.setAttribute("alterationEventTypes", alterationEnrichmentEventTypes);
       if (cacheMapUtil.hasCacheEnabled()) {
         Collection<String> cancerStudyIdCollection =
             extractCancerStudyIdsFromMolecularProfileCasesGroups(molecularProfileCasesGroupFilters);

src/main/resources/db-scripts/cgds.sql

@@ -132,6 +132,7 @@ CREATE TABLE `cancer_study` (
   `PMID` varchar(1024) DEFAULT NULL,
   `CITATION` varchar(200) DEFAULT NULL,
   `GROUPS` varchar(200) DEFAULT NULL,
+  `DOWNLOAD_GROUPS` varchar(200) DEFAULT NULL,
   `STATUS` int(1) DEFAULT NULL,
   `IMPORT_DATE` datetime DEFAULT NULL,
   `REFERENCE_GENOME_ID` int(4) DEFAULT 1,
@@ -744,4 +745,4 @@ CREATE TABLE `resource_study` (
 
 -- DB_SCHEMA_VERSION AND DERIVED_TABLE_SCHEMA_VERSION MUST BE KEPT IN SYNC WITH THE db.version AND derived_table.version PROPERTIES IN pom.xml
 INSERT INTO `info` (`DB_SCHEMA_VERSION`, `GENESET_VERSION`, `DERIVED_TABLE_SCHEMA_VERSION`, `GENE_TABLE_VERSION`)
-  VALUES ('2.14.5', NULL, '1.0.7', NULL);
+  VALUES ('2.14.6', NULL, '1.0.7', NULL);

src/main/resources/db-scripts/migration.sql

@@ -1089,3 +1089,8 @@ UPDATE `info` SET `DB_SCHEMA_VERSION`="2.14.4";
 ALTER TABLE `info` ADD COLUMN `GENE_TABLE_VERSION` varchar(24);
 UPDATE `info` SET `DERIVED_TABLE_SCHEMA_VERSION`="1.0.7";
 UPDATE `info` SET `DB_SCHEMA_VERSION`="2.14.5";
+
+##version: 2.14.6
+-- Add separate download permissions support for studies
+ALTER TABLE `cancer_study` ADD COLUMN `DOWNLOAD_GROUPS` varchar(200) DEFAULT NULL;
+UPDATE `info` SET `DB_SCHEMA_VERSION`="2.14.6";

src/main/resources/org/cbioportal/legacy/persistence/mybatis/SecurityMapper.xml

@@ -8,7 +8,7 @@
     <resultMap id="userAuthoritiesResultMap" type="org.cbioportal.legacy.model.UserAuthorities">
         <result property="email" column="email"/>
         <collection property="authorities" ofType="String">
-            <result property="authority" column="authority"/>
+            <result column="authority"/>
         </collection>
     </resultMap>