[HWORKS-869] Email validation regex doesn't support capital letters (…

…#1624)

hopsworks-IT/src/test/ruby/spec/admin_users_spec.rb

@@ -286,6 +286,16 @@
         expect(json_body[:status]).to be == 2
         expect(json_body[:password]).to be_nil
       end
+      it "should register new user with capital letters (should lowercase before saving)" do
+        email = "#{random_id}@EMAIL.com"
+        register_user_as_admin(email, "name", "last", password: "Pass123", maxNumProjects: "5",
+                               status: "ACTIVATED_ACCOUNT")
+        expect_status_details(201)
+        expect(json_body[:maxNumProjects]).to be == 5
+        expect(json_body[:status]).to be == 2
+        expect(json_body[:password]).to be_nil
+        expect(json_body[:email]).to eq(email.downcase)
+      end
       it "should add new activated user to kube config map" do
         if !kserve_installed
           skip "This test only runs with KServe installed"

hopsworks-api/src/main/java/io/hops/hopsworks/api/admin/UsersAdminResource.java

@@ -16,6 +16,7 @@
 
 package io.hops.hopsworks.api.admin;
 
+import com.google.common.base.Strings;
 import io.hops.hopsworks.api.admin.dto.NewUserDTO;
 import io.hops.hopsworks.api.filter.Audience;
 import io.hops.hopsworks.api.filter.apiKey.ApiKeyRequired;
