Skip to content

Commit

Permalink
Code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@labkey-sweta
labkey-sweta committed Jan 17, 2025
1 parent 223664b commit 14d72e5
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 112 deletions.
12 changes: 12 additions & 0 deletions src/org/labkey/test/pages/user/UserDetailsPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()
{
Expand All @@ -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);
}
}
133 changes: 61 additions & 72 deletions src/org/labkey/test/tests/ApiKeyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class ApiKeyTest extends BaseWebDriverTest
@BeforeClass
public static void setupProject()
{
ApiKeyTest init = (ApiKeyTest) getCurrentTest();
ApiKeyTest init = getCurrentTest();

init.doSetup();
}
Expand Down Expand Up @@ -107,74 +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, null);
verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false);
simpleSignIn();
log("Verify that session keys remain invalid after logging back in");
verifyInvalidAPIKey(apiKey, null);
}

private void verifyValidAPIKey(String apiKey) throws IOException
{
verifyValidAPIKey(apiKey, false, null);
}

private Connection verifyValidAPIKey(String apiKey, boolean basicAuth, @Nullable Connection connection) throws IOException
{
Connection cn;
if (connection == null)
cn = new Connection(WebTestHelper.getBaseURL(), basicAuth ? new BasicAuthCredentialsProvider(API_USERNAME, apiKey)
: new ApiKeyCredentialsProvider(apiKey));
else
cn = connection;
try
{
GetSchemasCommand cmd = new GetSchemasCommand();
GetSchemasResponse resp = cmd.execute(cn, getProjectName());
List<String> schemaNames = resp.getSchemaNames().stream().map(String::toLowerCase).collect(Collectors.toList());
Set<String> 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);
}
return cn;
}

private void verifyInvalidAPIKey(String apiKey, @Nullable Connection connection) throws IOException
{
boolean isSessionKey = !apiKey.startsWith(API_USERNAME);
Connection cn;
if (connection == null)
cn = new Connection(WebTestHelper.getBaseURL(), new ApiKeyCredentialsProvider(apiKey));
else
cn = connection;
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
Expand All @@ -192,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, null);
verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false);
}

@Test
Expand All @@ -215,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, null);
verifyValidAPIKey(createApiKeyConnection(apiKey, true));

log("Generate two other keys for use in testing deletion.");
generateAPIKey(null);
Expand All @@ -232,7 +179,7 @@ public void testStandardApiKey() throws IOException
.clickSiteSettings()
.setAllowApiKeys(false)
.save();
verifyValidAPIKey(apiKey);
verifyValidAPIKey(createApiKeyConnection(apiKey, false));

log("Verify key deletion via UI with disabled api key generation works.");
grid = deleteAPIKeyViaUI();
Expand All @@ -242,7 +189,7 @@ public void testStandardApiKey() throws IOException

log("Verify revoked/deleted api key");
deleteAPIKeys(_generatedApiKeys);
verifyInvalidAPIKey(apiKey, null);
verifyInvalidAPIKey(createApiKeyConnection(apiKey, false), false);
}

