Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove AccountPasswordPage from testsuite #15204

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Remove AccountPasswordPage from testsuite #15204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...ration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AbstractPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ protected URI getAuthServerRoot() {

abstract public void open() throws Exception;

public WebDriver getDriver() {
return driver;
}

public void setDriver(WebDriver driver) {
this.driver = driver ;
oauth.setDriver(driver);
Expand Down
81 changes: 0 additions & 81 deletions ...arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AccountPasswordPage.java
View file
Open in desktop

This file was deleted.

2 changes: 2 additions & 0 deletions ...integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AppPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ public void logout(String idTokenHint) {
oauth.idTokenHint(idTokenHint).openLogout();
}



}
16 changes: 4 additions & 12 deletions ...an/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.keycloak.testsuite.ProfileAssume;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.federation.DummyUserFederationProviderFactory;
import org.keycloak.testsuite.util.AccountHelper;
import org.keycloak.testsuite.util.ClientBuilder;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.RealmBuilder;
Expand Down Expand Up @@ -229,9 +230,7 @@ public void loginWithExistingUserWithBruteForceEnabled() {

loginWithExistingUser();

driver.navigate().to(getAccountPasswordUrl(getConsumerRoot(), bc.consumerRealmName()));

accountPasswordPage.changePassword("password", "password");
Assert.assertTrue(AccountHelper.updatePassword(adminClient.realm(bc.consumerRealmName()), bc.getUserLogin(), "password"));

logoutFromRealm(getProviderRoot(), bc.providerRealmName());

Expand Down Expand Up @@ -606,12 +605,7 @@ public void testWithLinkedFederationProvider() {
waitForAccountManagementTitle();
accountUpdateProfilePage.assertCurrent();

accountPage.password();
accountPasswordPage.changePassword("bad", "new-password", "new-password");
assertEquals("Invalid existing password.", accountPasswordPage.getError());

accountPasswordPage.changePassword("secret", "new-password", "new-password");
assertEquals("Your password has been updated.", accountUpdateProfilePage.getSuccess());
Assert.assertTrue(AccountHelper.updatePassword(adminClient.realm(bc.consumerRealmName()), "test-user", "new-password"));

logoutFromRealm(getProviderRoot(), bc.providerRealmName());
logoutFromRealm(getConsumerRoot(), bc.consumerRealmName());
Expand All @@ -623,9 +617,7 @@ public void testWithLinkedFederationProvider() {
waitForAccountManagementTitle();
accountUpdateProfilePage.assertCurrent();

accountPage.password();
accountPasswordPage.changePassword("new-password", "new-password");
assertEquals("Your password has been updated.", accountUpdateProfilePage.getSuccess());
Assert.assertTrue(AccountHelper.updatePassword(adminClient.realm(bc.consumerRealmName()), "test-user-noemail", "new-password"));
} finally {
removeUserByUsername(adminClient.realm(bc.consumerRealmName()), "test-user");
removeUserByUsername(adminClient.realm(bc.consumerRealmName()), "test-user-noemail");
Expand Down
4 changes: 0 additions & 4 deletions ...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
import org.keycloak.testsuite.pages.AccountApplicationsPage;
import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
import org.keycloak.testsuite.pages.AccountPasswordPage;
import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
import org.keycloak.testsuite.pages.ErrorPage;
import org.keycloak.testsuite.pages.IdpConfirmLinkPage;
Expand Down Expand Up @@ -94,9 +93,6 @@ public abstract class AbstractBaseBrokerTest extends AbstractKeycloakTest {
@Page
protected UpdateAccountInformationPage updateAccountInformationPage;

@Page
protected AccountPasswordPage accountPasswordPage;

@Page
protected ErrorPage errorPage;

Expand Down
38 changes: 19 additions & 19 deletions ...test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosSingleRealmTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import javax.ws.rs.core.Response;

import org.ietf.jgss.GSSCredential;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assume;
import org.junit.Test;
import org.keycloak.admin.client.resource.ClientResource;
Expand All @@ -46,6 +47,10 @@
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.util.AccountHelper;
import org.keycloak.testsuite.util.TestAppHelper;

import static org.keycloak.testsuite.admin.ApiUtil.findClientByClientId;

Expand All @@ -57,6 +62,9 @@
@DisableFeature(value = Profile.Feature.ACCOUNT2, skipRestart = true) // TODO remove this (KEYCLOAK-16228)
public abstract class AbstractKerberosSingleRealmTest extends AbstractKerberosTest {

@Page
protected AppPage appPage;

@Test
public void spnegoNotAvailableTest() throws Exception {
initHttpClient(false);
Expand Down Expand Up @@ -131,35 +139,27 @@ public void usernamePasswordLoginTest() throws Exception {
// Change editMode to READ_ONLY
updateProviderEditMode(UserStorageProvider.EditMode.READ_ONLY);

// Login with username/password from kerberos
changePasswordPage.open();
loginPage.assertCurrent();
loginPage.login("jduke", "theduke");
changePasswordPage.assertCurrent();
Comment on lines -134 to -138
Copy link
Contributor

@mabartos mabartos Oct 28, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this test case is included in the abstract test and is used for various additional test classes, IMHO, the login should be verified here via the LoginPage. Also, the test case is named usernamePasswordLoginTest, and one could say if I execute this successfully, I'm really confident the login works as expected.

We're not sure the login with password "theduke" worked before, as it's tested in the following steps.

TestAppHelper testAppHelper = new TestAppHelper(oauth, loginPage, appPage);

// Bad existing password
changePasswordPage.changePassword("theduke-invalid", "newPass", "newPass");
Assert.assertTrue(driver.getPageSource().contains("Invalid existing password."));
Assert.assertTrue(testAppHelper.login("jduke", "theduke"));
Assert.assertTrue(testAppHelper.logout());

// Change password is not possible as editMode is READ_ONLY
changePasswordPage.changePassword("theduke", "newPass", "newPass");
Assert.assertTrue(
driver.getPageSource().contains("You can't update your password as your account is read-only"));
Assert.assertFalse(AccountHelper.updatePassword(testRealmResource(), "jduke", "newPass"));

Assert.assertFalse(testAppHelper.login("jduke", "newPass"));

// Change editMode to UNSYNCED
updateProviderEditMode(UserStorageProvider.EditMode.UNSYNCED);

// Successfully change password now
changePasswordPage.changePassword("theduke", "newPass", "newPass");
Assert.assertTrue(driver.getPageSource().contains("Your password has been updated."));
changePasswordPage.logout();
Assert.assertTrue(AccountHelper.updatePassword(testRealmResource(), "jduke", "newPass"));

// Login with old password doesn't work, but with new password works
loginPage.login("jduke", "theduke");
loginPage.assertCurrent();
loginPage.login("jduke", "newPass");
changePasswordPage.assertCurrent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to verify the login was really successful? In the following steps, there's only used spnegoLogin(), which only sends HTTP requests.

changePasswordPage.logout();
Assert.assertFalse(testAppHelper.login("jduke", "theduke"));
Assert.assertTrue(testAppHelper.login("jduke", "newPass"));

testAppHelper.logout();

// Assert SPNEGO login still with the old password as mode is unsynced
events.clear();
Expand Down
5 changes: 0 additions & 5 deletions ...s/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.ProfileAssume;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.pages.AccountPasswordPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.util.KerberosRule;
import org.keycloak.testsuite.util.OAuthClient;
Expand All @@ -92,9 +91,6 @@ public abstract class AbstractKerberosTest extends AbstractAuthTest {
@Rule
public AssertEvents events = new AssertEvents(this);

@Page
protected AccountPasswordPage changePasswordPage;

protected abstract KerberosRule getKerberosRule();

protected abstract CommonKerberosConfig getKerberosConfig();
Expand Down Expand Up @@ -142,7 +138,6 @@ public void beforeAbstractKeycloakTest() throws Exception {
super.beforeAbstractKeycloakTest();

testRealmPage.setAuthRealm(TEST);
changePasswordPage.realm(TEST);

getKerberosRule().setKrb5ConfPath(testingClient.testing());

Expand Down
43 changes: 14 additions & 29 deletions ...tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosLdapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@
import org.keycloak.storage.ldap.kerberos.LDAPProviderKerberosConfig;
import org.keycloak.testsuite.ProfileAssume;
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
import org.keycloak.testsuite.util.AccountHelper;
import org.keycloak.testsuite.util.KerberosRule;
import org.keycloak.testsuite.KerberosEmbeddedServer;
import org.keycloak.testsuite.util.TestAppHelper;

/**
* Test for the LDAPStorageProvider with kerberos enabled (kerberos with LDAP integration)
Expand Down Expand Up @@ -121,53 +123,39 @@ public void testClientOverrideFlowUsingBrowserHttpChallenge() throws Exception {
public void validatePasswordPolicyTest() throws Exception{
updateProviderEditMode(UserStorageProvider.EditMode.WRITABLE);

changePasswordPage.open();
loginPage.open();
loginPage.login("jduke", "theduke");

updateProviderValidatePasswordPolicy(true);
changePasswordPage.changePassword("theduke", "jduke", "jduke");
Assert.assertTrue(driver.getPageSource().contains("Invalid"));

Assert.assertFalse(AccountHelper.updatePassword(testRealmResource(), "jduke", "jduke"));

updateProviderValidatePasswordPolicy(false);
changePasswordPage.changePassword("theduke", "jduke", "jduke");
Assert.assertTrue(driver.getPageSource().contains("Your password has been updated."));
Assert.assertTrue(AccountHelper.updatePassword(testRealmResource(), "jduke", "jduke"));

// Change password back
changePasswordPage.open();
changePasswordPage.changePassword("jduke", "theduke", "theduke");
Assert.assertTrue(AccountHelper.updatePassword(testRealmResource(), "jduke", "theduke"));
}

@Test
public void writableEditModeTest() throws Exception {
TestAppHelper testAppHelper = new TestAppHelper(oauth, loginPage, appPage);

// Change editMode to WRITABLE
updateProviderEditMode(UserStorageProvider.EditMode.WRITABLE);

// Login with username/password from kerberos
changePasswordPage.open();
// Only needed if you are providing a click thru to bypass kerberos. Currently there is a javascript
// to forward the user if kerberos isn't enabled.
//bypassPage.isCurrent();
//bypassPage.clickContinue();
loginPage.assertCurrent();
loginPage.login("jduke", "theduke");
Assert.assertTrue(changePasswordPage.isCurrent());

// Successfully change password now
changePasswordPage.changePassword("theduke", "newPass", "newPass");
Assert.assertTrue(driver.getPageSource().contains("Your password has been updated."));
changePasswordPage.logout();
Assert.assertTrue(AccountHelper.updatePassword(testRealmResource(), "jduke", "newPass"));

// Only needed if you are providing a click thru to bypass kerberos. Currently there is a javascript
// to forward the user if kerberos isn't enabled.
//bypassPage.isCurrent();
//bypassPage.clickContinue();

// Login with old password doesn't work, but with new password works
loginPage.login("jduke", "theduke");
Assert.assertTrue(loginPage.isCurrent());
loginPage.login("jduke", "newPass");
changePasswordPage.assertCurrent();
changePasswordPage.logout();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above with the assertion.


Assert.assertFalse(testAppHelper.login("jduke", "theduke"));
Assert.assertTrue(testAppHelper.login("jduke", "newPass"));

// Assert SPNEGO login with the new password as mode is writable
events.clear();
Expand All @@ -187,9 +175,6 @@ public void writableEditModeTest() throws Exception {
assertAuthenticationSuccess(codeUrl);

// Change password back
changePasswordPage.open();
loginPage.login("jduke", "newPass");
changePasswordPage.assertCurrent();
changePasswordPage.changePassword("newPass", "theduke", "theduke");
Assert.assertTrue(AccountHelper.updatePassword(testRealmResource(), "jduke", "theduke"));
}
}
4 changes: 0 additions & 4 deletions ...ian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/AbstractLDAPTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.ProfileAssume;
import org.keycloak.testsuite.pages.AccountPasswordPage;
import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.LoginPage;
Expand Down Expand Up @@ -60,9 +59,6 @@ public abstract class AbstractLDAPTest extends AbstractTestRealmKeycloakTest {
@Page
protected RegisterPage registerPage;

@Page
protected AccountPasswordPage changePasswordPage;

@Page
protected AccountUpdateProfilePage profilePage;

Expand Down