diff --git a/app/src/main/java/com/tangem/tap/domain/userWalletList/repository/DefaultUserWalletsListRepository.kt b/app/src/main/java/com/tangem/tap/domain/userWalletList/repository/DefaultUserWalletsListRepository.kt index 645eebc302..31d16f228b 100644 --- a/app/src/main/java/com/tangem/tap/domain/userWalletList/repository/DefaultUserWalletsListRepository.kt +++ b/app/src/main/java/com/tangem/tap/domain/userWalletList/repository/DefaultUserWalletsListRepository.kt @@ -496,10 +496,10 @@ internal class DefaultUserWalletsListRepository( is HotWalletPasswordRequester.Result.EnteredPassword -> { val decrypted = block(result.password.value) if (decrypted == null) { - passwordRequester.wrongPassword() + passwordRequester.wrongPassword(attemptRequest) requestPasswordRecursive(hotWalletId, block, biometryFallback) } else { - passwordRequester.successfulAuthentication() + passwordRequester.successfulAuthentication(attemptRequest) passwordRequester.dismiss() decrypted.right() } @@ -507,7 +507,7 @@ internal class DefaultUserWalletsListRepository( HotWalletPasswordRequester.Result.UseBiometry -> { biometryFallback() .onRight { - passwordRequester.successfulAuthentication() + passwordRequester.successfulAuthentication(attemptRequest) passwordRequester.dismiss() } .map { null } diff --git a/data/wallets/src/main/java/com/tangem/data/wallets/hot/DefaultHotWalletAccessor.kt b/data/wallets/src/main/java/com/tangem/data/wallets/hot/DefaultHotWalletAccessor.kt index fd499a0f3c..6079e22afd 100644 --- a/data/wallets/src/main/java/com/tangem/data/wallets/hot/DefaultHotWalletAccessor.kt +++ b/data/wallets/src/main/java/com/tangem/data/wallets/hot/DefaultHotWalletAccessor.kt @@ -28,6 +28,11 @@ class DefaultHotWalletAccessor @Inject constructor( private val scope: AppCoroutineScope, ) : HotWalletAccessor { + private data class RequestedAuth( + val auth: HotAuth, + val attemptRequest: HotWalletPasswordRequester.AttemptRequest?, + ) + private val contextualUnlockHotWallet: ConcurrentHashMap = ConcurrentHashMap() override suspend fun signHashes(hotWalletId: HotWalletId, dataToSign: List): List = @@ -84,8 +89,8 @@ class DefaultHotWalletAccessor @Inject constructor( private suspend fun hotSdkRequest(hotWalletId: HotWalletId, block: suspend (unlock: UnlockHotWallet) -> T): T { val isAccessCodeRequired = isAccessCodeRequired() - val auth = when (hotWalletId.authType) { - HotWalletId.AuthType.NoPassword -> HotAuth.NoAuth + val requestedAuth = when (hotWalletId.authType) { + HotWalletId.AuthType.NoPassword -> RequestedAuth(HotAuth.NoAuth, attemptRequest = null) HotWalletId.AuthType.Password -> requestPassword( hotWalletId = hotWalletId, hasBiometry = false, @@ -97,35 +102,32 @@ class DefaultHotWalletAccessor @Inject constructor( hasBiometry = false, ) } else { - HotAuth.Biometry + RequestedAuth(HotAuth.Biometry, attemptRequest = null) } } } - return runCatchingSdkErrors(hotWalletId, auth) { - block(UnlockHotWallet(hotWalletId, it)).also { - hotWalletPasswordRequester.successfulAuthentication() - hotWalletPasswordRequester.dismiss() - } + return runCatchingSdkErrors(hotWalletId, requestedAuth) { + block(UnlockHotWallet(hotWalletId, it)) } } private suspend fun runCatchingSdkErrors( hotWalletId: HotWalletId, - auth: HotAuth, + requestedAuth: RequestedAuth, block: suspend (auth: HotAuth) -> T, ): T { return runCatchingWrongPassInternal( hotWalletId = hotWalletId, - originalAuth = auth, - auth = auth, + originalAuth = requestedAuth, + requestedAuth = requestedAuth, block = { blockAuth -> val result = block(blockAuth) // Update biometry auth if the original auth was password updateBiometryAuthIfNeeded( hotWalletId = hotWalletId, - originalAuth = auth, + originalAuth = requestedAuth.auth, ) result @@ -161,13 +163,19 @@ class DefaultHotWalletAccessor @Inject constructor( private suspend fun runCatchingWrongPassInternal( hotWalletId: HotWalletId, - originalAuth: HotAuth, - auth: HotAuth, + originalAuth: RequestedAuth, + requestedAuth: RequestedAuth, block: suspend (auth: HotAuth) -> T, ): T = runSuspendCatching { - block(auth) + block(requestedAuth.auth).also { + val request = requestedAuth.attemptRequest + if (request != null) { + hotWalletPasswordRequester.successfulAuthentication(request) + } + hotWalletPasswordRequester.dismiss() + } }.getOrElse { exception -> - if (auth is HotAuth.Biometry && (exception.isBiometryError() || exception.isBiometryReset())) { + if (requestedAuth.auth is HotAuth.Biometry && (exception.isBiometryError() || exception.isBiometryReset())) { val shouldRetryBiometry = exception is TangemSdkError.AuthenticationCanceled // fallback to password if biometry fails @@ -179,7 +187,7 @@ class DefaultHotWalletAccessor @Inject constructor( return@getOrElse runCatchingWrongPassInternal( hotWalletId = hotWalletId, originalAuth = originalAuth, - auth = passAuth, + requestedAuth = passAuth, block = block, ) } @@ -190,29 +198,30 @@ class DefaultHotWalletAccessor @Inject constructor( // If the exception is a wrong password, we need to request the password again - hotWalletPasswordRequester.wrongPassword() + requestedAuth.attemptRequest?.let { hotWalletPasswordRequester.wrongPassword(it) } val passResult = requestPassword( hotWalletId = hotWalletId, - hasBiometry = originalAuth is HotAuth.Biometry, + hasBiometry = originalAuth.auth is HotAuth.Biometry, ) runCatchingWrongPassInternal( hotWalletId = hotWalletId, originalAuth = originalAuth, - auth = passResult, + requestedAuth = passResult, block = block, ) } - private suspend fun requestPassword(hotWalletId: HotWalletId, hasBiometry: Boolean): HotAuth { + private suspend fun requestPassword(hotWalletId: HotWalletId, hasBiometry: Boolean): RequestedAuth { val attemptRequest = HotWalletPasswordRequester.AttemptRequest( hotWalletId = hotWalletId, authMode = false, hasBiometry = hasBiometry, ) - return hotWalletPasswordRequester.requestPassword(attemptRequest).toAuth() + val auth = hotWalletPasswordRequester.requestPassword(attemptRequest).toAuth() ?: throw TangemSdkError.UserCancelled() + return RequestedAuth(auth, attemptRequest) } private suspend fun isAccessCodeRequired(): Boolean { diff --git a/domain/wallets/src/main/java/com/tangem/domain/wallets/hot/HotWalletPasswordRequester.kt b/domain/wallets/src/main/java/com/tangem/domain/wallets/hot/HotWalletPasswordRequester.kt index ce09a009a3..da4d02e39e 100644 --- a/domain/wallets/src/main/java/com/tangem/domain/wallets/hot/HotWalletPasswordRequester.kt +++ b/domain/wallets/src/main/java/com/tangem/domain/wallets/hot/HotWalletPasswordRequester.kt @@ -2,6 +2,7 @@ package com.tangem.domain.wallets.hot import com.tangem.hot.sdk.model.HotAuth import com.tangem.hot.sdk.model.HotWalletId +import java.util.UUID /** * Interface for requesting the password for a hot wallet. @@ -11,13 +12,19 @@ interface HotWalletPasswordRequester { /** * Sets state to show wrong password state. + * + * The result is attributed to [attemptRequest] (the request that produced it), not to whatever + * request currently owns the dialog. This prevents a late callback of one request from + * incrementing the failed-attempt counter of a different wallet that meanwhile replaced it. */ - suspend fun wrongPassword() + suspend fun wrongPassword(attemptRequest: AttemptRequest) /** * Sets state to show successful authentication state. + * + * Attributed to [attemptRequest], see [wrongPassword]. */ - suspend fun successfulAuthentication() + suspend fun successfulAuthentication(attemptRequest: AttemptRequest) /** * Requests the user to enter the password for the hot wallet. @@ -38,11 +45,14 @@ interface HotWalletPasswordRequester { * In auth mode user can be deleted after failed attempts. * @param hasBiometry Indicates whether to show biometric authentication option to the user. * Will be ignored if the device does not support biometry at the moment of the request. + * @param requestId Unique identity of this request, used to bind async result callbacks + * (wrong/successful) back to the exact request that produced them. */ data class AttemptRequest( val hotWalletId: HotWalletId, val authMode: Boolean, val hasBiometry: Boolean, + val requestId: String = UUID.randomUUID().toString(), ) sealed class Result { diff --git a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/DefaultHotAccessCodeRequestComponent.kt b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/DefaultHotAccessCodeRequestComponent.kt index 81c6c906cd..b105c04993 100644 --- a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/DefaultHotAccessCodeRequestComponent.kt +++ b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/DefaultHotAccessCodeRequestComponent.kt @@ -21,12 +21,12 @@ internal class DefaultHotAccessCodeRequestComponent @AssistedInject constructor( private val model: HotAccessCodeRequestModel = getOrCreateModel(params) - override suspend fun wrongPassword() { - model.wrongAccessCode() + override suspend fun wrongPassword(attemptRequest: HotWalletPasswordRequester.AttemptRequest) { + model.wrongAccessCode(attemptRequest) } - override suspend fun successfulAuthentication() { - model.successfulAuthentication() + override suspend fun successfulAuthentication(attemptRequest: HotWalletPasswordRequester.AttemptRequest) { + model.successfulAuthentication(attemptRequest) } override suspend fun requestPassword( diff --git a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModel.kt b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModel.kt index 1102b254e5..34dd1cf459 100644 --- a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModel.kt +++ b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModel.kt @@ -67,7 +67,7 @@ internal class HotAccessCodeRequestModel @Inject constructor( currentRequest.value = attemptRequest result.value = null // Reset the result when showing the dialog - subscribeToAttempts(id = attemptRequest.attemptId) + subscribeToAttempts(attemptRequest) uiState.update { it.copy( isShown = true, @@ -88,31 +88,37 @@ internal class HotAccessCodeRequestModel @Inject constructor( dismissState() } - suspend fun wrongAccessCode() { - val currentRequest = currentRequest.value ?: return - hotAccessCodeAttemptsRepository.incrementAttempts(currentRequest.attemptId) - uiState.update { - it.copy( - accessCodeColor = PinTextColor.WrongCode, - onAccessCodeChange = {}, - useBiometricVisible = currentRequest.isBiometryButtonVisible(), - ) + suspend fun wrongAccessCode(attemptRequest: HotWalletPasswordRequester.AttemptRequest) { + hotAccessCodeAttemptsRepository.incrementAttempts(attemptRequest.attemptId) + // Reflect the wrong-code UI only if this request still owns the visible dialog. + if (isCurrentRequest(attemptRequest)) { + uiState.update { state -> + state.copy( + accessCodeColor = PinTextColor.WrongCode, + onAccessCodeChange = {}, + useBiometricVisible = attemptRequest.isBiometryButtonVisible(), + ) + } } delay(timeMillis = 500) // Delay to show the wrong access code state } - suspend fun successfulAuthentication() { - val currentRequest = currentRequest.value ?: return - hotAccessCodeAttemptsRepository.resetAttempts(currentRequest.hotWalletId) - uiState.update { - it.copy( - accessCodeColor = PinTextColor.Success, - onAccessCodeChange = {}, - ) + suspend fun successfulAuthentication(attemptRequest: HotWalletPasswordRequester.AttemptRequest) { + hotAccessCodeAttemptsRepository.resetAttempts(attemptRequest.hotWalletId) + if (isCurrentRequest(attemptRequest)) { + uiState.update { + it.copy( + accessCodeColor = PinTextColor.Success, + onAccessCodeChange = {}, + ) + } } delay(timeMillis = 200) // Delay to show the success state } + private fun isCurrentRequest(attemptRequest: HotWalletPasswordRequester.AttemptRequest): Boolean = + currentRequest.value?.requestId == attemptRequest.requestId + private fun getInitialState() = HotAccessCodeRequestUM( onDismiss = ::dismiss, onAccessCodeChange = ::onAccessCodeChange, @@ -146,7 +152,8 @@ internal class HotAccessCodeRequestModel @Inject constructor( } } - private fun subscribeToAttempts(id: HotWalletAccessCodeAttemptsRepository.AttemptId) { + private fun subscribeToAttempts(attemptRequest: HotWalletPasswordRequester.AttemptRequest) { + val id = attemptRequest.attemptId fun remainingSecondsToText(remainingSeconds: Int): TextReference? { return if (remainingSeconds > 0) { resourceReference( @@ -202,7 +209,7 @@ internal class HotAccessCodeRequestModel @Inject constructor( ) } } - Attempts.Deletion -> deleteUserWallet() + Attempts.Deletion -> deleteUserWallet(attemptRequest) } } @@ -217,8 +224,10 @@ internal class HotAccessCodeRequestModel @Inject constructor( .any { it is UserWallet.Hot && it.hotWalletId == id } } - private suspend fun deleteUserWallet() { + private suspend fun deleteUserWallet(expectedRequest: HotWalletPasswordRequester.AttemptRequest) { val currentRequest = currentRequest.value ?: return + // Only delete if the request whose threshold was crossed is still the one owning the dialog. + if (currentRequest.requestId != expectedRequest.requestId) return val userWallet = userWalletsListRepository.userWalletsSync() .firstOrNull { it is UserWallet.Hot && it.hotWalletId == currentRequest.hotWalletId } ?: return diff --git a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/proxy/HotWalletPasswordRequesterProxy.kt b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/proxy/HotWalletPasswordRequesterProxy.kt index 1968bae87c..7c3799925f 100644 --- a/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/proxy/HotWalletPasswordRequesterProxy.kt +++ b/features/hot-wallet/impl/src/main/kotlin/com/tangem/features/hotwallet/accesscoderequest/proxy/HotWalletPasswordRequesterProxy.kt @@ -13,9 +13,11 @@ class HotWalletPasswordRequesterProxy @Inject constructor() : HotWalletPasswordR val componentRequester = MutableStateFlow(null) - override suspend fun wrongPassword() = call { wrongPassword() } + override suspend fun wrongPassword(attemptRequest: HotWalletPasswordRequester.AttemptRequest) = + call { wrongPassword(attemptRequest) } - override suspend fun successfulAuthentication() = call { successfulAuthentication() } + override suspend fun successfulAuthentication(attemptRequest: HotWalletPasswordRequester.AttemptRequest) = + call { successfulAuthentication(attemptRequest) } override suspend fun requestPassword( attemptRequest: HotWalletPasswordRequester.AttemptRequest, diff --git a/features/hot-wallet/impl/src/test/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModelTest.kt b/features/hot-wallet/impl/src/test/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModelTest.kt new file mode 100644 index 0000000000..e8edc8e418 --- /dev/null +++ b/features/hot-wallet/impl/src/test/kotlin/com/tangem/features/hotwallet/accesscoderequest/HotAccessCodeRequestModelTest.kt @@ -0,0 +1,169 @@ +package com.tangem.features.hotwallet.accesscoderequest + +import com.tangem.core.analytics.api.AnalyticsEventHandler +import com.tangem.domain.assetsdiscovery.usecase.StartAssetsDiscoveryUseCase +import com.tangem.domain.common.wallets.UserWalletsListRepository +import com.tangem.domain.models.wallet.UserWallet +import com.tangem.domain.models.wallet.UserWalletId +import com.tangem.domain.settings.CanUseBiometryUseCase +import com.tangem.domain.wallets.hot.HotWalletAccessCodeAttemptsRepository +import com.tangem.domain.wallets.hot.HotWalletAccessCodeAttemptsRepository.AttemptId +import com.tangem.domain.wallets.hot.HotWalletAccessCodeAttemptsRepository.Attempts +import com.tangem.domain.wallets.hot.HotWalletPasswordRequester.AttemptRequest +import com.tangem.domain.wallets.usecase.DeleteWalletUseCase +import com.tangem.features.hotwallet.HotWalletFeatureToggles +import com.tangem.hot.sdk.model.HotWalletId +import com.tangem.utils.coroutines.TestingCoroutineDispatcherProvider +import io.mockk.clearMocks +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +@OptIn(ExperimentalCoroutinesApi::class) +internal class HotAccessCodeRequestModelTest { + + private val hotAccessCodeAttemptsRepository: HotWalletAccessCodeAttemptsRepository = mockk(relaxed = true) + private val userWalletsListRepository: UserWalletsListRepository = mockk() + private val deleteWalletUseCase: DeleteWalletUseCase = mockk(relaxed = true) + private val canUseBiometryUseCase: CanUseBiometryUseCase = mockk(relaxed = true) + private val analyticsEventHandler: AnalyticsEventHandler = mockk(relaxUnitFun = true) + private val startAssetsDiscoveryUseCase: StartAssetsDiscoveryUseCase = mockk(relaxed = true) + private val hotWalletFeatureToggles: HotWalletFeatureToggles = mockk { + every { isAssetsDiscoveryEnabled } returns false + } + + private val hotWalletIdA: HotWalletId = mockk() + private val hotWalletIdB: HotWalletId = mockk() + private val walletIdA = UserWalletId("A") + private val walletIdB = UserWalletId("B") + + private val userWalletA: UserWallet.Hot = mockk { + every { hotWalletId } returns hotWalletIdA + every { walletId } returns walletIdA + } + private val userWalletB: UserWallet.Hot = mockk { + every { hotWalletId } returns hotWalletIdB + every { walletId } returns walletIdB + } + + private val requestA = AttemptRequest(hotWalletId = hotWalletIdA, authMode = true, hasBiometry = false) + private val requestB = AttemptRequest(hotWalletId = hotWalletIdB, authMode = true, hasBiometry = false) + + private val attemptIdA = AttemptId(hotWalletId = hotWalletIdA, auth = true) + private val attemptIdB = AttemptId(hotWalletId = hotWalletIdB, auth = true) + + @BeforeEach + fun setUp() { + clearMocks(hotAccessCodeAttemptsRepository, deleteWalletUseCase, answers = false) + coEvery { userWalletsListRepository.userWalletsSync() } returns listOf(userWalletA, userWalletB) + every { hotAccessCodeAttemptsRepository.getAttempts(any()) } returns emptyFlow() + } + + @Test + fun `GIVEN request A replaced by B WHEN late wrong code of A THEN attempt attributed to A not B`() = runTest { + // Arrange + val model = createModel(this) + model.show(requestA) + advanceUntilIdle() + model.show(requestB) // B now owns the dialog + advanceUntilIdle() + + // Act + model.wrongAccessCode(requestA) // late callback belonging to A + advanceUntilIdle() + + // Assert + coVerify(exactly = 1) { hotAccessCodeAttemptsRepository.incrementAttempts(attemptIdA) } + coVerify(exactly = 0) { hotAccessCodeAttemptsRepository.incrementAttempts(attemptIdB) } + + model.onDestroy() + } + + @Test + fun `GIVEN request A replaced by B WHEN late success of A THEN reset attributed to A not B`() = runTest { + // Arrange + val model = createModel(this) + model.show(requestA) + advanceUntilIdle() + model.show(requestB) + advanceUntilIdle() + + // Act + model.successfulAuthentication(requestA) // late callback belonging to A + advanceUntilIdle() + + // Assert + coVerify(exactly = 1) { hotAccessCodeAttemptsRepository.resetAttempts(hotWalletIdA) } + coVerify(exactly = 0) { hotAccessCodeAttemptsRepository.resetAttempts(hotWalletIdB) } + + model.onDestroy() + } + + @Test + fun `GIVEN request A owns the dialog WHEN wrong code of A THEN attempt incremented for A`() = runTest { + // Arrange + val model = createModel(this) + model.show(requestA) + advanceUntilIdle() + + // Act + model.wrongAccessCode(requestA) + advanceUntilIdle() + + // Assert + coVerify(exactly = 1) { hotAccessCodeAttemptsRepository.incrementAttempts(attemptIdA) } + + model.onDestroy() + } + + @Test + fun `GIVEN B reaches deletion threshold WHEN B owns the dialog THEN only B is deleted`() = runTest { + // Arrange + every { hotAccessCodeAttemptsRepository.getAttempts(attemptIdB) } returns flowOf(Attempts.Deletion) + val model = createModel(this) + + // Act + model.show(requestB) + advanceUntilIdle() + + // Assert + coVerify(exactly = 1) { deleteWalletUseCase(walletIdB) } + coVerify(exactly = 0) { deleteWalletUseCase(walletIdA) } + + model.onDestroy() + } + + private fun createModel(testScope: TestScope): HotAccessCodeRequestModel { + return HotAccessCodeRequestModel( + dispatchers = testScope.createTestingCoroutineDispatcherProvider(), + hotAccessCodeAttemptsRepository = hotAccessCodeAttemptsRepository, + userWalletsListRepository = userWalletsListRepository, + deleteWalletUseCase = deleteWalletUseCase, + canUseBiometryUseCase = canUseBiometryUseCase, + analyticsEventHandler = analyticsEventHandler, + startAssetsDiscoveryUseCase = startAssetsDiscoveryUseCase, + hotWalletFeatureToggles = hotWalletFeatureToggles, + ) + } + + private fun TestScope.createTestingCoroutineDispatcherProvider(): TestingCoroutineDispatcherProvider { + val testDispatcher = StandardTestDispatcher(testScheduler) + return TestingCoroutineDispatcherProvider( + main = testDispatcher, + mainImmediate = testDispatcher, + io = testDispatcher, + default = testDispatcher, + single = testDispatcher, + ) + } +} \ No newline at end of file