Consolidate unlock vault functionality for auth into a single helper method (#3690)

This commit is contained in:
David Perez 2024-08-07 10:45:04 -05:00 committed by GitHub
parent 22dae88b42
commit 9484eebc70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 92 additions and 161 deletions

View file

@ -6,6 +6,7 @@ import com.bitwarden.core.InitUserCryptoRequest
import com.bitwarden.crypto.HashPurpose
import com.bitwarden.crypto.Kdf
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountTokensJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson
@ -517,17 +518,13 @@ class AuthRepositoryImpl(
return error.toLoginErrorResult()
},
) {
vaultRepository.unlockVault(
userId = userId,
email = profile.email,
kdf = profile.toSdkParams(),
unlockVault(
accountProfile = profile,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.AuthRequest(
requestPrivateKey = requestPrivateKey,
method = AuthRequestMethod.UserKey(protectedUserKey = asymmetricalKey),
),
// We should already have the org keys from the login sync.
organizationKeys = authDiskSource.getOrganizationKeys(userId = userId),
)
}
@ -1359,7 +1356,6 @@ class AuthRepositoryImpl(
deviceData: DeviceDataModel?,
orgIdentifier: String?,
): LoginResult = userStateTransaction {
val userStateJson = loginResponse.toUserState(
previousUserState = authDiskSource.userState,
environmentUrlData = environmentRepository.environment.environmentUrlData,
@ -1377,7 +1373,6 @@ class AuthRepositoryImpl(
if (isDeviceUnlockAvailable) {
unlockVaultWithTdeOnLoginSuccess(
loginResponse = loginResponse,
userId = userId,
userStateJson = userStateJson,
deviceData = deviceData,
)
@ -1385,9 +1380,8 @@ class AuthRepositoryImpl(
password?.let {
unlockVaultWithPasswordOnLoginSuccess(
loginResponse = loginResponse,
userId = userId,
userStateJson = userStateJson,
password = password,
password = it,
)
}
}
@ -1477,9 +1471,11 @@ class AuthRepositoryImpl(
return LoginResult.TwoFactorRequired
}
/**
* Attempt to unlock the current user's vault with password data.
*/
private suspend fun unlockVaultWithPasswordOnLoginSuccess(
loginResponse: GetTokenResponseJson.Success,
userId: String,
userStateJson: UserStateJson,
password: String?,
): VaultUnlockResult? {
@ -1487,25 +1483,21 @@ class AuthRepositoryImpl(
val masterPassword = password ?: return null
val privateKey = loginResponse.privateKey ?: return null
val key = loginResponse.key ?: return null
return vaultRepository.unlockVault(
userId = userId,
email = userStateJson.activeAccount.profile.email,
kdf = userStateJson.activeAccount.profile.toSdkParams(),
userKey = key,
return unlockVault(
accountProfile = userStateJson.activeAccount.profile,
privateKey = privateKey,
masterPassword = masterPassword,
// We can separately unlock vault for organization data after
// receiving the sync response if this data is currently absent.
organizationKeys = null,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = masterPassword,
userKey = key,
),
)
}
/**
* Attempt to unlock the vault with trusted device specific data.
* Attempt to unlock the current user's vault with trusted device specific data.
*/
private suspend fun unlockVaultWithTdeOnLoginSuccess(
loginResponse: GetTokenResponseJson.Success,
userId: String,
userStateJson: UserStateJson,
deviceData: DeviceDataModel?,
): VaultUnlockResult? {
@ -1513,10 +1505,8 @@ class AuthRepositoryImpl(
// These values will only be null during the Just-in-Time provisioning flow.
if (loginResponse.privateKey != null && loginResponse.key != null) {
deviceData?.let { model ->
return vaultRepository.unlockVault(
userId = userId,
email = userStateJson.activeAccount.profile.email,
kdf = userStateJson.activeAccount.profile.toSdkParams(),
return unlockVault(
accountProfile = userStateJson.activeAccount.profile,
privateKey = loginResponse.privateKey,
initUserCryptoMethod = InitUserCryptoMethod.AuthRequest(
requestPrivateKey = model.privateKey,
@ -1530,9 +1520,6 @@ class AuthRepositoryImpl(
}
?: AuthRequestMethod.UserKey(protectedUserKey = model.asymmetricalKey),
),
// We can separately unlock vault for organization data after
// receiving the sync response if this data is currently absent.
organizationKeys = null,
)
// We are purposely not storing the master password hash here since it is not
// formatted in in a manner that we can use. We will store it properly the next
@ -1578,18 +1565,13 @@ class AuthRepositoryImpl(
?.let { request ->
// For approved requests the key will always be present.
val userKey = requireNotNull(request.key)
vaultUnlockResult = vaultRepository.unlockVault(
userId = userId,
email = userStateJson.activeAccount.profile.email,
kdf = userStateJson.activeAccount.profile.toSdkParams(),
vaultUnlockResult = unlockVault(
accountProfile = userStateJson.activeAccount.profile,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.AuthRequest(
requestPrivateKey = pendingRequest.requestPrivateKey,
method = AuthRequestMethod.UserKey(protectedUserKey = userKey),
),
// We can separately unlock vault for organization data after
// receiving the sync response if this data is currently absent.
organizationKeys = null,
)
authDiskSource.storeUserKey(userId = userId, userKey = userKey)
}
@ -1610,19 +1592,14 @@ class AuthRepositoryImpl(
return null
}
vaultUnlockResult = vaultRepository.unlockVault(
userId = userId,
email = userStateJson.activeAccount.profile.email,
kdf = userStateJson.activeAccount.profile.toSdkParams(),
vaultUnlockResult = unlockVault(
accountProfile = userStateJson.activeAccount.profile,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.DeviceKey(
deviceKey = deviceKey,
protectedDevicePrivateKey = encryptedPrivateKey,
deviceProtectedUserKey = encryptedUserKey,
),
// We can separately unlock vault for organization data after
// receiving the sync response if this data is currently absent.
organizationKeys = null,
)
if (vaultUnlockResult is VaultUnlockResult.Success) {
@ -1631,6 +1608,29 @@ class AuthRepositoryImpl(
return vaultUnlockResult
}
/**
* A helper function to unlock the vault for the user associated with the [accountProfile].
*/
private suspend fun unlockVault(
accountProfile: AccountJson.Profile,
privateKey: String,
initUserCryptoMethod: InitUserCryptoMethod,
): VaultUnlockResult {
val userId = accountProfile.userId
return vaultRepository.unlockVault(
userId = userId,
email = accountProfile.email,
kdf = accountProfile.toSdkParams(),
privateKey = privateKey,
initUserCryptoMethod = initUserCryptoMethod,
// The value for the organization keys here will typically be null. We can separately
// unlock the vault for organization data after receiving the sync response if this
// data is currently absent. These keys may be present during certain multi-phase login
// processes.
organizationKeys = authDiskSource.getOrganizationKeys(userId = userId),
)
}
/**
* A helper function to check for a vault unlock related error when logging in.
*

View file

@ -2,7 +2,6 @@ package com.x8bit.bitwarden.data.vault.repository
import android.net.Uri
import com.bitwarden.core.DateTime
import com.bitwarden.crypto.Kdf
import com.bitwarden.exporters.ExportFormat
import com.bitwarden.fido.Fido2CredentialAutofillView
import com.bitwarden.sdk.Fido2CredentialStore
@ -188,23 +187,6 @@ interface VaultRepository : CipherManager, VaultLockManager {
pin: String,
): VaultUnlockResult
/**
* Attempt to unlock the vault with the specified user information.
*
* Note that when [organizationKeys] is absent, no attempt will be made to unlock the vault
* for organization data.
*/
@Suppress("LongParameterList")
suspend fun unlockVault(
userId: String,
masterPassword: String,
email: String,
kdf: Kdf,
userKey: String,
privateKey: String,
organizationKeys: Map<String, String>?,
): VaultUnlockResult
/**
* Attempt to create a send. The [fileUri] _must_ be present when the given [SendView] has a
* [SendView.type] of [SendType.FILE].

View file

@ -4,7 +4,6 @@ import android.net.Uri
import com.bitwarden.core.DateTime
import com.bitwarden.core.InitOrgCryptoRequest
import com.bitwarden.core.InitUserCryptoMethod
import com.bitwarden.crypto.Kdf
import com.bitwarden.exporters.ExportFormat
import com.bitwarden.fido.Fido2CredentialAutofillView
import com.bitwarden.sdk.Fido2CredentialStore
@ -607,27 +606,6 @@ class VaultRepositoryImpl(
)
}
override suspend fun unlockVault(
userId: String,
masterPassword: String,
email: String,
kdf: Kdf,
userKey: String,
privateKey: String,
organizationKeys: Map<String, String>?,
): VaultUnlockResult =
unlockVault(
userId = userId,
email = email,
kdf = kdf,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = masterPassword,
userKey = userKey,
),
organizationKeys = organizationKeys,
)
override suspend fun createSend(
sendView: SendView,
fileUri: Uri?,

View file

@ -409,10 +409,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -475,10 +477,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.syncIfNecessary()
}
@ -1465,10 +1469,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -1508,10 +1514,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.syncIfNecessary()
}
@ -1548,10 +1556,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.AuthenticationError(expectedErrorMessage)
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -1592,10 +1602,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
}
@ -1678,10 +1690,9 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = any(),
initUserCryptoMethod = any(),
privateKey = any(),
organizationKeys = null,
masterPassword = PASSWORD,
)
}
}
@ -1715,10 +1726,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -1756,10 +1769,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.syncIfNecessary()
}
@ -1908,10 +1923,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -1996,10 +2013,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.InvalidStateError
every {
@ -2055,10 +2074,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.syncIfNecessary() } just runs
@ -2095,10 +2116,12 @@ class AuthRepositoryTest {
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key!!,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = PASSWORD,
userKey = successResponse.key!!,
),
privateKey = successResponse.privateKey!!,
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.syncIfNecessary()
}

View file

@ -1591,58 +1591,6 @@ class VaultRepositoryTest {
}
}
@Test
fun `unlockVault should delegate to the VaultLockManager`() = runTest {
val userId = "userId"
val kdf = MOCK_PROFILE.toSdkParams()
val email = MOCK_PROFILE.email
val masterPassword = "drowssap"
val userKey = "12345"
val privateKey = "54321"
val organizationKeys = mapOf("orgId1" to "orgKey1")
val mockVaultUnlockResult = mockk<VaultUnlockResult>()
coEvery {
vaultLockManager.unlockVault(
userId = userId,
kdf = kdf,
email = email,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = masterPassword,
userKey = userKey,
),
organizationKeys = organizationKeys,
)
} returns mockVaultUnlockResult
val result = vaultRepository.unlockVault(
userId = userId,
masterPassword = masterPassword,
kdf = kdf,
email = email,
userKey = userKey,
privateKey = privateKey,
organizationKeys = organizationKeys,
)
assertEquals(mockVaultUnlockResult, result)
coVerify(exactly = 1) {
vaultLockManager.unlockVault(
userId = userId,
kdf = kdf,
email = email,
privateKey = privateKey,
initUserCryptoMethod = InitUserCryptoMethod.Password(
password = masterPassword,
userKey = userKey,
),
organizationKeys = organizationKeys,
)
}
}
@Test
fun `getVaultItemStateFlow should update to Error when a sync fails generically`() =
runTest {