From 5d38db0855f8e48cf43795b73488121dd4810b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Wed, 21 Aug 2024 10:28:24 +0200 Subject: [PATCH 1/3] Add voucher code domain model --- .../mullvadvpn/lib/model/VoucherCode.kt | 31 +++++++++++++++++ .../mullvadvpn/lib/model/VoucherCodeTest.kt | 34 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt create mode 100644 android/lib/model/src/test/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCodeTest.kt diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt new file mode 100644 index 000000000000..9790096d5512 --- /dev/null +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt @@ -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/docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md + fun fromString(value: String): Either = 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 +} diff --git a/android/lib/model/src/test/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCodeTest.kt b/android/lib/model/src/test/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCodeTest.kt new file mode 100644 index 000000000000..270e8b250391 --- /dev/null +++ b/android/lib/model/src/test/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCodeTest.kt @@ -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()) + } +} From d1e33486cf2b4c027f1a87a1700891c7cd6bcbab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Wed, 21 Aug 2024 10:30:22 +0200 Subject: [PATCH 2/3] Add error for voucher code looks like an account number --- .../compose/dialog/RedeemVoucherDialog.kt | 13 ++++++++++++ .../viewmodel/VoucherDialogViewModel.kt | 21 ++++++++++++++++--- .../lib/daemon/grpc/ManagementService.kt | 7 +++++-- .../lib/model/RedeemVoucherError.kt | 4 ++++ .../resource/src/main/res/values/strings.xml | 1 + .../lib/shared/VoucherRepository.kt | 3 ++- gui/locales/messages.pot | 3 +++ 7 files changed, 46 insertions(+), 6 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/RedeemVoucherDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/RedeemVoucherDialog.kt index 093dc7ce9715..6d3d134e1ddf 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/RedeemVoucherDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/RedeemVoucherDialog.kt @@ -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, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModel.kt index 3d67b42bd1c7..857953f28bbf 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModel.kt @@ -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 @@ -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 @@ -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) } diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt index 95679cb43da9..36460ae1fa9f 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt @@ -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 @@ -575,8 +576,10 @@ class ManagementService( .mapLeft(SetWireguardConstraintsError::Unknown) .mapEmpty() - suspend fun submitVoucher(voucher: String): Either = - Either.catch { grpc.submitVoucher(StringValue.of(voucher)).toDomain() } + suspend fun submitVoucher( + voucher: VoucherCode + ): Either = + Either.catch { grpc.submitVoucher(StringValue.of(voucher.value)).toDomain() } .mapLeftStatus { when (it.status.code) { Status.Code.INVALID_ARGUMENT, diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RedeemVoucherError.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RedeemVoucherError.kt index d14a2f236b16..6df604c4a637 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RedeemVoucherError.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RedeemVoucherError.kt @@ -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() diff --git a/android/lib/resource/src/main/res/values/strings.xml b/android/lib/resource/src/main/res/values/strings.xml index ef6b0bce7afe..c43acdb496d9 100644 --- a/android/lib/resource/src/main/res/values/strings.xml +++ b/android/lib/resource/src/main/res/values/strings.xml @@ -33,6 +33,7 @@ Redeem Voucher code is invalid. Voucher code has already been used. + 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. An error occurred. Settings No internet connection diff --git a/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VoucherRepository.kt b/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VoucherRepository.kt index a5783a832eca..3ea55ccd9d55 100644 --- a/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VoucherRepository.kt +++ b/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VoucherRepository.kt @@ -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) } diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot index 76223176d523..725863c57bfd 100644 --- a/gui/locales/messages.pot +++ b/gui/locales/messages.pot @@ -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 "" From e12c64a82809d0bf17f3ab59cfafe3ed7ab7793c Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 21 Aug 2024 10:31:23 +0200 Subject: [PATCH 3/3] Fix voucher dialog view model tests --- .../viewmodel/VoucherDialogViewModelTest.kt | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModelTest.kt index ef3b34effc0a..3bd96de3cbf2 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModelTest.kt @@ -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 @@ -45,11 +46,12 @@ 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 @@ -57,7 +59,7 @@ class VoucherDialogViewModelTest { viewModel.onRedeem(voucher) // Assert - coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(voucher) } + coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(parsedVoucher) } } @Test @@ -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 { @@ -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 { @@ -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 {