libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Reverting removal of test assertions and keeping existing logic where only brokers the user is linked to is shown after identity-first login page

Browse source 
Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2024-07-02 11:41:03 -03:00
parent e1b735fc41
commit f010f7df9b
8 changed files with 118 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernameForm.java
View file

@ -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<IdentityProviderModel> identityProviders = LoginFormsUtil
Stream<IdentityProviderModel> identityProviders = LoginFormsUtil
.filterIdentityProviders(context.getRealm().getIdentityProvidersStream(), context.getSession(), context);
if (identityProviders.isEmpty()) {
if (identityProviders.findAny().isEmpty()) {
context.success();
return;
}

3
services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java
View file

@ -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<IdentityProviderModel> identityProviders = LoginFormsUtil
Stream<IdentityProviderModel> identityProviders = LoginFormsUtil
.filterIdentityProvidersForTheme(realm.getIdentityProvidersStream(), session, context);
IdentityProviderBean idpBean = new IdentityProviderBean(realm, session, identityProviders, baseUriWithCodeAndClientId);

52
services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java
View file

@ -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<IdentityProviderModel> filterIdentityProvidersForTheme(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) {
public static Stream<IdentityProviderModel> filterIdentityProvidersForTheme(Stream<IdentityProviderModel> 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<IdentityProviderModel> filterIdentityProviders(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) {
public static Stream<IdentityProviderModel> filterIdentityProviders(Stream<IdentityProviderModel> 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<String> 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<IdentityProviderModel> filterOrganizationBrokers(Stream<IdentityProviderModel> providers) {
if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) {
providers = providers.filter(identityProviderModel -> identityProviderModel.getOrganizationId() == null);
}
return providers;
}
}

24
services/src/main/java/org/keycloak/forms/login/freemarker/model/IdentityProviderBean.java
View file

@ -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 <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -50,23 +51,22 @@ public class IdentityProviderBean {
this.session = null;
}
public IdentityProviderBean(RealmModel realm, KeycloakSession session, List<IdentityProviderModel> identityProviders, URI baseURI) {
public IdentityProviderBean(RealmModel realm, KeycloakSession session, Stream<IdentityProviderModel> identityProviders, URI baseURI) {
this.realm = realm;
this.session = session;
if (!identityProviders.isEmpty()) {
List<IdentityProvider> orderedList = new ArrayList<>();
for (IdentityProviderModel identityProvider : identityProviders) {
if (identityProvider.isEnabled() && !identityProvider.isLinkOnly()) {
addIdentityProvider(orderedList, realm, baseURI, identityProvider);
}
}
List<IdentityProvider> 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;
}
}

24
services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java
View file

@ -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<IdentityProviderModel> 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;
}

3
testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/login.ftl
View file

@ -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">
<div id="kc-form">
<div id="kc-form-wrapper">
<#if realm.password>

3
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java
View file

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

49
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java
View file

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