KEYCLOAK-676 Validate email address format during registration or account mgmt

forms/common-themes/src/main/resources/theme/account/base/messages/messages.properties

@@ -10,6 +10,7 @@ successHeader=Success!
 username=Username
 
 missingFirstName=Please specify first name
+invalidEmail=Invalid email address
 missingLastName=Please specify last name
 missingEmail=Please specify email
 missingPassword=Please specify password.

forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties

@@ -31,6 +31,7 @@ clientCertificate=Client Certificate
 
 invalidUser=Invalid username or password.
 invalidPassword=Invalid username or password.
+invalidEmail=Invalid email address
 accountDisabled=Account is disabled, contact admin
 accountTemporarilyDisabled=Account is temporarily disabled, contact admin or try again later
 

services/src/main/java/org/keycloak/services/messages/Messages.java

@@ -35,6 +35,8 @@ public class Messages {
 
     public static final String INVALID_PASSWORD_CONFIRM = "invalidPasswordConfirm";
 
+    public static final String INVALID_EMAIL = "invalidEmail";
+
     public static final String INVALID_USER = "invalidUser";
 
     public static final String READ_ONLY_USER = "readOnlyUser";

services/src/main/java/org/keycloak/services/validation/Validation.java

@@ -6,9 +6,13 @@ import org.keycloak.services.messages.Messages;
 
 import javax.ws.rs.core.MultivaluedMap;
 import java.util.List;
+import java.util.regex.Pattern;
 
 public class Validation {
 
+    // Actually allow same emails like angular. See ValidationTest.testEmailValidation()
+    private static final Pattern EMAIL_PATTERN = Pattern.compile("[a-zA-Z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*");
+
     public static String validateRegistrationForm(MultivaluedMap<String, String> formData, List<String> requiredCredentialTypes) {
         if (isEmpty(formData.getFirst("firstName"))) {
             return Messages.MISSING_FIRST_NAME;
@@ -22,6 +26,10 @@ public class Validation {
             return Messages.MISSING_EMAIL;
         }
 
+        if (!isEmailValid(formData.getFirst("email"))) {
+            return Messages.INVALID_EMAIL;
+        }
+
         if (isEmpty(formData.getFirst("username"))) {
             return Messages.MISSING_USERNAME;
         }
@@ -56,6 +64,10 @@ public class Validation {
             return Messages.MISSING_EMAIL;
         }
 
+        if (!isEmailValid(formData.getFirst("email"))) {
+            return Messages.INVALID_EMAIL;
+        }
+
         return null;
     }
 
@@ -63,4 +75,9 @@ public class Validation {
         return s == null || s.length() == 0;
     }
 
+    public static boolean isEmailValid(String email) {
+        return EMAIL_PATTERN.matcher(email).matches();
+    }
+
+
 }

services/src/test/java/org/keycloak/test/ValidationTest.java

@@ -0,0 +1,27 @@
+package org.keycloak.test;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.services.validation.Validation;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class ValidationTest {
+
+    @Test
+    public void testEmailValidation() {
+        Assert.assertTrue(Validation.isEmailValid("abc@abc.cz"));
+        Assert.assertTrue(Validation.isEmailValid("435@coco.foo.cz"));
+        Assert.assertTrue(Validation.isEmailValid("A@something"));
+        Assert.assertTrue(Validation.isEmailValid("A@some-thing.foo"));
+        Assert.assertTrue(Validation.isEmailValid("1@A"));
+        Assert.assertFalse(Validation.isEmailValid("A@some_thing.foo"));
+        Assert.assertFalse(Validation.isEmailValid("@some_thing.foo"));
+        Assert.assertFalse(Validation.isEmailValid("abc@"));
+        Assert.assertFalse(Validation.isEmailValid("abc@."));
+        Assert.assertFalse(Validation.isEmailValid("abc@.foo"));
+        Assert.assertFalse(Validation.isEmailValid("abc@foo."));
+        Assert.assertFalse(Validation.isEmailValid("abc@foo..bar"));
+    }
+}

testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java

@@ -135,9 +135,9 @@ public class RequiredActionEmailVerificationTest {
     public void verifyEmailRegister() throws IOException, MessagingException {
         loginPage.open();
         loginPage.clickRegister();
-        registerPage.register("firstName", "lastName", "email", "verifyEmail", "password", "password");
+        registerPage.register("firstName", "lastName", "email@mail.com", "verifyEmail", "password", "password");
 
-        String userId = events.expectRegister("verifyEmail", "email").assertEvent().getUserId();
+        String userId = events.expectRegister("verifyEmail", "email@mail.com").assertEvent().getUserId();
 
         Assert.assertTrue(verifyEmailPage.isCurrent());
 
@@ -147,7 +147,7 @@ public class RequiredActionEmailVerificationTest {
 
         String body = (String) message.getContent();
 
-        Event sendEvent = events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).user(userId).detail("username", "verifyEmail").detail("email", "email").assertEvent();
+        Event sendEvent = events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).user(userId).detail("username", "verifyEmail").detail("email", "email@mail.com").assertEvent();
         String sessionId = sendEvent.getSessionId();
 
         String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID);
@@ -158,7 +158,7 @@ public class RequiredActionEmailVerificationTest {
 
         Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
 
-        events.expectRequiredAction(EventType.VERIFY_EMAIL).user(userId).session(sessionId).detail("username", "verifyEmail").detail("email", "email").detail(Details.CODE_ID, mailCodeId).assertEvent();
+        events.expectRequiredAction(EventType.VERIFY_EMAIL).user(userId).session(sessionId).detail("username", "verifyEmail").detail("email", "email@mail.com").detail(Details.CODE_ID, mailCodeId).assertEvent();
 
         events.expectLogin().user(userId).session(sessionId).detail("username", "verifyEmail").detail(Details.CODE_ID, mailCodeId).assertEvent();
     }

testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java

@@ -99,9 +99,9 @@ public class RequiredActionTotpSetupTest {
     public void setupTotpRegister() {
         loginPage.open();
         loginPage.clickRegister();
-        registerPage.register("firstName", "lastName", "email", "setupTotp", "password", "password");
+        registerPage.register("firstName", "lastName", "email@mail.com", "setupTotp", "password", "password");
 
-        String userId = events.expectRegister("setupTotp", "email").assertEvent().getUserId();
+        String userId = events.expectRegister("setupTotp", "email@mail.com").assertEvent().getUserId();
 
         totpPage.assertCurrent();
 
@@ -149,9 +149,9 @@ public class RequiredActionTotpSetupTest {
         // Register new user
         loginPage.open();
         loginPage.clickRegister();
-        registerPage.register("firstName2", "lastName2", "email2", "setupTotp2", "password2", "password2");
+        registerPage.register("firstName2", "lastName2", "email2@mail.com", "setupTotp2", "password2", "password2");
 
-        String userId = events.expectRegister("setupTotp2", "email2").assertEvent().getUserId();
+        String userId = events.expectRegister("setupTotp2", "email2@mail.com").assertEvent().getUserId();
 
         // Configure totp
         totpPage.assertCurrent();

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java

@@ -210,7 +210,7 @@ public class FederationProvidersIntegrationTest {
         registerPage.assertCurrent();
 
         // check existing username
-        registerPage.register("firstName", "lastName", "email", "existing", "Password1", "Password1");
+        registerPage.register("firstName", "lastName", "email@mail.cz", "existing", "Password1", "Password1");
         registerPage.assertCurrent();
         Assert.assertEquals("Username already exists", registerPage.getError());
 
@@ -226,7 +226,7 @@ public class FederationProvidersIntegrationTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "email2", "registerUserSuccess2", "Password1", "Password1");
+        registerPage.register("firstName", "lastName", "email2@check.cz", "registerUserSuccess2", "Password1", "Password1");
         Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
 
         KeycloakSession session = keycloakRule.startSession();

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java

@@ -75,12 +75,12 @@ public class RegisterTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "registerExistingUseremail", "test-user@localhost", "password", "password");
+        registerPage.register("firstName", "lastName", "registerExistingUser@email", "test-user@localhost", "password", "password");
 
         registerPage.assertCurrent();
         Assert.assertEquals("Username already exists", registerPage.getError());
 
-        events.expectRegister("test-user@localhost", "registerExistingUseremail").user((String) null).error("username_in_use").assertEvent();
+        events.expectRegister("test-user@localhost", "registerExistingUser@email").user((String) null).error("username_in_use").assertEvent();
     }
 
     @Test
@@ -89,12 +89,12 @@ public class RegisterTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "registerUserInvalidPasswordConfirmemail", "registerUserInvalidPasswordConfirm", "password", "invalid");
+        registerPage.register("firstName", "lastName", "registerUserInvalidPasswordConfirm@email", "registerUserInvalidPasswordConfirm", "password", "invalid");
 
         registerPage.assertCurrent();
         Assert.assertEquals("Password confirmation doesn't match", registerPage.getError());
 
-        events.expectRegister("registerUserInvalidPasswordConfirm", "registerUserInvalidPasswordConfirmemail").user((String) null).error("invalid_registration").assertEvent();
+        events.expectRegister("registerUserInvalidPasswordConfirm", "registerUserInvalidPasswordConfirm@email").user((String) null).error("invalid_registration").assertEvent();
     }
 
     @Test
@@ -103,12 +103,12 @@ public class RegisterTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "registerUserMissingPasswordemail", "registerUserMissingPassword", null, null);
+        registerPage.register("firstName", "lastName", "registerUserMissingPassword@email", "registerUserMissingPassword", null, null);
 
         registerPage.assertCurrent();
         Assert.assertEquals("Please specify password.", registerPage.getError());
 