/*
Expand All @@ -261,16 +208,17 @@ public void testSessionInvalidatesAfterAPIKeyChange() throws IOException
.save();

String apiKey1 = generateAPIKeyAndRecord(_generatedApiKeys);
Connection cn = verifyValidAPIKey(apiKey1, false, null);
Connection cn = createApiKeyConnection(apiKey1, false);
verifyValidAPIKey(cn);

log("Deleting the apikey");
deleteAPIKeys(_generatedApiKeys);

log("Verifying the session associated with deleted apikey is invalid");
verifyInvalidAPIKey(apiKey1, cn);
// verifyInvalidAPIKey(cn, false);

log("Verifying that new connection cannot be created after apikey is deleted");
verifyInvalidAPIKey(apiKey1, null);
verifyInvalidAPIKey(createApiKeyConnection(apiKey1, false), false);

log("Generating the apikey which expires in ten seconds");
goToAdminConsole()
Expand All @@ -279,17 +227,17 @@ public void testSessionInvalidatesAfterAPIKeyChange() throws IOException
.setApiKeyExpiration(CustomizeSitePage.KeyExpirationOptions.TEN_SECONDS)
.save();

log("Verify apikey generation fails");
log("Verify apikey expiration");
goToExternalToolPage();
String apikey2 = ApiKeyPanel.panelFinder(getDriver()).find().generateApiKey();

log("Verify valid apikey cannot be created before expiring");
verifyValidAPIKey(apikey2, false, null);
log("Verify apikey can be used before expiring");
verifyValidAPIKey(createApiKeyConnection(apikey2, false));

sleep(10000); // Wait for apikey to expire

log("Verify apikey cannot be created after the timer is expired");
verifyInvalidAPIKey(apikey2, null);
log("Verify apikey cannot be used after it has expired");
verifyInvalidAPIKey(createApiKeyConnection(apikey2, false), false);
}

@Test
Expand Down Expand Up @@ -392,6 +340,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<String> schemaNames = resp.getSchemaNames().stream().map(String::toLowerCase).collect(Collectors.toList());
Set<String> 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());
Expand Down
41 changes: 14 additions & 27 deletions src/org/labkey/test/tests/InvalidateSessionTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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;
Expand All @@ -11,7 +12,6 @@
import org.labkey.remoteapi.query.SelectRowsResponse;
import org.labkey.test.BaseWebDriverTest;
import org.labkey.test.categories.Daily;
import org.labkey.test.components.core.login.SetPasswordForm;
import org.labkey.test.util.ApiPermissionsHelper;
import org.labkey.test.util.PasswordUtil;
import org.labkey.test.util.PermissionsHelper;
Expand All @@ -20,9 +20,9 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Set;

import static org.junit.Assert.fail;
import static org.labkey.test.WebTestHelper.getCookies;

@Category({Daily.class})
public class InvalidateSessionTest extends BaseWebDriverTest
Expand All @@ -36,20 +36,24 @@ public static void setupProject()
init.doSetup();
}

@Override
protected @Nullable String getProjectName()
{
return null;
}

@Override
protected void doCleanup(boolean afterTest)
{
_containerHelper.deleteProject(getProjectName(), afterTest);
_userHelper.deleteUsers(afterTest, USER);
}

private void doSetup()
{
_containerHelper.createProject(getProjectName(), null);
_userHelper.createUser(USER);

ApiPermissionsHelper permissionsHelper = new ApiPermissionsHelper(this);
permissionsHelper.addMemberToRole(USER, "Editor", PermissionsHelper.MemberType.user);
permissionsHelper.addMemberToRole(USER, "Folder Administrator", PermissionsHelper.MemberType.user);
setInitialPassword(USER);
}

Expand All @@ -66,7 +70,7 @@ public void testSessionInvalidatesAfterPasswordChange() throws IOException
SelectRowsCommand selectCmd = new SelectRowsCommand("auditLog", "UserAuditEvent");
try
{
response = selectCmd.execute(cn, getProjectName());
response = selectCmd.execute(cn, "Home");
Assert.assertEquals("Did not establish the database connection before the password change", 200,
response.getStatusCode());
}
Expand All @@ -77,7 +81,8 @@ public void testSessionInvalidatesAfterPasswordChange() throws IOException

log("Changing the user password");
String newPassword = PasswordUtil.getPassword() + "&*&*";
goToChangePassword().setOldPassword(PasswordUtil.getPassword())
goToMyAccount().clickChangePassword()
.setOldPassword(PasswordUtil.getPassword())
.setPassword1(newPassword)
.setPassword2(newPassword)
.clickSubmit();
Expand All @@ -101,10 +106,8 @@ public void testSessionInvalidatesAfterPasswordChange() throws IOException
@Test
public void testCookieAndSessionFromLogout() throws IOException
{
goToProjectHome();

log("Capture the cookie after login");
Set<Cookie> beforeCookie = getDriver().manage().getCookies();
Cookie beforeCookie = getCookies(getCurrentUser()).get(Connection.JSESSIONID);

log("Establish the connection");
Connection cn = createDefaultConnection();
Expand Down Expand Up @@ -136,26 +139,10 @@ public void testCookieAndSessionFromLogout() throws IOException
}

log("Capture the cookie after logout");
Set<Cookie> afterCookie = getDriver().manage().getCookies();
Cookie afterCookie = getCookies(getCurrentUser()).get(Connection.JSESSIONID);
Assert.assertFalse("Before and after log out cookie should be different", beforeCookie.equals(afterCookie));
}

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());
}

@Override
protected String getProjectName()
{
return "InvalidateSessionTest Project";
}

@Override
public List<String> getAssociatedModules()
{
Expand Down
Loading

0 comments on commit 14d72e5

Please sign in to comment.