diff --git a/elisa/src/org/labkey/elisa/ElisaController.java b/elisa/src/org/labkey/elisa/ElisaController.java index 37228df59b..3c47cb8789 100644 --- a/elisa/src/org/labkey/elisa/ElisaController.java +++ b/elisa/src/org/labkey/elisa/ElisaController.java @@ -32,6 +32,7 @@ import org.labkey.api.exp.property.Domain; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.view.NotFoundException; import org.labkey.elisa.actions.ElisaUploadWizardAction; import org.labkey.elisa.query.CurveFitDb; import org.labkey.elisa.query.ElisaManager; @@ -109,10 +110,10 @@ public static class GetCurveFitXYPairs extends ReadOnlyApiAction= form.getxMax()) diff --git a/elispotassay/src/org/labkey/elispot/ElispotController.java b/elispotassay/src/org/labkey/elispot/ElispotController.java index 3c357b789e..b478d01c19 100644 --- a/elispotassay/src/org/labkey/elispot/ElispotController.java +++ b/elispotassay/src/org/labkey/elispot/ElispotController.java @@ -1,563 +1,637 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.labkey.elispot; - -import org.jetbrains.annotations.Nullable; -import org.json.JSONArray; -import org.json.JSONObject; -import org.labkey.api.action.ApiResponse; -import org.labkey.api.action.ApiSimpleResponse; -import org.labkey.api.action.FormHandlerAction; -import org.labkey.api.action.ReadOnlyApiAction; -import org.labkey.api.action.SimpleRedirectAction; -import org.labkey.api.action.SimpleViewAction; -import org.labkey.api.action.SpringActionController; -import org.labkey.api.assay.plate.AbstractPlateBasedAssayProvider; -import org.labkey.api.data.CompareType; -import org.labkey.api.data.ContainerFilter; -import org.labkey.api.data.DataRegionSelection; -import org.labkey.api.data.SimpleFilter; -import org.labkey.api.exp.Lsid; -import org.labkey.api.exp.ObjectProperty; -import org.labkey.api.exp.OntologyManager; -import org.labkey.api.exp.api.ExpData; -import org.labkey.api.exp.api.ExpMaterial; -import org.labkey.api.exp.api.ExpProtocol; -import org.labkey.api.exp.api.ExpRun; -import org.labkey.api.exp.api.ExperimentService; -import org.labkey.api.pipeline.PipelineService; -import org.labkey.api.pipeline.PipelineUrls; -import org.labkey.api.query.CrosstabView; -import org.labkey.api.query.FieldKey; -import org.labkey.api.query.QuerySettings; -import org.labkey.api.security.RequiresPermission; -import org.labkey.api.security.permissions.InsertPermission; -import org.labkey.api.security.permissions.ReadPermission; -import org.labkey.api.assay.plate.Plate; -import org.labkey.api.assay.plate.Position; -import org.labkey.api.assay.actions.AssayHeaderView; -import org.labkey.api.assay.AssayProtocolSchema; -import org.labkey.api.assay.AssayProvider; -import org.labkey.api.assay.AssayService; -import org.labkey.api.assay.AssayUrls; -import org.labkey.api.assay.plate.PlateReader; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.URLHelper; -import org.labkey.api.view.ActionURL; -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.VBox; -import org.labkey.api.view.ViewBackgroundInfo; -import org.labkey.api.view.WebPartView; -import org.labkey.elispot.pipeline.BackgroundSubtractionJob; -import org.labkey.elispot.pipeline.ElispotPipelineProvider; -import org.springframework.validation.BindException; -import org.springframework.validation.Errors; -import org.springframework.web.servlet.ModelAndView; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeMap; - -public class ElispotController extends SpringActionController -{ - private static final DefaultActionResolver _actionResolver = new DefaultActionResolver(ElispotController.class, ElispotUploadWizardAction.class); - - public ElispotController() - { - setActionResolver(_actionResolver); - } - - @RequiresPermission(ReadPermission.class) - public static class BeginAction extends SimpleViewAction - { - @Override - public ModelAndView getView(Object o, BindException errors) - { - return HttpView.redirect(urlProvider(AssayUrls.class).getAssayListURL(getContainer())); - } - - @Override - public void addNavTrail(NavTree root) - { - } - } - - @RequiresPermission(ReadPermission.class) - public class RunDetailsAction extends SimpleViewAction - { - private ExpProtocol _protocol; - private ExpRun _run; - private boolean _hasRunFilter; - - @Override - public ModelAndView getView(DetailsForm form, BindException errors) - { - _run = ExperimentService.get().getExpRun(form.getRowId()); - if (_run == null || !_run.getContainer().equals(getContainer())) - { - throw new NotFoundException("Run " + form.getRowId() + " does not exist."); - } - - _protocol = _run.getProtocol(); - _hasRunFilter = hasRunFilter(_protocol, getViewContext().getActionURL()); - - ElispotAssayProvider provider = (ElispotAssayProvider) AssayService.get().getProvider(_protocol); - if (null == provider) - throw new IllegalStateException("ElispotAssayProvider no found."); - - ElispotAssayProvider.DetectionMethodType method = provider.getDetectionMethod(getContainer(), _protocol); - - PlateSummaryBean bean = new PlateSummaryBean(); - bean.setRun(form.getRowId()); - bean.setFluorospot(method == ElispotAssayProvider.DetectionMethodType.FLUORESCENT); - - VBox view = new VBox(); - - JspView plateView = new JspView<>("/org/labkey/elispot/view/plateSummary.jsp", bean); - - plateView.setTitle("Plate Summary Information Run: " + form.getRowId()); - plateView.setFrame(WebPartView.FrameType.PORTAL); - - String tableName = ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME; - - // create the query view for antigen information - QuerySettings settings = new QuerySettings(getViewContext(), tableName, tableName); - settings.setAllowChooseView(true); - - CrosstabView queryView = new ElispotProtocolSchema(getUser(), getContainer(), provider, _protocol, null) - .createAntigenStatsQueryView(settings, errors, _hasRunFilter ? form.getRowId() : null); - - ElispotDetailsHeaderView header = new ElispotDetailsHeaderView(_protocol, provider, null); - ActionURL url = new ActionURL(RunDetailsAction.class, getContainer()).addParameter("rowId", form.getRowId()); - - if (_hasRunFilter) - { - header.getLinks().add(new NavTree("details for all runs", url)); - } - else - { - addRunFilter(_protocol, url, form.getRowId()); - header.getLinks().add(new NavTree("details for run " + form.getRowId(), url)); - } - view.addView(header); - view.addView(queryView); - view.addView(plateView); - return view; - } - - private boolean hasRunFilter(ExpProtocol protocol, ActionURL url) - { - String tableName = ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME; - SimpleFilter urlFilter = new SimpleFilter(getViewContext().getActionURL(), tableName); - List clauses = urlFilter.getClauses(); - - if (!clauses.isEmpty()) - { - AssayProvider provider = AssayService.get().getProvider(protocol); - FieldKey column = provider.getTableMetadata(protocol).getRunRowIdFieldKeyFromResults(); - - for (SimpleFilter.FilterClause clause : clauses) - { - if (clause.getFieldKeys().contains(column)) - return true; - } - } - return false; - } - - @Override - public void addNavTrail(NavTree root) - { - String title; - - if (_hasRunFilter) - title = "Run " + _run.getRowId() + " Details"; - else - title = "Details for All Runs"; - - ActionURL assayListURL = urlProvider(AssayUrls.class).getAssayListURL(_run.getContainer()); - ActionURL runListURL = urlProvider(AssayUrls.class).getAssayRunsURL(_run.getContainer(), _protocol); - ActionURL runDataURL = urlProvider(AssayUrls.class).getAssayResultsURL(_run.getContainer(), _protocol, _run.getRowId()); - - root.addChild("Assay List", assayListURL); - root.addChild(_protocol.getName() + " Runs", runListURL); - root.addChild(_protocol.getName() + " Data", runDataURL); - root.addChild(title); - } - } - - private List createWellInfoList(ExpRun run, ExpProtocol protocol, AbstractPlateBasedAssayProvider provider, - Plate template, PlateReader reader) - { - List wellInfos = new ArrayList<>(); - - List data = run.getOutputDatas(ExperimentService.get().getDataType(ElispotDataHandler.NAMESPACE)); - assert(data.size() == 1); - - Map inputs = new HashMap<>(); - for (Map.Entry e : run.getMaterialInputs().entrySet()) - inputs.put(e.getValue(), e.getKey()); - - for (int row=0; row < template.getRows(); row++) - { - for (int col=0; col < template.getColumns(); col++) - { - Position position = template.getPosition(row, col); - - Lsid dataRowLsid = ElispotDataHandler.getDataRowLsid(data.get(0).getLSID(), position); - for (RunDataRow dataRow : ElispotManager.get().getRunDataRows(dataRowLsid.toString(), run.getContainer())) - { - WellInfo wellInfo = new WellInfo(dataRow, position); - wellInfo.setSpotCount(reader.getWellDisplayValue(dataRow.getSpotCount())); - if (dataRow.getSpotSize() != null) - wellInfo.setSpotSize(reader.getWellDisplayValue(dataRow.getSpotSize())); - if (dataRow.getActivity() != null) - wellInfo.setActivity(String.valueOf(dataRow.getActivity())); - if (dataRow.getIntensity() != null) - wellInfo.setIntensity(String.valueOf(dataRow.getIntensity())); - - for (ObjectProperty prop : OntologyManager.getPropertyObjects(getContainer(), dataRowLsid.toString()).values()) - { - wellInfo.addWellProperty(prop); - } - wellInfos.add(wellInfo); - } - } - } - return wellInfos; - } - - private void addRunFilter(ExpProtocol protocol, ActionURL url, int rowId) - { - AssayProvider provider = AssayService.get().getProvider(protocol); - - url.addFilter(ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME, provider.getTableMetadata(protocol).getRunRowIdFieldKeyFromResults(), CompareType.EQUAL, rowId); - } - - private static class ElispotDetailsHeaderView extends AssayHeaderView - { - List _links = new ArrayList<>(); - - public ElispotDetailsHeaderView(ExpProtocol protocol, AssayProvider provider, ContainerFilter containerFilter) - { - super(protocol, provider, true, true, containerFilter); - - _links.add(new NavTree("view runs", PageFlowUtil.addLastFilterParameter(urlProvider(AssayUrls.class).getAssayRunsURL(getViewContext().getContainer(), _protocol, _containerFilter), AssayProtocolSchema.getLastFilterScope(_protocol)))); - } - - - @Override - public List getLinks() - { - return _links; - } - } - - @RequiresPermission(ReadPermission.class) - public class RunDetailRedirectAction extends SimpleRedirectAction - { - @Override - public ActionURL getRedirectURL(DetailsForm form) - { - ExpRun run = ExperimentService.get().getExpRun(form.getRowId()); - if (run == null) - { - throw new NotFoundException("Run " + form.getRowId() + " does not exist."); - } - - ActionURL url = new ActionURL(RunDetailsAction.class, getContainer()); - url.addParameter("rowId", form.getRowId()); - addRunFilter(run.getProtocol(), url, form.getRowId()); - - return url; - } - } - - @RequiresPermission(ReadPermission.class) - public class GetPlateSummary extends ReadOnlyApiAction - { - @Override - public ApiResponse execute(DetailsForm form, BindException errors) - { - ApiSimpleResponse response = new ApiSimpleResponse(); - - ExpRun run = ExperimentService.get().getExpRun(form.getRowId()); - if (run == null || !run.getContainer().equals(getContainer())) - { - throw new NotFoundException("Run " + form.getRowId() + " does not exist."); - } - - ExpProtocol protocol = run.getProtocol(); - - ElispotAssayProvider provider = (ElispotAssayProvider) AssayService.get().getProvider(protocol); - Plate template = provider.getPlate(getContainer(), protocol); - - String plateReaderName = null; - for (ObjectProperty prop : run.getObjectProperties().values()) - { - if (ElispotAssayProvider.READER_PROPERTY_NAME.equals(prop.getName())) - { - plateReaderName = prop.getStringValue(); - break; - } - } - - if (plateReaderName != null) - { - PlateReader reader = provider.getPlateReader(plateReaderName); - JSONArray rows = new JSONArray(); - Map analyteMap = new TreeMap<>(); - - for (WellInfo wellInfo : createWellInfoList(run, protocol, provider, template, reader)) - { - rows.put(wellInfo.toJSON()); - if (wellInfo.getAnalyte() != null) - { - if (wellInfo.getCytokine() != null) - analyteMap.put(wellInfo.getAnalyte(), wellInfo.getCytokine()); - else - analyteMap.put(wellInfo.getAnalyte(), ""); - } - } - response.put("summary", rows); - if (!analyteMap.isEmpty()) - { - response.put("analytes", analyteMap.keySet()); - response.put("analyteMap", analyteMap); - } - response.put("success", true); - } - return response; - } - } - - public static class DetailsForm - { - private int _rowId; - - public int getRowId() - { - return _rowId; - } - - public void setRowId(int rowId) - { - _rowId = rowId; - } - } - - public static class WellInfo - { - private String _dataRowLsid; - private String _spotCount = ""; - private String _spotSize = ""; - private String _activity = ""; - private String _intensity = ""; - private final Map _wellProperties = new LinkedHashMap<>(); - private final RunDataRow _runDataRow; - private final Position _position; - - public WellInfo(@Nullable RunDataRow runDataRow, Position position) - { - _runDataRow = runDataRow; - _position = position; - } - - public String getDataRowLsid() - { - return _dataRowLsid; - } - - public void setDataRowLsid(String dataRowLsid) - { - _dataRowLsid = dataRowLsid; - } - - public void addWellProperty(ObjectProperty prop) - { - _wellProperties.put(prop.getName(), prop); - } - - public Map getWellProperties() - { - return _wellProperties; - } - - public String getSpotCount() - { - return _spotCount; - } - - public void setSpotCount(String spotCount) - { - _spotCount = spotCount; - } - - public String getSpotSize() - { - return _spotSize; - } - - public void setSpotSize(String spotSize) - { - _spotSize = spotSize; - } - - public String getActivity() - { - return _activity; - } - - public void setActivity(String activity) - { - _activity = activity; - } - - public String getIntensity() - { - return _intensity; - } - - public void setIntensity(String intensity) - { - _intensity = intensity; - } - - public Position getPosition() - { - return _position; - } - - public String getAnalyte() - { - return _runDataRow.getAnalyte(); - } - - public String getCytokine() - { - return _runDataRow.getCytokine(); - } - - public JSONObject toJSON() - { - JSONObject well = new JSONObject(); - - well.put("spotCount", getSpotCount()); - well.put("activity", getActivity()); - well.put("intensity", getIntensity()); - well.put("dataRowLsid", getDataRowLsid()); - well.put("position", _position.toString()); - well.put("analyte", getAnalyte()); - well.put("cytokine", getCytokine()); - well.put("spotSize", getSpotSize()); - - JSONObject wellProps = new JSONObject(); - for (ObjectProperty prop : _wellProperties.values()) - { - // don't need the specimen lsid - if (!ElispotDataHandler.ELISPOT_INPUT_MATERIAL_DATA_PROPERTY.equals(prop.getName())) - wellProps.put(prop.getName(), String.valueOf(prop.value())); - } - - if (null != _runDataRow) - { - wellProps.put("SpotCount", _runDataRow.getSpotCount()); - wellProps.put("AntigenWellgroupName", _runDataRow.getAntigenWellgroupName()); - wellProps.put("WellgroupName", _runDataRow.getWellgroupName()); - wellProps.put("WellgroupLocation", _runDataRow.getWellgroupLocation()); - if (null != _runDataRow.getNormalizedSpotCount()) - wellProps.put("NormalizedSpotCount", _runDataRow.getNormalizedSpotCount()); - if (null != _runDataRow.getActivity()) - wellProps.put("Activity", _runDataRow.getActivity()); - if (null != _runDataRow.getAnalyte()) - wellProps.put("Analyte", _runDataRow.getAnalyte()); - if (null != _runDataRow.getCytokine()) - wellProps.put("Cytokine", _runDataRow.getCytokine()); - if (null != _runDataRow.getIntensity()) - wellProps.put("Intensity", _runDataRow.getIntensity()); - } - well.put("wellProperties", wellProps); - - return well; - } - } - - public static class PlateSummaryBean - { - private int _run; - private boolean _fluorospot; - - public int getRun() - { - return _run; - } - - public void setRun(int run) - { - _run = run; - } - - public boolean isFluorospot() - { - return _fluorospot; - } - - public void setFluorospot(boolean fluorospot) - { - _fluorospot = fluorospot; - } - } - - @RequiresPermission(InsertPermission.class) - public static class BackgroundSubtractionAction extends FormHandlerAction - { - @Override - public void validateCommand(Object target, Errors errors) - { - } - - @Override - public boolean handlePost(Object o, BindException errors) throws Exception - { - Set selections = DataRegionSelection.getSelected(getViewContext(), true); - if (!selections.isEmpty()) - { - ViewBackgroundInfo info = new ViewBackgroundInfo(getContainer(), getUser(), getViewContext().getActionURL()); - BackgroundSubtractionJob job = new BackgroundSubtractionJob(ElispotPipelineProvider.NAME, info, - PipelineService.get().findPipelineRoot(getContainer()), selections); - - PipelineService.get().queueJob(job); - - return true; - } - return false; - } - - @Override - public URLHelper getSuccessURL(Object o) - { - return urlProvider(PipelineUrls.class).urlBegin(getContainer()); - } - } -} +/* + * Copyright (c) 2008-2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.labkey.elispot; + +import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.math.NumberUtils; +import org.jetbrains.annotations.Nullable; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Before; +import org.junit.Test; +import org.labkey.api.action.ApiResponse; +import org.labkey.api.action.ApiSimpleResponse; +import org.labkey.api.action.FormHandlerAction; +import org.labkey.api.action.ReadOnlyApiAction; +import org.labkey.api.action.SimpleRedirectAction; +import org.labkey.api.action.SimpleViewAction; +import org.labkey.api.action.SpringActionController; +import org.labkey.api.assay.AssayProtocolSchema; +import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.AssayService; +import org.labkey.api.assay.AssayUrls; +import org.labkey.api.assay.actions.AssayHeaderView; +import org.labkey.api.assay.plate.AbstractPlateBasedAssayProvider; +import org.labkey.api.assay.plate.Plate; +import org.labkey.api.assay.plate.PlateReader; +import org.labkey.api.assay.plate.Position; +import org.labkey.api.data.CompareType; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.DataRegionSelection; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.exp.Lsid; +import org.labkey.api.exp.ObjectProperty; +import org.labkey.api.exp.OntologyManager; +import org.labkey.api.exp.api.ExpData; +import org.labkey.api.exp.api.ExpMaterial; +import org.labkey.api.exp.api.ExpProtocol; +import org.labkey.api.exp.api.ExpRun; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.pipeline.PipeRoot; +import org.labkey.api.pipeline.PipelineService; +import org.labkey.api.pipeline.PipelineUrls; +import org.labkey.api.query.CrosstabView; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.QuerySettings; +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.roles.ReaderRole; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.URLHelper; +import org.labkey.api.view.ActionURL; +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.VBox; +import org.labkey.api.view.ViewBackgroundInfo; +import org.labkey.api.view.WebPartView; +import org.labkey.elispot.pipeline.BackgroundSubtractionJob; +import org.labkey.elispot.pipeline.ElispotPipelineProvider; +import org.springframework.validation.BindException; +import org.springframework.validation.Errors; +import org.springframework.web.servlet.ModelAndView; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +public class ElispotController extends SpringActionController +{ + private static final DefaultActionResolver _actionResolver = new DefaultActionResolver(ElispotController.class, ElispotUploadWizardAction.class); + + public ElispotController() + { + setActionResolver(_actionResolver); + } + + @RequiresPermission(ReadPermission.class) + public static class BeginAction extends SimpleViewAction + { + @Override + public ModelAndView getView(Object o, BindException errors) + { + return HttpView.redirect(urlProvider(AssayUrls.class).getAssayListURL(getContainer())); + } + + @Override + public void addNavTrail(NavTree root) + { + } + } + + @RequiresPermission(ReadPermission.class) + public class RunDetailsAction extends SimpleViewAction + { + private ExpProtocol _protocol; + private ExpRun _run; + private boolean _hasRunFilter; + + @Override + public ModelAndView getView(DetailsForm form, BindException errors) + { + _run = ExperimentService.get().getExpRun(form.getRowId()); + if (_run == null || !_run.getContainer().equals(getContainer())) + { + throw new NotFoundException("Run " + form.getRowId() + " does not exist."); + } + + _protocol = _run.getProtocol(); + _hasRunFilter = hasRunFilter(_protocol, getViewContext().getActionURL()); + + ElispotAssayProvider provider = (ElispotAssayProvider) AssayService.get().getProvider(_protocol); + if (null == provider) + throw new IllegalStateException("ElispotAssayProvider no found."); + + ElispotAssayProvider.DetectionMethodType method = provider.getDetectionMethod(getContainer(), _protocol); + + PlateSummaryBean bean = new PlateSummaryBean(); + bean.setRun(form.getRowId()); + bean.setFluorospot(method == ElispotAssayProvider.DetectionMethodType.FLUORESCENT); + + VBox view = new VBox(); + + JspView plateView = new JspView<>("/org/labkey/elispot/view/plateSummary.jsp", bean); + + plateView.setTitle("Plate Summary Information Run: " + form.getRowId()); + plateView.setFrame(WebPartView.FrameType.PORTAL); + + String tableName = ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME; + + // create the query view for antigen information + QuerySettings settings = new QuerySettings(getViewContext(), tableName, tableName); + settings.setAllowChooseView(true); + + CrosstabView queryView = new ElispotProtocolSchema(getUser(), getContainer(), provider, _protocol, null) + .createAntigenStatsQueryView(settings, errors, _hasRunFilter ? form.getRowId() : null); + + ElispotDetailsHeaderView header = new ElispotDetailsHeaderView(_protocol, provider, null); + ActionURL url = new ActionURL(RunDetailsAction.class, getContainer()).addParameter("rowId", form.getRowId()); + + if (_hasRunFilter) + { + header.getLinks().add(new NavTree("details for all runs", url)); + } + else + { + addRunFilter(_protocol, url, form.getRowId()); + header.getLinks().add(new NavTree("details for run " + form.getRowId(), url)); + } + view.addView(header); + view.addView(queryView); + view.addView(plateView); + return view; + } + + private boolean hasRunFilter(ExpProtocol protocol, ActionURL url) + { + String tableName = ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME; + SimpleFilter urlFilter = new SimpleFilter(getViewContext().getActionURL(), tableName); + List clauses = urlFilter.getClauses(); + + if (!clauses.isEmpty()) + { + AssayProvider provider = AssayService.get().getProvider(protocol); + FieldKey column = provider.getTableMetadata(protocol).getRunRowIdFieldKeyFromResults(); + + for (SimpleFilter.FilterClause clause : clauses) + { + if (clause.getFieldKeys().contains(column)) + return true; + } + } + return false; + } + + @Override + public void addNavTrail(NavTree root) + { + String title; + + if (_hasRunFilter) + title = "Run " + _run.getRowId() + " Details"; + else + title = "Details for All Runs"; + + ActionURL assayListURL = urlProvider(AssayUrls.class).getAssayListURL(_run.getContainer()); + ActionURL runListURL = urlProvider(AssayUrls.class).getAssayRunsURL(_run.getContainer(), _protocol); + ActionURL runDataURL = urlProvider(AssayUrls.class).getAssayResultsURL(_run.getContainer(), _protocol, _run.getRowId()); + + root.addChild("Assay List", assayListURL); + root.addChild(_protocol.getName() + " Runs", runListURL); + root.addChild(_protocol.getName() + " Data", runDataURL); + root.addChild(title); + } + } + + private List createWellInfoList(ExpRun run, ExpProtocol protocol, AbstractPlateBasedAssayProvider provider, + Plate template, PlateReader reader) + { + List wellInfos = new ArrayList<>(); + + List data = run.getOutputDatas(ExperimentService.get().getDataType(ElispotDataHandler.NAMESPACE)); + assert(data.size() == 1); + + Map inputs = new HashMap<>(); + for (Map.Entry e : run.getMaterialInputs().entrySet()) + inputs.put(e.getValue(), e.getKey()); + + for (int row=0; row < template.getRows(); row++) + { + for (int col=0; col < template.getColumns(); col++) + { + Position position = template.getPosition(row, col); + + Lsid dataRowLsid = ElispotDataHandler.getDataRowLsid(data.get(0).getLSID(), position); + for (RunDataRow dataRow : ElispotManager.get().getRunDataRows(dataRowLsid.toString(), run.getContainer())) + { + WellInfo wellInfo = new WellInfo(dataRow, position); + wellInfo.setSpotCount(reader.getWellDisplayValue(dataRow.getSpotCount())); + if (dataRow.getSpotSize() != null) + wellInfo.setSpotSize(reader.getWellDisplayValue(dataRow.getSpotSize())); + if (dataRow.getActivity() != null) + wellInfo.setActivity(String.valueOf(dataRow.getActivity())); + if (dataRow.getIntensity() != null) + wellInfo.setIntensity(String.valueOf(dataRow.getIntensity())); + + for (ObjectProperty prop : OntologyManager.getPropertyObjects(getContainer(), dataRowLsid.toString()).values()) + { + wellInfo.addWellProperty(prop); + } + wellInfos.add(wellInfo); + } + } + } + return wellInfos; + } + + private void addRunFilter(ExpProtocol protocol, ActionURL url, int rowId) + { + AssayProvider provider = AssayService.get().getProvider(protocol); + + url.addFilter(ElispotProtocolSchema.ANTIGEN_STATS_TABLE_NAME, provider.getTableMetadata(protocol).getRunRowIdFieldKeyFromResults(), CompareType.EQUAL, rowId); + } + + private static class ElispotDetailsHeaderView extends AssayHeaderView + { + List _links = new ArrayList<>(); + + public ElispotDetailsHeaderView(ExpProtocol protocol, AssayProvider provider, ContainerFilter containerFilter) + { + super(protocol, provider, true, true, containerFilter); + + _links.add(new NavTree("view runs", PageFlowUtil.addLastFilterParameter(urlProvider(AssayUrls.class).getAssayRunsURL(getViewContext().getContainer(), _protocol, _containerFilter), AssayProtocolSchema.getLastFilterScope(_protocol)))); + } + + + @Override + public List getLinks() + { + return _links; + } + } + + @RequiresPermission(ReadPermission.class) + public class RunDetailRedirectAction extends SimpleRedirectAction + { + @Override + public ActionURL getRedirectURL(DetailsForm form) + { + // GitHub Kanban #1236: getExpRun() resolves by rowId; ensure the run belongs to the current container + ExpRun run = ExperimentService.get().getExpRun(form.getRowId(), getContainer()); + if (run == null) + { + throw new NotFoundException("Run " + form.getRowId() + " does not exist."); + } + + ActionURL url = new ActionURL(RunDetailsAction.class, getContainer()); + url.addParameter("rowId", form.getRowId()); + addRunFilter(run.getProtocol(), url, form.getRowId()); + + return url; + } + } + + @RequiresPermission(ReadPermission.class) + public class GetPlateSummary extends ReadOnlyApiAction + { + @Override + public ApiResponse execute(DetailsForm form, BindException errors) + { + ApiSimpleResponse response = new ApiSimpleResponse(); + + ExpRun run = ExperimentService.get().getExpRun(form.getRowId()); + if (run == null || !run.getContainer().equals(getContainer())) + { + throw new NotFoundException("Run " + form.getRowId() + " does not exist."); + } + + ExpProtocol protocol = run.getProtocol(); + + ElispotAssayProvider provider = (ElispotAssayProvider) AssayService.get().getProvider(protocol); + Plate template = provider.getPlate(getContainer(), protocol); + + String plateReaderName = null; + for (ObjectProperty prop : run.getObjectProperties().values()) + { + if (ElispotAssayProvider.READER_PROPERTY_NAME.equals(prop.getName())) + { + plateReaderName = prop.getStringValue(); + break; + } + } + + if (plateReaderName != null) + { + PlateReader reader = provider.getPlateReader(plateReaderName); + JSONArray rows = new JSONArray(); + Map analyteMap = new TreeMap<>(); + + for (WellInfo wellInfo : createWellInfoList(run, protocol, provider, template, reader)) + { + rows.put(wellInfo.toJSON()); + if (wellInfo.getAnalyte() != null) + { + if (wellInfo.getCytokine() != null) + analyteMap.put(wellInfo.getAnalyte(), wellInfo.getCytokine()); + else + analyteMap.put(wellInfo.getAnalyte(), ""); + } + } + response.put("summary", rows); + if (!analyteMap.isEmpty()) + { + response.put("analytes", analyteMap.keySet()); + response.put("analyteMap", analyteMap); + } + response.put("success", true); + } + return response; + } + } + + public static class DetailsForm + { + private int _rowId; + + public int getRowId() + { + return _rowId; + } + + public void setRowId(int rowId) + { + _rowId = rowId; + } + } + + public static class WellInfo + { + private String _dataRowLsid; + private String _spotCount = ""; + private String _spotSize = ""; + private String _activity = ""; + private String _intensity = ""; + private final Map _wellProperties = new LinkedHashMap<>(); + private final RunDataRow _runDataRow; + private final Position _position; + + public WellInfo(@Nullable RunDataRow runDataRow, Position position) + { + _runDataRow = runDataRow; + _position = position; + } + + public String getDataRowLsid() + { + return _dataRowLsid; + } + + public void setDataRowLsid(String dataRowLsid) + { + _dataRowLsid = dataRowLsid; + } + + public void addWellProperty(ObjectProperty prop) + { + _wellProperties.put(prop.getName(), prop); + } + + public Map getWellProperties() + { + return _wellProperties; + } + + public String getSpotCount() + { + return _spotCount; + } + + public void setSpotCount(String spotCount) + { + _spotCount = spotCount; + } + + public String getSpotSize() + { + return _spotSize; + } + + public void setSpotSize(String spotSize) + { + _spotSize = spotSize; + } + + public String getActivity() + { + return _activity; + } + + public void setActivity(String activity) + { + _activity = activity; + } + + public String getIntensity() + { + return _intensity; + } + + public void setIntensity(String intensity) + { + _intensity = intensity; + } + + public Position getPosition() + { + return _position; + } + + public String getAnalyte() + { + return _runDataRow.getAnalyte(); + } + + public String getCytokine() + { + return _runDataRow.getCytokine(); + } + + public JSONObject toJSON() + { + JSONObject well = new JSONObject(); + + well.put("spotCount", getSpotCount()); + well.put("activity", getActivity()); + well.put("intensity", getIntensity()); + well.put("dataRowLsid", getDataRowLsid()); + well.put("position", _position.toString()); + well.put("analyte", getAnalyte()); + well.put("cytokine", getCytokine()); + well.put("spotSize", getSpotSize()); + + JSONObject wellProps = new JSONObject(); + for (ObjectProperty prop : _wellProperties.values()) + { + // don't need the specimen lsid + if (!ElispotDataHandler.ELISPOT_INPUT_MATERIAL_DATA_PROPERTY.equals(prop.getName())) + wellProps.put(prop.getName(), String.valueOf(prop.value())); + } + + if (null != _runDataRow) + { + wellProps.put("SpotCount", _runDataRow.getSpotCount()); + wellProps.put("AntigenWellgroupName", _runDataRow.getAntigenWellgroupName()); + wellProps.put("WellgroupName", _runDataRow.getWellgroupName()); + wellProps.put("WellgroupLocation", _runDataRow.getWellgroupLocation()); + if (null != _runDataRow.getNormalizedSpotCount()) + wellProps.put("NormalizedSpotCount", _runDataRow.getNormalizedSpotCount()); + if (null != _runDataRow.getActivity()) + wellProps.put("Activity", _runDataRow.getActivity()); + if (null != _runDataRow.getAnalyte()) + wellProps.put("Analyte", _runDataRow.getAnalyte()); + if (null != _runDataRow.getCytokine()) + wellProps.put("Cytokine", _runDataRow.getCytokine()); + if (null != _runDataRow.getIntensity()) + wellProps.put("Intensity", _runDataRow.getIntensity()); + } + well.put("wellProperties", wellProps); + + return well; + } + } + + public static class PlateSummaryBean + { + private int _run; + private boolean _fluorospot; + + public int getRun() + { + return _run; + } + + public void setRun(int run) + { + _run = run; + } + + public boolean isFluorospot() + { + return _fluorospot; + } + + public void setFluorospot(boolean fluorospot) + { + _fluorospot = fluorospot; + } + } + + @RequiresPermission(InsertPermission.class) + public static class BackgroundSubtractionAction extends FormHandlerAction + { + @Override + public void validateCommand(Object target, Errors errors) + { + } + + @Override + 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); + + PipelineService.get().queueJob(job); + + return true; + } + return false; + } + + @Override + public URLHelper getSuccessURL(Object o) + { + return urlProvider(PipelineUrls.class).urlBegin(getContainer()); + } + } + + /** + * GitHub Kanban #1236 regression test + */ + public static class ContainerScopingTestCase extends AbstractContainerScopingTest + { + private Container _folderA; + private Container _folderB; + private User _readerA; + + @Before + public void setup() throws Exception + { + _folderA = createContainer("A"); + _folderB = createContainer("B"); + _readerA = createUserInRole(_folderA, ReaderRole.class); + } + + @Test + public void testRunDetailRedirectContainerScoping() throws Exception + { + // A run that genuinely exists in folder B. Its global rowId is valid; only its container differs. + ExpRun runInB = createRun(_folderB); + + // Deny: requesting B's run from folder A must 404 -- for a reader in A and for a site admin who can read B -- + // proving the rejection is the container scoping, not a permission failure. Pre-fix getExpRun(rowId) was + // global and only null-checked, so this issued a redirect that disclosed the foreign run's existence. + ActionURL foreignUrl = new ActionURL(RunDetailRedirectAction.class, _folderA).addParameter("rowId", runInB.getRowId()); + assertStatus(HttpServletResponse.SC_NOT_FOUND, get(foreignUrl, _readerA)); + assertStatus(HttpServletResponse.SC_NOT_FOUND, get(foreignUrl, getAdmin())); + + // Control: the same run resolves through the container-scoped lookup from its own container but not from + // folder A, demonstrating the mechanism the fix relies on (the run exists identically in both calls). + ExperimentService exp = ExperimentService.get(); + assertNotNull("Run should resolve within its own container", exp.getExpRun(runInB.getRowId(), _folderB)); + assertNull("Run must not resolve from a foreign container", exp.getExpRun(runInB.getRowId(), _folderA)); + } + + private ExpRun createRun(Container c) throws Exception + { + ExperimentService exp = ExperimentService.get(); + ExpRun run = exp.createExperimentRun(c, "elispot-2-scope-test"); + PipeRoot root = PipelineService.get().findPipelineRoot(c); + assertNotNull("Test requires a pipeline root for " + c.getName(), root); + run.setFilePathRoot(root.getRootPath()); + + // for this test case it won't matter if the run is not an elispot assay run + run.setProtocol(exp.ensureSampleDerivationProtocol(getAdmin())); + + ViewBackgroundInfo info = new ViewBackgroundInfo(c, getAdmin(), null); + return exp.saveSimpleExperimentRun(run, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), + Collections.emptyMap(), Collections.emptyMap(), info, null, false); + } + } +} diff --git a/elispotassay/src/org/labkey/elispot/ElispotModule.java b/elispotassay/src/org/labkey/elispot/ElispotModule.java index 508cc45a16..0f1f25a0b7 100644 --- a/elispotassay/src/org/labkey/elispot/ElispotModule.java +++ b/elispotassay/src/org/labkey/elispot/ElispotModule.java @@ -96,4 +96,10 @@ public void doStartup(ModuleContext moduleContext) PipelineService.get().registerPipelineProvider(new ElispotPipelineProvider(this)); } + + @Override + public @NotNull Set> getIntegrationTests() + { + return Set.of(ElispotController.ContainerScopingTestCase.class); + } } diff --git a/flow/src/org/labkey/flow/FlowModule.java b/flow/src/org/labkey/flow/FlowModule.java index bdefb2d370..dda5630d20 100644 --- a/flow/src/org/labkey/flow/FlowModule.java +++ b/flow/src/org/labkey/flow/FlowModule.java @@ -335,7 +335,10 @@ public Set getSchemaNames() 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 1c31f7bd9b..55afc28b59 100644 --- a/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java +++ b/flow/src/org/labkey/flow/controllers/editscript/EditScriptForm.java @@ -16,8 +16,8 @@ package org.labkey.flow.controllers.editscript; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.fhcrc.cpas.flow.script.xml.ScriptDocument; import org.jetbrains.annotations.NotNull; import org.labkey.api.security.permissions.UpdatePermission; @@ -92,6 +92,8 @@ public void reset() { throw new NotFoundException("scriptId not found: " + scriptIdStr); } + // GitHub Issue #1892: validate container + flowObject.checkContainer(getContainer(), getUser(), getViewContext().getActionURL()); _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 ca8a278388..9ecd139c3f 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; @@ -87,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; @@ -187,16 +198,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)); @@ -218,6 +233,7 @@ public class ChooseRunsToAnalyzeAction extends BaseAnalyzeRunsAction public ModelAndView getView(ChooseRunsToAnalyzeForm form, BindException errors) { script = form.getProtocol(); + checkContainer(script); return chooseRunsToAnalyze(form, errors); } } @@ -229,12 +245,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) { @@ -852,6 +875,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."); @@ -1504,4 +1533,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 long _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 af765567b3..65a4156f34 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..31ea53555f 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 long _wellIdA; + private long _scriptIdB; + private long _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 long 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 + { + long 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 da3ff7bb95..5eac05c882 100644 --- a/flow/src/org/labkey/flow/data/FlowProtocol.java +++ b/flow/src/org/labkey/flow/data/FlowProtocol.java @@ -74,6 +74,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; @@ -152,15 +153,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(); @@ -168,11 +173,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 da086456ba..3bf6a17c1a 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.collections.IntHashMap; @@ -47,7 +49,9 @@ 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; import org.labkey.api.module.ModuleLoader; import org.labkey.api.query.BatchValidationException; @@ -55,20 +59,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; @@ -1776,4 +1791,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", + "