[PM-24204] Correct TOTP generation to use cipherId instead of totpCode (#5599)

This commit is contained in:
Patrick Honkonen 2025-07-28 14:45:37 -04:00 committed by GitHub
parent f589546e6a
commit 02b5cbb199
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 40 additions and 82 deletions

View File

@ -24,11 +24,12 @@ class AutofillTotpManagerImpl(
if (settingsRepository.isAutoCopyTotpDisabled) return if (settingsRepository.isAutoCopyTotpDisabled) return
val isPremium = authRepository.userStateFlow.value?.activeAccount?.isPremium == true val isPremium = authRepository.userStateFlow.value?.activeAccount?.isPremium == true
if (!isPremium && !cipherView.organizationUseTotp) return if (!isPremium && !cipherView.organizationUseTotp) return
val totpCode = cipherView.login?.totp ?: return cipherView.login?.totp ?: return
val cipherId = cipherView.id ?: return
val totpResult = vaultRepository.generateTotp( val totpResult = vaultRepository.generateTotp(
time = clock.instant(), time = clock.instant(),
totpCode = totpCode, cipherId = cipherId,
) )
if (totpResult is GenerateTotpResult.Success) { if (totpResult is GenerateTotpResult.Success) {

View File

@ -1,6 +1,5 @@
package com.x8bit.bitwarden.data.vault.datasource.sdk package com.x8bit.bitwarden.data.vault.datasource.sdk
import com.bitwarden.core.DateTime
import com.bitwarden.core.DerivePinKeyResponse import com.bitwarden.core.DerivePinKeyResponse
import com.bitwarden.core.InitOrgCryptoRequest import com.bitwarden.core.InitOrgCryptoRequest
import com.bitwarden.core.InitUserCryptoMethod import com.bitwarden.core.InitUserCryptoMethod
@ -373,15 +372,6 @@ interface VaultSdkSource {
passwordHistoryList: List<PasswordHistory>, passwordHistoryList: List<PasswordHistory>,
): Result<List<PasswordHistoryView>> ): Result<List<PasswordHistoryView>>
/**
* Generate a verification code and the period using the totp code.
*/
suspend fun generateTotp(
userId: String,
totp: String,
time: DateTime,
): Result<TotpResponse>
/** /**
* Generate a verification code for the given [cipherListView] and [time]. * Generate a verification code for the given [cipherListView] and [time].
*/ */

View File

@ -1,6 +1,5 @@
package com.x8bit.bitwarden.data.vault.datasource.sdk package com.x8bit.bitwarden.data.vault.datasource.sdk
import com.bitwarden.core.DateTime
import com.bitwarden.core.DeriveKeyConnectorRequest import com.bitwarden.core.DeriveKeyConnectorRequest
import com.bitwarden.core.DerivePinKeyResponse import com.bitwarden.core.DerivePinKeyResponse
import com.bitwarden.core.InitOrgCryptoRequest import com.bitwarden.core.InitOrgCryptoRequest
@ -417,19 +416,6 @@ class VaultSdkSourceImpl(
.decryptList(list = passwordHistoryList) .decryptList(list = passwordHistoryList)
} }
override suspend fun generateTotp(
userId: String,
totp: String,
time: DateTime,
): Result<TotpResponse> = runCatchingWithLogs {
getClient(userId = userId)
.vault()
.generateTotp(
key = totp,
time = time,
)
}
override suspend fun generateTotpForCipherListView( override suspend fun generateTotpForCipherListView(
userId: String, userId: String,
cipherListView: CipherListView, cipherListView: CipherListView,

View File

@ -225,7 +225,7 @@ interface VaultRepository : CipherManager, VaultLockManager {
/** /**
* Attempt to get the verification code and the period. * 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. * Attempt to delete a send.

View File

@ -802,15 +802,24 @@ class VaultRepositoryImpl(
} }
override suspend fun generateTotp( override suspend fun generateTotp(
totpCode: String, cipherId: String,
time: DateTime, time: DateTime,
): GenerateTotpResult { ): GenerateTotpResult {
val userId = activeUserId val userId = activeUserId
?: return GenerateTotpResult.Error(error = NoActiveUserException()) ?: 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, time = time,
userId = userId, userId = userId,
totp = totpCode, cipherListView = cipherListView,
) )
.fold( .fold(
onSuccess = { onSuccess = {

View File

@ -414,7 +414,7 @@ class SearchViewModel @Inject constructor(
action: ListingItemOverflowAction.VaultAction.CopyTotpClick, action: ListingItemOverflowAction.VaultAction.CopyTotpClick,
) { ) {
viewModelScope.launch { viewModelScope.launch {
val result = vaultRepo.generateTotp(action.totpCode, clock.instant()) val result = vaultRepo.generateTotp(action.cipherId, clock.instant())
sendAction(SearchAction.Internal.GenerateTotpResultReceive(result)) sendAction(SearchAction.Internal.GenerateTotpResultReceive(result))
} }
} }

View File

@ -1239,7 +1239,7 @@ class VaultItemListingViewModel @Inject constructor(
action: ListingItemOverflowAction.VaultAction.CopyTotpClick, action: ListingItemOverflowAction.VaultAction.CopyTotpClick,
) { ) {
viewModelScope.launch { viewModelScope.launch {
val result = vaultRepository.generateTotp(action.totpCode, clock.instant()) val result = vaultRepository.generateTotp(action.cipherId, clock.instant())
sendAction(VaultItemListingsAction.Internal.GenerateTotpResultReceive(result)) sendAction(VaultItemListingsAction.Internal.GenerateTotpResultReceive(result))
} }
} }

View File

@ -136,7 +136,7 @@ sealed class ListingItemOverflowAction : Parcelable {
*/ */
@Parcelize @Parcelize
data class CopyTotpClick( data class CopyTotpClick(
val totpCode: String, val cipherId: String,
override val requiresPasswordReprompt: Boolean, override val requiresPasswordReprompt: Boolean,
) : VaultAction() { ) : VaultAction() {
override val title: Text get() = BitwardenString.copy_totp.asText() override val title: Text get() = BitwardenString.copy_totp.asText()

View File

@ -39,7 +39,7 @@ fun CipherListView.toOverflowActions(
this.login?.totp this.login?.totp
?.let { ?.let {
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = it, cipherId = cipherId,
requiresPasswordReprompt = hasMasterPassword, requiresPasswordReprompt = hasMasterPassword,
) )
} }

View File

@ -634,7 +634,7 @@ class VaultViewModel @Inject constructor(
action: ListingItemOverflowAction.VaultAction.CopyTotpClick, action: ListingItemOverflowAction.VaultAction.CopyTotpClick,
) { ) {
viewModelScope.launch { viewModelScope.launch {
val result = vaultRepository.generateTotp(action.totpCode, clock.instant()) val result = vaultRepository.generateTotp(action.cipherId, clock.instant())
sendAction(VaultAction.Internal.GenerateTotpResultReceive(result)) sendAction(VaultAction.Internal.GenerateTotpResultReceive(result))
} }
} }

View File

@ -128,7 +128,7 @@ class AutofillTotpManagerTest {
} }
every { loginView.totp } returns TOTP_CODE every { loginView.totp } returns TOTP_CODE
coEvery { coEvery {
vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), totpCode = TOTP_CODE) vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), cipherId = "cipherId")
} returns generateTotpResult } returns generateTotpResult
autofillTotpManager.tryCopyTotpToClipboard(cipherView = cipherView) autofillTotpManager.tryCopyTotpToClipboard(cipherView = cipherView)
@ -141,7 +141,7 @@ class AutofillTotpManagerTest {
settingsRepository.isAutoCopyTotpDisabled settingsRepository.isAutoCopyTotpDisabled
} }
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), totpCode = TOTP_CODE) vaultRepository.generateTotp(time = FIXED_CLOCK.instant(), cipherId = "cipherId")
} }
} }
} }

