25.7 fb container scoping

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<String> 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);

flow/src/org/labkey/flow/FlowModule.java

@@ -342,7 +342,10 @@ public Set<Class> getIntegrationTests()
             FlowController.TestCase.class,
             FlowProtocol.TestCase.class,
             PersistTests.class,
-            FlowManager.TestCase.class
+            FlowManager.TestCase.class,
+            FlowManager.ContainerScopingTestCase.class,
+            WellController.WellContainerScopingTestCase.class,
+            AnalysisScriptController.AnalysisScriptContainerScopingTestCase.class
         );
     }
 

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());

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<FlowFCSFile> 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));
+        }
+    }
 }

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;
     }
 

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<Attachme
             {
                 throw new NotFoundException();
             }
+            // GitHub Issue #1892: validate container
+            run.checkContainer(getContainer(), getUser(), getViewContext().getActionURL());
 
             return new Pair<>(new ExpRunAttachmentParent(run.getExperimentRun()), form.getName());
         }