Kanban Issue #1924: Additional container scoping updates

src/org/labkey/test/tests/AttachmentFieldTest.java

@@ -1,5 +1,6 @@
 package org.labkey.test.tests;
 
+import org.apache.hc.core5.http.HttpStatus;
 import org.assertj.core.api.Assertions;
 import org.jetbrains.annotations.Nullable;
 import org.junit.Assert;
@@ -9,25 +10,36 @@
 import org.labkey.test.BaseWebDriverTest;
 import org.labkey.test.Locator;
 import org.labkey.test.TestFileUtils;
+import org.labkey.test.WebTestHelper;
 import org.labkey.test.categories.Daily;
 import org.labkey.test.components.DomainDesignerPage;
 import org.labkey.test.components.domain.DomainFieldRow;
 import org.labkey.test.components.domain.DomainFormPanel;
+import org.labkey.test.pages.ReactAssayDesignerPage;
 import org.labkey.test.pages.experiment.UpdateSampleTypePage;
 import org.labkey.test.params.FieldDefinition;
 import org.labkey.test.params.experiment.SampleTypeDefinition;
+import org.labkey.test.util.ApiPermissionsHelper;
 import org.labkey.test.util.DataRegionTable;
+import org.labkey.test.util.PasswordUtil;
+import org.labkey.test.util.PermissionsHelper;
 import org.labkey.test.util.PortalHelper;
 import org.labkey.test.util.SampleTypeHelper;
 import org.labkey.test.util.TestDataGenerator;
 
+import org.openqa.selenium.By;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+
 import java.io.File;
+import java.net.URI;
 import java.util.List;
 
 @Category({Daily.class})
 @BaseWebDriverTest.ClassTimeout(minutes = 2)
 public class AttachmentFieldTest extends BaseWebDriverTest
 {
+    private static final String RESTRICTED_PROJECT = "AttachmentFieldTest Restricted Project";
+    private static final String RESTRICTED_USER = "restrictedreader@attachmentfieldtest.test";
     private final File SAMPLE_FILE = new File(TestFileUtils.getSampleData("fileTypes"), "jpg_sample.jpg");
 
     @BeforeClass
@@ -57,6 +69,14 @@ private void doSetup()
         portalHelper.addBodyWebPart("Lists");
     }
 
+    @Override
+    protected void doCleanup(boolean afterTest)
+    {
+        super.doCleanup(afterTest);
+        _containerHelper.deleteProject(RESTRICTED_PROJECT, false);
+        _userHelper.deleteUsers(false, RESTRICTED_USER);
+    }
+
     @Test
     public void testFileFieldInSampleType()
     {
@@ -138,4 +158,63 @@ public void testAttachmentFieldInLists()
         File downloadedFile = doAndWaitForDownload(() -> Locator.tagWithAttributeContaining("img", "title", SAMPLE_FILE.getName()).findElement(getDriver()).click());
         Assert.assertTrue("Downloaded file is empty", downloadedFile.length() > 0);
     }