-        events.expectRegister("registerUserMissingPassword", "registerUserMissingPasswordemail").user((String) null).error("invalid_registration").assertEvent();
+        events.expectRegister("registerUserMissingPassword", "registerUserMissingPassword@email").user((String) null).error("invalid_registration").assertEvent();
     }
 
     @Test
@@ -125,17 +125,17 @@ public class RegisterTest {
             loginPage.clickRegister();
             registerPage.assertCurrent();
 
-            registerPage.register("firstName", "lastName", "registerPasswordPolicyemail", "registerPasswordPolicy", "pass", "pass");
+            registerPage.register("firstName", "lastName", "registerPasswordPolicy@email", "registerPasswordPolicy", "pass", "pass");
 
             registerPage.assertCurrent();
             Assert.assertEquals("Invalid password: minimum length 8", registerPage.getError());
 
-            events.expectRegister("registerPasswordPolicy", "registerPasswordPolicyemail").user((String) null).error("invalid_registration").assertEvent();
+            events.expectRegister("registerPasswordPolicy", "registerPasswordPolicy@email").user((String) null).error("invalid_registration").assertEvent();
 
-            registerPage.register("firstName", "lastName", "registerPasswordPolicyemail", "registerPasswordPolicy", "password", "password");
+            registerPage.register("firstName", "lastName", "registerPasswordPolicy@email", "registerPasswordPolicy", "password", "password");
             Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
 
-            String userId = events.expectRegister("registerPasswordPolicy", "registerPasswordPolicyemail").assertEvent().getUserId();
+            String userId = events.expectRegister("registerPasswordPolicy", "registerPasswordPolicy@email").assertEvent().getUserId();
 
             events.expectLogin().user(userId).detail(Details.USERNAME, "registerPasswordPolicy").assertEvent();
         } finally {
@@ -154,12 +154,29 @@ public class RegisterTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "registerUserMissingUsernameemail", null, "password", "password");
+        registerPage.register("firstName", "lastName", "registerUserMissingUsername@email", null, "password", "password");
 
         registerPage.assertCurrent();
         Assert.assertEquals("Please specify username", registerPage.getError());
 
