Skip to content

Commit 14fdae7

Browse files
committed
Merge remote-tracking branch 'origin/release25.7-SNAPSHOT' into 25.7_fb_owasp_lists_audit
2 parents 439f6aa + a8c9d82 commit 14fdae7

6 files changed

Lines changed: 493 additions & 2 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ protected void htmlFormat()
175175
Locator.tag("img").withAttribute("alt", "plot of chunk blood-pressure-scatter")), // new
176176
Locator.tag("pre").containing("## \"1\",249318596,\"2008-05-17\",86,36,129,76,64"),
177177
Locator.tag("pre").withText("## knitr says hello to HTML!"),
178-
Locator.tag("pre").startsWith("## Error").containing(": non-numeric argument to binary operator"),
178+
Locator.tag("pre").startsWith("## Error").containing("non-numeric argument to binary operator"),
179179
Locator.tag("p").startsWith("Well, everything seems to be working. Let's ask R what is the value of \u03C0? Of course it is 3.141"),
180180
nonceCheckSuccessLoc // Inline script should run
181181
};

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.labkey.test.tests;
22

3+
import org.apache.hc.core5.http.HttpStatus;
34
import org.assertj.core.api.Assertions;
45
import org.jetbrains.annotations.Nullable;
56
import org.junit.Assert;
@@ -9,25 +10,36 @@
910
import org.labkey.test.BaseWebDriverTest;
1011
import org.labkey.test.Locator;
1112
import org.labkey.test.TestFileUtils;
13+
import org.labkey.test.WebTestHelper;
1214
import org.labkey.test.categories.Daily;
1315
import org.labkey.test.components.DomainDesignerPage;
1416
import org.labkey.test.components.domain.DomainFieldRow;
1517
import org.labkey.test.components.domain.DomainFormPanel;
18+
import org.labkey.test.pages.ReactAssayDesignerPage;
1619
import org.labkey.test.pages.experiment.UpdateSampleTypePage;
1720
import org.labkey.test.params.FieldDefinition;
1821
import org.labkey.test.params.experiment.SampleTypeDefinition;
22+
import org.labkey.test.util.ApiPermissionsHelper;
1923
import org.labkey.test.util.DataRegionTable;
24+
import org.labkey.test.util.PasswordUtil;
25+
import org.labkey.test.util.PermissionsHelper;
2026
import org.labkey.test.util.PortalHelper;
2127
import org.labkey.test.util.SampleTypeHelper;
2228
import org.labkey.test.util.TestDataGenerator;
2329

30+
import org.openqa.selenium.By;
31+
import org.openqa.selenium.support.ui.ExpectedConditions;
32+
2433
import java.io.File;
34+
import java.net.URI;
2535
import java.util.List;
2636

2737
@Category({Daily.class})
2838
@BaseWebDriverTest.ClassTimeout(minutes = 2)
2939
public class AttachmentFieldTest extends BaseWebDriverTest
3040
{
41+
private static final String RESTRICTED_PROJECT = "AttachmentFieldTest Restricted Project";
42+
private static final String RESTRICTED_USER = "[email protected]";
3143
private final File SAMPLE_FILE = new File(TestFileUtils.getSampleData("fileTypes"), "jpg_sample.jpg");
3244

3345
@BeforeClass
@@ -57,6 +69,14 @@ private void doSetup()
5769
portalHelper.addBodyWebPart("Lists");
5870
}
5971

