diff --git a/elisa/src/org/labkey/elisa/ElisaController.java b/elisa/src/org/labkey/elisa/ElisaController.java index d5a75c789f..815354b47b 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 888a37b528..b478d01c19 100644 --- a/elispotassay/src/org/labkey/elispot/ElispotController.java +++ b/elispotassay/src/org/labkey/elispot/ElispotController.java @@ -1,573 +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.apache.commons.lang3.math.NumberUtils; -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()) - { - // 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()); - } - } -} +/* + * 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 3d8072f0b1..b95008a9f8 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/luminex/src/org/labkey/luminex/LuminexController.java b/luminex/src/org/labkey/luminex/LuminexController.java index 0e118bec3c..0a62d5bbf8 100644 --- a/luminex/src/org/labkey/luminex/LuminexController.java +++ b/luminex/src/org/labkey/luminex/LuminexController.java @@ -733,7 +733,10 @@ public GuideSetConfirmDeleteView(DeleteForm form) Set selections = DataRegionSelection.getSelectedIntegers(getViewContext(), false); + // GitHub Kanban #1236: guide sets are scoped to the protocol rather than the container (see GuideSetTable.getContainerFilter), + // so scope this read to the current (validated, in-scope) protocol's RowId SimpleFilter filter = new SimpleFilter(); + filter.addCondition(FieldKey.fromParts("ProtocolId"), form.getProtocol().getRowId()); filter.addInClause(FieldKey.fromParts("RowId"), selections); List guideSets = new TableSelector(LuminexProtocolSchema.getTableInfoGuideSet(), filter, null).getArrayList(GuideSet.class); diff --git a/luminex/test/src/org/labkey/test/tests/luminex/LuminexGuideSetTest.java b/luminex/test/src/org/labkey/test/tests/luminex/LuminexGuideSetTest.java index a63fdfd83f..a6526baeea 100644 --- a/luminex/test/src/org/labkey/test/tests/luminex/LuminexGuideSetTest.java +++ b/luminex/test/src/org/labkey/test/tests/luminex/LuminexGuideSetTest.java @@ -40,6 +40,8 @@ import org.labkey.test.util.ExtHelper; import org.labkey.test.util.LogMethod; import org.labkey.test.util.PortalHelper; +import org.labkey.test.util.SimpleHttpRequest; +import org.labkey.test.util.SimpleHttpResponse; import org.labkey.test.util.WikiHelper; import org.labkey.test.util.luminex.LuminexGuideSetHelper; import org.openqa.selenium.WebElement; @@ -51,6 +53,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @Category({Daily.class, Assays.class}) @@ -701,6 +704,67 @@ public void testCrossProtocolGuideSetDeleteDenied() throws Exception assertEquals("Guide set should be deletable through its own protocol's query", 0, guideSetRowCount(connection, schemaB, folderB, guideSetIdB)); } + @Test // GitHub Kanban #... + public void testCrossProtocolGuideSetConfirmViewDenied() throws Exception + { + final String assayB = "GuideSetAuthConfirmAssayB"; + final String analyteB = "Auth Confirm Analyte"; + // distinctive, HTML-safe token that is only ever stored in protocol B's guide set comment + final String commentMarker = "LUM2DisclosureMarker"; + final String folderB = getProjectName() + "/" + TEST_ASSAY_SUBFOLDER; + Connection connection = createDefaultConnection(); + + log("Create a second Luminex assay design in the subfolder via the API"); + Protocol protocolB = new GetProtocolCommand("Luminex").execute(connection, folderB).getProtocol(); + protocolB.setName(assayB).setDescription("Second Luminex design for guide set confirm-view disclosure test"); + new SaveProtocolCommand(protocolB).execute(connection, folderB); + + final String schemaB = "assay.Luminex." + QueryKey.encodePart(assayB); + + log("Insert a guide set with a distinctive comment into the second assay design"); + InsertRowsCommand insertB = new InsertRowsCommand(schemaB, "GuideSet"); + Map guideSet = new HashMap<>(); + guideSet.put("AnalyteName", analyteB); + guideSet.put("CurrentGuideSet", false); + guideSet.put("Comment", commentMarker); + insertB.addRow(guideSet); + insertB.execute(connection, folderB); + long guideSetIdB = getGuideSetRowId(connection, schemaB, folderB, analyteB); + + log("Request the delete-confirm view through the first protocol (in the project), selecting protocol B's guide set - its comment must NOT be disclosed"); + String crossBody = getGuideSetConfirmView(getProjectName(), TEST_ASSAY_LUM, guideSetIdB); + assertFalse("Cross-protocol delete-confirm view must not disclose the other protocol's guide set comment", crossBody.contains(commentMarker)); + + log("Request the delete-confirm view through protocol B's own design - its comment is disclosed (positive control)"); + String ownBody = getGuideSetConfirmView(folderB, assayB, guideSetIdB); + assertTrue("Delete-confirm view should show the guide set comment for the guide set's own protocol", ownBody.contains(commentMarker)); + + log("Clean up the guide set inserted for this test"); + DeleteRowsCommand cleanup = new DeleteRowsCommand(schemaB, "GuideSet"); + Map key = new HashMap<>(); + key.put("RowId", guideSetIdB); + cleanup.addRow(key); + cleanup.execute(connection, folderB); + } + + /** + * GET the DeleteGuideSetAction confirm view (a FormViewAction GET renders getView, building the bean directly via a + * raw TableSelector). The selection is passed as the DataRegion ".select" request param, which getSelectedIntegers + * reads straight from request params - so no session selection key plumbing is needed. Returns the response body. + */ + private String getGuideSetConfirmView(String containerPath, String assayName, long guideSetRowId) throws Exception + { + Map params = new HashMap<>(); + params.put("assayName", assayName); + params.put(".select", guideSetRowId); + String url = WebTestHelper.buildURL("luminex", containerPath, "deleteGuideSet", params); + SimpleHttpRequest request = new SimpleHttpRequest(url); + request.copySession(getDriver()); + SimpleHttpResponse response = request.getResponse(); + assertEquals("Unexpected status from delete-confirm view", 200, response.getResponseCode()); + return response.getResponseBody(); + } + private int guideSetRowCount(Connection connection, String schema, String folderPath, long rowId) throws Exception { SelectRowsCommand select = new SelectRowsCommand(schema, "GuideSet"); diff --git a/nab/src/org/labkey/nab/NabAssayController.java b/nab/src/org/labkey/nab/NabAssayController.java index 00b0612c5c..2ac7d1b1b3 100644 --- a/nab/src/org/labkey/nab/NabAssayController.java +++ b/nab/src/org/labkey/nab/NabAssayController.java @@ -50,9 +50,9 @@ import org.labkey.api.assay.dilution.DilutionDataHandler; import org.labkey.api.assay.dilution.DilutionDataRow; import org.labkey.api.assay.dilution.DilutionManager; -import org.labkey.api.assay.dilution.query.DilutionProviderSchema; import org.labkey.api.assay.dilution.DilutionSummary; import org.labkey.api.assay.dilution.WellDataRow; +import org.labkey.api.assay.dilution.query.DilutionProviderSchema; import org.labkey.api.assay.nab.Luc5Assay; import org.labkey.api.assay.nab.NabUrls; import org.labkey.api.assay.nab.RenderAssayBean; @@ -676,7 +676,9 @@ public void export(SampleSpreadsheetForm sampleSpreadsheetForm, HttpServletRespo { ViewContext context = getViewContext(); ExpProtocol protocol = ExperimentService.get().getExpProtocol(sampleSpreadsheetForm.getProtocol()); - if (protocol == null) + // GitHub Kanban #1236: verify protocol is in scope for the specified container. + if (protocol == null || + AssayService.get().getAssayProtocols(getContainer()).stream().noneMatch(p -> p.getRowId() == protocol.getRowId())) { throw new NotFoundException("Protocol " + sampleSpreadsheetForm.getProtocol() + " does not exist."); } diff --git a/signalData/src/org/labkey/signaldata/SignalDataController.java b/signalData/src/org/labkey/signaldata/SignalDataController.java index 228fc5b3d1..b526f044a8 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataController.java +++ b/signalData/src/org/labkey/signaldata/SignalDataController.java @@ -16,6 +16,10 @@ package org.labkey.signaldata; +import jakarta.servlet.http.HttpServletResponse; +import org.jetbrains.annotations.Nullable; +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.MutatingApiAction; @@ -33,9 +37,14 @@ import org.labkey.api.pipeline.PipelineService; import org.labkey.api.query.QueryUpdateService; 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.ReadPermission; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.Path; import org.labkey.api.util.UnexpectedException; +import org.labkey.api.view.ActionURL; import org.labkey.api.webdav.WebdavResource; import org.labkey.api.webdav.WebdavService; import org.labkey.signaldata.assay.SignalDataAssayDataHandler; @@ -44,6 +53,7 @@ import java.io.File; import java.net.MalformedURLException; import java.net.URI; +import java.nio.file.Files; import java.sql.SQLException; import java.util.Collections; import java.util.HashMap; @@ -137,7 +147,7 @@ public ApiResponse execute(SignalDataResourceForm form, BindException errors) Map props = new HashMap<>(); Container c = getContainer(); - if (null != resource) + if (isAuthorizedResource(getUser(), c, resource)) { FileContentService svc = FileContentService.get(); ExpData data = svc.getDataObject(resource, c); @@ -212,4 +222,100 @@ public ApiResponse execute(SignalDataResourceForm form, BindException errors) return new ApiSimpleResponse(props); } } + + /** + * GitHub Issue 1236: the requested path is client-supplied and WebdavService.lookup() resolves globally with no ACL + * or container scoping. Before disclosing a resource's metadata or creating an exp.data row that references + * it, require that (a) the caller can actually read the resolved resource and (b) it lives under this + * container's pipeline root. Signal data files always reside under the container's own pipeline root, so a + * path resolving outside it indicates a cross-container probe rather than a legitimate request. + */ + static boolean isAuthorizedResource(User user, Container c, @Nullable WebdavResource resource) + { + if (null == resource || !resource.canRead(user, true)) + return false; + + File file = resource.getFile(); + if (null == file) + return false; + + PipeRoot root = PipelineService.get().findPipelineRoot(c); + return null != root && root.isUnderRoot(file); + } + + /** + * GitHub Issue 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 testGetSignalDataResourceContainerScoping() throws Exception + { + File foreignFile = writeFileUnderFileRoot(_folderB, "foreign.txt"); + ExperimentService exp = ExperimentService.get(); + + // Deny (read ACL): a caller who can read folder A but NOT folder B requests a file in B's pipeline root + // with test=true. The action returns 200 with empty props either way, so we assert the real side effect: + // no exp.data row is created in folder A referencing the foreign file. Pre-fix the global WebdavService + // lookup resolved the file with no ACL check and createData() persisted a reference in folder A. + assertStatus(HttpServletResponse.SC_OK, post(resourceUrl(_folderA, _folderB, "foreign.txt"), _readerA)); + assertNull("A foreign-folder file the caller cannot read must not be imported into folder A", + exp.getExpDataByURL(foreignFile, _folderA)); + + // Deny (pipeline-root containment): even a site admin who CAN read folder B must not be able to pull a + // file from B's pipeline root into folder A. This is the case the canRead check alone would miss. + assertStatus(HttpServletResponse.SC_OK, post(resourceUrl(_folderA, _folderB, "foreign.txt"), getAdmin())); + assertNull("A file outside the request container's pipeline root must not be imported, even for an admin", + exp.getExpDataByURL(foreignFile, _folderA)); + + // Positive control: the same request for a file under folder A's own pipeline root still creates the + // exp.data row, proving the fix did not break the legitimate in-container flow. + File localFile = writeFileUnderFileRoot(_folderA, "local.txt"); + assertStatus(HttpServletResponse.SC_OK, post(resourceUrl(_folderA, _folderA, "local.txt"), getAdmin())); + assertNotNull("A file under the request container's own pipeline root should still be imported", + exp.getExpDataByURL(localFile, _folderA)); + } + + /** + * Build a getSignalDataResource URL in {@code requestContainer} whose path points at a file under {@code fileContainer}'s @files root. + */ + private static ActionURL resourceUrl(Container requestContainer, Container fileContainer, String name) + { + String path = filesPath(fileContainer).append(name).toString(); + return new ActionURL(getSignalDataResourceAction.class, requestContainer) + .addParameter("path", path) + .addParameter("test", true); + } + + private static Path filesPath(Container c) + { + return WebdavService.getPath().append(c.getParsedPath()).append(FileContentService.FILES_LINK); + } + + private static File writeFileUnderFileRoot(Container c, String name) throws Exception + { + WebdavResource filesNode = WebdavService.get().lookup(filesPath(c)); + assertNotNull("Test requires a @files node for " + c.getName(), filesNode); + File dir = filesNode.getFile(); + assertNotNull("Test requires a file root for " + c.getName(), dir); + FileUtil.mkdirs(dir); + + File file = FileUtil.appendName(dir, name); + if (!file.exists()) + Files.writeString(file.toPath(), "test"); + return file; + } + } } \ No newline at end of file diff --git a/signalData/src/org/labkey/signaldata/SignalDataModule.java b/signalData/src/org/labkey/signaldata/SignalDataModule.java index a0a4a0b0c5..9f7b4882f7 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataModule.java +++ b/signalData/src/org/labkey/signaldata/SignalDataModule.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Collections; +import java.util.Set; public class SignalDataModule extends CodeOnlyModule { @@ -73,4 +74,11 @@ public Collection getSummary(Container c) { return Collections.emptyList(); } + + @Override + @NotNull + public Set getIntegrationTests() + { + return Set.of(SignalDataController.ContainerScopingTestCase.class); + } } \ No newline at end of file