From 2d7e93d9493775cd01b09ca6aca99e9f08749d22 Mon Sep 17 00:00:00 2001 From: Sweta Jewargikar Date: Mon, 27 Jan 2025 00:05:28 -0800 Subject: [PATCH 1/2] Automation test for invalidate sessions. (#2234) --- .../pages/core/admin/CustomizeSitePage.java | 1 + .../test/pages/user/UserDetailsPage.java | 12 ++ src/org/labkey/test/tests/ApiKeyTest.java | 179 +++++++++++------- .../test/tests/InvalidateSessionTest.java | 148 +++++++++++++++ .../test/tests/UserPermissionsTest.java | 77 ++++++-- .../test/tests/core/login/PasswordTest.java | 16 +- 6 files changed, 336 insertions(+), 97 deletions(-) create mode 100644 src/org/labkey/test/tests/InvalidateSessionTest.java diff --git a/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java b/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java index 6bafa8dcef..fa03ecc82f 100644 --- a/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java +++ b/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java @@ -295,6 +295,7 @@ public enum XFrameOption public enum KeyExpirationOptions implements OptionSelect.SelectOption { UNLIMITED(-1), + TEN_SECONDS(10), ONE_WEEK(7*SECONDS_PER_DAY), ONE_MONTH(30*SECONDS_PER_DAY), THREE_MONTHS(90*SECONDS_PER_DAY), diff --git a/src/org/labkey/test/pages/user/UserDetailsPage.java b/src/org/labkey/test/pages/user/UserDetailsPage.java index 72c9ecf386..3a6cd52bde 100644 --- a/src/org/labkey/test/pages/user/UserDetailsPage.java +++ b/src/org/labkey/test/pages/user/UserDetailsPage.java @@ -3,7 +3,9 @@ import org.labkey.test.Locator; import org.labkey.test.WebDriverWrapper; import org.labkey.test.WebTestHelper; +import org.labkey.test.components.core.login.SetPasswordForm; import org.labkey.test.pages.LabKeyPage; +import org.labkey.test.util.PasswordUtil; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; @@ -34,6 +36,15 @@ public ClonePermissionsPage clickClonePermission() return new ClonePermissionsPage(getDriver()); } + public SetPasswordForm clickChangePassword() + { + if (PasswordUtil.getUsername().equals(getCurrentUser())) + throw new IllegalArgumentException("Don't change the primary site admin user's password"); + + clickAndWait(elementCache().changePwdButton); + return new SetPasswordForm(getDriver()); + } + @Override protected ElementCache newElementCache() { @@ -44,5 +55,6 @@ protected class ElementCache extends LabKeyPage.ElementCache { WebElement editButton = Locator.lkButton("Edit").findWhenNeeded(this); WebElement cloneButton = Locator.lkButton("Clone Permissions").findWhenNeeded(this); + WebElement changePwdButton = Locator.lkButton("Change Password").findWhenNeeded(this); } } diff --git a/src/org/labkey/test/tests/ApiKeyTest.java b/src/org/labkey/test/tests/ApiKeyTest.java index 6d1873598c..6293e4af9f 100644 --- a/src/org/labkey/test/tests/ApiKeyTest.java +++ b/src/org/labkey/test/tests/ApiKeyTest.java @@ -72,21 +72,21 @@ public class ApiKeyTest extends BaseWebDriverTest private static final String API_USERNAME = "apikey"; private static final TestUser EDITOR_USER = new TestUser("editor@apikey.test"); - @Override - protected void doCleanup(boolean afterTest) throws TestTimeoutException - { - super.doCleanup(afterTest); - _userHelper.deleteUsers(false, EDITOR_USER); - } - @BeforeClass public static void setupProject() { - ApiKeyTest init = (ApiKeyTest) getCurrentTest(); + ApiKeyTest init = getCurrentTest(); init.doSetup(); } + @Override + protected void doCleanup(boolean afterTest) throws TestTimeoutException + { + super.doCleanup(afterTest); + _userHelper.deleteUsers(false, EDITOR_USER); + } + private void doSetup() { _containerHelper.createProject(getProjectName(), null); @@ -107,64 +107,21 @@ public void testSessionKey() throws IOException String apiKey = generateSessionKey(); - verifyValidAPIKey(apiKey); + verifyValidAPIKey(createApiKeyConnection(apiKey, false)); log("Verify session key remains valid if key generation is turned off"); goToAdminConsole() .clickSiteSettings() .setAllowSessionKeys(false) .save(); - verifyValidAPIKey(apiKey); + verifyValidAPIKey(createApiKeyConnection(apiKey, false)); signOut(); log("Verify that logging out invalidates session keys"); - verifyInvalidAPIKey(apiKey); + verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false); simpleSignIn(); log("Verify that session keys remain invalid after logging back in"); - verifyInvalidAPIKey(apiKey); - } - - private void verifyValidAPIKey(String apiKey) throws IOException - { - verifyValidAPIKey(apiKey, false); - } - - private void verifyValidAPIKey(String apiKey, boolean basicAuth) throws IOException - { - Connection cn = new Connection(WebTestHelper.getBaseURL(), basicAuth ? new BasicAuthCredentialsProvider(API_USERNAME, apiKey) : new ApiKeyCredentialsProvider(apiKey)); - try - { - GetSchemasCommand cmd = new GetSchemasCommand(); - GetSchemasResponse resp = cmd.execute(cn, getProjectName()); - List schemaNames = resp.getSchemaNames().stream().map(String::toLowerCase).collect(Collectors.toList()); - Set missingSchemas = new HashSet<>(Arrays.asList("pipeline", "lists", "core")); - missingSchemas.removeAll(schemaNames); - assertTrue("Some expected schemas missing. Schemas missing: " + missingSchemas, missingSchemas.isEmpty()); - } - catch (CommandException e) - { - throw new RuntimeException("Response: " + e.getStatusCode(), e); - } - } - - private void verifyInvalidAPIKey(String apiKey) throws IOException - { - boolean isSessionKey = !apiKey.startsWith(API_USERNAME); - Connection cn = new Connection(WebTestHelper.getBaseURL(), new ApiKeyCredentialsProvider(apiKey)); - try - { - GetSchemasCommand cmd = new GetSchemasCommand(); - cmd.execute(cn, getProjectName()); - if (isSessionKey) - fail("Session key didn't invalidate after logout"); - else - fail("API key should no longer be valid"); - } - catch(CommandException e) - { - assertEquals("Wrong response for invalid " + (isSessionKey ? "session" : "API") + " key", HttpStatus.SC_UNAUTHORIZED, e.getStatusCode()); - log("Success: command failed as expected."); - } + verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false); } @Test @@ -182,14 +139,14 @@ public void testNonAdminUser() throws IOException signIn(EDITOR_USER.getEmail(), EDITOR_USER.getPassword()); String keyDescription = "Key for editing"; String apiKey = generateAPIKey(keyDescription); - verifyValidAPIKey(apiKey); + verifyValidAPIKey(createApiKeyConnection(apiKey, false)); QueryGrid grid = new QueryGrid.QueryGridFinder(getDriver()).waitFor(); int beforeDeleteCount = grid.getRecordCount(); assertFalse("Row with description not found", grid.getRowMap("Description", keyDescription).isEmpty()); grid = deleteAPIKeyViaUI(); - assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount-1, grid.getRecordCount()); - verifyInvalidAPIKey(apiKey); + assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount - 1, grid.getRecordCount()); + verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false); } @Test @@ -205,9 +162,9 @@ public void testStandardApiKey() throws IOException String apiKey = generateAPIKeyAndRecord(_generatedApiKeys); log("Verify active API key via api authentication"); - verifyValidAPIKey(apiKey); + verifyValidAPIKey(createApiKeyConnection(apiKey, false)); log("Verify active API key via basic authentication"); - verifyValidAPIKey(apiKey, true); + verifyValidAPIKey(createApiKeyConnection(apiKey, true)); log("Generate two other keys for use in testing deletion."); generateAPIKey(null); @@ -215,24 +172,75 @@ public void testStandardApiKey() throws IOException QueryGrid grid = new QueryGrid.QueryGridFinder(getDriver()).waitFor(); int beforeDeleteCount = grid.getRecordCount(); grid = deleteAPIKeyViaUI(); - assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount-1, grid.getRecordCount()); + assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount - 1, grid.getRecordCount()); log("Verify existing active API key with disabled api key setting"); goToAdminConsole() .clickSiteSettings() .setAllowApiKeys(false) .save(); - verifyValidAPIKey(apiKey); + verifyValidAPIKey(createApiKeyConnection(apiKey, false)); log("Verify key deletion via UI with disabled api key generation works."); grid = deleteAPIKeyViaUI(); - assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount-2, grid.getRecordCount()); + assertEquals("Number of keys after UI deletion not as expected", beforeDeleteCount - 2, grid.getRecordCount()); // skip testing api key expiration since it's already covered in unit test and 10 seconds expiration option is dev mode only log("Verify revoked/deleted api key"); deleteAPIKeys(_generatedApiKeys); - verifyInvalidAPIKey(apiKey); + verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false); + } + + /* + Regression coverage for Secure Issue 51637: Invalidate sessions when their API key becomes invalid + */ + @Test + public void testSessionInvalidatesAfterAPIKeyChange() throws IOException + { + List> _generatedApiKeys = new ArrayList<>(); + + log("Generating an apikey which expire in one week"); + goToAdminConsole() + .clickSiteSettings() + .setAllowApiKeys(true) + .setApiKeyExpiration(CustomizeSitePage.KeyExpirationOptions.ONE_WEEK) + .save(); + + String apiKey1 = generateAPIKeyAndRecord(_generatedApiKeys); + Connection cn = createApiKeyConnection(apiKey1, false); + verifyValidAPIKey(cn); + + log("Deleting the apikey"); + deleteAPIKeys(_generatedApiKeys); + + /* + Regression coverage for Issue 52004: Session associated with APIKey can used even after APIKey is deleted. + */ + log("Verifying the session associated with deleted apikey is invalid"); + verifyInvalidAPIKey(cn, false); + + log("Verifying that new connection cannot be created after apikey is deleted"); + verifyInvalidAPIKey(createApiKeyConnection(apiKey1, false), false); + + log("Generating the apikey which expires in ten seconds"); + goToAdminConsole() + .clickSiteSettings() + .setAllowApiKeys(true) + .setApiKeyExpiration(CustomizeSitePage.KeyExpirationOptions.TEN_SECONDS) + .save(); + + log("Verify apikey expiration"); + goToExternalToolPage(); + String apikey2 = ApiKeyPanel.panelFinder(getDriver()).find().generateApiKey(); + + log("Verify apikey can be used before expiring"); + verifyValidAPIKey(createApiKeyConnection(apikey2, false)); + + sleep(10000); // Wait for apikey to expire + + log("Verify apikey cannot be used after it has expired"); + verifyInvalidAPIKey(createApiKeyConnection(apikey2, false), false); } @Test @@ -335,6 +343,47 @@ public void testSessionKeyDisabled() throws IOException } } + private void verifyValidAPIKey(Connection connection) throws IOException + { + try + { + GetSchemasCommand cmd = new GetSchemasCommand(); + GetSchemasResponse resp = cmd.execute(connection, getProjectName()); + List schemaNames = resp.getSchemaNames().stream().map(String::toLowerCase).collect(Collectors.toList()); + Set missingSchemas = new HashSet<>(Arrays.asList("pipeline", "lists", "core")); + missingSchemas.removeAll(schemaNames); + assertTrue("Some expected schemas missing. Schemas missing: " + missingSchemas, missingSchemas.isEmpty()); + } + catch (CommandException e) + { + throw new RuntimeException("Response: " + e.getStatusCode(), e); + } + } + + private Connection createApiKeyConnection(String apiKey, boolean basicAuth) + { + return new Connection(WebTestHelper.getBaseURL(), basicAuth ? new BasicAuthCredentialsProvider(API_USERNAME, apiKey) + : new ApiKeyCredentialsProvider(apiKey)); + } + + private void verifyInvalidAPIKey(Connection connection, boolean isSessionKey) throws IOException + { + try + { + GetSchemasCommand cmd = new GetSchemasCommand(); + cmd.execute(connection, getProjectName()); + if (isSessionKey) + fail("Session key didn't invalidate after logout"); + else + fail("API key should no longer be valid"); + } + catch (CommandException e) + { + assertEquals("Wrong response for invalid " + (isSessionKey ? "session" : "API") + " key", HttpStatus.SC_UNAUTHORIZED, e.getStatusCode()); + log("Success: command failed as expected."); + } + } + private void verifyAPIKeysTablePresence(boolean isAdmin) { beginAt(new URLBuilder("query", "begin", getProjectName()).setFragment("sbh-ssp-core").buildURL()); @@ -413,7 +462,7 @@ protected Map getLastAPIKeyRecord() throws IOException String keyField = "RowId"; Map record = response.getRows().get(0); Map newRow = new HashMap<>(); - Integer rowId = (Integer)((Map)record.get(keyField)).get("value"); + Integer rowId = (Integer) ((Map) record.get(keyField)).get("value"); newRow.put(keyField, rowId); return newRow; diff --git a/src/org/labkey/test/tests/InvalidateSessionTest.java b/src/org/labkey/test/tests/InvalidateSessionTest.java new file mode 100644 index 0000000000..a172f95f43 --- /dev/null +++ b/src/org/labkey/test/tests/InvalidateSessionTest.java @@ -0,0 +1,148 @@ +package org.labkey.test.tests; + +import org.apache.hc.core5.http.HttpStatus; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.labkey.remoteapi.CommandException; +import org.labkey.remoteapi.Connection; +import org.labkey.remoteapi.query.SelectRowsCommand; +import org.labkey.remoteapi.query.SelectRowsResponse; +import org.labkey.test.BaseWebDriverTest; +import org.labkey.test.categories.Daily; +import org.labkey.test.util.ApiPermissionsHelper; +import org.labkey.test.util.PasswordUtil; +import org.labkey.test.util.PermissionsHelper; +import org.openqa.selenium.Cookie; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + +import static org.junit.Assert.fail; + +@Category({Daily.class}) +public class InvalidateSessionTest extends BaseWebDriverTest +{ + private static final String USER = "pwd_change_user@invalidatesessiontest.test"; + + @BeforeClass + public static void setupProject() + { + InvalidateSessionTest init = getCurrentTest(); + init.doSetup(); + } + + @Override + protected @Nullable String getProjectName() + { + return null; + } + + @Override + protected void doCleanup(boolean afterTest) + { + _userHelper.deleteUsers(afterTest, USER); + } + + private void doSetup() + { + _userHelper.createUser(USER); + + ApiPermissionsHelper permissionsHelper = new ApiPermissionsHelper(this); + permissionsHelper.addMemberToRole(USER, "Folder Administrator", PermissionsHelper.MemberType.user); + setInitialPassword(USER); + } + + /* + Regression coverage for Secure Issue 51523: Invalidate sessions on password change + */ + @Test + public void testSessionInvalidatesAfterPasswordChange() throws IOException, CommandException + { + signOut(); + signIn(USER); + Connection cn = createDefaultConnection(); + SelectRowsResponse response; + SelectRowsCommand selectCmd = new SelectRowsCommand("auditLog", "UserAuditEvent"); + response = selectCmd.execute(cn, "Home"); + Assert.assertEquals("Did not establish the database connection before the password change", 200, + response.getStatusCode()); + + log("Changing the user password"); + String newPassword = PasswordUtil.getPassword() + "&*&*"; + goToMyAccount().clickChangePassword() + .setOldPassword(PasswordUtil.getPassword()) + .setPassword1(newPassword) + .setPassword2(newPassword) + .clickSubmit(); + + log("Verifying the session is invalid"); + try + { + selectCmd.execute(cn, getProjectName()); + fail("Test should have thrown command exception"); + } + catch (CommandException e) + { + Assert.assertEquals("Session should be expired", 401, e.getStatusCode()); + } + signOut(); + } + + /* + Regression coverage for Secure Issue 31493: Test for session and cookie persistence through login and logout + */ + @Test + public void testCookieAndSessionFromLogout() throws IOException, CommandException + { + log("Capture the cookie after login"); + Set beforeCookie = getDriver().manage().getCookies(); + + log("Establish the connection"); + Connection cn = createDefaultConnection(); + SelectRowsResponse response; + SelectRowsCommand selectCmd = new SelectRowsCommand("auditLog", "UserAuditEvent"); + response = selectCmd.execute(cn, getProjectName()); + Assert.assertEquals("Did not establish the database connection before the password change", 200, + response.getStatusCode()); + + log("Sign out"); + signOut(); + + log("Verify previously established connection be used after log out"); + try + { + selectCmd.execute(cn, getProjectName()); + fail("Should fail when logged out"); + } + catch (CommandException e) + { + Assert.assertEquals("Command execution should fail when logged out", HttpStatus.SC_UNAUTHORIZED, e.getStatusCode()); + } + + log("Capture the cookie after logout"); + Set afterCookie = getDriver().manage().getCookies(); + Assert.assertFalse("Before and after log out cookie should be different", getJSessionIdValue(beforeCookie).equals(getJSessionIdValue(afterCookie))); + } + + private String getJSessionIdValue(Set cookies) + { + String jsession = ""; + for (Cookie c : cookies) + { + if(c.getName().equals(Connection.JSESSIONID)) + jsession = c.getValue(); + } + return jsession; + } + + @Override + public List getAssociatedModules() + { + return Arrays.asList(); + } +} diff --git a/src/org/labkey/test/tests/UserPermissionsTest.java b/src/org/labkey/test/tests/UserPermissionsTest.java index ce52536fc4..3884a8b083 100644 --- a/src/org/labkey/test/tests/UserPermissionsTest.java +++ b/src/org/labkey/test/tests/UserPermissionsTest.java @@ -16,12 +16,17 @@ package org.labkey.test.tests; +import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.TestTimeoutException; import org.labkey.test.categories.Daily; +import org.labkey.test.pages.core.admin.ShowAuditLogPage; +import org.labkey.test.util.ApiPermissionsHelper; +import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.LogMethod; import org.labkey.test.util.PortalHelper; import org.openqa.selenium.WebElement; @@ -36,14 +41,13 @@ @BaseWebDriverTest.ClassTimeout(minutes = 7) public class UserPermissionsTest extends BaseWebDriverTest { - PortalHelper portalHelper = new PortalHelper(this); protected static final String PERM_PROJECT_NAME = "PermissionCheckProject"; protected static final String DENIED_SUB_FOLDER_NAME = "UnlinkedFolder"; protected static final String GAMMA_SUB_FOLDER_NAME = "GammaFolder"; protected static final String GAMMA_EDITOR_GROUP_NAME = "GammaEditor"; protected static final String GAMMA_AUTHOR_GROUP_NAME = "GammaAuthor"; protected static final String GAMMA_READER_GROUP_NAME = "GammaReader"; -// protected static final String GAMMA_RESTRICTED_READER_GROUP_NAME = "GammaRestrictedReader"; + // protected static final String GAMMA_RESTRICTED_READER_GROUP_NAME = "GammaRestrictedReader"; protected static final String GAMMA_SUBMITTER_GROUP_NAME = "GammaSubmitter"; protected static final String GAMMA_ADMIN_GROUP_NAME = "GammaAdmin"; //permissions @@ -54,11 +58,20 @@ public class UserPermissionsTest extends BaseWebDriverTest protected static final String GAMMA_AUTHOR_PAGE_TITLE = "This is a Test Message from : " + GAMMA_AUTHOR_USER; protected static final String GAMMA_READER_USER = "gammareader@security.test"; protected static final String GAMMA_PROJECT_ADMIN_USER = "gammaadmin@security.test"; + protected static final String GAMMA_SUBMITTER_USER = "gammasubmitter@security.test"; + PortalHelper portalHelper = new PortalHelper(this); //I can't really find any docs on what this is exactly? // protected static final String GAMMA_RESTRICTED_READER_USER = "gammarestricted@security.test"; // protected static final String GAMMA_SUBMITTER_USER = "gammasubmitter@security.test"; + @BeforeClass + public static void setupProject() + { + UserPermissionsTest init = getCurrentTest(); + init.doSetup(); + } + @Override public List getAssociatedModules() { @@ -83,23 +96,10 @@ protected void doCleanup(boolean afterTest) throws TestTimeoutException log(this.getClass().getName() + " Cleaning Up"); _containerHelper.deleteProject(PERM_PROJECT_NAME, afterTest); - deleteUsersIfPresent(GAMMA_EDITOR_USER, GAMMA_AUTHOR_USER, GAMMA_READER_USER, GAMMA_PROJECT_ADMIN_USER); - } - - @Test - public void testSteps() - { - enableEmailRecorder(); - userPermissionRightsTest(); + deleteUsersIfPresent(GAMMA_EDITOR_USER, GAMMA_AUTHOR_USER, GAMMA_READER_USER, GAMMA_PROJECT_ADMIN_USER, GAMMA_SUBMITTER_USER); } - /** - * Create some projects, create some groups, permissions for those groups - * Create some users, assign to groups and validate the permissions by - * impersonating the user. - */ - @LogMethod - private void userPermissionRightsTest() + private void doSetup() { _containerHelper.createProject(PERM_PROJECT_NAME, null); _permissionsHelper.createPermissionsGroup(GAMMA_EDITOR_GROUP_NAME); @@ -107,8 +107,8 @@ private void userPermissionRightsTest() _permissionsHelper.setPermissions(GAMMA_EDITOR_GROUP_NAME, "Editor"); createUserInProjectForGroup(GAMMA_EDITOR_USER, PERM_PROJECT_NAME, GAMMA_EDITOR_GROUP_NAME, false); - _containerHelper.createSubfolder(PERM_PROJECT_NAME, PERM_PROJECT_NAME, DENIED_SUB_FOLDER_NAME, "None", new String[] {"Messages", "Wiki"}, true); - _containerHelper.createSubfolder(PERM_PROJECT_NAME, DENIED_SUB_FOLDER_NAME, GAMMA_SUB_FOLDER_NAME, "None", new String[] {"Messages", "Wiki"}, true); + _containerHelper.createSubfolder(PERM_PROJECT_NAME, PERM_PROJECT_NAME, DENIED_SUB_FOLDER_NAME, "None", new String[]{"Messages", "Wiki"}, true); + _containerHelper.createSubfolder(PERM_PROJECT_NAME, DENIED_SUB_FOLDER_NAME, GAMMA_SUB_FOLDER_NAME, "None", new String[]{"Messages", "Wiki"}, true); portalHelper.addWebPart("Messages"); assertElementPresent(Locator.linkWithText("Messages")); portalHelper.addWebPart("Wiki"); @@ -123,6 +123,7 @@ private void userPermissionRightsTest() _permissionsHelper.assertPermissionSetting(GAMMA_READER_GROUP_NAME, "No Permissions"); _permissionsHelper.setPermissions(GAMMA_READER_GROUP_NAME, "Reader"); createUserInProjectForGroup(GAMMA_READER_USER, PERM_PROJECT_NAME, GAMMA_READER_GROUP_NAME, false); + //Create Author User clickProject(PERM_PROJECT_NAME); _permissionsHelper.enterPermissionsUI(); @@ -130,17 +131,28 @@ private void userPermissionRightsTest() _permissionsHelper.assertPermissionSetting(GAMMA_AUTHOR_GROUP_NAME, "No Permissions"); _permissionsHelper.setPermissions(GAMMA_AUTHOR_GROUP_NAME, "Author"); createUserInProjectForGroup(GAMMA_AUTHOR_USER, PERM_PROJECT_NAME, GAMMA_AUTHOR_GROUP_NAME, false); + //Create the Submitter User clickProject(PERM_PROJECT_NAME); _permissionsHelper.enterPermissionsUI(); _permissionsHelper.createPermissionsGroup(GAMMA_SUBMITTER_GROUP_NAME); _permissionsHelper.assertPermissionSetting(GAMMA_SUBMITTER_GROUP_NAME, "No Permissions"); _permissionsHelper.setPermissions(GAMMA_SUBMITTER_GROUP_NAME, "Submitter"); + // TODO: Add submitter to a group /* * I need a way to test submitter, I can't even view a folder where submitter has permissions when * impersonating on my local labkey, so may require special page? */ + } + + /* + Validate the permissions by impersonating the user. + */ + @Test + public void testUserPermissionRights() + { + enableEmailRecorder(); //Make sure the Editor can edit impersonate(GAMMA_EDITOR_USER); @@ -236,6 +248,33 @@ private void userPermissionRightsTest() signIn(); } + /* + Regression for Secure Issue 51187: Additional automation testing for group audit logs + */ + @Test + public void testAuditLogForGroupUpdates() + { + ApiPermissionsHelper permissionsHelper = new ApiPermissionsHelper(this); + + log("Add user to the group and verify logs"); + _userHelper.createUser(GAMMA_SUBMITTER_USER); + permissionsHelper.addUserToProjGroup(GAMMA_SUBMITTER_USER, getProjectName(), GAMMA_SUBMITTER_GROUP_NAME); + verifyAuditLog("User: " + GAMMA_SUBMITTER_USER + " was added as a member to Group: " + GAMMA_SUBMITTER_GROUP_NAME); + + log("Remove user from group and verify logs"); + goToProjectHome(); + permissionsHelper.removeUserFromGroup(GAMMA_SUBMITTER_GROUP_NAME, GAMMA_SUBMITTER_USER); + verifyAuditLog("User: " + GAMMA_SUBMITTER_USER + " was deleted from Group: " + GAMMA_SUBMITTER_GROUP_NAME); + } + + private void verifyAuditLog(String expectedComment) + { + ShowAuditLogPage showAuditLogPage = goToAdminConsole().clickAuditLog(); + showAuditLogPage.selectView("Group and role events"); + DataRegionTable table = showAuditLogPage.getLogTable(); + Assert.assertEquals("Incorrect audit log record for group events", expectedComment, table.getDataAsText(0, "Comment")); + } + private void clickLinkWithTextNoTarget(String text) { String href = getAttribute(Locator.linkWithText(text), "href"); diff --git a/src/org/labkey/test/tests/core/login/PasswordTest.java b/src/org/labkey/test/tests/core/login/PasswordTest.java index b3c55d2362..f72d2e7149 100644 --- a/src/org/labkey/test/tests/core/login/PasswordTest.java +++ b/src/org/labkey/test/tests/core/login/PasswordTest.java @@ -138,7 +138,7 @@ public void testStrongPassword() assertSignedInNotImpersonating(); impersonate(USER); - SetPasswordForm changePasswordForm = goToChangePassword(); + SetPasswordForm changePasswordForm = goToMyAccount().clickChangePassword(); log("Verify strength gauge for 'ChangePasswordAction'"); changePasswordForm.verifyPasswordStrengthGauge(USER, displayName); @@ -183,7 +183,7 @@ public void testReusePassword() assertTextNotPresent("Choose a new password."); } // fail, used 9 passwords ago. - goToChangePassword() + goToMyAccount().clickChangePassword() .setOldPassword(currentPassword) .setNewPassword(VERY_STRONG_PASSWORD + 1) .clickSubmitExpectingError("Your password must not match a recently used password."); @@ -357,19 +357,9 @@ protected String setInitialPassword(int userId, String password) @LogMethod (quiet = true) protected void changePassword(String oldPassword, @LoggedParam String password) { - goToChangePassword() + goToMyAccount().clickChangePassword() .setOldPassword(oldPassword) .setNewPassword(password) .clickSubmit(); } - - private SetPasswordForm goToChangePassword() - { - if (PasswordUtil.getUsername().equals(getCurrentUser())) - throw new IllegalArgumentException("Don't change the primary site admin user's password"); - - goToMyAccount(); - clickButton("Change Password"); - return new SetPasswordForm(getDriver()); - } } From 0bb5eef030836326f77547d09d894c12abc5125e Mon Sep 17 00:00:00 2001 From: Susan Hert Date: Mon, 27 Jan 2025 15:27:09 -0800 Subject: [PATCH 2/2] Issue 52038: Fix problems fields whose names and fieldKeys are different (#2243) --- src/org/labkey/test/BaseWebDriverTest.java | 5 ++ src/org/labkey/test/TestFileUtils.java | 50 +++++++++++++++++++ .../components/ui/grids/DetailTableEdit.java | 18 +++---- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/org/labkey/test/BaseWebDriverTest.java b/src/org/labkey/test/BaseWebDriverTest.java index 807e007edf..dfd321a9b2 100644 --- a/src/org/labkey/test/BaseWebDriverTest.java +++ b/src/org/labkey/test/BaseWebDriverTest.java @@ -215,6 +215,11 @@ public abstract class BaseWebDriverTest extends LabKeySiteWrapper implements Cle public static final double DELTA = 10E-10; + // See QueryKey.ILLEGAL TODO make that array public so it can be used here + public static final String[] ILLEGAL_QUERY_KEY_CHARACTERS = {"$", "/", "&", "}", "~", ",", "."}; + // See TSVWriter.shouldQuote. Generally we are not able to use the tab and new line characters when creating field names in the UI, but including here for completeness + public static final String[] TRICKY_IMPORT_FIELD_CHARACTERS = {"\\", "\"", "\\t", ",", "\\n", "\\r"}; + public static final String TRICKY_CHARACTERS = "><&/%\\' \"1\u00E4\u00F6\u00FC\u00C5"; public static final String TRICKY_CHARACTERS_NO_QUOTES = ">