72+
@Override
73+
protected void doCleanup(boolean afterTest)
74+
{
75+
super.doCleanup(afterTest);
76+
_containerHelper.deleteProject(RESTRICTED_PROJECT, false);
77+
_userHelper.deleteUsers(false, RESTRICTED_USER);
78+
}
79+
6080
@Test
6181
public void testFileFieldInSampleType()
6282
{
@@ -138,4 +158,63 @@ public void testAttachmentFieldInLists()
138158
File downloadedFile = doAndWaitForDownload(() -> Locator.tagWithAttributeContaining("img", "title", SAMPLE_FILE.getName()).findElement(getDriver()).click());
139159
Assert.assertTrue("Downloaded file is empty", downloadedFile.length() > 0);
140160
}
161+
162+
// Kanban #1924
163+
@Test
164+
public void testDownloadFileLinkCrossContainerPermission()
165+
{
166+
final String assayName = "CrossContainerAssay";
167+
final String runFieldName = "runFile";
168+
169+
log("Create restricted project with Assay folder type to provide a pipeline root for file storage");
170+
_containerHelper.createProject(RESTRICTED_PROJECT, "Assay");
171+
172+
log("Create a General assay with a run-level file link field");
173+
goToProjectHome(RESTRICTED_PROJECT);
174+
goToManageAssays();
175+
ReactAssayDesignerPage assayDesigner = _assayHelper.createAssayDesign("General", assayName);
176+
assayDesigner.setEditableRuns(true);
177+
assayDesigner.goToRunFields().addField(runFieldName).setType(FieldDefinition.ColumnType.File);
178+
assayDesigner.clickFinish();
179+
180+
log("Import a minimal assay run");
181+
clickAndWait(Locator.linkWithText(assayName));
182+
clickButton("Import Data");
183+
clickButton("Next");
184+
setFormElement(Locator.name("name"), "TestRun");
185+
setFormElement(Locator.name("TextAreaDataCollector.textArea"),
186+
"Specimen ID\tParticipant ID\tVisit ID\n100\t1A2B\t1");
187+
clickButton("Save and Finish");
188+
189+
log("Edit the run to set the file field");
190+
clickAndWait(Locator.linkWithText("view runs"));
191+
new DataRegionTable("Runs", getDriver()).clickEditRow(0);
192+
setFormElement(Locator.name("quf_" + runFieldName), SAMPLE_FILE);
193+
clickButton("Submit");
194+
waitForElement(DataRegionTable.updateLinkLocator());
195+
196+
log("Hover over the run file thumbnail to reveal the popup and get the objectURI-based downloadFileLink URL");
197+
mouseOver(Locator.xpath("//img[contains(@title, '" + SAMPLE_FILE.getName() + "')]"));
198+
longWait().until(ExpectedConditions.visibilityOfElementLocated(By.cssSelector("#helpDiv")));
199+
String restrictedDownloadUrl = Locator.xpath("//div[@id='helpDiv']//img[contains(@src, 'downloadFileLink')]")
200+
.findElement(getDriver()).getAttribute("src");
201+
Assertions.assertThat(restrictedDownloadUrl).as("Expected downloadFileLink URL with objectURI parameter")
202+
.contains("downloadFileLink")
203+
.contains("objectURI");
204+
205+
// Build a cross-container URL: keep the same objectURI (run LSID) and propertyId but use the main project's
206+
// container.
207+
String crossContainerUrl = WebTestHelper.buildURL("core", getProjectName(), "downloadFileLink")
208+
+ "?" + URI.create(restrictedDownloadUrl).getRawQuery();
209+
210+
log("Create a reader user with access to the main project only, not to the restricted project");
211+
_userHelper.createUser(RESTRICTED_USER);
212+
_userHelper.setInitialPassword(RESTRICTED_USER);
213+
new ApiPermissionsHelper(this).addMemberToRole(RESTRICTED_USER, "Reader", PermissionsHelper.MemberType.user, getProjectName());
214+
215+
log("Verify cross-container download is rejected with 403 when user lacks read permission on the object's container");
216+
int status = WebTestHelper.getHttpResponse(crossContainerUrl, RESTRICTED_USER, PasswordUtil.getPassword()).getResponseCode();
217+
Assert.assertEquals("Expected 403 Forbidden when user lacks read permission on the object's container",
218+
HttpStatus.SC_FORBIDDEN, status);
219+
}
141220
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.labkey.test.components.html.BootstrapMenu;
3232
import org.labkey.test.pages.ImportDataPage;
3333
import org.labkey.test.pages.study.specimen.ManageNotificationsPage;
34+
import org.labkey.test.util.ApiPermissionsHelper;
3435
import org.labkey.test.util.DataRegionTable;
3536
import org.labkey.test.util.LogMethod;
3637
import org.labkey.test.util.LoggedParam;
@@ -132,6 +133,7 @@ protected void setupRequestabilityRules()
132133
protected void doVerifySteps() throws IOException
133134
{
134135
verifyActorDetails();
136+
verifySpecimenEventsRedirect();
135137
createRequest();
136138
verifyViews();
137139
verifyAdditionalRequestFields();
@@ -325,6 +327,37 @@ private void verifyActorDetails()
325327
DataRegion(getDriver()).withName("SpecimenRequest").waitFor();
326328
}
327329