-        events.expectRegister(null, "registerUserMissingUsernameemail").removeDetail("username").error("invalid_registration").assertEvent();
+        events.expectRegister(null, "registerUserMissingUsername@email").removeDetail("username").error("invalid_registration").assertEvent();
+    }
+
+    @Test
+    public void registerUserMissingOrInvalidEmail() {
+        loginPage.open();
+        loginPage.clickRegister();
+        registerPage.assertCurrent();
+
+        registerPage.register("firstName", "lastName", null, "registerUserMissingEmail", "password", "password");
+        registerPage.assertCurrent();
+        Assert.assertEquals("Please specify email", registerPage.getError());
+        events.expectRegister("registerUserMissingEmail", null).removeDetail("email").error("invalid_registration").assertEvent();
+
+        registerPage.register("firstName", "lastName", "registerUserInvalidEmailemail", "registerUserInvalidEmail", "password", "password");
+        registerPage.assertCurrent();
+        Assert.assertEquals("Invalid email address", registerPage.getError());
+        events.expectRegister("registerUserInvalidEmail", "registerUserInvalidEmailemail").error("invalid_registration").assertEvent();
     }
 
     @Test
@@ -168,11 +185,11 @@ public class RegisterTest {
         loginPage.clickRegister();
         registerPage.assertCurrent();
 
-        registerPage.register("firstName", "lastName", "registerUserSuccessemail", "registerUserSuccess", "password", "password");
+        registerPage.register("firstName", "lastName", "registerUserSuccess@email", "registerUserSuccess", "password", "password");
 
         Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
 
-        String userId = events.expectRegister("registerUserSuccess", "registerUserSuccessemail").assertEvent().getUserId();
+        String userId = events.expectRegister("registerUserSuccess", "registerUserSuccess@email").assertEvent().getUserId();
         events.expectLogin().detail("username", "registerUserSuccess").user(userId).assertEvent();
     }