From 4b4dd6277e8589739a596b34ed4cd7825bbb9a68 Mon Sep 17 00:00:00 2001 From: Heath Harrelson Date: Thu, 24 Sep 2026 16:43:09 -0700 Subject: [PATCH] AUTHLIB-178 Custom LDAP Domain Validator Convert LDAP account email domain validation to a custom bean validator for consistency with other validation. --- CHANGELOG.md | 4 + .../server/controller/UserController.java | 21 +--- .../server/security/entity/User.java | 2 + .../validation/ValidLdapEmailDomain.java | 42 +++++++ .../ValidLdapEmailDomainValidator.java | 63 ++++++++++ .../password/PasswordGeneratorTest.java | 6 +- .../UserConstraintValidationTest.java | 6 +- .../test/ValidatorTestUtil.java | 44 +++++++ .../ValidEmailDomainValidatorTest.java | 114 ++++++++++++++++++ 9 files changed, 274 insertions(+), 28 deletions(-) create mode 100644 authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomain.java create mode 100644 authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomainValidator.java create mode 100644 authentication_lib/src/test/java/org/octri/authentication/test/ValidatorTestUtil.java create mode 100644 authentication_lib/src/test/java/org/octri/authentication/validation/ValidEmailDomainValidatorTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ed35129..66ee8ee2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add polling to unlock accounts after a configurable cooldown period (AUTHLIB-180) - Email domain validation for LDAP accounts (AUTHLIB-178) +### Changed + +- Extract custom LDAP domain validation class for consistency with other validation (AUTHLIB-178) + ## [4.3.0] - 2026-09-16 ### Added diff --git a/authentication_lib/src/main/java/org/octri/authentication/server/controller/UserController.java b/authentication_lib/src/main/java/org/octri/authentication/server/controller/UserController.java index ef3c177c..d1bf8478 100644 --- a/authentication_lib/src/main/java/org/octri/authentication/server/controller/UserController.java +++ b/authentication_lib/src/main/java/org/octri/authentication/server/controller/UserController.java @@ -351,26 +351,7 @@ private List validateUser(User user) { ? validator.validate(user, Default.class) : validator.validate(user, Emailable.class); - List errors = validationUtils.getErrors(user, validationResult); - if (invalidEmailDomain(user)) { - errors.add(new FieldError(User.class.getName(), "email", - "Email must end with @" + getLdapEmailDomain() + " for LDAP accounts")); - } - return errors; - } - - /** - * Checks that an LDAP user's email address belongs to the configured LDAP email domain. Blank emails are left to - * bean validation. - * - * @param user - * @return an error for the email field if the domain does not match - */ - private Boolean invalidEmailDomain(User user) { - var ldapEmailDomain = getLdapEmailDomain(); - return !StringUtils.isBlank(ldapEmailDomain) - && !StringUtils.isBlank(user.getEmail()) - && !SecurityHelper.hasEmailDomain(user, ldapEmailDomain); + return validationUtils.getErrors(user, validationResult); } /** diff --git a/authentication_lib/src/main/java/org/octri/authentication/server/security/entity/User.java b/authentication_lib/src/main/java/org/octri/authentication/server/security/entity/User.java index 191f1bd0..9e8775f6 100644 --- a/authentication_lib/src/main/java/org/octri/authentication/server/security/entity/User.java +++ b/authentication_lib/src/main/java/org/octri/authentication/server/security/entity/User.java @@ -9,6 +9,7 @@ import java.util.List; import org.octri.authentication.validation.Emailable; +import org.octri.authentication.validation.ValidLdapEmailDomain; import org.octri.common.domain.AbstractEntity; import org.octri.common.view.Labelled; import org.springframework.format.annotation.DateTimeFormat; @@ -34,6 +35,7 @@ * */ @Entity +@ValidLdapEmailDomain public class User extends AbstractEntity implements Labelled { private static final String INVALID_EMAIL_MESSAGE = "Please provide a valid email address"; diff --git a/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomain.java b/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomain.java new file mode 100644 index 00000000..7528d7d7 --- /dev/null +++ b/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomain.java @@ -0,0 +1,42 @@ +package org.octri.authentication.validation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import jakarta.validation.Constraint; +import jakarta.validation.Payload; + +/** + * Annotation that adds {@link ValidLdapEmailDomainValidator} validation to an entity. + */ +@Documented +@Constraint(validatedBy = ValidLdapEmailDomainValidator.class) +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface ValidLdapEmailDomain { + + /** + * Default constraint violation message template. + * + * @return the default template + */ + String message() default "Email domain must be {domain} for LDAP accounts."; + + /** + * Validation groups associated with the validation. Defaults to an empty array (default group). + * + * @return validation groups + */ + Class[] groups() default {}; + + /** + * Validation payloads associated with the validation. Defaults to an empty array. + * + * @return validation payloads + */ + Class[] payload() default {}; + +} diff --git a/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomainValidator.java b/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomainValidator.java new file mode 100644 index 00000000..c62c5f7c --- /dev/null +++ b/authentication_lib/src/main/java/org/octri/authentication/validation/ValidLdapEmailDomainValidator.java @@ -0,0 +1,63 @@ +package org.octri.authentication.validation; + +import org.apache.commons.lang3.StringUtils; +import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorContext; +import org.octri.authentication.config.LdapContextProperties; +import org.octri.authentication.server.security.SecurityHelper; +import org.octri.authentication.server.security.entity.AuthenticationMethod; +import org.octri.authentication.server.security.entity.User; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; + +/** + * Validates that {@link User} entities annotated with {@link ValidLdapEmailDomain} have an email address with the + * expected domain name when the authentication method is {@link AuthenticationMethod#LDAP}. + */ +public class ValidLdapEmailDomainValidator implements ConstraintValidator { + + private final LdapContextProperties ldapContextProperties; + + /** + * Constructor. + * + * @param ldapContextProperties + * LDAP configuration + */ + public ValidLdapEmailDomainValidator(LdapContextProperties ldapContextProperties) { + this.ldapContextProperties = ldapContextProperties; + } + + @Override + public boolean isValid(User user, ConstraintValidatorContext context) { + if (!hasValidEmailDomain(user)) { + // while this is a cross-field class validation, the violation only applies to the {@code email} property + context.disableDefaultConstraintViolation(); + var unwrappedContext = context.unwrap(HibernateConstraintValidatorContext.class); + unwrappedContext + .addMessageParameter("domain", "@" + ldapContextProperties.getEmailDomain()) + .buildConstraintViolationWithTemplate(context.getDefaultConstraintMessageTemplate()) + .addPropertyNode("email") + .addConstraintViolation(); + + return false; + } + + return true; + } + + private boolean needsDomainCheck(User user) { + return user != null && AuthenticationMethod.LDAP.equals(user.getAuthenticationMethod()) + && !StringUtils.isBlank(user.getEmail()) + && !StringUtils.isBlank(ldapContextProperties.getEmailDomain()); + } + + private boolean hasValidEmailDomain(User user) { + if (needsDomainCheck(user)) { + return SecurityHelper.hasEmailDomain(user, ldapContextProperties.getEmailDomain()); + } + + return true; + } + +} diff --git a/authentication_lib/src/test/java/org/octri/authentication/server/security/password/PasswordGeneratorTest.java b/authentication_lib/src/test/java/org/octri/authentication/server/security/password/PasswordGeneratorTest.java index 4cea736d..ab7adb50 100644 --- a/authentication_lib/src/test/java/org/octri/authentication/server/security/password/PasswordGeneratorTest.java +++ b/authentication_lib/src/test/java/org/octri/authentication/server/security/password/PasswordGeneratorTest.java @@ -11,11 +11,10 @@ import org.junit.jupiter.api.Test; import org.octri.authentication.server.security.entity.AuthenticationMethod; import org.octri.authentication.server.security.entity.User; +import org.octri.authentication.test.ValidatorTestUtil; import jakarta.validation.ConstraintViolation; -import jakarta.validation.Validation; import jakarta.validation.Validator; -import jakarta.validation.ValidatorFactory; public class PasswordGeneratorTest { @@ -25,8 +24,7 @@ public class PasswordGeneratorTest { @BeforeAll public static void setUp() { - ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); - validator = factory.getValidator(); + validator = ValidatorTestUtil.buildValidator(); } @BeforeEach diff --git a/authentication_lib/src/test/java/org/octri/authentication/server/security/password/UserConstraintValidationTest.java b/authentication_lib/src/test/java/org/octri/authentication/server/security/password/UserConstraintValidationTest.java index 7616fa57..4cba4565 100644 --- a/authentication_lib/src/test/java/org/octri/authentication/server/security/password/UserConstraintValidationTest.java +++ b/authentication_lib/src/test/java/org/octri/authentication/server/security/password/UserConstraintValidationTest.java @@ -13,12 +13,11 @@ import org.junit.jupiter.api.Test; import org.octri.authentication.server.security.entity.AuthenticationMethod; import org.octri.authentication.server.security.entity.User; +import org.octri.authentication.test.ValidatorTestUtil; import org.octri.authentication.validation.Emailable; import jakarta.validation.ConstraintViolation; -import jakarta.validation.Validation; import jakarta.validation.Validator; -import jakarta.validation.ValidatorFactory; import jakarta.validation.groups.Default; /** @@ -72,8 +71,7 @@ public class UserConstraintValidationTest { @BeforeAll public static void setUp() { - ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); - validator = factory.getValidator(); + validator = ValidatorTestUtil.buildValidator(); } @BeforeEach diff --git a/authentication_lib/src/test/java/org/octri/authentication/test/ValidatorTestUtil.java b/authentication_lib/src/test/java/org/octri/authentication/test/ValidatorTestUtil.java new file mode 100644 index 00000000..3aeffc0f --- /dev/null +++ b/authentication_lib/src/test/java/org/octri/authentication/test/ValidatorTestUtil.java @@ -0,0 +1,44 @@ +package org.octri.authentication.test; + +import org.octri.authentication.config.LdapContextProperties; +import org.octri.authentication.validation.ValidLdapEmailDomainValidator; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorFactory; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; + +/** + * Helper for building a {@link Validator} capable of constructing {@link ValidLdapEmailDomainValidator}, which requires + * constructor arguments that the default {@link ConstraintValidatorFactory} cannot supply. + */ +public class ValidatorTestUtil { + + public static Validator buildValidator() { + ValidatorFactory factory = Validation.byDefaultProvider().configure() + .constraintValidatorFactory(new ConstraintValidatorFactory() { + + @Override + @SuppressWarnings("unchecked") + public > T getInstance(Class key) { + if (key == ValidLdapEmailDomainValidator.class) { + return (T) new ValidLdapEmailDomainValidator(new LdapContextProperties()); + } + try { + return key.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } + + @Override + public void releaseInstance(ConstraintValidator instance) { + // No-op + } + }) + .buildValidatorFactory(); + return factory.getValidator(); + } + +} diff --git a/authentication_lib/src/test/java/org/octri/authentication/validation/ValidEmailDomainValidatorTest.java b/authentication_lib/src/test/java/org/octri/authentication/validation/ValidEmailDomainValidatorTest.java new file mode 100644 index 00000000..8ed5ebf6 --- /dev/null +++ b/authentication_lib/src/test/java/org/octri/authentication/validation/ValidEmailDomainValidatorTest.java @@ -0,0 +1,114 @@ +package org.octri.authentication.validation; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorContext; +import org.hibernate.validator.constraintvalidation.HibernateConstraintViolationBuilder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.octri.authentication.config.LdapContextProperties; +import org.octri.authentication.server.security.entity.AuthenticationMethod; +import org.octri.authentication.server.security.entity.User; + +import jakarta.validation.ConstraintValidatorContext; +import jakarta.validation.ConstraintValidatorContext.ConstraintViolationBuilder.NodeBuilderCustomizableContext; + +@ExtendWith(MockitoExtension.class) +public class ValidEmailDomainValidatorTest { + + private static final String EMAIL_DOMAIN = "example.com"; + + @Mock + private ConstraintValidatorContext context; + + private LdapContextProperties ldapContextProperties; + private ValidLdapEmailDomainValidator validator; + private User user; + + @BeforeEach + public void setUp() { + ldapContextProperties = new LdapContextProperties(); + ldapContextProperties.setEmailDomain(EMAIL_DOMAIN); + validator = new ValidLdapEmailDomainValidator(ldapContextProperties); + + user = new User(); + user.setAuthenticationMethod(AuthenticationMethod.LDAP); + user.setEmail("foo@" + EMAIL_DOMAIN); + } + + @Test + public void validForLdapUserWithMatchingDomain() { + assertTrue(validator.isValid(user, context), "Email matching the configured domain should be valid"); + } + + @Test + public void invalidForLdapUserWithMismatchedDomain() { + user.setEmail("foo@other.com"); + mockConstraintViolationBuilder(); + + assertFalse(validator.isValid(user, context), "Email not matching the configured domain should be invalid"); + } + + @Test + public void invalidLdapUserBuildsConstraintViolationWithDomainParameter() { + user.setEmail("foo@other.com"); + HibernateConstraintValidatorContext unwrappedContext = mockConstraintViolationBuilder(); + + validator.isValid(user, context); + + verify(unwrappedContext).addMessageParameter("domain", "@" + EMAIL_DOMAIN); + } + + @Test + public void validForTableBasedUserRegardlessOfEmailDomain() { + user.setAuthenticationMethod(AuthenticationMethod.TABLE_BASED); + user.setEmail("foo@other.com"); + + assertTrue(validator.isValid(user, context), "Table-based users should not be checked against the LDAP domain"); + } + + @Test + public void validForLdapUserWithNullEmail() { + user.setEmail(null); + + assertTrue(validator.isValid(user, context), "A null email should not fail domain validation"); + } + + @Test + public void validForNullUser() { + assertTrue(validator.isValid(null, context), "A null user should not fail domain validation"); + } + + @Test + public void validWhenConfiguredEmailDomainIsNull() { + ldapContextProperties.setEmailDomain(null); + user.setEmail("foo@other.com"); + + assertTrue(validator.isValid(user, context), "A null configured email domain should skip validation"); + } + + private HibernateConstraintValidatorContext mockConstraintViolationBuilder() { + HibernateConstraintValidatorContext unwrappedContext = mock(HibernateConstraintValidatorContext.class); + HibernateConstraintViolationBuilder builder = mock(HibernateConstraintViolationBuilder.class); + NodeBuilderCustomizableContext nodeBuilder = mock(NodeBuilderCustomizableContext.class); + + when(context.unwrap(HibernateConstraintValidatorContext.class)).thenReturn(unwrappedContext); + when(unwrappedContext.addMessageParameter("domain", "@" + EMAIL_DOMAIN)).thenReturn(unwrappedContext); + when(context.getDefaultConstraintMessageTemplate()) + .thenReturn("Email domain must be {domain} for LDAP accounts."); + when(unwrappedContext.buildConstraintViolationWithTemplate("Email domain must be {domain} for LDAP accounts.")) + .thenReturn(builder); + when(builder.addPropertyNode("email")).thenReturn(nodeBuilder); + when(nodeBuilder.addConstraintViolation()).thenReturn(context); + + return unwrappedContext; + } + +}