From 1b583a1bab2d090ac70ca9ab1528ceaeba50b3ca Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 10 May 2024 21:49:58 -0300 Subject: [PATCH] Email validation for managed members should only fail if it does not match the domain set to a broker Closes #29460 Signed-off-by: Pedro Igor --- .../browser/OrganizationAuthenticator.java | 40 +----- .../organization/utils/Organizations.java | 44 +++++++ .../OrganizationMemberValidator.java | 123 +++++++++++------- .../AbstractBrokerSelfRegistrationTest.java | 31 +++++ 4 files changed, 154 insertions(+), 84 deletions(-) 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 fe8d036f44d..7fca1fab7e8 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 @@ -17,6 +17,8 @@ package org.keycloak.organization.authentication.authenticators.browser; +import static org.keycloak.organization.utils.Organizations.resolveBroker; + import java.util.List; import java.util.Objects; @@ -85,7 +87,7 @@ public void action(AuthenticationFlowContext context) { return; } - IdentityProviderModel broker = resolveBroker(user); + IdentityProviderModel broker = resolveBroker(session, user); if (broker == null) { // not a managed member, continue with the regular flow @@ -149,42 +151,6 @@ private static boolean hasPublicBrokers(List brokers) { return brokers.stream().anyMatch(p -> Boolean.parseBoolean(p.getConfig().getOrDefault(OrganizationModel.BROKER_PUBLIC, Boolean.FALSE.toString()))); } - private IdentityProviderModel resolveBroker(UserModel user) { - OrganizationProvider provider = session.getProvider(OrganizationProvider.class); - RealmModel realm = session.getContext().getRealm(); - OrganizationModel organization = provider.getByMember(user); - - if (organization == null || !organization.isEnabled()) { - return null; - } - - if (provider.isManagedMember(organization, user)) { - // user is a managed member, try to resolve the origin broker and redirect automatically - List organizationBrokers = organization.getIdentityProviders().toList(); - List brokers = session.users().getFederatedIdentitiesStream(realm, user) - .map(f -> { - IdentityProviderModel broker = realm.getIdentityProviderByAlias(f.getIdentityProvider()); - - if (!organizationBrokers.contains(broker)) { - return null; - } - - FederatedIdentityModel identity = session.users().getFederatedIdentity(realm, user, broker.getAlias()); - - if (identity != null) { - return broker; - } - - return null; - }).filter(Objects::nonNull) - .toList(); - - return brokers.size() == 1 ? brokers.get(0) : null; - } - - return null; - } - private OrganizationProvider getOrganizationProvider() { return session.getProvider(OrganizationProvider.class); } diff --git a/services/src/main/java/org/keycloak/organization/utils/Organizations.java b/services/src/main/java/org/keycloak/organization/utils/Organizations.java index 685720d594f..dce6a652c79 100644 --- a/services/src/main/java/org/keycloak/organization/utils/Organizations.java +++ b/services/src/main/java/org/keycloak/organization/utils/Organizations.java @@ -17,11 +17,19 @@ package org.keycloak.organization.utils; +import java.util.List; +import java.util.Objects; + import org.keycloak.common.Profile; import org.keycloak.common.Profile.Feature; +import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.GroupModel; +import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.organization.OrganizationProvider; import org.keycloak.utils.StringUtil; public class Organizations { @@ -41,4 +49,40 @@ public static boolean canManageOrganizationGroup(KeycloakSession session, GroupM return true; } + + public static IdentityProviderModel resolveBroker(KeycloakSession session, UserModel user) { + OrganizationProvider provider = session.getProvider(OrganizationProvider.class); + RealmModel realm = session.getContext().getRealm(); + OrganizationModel organization = provider.getByMember(user); + + if (organization == null || !organization.isEnabled()) { + return null; + } + + if (provider.isManagedMember(organization, user)) { + // user is a managed member, try to resolve the origin broker and redirect automatically + List organizationBrokers = organization.getIdentityProviders().toList(); + List brokers = session.users().getFederatedIdentitiesStream(realm, user) + .map(f -> { + IdentityProviderModel broker = realm.getIdentityProviderByAlias(f.getIdentityProvider()); + + if (!organizationBrokers.contains(broker)) { + return null; + } + + FederatedIdentityModel identity = session.users().getFederatedIdentity(realm, user, broker.getAlias()); + + if (identity != null) { + return broker; + } + + return null; + }).filter(Objects::nonNull) + .toList(); + + return brokers.size() == 1 ? brokers.get(0) : null; + } + + return null; + } } diff --git a/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java b/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java index d7a05764a92..d74ba3fa86e 100644 --- a/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java +++ b/services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java @@ -17,6 +17,8 @@ package org.keycloak.organization.validator; +import static java.util.Optional.ofNullable; +import static org.keycloak.organization.utils.Organizations.resolveBroker; import static org.keycloak.validate.BuiltinValidators.emailValidator; import java.util.List; @@ -73,54 +75,81 @@ public boolean isSupported(Scope config) { } private void validateEmailDomain(String email, String inputHint, ValidationContext context, OrganizationModel organization) { - if (UserModel.EMAIL.equals(inputHint)) { - if (StringUtil.isBlank(email)) { - context.addError(new ValidationError(ID, inputHint, "Email not set")); - return; - } - - if (!emailValidator().validate(email, inputHint, context).isValid()) { - return; - } - - UserProfileAttributeValidationContext upContext = (UserProfileAttributeValidationContext) context; - AttributeContext attributeContext = upContext.getAttributeContext(); - UserModel user = attributeContext.getUser(); - String emailDomain = email.substring(email.indexOf('@') + 1); - List expectedDomains = organization.getDomains().map(OrganizationDomainModel::getName).toList(); - - if (UserProfileContext.IDP_REVIEW.equals(attributeContext.getContext())) { - KeycloakSession session = attributeContext.getSession(); - BrokeredIdentityContext brokerContext = (BrokeredIdentityContext) session.getAttribute(BrokeredIdentityContext.class.getName()); - - if (brokerContext != null) { - String alias = brokerContext.getIdpConfig().getAlias(); - IdentityProviderModel broker = organization.getIdentityProviders().filter((p) -> p.getAlias().equals(alias)).findAny().orElse(null); - - if (broker == null) { - return; - } - - String brokerDomain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); - - if (brokerDomain == null) { - return; - } - - expectedDomains = List.of(brokerDomain); - } - } else if (!organization.isManaged(user)) { - return; - } - - if (expectedDomains.isEmpty()) { - return; - } - - if (!expectedDomains.contains(emailDomain)) { - context.addError(new ValidationError(ID, inputHint, "Email domain does not match any domain from the organization")); - } + if (!UserModel.EMAIL.equals(inputHint)) { + return; + } + + if (StringUtil.isBlank(email)) { + context.addError(new ValidationError(ID, inputHint, "Email not set")); + return; + } + + if (!emailValidator().validate(email, inputHint, context).isValid()) { + return; + } + + UserProfileAttributeValidationContext upContext = (UserProfileAttributeValidationContext) context; + AttributeContext attributeContext = upContext.getAttributeContext(); + UserModel user = attributeContext.getUser(); + String emailDomain = email.substring(email.indexOf('@') + 1); + List expectedDomains = organization.getDomains().map(OrganizationDomainModel::getName).toList(); + + if (expectedDomains.isEmpty()) { + // no domain to check + return; + } + + if (UserProfileContext.IDP_REVIEW.equals(attributeContext.getContext())) { + expectedDomains = resolveExpectedDomainsWhenReviewingFederatedUserProfile(organization, attributeContext); + } else if (organization.isManaged(user)) { + expectedDomains = resolveExpectedDomainsForManagedUser(context, user); + } else { + // no validation happens for unmanaged users as they are realm users linked to an organization + return; } + + if (expectedDomains.isEmpty() || expectedDomains.contains(emailDomain)) { + // valid email domain + return; + } + + context.addError(new ValidationError(ID, inputHint, "Email domain does not match any domain from the organization")); + } + + private static List resolveExpectedDomainsForManagedUser(ValidationContext context, UserModel user) { + IdentityProviderModel broker = resolveBroker(context.getSession(), user); + + if (broker == null) { + return List.of(); + } + + String domain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + return ofNullable(domain).map(List::of).orElse(List.of()); + } + + private static List resolveExpectedDomainsWhenReviewingFederatedUserProfile(OrganizationModel organization, AttributeContext attributeContext) { + // validating in the context of the brokering flow + KeycloakSession session = attributeContext.getSession(); + BrokeredIdentityContext brokerContext = (BrokeredIdentityContext) session.getAttribute(BrokeredIdentityContext.class.getName()); + + if (brokerContext == null) { + return List.of(); + } + + String alias = brokerContext.getIdpConfig().getAlias(); + IdentityProviderModel broker = organization.getIdentityProviders() + .filter((p) -> p.getAlias().equals(alias)) + .findAny() + .orElse(null); + + if (broker == null) { + // the broker the user is authenticating is not linked to the organization + return List.of(); + } + + // expect the email domain to match the domain set to the broker or none if not set + String brokerDomain = broker.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + return ofNullable(brokerDomain).map(List::of).orElse(List.of()); } private OrganizationModel resolveOrganization(ValidationContext context, KeycloakSession session) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java index 5143d8f131f..ed54987275a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java @@ -769,6 +769,37 @@ public void testMemberFromBrokerRedirectedToOriginBroker() { appPage.assertCurrent(); } + @Test + public void testAllowUpdateEmailWithDifferentDomainThanOrgIfBrokerHasNoDomainSet() { + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + String email = bc.getUserEmail(); + assertBrokerRegistration(organization, email); + + IdentityProviderRepresentation idpRep = organization.identityProviders().getIdentityProviders().get(0); + idpRep.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep); + UserRepresentation user = getUserRepresentation(email); + user.setEmail("user@someother.com"); + testRealm().users().get(user.getId()).update(user); + } + + @Test + public void testFailUpdateEmailWithDifferentDomainThanOrgIfBrokerHasDomainSet() { + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + String email = bc.getUserEmail(); + assertBrokerRegistration(organization, email); + IdentityProviderRepresentation idpRep = organization.identityProviders().getIdentityProviders().get(0); + assertEquals(email.substring(email.indexOf('@') + 1), idpRep.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE)); + UserRepresentation user = getUserRepresentation(email); + user.setEmail("user@someother.com"); + try { + testRealm().users().get(user.getId()).update(user); + fail("invalid email domain"); + } catch (BadRequestException expected) { + + } + } + private void assertIsNotMember(String userEmail, OrganizationResource organization) { UsersResource users = adminClient.realm(bc.consumerRealmName()).users(); List reps = users.searchByEmail(userEmail, true);