Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better Voucher Code validation #6571

Merged
merged 3 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,23 @@ private fun EnterVoucherBody(
)
}
}
if (
state.voucherState is VoucherDialogState.Error &&
state.voucherState.error is RedeemVoucherError.EnteredAccountNumber
) {
Text(
modifier = Modifier.padding(top = Dimens.smallPadding),
text = stringResource(id = R.string.voucher_is_account_number),
color = MaterialTheme.colorScheme.onPrimary,
style = MaterialTheme.typography.bodySmall
)
}
}

private fun RedeemVoucherError.message(): Int =
when (this) {
RedeemVoucherError.TooShortVoucher,
RedeemVoucherError.EnteredAccountNumber,
RedeemVoucherError.InvalidVoucher -> R.string.invalid_voucher
RedeemVoucherError.VoucherAlreadyUsed -> R.string.voucher_already_used
RedeemVoucherError.RpcError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package net.mullvad.mullvadvpn.viewmodel

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import arrow.core.raise.either
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.combine
Expand All @@ -11,7 +12,9 @@ import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.compose.state.VoucherDialogState
import net.mullvad.mullvadvpn.compose.state.VoucherDialogUiState
import net.mullvad.mullvadvpn.constant.VOUCHER_LENGTH
import net.mullvad.mullvadvpn.lib.model.ParseVoucherCodeError
import net.mullvad.mullvadvpn.lib.model.RedeemVoucherError
import net.mullvad.mullvadvpn.lib.model.VoucherCode
import net.mullvad.mullvadvpn.lib.shared.VoucherRepository
import net.mullvad.mullvadvpn.util.VoucherRegexHelper

Expand All @@ -26,11 +29,23 @@ class VoucherDialogViewModel(private val voucherRepository: VoucherRepository) :
}
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), VoucherDialogUiState.INITIAL)

