diff --git a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java index edf3a0f758..4f16a0ceef 100644 --- a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java +++ b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java @@ -109,6 +109,11 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { } private boolean tryRedirectBroker(AuthenticationFlowContext context, OrganizationModel organization, UserModel user, String username, String domain) { + // the user has credentials set; do not redirect to allow the user to pick how to authenticate + if (user != null && user.credentialManager().getStoredCredentialsStream().findAny().isPresent()) { + return false; + } + List broker = resolveHomeBroker(session, user); if (broker.size() == 1) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java index fde7f3b188..b3af5428f9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java @@ -266,26 +266,26 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest { }; } - protected void openIdentityFirstLoginPage(String username, boolean autoIDPRedirect, IdentityProviderRepresentation idp, boolean isVisible, boolean clickIdp) { + protected void openIdentityFirstLoginPage(String username, boolean autoIDPRedirect, String idpAlias, boolean isVisible, boolean clickIdp) { oauth.clientId("broker-app"); loginPage.open(bc.consumerRealmName()); log.debug("Logging in"); Assert.assertFalse(loginPage.isPasswordInputPresent()); Assert.assertFalse(loginPage.isSocialButtonPresent(bc.getIDPAlias())); Assert.assertTrue(loginPage.isRegisterLinkPresent()); - if (idp != null) { + if (idpAlias != null) { if (isVisible) { - Assert.assertTrue(loginPage.isSocialButtonPresent(idp.getAlias())); + Assert.assertTrue(loginPage.isSocialButtonPresent(idpAlias)); } else { - Assert.assertFalse(loginPage.isSocialButtonPresent(idp.getAlias())); + Assert.assertFalse(loginPage.isSocialButtonPresent(idpAlias)); } } loginPage.loginUsername(username); if (clickIdp) { assertTrue(loginPage.isPasswordInputPresent()); - assertTrue(loginPage.isSocialButtonPresent(idp.getAlias())); - loginPage.clickSocial(idp.getAlias()); + assertTrue(loginPage.isSocialButtonPresent(idpAlias)); + loginPage.clickSocial(idpAlias); } waitForPage(driver, "sign in to", true); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java index e3683aaa79..7473f9b61d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.organization.broker; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; @@ -29,6 +30,7 @@ import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; +import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.Test; import org.keycloak.admin.client.resource.OrganizationIdentityProviderResource; @@ -49,6 +51,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.organization.admin.AbstractOrganizationTest; +import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.util.UserBuilder; public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganizationTest { @@ -158,7 +161,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz organization.identityProviders().addIdentityProvider(idp.getAlias()).close(); idp = organization.identityProviders().get(idp.getAlias()).toRepresentation(); - openIdentityFirstLoginPage("external@user.org", false, idp, false, false); + openIdentityFirstLoginPage("external@user.org", false, idp.getAlias(), false, false); Assert.assertTrue(loginPage.isPasswordInputPresent()); Assert.assertFalse(loginPage.isSocialButtonPresent(bc.getIDPAlias())); @@ -225,7 +228,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz brokerRep.getConfig().remove(IdentityProviderRedirectMode.EMAIL_MATCH.getKey()); testRealm().identityProviders().get(brokerRep.getAlias()).update(brokerRep); - openIdentityFirstLoginPage(bc.getUserEmail(), true, brokerRep, false, true); + openIdentityFirstLoginPage(bc.getUserEmail(), true, brokerRep.getAlias(), false, true); loginOrgIdp(bc.getUserEmail(), bc.getUserEmail(),true, false); @@ -249,7 +252,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz brokerRep.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString()); testRealm().identityProviders().get(brokerRep.getAlias()).update(brokerRep); - openIdentityFirstLoginPage(bc.getUserEmail(), true, brokerRep, false, true); + openIdentityFirstLoginPage(bc.getUserEmail(), true, brokerRep.getAlias(), false, true); // login to the organization identity provider and run the configured first broker login flow loginOrgIdp(bc.getUserEmail(), bc.getUserEmail(), true, false); @@ -343,6 +346,47 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz Assert.assertFalse(loginPage.isSocialButtonPresent(secondIdp.getAlias())); } + @Test + public void testNoIDPRedirectWhenUserHasCredentialsSet() { + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + IdentityProviderRepresentation idpRep = organization.identityProviders().getIdentityProviders().get(0); + idpRep.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString()); + testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep); + + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); + + // set the user's credentials + UserRepresentation user = testRealm().users().searchByEmail(bc.getUserEmail(), true).get(0); + ApiUtil.resetUserPassword(realmsResouce().realm(bc.consumerRealmName()).users().get(user.getId()), "updated-password", false); + + // logout to force the user to authenticate again + UserRepresentation account = getUserRepresentation(bc.getUserEmail()); + realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); + realmsResouce().realm(bc.providerRealmName()).logoutAll(); + + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + loginPage.loginUsername(bc.getUserEmail()); + + // after providing username/email, user can pick to authenticate by password or by org IDP + Assert.assertFalse(loginPage.isUsernameInputPresent()); + Assert.assertTrue(loginPage.isPasswordInputPresent()); + Assert.assertTrue(loginPage.isSocialButtonPresent(bc.getIDPAlias())); + + // log in by password + loginPage.login("updated-password"); + appPage.assertCurrent(); + MatcherAssert.assertThat(appPage.getRequestType(), is(AppPage.RequestType.AUTH_RESPONSE)); + + // logout again + realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); + realmsResouce().realm(bc.providerRealmName()).logoutAll(); + + // log in via IDP + openIdentityFirstLoginPage(bc.getUserEmail(), true, bc.getIDPAlias(), false, true); + loginOrgIdp(bc.getUserLogin(), bc.getUserEmail(), false, true); + } + @Test public void testFailUpdateEmailNotAssociatedOrganizationUsingAdminAPI() { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); @@ -408,7 +452,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove); organization.identityProviders().addIdentityProvider(idp.getAlias()).close(); - openIdentityFirstLoginPage(bc.getUserEmail(), true, idp, false, false); + openIdentityFirstLoginPage(bc.getUserEmail(), true, idp.getAlias(), false, false); loginOrgIdp(bc.getUserEmail(), bc.getUserEmail(),true, true); @@ -428,7 +472,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz idp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString()); testRealm().identityProviders().get(bc.getIDPAlias()).update(idp); - openIdentityFirstLoginPage(bc.getUserEmail(), false, idp, false, false); + openIdentityFirstLoginPage(bc.getUserEmail(), false, idp.getAlias(), false, false); Assert.assertFalse(loginPage.isPasswordInputPresent()); Assert.assertTrue(driver.getPageSource().contains("Your email domain matches the " + organizationName + " organization but you don't have an account yet.")); @@ -484,7 +528,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz organization.identityProviders().addIdentityProvider(idp.getAlias()).close(); String email = "external@user.org"; - openIdentityFirstLoginPage(email, true, idp, false, true); + openIdentityFirstLoginPage(email, true, idp.getAlias(), false, true); loginOrgIdp("external", email, true, true); @@ -520,7 +564,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz organization.identityProviders().addIdentityProvider(idp.getAlias()).close(); String email = "external@user.org"; - openIdentityFirstLoginPage(email, true, idp, false, true); + openIdentityFirstLoginPage(email, true, idp.getAlias(), false, true); loginOrgIdp(email, email, true, false); @@ -553,7 +597,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz organization.identityProviders().addIdentityProvider(idp.getAlias()).close(); String email = "external@user.org"; - openIdentityFirstLoginPage(email, true, idp, false, true); + openIdentityFirstLoginPage(email, true, idp.getAlias(), false, true); loginOrgIdp(email, "external@unknown.org", true, true); assertIsMember("external@unknown.org", organization); @@ -568,7 +612,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz // create a second broker without a domain set testRealm().identityProviders().create(idp).close(); - openIdentityFirstLoginPage("some@user.org", true, idp, true, true); + openIdentityFirstLoginPage("some@user.org", true, idp.getAlias(), true, true); loginOrgIdp("external", bc.getUserEmail(), true, true); @@ -598,7 +642,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz realmsResouce().realm(bc.providerRealmName()).users().create(user).close(); // select the organization broker to authenticate - openIdentityFirstLoginPage("user@different.org", true, idpRep, false, true); + openIdentityFirstLoginPage("user@different.org", true, idpRep.getAlias(), false, true); loginOrgIdp(user.getEmail(), user.getEmail(), true, true); } @@ -622,7 +666,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz .build(); realmsResouce().realm(bc.providerRealmName()).users().create(user).close(); - openIdentityFirstLoginPage(user.getEmail(), true, idpRep, false, true); + openIdentityFirstLoginPage(user.getEmail(), true, idpRep.getAlias(), false, true); loginOrgIdp(user.getEmail(), user.getEmail(),true, true);