[bot] Fast-forward for 26.3.14

pepdb/src/org/scharp/atlas/pepdb/PepDBController.java

@@ -427,7 +427,7 @@ public ModelAndView getView(PeptideAndGroupForm form, BindException errors) thro
                 pg = PepDBManager.getPeptideGroupByID(peptideGroupId);
             }
             catch (NumberFormatException ignored) {}
-            if (pg == null || !getContainer().getId().equalsIgnoreCase(pg.getContainerId()))
+            if (pg == null)
             {
                 throw new NotFoundException();
             }

pepdb/src/org/scharp/atlas/pepdb/PepDBManager.java

@@ -295,10 +295,12 @@ public static Peptides[] getParentPeptides(Peptides p) throws SQLException
 
     public static Peptides[] getHyphanatedParents(Peptides p) throws SQLException
     {
-        String sql = "select * from "+schema.getTableInfoPeptides()+" where peptides.protein_cat_id = ? " +
-                " and peptides.peptide_sequence LIKE '%"+p.getPeptide_sequence()+"%' " +
-                " and child = false";
-        Peptides[] peptides = new SqlSelector(schema.getSchema(), sql, new Object[]{p.getProtein_cat_id()}).getArray(Peptides.class);
+        // GitHub Kanban #1929: parameterize the LIKE clause for the stored peptide_sequence
+        SQLFragment sql = new SQLFragment("select * from "+schema.getTableInfoPeptides()+" where peptides.protein_cat_id = ? ", p.getProtein_cat_id());
+        sql.append(" and peptides.peptide_sequence LIKE ? ");
+        sql.add("%" + schema.getSchema().getSqlDialect().encodeLikeOpSearchString(p.getPeptide_sequence()) + "%");
+        sql.append(" and child = false");
+        Peptides[] peptides = new SqlSelector(schema.getSchema(), sql).getArray(Peptides.class);
         return peptides;
     }
 

pepdb/test/src/org/labkey/test/tests/pepdb/PepDBModuleTest.java

@@ -31,6 +31,7 @@
 import org.labkey.test.TestTimeoutException;
 import org.labkey.test.WebTestHelper;
 import org.labkey.test.categories.CustomModules;
+import org.labkey.test.util.DataRegionTable;
 import org.labkey.test.util.LogMethod;
 import org.labkey.test.util.PostgresOnlyTest;
 
@@ -180,6 +181,15 @@ public void testSteps() throws Exception
 
         clickAndWait(Locator.css("a.labkey-button > span"));
 