@@ -296,24 +297,29 @@ public Response registerUser(@QueryParam("accountType") UserAccountType accountT
   }
 
   private Response createUser(String email, String password, String givenName, String surname, int maxNumProjects,
-    String role, UserAccountStatus status, UriInfo uriInfo) throws UserException {
+                              String role, UserAccountStatus status, UriInfo uriInfo) throws UserException {
     UsersDTO newUser = new UsersDTO();
     newUser.setEmail(email);
     newUser.setFirstName(givenName);
     newUser.setLastName(surname);
     newUser.setMaxNumProjects(maxNumProjects > 0 ? maxNumProjects : settings.getMaxNumProjPerUser());
-    String passwordGen = password != null && !password.isEmpty()? password :
-      securityUtils.generateRandomString(UserValidator.TEMP_PASSWORD_LENGTH);
+    newUser.setTos(true);
+
+    String passwordGen = !Strings.isNullOrEmpty(password) ? password :
+        securityUtils.generateRandomString(UserValidator.TEMP_PASSWORD_LENGTH);
     newUser.setChosenPassword(passwordGen);
     newUser.setRepeatedPassword(passwordGen);
-    newUser.setTos(true);
-    userValidator.isValidNewUser(newUser, passwordGen.equals(password));
+
+    userValidator.isValidNewUser(newUser, !Strings.isNullOrEmpty(password));
+
     UserAccountStatus statusDefault = status != null ? status : UserAccountStatus.TEMP_PASSWORD;
-    Users user = usersController.registerUser(newUser, role != null ? role : Settings.DEFAULT_ROLE, statusDefault,
-      UserAccountType.M_ACCOUNT_TYPE);
+    Users user = usersController.registerUser(newUser,
+        role != null ? role : Settings.DEFAULT_ROLE, statusDefault,
+        UserAccountType.M_ACCOUNT_TYPE);
+
     NewUserDTO newUserDTO = new NewUserDTO(user);
     URI href = uriInfo.getAbsolutePathBuilder().path(user.getUid().toString()).build();
-    if (!passwordGen.equals(password)) {
+    if (Strings.isNullOrEmpty(password)) {
       newUserDTO.setPassword(passwordGen);
     }
     return Response.created(href).entity(newUserDTO).build();

hopsworks-common/pom.xml

@@ -197,6 +197,11 @@
       <artifactId>javase</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>commons-validator</groupId>
+      <artifactId>commons-validator</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>fish.payara.extras</groupId>
       <artifactId>payara-embedded-web</artifactId>

hopsworks-common/src/main/java/io/hops/hopsworks/common/security/utils/SecurityUtils.java

@@ -118,8 +118,7 @@ public Secret generateSecret() {
    */
   public Secret generateSecret(String password) {
     String salt = generateSecureRandomString(RANDOM_KEY_LEN);
-    Secret secret = new Secret(password, salt, UserValidator.PASSWORD_MIN_LENGTH, RANDOM_KEY_LEN);
-    return secret;
+    return new Secret(password, salt, UserValidator.PASSWORD_MIN_LENGTH, RANDOM_KEY_LEN);
   }
 
   /**

hopsworks-common/src/main/java/io/hops/hopsworks/common/user/UserValidator.java

@@ -39,18 +39,20 @@
 
 package io.hops.hopsworks.common.user;
 
-import com.google.common.base.Strings;
 import io.hops.hopsworks.common.dao.user.UserFacade;
 import io.hops.hopsworks.common.dao.user.UsersDTO;
 import io.hops.hopsworks.restutils.RESTCodes;
 import io.hops.hopsworks.exceptions.UserException;
 
+import org.apache.commons.validator.routines.EmailValidator;
+
 import javax.ejb.EJB;
 import javax.ejb.Stateless;
 import java.util.logging.Level;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+
 @Stateless
 public class UserValidator {
 
@@ -61,18 +63,11 @@ public class UserValidator {
   public static final int TEMP_PASSWORD_LENGTH = 8;
   public static final int PASSWORD_MAX_LENGTH = 255;
   private static final String PASSWORD_PATTERN = "(?=.*[a-z])(?=.*[A-Z])(?=.*[\\d\\W]).*$";
-  private static final String EMAIL_PATTERN = "[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)"
-    + "*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?";
 
-  public boolean isValidEmail(String email) throws UserException {
-    if (Strings.isNullOrEmpty(email)) {
-      throw new IllegalArgumentException("Email was not provided");
-    }
-    if (!isValid(email, EMAIL_PATTERN)) {
+  public void isValidEmail(String email) throws UserException {
+    if (!EmailValidator.getInstance().isValid(email)) {
       throw new UserException(RESTCodes.UserErrorCode.INVALID_EMAIL, Level.FINE);
     }
-
-    return true;
   }
 
   public boolean isValidPassword(String password, String confirmedPassword) throws UserException {

hopsworks-common/src/main/java/io/hops/hopsworks/common/user/UsersController.java

@@ -123,9 +123,6 @@ public class UsersController {
   private Instance<UserAccountHandler> userAccountHandlers;
 
   public QrCode registerUser(UsersDTO newUser, String validationKeyUrl) throws UserException {
-    if (newUser.getEmail() != null && !newUser.getEmail().isEmpty()) {
-      newUser.setEmail(newUser.getEmail().toLowerCase());
-    }
     userValidator.isValidNewUser(newUser);
     // if setting is changed user should still be able to use her device
     if (!Strings.isNullOrEmpty(settings.getTwoFactorAuth()) &&
@@ -155,15 +152,8 @@ public QrCode registerUser(UsersDTO newUser, String validationKeyUrl) throws Use
     return qrCode;
   }
 
-  public Users registerUser(UsersDTO newUser, String role, UserAccountStatus accountStatus, UserAccountType accountType)
-    throws UserException {
-    if (!Strings.isNullOrEmpty(newUser.getEmail())) {
-      newUser.setEmail(newUser.getEmail().toLowerCase());
-    }
-    userValidator.isValidEmail(newUser.getEmail());
-    if (userFacade.findByEmail(newUser.getEmail()) != null) {
-      throw new UserException(RESTCodes.UserErrorCode.USER_EXISTS, Level.FINE);
-    }
+  public Users registerUser(UsersDTO newUser, String role,
+                            UserAccountStatus accountStatus, UserAccountType accountType) throws UserException {
     Users user = createNewUser(newUser, accountStatus, accountType);
     BbcGroup group = bbcGroupFacade.findByGroupName(role);
     if (group != null) {
@@ -253,7 +243,9 @@ public Users createNewUser(UsersDTO newUser, UserAccountStatus accountStatus, Us
     Timestamp now = new Timestamp(new Date().getTime());
     int maxNumProjects = newUser.getMaxNumProjects() > 0? newUser.getMaxNumProjects() : settings.getMaxNumProjPerUser();
 
-    Users user = new Users(uname, secret.getSha256HexDigest(), newUser.getEmail(), newUser.getFirstName(),
+    // Here we need to enforce that the email address is lowercase
+    Users user = new Users(uname, secret.getSha256HexDigest(), newUser.getEmail().toLowerCase(),
+        newUser.getFirstName(),
         newUser.getLastName(), now, "-", accountStatus, otpSecret, activationKey,
         now, ValidationKeyType.EMAIL, accountType, now, maxNumProjects, newUser.isTwoFactor(),
         secret.getSalt(), newUser.getToursState());

hopsworks-common/src/test/io/hops/hopsworks/common/user/TestUserValidator.java

@@ -0,0 +1,46 @@
+/*
+ * This file is part of Hopsworks
+ * Copyright (C) 2023, Hopsworks AB. All rights reserved
+ *
+ * Hopsworks is free software: you can redistribute it and/or modify it under the terms of
+ * the GNU Affero General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * Hopsworks is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License along with this program.
+ * If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package io.hops.hopsworks.common.user;
+
+import io.hops.hopsworks.exceptions.UserException;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestUserValidator {
+
+  private UserValidator userValidator = new UserValidator();
+
+  @Test
+  public void testIsValidEmail_emtpyEmail() {
+    Assert.assertThrows(UserException.class, () -> userValidator.isValidEmail(""));
+  }
+
+  @Test
+  public void testIsValidEmail_lowercaseEmail() throws UserException{
+    userValidator.isValidEmail("[email protected]");
+  }
+
+  @Test
+  public void testIsValidEmail_uppercaseEmail() throws UserException{
+    userValidator.isValidEmail("[email protected]");
+  }
+
+  @Test
+  public void testIsValidEmail_othercharsEmail() throws UserException{
+    userValidator.isValidEmail("[email protected]");
+  }
+}

hopsworks-common/src/test/io/hops/hopsworks/common/user/TestUsersController.java

@@ -0,0 +1,65 @@
+/*
+ * This file is part of Hopsworks
+ * Copyright (C) 2023, Hopsworks AB. All rights reserved
+ *
+ * Hopsworks is free software: you can redistribute it and/or modify it under the terms of
+ * the GNU Affero General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * Hopsworks is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License along with this program.
+ * If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package io.hops.hopsworks.common.user;
+
+import io.hops.hopsworks.common.dao.user.UserFacade;
+import io.hops.hopsworks.common.dao.user.UsersDTO;
+import io.hops.hopsworks.common.security.utils.SecurityUtils;
+import io.hops.hopsworks.persistence.entity.user.security.ua.UserAccountStatus;
+import io.hops.hopsworks.persistence.entity.user.security.ua.UserAccountType;
+import io.hops.hopsworks.persistence.entity.user.Users;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+
+public class TestUsersController {
+
+  @InjectMocks
+  private UsersController usersController = new UsersController();
+
+  @Mock
+  private SecurityUtils securityUtils;
+
+  @Mock
+  private UserFacade userFacade;
+
+  @Before
+  public void setup() {
+    MockitoAnnotations.openMocks(this);
+    Mockito.when(securityUtils.generateSecret(Mockito.any())).thenCallRealMethod();
+  }
+
+  @Test
+  public void testCreateUserEmailLowercase() {
+    String email = "[email protected]";
+    UsersDTO userDTO = new UsersDTO();
+    userDTO.setEmail(email);
+    userDTO.setFirstName("first");
+    userDTO.setLastName("last");
+    userDTO.setTwoFactor(true);
+    userDTO.setChosenPassword("lolololololo");
+    userDTO.setMaxNumProjects(1);
+    Users user =
+        usersController.createNewUser(userDTO, UserAccountStatus.VERIFIED_ACCOUNT, UserAccountType.M_ACCOUNT_TYPE);
+    Assert.assertEquals(email.toLowerCase(), user.getEmail());
+  }
+
+}

hopsworks-persistence/src/main/java/io/hops/hopsworks/persistence/entity/user/Users.java

@@ -161,10 +161,6 @@ public class Users implements Serializable {
       max = 128)
   @Column(name = "password")
   private String password;
-  // @Pattern(regexp="[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)
-  //*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?",
-  //message="Invalid email")//if the field contains email address consider using 
-  //this annotation to enforce field validation need <validation-mode>AUTO</validation-mode>
   @Size(max = 254)
   @Column(name = "email")
   private String email;

pom.xml

@@ -97,6 +97,7 @@
     <commons-io.version>2.11.0</commons-io.version>
     <commons-logging.version>1.2</commons-logging.version>
     <commons.lang3.version>3.8.1</commons.lang3.version>
+    <commons-validator.version>1.7</commons-validator.version>
     <payara.extras.version>5.2022.5</payara.extras.version>
     <ehcache.version>3.7.0</ehcache.version>
     <opensearch-rest-high-level-client.version>1.3.3</opensearch-rest-high-level-client.version>
@@ -265,6 +266,11 @@
         <artifactId>commons-logging</artifactId>
         <version>${commons-logging.version}</version>
       </dependency>
+      <dependency>
+        <groupId>commons-validator</groupId>
+        <artifactId>commons-validator</artifactId>
+        <version>${commons-validator.version}</version>
+      </dependency>
       <dependency>
         <groupId>fish.payara.extras</groupId>
         <artifactId>payara-embedded-web</artifactId>