diff --git a/src/main/bundles/dev.bundle b/src/main/bundles/dev.bundle index 5452c13f..87aab93a 100644 Binary files a/src/main/bundles/dev.bundle and b/src/main/bundles/dev.bundle differ diff --git a/src/main/kotlin/com/faforever/userservice/backend/account/RegistrationService.kt b/src/main/kotlin/com/faforever/userservice/backend/account/RegistrationService.kt index 7c33a27b..38d51eb6 100644 --- 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 index f636b507..2dbee678 100644 --- 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 { 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 = 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 index 6697ac25..aa0bd14d 100644 --- 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 index bd24108b..56bdc6d3 100644 --- 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 @@ -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 index a746d711..9dd54b7c 100644 --- 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 index edc35529..1ded96c0 100644 --- 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 index 4c851e6c..b09bfce6 100644 --- 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 new file mode 100644 index 00000000..f661408c --- /dev/null +++ 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 new file mode 100644 index 00000000..4d9f4584 --- /dev/null +++ b/src/main/mjml/email-taken.mjml @@ -0,0 +1,60 @@ + + + + + Account exists notice + + + + + + + + +

Account exists notice

+

Dear {{existingUsername}},

+

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. +

+
+
+
+ + + + +

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. +

+

If you have no more access to this account, you can reset the password right here:

+
+ + Reset Password + +
+
+ + + + +

If you did not attempt to register a new account, please ignore this email.

+
+
+
+ + + + + + +

If the button above doesn't work, you can enter the following URL manually in your + browser: +

+

{{passwordResetUrl}}

+
+
+
+ +
+
diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 6f784238..3431367e 100644 --- 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 index f62bb200..915c8a90 100644 --- 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 { 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 { registrationService.register(username, email) } + assertThrows { 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 { registrationService.activate(RegisteredUser(username, email), ipAddress, password) diff --git a/src/test/resources/application.yaml b/src/test/resources/application.yaml index a089b52e..dd8876d0 100644 --- 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 new file mode 100644 index 00000000..cc2bd68e --- /dev/null +++ b/src/test/resources/mail/email-taken.html @@ -0,0 +1,3 @@ +{{desiredUsername}} +{{existingUsername}} +{{passwordResetUrl}} \ No newline at end of file