+
+    // Kanban #1924
+    @Test
+    public void testDownloadFileLinkCrossContainerPermission()
+    {
+        final String assayName = "CrossContainerAssay";
+        final String runFieldName = "runFile";
+
+        log("Create restricted project with Assay folder type to provide a pipeline root for file storage");
+        _containerHelper.createProject(RESTRICTED_PROJECT, "Assay");
+
+        log("Create a General assay with a run-level file link field");
+        goToProjectHome(RESTRICTED_PROJECT);
+        goToManageAssays();
+        ReactAssayDesignerPage assayDesigner = _assayHelper.createAssayDesign("General", assayName);
+        assayDesigner.setEditableRuns(true);
+        assayDesigner.goToRunFields().addField(runFieldName).setType(FieldDefinition.ColumnType.File);
+        assayDesigner.clickFinish();
+
+        log("Import a minimal assay run");
+        clickAndWait(Locator.linkWithText(assayName));
+        clickButton("Import Data");
+        clickButton("Next");
+        setFormElement(Locator.name("name"), "TestRun");
+        setFormElement(Locator.name("TextAreaDataCollector.textArea"),
+                "Specimen ID\tParticipant ID\tVisit ID\n100\t1A2B\t1");
+        clickButton("Save and Finish");
+
+        log("Edit the run to set the file field");
+        clickAndWait(Locator.linkWithText("view runs"));
+        new DataRegionTable("Runs", getDriver()).clickEditRow(0);
+        setFormElement(Locator.name("quf_" + runFieldName), SAMPLE_FILE);
+        clickButton("Submit");
+        waitForElement(DataRegionTable.updateLinkLocator());
+
+        log("Hover over the run file thumbnail to reveal the popup and get the objectURI-based downloadFileLink URL");
+        mouseOver(Locator.xpath("//img[contains(@title, '" + SAMPLE_FILE.getName() + "')]"));
+        longWait().until(ExpectedConditions.visibilityOfElementLocated(By.cssSelector("#helpDiv")));
+        String restrictedDownloadUrl = Locator.xpath("//div[@id='helpDiv']//img[contains(@src, 'downloadFileLink')]")
+                .findElement(getDriver()).getAttribute("src");
+        Assertions.assertThat(restrictedDownloadUrl).as("Expected downloadFileLink URL with objectURI parameter")
+                .contains("downloadFileLink")
+                .contains("objectURI");
+
+        // Build a cross-container URL: keep the same objectURI (run LSID) and propertyId but use the main project's
+        // container.
+        String crossContainerUrl = WebTestHelper.buildURL("core", getProjectName(), "downloadFileLink")
+                + "?" + URI.create(restrictedDownloadUrl).getRawQuery();
+
+        log("Create a reader user with access to the main project only, not to the restricted project");
+        _userHelper.createUser(RESTRICTED_USER);
+        _userHelper.setInitialPassword(RESTRICTED_USER);
+        new ApiPermissionsHelper(this).addMemberToRole(RESTRICTED_USER, "Reader", PermissionsHelper.MemberType.user, getProjectName());
+
+        log("Verify cross-container download is rejected with 403 when user lacks read permission on the object's container");
+        int status = WebTestHelper.getHttpResponse(crossContainerUrl, RESTRICTED_USER, PasswordUtil.getPassword()).getResponseCode();
+        Assert.assertEquals("Expected 403 Forbidden when user lacks read permission on the object's container",
+                HttpStatus.SC_FORBIDDEN, status);
+    }
 }

src/org/labkey/test/tests/core/security/GetContainerInfoAPITest.java

