From ea398c21daf9d50ef382365087f80a9b4e06f159 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Wed, 25 Oct 2023 11:09:22 +0200 Subject: [PATCH] Add a property to the User Profile Email Validator for max length of the local part Closes https://github.com/keycloak/keycloak/issues/24273 --- .../topics/users/user-profile.adoc | 7 +++- .../keycloak/utils/EmailValidationUtil.java | 18 +++++++-- .../validate/validators/EmailValidator.java | 37 +++++++++++++++++-- .../validate/BuiltinValidatorsTest.java | 13 +++++++ .../org/keycloak/validate/ValidatorTest.java | 19 ++++++++++ .../testsuite/forms/VerifyProfileTest.java | 14 +++++-- 6 files changed, 96 insertions(+), 12 deletions(-) diff --git a/docs/documentation/server_admin/topics/users/user-profile.adoc b/docs/documentation/server_admin/topics/users/user-profile.adoc index eab9cd4982..be05e39701 100644 --- a/docs/documentation/server_admin/topics/users/user-profile.adoc +++ b/docs/documentation/server_admin/topics/users/user-profile.adoc @@ -202,7 +202,8 @@ image:images/user-profile-validation.png[] |email |Check if the value has a valid e-mail format. -| None +| +*max-local-length*: an integer to define the maximum length for the local part of the email. It defaults to 64 per specification. |local-date |Check if the value has a valid format based on the realm and/or user locale. @@ -293,7 +294,9 @@ The JSON schema is defined as follows: "edit": [ "admin", "user" ] }, "validations": { - "email": {}, + "email": { + "max-local-length": 64 + }, "length": { "max": 255 } diff --git a/server-spi-private/src/main/java/org/keycloak/utils/EmailValidationUtil.java b/server-spi-private/src/main/java/org/keycloak/utils/EmailValidationUtil.java index c868b8ac3c..ecf34c8999 100644 --- a/server-spi-private/src/main/java/org/keycloak/utils/EmailValidationUtil.java +++ b/server-spi-private/src/main/java/org/keycloak/utils/EmailValidationUtil.java @@ -7,7 +7,15 @@ import org.keycloak.Config; import static java.util.regex.Pattern.CASE_INSENSITIVE; +/** + * Email Validator Utility to check email inputs based on + * + * hibernate-validator implementation. + */ public class EmailValidationUtil { + + public static final int MAX_LOCAL_PART_LENGTH = 64; + private static final String LOCAL_PART_ATOM = "[a-z0-9!#$%&'*+/=?^_`{|}~\u0080-\uFFFF-]"; private static final String LOCAL_PART_INSIDE_QUOTES_ATOM = "(?:[a-z0-9!#$%&'*.(),<>\\[\\]:; @+/=?^_`{|}~\u0080-\uFFFF-]|\\\\\\\\|\\\\\\\")"; /** @@ -32,6 +40,10 @@ public class EmailValidationUtil { public static boolean isValidEmail(String value) { + return isValidEmail(value, Config.scope("user-profile-declarative-user-profile").getInt(MAX_EMAIL_LOCAL_PART_LENGTH, MAX_LOCAL_PART_LENGTH)); + } + + public static boolean isValidEmail(String value, int maxEmailLocalPartLength) { if ( value == null || value.length() == 0 ) { return false; } @@ -49,16 +61,16 @@ public class EmailValidationUtil { String localPart = stringValue.substring( 0, splitPosition ); String domainPart = stringValue.substring( splitPosition + 1 ); - if ( !isValidEmailLocalPart( localPart ) ) { + if ( !isValidEmailLocalPart( localPart, maxEmailLocalPartLength ) ) { return false; } return isValidEmailDomainAddress( domainPart ); } - private static boolean isValidEmailLocalPart(String localPart) { + private static boolean isValidEmailLocalPart(String localPart, int maxEmailLocalPartLength) { - if ( localPart.length() > Config.scope("user-profile-declarative-user-profile").getInt(MAX_EMAIL_LOCAL_PART_LENGTH,64) ) { + if ( localPart.length() > maxEmailLocalPartLength) { return false; } Matcher matcher = LOCAL_PART_PATTERN.matcher( localPart ); diff --git a/server-spi-private/src/main/java/org/keycloak/validate/validators/EmailValidator.java b/server-spi-private/src/main/java/org/keycloak/validate/validators/EmailValidator.java index 247fdd5813..9a1cea2ded 100644 --- a/server-spi-private/src/main/java/org/keycloak/validate/validators/EmailValidator.java +++ b/server-spi-private/src/main/java/org/keycloak/validate/validators/EmailValidator.java @@ -16,15 +16,19 @@ */ package org.keycloak.validate.validators; -import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; +import org.keycloak.models.KeycloakSession; import org.keycloak.provider.ConfiguredProvider; import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; import org.keycloak.utils.EmailValidationUtil; import org.keycloak.validate.AbstractStringValidator; import org.keycloak.validate.ValidationContext; import org.keycloak.validate.ValidationError; +import org.keycloak.validate.ValidationResult; import org.keycloak.validate.ValidatorConfig; /** @@ -39,6 +43,7 @@ public class EmailValidator extends AbstractStringValidator implements Configure public static final String MESSAGE_INVALID_EMAIL = "error-invalid-email"; + public static final String MAX_LOCAL_PART_LENGTH_PROPERTY = "max-local-length"; @Override public String getId() { @@ -47,7 +52,14 @@ public class EmailValidator extends AbstractStringValidator implements Configure @Override protected void doValidate(String value, String inputHint, ValidationContext context, ValidatorConfig config) { - if (!EmailValidationUtil.isValidEmail(value)) { + Integer maxEmailLocalPartLength = null; + if (config != null) { + maxEmailLocalPartLength = config.getInt(MAX_LOCAL_PART_LENGTH_PROPERTY); + } + + if (!(maxEmailLocalPartLength != null + ? EmailValidationUtil.isValidEmail(value, maxEmailLocalPartLength) + : EmailValidationUtil.isValidEmail(value))) { context.addError(new ValidationError(ID, inputHint, MESSAGE_INVALID_EMAIL, value)); } } @@ -59,6 +71,25 @@ public class EmailValidator extends AbstractStringValidator implements Configure @Override public List getConfigProperties() { - return Collections.emptyList(); + return ProviderConfigurationBuilder.create().property() + .name(MAX_LOCAL_PART_LENGTH_PROPERTY) + .type(ProviderConfigProperty.STRING_TYPE) + .label("Maximum length for the local part") + .helpText("Maximum length for the local part of the email") + .defaultValue(EmailValidationUtil.MAX_LOCAL_PART_LENGTH) + .required(false) + .add().build(); + } + + @Override + public ValidationResult validateConfig(KeycloakSession session, ValidatorConfig config) { + Set errors = new LinkedHashSet<>(); + if (config != null && config.containsKey(MAX_LOCAL_PART_LENGTH_PROPERTY)) { + Integer maxLocalPartLength = config.getInt(MAX_LOCAL_PART_LENGTH_PROPERTY); + if (maxLocalPartLength == null || maxLocalPartLength <= 0) { + errors.add(new ValidationError(ID, MAX_LOCAL_PART_LENGTH_PROPERTY, ValidatorConfigValidator.MESSAGE_CONFIG_INVALID_NUMBER_VALUE, config.get(MAX_LOCAL_PART_LENGTH_PROPERTY))); + } + } + return new ValidationResult(errors); } } diff --git a/server-spi-private/src/test/java/org/keycloak/validate/BuiltinValidatorsTest.java b/server-spi-private/src/test/java/org/keycloak/validate/BuiltinValidatorsTest.java index 5dde09ef0b..7bbacd4fc3 100644 --- a/server-spi-private/src/test/java/org/keycloak/validate/BuiltinValidatorsTest.java +++ b/server-spi-private/src/test/java/org/keycloak/validate/BuiltinValidatorsTest.java @@ -12,6 +12,7 @@ import java.util.regex.Pattern; import org.junit.Assert; import org.junit.Test; import org.keycloak.validate.validators.DoubleValidator; +import org.keycloak.validate.validators.EmailValidator; import org.keycloak.validate.validators.IntegerValidator; import org.keycloak.validate.validators.LengthValidator; import org.keycloak.validate.validators.OptionsValidator; @@ -138,6 +139,18 @@ public class BuiltinValidatorsTest { Assert.assertFalse(validator.validate(" ", "email").isValid()); Assert.assertFalse(validator.validate("adminATexample.org", "email").isValid()); + + Assert.assertTrue(validator.validate("username@keycloak.org", "email", (ValidatorConfig) null).isValid()); + Assert.assertTrue(validator.validate("abcd012345678901234567890123456789012345678901234567890123456789@keycloak.org", "email").isValid()); + Assert.assertFalse(validator.validate("abcde012345678901234567890123456789012345678901234567890123456789@keycloak.org", "email").isValid()); + Assert.assertTrue(validator.validate("abcdef0123456789@keycloak.org", "email", + new ValidatorConfig(ImmutableMap.of(EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, "16"))).isValid()); + Assert.assertFalse(validator.validate("abcdefg0123456789@keycloak.org", "email", + new ValidatorConfig(ImmutableMap.of(EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, 16))).isValid()); + Assert.assertTrue(validator.validate("ab012345678901234567890123456789@keycloak.org", "email", + new ValidatorConfig(ImmutableMap.of(EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, "32"))).isValid()); + Assert.assertFalse(validator.validate("abc012345678901234567890123456789@keycloak.org", "email", + new ValidatorConfig(ImmutableMap.of(EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, 32))).isValid()); } @Test diff --git a/server-spi-private/src/test/java/org/keycloak/validate/ValidatorTest.java b/server-spi-private/src/test/java/org/keycloak/validate/ValidatorTest.java index 779ef6fd4e..f4d7952e14 100644 --- a/server-spi-private/src/test/java/org/keycloak/validate/ValidatorTest.java +++ b/server-spi-private/src/test/java/org/keycloak/validate/ValidatorTest.java @@ -14,6 +14,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Assert; import org.junit.Test; import org.keycloak.models.KeycloakSession; +import org.keycloak.validate.validators.EmailValidator; import org.keycloak.validate.validators.LengthValidator; import org.keycloak.validate.validators.NotBlankValidator; import org.keycloak.validate.validators.ValidatorConfigValidator; @@ -195,6 +196,24 @@ public class ValidatorTest { Assert.assertTrue(validator.validateConfig(session, configFromMap(Collections.singletonMap("min", "123"))).isValid()); } + @Test + public void validateEmailValidator() { + SimpleValidator validator = Validators.emailValidator(); + + Assert.assertTrue(validator.validateConfig(session, null).isValid()); + Assert.assertTrue(validator.validateConfig(session, ValidatorConfig.EMPTY).isValid()); + Assert.assertTrue(validator.validateConfig(session, configFromMap(Collections.singletonMap( + EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, 128))).isValid()); + Assert.assertTrue(validator.validateConfig(session, configFromMap(Collections.singletonMap( + EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, "128"))).isValid()); + Assert.assertFalse(validator.validateConfig(session, configFromMap(Collections.singletonMap( + EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, null))).isValid()); + Assert.assertFalse(validator.validateConfig(session, configFromMap(Collections.singletonMap( + EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, "a"))).isValid()); + Assert.assertFalse(validator.validateConfig(session, configFromMap(Collections.singletonMap( + EmailValidator.MAX_LOCAL_PART_LENGTH_PROPERTY, ""))).isValid()); + } + @Test public void validateValidatorConfigMultipleOptions() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java index 367f18039c..70b88d7d84 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/VerifyProfileTest.java @@ -362,6 +362,8 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { public void testDefaultProfile() { setUserProfileConfiguration(null); + testingClient.server(TEST_REALM_NAME).run(setEmptyFirstNameAndCustomAttribute()); + loginPage.open(); loginPage.login("login-test", "password"); @@ -719,13 +721,13 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { } @Test - public void testEMailRequiredInProfile() { + public void testEMailRequiredInProfileWithLocalPartLength() { setUserProfileConfiguration("{\"attributes\": [" + "{\"name\": \"firstName\"," + PERMISSIONS_ALL + ", \"required\": {}}," + "{\"name\": \"lastName\"," + PERMISSIONS_ALL + "}," + "{\"name\": \"username\"," + PERMISSIONS_ADMIN_ONLY + "}," - + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\":{\"roles\":[\"user\"]}}" + + "{\"name\": \"email\"," + PERMISSIONS_ALL + ", \"required\":{\"roles\":[\"user\"]}, \"validations\": {\"email\": {\"max-local-length\": \"16\"}}}" + "]}"); loginPage.open(); @@ -734,8 +736,12 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { // no email is set => expect verify profile page to be displayed verifyProfilePage.assertCurrent(); + // set e-mail with legth 17 => error + verifyProfilePage.updateEmail("abcdefg0123456789@bar.com", "HasNowMailFirst", "HasNowMailLast"); + verifyProfilePage.assertCurrent(); + // set e-mail, update firstname/lastname and complete login - verifyProfilePage.updateEmail("foo@bar.com", "HasNowMailFirst", "HasNowMailLast"); + verifyProfilePage.updateEmail("abcdef0123456789@bar.com", "HasNowMailFirst", "HasNowMailLast"); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); @@ -743,7 +749,7 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest { UserRepresentation user = getUser(userWithoutEmailId); assertEquals("HasNowMailFirst", user.getFirstName()); assertEquals("HasNowMailLast", user.getLastName()); - assertEquals("foo@bar.com", user.getEmail()); + assertEquals("abcdef0123456789@bar.com", user.getEmail()); } @Test