Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
85b47c2
Update initial Mappers to fix Patient View Page
haynescd Jan 6, 2025
fd83576
Fix queries to support cbioportal with clickhouse only
haynescd Jan 10, 2025
32f527c
Fix mybatis xml
haynescd Jun 2, 2025
611aa91
Remove GeneMemoizer call
haynescd Jun 3, 2025
de029ad
Optimize PatientMapper IN clauses for ClickHouse JDBC performance (#1…
alisman Aug 21, 2025
000498c
improve molecular data query (#11669)
onursumer Sep 2, 2025
c284181
add back clinical data mapper queries for clinical table (#11700)
gblaih Sep 10, 2025
3fd4662
Optimize ClinicalAttributeMapper sample ID queries for ClickHouse JDB…
alisman Sep 11, 2025
688a552
Comprehensive ArrayTypeHandler optimization for ClickHouse JDBC perfo…
alisman Sep 11, 2025
94aaee9
Fix capitalization of field names in sql
alisman Sep 17, 2025
6189f4f
make sure treatment mapper string comparisons are case-insensitive (#…
onursumer Sep 25, 2025
e5fdccf
fix broken resource data endpoints (#11719)
onursumer Sep 25, 2025
c90509b
improve generic_assay_data sql performance (#11706)
onursumer Sep 25, 2025
528230a
Refactor GenePanelServiceImpl to use batch repository call
alisman Sep 24, 2025
16ade59
Refactor getMutationsInMultipleMolecularProfiles to use single query
alisman Sep 25, 2025
9768d62
Refactor fetchStructuralVariants to use single query
alisman Sep 26, 2025
2503494
Fix spotless issues
alisman Sep 26, 2025
aa133a8
fix broken getPatientClinicalDataFromStudyViewFilter mapping (#11710)
onursumer Sep 19, 2025
4fb4d04
fix high security vulnerabilities (#11717)
gblaih Sep 22, 2025
2e5d568
Refactor Clickhouse enrichments endpoint to avoid redundant counting …
alisman Sep 26, 2025
72583b3
Optimize discrete copy number queries by reordering JOINs (#11740)
alisman Oct 10, 2025
b764fe6
Add studyExists method and fix ClickHouse GROUP BY issue (#11746)
alisman Oct 10, 2025
d89e0f0
truncate genetic alteration values for molecular data queries (#11734)
onursumer Oct 10, 2025
bfd1299
In the molecular data truncation SQL, add case for zero values (#11751)
alisman Oct 14, 2025
c2e4117
Fix column store study endpoint (#11747)
haynescd Oct 15, 2025
9387415
Fix sample list error (clickhouse-only) (#11753)
dippindots Oct 17, 2025
2ba9202
Fix legacy core tests after changes to legacy mapper for clickhouse-o…
onursumer Oct 30, 2025
95cb175
Use mutation_derived table in mutation mapper to avoid joins (#11786)
alisman Oct 31, 2025
84a22ab
Merge remote-tracking branch 'upstream/master' into demo-clickhouse-o…
alisman Oct 31, 2025
2164b45
Fix upper case column issues in merged code
alisman Oct 31, 2025
d2e6aa4
fix a potential null pointer exception with the VariantCountMyBatisRe…
onursumer Nov 2, 2025
536b22f
Rename combineStudyAndPatientIds to combineStudyAndEntityIds (#11780)
alisman Nov 6, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<frontend.version>v6.4.1</frontend.version>
<!-- THIS SHOULD BE KEPT IN SYNC TO VERSION IN CGDS.SQL -->
<db.version>2.14.5</db.version>
<derived_table.version>1.0.2</derived_table.version>
<derived_table.version>1.0.3</derived_table.version>

<!-- Version properties for dependencies that should have same version. -->
<!-- The rest can be set in the dependencyManagement section -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,10 @@ public interface CancerStudyMetadataMapper {
@Mapping(target = "importDate", source = "importDate", dateFormat = "yyyy-MM-dd HH:mm:ss")
@Mapping(target = "studyId", source = "cancerStudyIdentifier")
@Mapping(target = "cancerTypeId", source = "typeOfCancerId")
@Mapping(target = "cancerType", source = "typeOfCancer")
// TODO: ReadPermission needs to be implemented
@Mapping(target = "readPermission", source = "publicStudy")
CancerStudyMetadataDTO toDto(CancerStudyMetadata cancerStudyMetadata);

@Mapping(target = "importDate", source = "importDate", dateFormat = "yyyy-MM-dd HH:mm:ss")
@Mapping(target = "studyId", source = "cancerStudyIdentifier")
@Mapping(target = "cancerTypeId", source = "typeOfCancerId")
// TODO: ReadPermission needs to be implemented
@Mapping(target = "readPermission", source = "publicStudy")
List<CancerStudyMetadataDTO> toDtos(List<CancerStudyMetadata> cancerStudyMetadataList);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ public record CancerStudyMetadataDTO(
String referenceGenome,
Integer treatmentCount,
Integer structuralVariantCount,
TypeOfCancer typeOfCancer,
TypeOfCancer cancerType,
Boolean readPermission) {}

Large diffs are not rendered by default.

48 changes: 48 additions & 0 deletions src/main/java/org/cbioportal/application/security/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# 🛡️ Study Permission Evaluator

This class is an implementation of the **Spring Security `PermissionEvaluator`** interface. Its purpose is to enforce fine-grained access control across the application's data model, ensuring that authenticated users have the required `AccessLevel` to interact with specific cancer research entities.

Access control is centrally determined at the **`CancerStudy`** level. All other related entities (profiles, lists, patients) inherit their access policy from their associated study.

***

## Core Functionality

The evaluator implements two primary permission checking methods:

### 1. Object-Based Permission Check

| Method | `hasPermission(Authentication, Object targetDomainObject, Object permission)` |
| :--- | :--- |
| **Purpose** | Checks permission on a **single, loaded domain object instance** (e.g., a `MolecularProfile` object). |
| **Process** | It uses `extractCancerStudy()` to resolve the target object back to its parent `CancerStudy` and delegates the final access check to a business logic method (`hasAccessToCancerStudy`). |

### 2. ID-Based Permission Check

| Method | `hasPermission(Authentication, Serializable targetId, String targetType, Object permission)` |
| :--- | :--- |
| **Purpose** | Checks permission on resources identified by an **ID, a collection of IDs, or a filter object**. |
| **Process** | It resolves the IDs/filter into a collection of unique `CancerStudy` objects. It then enforces an **all-or-nothing** policy: access is granted only if the user has permission to **every single** associated `CancerStudy`. |

***

## Supported Target Types

The evaluator is designed to handle access checks for objects and identifiers spanning the core data model:

| Target Object/ID Type | Description |
| :--- | :--- |
| `CancerStudy` | Direct object or ID. The root entity for access control. |
| `MolecularProfile` | Object or ID. Access is determined by its parent study. |
| `SampleList` | Object or ID. Access is determined by its parent study. |
| `Patient` | Direct object. Access is determined by its parent study. |
| `Collection<...Ids>` | A bulk check on a collection of any supported ID type. |
| `*Filter` Objects | Various application-specific filter classes (e.g., `SampleFilter`, `StudyViewFilter`) that implicitly reference a set of `CancerStudy` IDs. |

***

## Dependencies

* **`Authentication`**: The standard Spring Security principal representing the logged-in user.
* **`cacheMapUtil`**: A utility used for high-performance retrieval of domain objects (like `CancerStudy`, `MolecularProfile`) by their string identifiers, crucial for efficient ID-based permission checks.
* **`AccessLevel`**: An external enumeration (cast from the `permission` object) defining the specific level of access requested (e.g., `READ`, `WRITE`).
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
@EnableMethodSecurity(prePostEnabled = true)
// 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}')")
"{'oauth2','saml', 'saml_plus_basic'}.contains('${authenticate}') or ('optional_oauth2' eq"
+ " '${authenticate}' and 'true' eq '${security.method_authorization_enabled}')")
public class MethodSecurityConfig {

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.cbioportal.shared.SortAndSearchCriteria;
import org.cbioportal.shared.enums.ProjectionType;
import org.springframework.context.annotation.Profile;
import org.springframework.security.access.prepost.PostFilter;
import org.springframework.stereotype.Service;

/**
Expand All @@ -25,7 +26,7 @@
* private final GetCancerStudyMetadataUseCase getCancerStudyMetadataUseCase;
*
* public CancerStudyController(GetCancerStudyMetadataUseCase getCancerStudyMetadataUseCase) {
* this.getCancerStudyMetadataUseCase = getCancerStudyMetadataUseCase;
* this.getCancerStudyMetadataUseCase = getCancerStudyMetadataUseCase;
* }
*
* // Retrieve detailed metadata for cancer studies
Expand All @@ -41,7 +42,7 @@
*/
@Service
@Profile("clickhouse")
public final class GetCancerStudyMetadataUseCase {
public class GetCancerStudyMetadataUseCase {

private final CancerStudyRepository studyRepository;

Expand Down Expand Up @@ -75,6 +76,8 @@ public GetCancerStudyMetadataUseCase(CancerStudyRepository studyRepository) {
* @see ProjectionType
* @see CancerStudyMetadata
*/
@PostFilter(
"hasPermission(filterObject, T(org.cbioportal.legacy.utils.security.AccessLevel).READ)")
public List<CancerStudyMetadata> execute(
ProjectionType projectionType, SortAndSearchCriteria sortAndSearchCriteria) {
return switch (projectionType) {
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this empty file?

Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public List<Sample> execute(
String sortBy,
String direction)
throws StudyNotFoundException {
studyService.getStudy(studyId);
studyService.studyExists(studyId);

return sampleRepository.getAllSamplesInStudy(
studyId, projection, pageSize, pageNumber, sortBy, direction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public GetMetaSamplesInStudyUseCase(
}

public BaseMeta execute(String studyId) throws StudyNotFoundException {
studyService.getStudy(studyId);
studyService.studyExists(studyId);

return sampleRepository.getMetaSamplesInStudy(studyId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public GetSampleInStudyUseCase(SampleRepository sampleRepository, StudyService s

public Sample execute(String studyId, String sampleId)
throws SampleNotFoundException, StudyNotFoundException {
studyService.getStudy(studyId);
studyService.studyExists(studyId);
Sample sample = sampleRepository.getSampleInStudy(studyId, sampleId);

if (sample == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ List<ClinicalEvent> getSamplesOfPatientsPerEventType(
List<String> studyIds, List<String> sampleIds);

List<ClinicalEvent> getPatientsDistinctClinicalEventInStudies(
List<String> studyIds, List<String> patientIds);
List<String> studyIds, List<String> patientIds, List<ClinicalEvent> clinicalEvents);

List<ClinicalEvent> getTimelineEvents(
List<String> studyIds, List<String> patientIds, List<ClinicalEvent> clinicalEvents);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static java.util.stream.Collectors.groupingBy;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -86,7 +87,8 @@ public Map<String, Set<String>> getSamplesOfPatientsPerEventTypeInStudy(
@Override
public List<ClinicalEvent> getPatientsDistinctClinicalEventInStudies(
List<String> studyIds, List<String> patientIds) {
return clinicalEventMapper.getPatientsDistinctClinicalEventInStudies(studyIds, patientIds);
return clinicalEventMapper.getPatientsDistinctClinicalEventInStudies(
studyIds, patientIds, Collections.emptyList());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public List<Geneset> getAllGenesets(String projection, Integer pageSize, Integer
projection,
pageSize,
PaginationCalculator.offset(pageSize, pageNumber),
"EXTERNAL_ID",
"external_id",
"ASC");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,25 +62,16 @@ public List<Mutation> getMutationsInMultipleMolecularProfiles(
String sortBy,
String direction) {

return molecularProfileCaseIdentifierUtil
.getGroupedCasesByMolecularProfileId(molecularProfileIds, sampleIds)
.entrySet()
.stream()
.flatMap(
entry ->
mutationMapper
.getMutationsInMultipleMolecularProfiles(
Arrays.asList(entry.getKey()),
new ArrayList<>(entry.getValue()),
entrezGeneIds,
false,
projection,
pageSize,
PaginationCalculator.offset(pageSize, pageNumber),
sortBy,
direction)
.stream())
.collect(Collectors.toList());
return mutationMapper.getMutationsInMultipleMolecularProfiles(
molecularProfileIds,
sampleIds,
entrezGeneIds,
false,
projection,
pageSize,
PaginationCalculator.offset(pageSize, pageNumber),
sortBy,
direction);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,4 @@ List<ResourceData> getResourceDataForStudy(
Integer offset,
String sortBy,
String direction);

List<ResourceData> getResourceDataForAllPatientsInStudy(
String studyId,
String resourceId,
String projection,
Integer limit,
Integer offset,
String sortBy,
String direction);

List<ResourceData> getResourceDataForAllSamplesInStudy(
String studyId,
String resourceId,
String projection,
Integer limit,
Integer offset,
String sortBy,
String direction);
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ public List<ResourceData> getResourceDataForAllPatientsInStudy(
String sortBy,
String direction) {

return resourceDataMapper.getResourceDataForAllPatientsInStudy(
return resourceDataMapper.getResourceDataOfPatientInStudy(
studyId,
null,
resourceId,
projection,
pageSize,
Expand All @@ -106,8 +107,9 @@ public List<ResourceData> getResourceDataForAllSamplesInStudy(
String sortBy,
String direction) {

return resourceDataMapper.getResourceDataForAllSamplesInStudy(
return resourceDataMapper.getResourceDataOfSampleInStudy(
studyId,
null,
resourceId,
projection,
pageSize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@

package org.cbioportal.legacy.persistence.mybatis;

import static java.util.Arrays.asList;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import org.cbioportal.legacy.model.GeneFilterQuery;
import org.cbioportal.legacy.model.StructuralVariant;
import org.cbioportal.legacy.model.StructuralVariantFilterQuery;
Expand All @@ -52,20 +49,8 @@ public List<StructuralVariant> fetchStructuralVariants(
if (molecularProfileIds == null || molecularProfileIds.isEmpty()) {
return new ArrayList<>();
}
return molecularProfileCaseIdentifierUtil
.getGroupedCasesByMolecularProfileId(molecularProfileIds, sampleIds)
.entrySet()
.stream()
.flatMap(
entry ->
structuralVariantMapper
.fetchStructuralVariants(
asList(entry.getKey()),
new ArrayList<>(entry.getValue()),
entrezGeneIds,
structuralVariantQueries)
.stream())
.collect(Collectors.toList());
return structuralVariantMapper.fetchStructuralVariants(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change was made because fetching all profiles at once is much more performant. it is always true that sampleId collection and molecular profile collection will be of equal length

molecularProfileIds, sampleIds, entrezGeneIds, structuralVariantQueries);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.cbioportal.legacy.persistence.mybatis.util;

import java.util.List;

/** Utility class for SQL operations in MyBatis mappers */
public class SqlUtils {

/**
* Combines study IDs and patient/sample IDs into unique keys for efficient array parameter usage.
* This helps optimize ClickHouse JDBC performance by reducing the number of prepared statement
* parameters.
*
* @param studyIds List of study identifiers
* @param entityIds List of patient or sample identifiers (corresponding to studyIds by index)
* @return Array of combined unique keys in format "studyId:entityId"
*/
public static String[] combineStudyAndEntityIds(List<String> studyIds, List<String> entityIds) {
if (studyIds == null || entityIds == null || studyIds.size() != entityIds.size()) {
throw new IllegalArgumentException(
"studyIds and entityIds must be non-null and have the same size");
}

String[] combinedKeys = new String[studyIds.size()];
for (int i = 0; i < studyIds.size(); i++) {
combinedKeys[i] = studyIds.get(i) + ":" + entityIds.get(i);
}

return combinedKeys;
}

/**
* Converts a List of strings to a String array for use with ArrayTypeHandler. ArrayTypeHandler
* requires Java arrays, not ArrayList objects.
*
* @param list List of strings to convert
* @return Array of strings
*/
public static String[] listToArray(List<String> list) {
if (list == null) {
return null;
}
return list.toArray(new String[0]);
}
}
2 changes: 2 additions & 0 deletions src/main/java/org/cbioportal/legacy/service/StudyService.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ List<CancerStudy> getAllStudies(
CancerStudyTags getTags(String studyId, AccessLevel accessLevel);

List<CancerStudyTags> getTagsForMultipleStudies(List<String> studyIds);

void studyExists(String studyId) throws StudyNotFoundException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public BaseMeta getMetaClinicalAttributes() {
public ClinicalAttribute getClinicalAttribute(String studyId, String clinicalAttributeId)
throws ClinicalAttributeNotFoundException, StudyNotFoundException {

studyService.getStudy(studyId);
studyService.studyExists(studyId);

ClinicalAttribute clinicalAttribute =
clinicalAttributeRepository.getClinicalAttribute(studyId, clinicalAttributeId);
Expand All @@ -72,7 +72,7 @@ public List<ClinicalAttribute> getAllClinicalAttributesInStudy(
String direction)
throws StudyNotFoundException {

studyService.getStudy(studyId);
studyService.studyExists(studyId);

return clinicalAttributeRepository.getAllClinicalAttributesInStudy(
studyId, projection, pageSize, pageNumber, sortBy, direction);
Expand All @@ -81,7 +81,7 @@ public List<ClinicalAttribute> getAllClinicalAttributesInStudy(
@Override
public BaseMeta getMetaClinicalAttributesInStudy(String studyId) throws StudyNotFoundException {

studyService.getStudy(studyId);
studyService.studyExists(studyId);

return clinicalAttributeRepository.getMetaClinicalAttributesInStudy(studyId);
}
Expand Down
Loading
Loading