From 22173093e1ce34a6fd7ee85a02ab6546b27f3468 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 16 Aug 2026 21:23:05 -0700 Subject: [PATCH 1/7] Fix security issues flagged by Claude --- .../labkey/openldapsync/ldap/LdapSyncRunner.java | 4 ++-- .../sequenceanalysis/distinctAnalysisSets.sql | 3 +++ .../SequenceAnalysis/window/AddFileSetsWindow.js | 3 ++- blast/src/org/labkey/blast/BLASTController.java | 13 ++++--------- .../pipeline/AbstractClusterExecutionEngine.java | 3 ++- .../src/org/labkey/jbrowse/JBrowseController.java | 2 +- .../pipeline/AbstractSingleCellPipelineStep.java | 4 ++-- .../org/labkey/singlecell/SingleCellController.java | 2 +- 8 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 SequenceAnalysis/resources/queries/sequenceanalysis/distinctAnalysisSets.sql diff --git a/OpenLdapSync/src/org/labkey/openldapsync/ldap/LdapSyncRunner.java b/OpenLdapSync/src/org/labkey/openldapsync/ldap/LdapSyncRunner.java index a30d76960..3324cadaf 100644 --- a/OpenLdapSync/src/org/labkey/openldapsync/ldap/LdapSyncRunner.java +++ b/OpenLdapSync/src/org/labkey/openldapsync/ldap/LdapSyncRunner.java @@ -269,16 +269,16 @@ private void setUserActive(User u, boolean active, String reason) try { log("Changing active state of user: " + u.getEmail() + " to " + active + (reason == null ? "" : ", reason: " + reason)); - _usersInactivated++; if (!_previewOnly) { UserManager.setUserActive(_settings.getLabKeyAdminUser(), u, active); } + _usersInactivated++; } catch (SecurityManager.UserManagementException e) { - _log.error("Unable to deactive user: " + u.getEmail()); + _log.error("Unable to deactivate user: " + u.getEmail(), e); } } diff --git a/SequenceAnalysis/resources/queries/sequenceanalysis/distinctAnalysisSets.sql b/SequenceAnalysis/resources/queries/sequenceanalysis/distinctAnalysisSets.sql new file mode 100644 index 000000000..ff1525341 --- /dev/null +++ b/SequenceAnalysis/resources/queries/sequenceanalysis/distinctAnalysisSets.sql @@ -0,0 +1,3 @@ +SELECT + DISTINCT rowid, name +FROM sequenceanalysis.analysisSets \ No newline at end of file diff --git a/SequenceAnalysis/resources/web/SequenceAnalysis/window/AddFileSetsWindow.js b/SequenceAnalysis/resources/web/SequenceAnalysis/window/AddFileSetsWindow.js index 842547ebd..b167b7672 100644 --- a/SequenceAnalysis/resources/web/SequenceAnalysis/window/AddFileSetsWindow.js +++ b/SequenceAnalysis/resources/web/SequenceAnalysis/window/AddFileSetsWindow.js @@ -50,7 +50,8 @@ Ext4.define('SequenceAnalysis.window.AddFileSetsWindow', { type: 'labkey-store', containerPath: Laboratory.Utils.getQueryContainerPath(), schemaName: 'laboratory', - sql: 'SELECT DISTINCT rowid, name FROM sequenceanalysis.analysisSets', + queryName: 'distinctAnalysisSets', + columns: 'rowid,name', autoLoad: true }, valueField: 'rowid', diff --git a/blast/src/org/labkey/blast/BLASTController.java b/blast/src/org/labkey/blast/BLASTController.java index 1764fd2d7..b2246d8b9 100644 --- a/blast/src/org/labkey/blast/BLASTController.java +++ b/blast/src/org/labkey/blast/BLASTController.java @@ -262,7 +262,6 @@ public String getResponse(RunBlastForm form, Map> if (files.isEmpty()) { //save query to string - AssayFileWriter writer = new AssayFileWriter(); try { FileLike targetDirectory = AssayFileWriter.ensureUploadDirectory(getContainer()); @@ -280,24 +279,20 @@ public String getResponse(RunBlastForm form, Map> } } - if (!inputFiles.isEmpty()) + for (FileLike input : inputFiles) { - for (FileLike input : inputFiles) - { - String jobId = BLASTManager.get().runBLASTN(getContainer(), getUser(), form.getDatabase(), input.toNioPathForRead().toFile(), form.getTask(), form.getTitle(), form.getSaveResults(), params, true); - resp.put("jobId", jobId); - } + String jobId = BLASTManager.get().runBLASTN(getContainer(), getUser(), form.getDatabase(), input.toNioPathForRead().toFile(), form.getTask(), form.getTitle(), form.getSaveResults(), params, true); + resp.put("jobId", jobId); } resp.put("success", true); } catch (Exception e) { - ExceptionUtil.logExceptionToMothership(getViewContext().getRequest(), e); getViewContext().getResponse().setStatus(HttpServletResponse.SC_BAD_REQUEST); logger.error(e.getMessage(), e); resp.put("success", false); - resp.put("exception", e.getMessage()); + resp.put("exception", "Error running BLASTN"); } return resp.toString(); diff --git a/cluster/src/org/labkey/cluster/pipeline/AbstractClusterExecutionEngine.java b/cluster/src/org/labkey/cluster/pipeline/AbstractClusterExecutionEngine.java index ba520f4d6..e9a24f386 100644 --- a/cluster/src/org/labkey/cluster/pipeline/AbstractClusterExecutionEngine.java +++ b/cluster/src/org/labkey/cluster/pipeline/AbstractClusterExecutionEngine.java @@ -27,6 +27,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.util.NetworkDrive; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; @@ -785,7 +786,7 @@ protected List execute(String command, @Nullable File workDir) try { - Process p = Runtime.getRuntime().exec(command, null, workDir); + Process p = new LabKeyProcessBuilder(command).directory(workDir).start(); try { String output = IOUtils.toString(p.getInputStream(), StringUtilsLabKey.DEFAULT_CHARSET); diff --git a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java index 1831d7763..040600f29 100644 --- a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java +++ b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java @@ -347,7 +347,7 @@ public static class JBrowseAction extends SimpleViewAction public ModelAndView getView(BrowserForm form, BindException errors) { String guid = form.getEffectiveSessionId(); - JBrowseSession db = isValidUUID(guid) ? new TableSelector(JBrowseSchema.getInstance().getTable(JBrowseSchema.TABLE_DATABASES), new SimpleFilter(FieldKey.fromString("objectid"), form.getEffectiveSessionId()), null).getObject(JBrowseSession.class) : null; + JBrowseSession db = isValidUUID(guid) ? new TableSelector(QueryService.get().getUserSchema(getUser(), getContainer(), JBrowseSchema.NAME).getTable(JBrowseSchema.TABLE_DATABASES), new SimpleFilter(FieldKey.fromString("objectid"), form.getEffectiveSessionId()), null).getObject(JBrowseSession.class) : null; _title = db == null ? "JBrowse" : db.getName(); form.setPageTitle(_title); diff --git a/singlecell/api-src/org/labkey/api/singlecell/pipeline/AbstractSingleCellPipelineStep.java b/singlecell/api-src/org/labkey/api/singlecell/pipeline/AbstractSingleCellPipelineStep.java index 45d161346..22e6c8923 100644 --- a/singlecell/api-src/org/labkey/api/singlecell/pipeline/AbstractSingleCellPipelineStep.java +++ b/singlecell/api-src/org/labkey/api/singlecell/pipeline/AbstractSingleCellPipelineStep.java @@ -54,7 +54,7 @@ public Output execute(SequenceOutputHandler.JobContext ctx, List { @Override From acb1613e4e98f823b6d76d874032b1f3f555f0e1 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 16 Aug 2026 21:24:33 -0700 Subject: [PATCH 2/7] More updates to address claude issues --- .../discvrcore/DiscvrCoreController.java | 79 +++++++++++-------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/discvrcore/src/org/labkey/discvrcore/DiscvrCoreController.java b/discvrcore/src/org/labkey/discvrcore/DiscvrCoreController.java index 0c2d5c865..fc5928592 100644 --- a/discvrcore/src/org/labkey/discvrcore/DiscvrCoreController.java +++ b/discvrcore/src/org/labkey/discvrcore/DiscvrCoreController.java @@ -38,6 +38,7 @@ import org.labkey.api.query.DetailsURL; import org.labkey.api.query.UserSchema; import org.labkey.api.security.RequiresPermission; +import org.labkey.api.security.RequiresSiteAdmin; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.util.DOM; import org.labkey.api.util.GUID; @@ -58,6 +59,8 @@ import java.util.Map; import java.util.TreeMap; +import static org.labkey.api.util.DOM.Attribute.name; +import static org.labkey.api.util.DOM.Attribute.type; import static org.labkey.api.util.DOM.Attribute.valign; import static org.labkey.api.util.DOM.at; import static org.labkey.api.util.DOM.cl; @@ -170,10 +173,11 @@ public ModelAndView getConfirmView(MoveWorkbookForm form, BindException errors) return new SimpleErrorView(errors); } - String sb = "This will move this workbook to the selected folder, renaming this workbook to match the series in that container. Note: there are many reasons this can be problematic, so please do this with great care

" + - ""; - - return new HtmlView(sb); + return new HtmlView(DOM.DIV( + h("This will move this workbook to the selected folder, renaming this workbook to match the series in that container. Note: there are many reasons this can be problematic, so please do this with great care"), + DOM.P(), + DOM.INPUT(at(name, "targetContainer", type, "text")) + )); } @Override @@ -210,6 +214,12 @@ public boolean handlePost(MoveWorkbookForm form, BindException errors) throws Ex return false; } + if (!target.hasPermission(getUser(),AdminPermission.class)) + { + errors.reject(ERROR_MSG, "Insufficient permissions in the target: " + form.getTargetContainer()); + return false; + } + if (ContainerManager.isSystemContainer(target)) { errors.reject(ERROR_MSG, "Cannot move to system containers: " + form.getTargetContainer()); @@ -222,33 +232,36 @@ public boolean handlePost(MoveWorkbookForm form, BindException errors) throws Ex return false; } - //NOTE: transaction causing problems for larger sites? - //try (DbScope.Transaction transaction = CoreSchema.getInstance().getSchema().getScope().ensureTransaction()) - //{ - //first rename workbook to make unique - String tempName = new GUID().toString(); - int sortOrder = (int) DbSequenceManager.get(target, ContainerManager.WORKBOOK_DBSEQUENCE_NAME).next(); - _log.info("renaming workbook to in preparation for move from: " + toMove.getPath() + " to: " + tempName); - ContainerManager.rename(toMove, getUser(), tempName); - toMove = ContainerManager.getForId(toMove.getId()); - - //then move parent - _log.info("moving workbook from: " + toMove.getPath() + " to: " + target.getPath()); - ContainerManager.move(toMove, target, getUser()); - toMove = ContainerManager.getForId(toMove.getId()); - - //finally move to correct name - _log.info("renaming workbook from: " + toMove.getPath() + " to: " + sortOrder); - ContainerManager.rename(toMove, getUser(), String.valueOf(sortOrder)); - toMove.setSortOrder(sortOrder); - new SqlExecutor(CoreSchema.getInstance().getSchema()).execute("UPDATE core.containers SET SortOrder = ? WHERE EntityId = ?", toMove.getSortOrder(), toMove.getId()); - toMove = ContainerManager.getForId(toMove.getId()); - - //transaction.commit(); - _log.info("workbook move finished"); - - _movedWb = toMove; - //} + //NOTE: This does not use a transaction because this has been problematic in larger sites: + try + { + //first rename workbook to make unique + String tempName = new GUID().toString(); + int sortOrder = (int) DbSequenceManager.get(target, ContainerManager.WORKBOOK_DBSEQUENCE_NAME).next(); + _log.info("renaming workbook to in preparation for move from: " + toMove.getPath() + " to: " + tempName); + ContainerManager.rename(toMove, getUser(), tempName); + toMove = ContainerManager.getForId(toMove.getId()); + + //then move parent + _log.info("moving workbook from: " + toMove.getPath() + " to: " + target.getPath()); + ContainerManager.move(toMove, target, getUser()); + toMove = ContainerManager.getForId(toMove.getId()); + + //finally move to correct name + _log.info("renaming workbook from: " + toMove.getPath() + " to: " + sortOrder); + ContainerManager.rename(toMove, getUser(), String.valueOf(sortOrder)); + toMove.setSortOrder(sortOrder); + new SqlExecutor(CoreSchema.getInstance().getSchema()).execute("UPDATE core.containers SET SortOrder = ? WHERE EntityId = ?", toMove.getSortOrder(), toMove.getId()); + toMove = ContainerManager.getForId(toMove.getId()); + + //transaction.commit(); + _log.info("workbook move finished"); + _movedWb = toMove; + } + catch (Exception e) + { + _log.error("Unable to move workbook. This was left in an incomplete state!: " + form.getTargetContainer(), e); + } return true; } @@ -303,8 +316,8 @@ public ActionURL getRedirectURL(Object o) } } - @UtilityAction(label = "Add Custom Core.Container Indexes", description = "Provides a mechanism to truncate the query and dataset audit tables for a container") - @RequiresPermission(AdminPermission.class) + @UtilityAction(label = "Add Custom Core.Container Indexes", description = "This adds additional indexes to core.container") + @RequiresSiteAdmin public static class AddCustomIndexesAction extends ConfirmAction { @Override From c2a3d5338d678e7f3b5778092c13389d5ba121bc Mon Sep 17 00:00:00 2001 From: bbimber Date: Mon, 17 Aug 2026 09:31:58 -0700 Subject: [PATCH 3/7] More updates to address claude issues --- .../sequenceanalysis/SequenceOutputFile.java | 30 ++++++ .../SequenceAnalysisController.java | 99 ++++++++++++------- .../api/studies/study/StudyDefinition.java | 6 +- .../org/labkey/studies/StudiesManager.java | 24 ++++- blast/src/org/labkey/blast/BLASTManager.java | 7 +- .../org/labkey/cluster/ClusterController.java | 12 +++ .../org/labkey/jbrowse/JBrowseController.java | 10 +- .../org/labkey/jbrowse/JBrowseFieldUtils.java | 4 +- .../labkey/jbrowse/JBrowseLuceneSearch.java | 4 +- .../labkey/jbrowse/model/JBrowseSession.java | 25 +++++ .../singlecell/SingleCellController.java | 4 +- 11 files changed, 171 insertions(+), 54 deletions(-) diff --git a/SequenceAnalysis/api-src/org/labkey/api/sequenceanalysis/SequenceOutputFile.java b/SequenceAnalysis/api-src/org/labkey/api/sequenceanalysis/SequenceOutputFile.java index f366fb6b7..6c4731f30 100644 --- a/SequenceAnalysis/api-src/org/labkey/api/sequenceanalysis/SequenceOutputFile.java +++ b/SequenceAnalysis/api-src/org/labkey/api/sequenceanalysis/SequenceOutputFile.java @@ -16,6 +16,7 @@ package org.labkey.api.sequenceanalysis; import com.fasterxml.jackson.annotation.JsonIgnore; +import org.apache.logging.log4j.Logger; import org.json.JSONObject; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; @@ -24,6 +25,11 @@ import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.pipeline.PipelineJobService; +import org.labkey.api.security.User; +import org.labkey.api.security.permissions.Permission; +import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.UnauthorizedException; import java.io.File; import java.io.Serializable; @@ -34,6 +40,8 @@ */ public class SequenceOutputFile implements Serializable { + private static final Logger _log = LogHelper.getLogger(SequenceOutputFile.class, "Messages related to SequenceOutputFile"); + private Integer _rowid; private String _name; private String _description; @@ -211,6 +219,28 @@ public void setModified(Date modified) _modified = modified; } + public static SequenceOutputFile getForId(Integer rowId, User u) + { + return getForId(rowId, u, ReadPermission.class); + } + + public static SequenceOutputFile getForId(Integer rowId, User u, Class perm) + { + SequenceOutputFile so = getForId(rowId); + if (so.getContainerObj() == null) + { + _log.error("SequenceOutputFile lacks a valid container: " + rowId); + return null; + } + + if (!so.getContainerObj().hasPermission(u, perm)) + { + throw new UnauthorizedException("Insufficient permissions: " + rowId); + } + + return so; + } + public static SequenceOutputFile getForId(Integer rowId) { if (PipelineJobService.get().getLocationType() != PipelineJobService.LocationType.WebServer) diff --git a/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java b/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java index 3f74c0df3..14b8b2beb 100644 --- a/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java +++ b/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java @@ -101,6 +101,7 @@ import org.labkey.api.security.IgnoresTermsOfUse; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.RequiresSiteAdmin; +import org.labkey.api.security.User; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.InsertPermission; @@ -125,6 +126,7 @@ import org.labkey.api.util.FileType; import org.labkey.api.util.FileUtil; import org.labkey.api.util.HtmlString; +import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.JsonUtil; import org.labkey.api.util.NetworkDrive; import org.labkey.api.util.PageFlowUtil; @@ -578,7 +580,7 @@ public void validateCommand(DeleteForm form, Errors errors) } } - private void findAnalysesToDelete(Collection keys, StringBuilder msg, Set outputFileIds, Set expRunsToDelete) + private void findAnalysesToDelete(Collection keys, HtmlStringBuilder msg, Set outputFileIds, Set expRunsToDelete) { appendTotal(msg, SequenceAnalysisSchema.TABLE_ALIGNMENT_SUMMARY, "Alignment Records", keys, "analysis_id", "rowid"); outputFileIds.addAll(appendTotal(msg, SequenceAnalysisSchema.TABLE_OUTPUTFILES, "Output Files", keys, "analysis_id", "rowid")); @@ -606,16 +608,16 @@ public ModelAndView getConfirmView(DeleteForm form, BindException errors) throws Set analysisIds = new IntHashSet(); Set outputFileIds = new IntHashSet(); - StringBuilder msg = new StringBuilder("Are you sure you want to delete the following " + keys.size() + " "); + HtmlStringBuilder msg = HtmlStringBuilder.of("Are you sure you want to delete the following " + keys.size() + " "); if (SequenceAnalysisSchema.TABLE_ANALYSES.equals(_table.getName())) { - msg.append("analyses: " + StringUtils.join(keys, ", ") + "? This will delete the analyses, plus all associated data. This includes:
"); + msg.append("analyses: " + StringUtils.join(keys, ", ") + "? This will delete the analyses, plus all associated data. This includes:").unsafeAppend("
"); analysisIds.addAll(keys); findAnalysesToDelete(keys, msg, outputFileIds, expRunsToDelete); } else if (SequenceAnalysisSchema.TABLE_READSETS.equals(_table.getName())) { - msg.append("readsets: " + StringUtils.join(keys, ", ") + "? This will delete the readsets, plus all associated data. This includes:
"); + msg.append("readsets: " + StringUtils.join(keys, ", ") + "? This will delete the readsets, plus all associated data. This includes:").unsafeAppend("
"); readsetIds.addAll(keys); readDataIds.addAll(appendTotal(msg, SequenceAnalysisSchema.TABLE_READ_DATA, "Sequence File Records", keys, "readset", "rowid")); analysisIds.addAll(appendTotal(msg, SequenceAnalysisSchema.TABLE_ANALYSES, "Analyses", keys, "readset", "rowid")); @@ -630,7 +632,7 @@ else if (SequenceAnalysisSchema.TABLE_READSETS.equals(_table.getName())) } else if (SequenceAnalysisSchema.TABLE_REF_NT_SEQUENCES.equals(_table.getName())) { - msg.append("NT reference sequences: " + StringUtils.join(keys, ", ") + "? This will delete the reference sequences, plus all associated data. This includes:
"); + msg.append("NT reference sequences: " + StringUtils.join(keys, ", ") + "? This will delete the reference sequences, plus all associated data. This includes:").unsafeAppend("
"); appendTotal(msg, SequenceAnalysisSchema.TABLE_REF_AA_SEQUENCES, "Reference AA Sequences", keys, "ref_nt_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_NT_FEATURES, "NT Features", keys, "ref_nt_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_COVERAGE, "Coverage Records", keys, "ref_nt_id", "rowid"); @@ -641,14 +643,14 @@ else if (SequenceAnalysisSchema.TABLE_REF_NT_SEQUENCES.equals(_table.getName())) } else if (SequenceAnalysisSchema.TABLE_REF_AA_SEQUENCES.equals(_table.getName())) { - msg.append("AA reference sequences: " + StringUtils.join(keys, ", ") + "? This will delete the reference sequences, plus all associated data. This includes:
"); + msg.append("AA reference sequences: " + StringUtils.join(keys, ", ") + "? This will delete the reference sequences, plus all associated data. This includes:").unsafeAppend("
"); appendTotal(msg, SequenceAnalysisSchema.TABLE_AA_FEATURES, "AA features", keys, "ref_aa_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_DRUG_RESISTANCE, "drug resistance mutations", keys, "ref_aa_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_AA_SNP_BY_CODON, "AA SNP Records", keys, "ref_aa_id", "rowid"); } else if (SequenceAnalysisSchema.TABLE_REF_LIBRARIES.equals(_table.getName())) { - msg.append("Reference genomes: " + StringUtils.join(keys, ", ") + "? This will delete the reference genomes, plus all associated data. This includes:
"); + msg.append("Reference genomes: " + StringUtils.join(keys, ", ") + "? This will delete the reference genomes, plus all associated data. This includes:").unsafeAppend("
"); appendTotal(msg, SequenceAnalysisSchema.TABLE_REF_LIBRARY_MEMBERS, "genome sequences", keys, "library_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_LIBRARY_TRACKS, "tracks", keys, "library_id", "rowid"); appendTotal(msg, SequenceAnalysisSchema.TABLE_CHAIN_FILES, "chain files from this genome", keys, "genomeId1", "rowid"); @@ -657,7 +659,7 @@ else if (SequenceAnalysisSchema.TABLE_REF_LIBRARIES.equals(_table.getName())) } else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) { - msg.append("output files: " + StringUtils.join(keys, ", ") + "?
"); + msg.append("output files: " + StringUtils.join(keys, ", ") + "?").unsafeAppend("
"); outputFileIds.addAll(keys); //we will delete these expDatas, so find any analysis records matching this file @@ -667,7 +669,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) if (!additionalAnalysisIds.isEmpty()) { msg.append("

"); - msg.append("The following " + additionalAnalysisIds.size() + " analyses will also be deleted, along with these associated records/files:
"); + msg.append("The following " + additionalAnalysisIds.size() + " analyses will also be deleted, along with these associated records/files:").unsafeAppend("
"); findAnalysesToDelete(additionalAnalysisIds, msg, outputFileIds, expRunsToDelete); } @@ -690,8 +692,8 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) if (!expRunsToDelete.isEmpty()) { - msg.append("

"); - msg.append("The following pipeline jobs appear to be unused, and will be deleted. Be aware, this will delete all files as well:

"); + msg.unsafeAppend("

"); + msg.append("The following pipeline jobs appear to be unused, and will be deleted. Be aware, this will delete all files as well:").unsafeAppend("

"); for (Integer runId : expRunsToDelete) { if (runId == null) @@ -703,7 +705,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) ExpRun run = ExperimentService.get().getExpRun(runId); if (run != null) { - msg.append("Pipeline run: " + run.getName() + "
"); + msg.append("Pipeline run: " + run.getName()).unsafeAppend("
"); if (run.getJobId() != null) { PipelineStatusFile sf = PipelineService.get().getStatusFile(run.getJobId()); @@ -712,7 +714,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) File target = new File(sf.getFilePath()).getParentFile(); String relativePath = FileUtil.relativize(PipelineService.get().getPipelineRootSetting(run.getContainer()).getRootPath(), target, false); ActionURL url = PageFlowUtil.urlProvider(PipelineUrls.class).urlBrowse(sf.lookupContainer(), getViewContext().getActionURL(), relativePath); - msg.append("Folder: " + target.getPath() + "
"); + msg.append("Folder: ").unsafeAppend("" + h(target.getPath()) + "
"); } } msg.append("
"); @@ -724,7 +726,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) setTitle("Delete Sequence Records"); - return new HtmlView(msg.toString()); + return new HtmlView(msg); } @Override @@ -786,12 +788,12 @@ private Set getExpRunIds(String tableName, Collection keys, St return new HashSet<>(ts.getArrayList(Integer.class)); } - private Set appendTotal(StringBuilder sb, String tableName, String noun, Collection keys, String filterCol, String pkCol) + private Set appendTotal(HtmlStringBuilder sb, String tableName, String noun, Collection keys, String filterCol, String pkCol) { SimpleFilter filter = new SimpleFilter(FieldKey.fromString(filterCol), keys, CompareType.IN); TableSelector ts = new TableSelector(SequenceAnalysisSchema.getInstance().getSchema().getTable(tableName), PageFlowUtil.set(pkCol), filter, null); Set total = new IntHashSet(ts.getArrayList(Integer.class)); - sb.append("
" + total.size() + " " + noun); + sb.unsafeAppend("
").append(total.size() + " " + noun); return total; } @@ -1043,9 +1045,18 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors) { ExpData data = ExperimentService.get().getExpData(id); if (data != null) + { + if (!data.getContainer().hasPermission(getUser(), ReadPermission.class)) + { + throw new UnauthorizedException("Insufficient permissions to read: " + id); + } + datas.add(data); + } else + { errorsList.add("Unable to find file with ExpData Id: " + id); + } } } @@ -1053,7 +1064,6 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors) { //TODO: consider proper container?? PipeRoot root = PipelineService.get().findPipelineRoot(getContainer()); - if (null == root) { throw new PipelineJobException("Unable to find pipeline root for container: " + getContainer().getPath()); @@ -1088,6 +1098,7 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors) errorsList.add("File has a duplicate basename: " + basename); map.put("error", "File has a duplicate basename: " + basename); } + distinctBasenames.add(basename); if (!f.exists()) { @@ -1130,6 +1141,7 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors) errorsList.add("File has a duplicate basename: " + basename); map.put("error", "File has a duplicate basename: " + basename); } + distinctBasenames.add(basename); if (!d.getFile().exists()) { @@ -1229,20 +1241,22 @@ public ApiResponse execute(AnalyzeBamForm form, BindException errors) throws Exc return null; } - Logger log = LogManager.getLogger(SequenceAnalysisController.class); - for (AnalysisModel m : models) { File inputFile = m.getAlignmentData().getFile(); File refFile = m.getReferenceLibraryData(getUser()).getFile(); Container c = ContainerManager.getForId(m.getContainer()); + if (!c.hasPermission(getUser(), ReadPermission.class)) + { + throw new UnauthorizedException("Insufficient permissions to read: " + m.getRowId()); + } - log.info("Calculating avg quality scores"); - AvgBaseQualityAggregator avg = new AvgBaseQualityAggregator(log, inputFile, refFile); + _log.info("Calculating avg quality scores"); + AvgBaseQualityAggregator avg = new AvgBaseQualityAggregator(_log, inputFile, refFile); if (form.getRefName() != null && form.getStart() > 0 && form.getStop() > 0) { - log.info("Using reference: " + form.getRefName() + " and the interval: " + form.getStart() + " / " + form.getStop()); + _log.info("Using reference: " + form.getRefName() + " and the interval: " + form.getStart() + " / " + form.getStop()); avg.calculateAvgQuals(form.getRefName(), form.getStart(), form.getStop()); } else @@ -1250,10 +1264,10 @@ public ApiResponse execute(AnalyzeBamForm form, BindException errors) throws Exc avg.calculateAvgQuals(); } - log.info("\tCalculation complete"); + _log.info("\tCalculation complete"); - log.info("Inspecting alignments in BAM"); - BamIterator bi = new BamIterator(inputFile, refFile, log); + _log.info("Inspecting alignments in BAM"); + BamIterator bi = new BamIterator(inputFile, refFile, _log); //NOTE: this is a hack for testing purposes. need a registry mechanism List aggregators = new ArrayList<>(); @@ -1271,15 +1285,15 @@ public ApiResponse execute(AnalyzeBamForm form, BindException errors) throws Exc if (aggregatorTypes.contains("coverage")) { - coverage = new NtCoverageAggregator(log, refFile, avg, params); + coverage = new NtCoverageAggregator(_log, refFile, avg, params); aggregators.add(coverage); } if (aggregatorTypes.contains("ntbypos")) { - NtSnpByPosAggregator ntSnp = new NtSnpByPosAggregator(log, refFile, avg, params); + NtSnpByPosAggregator ntSnp = new NtSnpByPosAggregator(_log, refFile, avg, params); if (coverage == null) - coverage = new NtCoverageAggregator(log, refFile, avg, params); + coverage = new NtCoverageAggregator(_log, refFile, avg, params); ntSnp.setCoverageAggregator(coverage, aggregatorTypes.contains("coverage")); aggregators.add(ntSnp); @@ -1287,9 +1301,9 @@ public ApiResponse execute(AnalyzeBamForm form, BindException errors) throws Exc if (aggregatorTypes.contains("aabycodon")) { - AASnpByCodonAggregator aaCodon = new AASnpByCodonAggregator(log, refFile, avg, params); + AASnpByCodonAggregator aaCodon = new AASnpByCodonAggregator(_log, refFile, avg, params); if (coverage == null) - coverage = new NtCoverageAggregator(log, refFile, avg, params); + coverage = new NtCoverageAggregator(_log, refFile, avg, params); aaCodon.setCoverageAggregator(coverage, aggregatorTypes.contains("coverage")); aggregators.add(aaCodon); @@ -1798,13 +1812,13 @@ public ApiResponse execute(AnalyzeForm form, BindException errors) throws Except jobs.addAll(AlignmentAnalysisJob.createForAnalyses(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getAnalysisIds(), form.isSubmitJobToReadsetContainer())); break; case readsetImport: - jobs.addAll(ReadsetImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr))); + jobs.addAll(ReadsetImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr, getUser()))); break; case illuminaImport: - jobs.addAll(IlluminaImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr))); + jobs.addAll(IlluminaImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr, getUser()))); break; case alignmentImport: - jobs.addAll(AlignmentImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr))); + jobs.addAll(AlignmentImportJob.create(getContainer(), getUser(), form.getJobName(), form.getDescription(), form.getJobParameters(), form.getFiles(pr, getUser()))); break; default: throw new PipelineJobException("Unknown analysis type: " + form.getType()); @@ -1877,7 +1891,7 @@ public boolean isSubmitJobToReadsetContainer() return getJobParameters() != null && getJobParameters().optBoolean("submitJobToReadsetContainer", false); } - public List getFiles(PipeRoot pr) throws PipelineValidationException + public List getFiles(PipeRoot pr, User u) throws PipelineValidationException { if (getJobParameters() == null || getJobParameters().get("inputFiles") == null) { @@ -1900,6 +1914,10 @@ else if (!d.getFile().exists()) { throw new PipelineValidationException("Missing file for data: " + o.get("dataId")); } + else if (d.getContainer().hasPermission(u, ReadPermission.class)) + { + throw new UnauthorizedException("You do not have permission to read data: " + o.get("dataId")); + } ret.add(d.getFileLike()); } @@ -3223,7 +3241,7 @@ public ApiResponse execute(CheckFileStatusForm form, BindException errors) TableInfo ti = SequenceAnalysisSchema.getTable(SequenceAnalysisSchema.TABLE_OUTPUTFILES); for (int outputFileId : form.getOutputFileIds()) { - SequenceOutputFile so = SequenceOutputFile.getForId(outputFileId); + SequenceOutputFile so = SequenceOutputFile.getForId(outputFileId, getUser()); outputFiles.put(so.getRowid().toString(), so.toJSON()); Map rowMap = new TableSelector(ti, PageFlowUtil.set("dataId", "library_id"), new SimpleFilter(FieldKey.fromString("rowid"), outputFileId), null).getMap(); @@ -3251,6 +3269,11 @@ public ApiResponse execute(CheckFileStatusForm form, BindException errors) continue; } + if (!d.getContainer().hasPermission(getUser(), ReadPermission.class)) + { + throw new UnauthorizedException("Insufficient permissions to read files"); + } + JSONObject o = getDataJson(handler, outputFileId); o.put("libraryId", libraryId); arr.put(o); @@ -3296,7 +3319,7 @@ public ApiResponse execute(CheckFileStatusForm form, BindException errors) private JSONObject getDataJson(SequenceOutputHandler handler, Integer outputFileId) { JSONObject o = new JSONObject(); - SequenceOutputFile outputFile = SequenceOutputFile.getForId(outputFileId); + SequenceOutputFile outputFile = SequenceOutputFile.getForId(outputFileId, getUser()); if (outputFile == null) { _log.error("getDataJson was provided with an invalid outputFileId: " + outputFileId, new Exception()); @@ -3735,7 +3758,7 @@ public ApiResponse execute(RunSequenceHandlerForm form, BindException errors) th { for (int outputFileId : form.getOutputFileIds()) { - SequenceOutputFile o = SequenceOutputFile.getForId(outputFileId); + SequenceOutputFile o = SequenceOutputFile.getForId(outputFileId, getUser(), InsertPermission.class); if (o == null || o.getFile() == null) { errors.reject(ERROR_MSG, "Unable to find file: " + outputFileId); @@ -4901,7 +4924,7 @@ public ApiResponse execute(OutputFilesForm form, BindException errors) throws Ex Map fileMap = new IntHashMap<>(); for (Integer rowId : form.getOutputFileIds()) { - SequenceOutputFile f = SequenceOutputFile.getForId(rowId); + SequenceOutputFile f = SequenceOutputFile.getForId(rowId, getUser()); if (f != null) { fileMap.put(f.getRowid(), f.toJSON()); diff --git a/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java index cff4174ba..d89475e81 100644 --- a/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java +++ b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java @@ -2,12 +2,12 @@ import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectWriter; import org.json.JSONObject; +import org.labkey.api.security.User; import java.util.Date; import java.util.List; @@ -660,9 +660,9 @@ public String toJson() throws JsonProcessingException return ow.writeValueAsString(this); } - public static StudyDefinition getForId(int studyId) + public static StudyDefinition getForId(int studyId, User u) { - // TODO: implement this. This should query the DB and return a populated StudyDefinition + // TODO: implement this. This should query the DB and return a populated StudyDefinition. It should make sure the passed user has ReadPermission on that container return null; } diff --git a/Studies/src/org/labkey/studies/StudiesManager.java b/Studies/src/org/labkey/studies/StudiesManager.java index f7c23f2b3..fa251b8f9 100644 --- a/Studies/src/org/labkey/studies/StudiesManager.java +++ b/Studies/src/org/labkey/studies/StudiesManager.java @@ -63,7 +63,6 @@ public StudyDefinition insertOrUpdateStudyDefinition(StudyDefinition sd, Contain DbScope scope = schema.getScope(); UserSchema us = QueryService.get().getUserSchema(u, c, StudiesSchema.NAME); - TableInfo tblStudies = us.getTable(StudiesSchema.TABLE_STUDIES); TableInfo tblCohorts = us.getTable(StudiesSchema.TABLE_COHORTS); TableInfo tblAnchorEvents = us.getTable(StudiesSchema.TABLE_ANCHOR_EVENTS); @@ -71,7 +70,26 @@ public StudyDefinition insertOrUpdateStudyDefinition(StudyDefinition sd, Contain try (DbScope.Transaction tx = scope.ensureTransaction()) { - sd.setContainer(c.getEntityId().toString()); + if (sd.getRowId() != null) + { + TableSelector ts = new TableSelector(tblStudies, PageFlowUtil.set("container"), new SimpleFilter(FieldKey.fromString("rowId"), sd.getRowId()), null); + if (!ts.exists()) + { + throw new IllegalArgumentException("Unable to find existing study with rowId: " + sd.getRowId()); + } + + String existingContainerId = ts.getObject(String.class); + Container existingContainer = ContainerManager.getForId(existingContainerId); + if (!c.equals(existingContainer)) + { + throw new IllegalArgumentException("The study is from the wrong container: " + sd.getRowId()); + } + } + else + { + sd.setContainer(c.getEntityId().toString()); + } + sd = upsertStudy(sd, tblStudies, c, u); upsertChildRecords( @@ -198,7 +216,9 @@ private void upsertChildRecords(int studyRowId, { List> ret = qus.insertRows(u, c, inserts, bve, null, null); for (int i = 0; i < ret.size(); i++) + { setRowId.set(insertBeans.get(i), asInteger(ret.get(i).get("rowId"))); + } } if (!updates.isEmpty()) diff --git a/blast/src/org/labkey/blast/BLASTManager.java b/blast/src/org/labkey/blast/BLASTManager.java index a578ba505..d684401b0 100644 --- a/blast/src/org/labkey/blast/BLASTManager.java +++ b/blast/src/org/labkey/blast/BLASTManager.java @@ -22,6 +22,7 @@ import org.json.JSONObject; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ContainerType; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.PropertyManager.WritablePropertyMap; import org.labkey.api.data.SimpleFilter; @@ -139,7 +140,7 @@ public Container getContainerForDatabase(String databaseId) public void createDatabase(Container c, User u, Integer libraryId) throws IllegalArgumentException, IOException { //only create once per library - TableInfo databases = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_DATABASES); + TableInfo databases = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_DATABASES); TableSelector dbTs = new TableSelector(databases, PageFlowUtil.set("objectid"), new SimpleFilter(FieldKey.fromString("libraryid"), libraryId), null); if (dbTs.exists()) { @@ -159,14 +160,14 @@ public void createDatabase(Container c, User u, Integer libraryId) throws Illega public String runBLASTN(Container c, User u, String blastDb, File input, String task, String title, boolean saveResults, Map params, boolean async) throws IllegalArgumentException, IOException { - TableInfo databases = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_DATABASES); + TableInfo databases = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_DATABASES); TableSelector ts = new TableSelector(databases, new SimpleFilter(FieldKey.fromString("objectid"), blastDb), null); if (!ts.exists()) { throw new IllegalArgumentException("Unable to find BLAST DB: " + blastDb); } - TableInfo jobs = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_BLAST_JOBS); + TableInfo jobs = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_BLAST_JOBS); BlastJob databaseRecord = new BlastJob(); databaseRecord.setDatabaseId(blastDb); databaseRecord.setTitle(title); diff --git a/cluster/src/org/labkey/cluster/ClusterController.java b/cluster/src/org/labkey/cluster/ClusterController.java index a5f5dba05..40fcc8342 100644 --- a/cluster/src/org/labkey/cluster/ClusterController.java +++ b/cluster/src/org/labkey/cluster/ClusterController.java @@ -330,6 +330,12 @@ public boolean handlePost(JobIdsForm form, BindException errors) throws Exceptio return false; } + if (!sf.lookupContainer().hasPermission(getUser(), AdminPermission.class)) + { + errors.reject(ERROR_MSG, "Insufficient permissions to update: " + id); + return false; + } + sfs.add(sf); } @@ -422,6 +428,12 @@ public boolean handlePost(JobIdsForm form, BindException errors) throws Exceptio return false; } + if (!sf.lookupContainer().hasPermission(getUser(), AdminPermission.class)) + { + errors.reject(ERROR_MSG, "Insufficient permissions to update: " + id); + return false; + } + sfs.add(sf); } diff --git a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java index 040600f29..b75b6e0ba 100644 --- a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java +++ b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java @@ -726,7 +726,7 @@ else if (MGAP_FILTERED.equalsIgnoreCase(form.getSession())) } else { - JBrowseSession db = JBrowseSession.getForId(form.getSession()); + JBrowseSession db = JBrowseSession.getForId(form.getSession(), getUser()); if (db == null) { errors.reject(ERROR_MSG, "Unknown session: " + form.getSession()); @@ -813,7 +813,7 @@ public ApiResponse execute(LuceneQueryForm form, BindException errors) { try { - JBrowseSession session = JBrowseFieldUtils.getSession(form.getSessionId()); + JBrowseSession session = JBrowseFieldUtils.getSession(form.getSessionId(), getUser()); JsonFile jsonFile = JBrowseFieldUtils.getTrack(session, form.getTrackId(), getUser()); Map fields = JBrowseFieldUtils.getIndexedFields(jsonFile, getUser(), getContainer()); @@ -904,6 +904,12 @@ public ApiResponse execute(LuceneQueryForm form, BindException errors) try { JBrowseLuceneSearch searcher = JBrowseLuceneSearch.create(form.getSessionId(), form.getTrackId(), getUser()); + if (!searcher.getContainer().equals(getContainer())) + { + errors.reject(ERROR_MSG, "Invalid session: " + form.getSessionId()); + return null; + } + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd_HH-mm-ss"); String timestamp = LocalDateTime.now().format(formatter); String filename = "mGAP_results_" + timestamp + ".csv"; diff --git a/jbrowse/src/org/labkey/jbrowse/JBrowseFieldUtils.java b/jbrowse/src/org/labkey/jbrowse/JBrowseFieldUtils.java index 13d22ced1..7807c10e5 100644 --- a/jbrowse/src/org/labkey/jbrowse/JBrowseFieldUtils.java +++ b/jbrowse/src/org/labkey/jbrowse/JBrowseFieldUtils.java @@ -170,9 +170,9 @@ public static void clearCache() _cache.clear(); } - public static JBrowseSession getSession(String sessionId) + public static JBrowseSession getSession(String sessionId, User u) { - JBrowseSession session = JBrowseSession.getForId(sessionId); + JBrowseSession session = JBrowseSession.getForId(sessionId, u); if (session == null) { throw new IllegalArgumentException("Unable to find JBrowse session: " + sessionId); diff --git a/jbrowse/src/org/labkey/jbrowse/JBrowseLuceneSearch.java b/jbrowse/src/org/labkey/jbrowse/JBrowseLuceneSearch.java index d09584b4d..7a4c2de64 100644 --- a/jbrowse/src/org/labkey/jbrowse/JBrowseLuceneSearch.java +++ b/jbrowse/src/org/labkey/jbrowse/JBrowseLuceneSearch.java @@ -111,14 +111,14 @@ private static synchronized ExecutorService getSearchExecutor() return _executor; } - private Container getContainer() + public Container getContainer() { return ContainerManager.getForId(_session.getContainer()); } public static JBrowseLuceneSearch create(String sessionId, String trackId, User u) { - JBrowseSession session = getSession(sessionId); + JBrowseSession session = getSession(sessionId, u); return new JBrowseLuceneSearch(session, getTrack(session, trackId, u), u); } diff --git a/jbrowse/src/org/labkey/jbrowse/model/JBrowseSession.java b/jbrowse/src/org/labkey/jbrowse/model/JBrowseSession.java index f8cbecc26..e5af19eb6 100644 --- a/jbrowse/src/org/labkey/jbrowse/model/JBrowseSession.java +++ b/jbrowse/src/org/labkey/jbrowse/model/JBrowseSession.java @@ -20,11 +20,13 @@ import org.labkey.api.pipeline.PipelineJobException; import org.labkey.api.query.FieldKey; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.sequenceanalysis.SequenceAnalysisService; import org.labkey.api.sequenceanalysis.pipeline.ReferenceGenome; import org.labkey.api.util.FileUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.view.UnauthorizedException; import org.labkey.jbrowse.JBrowseManager; import org.labkey.jbrowse.JBrowseSchema; @@ -168,6 +170,29 @@ public static void onDatabaseDelete(String containerId, final String databaseId) Table.delete(ti, new SimpleFilter(FieldKey.fromString("database"), databaseId, CompareType.EQUAL)); } + public static JBrowseSession getForId(String objectId, User u) + { + JBrowseSession s = getForId(objectId); + if (s == null) + { + return null; + } + + Container c = s.getContainerObj(); + if (c == null) + { + _log.error("JBrowse session lacks a valid container: " + objectId); + return null; + } + + if (!c.hasPermission(u, ReadPermission.class)) + { + throw new UnauthorizedException("Insufficient permissions to read JBrowse session"); + } + + return s; + } + public static JBrowseSession getForId(String objectId) { return new TableSelector(JBrowseSchema.getInstance().getSchema().getTable(JBrowseSchema.TABLE_DATABASES)).getObject(objectId, JBrowseSession.class); diff --git a/singlecell/src/org/labkey/singlecell/SingleCellController.java b/singlecell/src/org/labkey/singlecell/SingleCellController.java index 242c3bc17..364846906 100644 --- a/singlecell/src/org/labkey/singlecell/SingleCellController.java +++ b/singlecell/src/org/labkey/singlecell/SingleCellController.java @@ -116,7 +116,7 @@ public void export(OutputFilesForm form, HttpServletResponse response, BindExcep for (Integer rowId : form.getOutputFileIds()) { - SequenceOutputFile so = SequenceOutputFile.getForId(rowId); + SequenceOutputFile so = SequenceOutputFile.getForId(rowId, getUser()); if (so != null) { File loupe = so.getFile(); @@ -597,7 +597,7 @@ public boolean handlePost(Object form, BindException errors) throws Exception UserSchema sa = QueryService.get().getUserSchema(getUser(), getContainer(), SingleCellSchema.SEQUENCE_SCHEMA_NAME); List> toUpdate = new ArrayList<>(); new TableSelector(sa.getTable("outputfiles"), PageFlowUtil.set("rowid"), new SimpleFilter(FieldKey.fromString("category"), "Seurat Object Prototype").addCondition(FieldKey.fromString("modified"), "2025-04-29", CompareType.DATE_LT), null).forEach(Integer.class, rowId -> { - SequenceOutputFile so = SequenceOutputFile.getForId(rowId); + SequenceOutputFile so = SequenceOutputFile.getForId(rowId, getUser()); if (so == null) { throw new IllegalStateException("Unable to create SequenceOutputFile for: " + rowId); From 52e17dfa6b770cbbba7c746d20a96d503cacf0ab Mon Sep 17 00:00:00 2001 From: bbimber Date: Mon, 17 Aug 2026 11:28:57 -0700 Subject: [PATCH 4/7] More lenient container comparison --- jbrowse/src/org/labkey/jbrowse/JBrowseController.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java index b75b6e0ba..7840be8cf 100644 --- a/jbrowse/src/org/labkey/jbrowse/JBrowseController.java +++ b/jbrowse/src/org/labkey/jbrowse/JBrowseController.java @@ -38,6 +38,7 @@ import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ContainerType; import org.labkey.api.data.DbScope; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -904,7 +905,7 @@ public ApiResponse execute(LuceneQueryForm form, BindException errors) try { JBrowseLuceneSearch searcher = JBrowseLuceneSearch.create(form.getSessionId(), form.getTrackId(), getUser()); - if (!searcher.getContainer().equals(getContainer())) + if (!searcher.getContainer().getContainerFor(ContainerType.DataType.tabParent).equals(getContainer().getContainerFor(ContainerType.DataType.tabParent))) { errors.reject(ERROR_MSG, "Invalid session: " + form.getSessionId()); return null; From 8c1c0e945442d9f5d0c26b2447c0228060b10713 Mon Sep 17 00:00:00 2001 From: bbimber Date: Mon, 17 Aug 2026 13:36:20 -0700 Subject: [PATCH 5/7] Improve accuracy of test --- .../test/tests/external/labModules/SequenceTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java b/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java index a8c49a0c7..580dc15ce 100644 --- a/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java +++ b/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java @@ -230,10 +230,16 @@ private void importReadsetMetadata() waitForElement(Locator.tagContainingText("a", "SRA2")); dr.checkAllOnPage(); + Assert.assertEquals("Incorrect checked row count", 3, dr.getCheckedCount()); dr.clickHeaderButtonAndWait("Delete"); clickButton("OK"); _readsetCt -= 3; + + log("verifying readset count correct"); + goToProjectHome(); + waitForText("Sequence Readsets"); + waitForElement(LabModuleHelper.getNavPanelItem("Sequence Readsets:", _readsetCt.toString())); } /** @@ -417,7 +423,7 @@ private void importIlluminaTest() throws Exception } /** - * This method has several puposes. It will verify that the records from illuminaImportTest() were + * This method has several purposes. It will verify that the records from illuminaImportTest() were * created properly. It also exercises various features associated with the readset grid, including * the FASTQC report and downloading of results * From 93b3b3a1bc00528458fff3fc75f3247c43c96ce3 Mon Sep 17 00:00:00 2001 From: bbimber Date: Mon, 17 Aug 2026 13:47:00 -0700 Subject: [PATCH 6/7] Bugfix to sequence delete --- .../labkey/sequenceanalysis/SequenceAnalysisController.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java b/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java index 14b8b2beb..6849a9c81 100644 --- a/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java +++ b/SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java @@ -668,7 +668,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) if (!additionalAnalysisIds.isEmpty()) { - msg.append("

"); + msg.unsafeAppend("

"); msg.append("The following " + additionalAnalysisIds.size() + " analyses will also be deleted, along with these associated records/files:").unsafeAppend("
"); findAnalysesToDelete(additionalAnalysisIds, msg, outputFileIds, expRunsToDelete); } @@ -722,7 +722,7 @@ else if (SequenceAnalysisSchema.TABLE_OUTPUTFILES.equals(_table.getName())) } } - msg.append(" Date: Mon, 17 Aug 2026 17:36:50 -0700 Subject: [PATCH 7/7] Bugfix to sequence delete --- .../labkey/test/tests/external/labModules/SequenceTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java b/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java index 580dc15ce..ec07524df 100644 --- a/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java +++ b/SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java @@ -238,8 +238,7 @@ private void importReadsetMetadata() log("verifying readset count correct"); goToProjectHome(); - waitForText("Sequence Readsets"); - waitForElement(LabModuleHelper.getNavPanelItem("Sequence Readsets:", _readsetCt.toString())); + waitForElement(LabModuleHelper.getNavPanelItem("Readsets:", _readsetCt.toString())); } /**