From 34f786da7787c6b343f0007c233354233447e381 Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Tue, 14 Oct 2025 13:05:26 -0700 Subject: [PATCH 1/2] Permission validation for related issues (#2739) --- .../labkey/test/tests/issues/IssuesTest.java | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/org/labkey/test/tests/issues/IssuesTest.java b/src/org/labkey/test/tests/issues/IssuesTest.java index 7d4c75b21c..d077673582 100644 --- a/src/org/labkey/test/tests/issues/IssuesTest.java +++ b/src/org/labkey/test/tests/issues/IssuesTest.java @@ -83,12 +83,14 @@ public class IssuesTest extends BaseWebDriverTest private static final String USER1 = "user1_issuetest@issues.test"; private static final String USER2 = "user2_issuetest@issues.test"; private static final String USER3 = "user3_issuetest@issues.test"; + private static final String CLIENT_USER1 = "clientuser1_issuetest@issues.test"; private static final String user = "reader@issues.test"; private static final Map ISSUE_0 = new HashMap<>(Maps.of("title", ISSUE_TITLE_0, "priority", "2", "comment", "a bright flash of light")); private static final Map ISSUE_1 = new HashMap<>(Maps.of("title", ISSUE_TITLE_1, "priority", "1", "comment", "alien autopsy")); private static final String ISSUE_SUMMARY_WEBPART_NAME = "Issues Summary"; private static final String ISSUE_LIST_REGION_NAME = "issues-issues"; private static final String TEST_GROUP = "testers"; + private static final String CLIENT_GROUP = "clients"; private static final String TEST_EMAIL_TEMPLATE = "You can review this issue here: ^detailsURL^\n" + "Modified by: ^user^\n" + @@ -102,6 +104,7 @@ public class IssuesTest extends BaseWebDriverTest private static String NAME; protected IssuesHelper _issuesHelper; private final ApiPermissionsHelper _permissionsHelper = new ApiPermissionsHelper(this); + private static String CLIENT_PORTAL = "Client Issues"; public IssuesTest() { @@ -155,8 +158,9 @@ protected String getProjectName() @Override protected void doCleanup(boolean afterTest) throws TestTimeoutException { - _userHelper.deleteUsers(false, USER1, USER2); + _userHelper.deleteUsers(false, USER1, USER2, USER3, CLIENT_USER1); _containerHelper.deleteProject(getProjectName(), afterTest); + _containerHelper.deleteProject(CLIENT_PORTAL, afterTest); } public void doInit() @@ -188,6 +192,15 @@ public void doInit() waitAndClickAndWait(Locator.linkContainingText(ISSUE_SUMMARY_WEBPART_NAME)); _issuesHelper.addIssue(ISSUE_0); _issuesHelper.addIssue(ISSUE_1); + + // Create a second project with different permissions + _containerHelper.createProject(CLIENT_PORTAL); + goToProjectHome(CLIENT_PORTAL); + _userHelper.createUser(CLIENT_USER1); + _permissionsHelper.createPermissionsGroup(CLIENT_GROUP); + _permissionsHelper.setPermissions(CLIENT_GROUP, "Editor"); + _permissionsHelper.addUserToProjGroup(CLIENT_USER1, CLIENT_PORTAL, CLIENT_GROUP); + _issuesHelper.createNewIssuesList("tickets", _containerHelper); } @Before @@ -833,7 +846,32 @@ public void relatedIssueTest() .clickSubMenu(false, "Hide related comments"); assertElementNotVisible(related); - // NOTE: still need to test for case where user doesn't have permission to related issue... + // related issue permission tests + Locator commentLocator = Locator.name("related"); + goToProjectHome(CLIENT_PORTAL); + waitAndClickAndWait(Locator.linkContainingText(ISSUE_SUMMARY_WEBPART_NAME)); + String clientIssueId = _issuesHelper.addIssue(Maps.of("assignedTo", NAME, "title", "Client ticket", "priority", "3", "related", issueIdA)).getIssueId(); + + // impersonate a user without permissions to the related issues + impersonate(CLIENT_USER1); + clickAndWait(Locator.linkWithText("Issues List")); + clickAndWait(Locator.linkWithText(clientIssueId)); + updateIssue(); + setFormElement(Locator.name("comment"), "This should work"); + clickButton("Save"); + + // try to add related issue they don't have permission to see + updateIssue(); + setFormElement(relatedLocator, String.format("%s,%s", issueIdA, issueIdB)); + clickButton("Save"); + assertTextPresent("User does not have Read Permission for related issue"); + + // Issue 53820 try to remove a related issue they don't have permission to see + setFormElement(relatedLocator, ""); + clickButton("Save"); + assertTextPresent(String.format("User does not have Read Permission for related issue '%s'", issueIdA)); + clickButton("Cancel"); + stopImpersonating(); } @Test From 99e74889e428faaf27f7e20f44fee6f2e449fc90 Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Wed, 15 Oct 2025 09:05:38 -0700 Subject: [PATCH 2/2] Fix some typos in BaseUpgradeTest (#2745) --- .../test/tests/upgrade/BaseUpgradeTest.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java b/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java index ffaeacacbc..3daaec5540 100644 --- a/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java +++ b/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java @@ -24,9 +24,9 @@ import static org.apache.commons.lang3.StringUtils.trimToNull; /** - * Base test class for tests that setup data and configure a server then verify the persistence or modification of those - * data and configurations after upgrading to a newer version of LabKey.
- * The {@code EariestVersion} and {@code LatestVersion} annotations can be used to skip particular tests when they are + * Base test class for tests that set up data and configure a server, then verify the persistence or modification of + * those data and configurations after upgrading to a newer version of LabKey.
+ * The {@link EarliestVersion} and {@link LatestVersion} annotations can be used to skip particular tests when they are * not relevant to the version of LabKey being upgraded from (specified in the {@code webtest.upgradePreviousVersion} * system property).
* The setup steps will be skipped if the {@code webtest.upgradeSetup} system property is set to {@code false}. @@ -77,7 +77,8 @@ public List getAssociatedModules() */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.METHOD}) - protected @interface EariestVersion { + protected @interface EarliestVersion + { String value(); } @@ -98,12 +99,12 @@ private static class UpgradeVersionCheck implements TestRule @Override public @NotNull Statement apply(Statement base, Description description) { - String eariestVersion = Optional.ofNullable(description.getAnnotation(EariestVersion.class)) - .map(EariestVersion::value).orElse(null); + String earliestVersion = Optional.ofNullable(description.getAnnotation(EarliestVersion.class)) + .map(EarliestVersion::value).orElse(null); String latestVersion = Optional.ofNullable(description.getAnnotation(LatestVersion.class)) .map(LatestVersion::value).orElse(null); - if (isUpgradeSetupPhase || previousVersion == null || (eariestVersion == null && latestVersion == null)) + if (isUpgradeSetupPhase || previousVersion == null || (earliestVersion == null && latestVersion == null)) { return base; // Run the test normally } @@ -114,7 +115,7 @@ private static class UpgradeVersionCheck implements TestRule public void evaluate() throws Throwable { Assume.assumeTrue("Test doesn't support upgrading from version: " + previousVersion, - VersionRange.versionRange(eariestVersion, latestVersion).contains(previousVersion) + VersionRange.versionRange(earliestVersion, latestVersion).contains(previousVersion) ); base.evaluate(); }