From 44fd5aed542967c9399275281a3db926b6963dff Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 16 Jun 2026 16:02:20 -0700 Subject: [PATCH 1/6] GitHub Issue 1243: FILE-3: check resource container --- .../UpdateFilePropsContainerScopeTest.java | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java diff --git a/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java b/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java new file mode 100644 index 0000000000..69c0ecb67e --- /dev/null +++ b/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2026 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.test.tests.filecontent; + +import org.jetbrains.annotations.Nullable; +import org.json.JSONArray; +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.test.BaseWebDriverTest; +import org.labkey.test.TestFileUtils; +import org.labkey.test.WebTestHelper; +import org.labkey.test.categories.Daily; +import org.labkey.test.categories.FileBrowser; +import org.labkey.test.components.DomainDesignerPage; +import org.labkey.test.util.PortalHelper; +import org.labkey.test.util.core.webdav.WebDavUtils; + +import java.io.File; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +@Category({Daily.class, FileBrowser.class}) +@BaseWebDriverTest.ClassTimeout(minutes = 4) +public class UpdateFilePropsContainerScopeTest extends BaseWebDriverTest +{ + private static final String FOLDER_A = "FolderA"; + private static final String FOLDER_B = "FolderB"; + private static final String CUSTOM_PROPERTY = "CustomProp"; + + @Override + protected @Nullable String getProjectName() + { + return getClass().getSimpleName() + "Project"; + } + + @Override + public List getAssociatedModules() + { + return Arrays.asList("filecontent"); + } + + @BeforeClass + public static void doSetup() + { + UpdateFilePropsContainerScopeTest init = getCurrentTest(); + init.doSetupSteps(); + } + + @Override + protected void doCleanup(boolean afterTest) + { + _containerHelper.deleteProject(getProjectName(), afterTest); + } + + private void doSetupSteps() + { + _containerHelper.createProject(getProjectName(), null); + _containerHelper.createSubfolder(getProjectName(), FOLDER_A); + _containerHelper.createSubfolder(getProjectName(), FOLDER_B); + + // FolderA needs a file properties domain so UpdateFilePropsAction's validation block runs. + navigateToFolder(getProjectName(), FOLDER_A); + new PortalHelper(this).doInAdminMode(p -> p.addWebPart("Files")); + DomainDesignerPage designer = _fileBrowserHelper.goToEditProperties(); + designer.fieldsPanel().addField(CUSTOM_PROPERTY); + designer.clickFinish(); + + navigateToFolder(getProjectName(), FOLDER_B); + new PortalHelper(this).doInAdminMode(p -> p.addWebPart("Files")); + } + + @Test + public void testForeignContainerFileRejected() throws Exception + { + final File localFile = TestFileUtils.getSampleData("security/InlineFile.html"); + navigateToFolder(getProjectName(), FOLDER_A); + _fileBrowserHelper.uploadFile(localFile); + String localFileUrl = WebDavUtils.buildBaseWebDavUrl(getProjectName() + "/" + FOLDER_A) + localFile.getName(); + + final File foreignFile = TestFileUtils.getSampleData("security/InlineFile2.html"); + navigateToFolder(getProjectName(), FOLDER_B); + _fileBrowserHelper.uploadFile(foreignFile); + String foreignFileUrl = WebDavUtils.buildBaseWebDavUrl(getProjectName() + "/" + FOLDER_B) + foreignFile.getName(); + + log("Same-container file id should be accepted."); + updateFileProps(FOLDER_A, localFileUrl, localFile.getName()); + + log("Foreign-container file id should be rejected."); + try + { + updateFileProps(FOLDER_A, foreignFileUrl, foreignFile.getName()); + fail("Expected rejection: UpdateFilePropsAction must refuse a file id resolving outside the current folder."); + } + catch (CommandException ex) + { + assertTrue("Expected 'Invalid file' in error message, got: " + ex.getMessage(), + ex.getMessage().contains("Invalid file")); + } + } + + private void updateFileProps(String folder, String fileId, String fileName) throws Exception + { + JSONObject entry = new JSONObject(); + entry.put("id", fileId); + entry.put("name", fileName); + entry.put(CUSTOM_PROPERTY, "value"); + JSONArray files = new JSONArray(); + files.put(entry); + JSONObject body = new JSONObject(); + body.put("files", files); + + SimplePostCommand cmd = new SimplePostCommand("filecontent", "updateFileProps"); + cmd.setJsonObject(body); + cmd.execute(WebTestHelper.getRemoteApiConnection(), getProjectName() + "/" + folder); + } + + @Override + public BrowserType bestBrowser() + { + return BrowserType.CHROME; + } +} From 4d4554cfe084e7e6266700036fb9d18cd2e41f1b Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 16 Jun 2026 16:11:10 -0700 Subject: [PATCH 2/6] fix test --- .../tests/filecontent/UpdateFilePropsContainerScopeTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java b/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java index 69c0ecb67e..a6c8732293 100644 --- a/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java +++ b/src/org/labkey/test/tests/filecontent/UpdateFilePropsContainerScopeTest.java @@ -97,6 +97,7 @@ public void testForeignContainerFileRejected() throws Exception _fileBrowserHelper.uploadFile(localFile); String localFileUrl = WebDavUtils.buildBaseWebDavUrl(getProjectName() + "/" + FOLDER_A) + localFile.getName(); + goToProjectHome(); final File foreignFile = TestFileUtils.getSampleData("security/InlineFile2.html"); navigateToFolder(getProjectName(), FOLDER_B); _fileBrowserHelper.uploadFile(foreignFile); From 2e15cf73f072f8d983d53ed2b22824ab9649d326 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 17 Jun 2026 12:38:51 -0700 Subject: [PATCH 3/6] Add redundant permission check and test --- src/org/labkey/test/tests/SpecimenTest.java | 65 +++++++++++++++------ 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/org/labkey/test/tests/SpecimenTest.java b/src/org/labkey/test/tests/SpecimenTest.java index 0277fc6e05..4ab23080a3 100644 --- a/src/org/labkey/test/tests/SpecimenTest.java +++ b/src/org/labkey/test/tests/SpecimenTest.java @@ -31,10 +31,12 @@ import org.labkey.test.components.html.BootstrapMenu; import org.labkey.test.pages.ImportDataPage; import org.labkey.test.pages.study.specimen.ManageNotificationsPage; +import org.labkey.test.util.ApiPermissionsHelper; import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; import org.labkey.test.util.PasswordUtil; +import org.labkey.test.util.PermissionsHelper; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.StudyHelper; import org.labkey.test.util.TextSearcher; @@ -131,23 +133,24 @@ protected void setupRequestabilityRules() @LogMethod protected void doVerifySteps() throws IOException { - verifyActorDetails(); - createRequest(); - verifyViews(); - verifyAdditionalRequestFields(); - verifyNotificationEmails(); - verifyInactiveUsersInRequests(); - verifyRequestCancel(); - verifyReports(); - exportSpecimenTest(); - verifyRequestingLocationRestriction(); - verifySpecimenTableAttachments(); - searchTest(); - verifySpecimenGroupings(); - verifyRequestEnabled(); - disableRequests(); - verifyRequestsDisabled(); - verifyDrawTimestamp(); +// verifyActorDetails(); +// createRequest(); +// verifyViews(); +// verifyAdditionalRequestFields(); +// verifyNotificationEmails(); +// verifyInactiveUsersInRequests(); +// verifyRequestCancel(); +// verifyReports(); +// exportSpecimenTest(); +// verifyRequestingLocationRestriction(); +// verifySpecimenTableAttachments(); +// searchTest(); +// verifySpecimenGroupings(); +// verifyRequestEnabled(); +// disableRequests(); +// verifyRequestsDisabled(); +// verifyDrawTimestamp(); + verifySpecimenEventsRedirect(); } private void checkSpecimenReport() @@ -1048,7 +1051,7 @@ private void verifyDrawTimestampConflict(String qcControl, String timestamp, Str { if (StringUtils.isBlank(qcControl)) { - // no conflict, so all three fields shold be valid + // no conflict, so all three fields should be valid assertTrue(StringUtils.isNotBlank(timestamp)); assertTrue(StringUtils.isNotBlank(time)); assertTrue(StringUtils.isNotBlank(date)); @@ -1112,4 +1115,30 @@ private void verifyHistory(int vialIndex, @LoggedParam String qcControl) goBack(); DataRegion(getDriver()).withName("SpecimenDetail").waitFor(); } + + // Simulate SpecimenForeignKey redirect behavior + @LogMethod (quiet = true) + private void verifySpecimenEventsRedirect() + { + String targetStudyId = getContainerId(); + + // Create an empty second study where we'll initiate the redirect + String folderName = "Another Study"; + _containerHelper.createSubfolder(getProjectName(), folderName, "Study"); + createDefaultStudy(); + new ApiPermissionsHelper(this).setSiteGroupPermissions("Guests", "Reader"); + + // Happy path - admin should redirect + String baseUrl = WebTestHelper.getBaseURL() + "/" + getProjectName() + "/" + folderName + "/specimen-specimenEventsRedirect.view?targetStudy=" + targetStudyId + "&id="; + String url = baseUrl + "AAA07XK5-01"; + beginAt(url); + assertTextPresent("Vial History", "999320812", "350V0600294A"); + + // Guest has access in Another Study but not in the target study (My Study), so should not redirect + signOut(); + beginAt(baseUrl + "abcdefg_123456"); // Bogus ID and user doesn't have read permission + assertTextPresent("Unable to resolve the Specimen ID and target Study"); + beginAt(url); // Valid ID, but user doesn't have read permission to target study, so same error + assertTextPresent("Unable to resolve the Specimen ID and target Study"); + } } From c522d98f8e85ecf347c3c4015373befa8c86f0d0 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 17 Jun 2026 13:08:15 -0700 Subject: [PATCH 4/6] Uncomment --- src/org/labkey/test/tests/SpecimenTest.java | 35 ++++++++++----------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/org/labkey/test/tests/SpecimenTest.java b/src/org/labkey/test/tests/SpecimenTest.java index 4ab23080a3..c62010a59b 100644 --- a/src/org/labkey/test/tests/SpecimenTest.java +++ b/src/org/labkey/test/tests/SpecimenTest.java @@ -36,7 +36,6 @@ import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; import org.labkey.test.util.PasswordUtil; -import org.labkey.test.util.PermissionsHelper; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.StudyHelper; import org.labkey.test.util.TextSearcher; @@ -133,23 +132,23 @@ protected void setupRequestabilityRules() @LogMethod protected void doVerifySteps() throws IOException { -// verifyActorDetails(); -// createRequest(); -// verifyViews(); -// verifyAdditionalRequestFields(); -// verifyNotificationEmails(); -// verifyInactiveUsersInRequests(); -// verifyRequestCancel(); -// verifyReports(); -// exportSpecimenTest(); -// verifyRequestingLocationRestriction(); -// verifySpecimenTableAttachments(); -// searchTest(); -// verifySpecimenGroupings(); -// verifyRequestEnabled(); -// disableRequests(); -// verifyRequestsDisabled(); -// verifyDrawTimestamp(); + verifyActorDetails(); + createRequest(); + verifyViews(); + verifyAdditionalRequestFields(); + verifyNotificationEmails(); + verifyInactiveUsersInRequests(); + verifyRequestCancel(); + verifyReports(); + exportSpecimenTest(); + verifyRequestingLocationRestriction(); + verifySpecimenTableAttachments(); + searchTest(); + verifySpecimenGroupings(); + verifyRequestEnabled(); + disableRequests(); + verifyRequestsDisabled(); + verifyDrawTimestamp(); verifySpecimenEventsRedirect(); } From dfee8f1aded032ed9b1f52a15872e8aa77cfb451 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 17 Jun 2026 13:16:48 -0700 Subject: [PATCH 5/6] Doesn't need to be a study --- src/org/labkey/test/tests/SpecimenTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/org/labkey/test/tests/SpecimenTest.java b/src/org/labkey/test/tests/SpecimenTest.java index c62010a59b..59bad05bb9 100644 --- a/src/org/labkey/test/tests/SpecimenTest.java +++ b/src/org/labkey/test/tests/SpecimenTest.java @@ -1121,10 +1121,10 @@ private void verifySpecimenEventsRedirect() { String targetStudyId = getContainerId(); - // Create an empty second study where we'll initiate the redirect + // Create an empty second folder (doesn't need to be a study) with guest read. We'll attempt to invoke the + // redirect action from this folder. String folderName = "Another Study"; _containerHelper.createSubfolder(getProjectName(), folderName, "Study"); - createDefaultStudy(); new ApiPermissionsHelper(this).setSiteGroupPermissions("Guests", "Reader"); // Happy path - admin should redirect From 349ea63785ef97947312e1eb5c6f56fc96cca5b6 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 17 Jun 2026 16:21:58 -0700 Subject: [PATCH 6/6] Fix new case --- src/org/labkey/test/tests/SpecimenTest.java | 59 +++++++++++---------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/org/labkey/test/tests/SpecimenTest.java b/src/org/labkey/test/tests/SpecimenTest.java index 59bad05bb9..b91c164d34 100644 --- a/src/org/labkey/test/tests/SpecimenTest.java +++ b/src/org/labkey/test/tests/SpecimenTest.java @@ -133,6 +133,7 @@ protected void setupRequestabilityRules() protected void doVerifySteps() throws IOException { verifyActorDetails(); + verifySpecimenEventsRedirect(); createRequest(); verifyViews(); verifyAdditionalRequestFields(); @@ -149,7 +150,6 @@ protected void doVerifySteps() throws IOException disableRequests(); verifyRequestsDisabled(); verifyDrawTimestamp(); - verifySpecimenEventsRedirect(); } private void checkSpecimenReport() @@ -327,6 +327,37 @@ private void verifyActorDetails() DataRegion(getDriver()).withName("SpecimenRequest").waitFor(); } + // Simulate SpecimenForeignKey redirect behavior + @LogMethod (quiet = true) + private void verifySpecimenEventsRedirect() + { + String targetStudyId = getContainerId(); + + // Create an empty second folder (doesn't need to be a study) with guest read. We'll attempt to invoke the + // redirect action from this folder. + String folderName = "Another Study"; + _containerHelper.createSubfolder(getProjectName(), folderName, "Study"); + new ApiPermissionsHelper(this).setSiteGroupPermissions("Guests", "Reader"); + + // Happy path - admin should redirect + String baseUrl = WebTestHelper.getBaseURL() + "/" + getProjectName() + "/" + folderName + "/specimen-specimenEventsRedirect.view?targetStudy=" + targetStudyId + "&id="; + String url = baseUrl + "AAA07XK5-01"; + beginAt(url); + assertTextPresent("Vial History", "999320812", "350V0600294A"); + + // Guest has access in Another Study but not in the target study (My Study), so should not redirect + signOut(); + beginAt(baseUrl + "abcdefg_123456"); // Bogus ID and user doesn't have read permission + assertTextPresent("Unable to resolve the Specimen ID and target Study"); + beginAt(url); // Valid ID, but user doesn't have read permission to target study, so same error + assertTextPresent("Unable to resolve the Specimen ID and target Study"); + + // Sign in and back to the main study + signIn(); + goToProjectHome(); + clickFolder(getFolderName()); + } + @LogMethod private void createRequest() { @@ -1114,30 +1145,4 @@ private void verifyHistory(int vialIndex, @LoggedParam String qcControl) goBack(); DataRegion(getDriver()).withName("SpecimenDetail").waitFor(); } - - // Simulate SpecimenForeignKey redirect behavior - @LogMethod (quiet = true) - private void verifySpecimenEventsRedirect() - { - String targetStudyId = getContainerId(); - - // Create an empty second folder (doesn't need to be a study) with guest read. We'll attempt to invoke the - // redirect action from this folder. - String folderName = "Another Study"; - _containerHelper.createSubfolder(getProjectName(), folderName, "Study"); - new ApiPermissionsHelper(this).setSiteGroupPermissions("Guests", "Reader"); - - // Happy path - admin should redirect - String baseUrl = WebTestHelper.getBaseURL() + "/" + getProjectName() + "/" + folderName + "/specimen-specimenEventsRedirect.view?targetStudy=" + targetStudyId + "&id="; - String url = baseUrl + "AAA07XK5-01"; - beginAt(url); - assertTextPresent("Vial History", "999320812", "350V0600294A"); - - // Guest has access in Another Study but not in the target study (My Study), so should not redirect - signOut(); - beginAt(baseUrl + "abcdefg_123456"); // Bogus ID and user doesn't have read permission - assertTextPresent("Unable to resolve the Specimen ID and target Study"); - beginAt(url); // Valid ID, but user doesn't have read permission to target study, so same error - assertTextPresent("Unable to resolve the Specimen ID and target Study"); - } }