Skip to content

Commit

Permalink
Email validation for managed members should only fail if it does not …
Browse files Browse the repository at this point in the history 
…match the domain set to a broker

Closes #29460

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
  • Loading branch information
@pedroigor @ahus1
pedroigor authored and ahus1 committed May 14, 2024
1 parent 8178f74 commit 1b583a1
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 84 deletions.
40 changes: 3 additions & 37 deletions ...eycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -149,42 +151,6 @@ private static boolean hasPublicBrokers(List<IdentityProviderModel> 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<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);
}
Expand Down
44 changes: 44 additions & 0 deletions services/src/main/java/org/keycloak/organization/utils/Organizations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<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;
}
}
123 changes: 76 additions & 47 deletions services/src/main/java/org/keycloak/organization/validator/OrganizationMemberValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<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 (!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<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) {
Expand Down
31 changes: 31 additions & 0 deletions ...st/java/org/keycloak/testsuite/organization/admin/AbstractBrokerSelfRegistrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<UserRepresentation> reps = users.searchByEmail(userEmail, true);
Expand Down

0 comments on commit 1b583a1

Please sign in to comment.