diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java index 13ab38c0f0..635db3157a 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java @@ -17,7 +17,7 @@ package org.keycloak.authentication.authenticators.browser; -import java.util.List; +import java.util.stream.Stream; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.forms.login.LoginFormsProvider; @@ -34,9 +34,9 @@ public final class UsernameForm extends UsernamePasswordForm { public void authenticate(AuthenticationFlowContext context) { if (context.getUser() != null) { // We can skip the form when user is re-authenticating. Unless current user has some IDP set, so he can re-authenticate with that IDP - List identityProviders = LoginFormsUtil + Stream identityProviders = LoginFormsUtil .filterIdentityProviders(context.getRealm().getIdentityProvidersStream(), context.getSession(), context); - if (identityProviders.isEmpty()) { + if (identityProviders.findAny().isEmpty()) { context.success(); return; } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index dde858e890..7f38d6654c 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -101,6 +101,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Properties; import java.util.function.Function; +import java.util.stream.Stream; import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PASSWORD; import static org.keycloak.organization.utils.Organizations.resolveOrganization; @@ -479,7 +480,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { if (realm != null) { attributes.put("realm", new RealmBean(realm)); - List identityProviders = LoginFormsUtil + Stream identityProviders = LoginFormsUtil .filterIdentityProvidersForTheme(realm.getIdentityProvidersStream(), session, context); IdentityProviderBean idpBean = new IdentityProviderBean(realm, session, identityProviders, baseUriWithCodeAndClientId); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java b/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java index 26d59ed343..c073464283 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java @@ -21,14 +21,15 @@ import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; +import org.keycloak.common.Profile; +import org.keycloak.common.Profile.Feature; +import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; -import org.keycloak.models.OrganizationModel; import org.keycloak.models.UserModel; import org.keycloak.services.resources.LoginActionsService; import org.keycloak.sessions.AuthenticationSessionModel; -import java.util.List; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -41,12 +42,11 @@ import java.util.stream.Stream; */ public class LoginFormsUtil { - public static List filterIdentityProvidersForTheme(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { + public static Stream filterIdentityProvidersForTheme(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { if (context != null) { AuthenticationSessionModel authSession = context.getAuthenticationSession(); String currentFlowPath = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH); UserModel currentUser = context.getUser(); - Object organization = session.getAttribute(OrganizationModel.class.getName()); // Fixing #14173 // If the current user is not null, then it's a re-auth, and we should filter the possible options with the pre-14173 logic // If the current user is null, then it's one of the following cases: @@ -57,14 +57,14 @@ public class LoginFormsUtil { // - in this case the current user is null AND the current flow is NOT the FIRST_BROKER_LOGIN_PATH // - so we should show all the possible IdPs to the user trying to log in (this is the bug in #14173) // - so we're skipping this branch, and retunring everything at the end of the method - if (organization == null && (currentUser != null || Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath))) { + if (currentUser != null || Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath)) { return filterIdentityProviders(providers, session, context); } } - return providers.collect(Collectors.toList()); + return filterOrganizationBrokers(providers); } - public static List filterIdentityProviders(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { + public static Stream filterIdentityProviders(Stream providers, KeycloakSession session, AuthenticationFlowContext context) { if (context != null) { AuthenticationSessionModel authSession = context.getAuthenticationSession(); @@ -73,25 +73,43 @@ public class LoginFormsUtil { final IdentityProviderModel existingIdp = (serializedCtx == null) ? null : serializedCtx.deserialize(session, authSession).getIdpConfig(); final Set federatedIdentities; - if (context.getUser() != null) { - federatedIdentities = session.users().getFederatedIdentitiesStream(session.getContext().getRealm(), context.getUser()) - .map(federatedIdentityModel -> federatedIdentityModel.getIdentityProvider()) + UserModel user = context.getUser(); + if (user != null) { + federatedIdentities = session.users().getFederatedIdentitiesStream(session.getContext().getRealm(), user) + .map(FederatedIdentityModel::getIdentityProvider) .collect(Collectors.toSet()); } else { - federatedIdentities = null; + federatedIdentities = Set.of(); } - return providers + return filterOrganizationBrokers(providers) .filter(p -> { // Filter current IDP during first-broker-login flow. Re-authentication with the "linked" broker should not be possible if (existingIdp == null) return true; return !Objects.equals(p.getAlias(), existingIdp.getAlias()); }) - .filter(idp -> { // In case that we already have user established in authentication session, we show just providers already linked to this user - if (federatedIdentities == null) return true; + .filter(idp -> { + // user not established in authentication session, he can choose to authenticate using any broker + if (user == null) { + return true; + } + + if (federatedIdentities.isEmpty()) { + // user established but not linked to any broker, he can choose to authenticate using any organization broker + return idp.getOrganizationId() != null; + } + + // user established, we show just providers already linked to this user return federatedIdentities.contains(idp.getAlias()); - }) - .collect(Collectors.toList()); + }); } - return providers.collect(Collectors.toList()); + + return filterOrganizationBrokers(providers); + } + + private static Stream filterOrganizationBrokers(Stream providers) { + if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) { + providers = providers.filter(identityProviderModel -> identityProviderModel.getOrganizationId() == null); + } + return providers; } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java index e01b3ed616..f689a5d5f1 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Properties; +import java.util.stream.Stream; /** * @author Stian Thorgersen @@ -50,23 +51,22 @@ public class IdentityProviderBean { this.session = null; } - public IdentityProviderBean(RealmModel realm, KeycloakSession session, List identityProviders, URI baseURI) { + public IdentityProviderBean(RealmModel realm, KeycloakSession session, Stream identityProviders, URI baseURI) { this.realm = realm; this.session = session; - if (!identityProviders.isEmpty()) { - List orderedList = new ArrayList<>(); - for (IdentityProviderModel identityProvider : identityProviders) { - if (identityProvider.isEnabled() && !identityProvider.isLinkOnly()) { - addIdentityProvider(orderedList, realm, baseURI, identityProvider); - } - } + List orderedList = new ArrayList<>(); - if (!orderedList.isEmpty()) { - orderedList.sort(IDP_COMPARATOR_INSTANCE); - providers = orderedList; - displaySocial = true; + identityProviders.forEach(identityProvider -> { + if (identityProvider.isEnabled() && !identityProvider.isLinkOnly()) { + addIdentityProvider(orderedList, realm, baseURI, identityProvider); } + }); + + if (!orderedList.isEmpty()) { + orderedList.sort(IDP_COMPARATOR_INSTANCE); + providers = orderedList; + displaySocial = true; } } 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 fb6e604380..cb94459154 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 @@ -97,23 +97,25 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { return; } + context.setUser(user); + + if (organization != null) { + context.form().setAttributeMapper(attributes -> { + attributes.put("org", new OrganizationBean(session, organization, user)); + return attributes; + }); + } + List broker = resolveBroker(session, user); - if (broker.isEmpty()) { - // not a managed member, continue with the regular flow - if (organization != null) { - context.form().setAttributeMapper(attributes -> { - attributes.put("org", new OrganizationBean(session, organization, user)); - return attributes; - }); - } - context.setUser(user); - context.attempted(); - } else if (broker.size() == 1) { + if (broker.size() == 1) { // user is a managed member and associated with a broker, redirect automatically redirect(context, broker.get(0).getAlias(), user.getEmail()); + return; } + context.attempted(); + return; } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/login.ftl b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/login.ftl index 12c005d9be..b9074adad3 100755 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/login.ftl +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/login.ftl @@ -1,9 +1,8 @@ <#import "template.ftl" as layout> <#import "test-org-commons.ftl" as commons> <@layout.registrationLayout displayMessage=!messagesPerField.existsError('username','password') displayInfo=realm.password && realm.registrationAllowed && !registrationDisabled??; section> - <#if section = "header"> + <#if section = "form"> <@commons.assertions org/> - <#elseif section = "form">
<#if realm.password> diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java index 44aace92f6..a2962ac43e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java @@ -87,6 +87,7 @@ public class OrganizationThemeTest extends AbstractOrganizationTest { loginPage.open(bc.consumerRealmName()); Assert.assertTrue(driver.getPageSource().contains("Sign-in to the realm")); loginPage.loginUsername("tom@myorg.com"); + Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization")); Assert.assertTrue(loginPage.isPasswordInputPresent()); } @@ -204,6 +205,8 @@ public class OrganizationThemeTest extends AbstractOrganizationTest { // organization available to identity-first login page loginPage.open(bc.consumerRealmName()); loginPage.loginUsername(user.getEmail()); + Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization")); + Assert.assertTrue(driver.getPageSource().contains("User is member of " + orgRep.getName())); Assert.assertTrue(loginPage.isPasswordInputPresent()); } } 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 6a3b8d816d..e66756d48b 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 @@ -291,6 +291,55 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz assertIsMember(bc.getUserEmail(), organization); } + @Test + public void testShowOnlyBrokersLinkedUserInPasswordPage() { + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + OrganizationIdentityProviderResource broker = organization.identityProviders().get(bc.getIDPAlias()); + IdentityProviderRepresentation brokerRep = broker.toRepresentation(); + brokerRep.getConfig().put(IdentityProviderRedirectMode.EMAIL_MATCH.getKey(), Boolean.FALSE.toString()); + testRealm().identityProviders().get(brokerRep.getAlias()).update(brokerRep); + IdentityProviderRepresentation secondIdp = bc.setUpIdentityProvider(); + secondIdp.setAlias("second-idp"); + secondIdp.setInternalId(null); + secondIdp.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + secondIdp.getConfig().put(OrganizationModel.BROKER_PUBLIC, Boolean.TRUE.toString()); + testRealm().identityProviders().create(secondIdp).close(); + getCleanup().addCleanup(testRealm().identityProviders().get("second-idp")::remove); + organization.identityProviders().addIdentityProvider(secondIdp.getAlias()).close(); + + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + String email = bc.getUserEmail(); + loginPage.loginUsername(email); + // second-idp shown because user is not linked yet to any broker + Assert.assertTrue(loginPage.isSocialButtonPresent(secondIdp.getAlias())); + brokerRep.getConfig().put(IdentityProviderRedirectMode.EMAIL_MATCH.getKey(), Boolean.TRUE.toString()); + testRealm().identityProviders().get(brokerRep.getAlias()).update(brokerRep); + + assertBrokerRegistration(organization, email); + + // logout to force the user to authenticate again + UserRepresentation account = getUserRepresentation(email); + realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); + realmsResouce().realm(bc.providerRealmName()).logoutAll(); + + broker = organization.identityProviders().get(bc.getIDPAlias()); + brokerRep = broker.toRepresentation(); + organization.identityProviders().get(brokerRep.getAlias()).delete().close(); + IdentityProviderRepresentation finalBrokerRep = brokerRep; + getCleanup().addCleanup(() -> organization.identityProviders().addIdentityProvider(finalBrokerRep.getInternalId()).close()); + + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + log.debug("Logging in"); + loginPage.loginUsername(email); + Assert.assertFalse(loginPage.isUsernameInputPresent()); + Assert.assertTrue(loginPage.isPasswordInputPresent()); + Assert.assertTrue(loginPage.isSocialButtonPresent(bc.getIDPAlias())); + // second-idp not shown because user is linked to another broker + Assert.assertFalse(loginPage.isSocialButtonPresent(secondIdp.getAlias())); + } + @Test public void testFailUpdateEmailNotAssociatedOrganizationUsingAdminAPI() { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());