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 <pigor.craveiro@gmail.com>

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 class OrganizationAuthenticator extends IdentityProviderAuthenticator {
                 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 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
         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<IdentityProviderModel> organizationBrokers = organization.getIdentityProviders().toList();
-            List<IdentityProviderModel> 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);
     }

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 class Organizations {
 
         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<IdentityProviderModel> organizationBrokers = organization.getIdentityProviders().toList();
+            List<IdentityProviderModel> 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;
+    }
 }

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 class OrganizationMemberValidator extends AbstractSimpleValidator impleme
     }
 
     private void validateEmailDomain(String email, String inputHint, ValidationContext context, OrganizationModel organization) {
-        if (UserModel.EMAIL.equals(inputHint)) {
+        if (!UserModel.EMAIL.equals(inputHint)) {
-            if (StringUtil.isBlank(email)) {
+            return;
-                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<String> 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 (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<String> 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<String> 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<String> 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) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java

@@ -769,6 +769,37 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz
         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<UserRepresentation> reps = users.searchByEmail(userEmail, true);