From 02b5cbb199f482116abb06df106b9af22425904f Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Mon, 28 Jul 2025 14:45:37 -0400 Subject: [PATCH] [PM-24204] Correct TOTP generation to use cipherId instead of totpCode (#5599) --- .../manager/AutofillTotpManagerImpl.kt | 5 +-- .../vault/datasource/sdk/VaultSdkSource.kt | 10 ------ .../datasource/sdk/VaultSdkSourceImpl.kt | 14 --------- .../data/vault/repository/VaultRepository.kt | 2 +- .../vault/repository/VaultRepositoryImpl.kt | 15 +++++++-- .../feature/search/SearchViewModel.kt | 2 +- .../itemlisting/VaultItemListingViewModel.kt | 2 +- .../model/ListingItemOverflowAction.kt | 2 +- .../feature/util/CipherListViewExtensions.kt | 2 +- .../ui/vault/feature/vault/VaultViewModel.kt | 2 +- .../manager/AutofillTotpManagerTest.kt | 4 +-- .../datasource/sdk/VaultSdkSourceTest.kt | 31 ------------------- .../data/vault/manager/TotpCodeManagerTest.kt | 4 +-- .../vault/repository/VaultRepositoryTest.kt | 9 ++++-- .../feature/search/SearchViewModelTest.kt | 4 +-- .../feature/search/util/SearchUtil.kt | 2 +- .../VaultItemListingViewModelTest.kt | 4 +-- .../util/VaultItemListingDataUtil.kt | 2 +- .../util/CipherListViewExtensionsTest.kt | 2 +- .../vault/feature/vault/VaultViewModelTest.kt | 4 +-- 20 files changed, 40 insertions(+), 82 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerImpl.kt index 197ab65613..a0f81a895e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerImpl.kt @@ -24,11 +24,12 @@ class AutofillTotpManagerImpl( if (settingsRepository.isAutoCopyTotpDisabled) return val isPremium = authRepository.userStateFlow.value?.activeAccount?.isPremium == true if (!isPremium && !cipherView.organizationUseTotp) return - val totpCode = cipherView.login?.totp ?: return + cipherView.login?.totp ?: return + val cipherId = cipherView.id ?: return val totpResult = vaultRepository.generateTotp( time = clock.instant(), - totpCode = totpCode, + cipherId = cipherId, ) if (totpResult is GenerateTotpResult.Success) { diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt index 91f3329927..722ac10922 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.data.vault.datasource.sdk -import com.bitwarden.core.DateTime import com.bitwarden.core.DerivePinKeyResponse import com.bitwarden.core.InitOrgCryptoRequest import com.bitwarden.core.InitUserCryptoMethod @@ -373,15 +372,6 @@ interface VaultSdkSource { passwordHistoryList: List, ): Result> - /** - * Generate a verification code and the period using the totp code. - */ - suspend fun generateTotp( - userId: String, - totp: String, - time: DateTime, - ): Result - /** * Generate a verification code for the given [cipherListView] and [time]. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt index 368fa9cece..0d75d8cab7 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.data.vault.datasource.sdk -import com.bitwarden.core.DateTime import com.bitwarden.core.DeriveKeyConnectorRequest import com.bitwarden.core.DerivePinKeyResponse import com.bitwarden.core.InitOrgCryptoRequest @@ -417,19 +416,6 @@ class VaultSdkSourceImpl( .decryptList(list = passwordHistoryList) } - override suspend fun generateTotp( - userId: String, - totp: String, - time: DateTime, - ): Result = runCatchingWithLogs { - getClient(userId = userId) - .vault() - .generateTotp( - key = totp, - time = time, - ) - } - override suspend fun generateTotpForCipherListView( userId: String, cipherListView: CipherListView, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt index b4e2dea6b6..289eaea3b6 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt @@ -225,7 +225,7 @@ interface VaultRepository : CipherManager, VaultLockManager { /** * Attempt to get the verification code and the period. */ - suspend fun generateTotp(totpCode: String, time: DateTime): GenerateTotpResult + suspend fun generateTotp(cipherId: String, time: DateTime): GenerateTotpResult /** * Attempt to delete a send. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index e84fd9e303..9baa2ce45e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -802,15 +802,24 @@ class VaultRepositoryImpl( } override suspend fun generateTotp( - totpCode: String, + cipherId: String, time: DateTime, ): GenerateTotpResult { val userId = activeUserId ?: return GenerateTotpResult.Error(error = NoActiveUserException()) - return vaultSdkSource.generateTotp( + val cipherListView = decryptCipherListResultStateFlow + .value + .data + ?.successes + ?.find { it.id == cipherId } + ?: return GenerateTotpResult.Error( + error = IllegalArgumentException(cipherId), + ) + + return vaultSdkSource.generateTotpForCipherListView( time = time, userId = userId, - totp = totpCode, + cipherListView = cipherListView, ) .fold( onSuccess = { diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt index 682b89e2d9..26f28d9dcc 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt @@ -414,7 +414,7 @@ class SearchViewModel @Inject constructor( action: ListingItemOverflowAction.VaultAction.CopyTotpClick, ) { viewModelScope.launch { - val result = vaultRepo.generateTotp(action.totpCode, clock.instant()) + val result = vaultRepo.generateTotp(action.cipherId, clock.instant()) sendAction(SearchAction.Internal.GenerateTotpResultReceive(result)) } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt index 43cd8bb4a2..5fe5e0292f 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt @@ -1239,7 +1239,7 @@ class VaultItemListingViewModel @Inject constructor( action: ListingItemOverflowAction.VaultAction.CopyTotpClick, ) { viewModelScope.launch { - val result = vaultRepository.generateTotp(action.totpCode, clock.instant()) + val result = vaultRepository.generateTotp(action.cipherId, clock.instant()) sendAction(VaultItemListingsAction.Internal.GenerateTotpResultReceive(result)) } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/model/ListingItemOverflowAction.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/model/ListingItemOverflowAction.kt index bda0d3b2d1..8c4e224a60 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/model/ListingItemOverflowAction.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/model/ListingItemOverflowAction.kt @@ -136,7 +136,7 @@ sealed class ListingItemOverflowAction : Parcelable { */ @Parcelize data class CopyTotpClick( - val totpCode: String, + val cipherId: String, override val requiresPasswordReprompt: Boolean, ) : VaultAction() { override val title: Text get() = BitwardenString.copy_totp.asText() diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensions.kt index 64e2602697..ba604dc028 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensions.kt @@ -39,7 +39,7 @@ fun CipherListView.toOverflowActions( this.login?.totp ?.let { ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = it, + cipherId = cipherId, requiresPasswordReprompt = hasMasterPassword, ) } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt index a9ec4ac87b..9df8fb47a2 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt @@ -634,7 +634,7 @@ class VaultViewModel @Inject constructor( action: ListingItemOverflowAction.VaultAction.CopyTotpClick, ) { viewModelScope.launch { - val result = vaultRepository.generateTotp(action.totpCode, clock.instant()) + val result = vaultRepository.generateTotp(action.cipherId, clock.instant()) sendAction(VaultAction.Internal.GenerateTotpResultReceive(result)) } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerTest.kt index d4daf16210..d4a3f5bd84 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/manager/AutofillTotpManagerTest.kt @@ -128,7 +128,7 @@ class AutofillTotpManagerTest { } every { loginView.totp } returns TOTP_CODE coEvery { - vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), totpCode = TOTP_CODE) + vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), cipherId = "cipherId") } returns generateTotpResult autofillTotpManager.tryCopyTotpToClipboard(cipherView = cipherView) @@ -141,7 +141,7 @@ class AutofillTotpManagerTest { settingsRepository.isAutoCopyTotpDisabled } coVerify(exactly = 1) { - vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), totpCode = TOTP_CODE) + vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), cipherId = "cipherId") } } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt index bd0a54b6da..23b50fa7f9 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt @@ -69,9 +69,6 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.security.MessageDigest -import java.time.Clock -import java.time.Instant -import java.time.ZoneOffset @Suppress("LargeClass") class VaultSdkSourceTest { @@ -977,30 +974,6 @@ class VaultSdkSourceTest { coVerify { sdkClientManager.getOrCreateClient(userId = userId) } } - @Test - fun `generateTotp should call SDK and return a Result with correct data`() = runTest { - val userId = "userId" - val totpResponse = TotpResponse("TestCode", 30u) - coEvery { clientVault.generateTotp(any(), any()) } returns totpResponse - - val time = FIXED_CLOCK.instant() - val result = vaultSdkSource.generateTotp( - userId = userId, - totp = "Totp", - time = time, - ) - - assertEquals(totpResponse.asSuccess(), result) - coVerify { - clientVault.generateTotp( - key = "Totp", - time = time, - ) - } - - coVerify { sdkClientManager.getOrCreateClient(userId = userId) } - } - @Test fun `generateTotpForCipherListView should call SDK and return a Result with correct data`() = runTest { @@ -1422,7 +1395,3 @@ private val DEFAULT_FIDO_2_AUTH_REQUEST = AuthenticateFido2CredentialRequest( isUserVerificationSupported = true, selectedCipherView = createMockCipherView(number = 1), ) -private val FIXED_CLOCK: Clock = Clock.fixed( - Instant.parse("2023-10-27T12:00:00Z"), - ZoneOffset.UTC, -) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/TotpCodeManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/TotpCodeManagerTest.kt index 46a17c8e34..7c8e93e9f7 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/TotpCodeManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/TotpCodeManagerTest.kt @@ -143,10 +143,10 @@ class TotpCodeManagerTest { runTest { val totpResponse = TotpResponse("123456", 30u) coEvery { - vaultSdkSource.generateTotp( + vaultSdkSource.generateTotpForCipherListView( userId = any(), - totp = any(), time = any(), + cipherListView = any(), ) } returns totpResponse.asSuccess() diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 5a8e8c4bab..207b27bfd6 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -2740,7 +2740,7 @@ class VaultRepositoryTest { fakeAuthDiskSource.userState = null val result = vaultRepository.generateTotp( - totpCode = "totpCode", + cipherId = "totpCode", time = DateTime.now(), ) @@ -2753,13 +2753,16 @@ class VaultRepositoryTest { @Test fun `generateTotp should return a success result on getting a code`() = runTest { val totpResponse = TotpResponse("Testcode", 30u) + val userId = "mockId-1" coEvery { - vaultSdkSource.generateTotp(any(), any(), any()) + vaultSdkSource.generateTotpForCipherListView(any(), any(), any()) } returns totpResponse.asSuccess() fakeAuthDiskSource.userState = MOCK_USER_STATE + setVaultToUnlocked(userId = userId) + setupDataStateFlow(userId = userId) val result = vaultRepository.generateTotp( - totpCode = "testCode", + cipherId = "mockId-1", time = DateTime.now(), ) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt index 936328320f..2b1c30ffa1 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt @@ -1007,7 +1007,7 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ), @@ -1035,7 +1035,7 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ), diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchUtil.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchUtil.kt index 0cf1105285..64e172eb0d 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchUtil.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchUtil.kt @@ -52,7 +52,7 @@ fun createMockDisplayItemForCipher( cipherId = "mockId-$number", ), ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = "mockTotp-$number", + cipherId = "mockId-$number", requiresPasswordReprompt = true, ), ListingItemOverflowAction.VaultAction.ViewClick( diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt index 8b93fc4f52..84ba22dea2 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt @@ -1954,7 +1954,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { viewModel.trySendAction( VaultItemListingsAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ), @@ -1982,7 +1982,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { viewModel.trySendAction( VaultItemListingsAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ), diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataUtil.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataUtil.kt index a0513c93c4..a3dab54fb8 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataUtil.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataUtil.kt @@ -62,7 +62,7 @@ fun createMockDisplayItemForCipher( cipherId = "mockId-$number", ), ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = "mockTotp-$number", + cipherId = "mockId-$number", requiresPasswordReprompt = requiresPasswordReprompt, ), ListingItemOverflowAction.VaultAction.ViewClick( diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensionsTest.kt index 9a6cd3524d..c3425d36de 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CipherListViewExtensionsTest.kt @@ -45,7 +45,7 @@ class CipherListViewExtensionsTest { cipherId = id, ), ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = id, requiresPasswordReprompt = false, ), ListingItemOverflowAction.VaultAction.ViewClick( diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt index b88bf4213f..9063d4131a 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt @@ -1931,7 +1931,7 @@ class VaultViewModelTest : BaseViewModelTest() { viewModel.trySendAction( VaultAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ), @@ -1959,7 +1959,7 @@ class VaultViewModelTest : BaseViewModelTest() { viewModel.trySendAction( VaultAction.OverflowOptionClick( ListingItemOverflowAction.VaultAction.CopyTotpClick( - totpCode = totpCode, + cipherId = totpCode, requiresPasswordReprompt = false, ), ),