+        // Verify peptide group detail page
+        clickAndWait(Locator.linkWithText("List Peptide Groups"));
+        clickAndWait(Locator.linkWithText("gagptegprac"));
+        assertTextPresentInThisOrder(
+                "Group Information from peptide_group table for group : gagptegprac",
+                "There are (16) peptides in the 'gagptegprac' peptide group."
+        );
+        DataRegionTable pepTable = new DataRegionTable("group_peptides", getDriver());
+        assertEquals("Peptide table for group does not have expected number of rows", 16, pepTable.getDataRowCount());
 
         /*
          *

studydesign/src/org/labkey/studydesign/StudyDesignController.java

@@ -19,6 +19,7 @@
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONArray;
 import org.json.JSONObject;
+import org.junit.Test;
 import org.labkey.api.action.ApiJsonForm;
 import org.labkey.api.action.ApiResponse;
 import org.labkey.api.action.ApiSimpleResponse;
@@ -31,11 +32,13 @@
 import org.labkey.api.data.DbScope;
 import org.labkey.api.data.SimpleFilter;
 import org.labkey.api.data.Table;
+import org.labkey.api.data.TableSelector;
 import org.labkey.api.query.FieldKey;
 import org.labkey.api.query.ValidationException;
 import org.labkey.api.security.ActionNames;
 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.permissions.UpdatePermission;
 import org.labkey.api.study.Cohort;
@@ -53,6 +56,7 @@
 import org.labkey.api.view.HttpView;
 import org.labkey.api.view.JspView;
 import org.labkey.api.view.NavTree;
+import org.labkey.api.view.NotFoundException;
 import org.labkey.studydesign.model.AssaySpecimenConfigImpl;
 import org.labkey.studydesign.model.AssaySpecimenVisitImpl;
 import org.labkey.studydesign.model.DoseAndRoute;
@@ -946,4 +950,52 @@ private void updateAssayPlan(User user, Study study, String assayPlan)
             StudyService.get().updateAssayPlan(user, study, assayPlan);
         }
     }
+
+    /**
+     * Verifies the cross-container guard on the DoseAndRoute save path reached by UpdateStudyProductsAction. That
+     * action's DoseAndRoute branch alone writes through the raw storage table keyed by a client-supplied RowId, so the
+     * guard lives in TreatmentManager.saveStudyProductDoseAndRoute and is exercised directly here (building a full study
+     * + product + JSON product payload to drive the action end-to-end would add a large fixture for the same assertion).
+     */
+    public static class ContainerScopingTestCase extends AbstractContainerScopingTest
+    {
+        @Test
+        public void testSaveDoseAndRouteContainerScoping()
+        {
+            User admin = getAdmin();
+            Container folderA = createContainer("A");
+            Container folderB = createContainer("B");
+
+            // A DoseAndRoute row that lives in folder B
+            DoseAndRoute existing = Table.insert(admin, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(),
+                    new DoseAndRoute("1 mg", "IV", 1, folderB));
+            int rowId = existing.getRowId();
+
+            // Deny: an update keyed by B's RowId while operating in folder A must be rejected, not silently
+            // overwrite/re-home the foreign row. This is the path an Editor in folder A reaches via
+            // UpdateStudyProductsAction by submitting a products[].DoseAndRoute[].RowId owned by folder B.
+            DoseAndRoute attack = new DoseAndRoute("HACKED", "HACKED", 1, folderA);
+            attack.setRowId(rowId);
+            try
+            {
+                TreatmentManager.getInstance().saveStudyProductDoseAndRoute(folderA, admin, attack);
+                fail("Expected NotFoundException updating a DoseAndRoute owned by another container");
+            }
+            catch (NotFoundException ignored) {}
+
+            // The row in folder B must be untouched
+            DoseAndRoute reloaded = new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute())
+                    .getObject(rowId, DoseAndRoute.class);
+            assertNotNull("DoseAndRoute must still exist", reloaded);
+            assertEquals("Dose must be unchanged after a cross-container update", "1 mg", reloaded.getDose());
+
+            // Positive control: updating through the row's own container succeeds and persists the change
+            DoseAndRoute ok = new DoseAndRoute("2 mg", "IM", 1, folderB);
+            ok.setRowId(rowId);
+            TreatmentManager.getInstance().saveStudyProductDoseAndRoute(folderB, admin, ok);
+            DoseAndRoute afterOk = new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute())
+                    .getObject(rowId, DoseAndRoute.class);
+            assertEquals("Dose should be updated by a same-container request", "2 mg", afterOk.getDose());
+        }
+    }
 }

studydesign/src/org/labkey/studydesign/StudyDesignModule.java

@@ -75,8 +75,9 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext)
     public @NotNull Set<Class<?>> getIntegrationTests()
     {
         return Set.of(
-            TreatmentManager.TreatmentDataTestCase.class,
-            TreatmentManager.AssayScheduleTestCase.class
+                TreatmentManager.TreatmentDataTestCase.class,
+                TreatmentManager.AssayScheduleTestCase.class,
+                StudyDesignController.ContainerScopingTestCase.class
         );
     }
 }

studydesign/src/org/labkey/studydesign/model/TreatmentManager.java

@@ -33,6 +33,7 @@
 import org.labkey.api.module.ModuleLoader;
 import org.labkey.api.query.BatchValidationException;
 import org.labkey.api.query.FieldKey;
+import org.labkey.api.view.NotFoundException;
 import org.labkey.api.query.FilteredTable;
 import org.labkey.api.query.QueryService;
 import org.labkey.api.query.QueryUpdateService;
@@ -320,7 +321,10 @@ public void deleteStudyProduct(Container container, User user, int rowId)
             deleteProductAntigens(container, user, rowId);
 
             // delete the associated doses and routes for this product
