feat(registration): do not expose email is taken (#376)

FAForever · Jan 23, 2025 · 74832ad · 74832ad
1 parent 37399a6
commit 74832ad
Show file tree

Hide file tree

Showing 14 changed files with 164 additions and 27 deletions.
diff --git a/src/main/bundles/dev.bundle b/src/main/bundles/dev.bundle
diff --git a/src/main/kotlin/com/faforever/userservice/backend/account/RegistrationService.kt b/src/main/kotlin/com/faforever/userservice/backend/account/RegistrationService.kt
@@ -22,8 +22,10 @@ enum class UsernameStatus {
     USERNAME_TAKEN, USERNAME_RESERVED, USERNAME_AVAILABLE,
 }
 
-enum class EmailStatus {
-    EMAIL_TAKEN, EMAIL_BLACKLISTED, EMAIL_AVAILABLE,
+sealed interface EmailStatusResponse {
+    data object EmailBlackListed : EmailStatusResponse
+    data object EmailAvailable : EmailStatusResponse
+    data class EmailTaken(val existingUsername: String) : EmailStatusResponse
 }
 
 data class RegisteredUser(
@@ -49,10 +51,22 @@ class RegistrationService(
     }
 
     fun register(username: String, email: String) {
-        checkUsernameAndEmail(username, email)
+        checkUsername(username)
+
+        when (val emailStatus = emailAvailable(email)) {
+            is EmailStatusResponse.EmailBlackListed -> throw IllegalStateException("Email provider is blacklisted")
+            is EmailStatusResponse.EmailTaken -> onEmailTaken(username, emailStatus.existingUsername, email)
+            is EmailStatusResponse.EmailAvailable -> {
+                sendActivationEmail(username, email)
+                metricHelper.incrementUserRegistrationCounter()
+            }
+        }
+    }
 
-        sendActivationEmail(username, email)
-        metricHelper.incrementUserRegistrationCounter()
+    private fun onEmailTaken(desiredUsername: String, existingUsername: String, email: String) {
+        val passwordResetUrl =
+            fafProperties.account().passwordReset().passwordResetInitiateEmailUrlFormat().format(email)
+        emailService.sendEmailAlreadyTakenMail(desiredUsername, existingUsername, email, passwordResetUrl)
     }
 
     private fun sendActivationEmail(username: String, email: String) {
@@ -84,14 +98,14 @@ class RegistrationService(
     }
 
     @Transactional
-    fun emailAvailable(email: String): EmailStatus {
+    fun emailAvailable(email: String): EmailStatusResponse {
         val onBlacklist = domainBlacklistRepository.existsByDomain(email.substring(email.lastIndexOf('@') + 1))
         if (onBlacklist) {
-            return EmailStatus.EMAIL_BLACKLISTED
+            return EmailStatusResponse.EmailBlackListed
         }
 
-        val exists = userRepository.existsByEmail(email)
-        return if (exists) EmailStatus.EMAIL_TAKEN else EmailStatus.EMAIL_AVAILABLE
+        val user = userRepository.findByEmail(email)
+        return if (user != null) EmailStatusResponse.EmailTaken(user.username) else EmailStatusResponse.EmailAvailable
     }
 
     fun validateRegistrationToken(registrationToken: String): RegisteredUser {
@@ -115,7 +129,9 @@ class RegistrationService(
         val email = registeredUser.email
         val encodedPassword = passwordEncoder.encode(password)
 
-        checkUsernameAndEmail(username, email)
+        checkUsername(username)
+        val emailStatus = emailAvailable(email)
+        require(emailStatus is EmailStatusResponse.EmailAvailable) { "Email unavailable" }
 
         val user = User(
             username = username,
@@ -134,15 +150,8 @@ class RegistrationService(
         return user
     }
 
-    private fun checkUsernameAndEmail(username: String, email: String) {
+    private fun checkUsername(username: String) {
         val usernameStatus = usernameAvailable(username)
-        if (usernameStatus != UsernameStatus.USERNAME_AVAILABLE) {
-            throw IllegalArgumentException("Username unavailable")
-        }
-
-        val emailStatus = emailAvailable(email)
-        if (emailStatus != EmailStatus.EMAIL_AVAILABLE) {
-            throw IllegalArgumentException("Email unavailable")
-        }
+        require(usernameStatus == UsernameStatus.USERNAME_AVAILABLE) { "Username unavailable" }
     }
 }
diff --git a/src/main/kotlin/com/faforever/userservice/backend/domain/User.kt b/src/main/kotlin/com/faforever/userservice/backend/domain/User.kt
@@ -64,6 +64,9 @@ class UserRepository : PanacheRepositoryBase<User, Int> {
     fun findByUsernameOrEmail(usernameOrEmail: String): User? =
         find("username = ?1 or email = ?1", usernameOrEmail).firstResult()
 
+    fun findByEmail(email: String): User? =
+        find("email = ?1", email).firstResult()
+
     fun findUserPermissions(userId: Int): List<Permission> =
         getEntityManager().createNativeQuery(
             """

diff --git a/src/main/kotlin/com/faforever/userservice/backend/email/EmailService.kt b/src/main/kotlin/com/faforever/userservice/backend/email/EmailService.kt
@@ -66,4 +66,14 @@ class EmailService(
         val mailBody = mailBodyBuilder.buildPasswordResetBody(username, passwordResetUrl)
         mailSender.sendMail(email, properties.account().passwordReset().subject(), mailBody, ContentType.HTML)
     }
+
+    fun sendEmailAlreadyTakenMail(
+        desiredUsername: String,
+        existingUsername: String,
+        email: String,
+        passwordResetUrl: String,
+    ) {
+        val mailBody = mailBodyBuilder.buildEmailTakenBody(desiredUsername, existingUsername, passwordResetUrl)
+        mailSender.sendMail(email, properties.account().registration().emailTakenSubject(), mailBody, ContentType.HTML)
+    }
 }
diff --git a/src/main/kotlin/com/faforever/userservice/backend/email/MailBodyBuilder.kt b/src/main/kotlin/com/faforever/userservice/backend/email/MailBodyBuilder.kt
@@ -22,6 +22,7 @@ class MailBodyBuilder(private val properties: FafProperties) {
         ACCOUNT_ACTIVATION("username", "activationUrl"),
         WELCOME_TO_FAF("username"),
         PASSWORD_RESET("username", "passwordResetUrl"),
+        EMAIL_ALREADY_TAKEN("desiredUsername", "existingUsername", "passwordResetUrl"),
         ;
 
         val variables: Set<String>
@@ -36,6 +37,7 @@ class MailBodyBuilder(private val properties: FafProperties) {
             Template.ACCOUNT_ACTIVATION -> properties.account().registration().activationMailTemplatePath()
             Template.WELCOME_TO_FAF -> properties.account().registration().welcomeMailTemplatePath()
             Template.PASSWORD_RESET -> properties.account().passwordReset().mailTemplatePath()
+            Template.EMAIL_ALREADY_TAKEN -> properties.account().registration().emailTakenMailTemplatePath()
         }
         return Path.of(path)
     }
@@ -125,4 +127,14 @@ class MailBodyBuilder(private val properties: FafProperties) {
                 "passwordResetUrl" to passwordResetUrl,
             ),
         )
+
+    fun buildEmailTakenBody(desiredUsername: String, existingUsername: String, passwordResetUrl: String) =
+        populate(
+            Template.EMAIL_ALREADY_TAKEN,
+            mapOf(
+                "desiredUsername" to desiredUsername,
+                "existingUsername" to existingUsername,
+                "passwordResetUrl" to passwordResetUrl,
+            ),
+        )
 }
diff --git a/src/main/kotlin/com/faforever/userservice/config/FafProperties.kt b/src/main/kotlin/com/faforever/userservice/config/FafProperties.kt
@@ -6,7 +6,7 @@ import io.smallrye.config.WithName
 import jakarta.validation.constraints.NotBlank
 import jakarta.validation.constraints.NotNull
 import java.net.URI
-import java.util.*
+import java.util.Optional
 
 @ConfigMapping(prefix = "faf")
 interface FafProperties {
@@ -106,6 +106,12 @@ interface FafProperties {
             @NotBlank
             fun welcomeMailTemplatePath(): String
 
+            @NotBlank
+            fun emailTakenMailTemplatePath(): String
+
+            @NotBlank
+            fun emailTakenSubject(): String
+
             @NotBlank
             fun termsOfServiceUrl(): String
 
@@ -123,6 +129,9 @@ interface FafProperties {
             @NotBlank
             fun passwordResetUrlFormat(): String
 
+            @NotBlank
+            fun passwordResetInitiateEmailUrlFormat(): String
+
             @NotBlank
             fun subject(): String
 

diff --git a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaEmailView.kt b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaEmailView.kt
@@ -17,12 +17,14 @@ import com.vaadin.flow.component.orderedlayout.HorizontalLayout
 import com.vaadin.flow.component.orderedlayout.VerticalLayout
 import com.vaadin.flow.component.textfield.TextField
 import com.vaadin.flow.data.value.ValueChangeMode
+import com.vaadin.flow.router.BeforeEnterEvent
+import com.vaadin.flow.router.BeforeEnterObserver
 import com.vaadin.flow.router.Route
 
 @Route("/recover-account/email", layout = CardLayout::class)
 class RecoverViaEmailView(
     private val recoveryService: RecoveryService,
-) : CompactVerticalLayout() {
+) : CompactVerticalLayout(), BeforeEnterObserver {
 
     private val usernameOrEmailDescription =
         Paragraph(
@@ -81,4 +83,13 @@ class RecoverViaEmailView(
             open()
         }
     }
+
+    override fun beforeEnter(event: BeforeEnterEvent?) {
+        val possibleIdentifier = event?.location?.queryParameters?.parameters?.get("identifier")?.get(0)
+        if (!possibleIdentifier.isNullOrBlank()) {
+            usernameOrEmail.value = possibleIdentifier
+            usernameOrEmail.isReadOnly = true
+            requestEmail()
+        }
+    }
 }
diff --git a/src/main/kotlin/com/faforever/userservice/ui/view/registration/RegisterView.kt b/src/main/kotlin/com/faforever/userservice/ui/view/registration/RegisterView.kt
@@ -1,6 +1,6 @@
 package com.faforever.userservice.ui.view.registration
 
-import com.faforever.userservice.backend.account.EmailStatus
+import com.faforever.userservice.backend.account.EmailStatusResponse
 import com.faforever.userservice.backend.account.RegistrationService
 import com.faforever.userservice.backend.account.UsernameStatus
 import com.faforever.userservice.backend.recaptcha.RecaptchaService
@@ -141,7 +141,7 @@ class RegisterView(
             ).bind("username")
 
         binder.forField(email).withValidator(EmailValidator(getTranslation("register.email.invalid"))).withValidator(
-            { email -> registrationService.emailAvailable(email) != EmailStatus.EMAIL_BLACKLISTED },
+            { email -> registrationService.emailAvailable(email) !is EmailStatusResponse.EmailBlackListed },
             getTranslation("register.email.blacklisted"),
         ).bind("email")
 

diff --git a/src/main/mjml/dummy/test-email-taken.html b/src/main/mjml/dummy/test-email-taken.html
@@ -0,0 +1,5 @@
+To generate proper html code use the template in src/main/mjml with an mjml parser (e.g. use the IntelliJ plugin for MJML or follow the instructions on https://documentation.mjml.io/#usage
+Following variables are available:
+desiredUsername: {{desiredUsername}}
+existingUsername: {{existingUsername}}
+passwordResetUrl: {{passwordResetUrl}}
diff --git a/src/main/mjml/email-taken.mjml b/src/main/mjml/email-taken.mjml
@@ -0,0 +1,60 @@
+<mjml>
+  <mj-head>
+    <mj-include path="_style.mjml"/>
+    <mj-preview>
+      Account exists notice
+    </mj-preview>
+  </mj-head>
+  <mj-body background-color="#fafafa">
+    <mj-include path="_header.mjml"/>
+
+    <mj-section background-color="#ffffff">
+      <mj-column>
+        <mj-text>
+          <h1>Account exists notice</h1>
+          <p>Dear {{existingUsername}},</p>
+          <p>We noticed someone tried to register an account with the username "{{desiredUsername}}"
+            using your email address. However, your email is already associated with an existing
+            account.
+          </p>
+        </mj-text>
+      </mj-column>
+    </mj-section>
+
+    <mj-section>
+      <mj-column>
+        <mj-text>
+          <p>According to our rules each user may have only have one (1) FAF account.
+            Maybe you forgot, that you already registered in the past, thus we send you this notice.
+          </p>
+          <p>If you have no more access to this account, you can reset the password right here:</p>
+        </mj-text>
+        <mj-button href="{{passwordResetUrl}}">
+          Reset Password
+        </mj-button>
+      </mj-column>
+    </mj-section>
+
+    <mj-section background-color="gray" padding="0">
+      <mj-column>
+        <mj-text color="#d3d3d3">
+          <p>If you did not attempt to register a new account, please ignore this email.</p>
+        </mj-text>
+      </mj-column>
+    </mj-section>
+
+    <mj-include path="_footer.mjml"/>
+
+    <mj-section padding="0">
+      <mj-column>
+        <mj-text color="gray">
+          <p>If the button above doesn't work, you can enter the following URL manually in your
+            browser:
+          </p>
+          <p>{{passwordResetUrl}}</p>
+        </mj-text>
+      </mj-column>
+    </mj-section>
+
+  </mj-body>
+</mjml>
diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml
@@ -12,12 +12,15 @@ faf:
       subject: ${REGISTRATION_EMAIL_SUBJECT:FAF user registration}
       activation-mail-template-path: ${ACCOUNT_ACTIVATION_MAIL_TEMPLATE_PATH:/config/mail/account-activation.html}
       welcome-subject: ${WELCOME_MAIL_SUBJECT:Welcome to FAF}
+      email-taken-subject: ${EMAIL_TAKEN_SUBJECT:Account exists notice}
       welcome-mail-template-path: ${WELCOME_MAIL_TEMPLATE_PATH:/config/mail/welcome-to-faf.html}
+      email-taken-mail-template-path: ${EMAIL_TAKEN_MAIL_TEMPLATE_PATH:/config/mail/email-taken.html}
       terms-of-service-url: ${FAF_TERMS_OF_SERVICE:https://faforever.com/tos}
       privacy-statement-url: ${FAF_PRIVACY_STATEMENT:https://faforever.com/privacy}
       rules-url: ${FAF_RULES:https://faforever.com/rules}
     password-reset:
       password-reset-url-format: ${faf.self-url}/recover-account/set-password?token=%s
+      password-reset-initiate-email-url-format: ${faf.self-url}/recover-account/email?identifier=%s
       subject: ${PASSWORD_RESET_EMAIL_SUBJECT:FAF password reset}
       mail-template-path: ${PASSWORD_RESET_MAIL_TEMPLATE_PATH:/config/mail/password-reset.html}
     username:
@@ -103,6 +106,7 @@ quarkus:
       registration:
         activation-mail-template-path: ${ACCOUNT_ACTIVATION_MAIL_TEMPLATE_PATH:../../../../src/main/mjml/dummy/test-account-activation.html}
         welcome-mail-template-path: ${WELCOME_MAIL_TEMPLATE_PATH:../../../../src/main/mjml/dummy/test-welcome-to-faf.html}
+        email-taken-mail-template-path: ${EMAIL_TAKEN_MAIL_TEMPLATE_PATH:../../../../src/main/mjml/dummy/test-email-taken.html}
       password-reset:
         mail-template-path: ${PASSWORD_RESET_MAIL_TEMPLATE_PATH:../../../../src/main/mjml/dummy/test-password-reset.html}
     irc:

diff --git a/src/test/kotlin/com/faforever/userservice/backend/account/RegistrationServiceTest.kt b/src/test/kotlin/com/faforever/userservice/backend/account/RegistrationServiceTest.kt
@@ -5,11 +5,13 @@ import com.faforever.userservice.backend.domain.IpAddress
 import com.faforever.userservice.backend.domain.NameRecordRepository
 import com.faforever.userservice.backend.domain.User
 import com.faforever.userservice.backend.domain.UserRepository
+import com.faforever.userservice.backend.email.EmailService
 import com.faforever.userservice.backend.security.FafTokenService
 import com.faforever.userservice.config.FafProperties
 import io.quarkus.mailer.MockMailbox
 import io.quarkus.test.InjectMock
 import io.quarkus.test.junit.QuarkusTest
+import io.quarkus.test.junit.mockito.InjectSpy
 import jakarta.inject.Inject
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.hasSize
@@ -20,6 +22,7 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.mockito.ArgumentMatchers.anyString
 import org.mockito.kotlin.any
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 
@@ -35,6 +38,9 @@ class RegistrationServiceTest {
         private val user = User(1, username, password, email, null)
     }
 
+    @InjectSpy
+    private lateinit var emailService: EmailService
+
     @Inject
     private lateinit var registrationService: RegistrationService
 
@@ -87,16 +93,20 @@ class RegistrationServiceTest {
 
     @Test
     fun registerEmailTaken() {
-        whenever(userRepository.existsByEmail(anyString())).thenReturn(true)
+        whenever(userRepository.findByEmail(anyString())).thenReturn(user)
+        val newTestUsername = "newUsername"
 
-        assertThrows<IllegalArgumentException> { registrationService.register(username, email) }
+        registrationService.register(newTestUsername, email)
+
+        val expectedLink = fafProperties.account().passwordReset().passwordResetInitiateEmailUrlFormat().format(email)
+        verify(emailService, times(1)).sendEmailAlreadyTakenMail(newTestUsername, username, email, expectedLink)
     }
 
     @Test
     fun registerEmailBlacklisted() {
         whenever(domainBlacklistRepository.existsByDomain(anyString())).thenReturn(true)
 
-        assertThrows<IllegalArgumentException> { registrationService.register(username, email) }
+        assertThrows<IllegalStateException> { registrationService.register(username, email) }
     }
 
     @Test
@@ -131,7 +141,7 @@ class RegistrationServiceTest {
 
     @Test
     fun activateEmailTaken() {
-        whenever(userRepository.existsByEmail(anyString())).thenReturn(true)
+        whenever(userRepository.findByEmail(anyString())).thenReturn(user)
 
         assertThrows<IllegalArgumentException> {
             registrationService.activate(RegisteredUser(username, email), ipAddress, password)

diff --git a/src/test/resources/application.yaml b/src/test/resources/application.yaml
@@ -5,6 +5,7 @@ faf:
     registration:
       activation-mail-template-path: src/test/resources/mail/account-activation.html
       welcome-mail-template-path: src/test/resources/mail/welcome-to-faf.html
+      email-taken-mail-template-path: src/test/resources/mail/email-taken.html
 
   irc:
     fixed:

diff --git a/src/test/resources/mail/email-taken.html b/src/test/resources/mail/email-taken.html
@@ -0,0 +1,3 @@
+{{desiredUsername}}
+{{existingUsername}}
+{{passwordResetUrl}}