@@ -0,0 +1,102 @@
+package org.labkey.test.tests.core.security;
+
+import org.apache.hc.core5.http.HttpStatus;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.labkey.test.BaseWebDriverTest;
+import org.labkey.test.TestTimeoutException;
+import org.labkey.test.WebTestHelper;
+import org.labkey.test.categories.Daily;
+import org.labkey.test.util.APIContainerHelper;
+import org.labkey.test.util.ApiPermissionsHelper;
+import org.labkey.test.util.PasswordUtil;
+import org.labkey.test.util.PermissionsHelper;
+
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests cross-container permission enforcement in CoreController.GetContainerInfoAction (Kanban #1924).
+ *
+ * The action accepts an optional {@code containerPath} parameter. Prior to the fix, a user who had
+ * ReadPermission on the request container could supply any container path and receive information about
+ * it — even containers they had no access to. The fix adds a check that the user also has ReadPermission
+ * on the resolved container before returning any data.
+ */
+@Category({Daily.class})
+public class GetContainerInfoAPITest extends BaseWebDriverTest
+{
+    private static final String READABLE_PROJECT = "GetContainerInfoAPITest Readable";
+    private static final String RESTRICTED_PROJECT = "GetContainerInfoAPITest Restricted";
+    private static final String READER_USER = "reader@getcontainerinfoapi.test";
+
+    private final ApiPermissionsHelper _permissions = new ApiPermissionsHelper(this);
+
+    public GetContainerInfoAPITest()
+    {
+        ((APIContainerHelper) _containerHelper).setNavigateToCreatedFolders(false);
+    }
+
+    @BeforeClass
+    public static void setupProject()
+    {
+        GetContainerInfoAPITest init = getCurrentTest();
+        init.doSetup();
+    }
+
+    private void doSetup()
+    {
+        _containerHelper.createProject(READABLE_PROJECT, "Collaboration");
+        _containerHelper.createProject(RESTRICTED_PROJECT, "Collaboration");
+
+        _userHelper.createUser(READER_USER);
+        _userHelper.setInitialPassword(READER_USER);
+        _permissions.addMemberToRole(READER_USER, "Reader", PermissionsHelper.MemberType.user, READABLE_PROJECT);
+        // Intentionally not granting the user any role in RESTRICTED_PROJECT
+    }
+
+    @Override
+    protected void doCleanup(boolean afterTest) throws TestTimeoutException
+    {
+        _containerHelper.deleteProject(READABLE_PROJECT, afterTest);
+        _containerHelper.deleteProject(RESTRICTED_PROJECT, afterTest);
+        _userHelper.deleteUsers(false, READER_USER);
+    }
+
+    @Override
+    protected String getProjectName()
+    {
+        return null;
+    }
+
+    @Override
+    public List<String> getAssociatedModules()
+    {
+        return null;
+    }
+
+    // Kanban #1924
+    @Test
+    public void testGetContainerInfoAccessControl()
+    {
+        // Cross-container denial: user makes the request from READABLE_PROJECT (passing @RequiresPermission),
+        // but containerPath resolves to RESTRICTED_PROJECT where the user has no ReadPermission. Expect 403.
+        String restrictedUrl = WebTestHelper.buildURL("core", READABLE_PROJECT, "getContainerInfo",
+                Map.of("containerPath", RESTRICTED_PROJECT, "newFolderType", "Collaboration"));
+        int restrictedStatus = WebTestHelper.getHttpResponse(restrictedUrl, READER_USER, PasswordUtil.getPassword())
+                .getResponseCode();
+        assertEquals("Expected 403 when user lacks ReadPermission on the containerPath container",
+                HttpStatus.SC_FORBIDDEN, restrictedStatus);
+
+        // Same-container success: containerPath resolves to READABLE_PROJECT where the user is a Reader. Expect 200.
+        String readableUrl = WebTestHelper.buildURL("core", READABLE_PROJECT, "getContainerInfo",
+                Map.of("containerPath", READABLE_PROJECT, "newFolderType", "Collaboration"));
+        int readableStatus = WebTestHelper.getHttpResponse(readableUrl, READER_USER, PasswordUtil.getPassword())
+                .getResponseCode();
+        assertEquals("Expected 200 when user has ReadPermission on the containerPath container",
+                HttpStatus.SC_OK, readableStatus);
+    }
+}

src/org/labkey/test/tests/experiment/GetEntitySequenceAPITest.java

@@ -0,0 +1,134 @@
+package org.labkey.test.tests.experiment;
+
+import org.apache.hc.core5.http.HttpStatus;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.labkey.remoteapi.CommandException;
+import org.labkey.remoteapi.query.Filter;
+import org.labkey.remoteapi.query.SelectRowsCommand;
+import org.labkey.test.BaseWebDriverTest;
+import org.labkey.test.TestTimeoutException;
+import org.labkey.test.WebTestHelper;
+import org.labkey.test.categories.Daily;
+import org.labkey.test.params.experiment.DataClassDefinition;
+import org.labkey.test.params.experiment.SampleTypeDefinition;
+import org.labkey.test.util.APIContainerHelper;
+import org.labkey.test.util.ApiPermissionsHelper;
+import org.labkey.test.util.PasswordUtil;
+import org.labkey.test.util.PermissionsHelper;
+import org.labkey.test.util.exp.DataClassAPIHelper;
+import org.labkey.test.util.exp.SampleTypeAPIHelper;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+@Category({Daily.class})
+public class GetEntitySequenceAPITest extends BaseWebDriverTest
+{
+    private static final String READABLE_PROJECT = "GetEntitySequenceAPITest Readable";
+    private static final String RESTRICTED_PROJECT = "GetEntitySequenceAPITest Restricted";
+    private static final String READER_USER = "reader@getentitysequenceapi.test";
+    private static final String SAMPLE_TYPE_NAME = "GetEntitySequenceTest_SampleType";
+    private static final String DATA_CLASS_NAME = "GetEntitySequenceTest_DataClass";
+
+    private final ApiPermissionsHelper _permissions = new ApiPermissionsHelper(this);
+
+    public GetEntitySequenceAPITest()
+    {
+        ((APIContainerHelper) _containerHelper).setNavigateToCreatedFolders(false);
+    }
+
+    @BeforeClass
+    public static void setupProject()
+    {
+        GetEntitySequenceAPITest init = getCurrentTest();
+        init.doSetup();
+    }
+
+    private void doSetup()
+    {
+        _containerHelper.createProject(READABLE_PROJECT, null);
+        _containerHelper.createProject(RESTRICTED_PROJECT, null);
+
+        SampleTypeAPIHelper.createEmptySampleType(RESTRICTED_PROJECT, new SampleTypeDefinition(SAMPLE_TYPE_NAME));
+        DataClassAPIHelper.createEmptyDataClass(RESTRICTED_PROJECT, new DataClassDefinition(DATA_CLASS_NAME));
+
+        _userHelper.createUser(READER_USER);
+        _userHelper.setInitialPassword(READER_USER);
+        _permissions.addMemberToRole(READER_USER, "Reader", PermissionsHelper.MemberType.user, READABLE_PROJECT);
+        // Intentionally not granting the user any role in RESTRICTED_PROJECT
+    }
+
+    @Override
+    protected void doCleanup(boolean afterTest) throws TestTimeoutException
+    {
+        _containerHelper.deleteProject(READABLE_PROJECT, afterTest);
+        _containerHelper.deleteProject(RESTRICTED_PROJECT, afterTest);
+        _userHelper.deleteUsers(false, READER_USER);
+    }
+
+    @Override
+    protected String getProjectName()
+    {
+        return null;
+    }
+
+    @Override
+    public List<String> getAssociatedModules()
+    {
+        return List.of("experiment");
+    }
+
+    // Kanban #1924 Verify we prevent getting the sequence value from a container the user does not have access to
+    @Test
+    public void testGetEntitySequenceSampleTypeAccessControl() throws IOException, CommandException
+    {
+        // The sample type lives in RESTRICTED_PROJECT. getSampleType(rowId) fetches globally so the lookup
+        // succeeds; the fix then checks the user has ReadPermission on the sample type's container.
+        SelectRowsCommand cmd = new SelectRowsCommand("exp", "SampleSets");
+        cmd.addFilter("Name", SAMPLE_TYPE_NAME, Filter.Operator.EQUAL);
+        cmd.setMaxRows(1);
+        int sampleTypeRowId = ((Number) cmd.execute(createDefaultConnection(), RESTRICTED_PROJECT)
+                .getRows().get(0).get("RowId")).intValue();
+
+        // User has ReadPermission on READABLE_PROJECT (passes @RequiresPermission), but the sample type
+        // belongs to RESTRICTED_PROJECT where the user has no access. seqType=genId is the only path
+        // that triggers the cross-container check for sample types.
+        String url = WebTestHelper.buildURL("experiment", READABLE_PROJECT, "getEntitySequence",
+                Map.of("kindName", SampleTypeAPIHelper.SAMPLE_TYPE_DOMAIN_KIND,
+                       "seqType", "genId",
+                       "rowId", String.valueOf(sampleTypeRowId)));
+        int status = WebTestHelper.getHttpResponse(url, READER_USER, PasswordUtil.getPassword()).getResponseCode();
+        assertEquals("Expected 403 when user lacks ReadPermission on the sample type's container",
+                HttpStatus.SC_FORBIDDEN, status);
+    }
+
+    // Kanban #1924 Verify we prevent getting the sequence value from a container the user does not have access to
+
+    @Test
+    public void testGetEntitySequenceDataClassAccessControl() throws IOException, CommandException
+    {
+        // The data class lives in RESTRICTED_PROJECT. getDataClass(rowId) fetches globally so the lookup
+        // succeeds; the fix then checks the user has ReadPermission on the data class's container.
+        // seqType=genId is the only value accepted when kindName=DataClass.
+        SelectRowsCommand cmd = new SelectRowsCommand("exp", "DataClasses");
+        cmd.addFilter("Name", DATA_CLASS_NAME, Filter.Operator.EQUAL);
+        cmd.setMaxRows(1);
+        int dataClassRowId = ((Number) cmd.execute(createDefaultConnection(), RESTRICTED_PROJECT)
+                .getRows().get(0).get("RowId")).intValue();
+
+        // User has ReadPermission on READABLE_PROJECT (passes @RequiresPermission), but the data class
+        // belongs to RESTRICTED_PROJECT where the user has no access.
+        String url = WebTestHelper.buildURL("experiment", READABLE_PROJECT, "getEntitySequence",
+                Map.of("kindName", "DataClass",
+                       "seqType", "genId",
+                       "rowId", String.valueOf(dataClassRowId)));
+        int status = WebTestHelper.getHttpResponse(url, READER_USER, PasswordUtil.getPassword()).getResponseCode();
+        assertEquals("Expected 403 when user lacks ReadPermission on the data class's container",
+                HttpStatus.SC_FORBIDDEN, status);
+    }
+}