330+
// Simulate SpecimenForeignKey redirect behavior
331+
@LogMethod (quiet = true)
332+
private void verifySpecimenEventsRedirect()
333+
{
334+
String targetStudyId = getContainerId();
335+
336+
// Create an empty second folder (doesn't need to be a study) with guest read. We'll attempt to invoke the
337+
// redirect action from this folder.
338+
String folderName = "Another Study";
339+
_containerHelper.createSubfolder(getProjectName(), folderName, "Study");
340+
new ApiPermissionsHelper(this).setSiteGroupPermissions("Guests", "Reader");
341+
342+
// Happy path - admin should redirect
343+
String baseUrl = WebTestHelper.getBaseURL() + "/" + getProjectName() + "/" + folderName + "/specimen-specimenEventsRedirect.view?targetStudy=" + targetStudyId + "&id=";
344+
String url = baseUrl + "AAA07XK5-01";
345+
beginAt(url);
346+
assertTextPresent("Vial History", "999320812", "350V0600294A");
347+
348+
// Guest has access in Another Study but not in the target study (My Study), so should not redirect
349+
signOut();
350+
beginAt(baseUrl + "abcdefg_123456"); // Bogus ID and user doesn't have read permission
351+
assertTextPresent("Unable to resolve the Specimen ID and target Study");
352+
beginAt(url); // Valid ID, but user doesn't have read permission to target study, so same error
353+
assertTextPresent("Unable to resolve the Specimen ID and target Study");
354+
355+
// Sign in and back to the main study
356+
signIn();
357+
goToProjectHome();
358+
clickFolder(getFolderName());
359+
}
360+
328361
@LogMethod
329362
private void createRequest()
330363
{
@@ -1048,7 +1081,7 @@ private void verifyDrawTimestampConflict(String qcControl, String timestamp, Str
10481081
{
10491082
if (StringUtils.isBlank(qcControl))
10501083
{
1051-
// no conflict, so all three fields shold be valid
1084+
// no conflict, so all three fields should be valid
10521085
assertTrue(StringUtils.isNotBlank(timestamp));
10531086
assertTrue(StringUtils.isNotBlank(time));
10541087
assertTrue(StringUtils.isNotBlank(date));
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package org.labkey.test.tests.core.security;
2+
3+
import org.apache.hc.core5.http.HttpStatus;
4+
import org.junit.BeforeClass;
5+
import org.junit.Test;
6+
import org.junit.experimental.categories.Category;
7+
import org.labkey.test.BaseWebDriverTest;
8+
import org.labkey.test.TestTimeoutException;
9+
import org.labkey.test.WebTestHelper;
10+
import org.labkey.test.categories.Daily;
11+
import org.labkey.test.util.APIContainerHelper;
12+
import org.labkey.test.util.ApiPermissionsHelper;
13+
import org.labkey.test.util.PasswordUtil;
14+
import org.labkey.test.util.PermissionsHelper;
15+
16+
import java.util.List;
17+
import java.util.Map;
18+
19+
import static org.junit.Assert.assertEquals;
20+
21+
/**
22+
* Tests cross-container permission enforcement in CoreController.GetContainerInfoAction (Kanban #1924).
23+
*
24+
* The action accepts an optional {@code containerPath} parameter. Prior to the fix, a user who had
25+
* ReadPermission on the request container could supply any container path and receive information about
26+
* it — even containers they had no access to. The fix adds a check that the user also has ReadPermission
27+
* on the resolved container before returning any data.
28+
*/
29+
@Category({Daily.class})
30+
public class GetContainerInfoAPITest extends BaseWebDriverTest
31+
{
32+
private static final String READABLE_PROJECT = "GetContainerInfoAPITest Readable";
33+
private static final String RESTRICTED_PROJECT = "GetContainerInfoAPITest Restricted";
34+
private static final String READER_USER = "[email protected]";
35+
36+
private final ApiPermissionsHelper _permissions = new ApiPermissionsHelper(this);
37+
38+
public GetContainerInfoAPITest()
39+
{
40+
((APIContainerHelper) _containerHelper).setNavigateToCreatedFolders(false);
41+
}
42+
43+
@BeforeClass
44+
public static void setupProject()
45+
{
46+
GetContainerInfoAPITest init = getCurrentTest();
47+
init.doSetup();
48+
}
49+
50+
private void doSetup()
51+
{
52+
_containerHelper.createProject(READABLE_PROJECT, "Collaboration");
53+
_containerHelper.createProject(RESTRICTED_PROJECT, "Collaboration");
54+
55+
_userHelper.createUser(READER_USER);
56+
_userHelper.setInitialPassword(READER_USER);
57+
_permissions.addMemberToRole(READER_USER, "Reader", PermissionsHelper.MemberType.user, READABLE_PROJECT);
58+
// Intentionally not granting the user any role in RESTRICTED_PROJECT
59+
}
60+
61+
@Override
62+
protected void doCleanup(boolean afterTest) throws TestTimeoutException
63+
{
64+
_containerHelper.deleteProject(READABLE_PROJECT, afterTest);
65+
_containerHelper.deleteProject(RESTRICTED_PROJECT, afterTest);
66+
_userHelper.deleteUsers(false, READER_USER);
67+
}
68+
69+
@Override
70+
protected String getProjectName()
71+
{
72+
return null;
73+
}
74+
75+
@Override
76+
public List<String> getAssociatedModules()
77+
{
78+
return null;
79+
}
80+
81+
// Kanban #1924
82+
@Test
83+
public void testGetContainerInfoAccessControl()
84+
{
85+
// Cross-container denial: user makes the request from READABLE_PROJECT (passing @RequiresPermission),
86+
// but containerPath resolves to RESTRICTED_PROJECT where the user has no ReadPermission. Expect 403.
87+
String restrictedUrl = WebTestHelper.buildURL("core", READABLE_PROJECT, "getContainerInfo",
88+
Map.of("containerPath", RESTRICTED_PROJECT, "newFolderType", "Collaboration"));
89+
int restrictedStatus = WebTestHelper.getHttpResponse(restrictedUrl, READER_USER, PasswordUtil.getPassword())
90+
.getResponseCode();
91+
assertEquals("Expected 403 when user lacks ReadPermission on the containerPath container",
92+
HttpStatus.SC_FORBIDDEN, restrictedStatus);
93+
94+
// Same-container success: containerPath resolves to READABLE_PROJECT where the user is a Reader. Expect 200.
95+
String readableUrl = WebTestHelper.buildURL("core", READABLE_PROJECT, "getContainerInfo",
96+
Map.of("containerPath", READABLE_PROJECT, "newFolderType", "Collaboration"));
97+
int readableStatus = WebTestHelper.getHttpResponse(readableUrl, READER_USER, PasswordUtil.getPassword())
98+
.getResponseCode();
99+
assertEquals("Expected 200 when user has ReadPermission on the containerPath container",
100+
HttpStatus.SC_OK, readableStatus);
101+
}
102+
}

0 commit comments

Comments
 (0)