validate new user name, email, password in update api

backend/src/main/scala/com/softwaremill/bootzooka/user/UserService.scala

@@ -54,9 +54,7 @@ class UserService(
     }
 
     for {
-      _ <- UserRegisterValidator
-        .validate(login, email, password)
-        .fold(msg => Fail.IncorrectInput(msg).raiseError[ConnectionIO, Unit], _ => ().pure[ConnectionIO])
+      _ <- UserValidator(Some(login), Some(email), Some(password)).asTask()
       _ <- checkUserDoesNotExist()
       apiKey <- doRegister()
     } yield apiKey
@@ -78,22 +76,34 @@ class UserService(
         case Some(user) if user.id != userId => Fail.IncorrectInput(LoginAlreadyUsed).raiseError[ConnectionIO, Boolean]
         case Some(user) if user.login == newLogin => false.pure[ConnectionIO]
         case _ =>
-          logger.debug(s"Changing login for user: $userId, to: $newLogin")
-          userModel.updateLogin(userId, newLogin, newLoginLowerCased).map(_ => true)
+          for {
+            _ <- validateLogin(newLogin)
+            _ = logger.debug(s"Changing login for user: $userId, to: $newLogin")
+            _ <- userModel.updateLogin(userId, newLogin, newLoginLowerCased)
+          } yield true
       }
     }
 
+    def validateLogin(newLogin: String) =
+      UserValidator(Some(newLogin), None, None).asTask()
+
     def changeEmail(newEmail: String): ConnectionIO[Boolean] = {
       val newEmailLowerCased = newEmail.lowerCased
       userModel.findByEmail(newEmailLowerCased).flatMap {
         case Some(user) if user.id != userId => Fail.IncorrectInput(EmailAlreadyUsed).raiseError[ConnectionIO, Boolean]
         case Some(user) if user.emailLowerCased == newEmailLowerCased => false.pure[ConnectionIO]
         case _ =>
-          logger.debug(s"Changing email for user: $userId, to: $newEmail")
-          userModel.updateEmail(userId, newEmailLowerCased).map(_ => true)
+          for {
+            _ <- validateEmail(newEmailLowerCased)
+            _ = logger.debug(s"Changing email for user: $userId, to: $newEmail")
+            _ <- userModel.updateEmail(userId, newEmailLowerCased)
+          } yield true
       }
     }
 
