diff --git a/src/org/labkey/test/tests/AttachmentFieldTest.java b/src/org/labkey/test/tests/AttachmentFieldTest.java index 48e14a7046..4e39f4e2e2 100644 --- a/src/org/labkey/test/tests/AttachmentFieldTest.java +++ b/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); + } } diff --git a/src/org/labkey/test/tests/core/security/GetContainerInfoAPITest.java b/src/org/labkey/test/tests/core/security/GetContainerInfoAPITest.java new file mode 100644 index 0000000000..1951dfd07b --- /dev/null +++ b/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 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); + } +} diff --git a/src/org/labkey/test/tests/experiment/GetEntitySequenceAPITest.java b/src/org/labkey/test/tests/experiment/GetEntitySequenceAPITest.java new file mode 100644 index 0000000000..6832c123c4 --- /dev/null +++ b/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 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); + } +}