Skip to content

Commit

Permalink
Refactor flow to reduce exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
Brutus5000 committed Jan 16, 2025
1 parent 7cef09c commit 5ee0509
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 90 deletions.
Binary file modified src/main/bundles/dev.bundle
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -58,52 +58,72 @@ class RecoveryService(
fafProperties.account().passwordReset().passwordResetUrlFormat().format("STEAM"),
)

fun parseRecoveryHttpRequest(parameters: Map<String, List<String>>): Pair<Type, User?> {
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<String, List<String>>): 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,31 @@ class SteamService(
.build().toString()
}

fun parseSteamIdFromRequestParameters(parameters: Map<String, List<String>>): 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<String, List<String>>) {
fun parseSteamIdFromRequestParameters(parameters: Map<String, List<String>>): 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<String, List<String>>): Boolean {
LOG.debug("Checking valid OpenID 2.0 redirect against Steam API, parameters: {}", parameters)

val uriBuilder = UriBuilder.fromUri(fafProperties.steam().loginUrlFormat())
Expand All @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand All @@ -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)
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}
}
2 changes: 1 addition & 1 deletion src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -113,69 +113,64 @@ class RecoveryServiceTest {
@Test
fun testParseRecoveryHttpRequestWithEmptyParameters() {
// Execute
assertThrows<InvalidRecoveryException> {
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
fun testParseRecoveryHttpRequestWithInvalidTokenClaims() {
// 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<InvalidRecoveryException> {
recoveryService.parseRecoveryHttpRequest(parameters)
}
val result = recoveryService.parseRecoveryHttpRequest(parameters)

// Verify
assertThat(result, instanceOf(ParsingResult.Invalid::class.java))
verify(metricHelper).incrementPasswordResetViaEmailFailedCounter()
}

Expand All @@ -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<InvalidRecoveryException> {
recoveryService.parseRecoveryHttpRequest(parameters)
}
val result = recoveryService.parseRecoveryHttpRequest(parameters)

// Verify
assertThat(result, instanceOf(ParsingResult.Invalid::class.java))
verify(metricHelper).incrementPasswordResetViaEmailFailedCounter()
}

Expand All @@ -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<InvalidRecoveryException> {
recoveryService.parseRecoveryHttpRequest(parameters)
}
val result = recoveryService.parseRecoveryHttpRequest(parameters)

// Verify
assertThat(result, instanceOf(ParsingResult.Invalid::class.java))

verify(metricHelper).incrementPasswordResetViaEmailFailedCounter()
}

Expand All @@ -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
Expand Down

0 comments on commit 5ee0509

Please sign in to comment.