+    def validateEmail(newEmailLowerCased: String) =
+      UserValidator(None, Some(newEmailLowerCased), None).asTask()
+
     def doChange(newLogin: String, newEmail: String): ConnectionIO[Boolean] = {
       for {
         loginUpdated <- changeLogin(newLogin)
@@ -119,6 +129,7 @@ class UserService(
     for {
       user <- userOrNotFound(userModel.findById(userId))
       _ <- verifyPassword(user, currentPassword, validationErrorMsg = "Incorrect current password")
+      _ <- validatePassword(newPassword)
       _ = logger.debug(s"Changing password for user: $userId")
       _ <- userModel.updatePassword(userId, User.hashPassword(newPassword))
       confirmationEmail = emailTemplates.passwordChangeNotification(user.login)
@@ -139,28 +150,50 @@ class UserService(
       Fail.Unauthorized(validationErrorMsg).raiseError[ConnectionIO, Unit]
     }
   }
+
+  private def validatePassword(password: String) =
+    UserValidator(None, None, Some(password)).asTask()
 }
 
-object UserRegisterValidator {
-  private val ValidationOk = Right(())
+object UserValidator {
   val MinLoginLength = 3
+}
+
+case class UserValidator(loginOpt: Option[String], emailOpt: Option[String], passwordOpt: Option[String]) {
+  private val ValidationOk = Right(())
+
+  private val emailRegex =
+    """^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$""".r
 
-  def validate(login: String, email: String, password: String): Either[String, Unit] =
+  val result: Either[String, Unit] = {
     for {
-      _ <- validLogin(login.trim)
-      _ <- validEmail(email.trim)
-      _ <- validPassword(password.trim)
+      _ <- validateLogin(loginOpt)
+      _ <- validateEmail(emailOpt)
+      _ <- validatePassword(passwordOpt)
     } yield ()
+  }
 
-  private def validLogin(login: String): Either[String, Unit] =
-    if (login.length >= MinLoginLength) ValidationOk else Left("Login is too short!")
+  def asTask(): ConnectionIO[Unit] =
+    result.fold(msg => Fail.IncorrectInput(msg).raiseError[ConnectionIO, Unit], _ => ().pure[ConnectionIO])
 
-  private val emailRegex =
-    """^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$""".r
+  private def validateLogin(loginOpt: Option[String]): Either[String, Unit] =
+    loginOpt.map(_.trim) match {
+      case Some(login) =>
+        if (login.length >= UserValidator.MinLoginLength) ValidationOk else Left("Login is too short!")
+      case None => ValidationOk
+    }
 
-  private def validEmail(email: String) =
-    if (emailRegex.findFirstMatchIn(email).isDefined) ValidationOk else Left("Invalid e-mail format!")
+  private def validateEmail(emailOpt: Option[String]): Either[String, Unit] =
+    emailOpt.map(_.trim) match {
+      case Some(email) =>
+        if (emailRegex.findFirstMatchIn(email).isDefined) ValidationOk else Left("Invalid e-mail format!")
+      case None => ValidationOk
+    }
 
-  private def validPassword(password: String) =
-    if (password.nonEmpty) ValidationOk else Left("Password cannot be empty!")
+  private def validatePassword(passwordOpt: Option[String]): Either[String, Unit] =
+    passwordOpt.map(_.trim) match {
+      case Some(password) =>
+        if (password.nonEmpty) ValidationOk else Left("Password cannot be empty!")
+      case None => ValidationOk
+    }
 }

backend/src/test/scala/com/softwaremill/bootzooka/user/UserApiTest.scala

@@ -185,6 +185,22 @@ class UserApiTest extends BaseTest with TestEmbeddedPostgres with Eventually {
     loginUser(login, newPassword, None).status shouldBe Status.Unauthorized
   }
 
+  "/user/changepassword" should "not change the password if the new password is invalid" in {
+    // given
+    val RegisteredUser(login, _, password, apiKey) = newRegisteredUsed()
+    val newPassword = ""
+
+    // when
+    val response1 = changePassword(apiKey, password, newPassword)
+
+    // then
+    response1.status shouldBe Status.BadRequest
+    response1.shouldDeserializeToError shouldBe "Password cannot be empty!"
+
+    loginUser(login, password, None).status shouldBe Status.Ok
+    loginUser(login, newPassword, None).status shouldBe Status.Unauthorized
+  }
+
   "/user" should "update the login" in {
     // given
     val RegisteredUser(login, email, _, apiKey) = newRegisteredUsed()
@@ -199,6 +215,22 @@ class UserApiTest extends BaseTest with TestEmbeddedPostgres with Eventually {
     getUser(apiKey).shouldDeserializeTo[GetUser_OUT].email shouldBe email
   }
 
+  "/user" should "update the login if the new login is invalid" in {
+    // given
+    val RegisteredUser(login, email, _, apiKey) = newRegisteredUsed()
+    val newLogin = "a"
+
+    // when
+    val response1 = updateUser(apiKey, newLogin, email)
+
+    // then
+    response1.status shouldBe Status.BadRequest
+    response1.shouldDeserializeToError shouldBe "Login is too short!"
+
+    getUser(apiKey).shouldDeserializeTo[GetUser_OUT].login shouldBe login
+    getUser(apiKey).shouldDeserializeTo[GetUser_OUT].email shouldBe email
+  }
+
   "/user" should "update the email" in {
     // given
     val RegisteredUser(login, _, _, apiKey) = newRegisteredUsed()
@@ -212,4 +244,20 @@ class UserApiTest extends BaseTest with TestEmbeddedPostgres with Eventually {
     getUser(apiKey).shouldDeserializeTo[GetUser_OUT].login shouldBe login
     getUser(apiKey).shouldDeserializeTo[GetUser_OUT].email shouldBe newEmail
   }
+
+  "/user" should "not update the email if the new email is invalid" in {
+    // given
+    val RegisteredUser(login, email, _, apiKey) = newRegisteredUsed()
+    val newEmail = "aaa"
+
+    // when
+    val response1 = updateUser(apiKey, login, newEmail)
+
+    // then
+    response1.status shouldBe Status.BadRequest
+    response1.shouldDeserializeToError shouldBe "Invalid e-mail format!"
+
+    getUser(apiKey).shouldDeserializeTo[GetUser_OUT].login shouldBe login
+    getUser(apiKey).shouldDeserializeTo[GetUser_OUT].email shouldBe email
+  }
 }

backend/src/test/scala/com/softwaremill/bootzooka/user/UserValidatorSpec.scala

@@ -0,0 +1,53 @@
+package com.softwaremill.bootzooka.user
+
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class UserValidatorSpec extends AnyFlatSpec with Matchers {
+  private def validate(userName: String, email: String, password: String) =
+    UserValidator(Some(userName), Some(email), Some(password)).result
+
+  "validate" should "accept valid data" in {
+    val dataIsValid = validate("login", "admin@bootzooka.com", "password")
+
+    dataIsValid shouldBe Right(())
+  }
+
+  "validate" should "not accept login containing only empty spaces" in {
+    val dataIsValid = validate("   ", "admin@bootzooka.com", "password")
+
+    dataIsValid.isLeft shouldBe true
+  }
+
+  "validate" should "not accept too short login" in {
+    val tooShortLogin = "a" * (UserValidator.MinLoginLength - 1)
+    val dataIsValid   = validate(tooShortLogin, "admin@bootzooka.com", "password")
+
+    dataIsValid.isLeft shouldBe true
+  }
+
+  "validate" should "not accept too short login after trimming" in {
+    val loginTooShortAfterTrim = "a" * (UserValidator.MinLoginLength - 1) + "   "
+    val dataIsValid            = validate(loginTooShortAfterTrim, "admin@bootzooka.com", "password")
+
+    dataIsValid.isLeft shouldBe true
+  }
+
+  "validate" should "not accept missing email with spaces only" in {
+    val dataIsValid = validate("login", "   ", "password")
+
+    dataIsValid.isLeft shouldBe true
+  }
+
+  "validate" should "not accept invalid email" in {
+    val dataIsValid = validate("login", "invalidEmail", "password")
+
+    dataIsValid.isLeft shouldBe true
+  }
+
+  "validate" should "not accept password with empty spaces only" in {
+    val dataIsValid = validate("login", "admin@bootzooka.com", "    ")
+
+    dataIsValid.isLeft shouldBe true
+  }
+}