diff --git a/src/org/labkey/test/tests/elispotassay/ElispotAssayTest.java b/src/org/labkey/test/tests/elispotassay/ElispotAssayTest.java index 0cda0ff4fd..c26f630f0d 100644 --- a/src/org/labkey/test/tests/elispotassay/ElispotAssayTest.java +++ b/src/org/labkey/test/tests/elispotassay/ElispotAssayTest.java @@ -21,11 +21,16 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.labkey.api.query.QueryKey; +import org.labkey.remoteapi.CommandException; +import org.labkey.remoteapi.query.SelectRowsCommand; +import org.labkey.remoteapi.query.SelectRowsResponse; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.SortDirection; import org.labkey.test.TestFileUtils; import org.labkey.test.TestTimeoutException; +import org.labkey.test.WebTestHelper; import org.labkey.test.categories.Assays; import org.labkey.test.categories.Daily; import org.labkey.test.components.CrosstabDataRegion; @@ -39,15 +44,19 @@ import org.labkey.test.util.PipelineStatusTable; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.QCAssayScriptHelper; +import org.labkey.test.util.SimpleHttpRequest; +import org.labkey.test.util.SimpleHttpResponse; import org.openqa.selenium.NoSuchElementException; import java.io.File; +import java.io.IOException; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.labkey.test.components.PlateSummary.Row.A; import static org.labkey.test.components.PlateSummary.Row.C; import static org.labkey.test.components.PlateSummary.Row.E; @@ -519,10 +528,61 @@ protected void runTransformTest() protected void doBackgroundSubtractionTest() { removeTransformScript(); + verifyCrossContainerBackgroundSubtractionDenied(); verifyBackgroundSubtractionOnExistingRun(); verifyBackgroundSubtractionOnNewRun(); } + // GitHub Kanban #1892: verify BackgroundSubtractionAction selected run in the current container. + @LogMethod + protected void verifyCrossContainerBackgroundSubtractionDenied() + { + final String crossFolder = "BackgroundSubtractionAuth"; + _containerHelper.createSubfolder(getProjectName(), crossFolder, "Assay"); + + // A run RowId from the assay in the project - "foreign" from the subfolder's perspective. + long foreignRunId = getFirstElispotRunId(); + + log("POST background subtraction for the project's run (RowId " + foreignRunId + ") from the sibling subfolder - must be denied"); + String url = WebTestHelper.buildURL("elispot-assay", getProjectName() + "/" + crossFolder, "backgroundSubtraction", + Map.of(".select", String.valueOf(foreignRunId))); + SimpleHttpRequest request = new SimpleHttpRequest(url, "POST"); + request.copySession(getDriver()); + SimpleHttpResponse response; + try + { + response = request.getResponse(); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + // The run is not in the subfolder, so the action should report it as not found (HTTP 404) rather than queuing the job. + assertEquals("Cross-container background subtraction should be denied for a run in another folder", 404, response.getResponseCode()); + assertTrue("Cross-container background subtraction error message not as expected", response.getResponseBody().contains("Run " + foreignRunId + " does not exist")); + + log("Verify the project's run was not modified by the cross-container request"); + clickProject(TEST_ASSAY_PRJ_ELISPOT); + clickAndWait(Locator.linkWithText(TEST_ASSAY_ELISPOT)); + DataRegionTable runTable = new DataRegionTable("Runs", this); + for (String item : runTable.getColumnDataAsText("Background Subtraction")) + assertEquals("Cross-container request must not change background subtraction", "false", item); + } + + private long getFirstElispotRunId() + { + SelectRowsCommand select = new SelectRowsCommand("assay.ELISpot." + QueryKey.encodePart(TEST_ASSAY_ELISPOT), "Runs"); + try + { + SelectRowsResponse response = select.execute(createDefaultConnection(), getProjectName()); + return ((Number) response.getRowset().iterator().next().getValue("RowId")).longValue(); + } + catch (IOException | CommandException e) + { + throw new RuntimeException(e); + } + } + // Unable to apply background substitution to runs imported with a transform script. protected void removeTransformScript() { diff --git a/src/org/labkey/test/tests/nab/NabAssayTest.java b/src/org/labkey/test/tests/nab/NabAssayTest.java index 1585551864..71181edf31 100644 --- a/src/org/labkey/test/tests/nab/NabAssayTest.java +++ b/src/org/labkey/test/tests/nab/NabAssayTest.java @@ -16,9 +16,16 @@ package org.labkey.test.tests.nab; +import org.jetbrains.annotations.Nullable; +import org.json.JSONObject; import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.labkey.remoteapi.CommandException; +import org.labkey.remoteapi.SimplePostCommand; +import org.labkey.remoteapi.query.Filter; +import org.labkey.remoteapi.query.SelectRowsCommand; +import org.labkey.remoteapi.query.SelectRowsResponse; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.Locators; @@ -37,13 +44,18 @@ import org.labkey.test.pages.query.NewQueryPage; import org.labkey.test.pages.query.SourceQueryPage; import org.labkey.test.tests.AbstractAssayTest; +import org.labkey.test.util.APIAssayHelper; +import org.labkey.test.util.ApiPermissionsHelper; import org.labkey.test.util.AssayImportOptions; import org.labkey.test.util.AssayImporter; import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.DilutionAssayHelper; import org.labkey.test.util.LogMethod; +import org.labkey.test.util.PermissionsHelper; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.QCAssayScriptHelper; +import org.labkey.test.util.SimpleHttpRequest; +import org.labkey.test.util.SimpleHttpResponse; import org.labkey.test.util.TestLogger; import org.labkey.test.util.WikiHelper; import org.openqa.selenium.WebDriverException; @@ -51,13 +63,16 @@ import org.openqa.selenium.support.ui.ExpectedConditions; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; @Category({Daily.class, Assays.class}) @BaseWebDriverTest.ClassTimeout(minutes = 15) @@ -73,6 +88,10 @@ public class NabAssayTest extends AbstractAssayTest protected final static String TEST_ASSAY_USR_NAB_READER = "nabreader1@security.test"; private final static String TEST_ASSAY_GRP_NAB_READER = "Nab Dataset Reader"; + // Container-scoping fixtures (GitHub Kanban #1892, NAB-1/2/8/9): a "bystander" folder and a user privileged only there. + private final static String TEST_ASSAY_FLDR_NAB_SCOPE = "NabScopeBystanderFolder"; + private final static String TEST_ASSAY_USR_NAB_SCOPE = "nabscope@security.test"; + private static final String NAB_FILENAME2 = "m0902053;3999.xls"; protected final File TEST_ASSAY_NAB_FILE1 = TestFileUtils.getSampleData("Nab/m0902051;3997.xls"); protected final File TEST_ASSAY_NAB_FILE2 = TestFileUtils.getSampleData("Nab/" + NAB_FILENAME2); @@ -171,6 +190,8 @@ protected void doCleanup(boolean afterTest) throws TestTimeoutException { super.doCleanup(afterTest); + _userHelper.deleteUsers(false, TEST_ASSAY_USR_NAB_SCOPE); + try { new QCAssayScriptHelper(this).deleteEngine(); @@ -347,6 +368,13 @@ public void runUITests() build()).doImport(); verifyRunDetails(); + + verifyCrossContainerExcludedWellsDenied(); + verifyCrossContainerQCControlInfoDenied(); + verifyCrossContainerQCDataDenied(); + verifyCrossContainerSaveQCControlInfoDenied(); + verifyCrossContainerGraphDenied(); + // Test editing runs // Set the design to allow editing clickAndWait(Locator.linkWithText("View Runs")); @@ -383,6 +411,9 @@ public void runUITests() startSystemMaintenance("Database"); waitForSystemMaintenanceCompletion(); + // Verify cross-container access control for the run/specimen-resolving actions (NAB-1/2/8/9) while the imported runs are still present. + verifyContainerScopedAccessControl(); + // Return to the run list navigateToFolder(getProjectName(), TEST_ASSAY_FLDR_NAB); clickAndWait(Locator.linkWithText(TEST_ASSAY_NAB)); @@ -489,6 +520,207 @@ public void runUITests() runNabQCTest(); } + /** + * GitHub Kanban #1892: Selenium coverage for the NAb container-scoping fixes (NAB-1, NAB-2, NAB-8, NAB-9). Each of these + * actions resolves a run (by global rowId) or a NAb specimen object id (resolved to its run by a global, cross-container + * lookup) without an intrinsic container check. A user privileged only in a bystander folder must not be able to reach a + * run living in the (foreign) assay folder by pointing one of these actions at its row/object id while scoping the request + * to the bystander folder. We capture the ids as the admin, then issue the requests as an impersonated bystander Editor. + */ + @LogMethod + private void verifyContainerScopedAccessControl() + { + // Capture a protocol id, a run rowId, and a NAb specimen object id from the (foreign) assay folder — done as the admin, before impersonating. + int protocolId = ((APIAssayHelper) _assayHelper).getIdFromAssayName(TEST_ASSAY_NAB, "/" + getProjectName()); + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + int objectId = firstRowId("Data", runFolderPath, null); + assertTrue("Expected an imported NAb run and specimen to scope against", runId > 0 && objectId > 0); + + // A user who is an Editor (read + delete) in the bystander folder only — no access to the assay folder where the run lives. + _containerHelper.createSubfolder(getProjectName(), TEST_ASSAY_FLDR_NAB_SCOPE); + String bystanderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB_SCOPE; + _userHelper.createUser(TEST_ASSAY_USR_NAB_SCOPE); + new ApiPermissionsHelper(this).addMemberToRole(TEST_ASSAY_USR_NAB_SCOPE, "Editor", PermissionsHelper.MemberType.user, "/" + bystanderPath); + + impersonate(TEST_ASSAY_USR_NAB_SCOPE); + try + { + // NAB-2: DownloadDatafileAction resolves the run by global rowId. + assertForeignContainerRejected("downloadDatafile (NAB-2)", + WebTestHelper.buildURL("nabassay", bystanderPath, "downloadDatafile", Map.of("rowId", String.valueOf(runId))), "GET"); + + // NAB-8: NabMultiGraphAction -> MultiGraphAction.getView resolves the object ids to runs. + assertForeignContainerRejected("nabMultiGraph (NAB-8)", + WebTestHelper.buildURL("nabassay", bystanderPath, "nabMultiGraph", Map.of("protocolId", String.valueOf(protocolId), "id", String.valueOf(objectId))), "GET"); + + // NAB-9: NabGraphSelectedAction -> GraphSelectedAction.getView resolves the object ids to runs. + assertForeignContainerRejected("nabGraphSelected (NAB-9)", + WebTestHelper.buildURL("nabassay", bystanderPath, "nabGraphSelected", Map.of("protocolId", String.valueOf(protocolId), "id", String.valueOf(objectId))), "GET"); + + // NAB-1: DeleteRunAction resolves the run by global rowId; this action is a POST. + assertForeignContainerRejected("deleteRun (NAB-1)", + WebTestHelper.buildURL("nabassay", bystanderPath, "deleteRun", Map.of("rowId", String.valueOf(runId))), "POST"); + } + finally + { + stopImpersonating(); + } + + // The run must survive the rejected cross-container delete attempt. + assertEquals("Foreign-container delete must not remove the run", runId, firstRowId("Runs", runFolderPath, List.of(new Filter("RowId", runId)))); + } + + /** Fetch the RowId of the first row of an assay.NAb query across the project's subfolders, as the admin. Returns -1 if none. */ + private int firstRowId(String queryName, String containerPath, @Nullable List filters) + { + SelectRowsCommand command = new SelectRowsCommand("assay.NAb." + TEST_ASSAY_NAB, queryName); + command.setColumns(List.of("RowId")); + if (filters != null) + command.setFilters(filters); + try + { + SelectRowsResponse response = command.execute(createDefaultConnection(), containerPath); + return response.getRows().isEmpty() ? -1 : ((Number) response.getRows().get(0).get("RowId")).intValue(); + } + catch (IOException | CommandException e) + { + throw new RuntimeException(e); + } + } + + private void assertForeignContainerRejected(String description, String url, String requestMethod) + { + SimpleHttpRequest request = new SimpleHttpRequest(url, requestMethod); + request.copySession(getDriver()); // execute as the impersonated bystander user (carries CSRF token for the POST) + request.clearLogin(); // rely solely on the impersonated session, not admin basic-auth + SimpleHttpResponse response; + try + { + response = request.getResponse(); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + assertEquals("Foreign-container request should be rejected with 404: " + description, 404, response.getResponseCode()); + assertTrue("Foreign-container rejection for " + description + " should report the resource does not exist, was: " + response.getResponseBody(), + response.getResponseBody().contains("exist")); + } + + // GitHub Kanban #1892: verify GetExcludedWellsAction resolves run by RowId and container + @LogMethod + private void verifyCrossContainerExcludedWellsDenied() + { + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + + log("Excluded wells request from the run's own folder should succeed"); + SimpleHttpResponse ownFolder = getNabRunApi("getExcludedWells.api", runFolderPath, runId); + assertEquals("Excluded wells request in the run's own folder should succeed", 200, ownFolder.getResponseCode()); + assertTrue("Excluded wells response should include the excluded payload", ownFolder.getResponseBody().contains("\"excluded\"")); + + log("The same run requested from a different container (the project) must be denied, not disclosed"); + SimpleHttpResponse crossContainer = getNabRunApi("getExcludedWells.api", getProjectName(), runId); + assertEquals("Cross-container excluded wells request should be denied", 400, crossContainer.getResponseCode()); + assertFalse("Cross-container request must not disclose excluded well data", crossContainer.getResponseBody().contains("\"excluded\"")); + assertTrue("Cross-container error message not as expected", crossContainer.getResponseBody().contains("NAb Run " + runId + " does not exist.")); + } + + // GitHub Kanban #1892: verify GetQCControlInfoAction resolves run by RowId and container + @LogMethod + private void verifyCrossContainerQCControlInfoDenied() + { + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + + log("QC control info request from the run's own folder should succeed"); + SimpleHttpResponse ownFolder = getNabRunApi("getQCControlInfo.api", runFolderPath, runId); + assertEquals("QC control info request in the run's own folder should succeed", 200, ownFolder.getResponseCode()); + assertTrue("QC control info response should include the plates payload", ownFolder.getResponseBody().contains("\"plates\"")); + + log("The same run requested from a different container (the project) must be denied, not disclosed"); + SimpleHttpResponse crossContainer = getNabRunApi("getQCControlInfo.api", getProjectName(), runId); + assertEquals("Cross-container QC control info request should be denied", 404, crossContainer.getResponseCode()); + assertFalse("Cross-container request must not disclose plate data", crossContainer.getResponseBody().contains("\"plates\"")); + assertTrue("Cross-container error message not as expected", crossContainer.getResponseBody().contains("Run " + runId + " does not exist.")); + } + + // GitHub Kanban #1892: verify QCDataAction resolves run by RowId and container + @LogMethod + private void verifyCrossContainerQCDataDenied() + { + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + + log("QC data view from the run's own folder should render"); + SimpleHttpResponse ownFolder = getNabRunApi("qcData.view", runFolderPath, runId); + assertEquals("QC data view in the run's own folder should render", 200, ownFolder.getResponseCode()); + + log("The same run requested from a different container (the project) must be denied, not disclosed"); + SimpleHttpResponse crossContainer = getNabRunApi("qcData.view", getProjectName(), runId); + assertEquals("Cross-container QC data view should be denied", 404, crossContainer.getResponseCode()); + assertTrue("Cross-container error message not as expected", crossContainer.getResponseBody().contains("Run " + runId + " does not exist.")); + } + + // GitHub Kanban #1892: verify SaveQCControlInfoAction resolves run by RunId and container (write path) + @LogMethod + private void verifyCrossContainerSaveQCControlInfoDenied() + { + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + + log("Saving QC control info for the run from a different container (the project) must be denied"); + SimplePostCommand command = new SimplePostCommand("nabassay", "saveQCControlInfo"); + command.setJsonObject(new JSONObject().put("runId", runId)); + try + { + command.execute(createDefaultConnection(), getProjectName()); + fail("Cross-container QC control info save should have been denied"); + } + catch (CommandException e) + { + assertEquals("Cross-container QC control info save should be denied", 400, e.getStatusCode()); + assertTrue("Cross-container error message not as expected", e.getMessage().contains("NAb Run " + runId + " does not exist.")); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + } + + // GitHub Kanban #1892: verify DilutionGraphAction (NAb GraphAction) resolves run by RowId and container + @LogMethod + private void verifyCrossContainerGraphDenied() + { + String runFolderPath = getProjectName() + "/" + TEST_ASSAY_FLDR_NAB; + int runId = firstRowId("Runs", runFolderPath, null); + + log("NAb graph from the run's own folder should render"); + SimpleHttpResponse ownFolder = getNabRunApi("graph.view", runFolderPath, runId); + assertEquals("NAb graph in the run's own folder should render", 200, ownFolder.getResponseCode()); + + log("The same run requested from a different container (the project) must be denied, not disclosed"); + SimpleHttpResponse crossContainer = getNabRunApi("graph.view", getProjectName(), runId); + assertEquals("Cross-container NAb graph should be denied", 404, crossContainer.getResponseCode()); + assertTrue("Cross-container error message not as expected", crossContainer.getResponseBody().contains("Run " + runId + " does not exist.")); + } + + private SimpleHttpResponse getNabRunApi(String action, String containerPath, long runId) + { + String url = WebTestHelper.buildURL("nabassay", containerPath, action, Map.of("rowId", String.valueOf(runId))); + SimpleHttpRequest request = new SimpleHttpRequest(url); + request.copySession(getDriver()); + try + { + return request.getResponse(); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + } + //Issue 17050: UnsupportedOperationException from org.labkey.nab.query.NabProtocolSchema$NabResultsQueryView.createDataView private void directBrowserQueryTest() {