-            Table.delete(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), new SimpleFilter(FieldKey.fromParts("ProductId"), rowId));
+            // GitHub Kanban #1929: scope to the container in addition to ProductId
+            SimpleFilter doseAndRouteFilter = SimpleFilter.createContainerFilter(container);
+            doseAndRouteFilter.addCondition(FieldKey.fromParts("ProductId"), rowId);
+            Table.delete(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRouteFilter);
 
             // delete the associated treatment study product mappings (provision table)
             SimpleFilter filter = SimpleFilter.createContainerFilter(container);
@@ -364,20 +368,30 @@ public DoseAndRoute saveStudyProductDoseAndRoute(Container container, User user,
     {
         if (doseAndRoute.isNew())
             return Table.insert(user, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRoute);
-        else
-            return Table.update(user, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRoute, doseAndRoute.getRowId());
+
+        // GitHub Kanban #1929: verify the existing row is in this container before updating
+        SimpleFilter filter = SimpleFilter.createContainerFilter(container);
+        filter.addCondition(FieldKey.fromParts("RowId"), doseAndRoute.getRowId());
+        if (!new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), filter, null).exists())
+            throw new NotFoundException("No dose and route found for rowId: " + doseAndRoute.getRowId());
+
+        return Table.update(user, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRoute, doseAndRoute.getRowId());
     }
 
     public Collection<DoseAndRoute> getStudyProductsDoseAndRoute(Container container, User user, int productId)
     {
-        SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ProductId"), productId);
+        // GitHub Kanban #1929: scope to the container in addition to ProductId
+        SimpleFilter filter = SimpleFilter.createContainerFilter(container);
+        filter.addCondition(FieldKey.fromParts("ProductId"), productId);
         return new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), filter, null).getCollection(DoseAndRoute.class);
     }
 
     @Nullable
     public DoseAndRoute getDoseAndRoute(Container container, String dose, String route, int productId)
     {
-        SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ProductId"), productId);
+        // GitHub Kanban #1929: scope to the container in addition to ProductId
+        SimpleFilter filter = SimpleFilter.createContainerFilter(container);
+        filter.addCondition(FieldKey.fromParts("ProductId"), productId);
         if (dose != null)
             filter.addCondition(FieldKey.fromParts("Dose"), dose);
         else
@@ -991,6 +1005,125 @@ private void tearDown()
                 assertTrue(ContainerManager.delete(_junitStudy.getContainer(), _context.getUser()));
             }
         }