View File

@ -69,9 +69,6 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test import org.junit.jupiter.api.Test
import java.security.MessageDigest import java.security.MessageDigest
import java.time.Clock
import java.time.Instant
import java.time.ZoneOffset
@Suppress("LargeClass") @Suppress("LargeClass")
class VaultSdkSourceTest { class VaultSdkSourceTest {
@ -977,30 +974,6 @@ class VaultSdkSourceTest {
coVerify { sdkClientManager.getOrCreateClient(userId = userId) } 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 @Test
fun `generateTotpForCipherListView should call SDK and return a Result with correct data`() = fun `generateTotpForCipherListView should call SDK and return a Result with correct data`() =
runTest { runTest {
@ -1422,7 +1395,3 @@ private val DEFAULT_FIDO_2_AUTH_REQUEST = AuthenticateFido2CredentialRequest(
isUserVerificationSupported = true, isUserVerificationSupported = true,
selectedCipherView = createMockCipherView(number = 1), selectedCipherView = createMockCipherView(number = 1),
) )
private val FIXED_CLOCK: Clock = Clock.fixed(
Instant.parse("2023-10-27T12:00:00Z"),
ZoneOffset.UTC,
)

View File

@ -143,10 +143,10 @@ class TotpCodeManagerTest {
runTest { runTest {
val totpResponse = TotpResponse("123456", 30u) val totpResponse = TotpResponse("123456", 30u)
coEvery { coEvery {
vaultSdkSource.generateTotp( vaultSdkSource.generateTotpForCipherListView(
userId = any(), userId = any(),
totp = any(),
time = any(), time = any(),
cipherListView = any(),
) )
} returns totpResponse.asSuccess() } returns totpResponse.asSuccess()

View File

@ -2740,7 +2740,7 @@ class VaultRepositoryTest {
fakeAuthDiskSource.userState = null fakeAuthDiskSource.userState = null
val result = vaultRepository.generateTotp( val result = vaultRepository.generateTotp(
totpCode = "totpCode", cipherId = "totpCode",
time = DateTime.now(), time = DateTime.now(),
) )
@ -2753,13 +2753,16 @@ class VaultRepositoryTest {
@Test @Test
fun `generateTotp should return a success result on getting a code`() = runTest { fun `generateTotp should return a success result on getting a code`() = runTest {
val totpResponse = TotpResponse("Testcode", 30u) val totpResponse = TotpResponse("Testcode", 30u)
val userId = "mockId-1"
coEvery { coEvery {
vaultSdkSource.generateTotp(any(), any(), any()) vaultSdkSource.generateTotpForCipherListView(any(), any(), any())
} returns totpResponse.asSuccess() } returns totpResponse.asSuccess()
fakeAuthDiskSource.userState = MOCK_USER_STATE fakeAuthDiskSource.userState = MOCK_USER_STATE
setVaultToUnlocked(userId = userId)
setupDataStateFlow(userId = userId)
val result = vaultRepository.generateTotp( val result = vaultRepository.generateTotp(
totpCode = "testCode", cipherId = "mockId-1",
time = DateTime.now(), time = DateTime.now(),
) )

View File

@ -1007,7 +1007,7 @@ class SearchViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
SearchAction.OverflowOptionClick( SearchAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),
@ -1035,7 +1035,7 @@ class SearchViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
SearchAction.OverflowOptionClick( SearchAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),

View File

@ -52,7 +52,7 @@ fun createMockDisplayItemForCipher(
cipherId = "mockId-$number", cipherId = "mockId-$number",
), ),
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = "mockTotp-$number", cipherId = "mockId-$number",
requiresPasswordReprompt = true, requiresPasswordReprompt = true,
), ),
ListingItemOverflowAction.VaultAction.ViewClick( ListingItemOverflowAction.VaultAction.ViewClick(

View File

@ -1954,7 +1954,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
VaultItemListingsAction.OverflowOptionClick( VaultItemListingsAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),
@ -1982,7 +1982,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
VaultItemListingsAction.OverflowOptionClick( VaultItemListingsAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),

View File

@ -62,7 +62,7 @@ fun createMockDisplayItemForCipher(
cipherId = "mockId-$number", cipherId = "mockId-$number",
), ),
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = "mockTotp-$number", cipherId = "mockId-$number",
requiresPasswordReprompt = requiresPasswordReprompt, requiresPasswordReprompt = requiresPasswordReprompt,
), ),
ListingItemOverflowAction.VaultAction.ViewClick( ListingItemOverflowAction.VaultAction.ViewClick(

View File

@ -45,7 +45,7 @@ class CipherListViewExtensionsTest {
cipherId = id, cipherId = id,
), ),
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = id,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
ListingItemOverflowAction.VaultAction.ViewClick( ListingItemOverflowAction.VaultAction.ViewClick(

View File

@ -1931,7 +1931,7 @@ class VaultViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
VaultAction.OverflowOptionClick( VaultAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),
@ -1959,7 +1959,7 @@ class VaultViewModelTest : BaseViewModelTest() {
viewModel.trySendAction( viewModel.trySendAction(
VaultAction.OverflowOptionClick( VaultAction.OverflowOptionClick(
ListingItemOverflowAction.VaultAction.CopyTotpClick( ListingItemOverflowAction.VaultAction.CopyTotpClick(
totpCode = totpCode, cipherId = totpCode,
requiresPasswordReprompt = false, requiresPasswordReprompt = false,
), ),
), ),