fun onRedeem(voucherCode: String) {
fun onRedeem(voucherInput: String) {
vmState.update { VoucherDialogState.Verifying }
viewModelScope.launch {
voucherRepository
.submitVoucher(voucherCode)
either {
val voucherCode =
VoucherCode.fromString(voucherInput)
.mapLeft {
when (it) {
is ParseVoucherCodeError.AllDigit ->
RedeemVoucherError.EnteredAccountNumber
is ParseVoucherCodeError.TooShort ->
RedeemVoucherError.TooShortVoucher
}
}
.bind()
voucherRepository.submitVoucher(voucherCode).bind()
}
.fold(
{ error -> setError(error) },
{ success -> handleAddedTime(success.timeAdded) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import net.mullvad.mullvadvpn.compose.state.VoucherDialogState
import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule
import net.mullvad.mullvadvpn.lib.model.RedeemVoucherError
import net.mullvad.mullvadvpn.lib.model.RedeemVoucherSuccess
import net.mullvad.mullvadvpn.lib.model.VoucherCode
import net.mullvad.mullvadvpn.lib.shared.VoucherRepository
import org.joda.time.DateTime
import org.junit.jupiter.api.AfterEach
Expand Down Expand Up @@ -45,19 +46,20 @@ class VoucherDialogViewModelTest {
@Test
fun `ensure onRedeem invokes submit on VoucherRedeemer with same voucher code`() = runTest {
val voucher = DUMMY_INVALID_VOUCHER
val parsedVoucher = VoucherCode.fromString(voucher).getOrNull()!!

// Arrange
val timeAdded = 0L
val newExpiry = DateTime()
coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
coEvery { mockVoucherRepository.submitVoucher(parsedVoucher) } returns
RedeemVoucherSuccess(timeAdded, newExpiry).right()

// Act
assertIs<VoucherDialogState.Default>(viewModel.uiState.value.voucherState)
viewModel.onRedeem(voucher)

// Assert
coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(voucher) }
coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(parsedVoucher) }
}

@Test
Expand All @@ -66,8 +68,9 @@ class VoucherDialogViewModelTest {

// Arrange
every { mockVoucherSubmission.timeAdded } returns 0
coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
RedeemVoucherError.InvalidVoucher.left()
coEvery {
mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
} returns RedeemVoucherError.InvalidVoucher.left()

// Act, Assert
viewModel.uiState.test {
Expand All @@ -84,8 +87,9 @@ class VoucherDialogViewModelTest {

// Arrange
every { mockVoucherSubmission.timeAdded } returns 0
coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
RedeemVoucherSuccess(0, DateTime()).right()
coEvery {
mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
} returns RedeemVoucherSuccess(0, DateTime()).right()

// Act, Assert
viewModel.uiState.test {
Expand All @@ -102,8 +106,9 @@ class VoucherDialogViewModelTest {

// Arrange
every { mockVoucherSubmission.timeAdded } returns 0
coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
RedeemVoucherError.VoucherAlreadyUsed.left()
coEvery {
mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
} returns RedeemVoucherError.VoucherAlreadyUsed.left()

// Act, Assert
viewModel.uiState.test {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ import net.mullvad.mullvadvpn.lib.model.UnknownApiAccessMethodError
import net.mullvad.mullvadvpn.lib.model.UnknownCustomListError
import net.mullvad.mullvadvpn.lib.model.UpdateApiAccessMethodError
import net.mullvad.mullvadvpn.lib.model.UpdateCustomListError
import net.mullvad.mullvadvpn.lib.model.VoucherCode
import net.mullvad.mullvadvpn.lib.model.WebsiteAuthToken
import net.mullvad.mullvadvpn.lib.model.WireguardConstraints as ModelWireguardConstraints
import net.mullvad.mullvadvpn.lib.model.WireguardEndpointData as ModelWireguardEndpointData
Expand Down Expand Up @@ -575,8 +576,10 @@ class ManagementService(
.mapLeft(SetWireguardConstraintsError::Unknown)
.mapEmpty()

suspend fun submitVoucher(voucher: String): Either<RedeemVoucherError, RedeemVoucherSuccess> =
Either.catch { grpc.submitVoucher(StringValue.of(voucher)).toDomain() }
suspend fun submitVoucher(
voucher: VoucherCode
): Either<RedeemVoucherError, RedeemVoucherSuccess> =
Either.catch { grpc.submitVoucher(StringValue.of(voucher.value)).toDomain() }
.mapLeftStatus {
when (it.status.code) {
Status.Code.INVALID_ARGUMENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ sealed class RedeemVoucherError {

data object VoucherAlreadyUsed : RedeemVoucherError()

data object TooShortVoucher : RedeemVoucherError()

data object EnteredAccountNumber : RedeemVoucherError()

data object RpcError : RedeemVoucherError()

data class Unknown(val error: Throwable) : RedeemVoucherError()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package net.mullvad.mullvadvpn.lib.model

import arrow.core.Either
import arrow.core.raise.either
import arrow.core.raise.ensure

@JvmInline
value class VoucherCode private constructor(val value: String) {

companion object {
// Parsing reference:
// <services-repository>/services/docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md
fun fromString(value: String): Either<ParseVoucherCodeError, VoucherCode> = either {
val trimmedValue = value.trim()
ensure(trimmedValue.length >= MIN_VOUCHER_LENGTH) {
ParseVoucherCodeError.TooShort(trimmedValue)
}
ensure(!value.all { it.isDigit() }) { ParseVoucherCodeError.AllDigit(trimmedValue) }
VoucherCode(trimmedValue)
}

const val MIN_VOUCHER_LENGTH = 16
}
}

sealed interface ParseVoucherCodeError {

data class AllDigit(val value: String) : ParseVoucherCodeError

data class TooShort(val value: String) : ParseVoucherCodeError
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package net.mullvad.mullvadvpn.lib.model

import kotlin.test.assertEquals
import kotlin.test.assertTrue
import org.junit.jupiter.api.Test

class VoucherCodeTest {
@Test
fun `parsing a too short voucher code should return TooShort`() {
val input = "mycode"
val result = VoucherCode.fromString(input)

assertTrue(result.isLeft())
assertEquals(ParseVoucherCodeError.TooShort(input), result.leftOrNull())
}

@Test
fun `numbers only should not be allowed`() {
val input = "1234123412341234"
val result = VoucherCode.fromString(input)

assertTrue(result.isLeft())
assertEquals(ParseVoucherCodeError.AllDigit(input), result.leftOrNull())
}

@Test
fun `number only input when too short should return TooShort`() {
val input = "123412341234"
val result = VoucherCode.fromString(input)

assertTrue(result.isLeft())
assertEquals(ParseVoucherCodeError.TooShort(input), result.leftOrNull())
}
}
1 change: 1 addition & 0 deletions android/lib/resource/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
<string name="redeem">Redeem</string>
<string name="invalid_voucher">Voucher code is invalid.</string>
<string name="voucher_already_used">Voucher code has already been used.</string>
<string name="voucher_is_account_number">It looks like you’ve entered an account number instead of a voucher code. If you would like to change the active account, please log out first.</string>
<string name="error_occurred">An error occurred.</string>
<string name="settings">Settings</string>
<string name="no_internet_connection">No internet connection</string>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package net.mullvad.mullvadvpn.lib.shared

import net.mullvad.mullvadvpn.lib.daemon.grpc.ManagementService
import net.mullvad.mullvadvpn.lib.model.VoucherCode

class VoucherRepository(
private val managementService: ManagementService,
private val accountRepository: AccountRepository
) {
suspend fun submitVoucher(voucher: String) =
suspend fun submitVoucher(voucher: VoucherCode) =
managementService.submitVoucher(voucher).onRight {
accountRepository.onVoucherRedeemed(it.newExpiryDate)
}
Expand Down
3 changes: 3 additions & 0 deletions gui/locales/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -2318,6 +2318,9 @@ msgstr ""
msgid "Invalid or missing value \"%s\""
msgstr ""

msgid "It looks like you’ve entered an account number instead of a voucher code. If you would like to change the active account, please log out first."
msgstr ""

msgid "Lets you select apps that should access the Internet directly without going through the VPN tunnel."
msgstr ""

Expand Down
Loading