diff --git a/elispotassay/src/org/labkey/elispot/ElispotController.java b/elispotassay/src/org/labkey/elispot/ElispotController.java index 3c357b789e..888a37b528 100644 --- a/elispotassay/src/org/labkey/elispot/ElispotController.java +++ b/elispotassay/src/org/labkey/elispot/ElispotController.java @@ -16,6 +16,7 @@ package org.labkey.elispot; +import org.apache.commons.lang3.math.NumberUtils; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; @@ -543,6 +544,15 @@ public boolean handlePost(Object o, BindException errors) throws Exception Set selections = DataRegionSelection.getSelected(getViewContext(), true); if (!selections.isEmpty()) { + // GitHub Kanban #1892: Verify each selected run belongs to the current container before queuing + for (String selection : selections) + { + int rowId = NumberUtils.toInt(selection, -1); + ExpRun run = rowId != -1 ? ExperimentService.get().getExpRun(rowId, getContainer()) : null; + if (run == null) + throw new NotFoundException("Run " + selection + " does not exist."); + } + ViewBackgroundInfo info = new ViewBackgroundInfo(getContainer(), getUser(), getViewContext().getActionURL()); BackgroundSubtractionJob job = new BackgroundSubtractionJob(ElispotPipelineProvider.NAME, info, PipelineService.get().findPipelineRoot(getContainer()), selections); diff --git a/flow/src/org/labkey/flow/FlowModule.java b/flow/src/org/labkey/flow/FlowModule.java index cbe2d66b21..211007e1f1 100644 --- a/flow/src/org/labkey/flow/FlowModule.java +++ b/flow/src/org/labkey/flow/FlowModule.java @@ -342,7 +342,10 @@ public Set getIntegrationTests() FlowController.TestCase.class, FlowProtocol.TestCase.class, PersistTests.class, - FlowManager.TestCase.class + FlowManager.TestCase.class, + FlowManager.ContainerScopingTestCase.class, + WellController.WellContainerScopingTestCase.class, + AnalysisScriptController.AnalysisScriptContainerScopingTestCase.class ); } diff --git a/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java b/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java index b8632153ff..cde1ea674e 100644 --- a/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java +++ b/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java @@ -73,7 +73,12 @@ public void reset() { try { - String scriptIdStr = getRequest().getParameter("scriptId"); + ActionURL url = getViewContext().getActionURL(); + // Read scriptId from the action URL (falling back to the request parameter) so it resolves under both + // real browser requests and in-JVM mock dispatch, mirroring how runId/compId below are read from the URL. + String scriptIdStr = url.getParameter("scriptId"); + if (scriptIdStr == null) + scriptIdStr = getRequest().getParameter("scriptId"); if (scriptIdStr == null) { throw new NotFoundException("scriptId required"); @@ -92,6 +97,8 @@ public void reset() { throw new NotFoundException("scriptId not found: " + scriptIdStr); } + // GitHub Issue #1892: validate container + flowObject.checkContainer(getContainer(), getUser(), url); _runCount = flowObject.getRunCount(); step = FlowProtocolStep.fromRequest(getRequest()); _run = FlowRun.fromURL(getViewContext().getActionURL(), getRequest(), getViewContext().getContainer(), getUser()); diff --git a/flow/src/org/labkey/flow/controllers/executescript/AnalysisScriptController.java b/flow/src/org/labkey/flow/controllers/executescript/AnalysisScriptController.java index 8089be37c2..8e901c4657 100644 --- a/flow/src/org/labkey/flow/controllers/executescript/AnalysisScriptController.java +++ b/flow/src/org/labkey/flow/controllers/executescript/AnalysisScriptController.java @@ -20,8 +20,11 @@ import org.apache.commons.io.filefilter.IOFileFilter; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; +import org.fhcrc.cpas.flow.script.xml.ScriptDocument; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Before; +import org.junit.Test; import org.labkey.api.action.FormViewAction; import org.labkey.api.action.SimpleRedirectAction; import org.labkey.api.action.SimpleViewAction; @@ -36,11 +39,16 @@ import org.labkey.api.pipeline.PipelineUrls; import org.labkey.api.pipeline.browse.PipelinePathForm; import org.labkey.api.security.RequiresPermission; +import org.labkey.api.security.User; +import org.labkey.api.security.permissions.AbstractContainerScopingTest; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.security.roles.EditorRole; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; +import org.labkey.api.test.TestWhen; import org.labkey.api.usageMetrics.SimpleMetricsService; import org.labkey.api.util.FileUtil; import org.labkey.api.util.PageFlowUtil; @@ -54,6 +62,7 @@ import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; +import org.labkey.api.view.NotFoundException; import org.labkey.api.view.RedirectException; import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.api.view.template.PageConfig; @@ -86,11 +95,14 @@ import org.labkey.flow.util.SampleUtil; import org.labkey.vfs.FileLike; import org.labkey.vfs.FileSystemLike; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.servlet.ModelAndView; +import jakarta.servlet.http.HttpServletResponse; + import java.io.File; import java.io.IOException; import java.net.URI; @@ -185,16 +197,20 @@ protected ModelAndView analyzeRuns(ChooseRunsToAnalyzeForm form, int[] runIds, S DataRegionSelection.clearAll(getViewContext()); FlowExperiment experiment = FlowExperiment.fromLSID(experimentLSID); + checkContainer(experiment); String experimentName = form.ff_analysisName; if (experiment != null) { experimentName = experiment.getName(); } FlowScript analysis = form.getProtocol(); + checkContainer(analysis); AnalyzeJob job = new AnalyzeJob(getViewBackgroundInfo(), experimentName, experimentLSID, FlowProtocol.ensureForContainer(getUser(), getContainer()), analysis, form.getProtocolStep(), runIds, PipelineService.get().findPipelineRoot(getContainer())); if (form.getCompensationMatrixId() != 0) { - job.setCompensationMatrix(FlowCompensationMatrix.fromCompId(form.getCompensationMatrixId())); + FlowCompensationMatrix comp = FlowCompensationMatrix.fromCompId(form.getCompensationMatrixId()); + checkContainer(comp); + job.setCompensationMatrix(comp); } job.setCompensationExperimentLSID(form.getCompensationExperimentLSID()); return HttpView.redirect(executeScript(job)); @@ -216,6 +232,7 @@ public class ChooseRunsToAnalyzeAction extends BaseAnalyzeRunsAction public ModelAndView getView(ChooseRunsToAnalyzeForm form, BindException errors) { script = form.getProtocol(); + checkContainer(script); return chooseRunsToAnalyze(form, errors); } } @@ -227,12 +244,19 @@ public class AnalyzeSelectedRunsAction extends BaseAnalyzeRunsAction public ModelAndView getView(ChooseRunsToAnalyzeForm form, BindException errors) throws Exception { script = form.getProtocol(); + checkContainer(script); int[] runIds = form.getSelectedRunIds(); if (runIds.length == 0) { errors.reject(ERROR_MSG, "Please select at least one run to analyze."); return chooseRunsToAnalyze(form, errors); } + for (int runId : runIds) + { + FlowRun run = FlowRun.fromRunId(runId); + if (run == null || !run.getContainer().hasPermission(getUser(), ReadPermission.class)) + throw new NotFoundException("Run not found: " + runId); + } String experimentLSID = form.getAnalysisLSID(); if (experimentLSID == null) { @@ -843,6 +867,12 @@ private SampleIdMap getSelectedFCSFiles(ImportAnalysisForm form, Er return null; } + if (!file.getContainer().equals(getContainer())) + { + errors.reject(ERROR_MSG, "Resolved FCS file '" + file.getName() + "' is not in this folder."); + return null; + } + if (!file.isOriginalFCSFile()) { errors.reject(ERROR_MSG, "Resolved FCS file '" + file.getName() + "' is a FCS files created from importing an external analysis."); @@ -1495,4 +1525,51 @@ public void addNavTrail(NavTree root) root.addChild(display); } } + + @TestWhen(TestWhen.When.BVT) + public static class AnalysisScriptContainerScopingTestCase extends AbstractContainerScopingTest + { + private User _admin; + private Container _folderA; + private Container _folderB; + private int _scriptIdB; + + @Before + public void setUp() throws Exception + { + _admin = getAdmin(); + _folderA = createContainer("A"); + _folderB = createContainer("B"); + + // Pre-create the flow protocol/steps in B so the valid-case ChooseRunsToAnalyzeForm.populate() finds them. + FlowProtocol.ensureForContainer(_admin, _folderB); + + // Create a flow analysis script with a single analysis step so populate() has a usable protocol step. + ScriptDocument doc = ScriptDocument.Factory.newInstance(); + doc.addNewScript().addNewAnalysis(); + _scriptIdB = FlowScript.create(_admin, _folderB, getClass().getSimpleName() + "-" + "scriptB", doc.toString()).getScriptId(); + } + + // ChooseRunsToAnalyzeForm resolves the analysis script by global rowId; a foreign script must be scoped. + @Test + public void testAnalysisScriptContainerScoping() throws Exception + { + ActionURL foreignUrl = new ActionURL(ChooseRunsToAnalyzeAction.class, _folderA).addParameter("scriptId", _scriptIdB); + + User folderAEditor = createUserInRole(_folderA, EditorRole.class); + assertStatus(HttpServletResponse.SC_NOT_FOUND, get(foreignUrl, folderAEditor)); + + User folderBEditor = createUserInRole(_folderA, EditorRole.class); + grantRole(folderBEditor, _folderB, ReaderRole.class); + MockHttpServletResponse resp = get(foreignUrl, folderBEditor); + assertStatus(HttpServletResponse.SC_FOUND, resp); + String location = resp.getRedirectedUrl(); + assertNotNull("Redirect must have a Location", location); + assertTrue("Redirect should target the script's own container, was: " + location, location.contains(_folderB.getPath())); + + // valid case: the script in its own container is accepted (renders the choose-runs wizard). + assertStatus(HttpServletResponse.SC_OK, + get(new ActionURL(ChooseRunsToAnalyzeAction.class, _folderB).addParameter("scriptId", _scriptIdB), _admin)); + } + } } diff --git a/flow/src/org/labkey/flow/controllers/protocol/ProtocolForm.java b/flow/src/org/labkey/flow/controllers/protocol/ProtocolForm.java index b1b7d75dc0..311d04c42c 100644 --- a/flow/src/org/labkey/flow/controllers/protocol/ProtocolForm.java +++ b/flow/src/org/labkey/flow/controllers/protocol/ProtocolForm.java @@ -35,7 +35,7 @@ public FlowProtocol getProtocol() throws UnauthorizedException { if (_protocol != null) return _protocol; - _protocol = FlowProtocol.fromURLRedirectIfNull(getUser(), getViewContext().getActionURL(), getRequest()); + _protocol = FlowProtocol.fromURLRedirectIfNull(getUser(), getViewContext(), getRequest()); return _protocol; } diff --git a/flow/src/org/labkey/flow/controllers/run/RunController.java b/flow/src/org/labkey/flow/controllers/run/RunController.java index 01dbc2a010..7a90149d16 100644 --- a/flow/src/org/labkey/flow/controllers/run/RunController.java +++ b/flow/src/org/labkey/flow/controllers/run/RunController.java @@ -232,6 +232,8 @@ public void validate(DownloadRunForm form, BindException errors) errors.reject(ERROR_MSG, "run not found"); return; } + // GitHub Issue #1892: validate container + _run.checkContainer(getContainer(), getUser(), getActionURL()); FlowWell[] wells = _run.getWells(true); if (wells.length == 0) @@ -464,6 +466,8 @@ else if (form.getSendTo() == ExportAnalysisForm.SendTo.Script) if (run == null) throw new NotFoundException("Flow run not found"); + // GitHub Issue #1892: validate container + run.checkContainer(getContainer(), getUser(), getActionURL()); runs.add(run); } _runs = runs; @@ -477,6 +481,8 @@ else if (wellId != null && wellId.length > 0) if (well == null) throw new NotFoundException("Flow well not found"); + // GitHub Issue #1892: validate container + well.checkContainer(getContainer(), getUser(), getActionURL()); wells.add(well); } _wells = wells; @@ -941,6 +947,8 @@ public static class DownloadAttachmentAction extends BaseDownloadAction(new ExpRunAttachmentParent(run.getExperimentRun()), form.getName()); } diff --git a/flow/src/org/labkey/flow/controllers/well/WellController.java b/flow/src/org/labkey/flow/controllers/well/WellController.java index a76cb82093..366365f472 100644 --- a/flow/src/org/labkey/flow/controllers/well/WellController.java +++ b/flow/src/org/labkey/flow/controllers/well/WellController.java @@ -20,16 +20,22 @@ import org.apache.commons.lang3.time.DateUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Before; +import org.junit.Test; import org.labkey.api.action.FormViewAction; import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.ReturnUrlForm; import org.labkey.api.action.SimpleErrorView; import org.labkey.api.action.SimpleViewAction; +import org.labkey.api.data.Container; import org.labkey.api.data.DataRegionSelection; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; +import org.labkey.api.exp.api.ExpData; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.jsp.FormPage; import org.labkey.api.jsp.JspBase; import org.labkey.api.pipeline.PipeRoot; @@ -38,11 +44,16 @@ import org.labkey.api.security.ContextualRoles; import org.labkey.api.security.RequiresNoPermission; import org.labkey.api.security.RequiresPermission; +import org.labkey.api.security.User; +import org.labkey.api.security.permissions.AbstractContainerScopingTest; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.security.roles.ReaderRole; +import org.labkey.api.test.TestWhen; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HeartBeat; +import org.labkey.api.util.JunitUtil; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.URIUtil; @@ -52,6 +63,7 @@ import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; import org.labkey.api.view.NotFoundException; +import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.api.view.ViewContext; import org.labkey.flow.analysis.model.FCSHeader; import org.labkey.flow.analysis.web.FCSAnalyzer; @@ -63,16 +75,23 @@ import org.labkey.flow.controllers.FlowParam; import org.labkey.flow.data.FlowCompensationMatrix; import org.labkey.flow.data.FlowDataObject; +import org.labkey.flow.data.FlowDataType; +import org.labkey.flow.data.FlowFCSFile; +import org.labkey.flow.data.FlowProtocol; import org.labkey.flow.data.FlowProtocolStep; import org.labkey.flow.data.FlowRun; import org.labkey.flow.data.FlowWell; import org.labkey.flow.persist.AttributeCache; +import org.labkey.flow.persist.AttributeSet; +import org.labkey.flow.persist.AttributeSetHelper; import org.labkey.flow.persist.FlowManager; import org.labkey.flow.persist.ObjectType; import org.labkey.flow.query.FlowTableType; import org.labkey.flow.script.FlowAnalyzer; +import org.labkey.flow.script.KeywordsJob; import org.labkey.flow.util.KeywordUtil; import org.labkey.flow.view.GraphColumn; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; @@ -286,7 +305,9 @@ public ModelAndView getView(EditWellForm form, boolean reshow, BindException err for (String wellId : selected) { - _wells.add(FlowWell.fromWellId(Integer.parseInt(wellId))); + FlowWell well = FlowWell.fromWellId(Integer.parseInt(wellId)); + well.checkContainer(getContainer(), getUser(), getActionURL()); + _wells.add(well); } DataRegionSelection.clearAll(form.getViewContext()); } @@ -372,6 +393,19 @@ public void addNavTrail(NavTree root) } } + private @NotNull FlowWell checkContainer(ChooseGraphForm form) + { + FlowWell well = form.getWell(); + if (well == null) + throw new NotFoundException("Well not found"); + + checkContainer(well); + checkContainer(form.getScript()); + checkContainer(form.getCompensationMatrix()); + + return well; + } + @RequiresPermission(ReadPermission.class) public class ChooseGraphAction extends SimpleViewAction { @@ -380,11 +414,7 @@ public class ChooseGraphAction extends SimpleViewAction @Override public ModelAndView getView(ChooseGraphForm form, BindException errors) { - _well = form.getWell(); - if (null == _well) - { - throw new NotFoundException(); - } + _well = checkContainer(form); URI fileURI = _well.getFCSURI(); if (fileURI == null) @@ -497,10 +527,7 @@ public class GenerateGraphAction extends SimpleViewAction @Override public ModelAndView getView(ChooseGraphForm form, BindException errors) throws IOException { - FlowWell well = form.getWell(); - if (well == null) - throw new NotFoundException("Well not found"); - + checkContainer(form); String graph = getParam(FlowParam.graph); if (graph == null) throw new NotFoundException("Graph spec required"); @@ -886,4 +913,94 @@ public Object execute(Object o, BindException errors) return null; } } + + @TestWhen(TestWhen.When.BVT) + public static class WellContainerScopingTestCase extends AbstractContainerScopingTest + { + private User _admin; + private Container _folderA; + private Container _folderB; + private int _wellIdA; + private int _scriptIdB; + private int _compIdB; + + @Before + public void setUp() throws Exception + { + _admin = getAdmin(); + _folderA = createContainer("A"); + _folderB = createContainer("B"); + + _wellIdA = createFlowDataId(_folderA, FlowDataType.FCSFile, "wellA"); + _scriptIdB = createFlowDataId(_folderB, FlowDataType.Script, "scriptB"); + _compIdB = createFlowDataId(_folderB, FlowDataType.CompensationMatrix, "compB"); + } + + private int createFlowDataId(Container c, FlowDataType type, String name) throws Exception + { + URI uri = new URI("file:///" + getClass().getSimpleName() + "-" + name + ".flowdata.xml"); + ExpData data = ExperimentService.get().createData(c, type, getClass().getSimpleName() + "-" + name); + data.setDataFileURI(uri); + data.save(_admin); + // fromWellId/fromCompId resolve via FlowDataObject.fromData(), which returns null unless the data has an + // AttrObject (these FlowDataTypes set requireAttrObject). Attach a minimal one so the object resolves. + AttributeSetHelper.save(new AttributeSet(type.getObjectType(), uri), _admin, data); + return data.getRowId(); + } + + // A foreign object must be 404 for a caller with no rights to its container, and a 302 redirect to that + // container for a caller who can read it. + private void assertForeignRejected(ActionURL foreignUrl) throws Exception + { + User readerAonly = createUserInRole(_folderA, ReaderRole.class); + assertStatus(HttpServletResponse.SC_NOT_FOUND, get(foreignUrl, readerAonly)); + + User readerAreaderB = createUserInRole(_folderA, ReaderRole.class); + grantRole(readerAreaderB, _folderB, ReaderRole.class); + MockHttpServletResponse resp = get(foreignUrl, readerAreaderB); + assertStatus(HttpServletResponse.SC_FOUND, resp); + String location = resp.getRedirectedUrl(); + assertNotNull("Redirect must have a Location", location); + assertTrue("Redirect should target the object's own container, was: " + location, location.contains(_folderB.getPath())); + } + + // ChooseGraphForm.getWell() resolves the well by global rowId. The valid case imports a legitimate run, so the + // well is backed by a readable FCS file, and the graph actually renders. + @Test + public void testWellContainerScoping() throws Exception + { + int foreignWellId = createFlowDataId(_folderB, FlowDataType.FCSFile, "wellB"); + assertForeignRejected(new ActionURL(GenerateGraphAction.class, _folderA).addParameter("wellId", foreignWellId)); + + // valid case: import a real run into B, then render a graph from its well in its own container + FlowProtocol protocol = FlowProtocol.ensureForContainer(_admin, _folderB); + PipeRoot root = PipelineService.get().findPipelineRoot(_folderB); + ViewBackgroundInfo info = new ViewBackgroundInfo(_folderB, _admin, null); + File dir = JunitUtil.getSampleData(null, "flow/flowjoquery/microFCS"); + FlowFCSFile realWell = new KeywordsJob(info, protocol, List.of(dir), null, root).go().get(0).getFCSFiles()[0]; + + // Build a graph spec from two of the well's real FCS parameter names. + List params = new ArrayList<>(FlowAnalyzer.getParameters(realWell, null).keySet()); + String graph = "(" + params.get(0) + ":" + params.get(1) + ")"; + ActionURL ownUrl = new ActionURL(GenerateGraphAction.class, _folderB) + .addParameter("wellId", realWell.getWellId()).addParameter("graph", graph); + assertStatus(HttpServletResponse.SC_OK, get(ownUrl, _admin)); + } + + // ChooseGraphForm.getScript() resolves an analysis script by global rowId; a foreign script must be scoped. + @Test + public void testScriptContainerScoping() throws Exception + { + assertForeignRejected(new ActionURL(GenerateGraphAction.class, _folderA) + .addParameter("wellId", _wellIdA).addParameter("scriptId", _scriptIdB)); + } + + // ChooseGraphForm.getCompensationMatrix() resolves a comp matrix by global rowId; a foreign one must be scoped. + @Test + public void testCompensationMatrixContainerScoping() throws Exception + { + assertForeignRejected(new ActionURL(GenerateGraphAction.class, _folderA) + .addParameter("wellId", _wellIdA).addParameter("compId", _compIdB)); + } + } } diff --git a/flow/src/org/labkey/flow/data/FlowProtocol.java b/flow/src/org/labkey/flow/data/FlowProtocol.java index c58bf74a9e..c34c860c97 100644 --- a/flow/src/org/labkey/flow/data/FlowProtocol.java +++ b/flow/src/org/labkey/flow/data/FlowProtocol.java @@ -16,6 +16,7 @@ package org.labkey.flow.data; +import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -70,6 +71,7 @@ import org.labkey.api.view.RedirectException; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewBackgroundInfo; +import org.labkey.api.view.ViewContext; import org.labkey.flow.controllers.FlowController; import org.labkey.flow.controllers.FlowParam; import org.labkey.flow.controllers.protocol.ProtocolController; @@ -79,7 +81,6 @@ import org.labkey.flow.query.FlowTableType; import org.labkey.flow.script.KeywordsJob; -import jakarta.servlet.http.HttpServletRequest; import java.io.File; import java.sql.ResultSet; import java.sql.SQLException; @@ -149,15 +150,19 @@ static public boolean isDefaultProtocol(ExpProtocol protocol) DEFAULT_PROTOCOL_NAME.equals(protocol.getName()); } - static public FlowProtocol fromURL(User user, ActionURL url, HttpServletRequest request) throws UnauthorizedException + static public FlowProtocol fromURL(User user, ViewContext context, HttpServletRequest request) throws UnauthorizedException { + ActionURL url = context.getActionURL(); FlowProtocol ret = fromProtocolId(getIntParam(url, request, FlowParam.experimentId)); if (ret == null) { - ret = FlowProtocol.getForContainer(ContainerManager.getForPath(url.getExtraPath())); + ret = FlowProtocol.getForContainer(context.getContainer()); } if (ret == null) return null; + + // GitHub Issue #1892: validate container + ret.checkContainer(context.getContainer(), user, url); if (!ret.getContainer().hasPermission(user, ReadPermission.class)) { throw new UnauthorizedException(); @@ -165,11 +170,11 @@ static public FlowProtocol fromURL(User user, ActionURL url, HttpServletRequest return ret; } - public static FlowProtocol fromURLRedirectIfNull(User user, ActionURL url, HttpServletRequest request) + public static FlowProtocol fromURLRedirectIfNull(User user, ViewContext context, HttpServletRequest request) { - FlowProtocol protocol = fromURL(user, url, request); + FlowProtocol protocol = fromURL(user, context, request); if (protocol == null) - throw new RedirectException(url.clone().setAction(FlowController.BeginAction.class)); + throw new RedirectException(context.getActionURL().clone().setAction(FlowController.BeginAction.class)); return protocol; } diff --git a/flow/src/org/labkey/flow/persist/FlowManager.java b/flow/src/org/labkey/flow/persist/FlowManager.java index 6e43a77375..d8451d818f 100644 --- a/flow/src/org/labkey/flow/persist/FlowManager.java +++ b/flow/src/org/labkey/flow/persist/FlowManager.java @@ -16,12 +16,14 @@ package org.labkey.flow.persist; +import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.collections4.iterators.ArrayIterator; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Before; import org.junit.Test; import org.labkey.api.audit.AuditLogService; import org.labkey.api.data.Aggregate; @@ -46,6 +48,7 @@ import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.api.ExpData; +import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.query.SamplesSchema; @@ -55,20 +58,31 @@ import org.labkey.api.query.QueryService; import org.labkey.api.query.UserSchema; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.AbstractContainerScopingTest; +import org.labkey.api.security.roles.EditorRole; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.util.DateUtil; import org.labkey.api.util.FileUtil; import org.labkey.api.util.UnexpectedException; +import org.labkey.api.view.ActionURL; import org.labkey.flow.FlowModule; import org.labkey.flow.analysis.web.GraphSpec; import org.labkey.flow.analysis.web.StatisticSpec; +import org.labkey.flow.controllers.FlowParam; +import org.labkey.flow.controllers.editscript.ScriptController; import org.labkey.flow.controllers.executescript.AnalysisEngine; +import org.labkey.flow.controllers.protocol.ProtocolController; +import org.labkey.flow.controllers.run.RunController; import org.labkey.flow.data.AttributeType; import org.labkey.flow.data.FlowDataObject; +import org.labkey.flow.data.FlowDataType; import org.labkey.flow.data.FlowProperty; import org.labkey.flow.data.FlowProtocol; +import org.labkey.flow.data.FlowScript; import org.labkey.flow.data.ICSMetadata; import org.labkey.flow.query.FlowSchema; import org.labkey.flow.query.FlowTableType; +import org.springframework.mock.web.MockHttpServletResponse; import java.net.URI; import java.sql.ResultSet; @@ -1802,4 +1816,138 @@ public void metrics() assertNotNull(metrics.get("flowTempTableCount")); } } + + public static class ContainerScopingTestCase extends AbstractContainerScopingTest + { + private Container _folderA; + private Container _folderB; + private User _admin; + + @Before + public void setup() + { + // Regression test for FLOW-1. A FlowScript is addressed by a global scriptId. + _admin = getAdmin(); + _folderA = createContainer("A"); + _folderB = createContainer("B"); + } + + @Test + public void testScriptContainerScoping() throws Exception + { + // GitHub Issue #1892: Regression test for FLOW-1. + FlowScript script = FlowScript.create(_admin, _folderB, "scope-test", + "