+
+        // GitHub Kanban #1929: getStudyProductsDoseAndRoute / getDoseAndRoute / deleteStudyProduct container scoping checks
+        @Test
+        public void testDoseAndRouteContainerScoping() throws Exception
+        {
+            TestContext context = TestContext.get();
+            User user = context.getUser();
+            Container containerA = null;
+            Container containerB = null;
+            try
+            {
+                containerA = createStudyContainer(context, GUID.makeHash());
+                containerB = createStudyContainer(context, GUID.makeHash());
+
+                int productIdA = insertStudyProduct(containerA, user, "Immunogen A", "Immunogen");
+                int productIdB = insertStudyProduct(containerB, user, "Immunogen B", "Immunogen");
+
+                _manager.saveStudyProductDoseAndRoute(containerA, user, new DoseAndRoute("Dose A", "Route A", productIdA, containerA));
+                _manager.saveStudyProductDoseAndRoute(containerB, user, new DoseAndRoute("Dose B", "Route B", productIdB, containerB));
+
+                // getStudyProductsDoseAndRoute: scoped by container, not ProductId alone
+                assertEquals("Dose/route should be returned within its own container", 1,
+                        _manager.getStudyProductsDoseAndRoute(containerB, user, productIdB).size());
+                assertEquals("Dose/route must not be returned from another container", 0,
+                        _manager.getStudyProductsDoseAndRoute(containerA, user, productIdB).size());
+
+                // getDoseAndRoute: same scoping
+                assertNotNull("getDoseAndRoute should find the row within its own container",
+                        _manager.getDoseAndRoute(containerB, "Dose B", "Route B", productIdB));
+                assertNull("getDoseAndRoute must not find the row from another container",
+                        _manager.getDoseAndRoute(containerA, "Dose B", "Route B", productIdB));
+
+                // deleteStudyProduct's dose/route delete is now container scoped; deleting the product in its own
+                // container removes its dose/route (the cross-container case is unreachable since ProductId is unique).
+                _manager.deleteStudyProduct(containerB, user, productIdB);
+                assertEquals("deleteStudyProduct should remove the dose/route within its own container", 0,
+                        _manager.getStudyProductsDoseAndRoute(containerB, user, productIdB).size());
+            }
+            finally
+            {
+                if (null != containerB)
+                    ContainerManager.delete(containerB, user);
+                if (null != containerA)
+                    ContainerManager.delete(containerA, user);
+            }
+        }
+
+        // GitHub Kanban #1929: saveStudyProductDoseAndRoute rejects updating a dose/route row from another container
+        @Test
+        public void testCrossContainerDoseAndRouteUpdateDenied()
+        {
+            TestContext context = TestContext.get();
+            User user = context.getUser();
+            Container containerA = null;
+            Container containerB = null;
+            try
+            {
+                containerA = createStudyContainer(context, GUID.makeHash());
+                containerB = createStudyContainer(context, GUID.makeHash());
+
+                int productIdA = insertStudyProduct(containerA, user, "Immunogen A", "Immunogen");
+                int productIdB = insertStudyProduct(containerB, user, "Immunogen B", "Immunogen");
+
+                // a dose/route owned by container B
+                DoseAndRoute savedB = _manager.saveStudyProductDoseAndRoute(containerB, user, new DoseAndRoute("Dose B", "Route B", productIdB, containerB));
+                int rowIdB = savedB.getRowId();
+
+                // an editor in container A submits an update carrying container B's RowId
+                DoseAndRoute foreign = new DoseAndRoute("Rejected", "Rejected", productIdA, containerA);
+                foreign.setRowId(rowIdB);
+                try
+                {
+                    _manager.saveStudyProductDoseAndRoute(containerA, user, foreign);
+                    fail("Expected NotFoundException updating a dose/route row from another container");
+                }
+                catch (NotFoundException expected)
+                {
+                    // expected
+                }
+
+                // container B's row must be untouched (neither overwritten nor repointed into container A)
+                assertNotNull("Cross-container update must not modify container B's dose/route",
+                        _manager.getDoseAndRoute(containerB, "Dose B", "Route B", productIdB));
+                assertNull("Cross-container update must not have repointed the row into container A",
+                        _manager.getDoseAndRoute(containerA, "Rejected", "Rejected", productIdA));
+
+                // positive control: updating the row from within its own container succeeds
+                DoseAndRoute updateB = new DoseAndRoute("Dose B2", "Route B2", productIdB, containerB);
+                updateB.setRowId(rowIdB);
+                _manager.saveStudyProductDoseAndRoute(containerB, user, updateB);
+                assertNotNull("In-container update should succeed",
+                        _manager.getDoseAndRoute(containerB, "Dose B2", "Route B2", productIdB));
+            }
+            finally
+            {
+                if (null != containerB)
+                    ContainerManager.delete(containerB, user);
+                if (null != containerA)
+                    ContainerManager.delete(containerA, user);
+            }
+        }
+
+        private Container createStudyContainer(TestContext context, String name)
+        {
+            Container junit = JunitUtil.getTestContainer();
+            Container c = ContainerManager.createContainer(junit, name, context.getUser());
+            Set<Module> modules = new HashSet<>(c.getActiveModules());
+            modules.add(ModuleLoader.getInstance().getModule("studydesign"));
+            c.setActiveModules(modules);
+            StudyService.get().createStudy(c, context.getUser(), "Junit Study " + name, TimepointType.VISIT, true);
+            return c;
+        }
+
+        private int insertStudyProduct(Container c, User user, String label, String role)
+        {
+            UserSchema schema = QueryService.get().getUserSchema(user, c, StudyDesignQuerySchema.STUDY_SCHEMA_NAME);
+            TableInfo ti = ((FilteredTable) schema.getTable(StudyDesignQuerySchema.PRODUCT_TABLE_NAME)).getRealTable();
+            return Table.insert(user, ti, new ProductImpl(c, label, role)).getRowId();
+        }
     }
 
     @TestWhen(TestWhen.When.BVT)