diff --git a/src/main/bundles/dev.bundle b/src/main/bundles/dev.bundle index 9ffb2c2a..5452c13f 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/RecoveryService.kt b/src/main/kotlin/com/faforever/userservice/backend/account/RecoveryService.kt index 3c6a9067..6309037c 100644 --- a/src/main/kotlin/com/faforever/userservice/backend/account/RecoveryService.kt +++ b/src/main/kotlin/com/faforever/userservice/backend/account/RecoveryService.kt @@ -58,52 +58,72 @@ class RecoveryService( fafProperties.account().passwordReset().passwordResetUrlFormat().format("STEAM"), ) - fun parseRecoveryHttpRequest(parameters: Map>): Pair { + sealed interface ParsingResult { + data class ExtractedUser(val type: Type, val user: User) : ParsingResult + data class ValidNoUser(val type: Type) : ParsingResult + data class Invalid(val cause: Exception) : ParsingResult + } + + fun parseRecoveryHttpRequest(parameters: Map>): ParsingResult { // At first glance it may seem strange that a service is parsing http request parameters, // but the parameters of the request are determined by this service itself in the request reset phase! - val token = parameters["token"]?.first() - LOG.debug("Extracted token: {}", token) - val steamId = steamService.parseSteamIdFromRequestParameters(parameters) - LOG.debug("Extracted Steam id: {}", steamId) - - return when { - steamId != null -> Type.STEAM to steamService.findUserBySteamId(steamId).also { user -> - if (user == null) metricHelper.incrementPasswordResetViaSteamFailedCounter() - } - - token != null -> Type.EMAIL to extractUserFromEmailRecoveryToken(token) - else -> { + val token = parameters["token"]?.firstOrNull() + LOG.debug("Extracted token: {}", token) + return when (token) { + null -> { metricHelper.incrementPasswordResetViaEmailFailedCounter() - throw InvalidRecoveryException("Could not extract recovery type or user from HTTP request") + ParsingResult.Invalid(InvalidRecoveryException("Could not extract token")) } + "STEAM" -> when (val result = steamService.parseSteamIdFromRequestParameters(parameters)) { + is SteamService.ParsingResult.NoSteamIdPresent, + is SteamService.ParsingResult.InvalidRedirect, + -> { + metricHelper.incrementPasswordResetViaSteamFailedCounter() + ParsingResult.Invalid( + InvalidRecoveryException("Steam based recovery attempt is invalid"), + ) + } + is SteamService.ParsingResult.ExtractedId -> { + val user = steamService.findUserBySteamId(result.steamId) + if (user == null) { + metricHelper.incrementPasswordResetViaSteamFailedCounter() + ParsingResult.ValidNoUser(Type.STEAM) + } else { + ParsingResult.ExtractedUser(Type.STEAM, user) + } + } + } + + // Email + else -> extractUserFromEmailRecoveryToken(token) } } - private fun extractUserFromEmailRecoveryToken(emailRecoveryToken: String): User { + private fun extractUserFromEmailRecoveryToken(emailRecoveryToken: String): ParsingResult { val claims = try { fafTokenService.getTokenClaims(FafTokenType.PASSWORD_RESET, emailRecoveryToken) } catch (exception: Exception) { metricHelper.incrementPasswordResetViaEmailFailedCounter() LOG.error("Unable to extract claims", exception) - throw InvalidRecoveryException("Unable to extract claims from token") + return ParsingResult.Invalid(InvalidRecoveryException("Unable to extract claims from token")) } val userId = claims[KEY_USER_ID] if (userId.isNullOrBlank()) { metricHelper.incrementPasswordResetViaEmailFailedCounter() - throw InvalidRecoveryException("No user id found in token claims") + return ParsingResult.Invalid(InvalidRecoveryException("No user id found in token claims")) } val user = userRepository.findById(userId.toInt()) if (user == null) { metricHelper.incrementPasswordResetViaEmailFailedCounter() - throw InvalidRecoveryException("User with id $userId not found") + return ParsingResult.Invalid(InvalidRecoveryException("User with id $userId not found")) } - return user + return ParsingResult.ExtractedUser(Type.EMAIL, user) } fun resetPassword(type: Type, userId: Int, newPassword: String) { diff --git a/src/main/kotlin/com/faforever/userservice/backend/steam/SteamService.kt b/src/main/kotlin/com/faforever/userservice/backend/steam/SteamService.kt index 24bcf77c..936774cf 100644 --- a/src/main/kotlin/com/faforever/userservice/backend/steam/SteamService.kt +++ b/src/main/kotlin/com/faforever/userservice/backend/steam/SteamService.kt @@ -35,20 +35,31 @@ class SteamService( .build().toString() } - fun parseSteamIdFromRequestParameters(parameters: Map>): String? { - if (!parameters.containsKey(OPENID_IDENTITY_KEY)) { - LOG.debug("No OpenID identity key present") - return null - } - - validateSteamRedirect(parameters) - - LOG.trace("Parsing steam id from request parameters: {}", parameters) - return parameters[OPENID_IDENTITY_KEY]?.get(0) - ?.let { identityUrl -> identityUrl.substring(identityUrl.lastIndexOf("/") + 1) } + sealed interface ParsingResult { + data object NoSteamIdPresent : ParsingResult + data object InvalidRedirect : ParsingResult + data class ExtractedId(val steamId: String) : ParsingResult } - private fun validateSteamRedirect(parameters: Map>) { + fun parseSteamIdFromRequestParameters(parameters: Map>): ParsingResult = + when { + !isValidSteamRedirect(parameters) -> ParsingResult.InvalidRedirect + else -> { + LOG.trace("Parsing steam id from request parameters: {}", parameters) + parameters[OPENID_IDENTITY_KEY]?.firstOrNull() + ?.let { identityUrl -> identityUrl.substring(identityUrl.lastIndexOf("/") + 1) } + ?.let { steamId -> + ParsingResult.ExtractedId(steamId).also { + LOG.debug("Extracted Steam id: {}", steamId) + } + } + ?: ParsingResult.NoSteamIdPresent.also { + LOG.debug("No OpenID identity key present") + } + } + } + + private fun isValidSteamRedirect(parameters: Map>): Boolean { LOG.debug("Checking valid OpenID 2.0 redirect against Steam API, parameters: {}", parameters) val uriBuilder = UriBuilder.fromUri(fafProperties.steam().loginUrlFormat()) @@ -68,11 +79,11 @@ class SteamService( val result = response.body() if (result == null || !result.contains("is_valid:true")) { - throw InvalidSteamRedirectException( - "Could not verify steam redirect for identity: ${parameters[OPENID_IDENTITY_KEY]}", - ) + LOG.debug("Could not verify steam redirect for identity: {}", parameters[OPENID_IDENTITY_KEY]) + return false } else { LOG.debug("Steam response successfully validated.") + return true } } diff --git a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverAccountView.kt b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverAccountView.kt index dad140a8..42acdd3d 100644 --- a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverAccountView.kt +++ b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverAccountView.kt @@ -20,7 +20,7 @@ class RecoverAccountView : private val emailSection = VerticalLayout( Button(getTranslation("recovery.selectMethod.email.link")) { - getUI().ifPresent{ ui -> ui.navigate(RecoverViaEmailView::class.java) } + getUI().ifPresent { ui -> ui.navigate(RecoverViaEmailView::class.java) } }.apply { addThemeVariants(ButtonVariant.LUMO_PRIMARY) }, @@ -33,7 +33,7 @@ class RecoverAccountView : private val steamSection = VerticalLayout( Button(getTranslation("recovery.selectMethod.steam.link")) { - getUI().ifPresent{ ui -> ui.navigate(RecoverViaSteamView::class.java) } + getUI().ifPresent { ui -> ui.navigate(RecoverViaSteamView::class.java) } }.apply { addThemeVariants(ButtonVariant.LUMO_PRIMARY) }, diff --git a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverSetPasswordView.kt b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverSetPasswordView.kt index 00666226..e751e90f 100644 --- a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverSetPasswordView.kt +++ b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverSetPasswordView.kt @@ -102,27 +102,19 @@ class RecoverSetPasswordView( override fun beforeEnter(event: BeforeEnterEvent?) { val parameters = event?.location?.queryParameters?.parameters ?: emptyMap() - val (recoveryType, user) = try { - recoveryService.parseRecoveryHttpRequest(parameters) - } catch (e: Exception) { - showDialog("recovery.setPassword.failed.title", "recovery.setPassword.invalidToken") - return - } - - if (user == null) { - when (recoveryType) { - RecoveryService.Type.EMAIL -> - showDialog("recovery.setPassword.failed.title", "recovery.setPassword.invalidToken") - RecoveryService.Type.STEAM -> - showDialog("recovery.setPassword.failed.title", "recovery.steam.unknownUser") + when (val result = recoveryService.parseRecoveryHttpRequest(parameters)) { + is RecoveryService.ParsingResult.Invalid -> + showDialog("recovery.setPassword.failed.title", "recovery.setPassword.invalidToken") + is RecoveryService.ParsingResult.ValidNoUser -> + showDialog("recovery.setPassword.failed.title", "recovery.setPassword.unknownUser") + is RecoveryService.ParsingResult.ExtractedUser -> { + this.recoveryType = result.type + this.user = result.user + + usernameInRecovery.element.text = + getTranslation("recovery.setPassword.usernameInRecovery", user?.username ?: "") } - } else { - usernameInRecovery.element.text = - getTranslation("recovery.setPassword.usernameInRecovery", user.username ?: "") } - - this.recoveryType = recoveryType - this.user = user } private fun showDialog(titleKey: String, messageKey: String?) { diff --git a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaSteamView.kt b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaSteamView.kt index dc0a10c2..a3aca111 100644 --- a/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaSteamView.kt +++ b/src/main/kotlin/com/faforever/userservice/ui/view/recovery/RecoverViaSteamView.kt @@ -69,6 +69,6 @@ class RecoverViaSteamView( private fun redirectToSteam() { val steamUrl = recoveryService.buildSteamLoginUrl() - getUI().ifPresent{ ui -> ui.page.setLocation(steamUrl)} + getUI().ifPresent { ui -> ui.page.setLocation(steamUrl) } } } diff --git a/src/main/resources/i18n/messages.properties b/src/main/resources/i18n/messages.properties index e2f56994..ca278196 100644 --- a/src/main/resources/i18n/messages.properties +++ b/src/main/resources/i18n/messages.properties @@ -100,7 +100,7 @@ recovery.email.sent.title=Recovery email sent recovery.email.sent.hint=Don't forget to also check your spam folder! Disclaimer: Due to data privacy regulations we cannot tell you if the username or email actually exists in our system and an email was actually sent. recovery.steam.title=Reset password via Steam recovery.steam.disclaimer=You will be forwarded to the Steam website to login with your Steam username and password. FAForever can not see your Steam password, nor do we get access to your Steam account in any way. -recovery.steam.unknownUser=There is no user associated with your Steam account. +recovery.setPassword.unknownUser=There is no user associated with your Steam account. recovery.setPassword.title=Set new password recovery.setPassword.usernameInRecovery=You are recovering user ''{0}''. recovery.setPassword.submit=Set new password diff --git a/src/test/kotlin/com/faforever/userservice/backend/account/RecoveryServiceTest.kt b/src/test/kotlin/com/faforever/userservice/backend/account/RecoveryServiceTest.kt index b9722ece..6d8327e9 100644 --- a/src/test/kotlin/com/faforever/userservice/backend/account/RecoveryServiceTest.kt +++ b/src/test/kotlin/com/faforever/userservice/backend/account/RecoveryServiceTest.kt @@ -1,5 +1,6 @@ package com.faforever.userservice.backend.account +import com.faforever.userservice.backend.account.RecoveryService.ParsingResult import com.faforever.userservice.backend.domain.User import com.faforever.userservice.backend.domain.UserRepository import com.faforever.userservice.backend.email.EmailService @@ -16,10 +17,9 @@ import jakarta.inject.Inject import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.containsString import org.hamcrest.Matchers.equalTo -import org.hamcrest.Matchers.nullValue +import org.hamcrest.Matchers.instanceOf import org.hamcrest.Matchers.startsWith import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.verify @@ -113,51 +113,49 @@ class RecoveryServiceTest { @Test fun testParseRecoveryHttpRequestWithEmptyParameters() { // Execute - assertThrows { - recoveryService.parseRecoveryHttpRequest(emptyMap()) - } + val result = recoveryService.parseRecoveryHttpRequest(emptyMap()) // Verify + assertThat(result, instanceOf(ParsingResult.Invalid::class.java)) + verify(metricHelper).incrementPasswordResetViaEmailFailedCounter() } @Test fun testParseRecoveryHttpRequestWithUnknownSteamId() { // Prepare - val parameters = mapOf("some" to listOf("fake", "values")) + val parameters = mapOf("token" to listOf("STEAM")) whenever(steamService.parseSteamIdFromRequestParameters(parameters)) - .thenReturn("someSteamId") + .thenReturn(SteamService.ParsingResult.ExtractedId("someSteamId")) whenever(steamService.findUserBySteamId("someSteamId")) .thenReturn(null) // Execute - val (type, user) = recoveryService.parseRecoveryHttpRequest(parameters) + val result = recoveryService.parseRecoveryHttpRequest(parameters) as ParsingResult.ValidNoUser // Verify - assertThat(type, equalTo(RecoveryService.Type.STEAM)) - assertThat(user, nullValue()) - + assertThat(result.type, equalTo(RecoveryService.Type.STEAM)) verify(metricHelper).incrementPasswordResetViaSteamFailedCounter() } @Test fun testParseRecoveryHttpRequestWithKnownSteamId() { // Prepare - val parameters = mapOf("some" to listOf("fake", "values")) + val parameters = mapOf("token" to listOf("STEAM")) val testUser = buildTestUser() whenever(steamService.parseSteamIdFromRequestParameters(parameters)) - .thenReturn("someSteamId") + .thenReturn(SteamService.ParsingResult.ExtractedId("someSteamId")) whenever(steamService.findUserBySteamId("someSteamId")) .thenReturn(testUser) // Execute - val (type, user) = recoveryService.parseRecoveryHttpRequest(parameters) + val result = recoveryService.parseRecoveryHttpRequest(parameters) as ParsingResult.ExtractedUser // Verify - assertThat(type, equalTo(RecoveryService.Type.STEAM)) - assertThat(user, equalTo(testUser)) + assertThat(result.type, equalTo(RecoveryService.Type.STEAM)) + assertThat(result.user, equalTo(testUser)) } @Test @@ -165,17 +163,14 @@ class RecoveryServiceTest { // Prepare val parameters = mapOf("token" to listOf("tokenValue")) - whenever(steamService.parseSteamIdFromRequestParameters(parameters)) - .thenReturn(null) whenever(fafTokenService.getTokenClaims(PASSWORD_RESET, "tokenValue")) .thenThrow(RuntimeException("invalid token claim")) // Execute - assertThrows { - recoveryService.parseRecoveryHttpRequest(parameters) - } + val result = recoveryService.parseRecoveryHttpRequest(parameters) // Verify + assertThat(result, instanceOf(ParsingResult.Invalid::class.java)) verify(metricHelper).incrementPasswordResetViaEmailFailedCounter() } @@ -185,16 +180,15 @@ class RecoveryServiceTest { val parameters = mapOf("token" to listOf("tokenValue")) whenever(steamService.parseSteamIdFromRequestParameters(parameters)) - .thenReturn(null) + .thenReturn(SteamService.ParsingResult.NoSteamIdPresent) whenever(fafTokenService.getTokenClaims(PASSWORD_RESET, "tokenValue")) .thenReturn(emptyMap()) // Execute - assertThrows { - recoveryService.parseRecoveryHttpRequest(parameters) - } + val result = recoveryService.parseRecoveryHttpRequest(parameters) // Verify + assertThat(result, instanceOf(ParsingResult.Invalid::class.java)) verify(metricHelper).incrementPasswordResetViaEmailFailedCounter() } @@ -203,18 +197,16 @@ class RecoveryServiceTest { // Prepare val parameters = mapOf("token" to listOf("tokenValue")) - whenever(steamService.parseSteamIdFromRequestParameters(parameters)) - .thenReturn(null) whenever(fafTokenService.getTokenClaims(PASSWORD_RESET, "tokenValue")) .thenReturn(mapOf("id" to "12345")) whenever(userRepository.findById(12345)).thenReturn(null) // Execute - assertThrows { - recoveryService.parseRecoveryHttpRequest(parameters) - } + val result = recoveryService.parseRecoveryHttpRequest(parameters) // Verify + assertThat(result, instanceOf(ParsingResult.Invalid::class.java)) + verify(metricHelper).incrementPasswordResetViaEmailFailedCounter() } @@ -231,11 +223,11 @@ class RecoveryServiceTest { whenever(userRepository.findById(12345)).thenReturn(testUser) // Execute - val (type, user) = recoveryService.parseRecoveryHttpRequest(parameters) + val result = recoveryService.parseRecoveryHttpRequest(parameters) as ParsingResult.ExtractedUser // Verify - assertThat(type, equalTo(RecoveryService.Type.EMAIL)) - assertThat(user, equalTo(testUser)) + assertThat(result.type, equalTo(RecoveryService.Type.EMAIL)) + assertThat(result.user, equalTo(testUser)) } @Test