Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Email validation for managed members should only fail if it does not match the domain set to a broker #29461

Merged
merged 1 commit into from
May 14, 2024
Merged

Email validation for managed members should only fail if it does not match the domain set to a broker #29461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 3 additions & 37 deletions ...eycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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()) {
pedroigor marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Loading