mirror of
https://github.com/bitwarden/android.git
synced 2024-11-21 17:05:44 +03:00
PM-11264: Ensure user has valid timeout action after migrating to Key Connector (#3807)
This commit is contained in:
parent
91f039ecb6
commit
eb2ba8e598
6 changed files with 219 additions and 11 deletions
|
@ -73,6 +73,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult
|
|||
import com.x8bit.bitwarden.data.auth.repository.util.WebAuthResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.activeUserIdChangesFlow
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.policyInformation
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toRemovedPasswordUserStateJson
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toUserState
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toUserStateJsonWithPassword
|
||||
|
@ -873,7 +874,13 @@ class AuthRepositoryImpl(
|
|||
masterPassword = masterPassword,
|
||||
kdf = profile.toSdkParams(),
|
||||
)
|
||||
.onSuccess { vaultRepository.sync() }
|
||||
.onSuccess {
|
||||
authDiskSource.userState = authDiskSource
|
||||
.userState
|
||||
?.toRemovedPasswordUserStateJson(userId = userId)
|
||||
vaultRepository.sync()
|
||||
settingsRepository.setDefaultsIfNecessary(userId = userId)
|
||||
}
|
||||
.fold(
|
||||
onFailure = { RemovePasswordResult.Error },
|
||||
onSuccess = { RemovePasswordResult.Success },
|
||||
|
|
|
@ -13,6 +13,32 @@ import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockData
|
|||
import com.x8bit.bitwarden.data.vault.repository.util.statusFor
|
||||
import com.x8bit.bitwarden.ui.platform.base.util.toHexColorRepresentation
|
||||
|
||||
/**
|
||||
* Updates the given [UserStateJson] with the data to indicate that the password has been removed.
|
||||
* The original will be returned if the [userId] does not match any accounts in the [UserStateJson].
|
||||
*/
|
||||
fun UserStateJson.toRemovedPasswordUserStateJson(
|
||||
userId: String,
|
||||
): UserStateJson {
|
||||
val account = this.accounts[userId] ?: return this
|
||||
val profile = account.profile
|
||||
val updatedUserDecryptionOptions = profile
|
||||
.userDecryptionOptions
|
||||
?.copy(hasMasterPassword = false)
|
||||
?: UserDecryptionOptionsJson(
|
||||
hasMasterPassword = false,
|
||||
trustedDeviceUserDecryptionOptions = null,
|
||||
keyConnectorUserDecryptionOptions = null,
|
||||
)
|
||||
val updatedProfile = profile.copy(userDecryptionOptions = updatedUserDecryptionOptions)
|
||||
val updatedAccount = account.copy(profile = updatedProfile)
|
||||
return this.copy(
|
||||
accounts = accounts
|
||||
.toMutableMap()
|
||||
.apply { replace(userId, updatedAccount) },
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Updates the given [UserStateJson] with the data from the [syncResponse] to return a new
|
||||
* [UserStateJson]. The original will be returned if the sync response does not match any accounts
|
||||
|
|
|
@ -325,14 +325,23 @@ class SettingsRepositoryImpl(
|
|||
|
||||
override fun setDefaultsIfNecessary(userId: String) {
|
||||
// Set Vault Settings defaults
|
||||
if (!isVaultTimeoutActionSet(userId = userId)) {
|
||||
val hasMasterPassword = authDiskSource
|
||||
.userState
|
||||
?.activeAccount
|
||||
?.profile
|
||||
?.userDecryptionOptions
|
||||
?.hasMasterPassword != false
|
||||
val timeoutAction = settingsDiskSource.getVaultTimeoutAction(userId = userId)
|
||||
val hasPin = authDiskSource.getPinProtectedUserKey(userId = userId) != null
|
||||
val hasBiometrics = authDiskSource.getUserBiometricUnlockKey(userId = userId) != null
|
||||
// The timeout action cannot be "lock" if you do not have master password, pin, or
|
||||
// biometrics unlock enabled.
|
||||
val hasInvalidTimeoutAction = timeoutAction == VaultTimeoutAction.LOCK &&
|
||||
!hasPin &&
|
||||
!hasBiometrics &&
|
||||
!hasMasterPassword
|
||||
if (!isVaultTimeoutActionSet(userId = userId) || hasInvalidTimeoutAction) {
|
||||
storeVaultTimeout(userId, VaultTimeout.FifteenMinutes)
|
||||
val hasMasterPassword = authDiskSource
|
||||
.userState
|
||||
?.activeAccount
|
||||
?.profile
|
||||
?.userDecryptionOptions
|
||||
?.hasMasterPassword != false
|
||||
storeVaultTimeoutAction(
|
||||
userId = userId,
|
||||
vaultTimeoutAction = if (!hasMasterPassword) {
|
||||
|
|
|
@ -87,6 +87,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.DuoCallbackTokenResult
|
|||
import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.WebAuthResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toOrganizations
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toRemovedPasswordUserStateJson
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toUserState
|
||||
import com.x8bit.bitwarden.data.auth.util.YubiKeyResult
|
||||
|
@ -255,12 +256,18 @@ class AuthRepositoryTest {
|
|||
|
||||
@BeforeEach
|
||||
fun beforeEach() {
|
||||
mockkStatic(GetTokenResponseJson.Success::toUserState)
|
||||
mockkStatic(
|
||||
GetTokenResponseJson.Success::toUserState,
|
||||
UserStateJson::toRemovedPasswordUserStateJson,
|
||||
)
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
fun tearDown() {
|
||||
unmockkStatic(GetTokenResponseJson.Success::toUserState)
|
||||
unmockkStatic(
|
||||
GetTokenResponseJson.Success::toUserState,
|
||||
UserStateJson::toRemovedPasswordUserStateJson,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -4341,13 +4348,19 @@ class AuthRepositoryTest {
|
|||
kdf = PROFILE_1.toSdkParams(),
|
||||
)
|
||||
} returns Unit.asSuccess()
|
||||
every {
|
||||
SINGLE_USER_STATE_1.toRemovedPasswordUserStateJson(userId = USER_ID_1)
|
||||
} returns SINGLE_USER_STATE_1
|
||||
every { vaultRepository.sync() } just runs
|
||||
every { settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } just runs
|
||||
|
||||
val result = repository.removePassword(masterPassword = PASSWORD)
|
||||
|
||||
assertEquals(RemovePasswordResult.Success, result)
|
||||
verify(exactly = 1) {
|
||||
SINGLE_USER_STATE_1.toRemovedPasswordUserStateJson(userId = USER_ID_1)
|
||||
vaultRepository.sync()
|
||||
settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -24,6 +24,117 @@ import org.junit.jupiter.api.Assertions.assertEquals
|
|||
import org.junit.jupiter.api.Test
|
||||
|
||||
class UserStateJsonExtensionsTest {
|
||||
@Test
|
||||
fun `toUpdatedUserStateJn should do nothing for a non-matching account`() {
|
||||
val originalUserState = UserStateJson(
|
||||
activeUserId = "activeUserId",
|
||||
accounts = mapOf("activeUserId" to mockk()),
|
||||
)
|
||||
assertEquals(
|
||||
originalUserState,
|
||||
originalUserState.toRemovedPasswordUserStateJson(userId = "nonActiveUserId"),
|
||||
)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `toUpdatedUserStateJn should create user decryption options without a password if not present`() {
|
||||
val originalProfile = AccountJson.Profile(
|
||||
userId = "activeUserId",
|
||||
email = "email",
|
||||
isEmailVerified = true,
|
||||
name = "name",
|
||||
stamp = null,
|
||||
organizationId = null,
|
||||
avatarColorHex = null,
|
||||
hasPremium = true,
|
||||
forcePasswordResetReason = null,
|
||||
kdfType = KdfTypeJson.ARGON2_ID,
|
||||
kdfIterations = 600000,
|
||||
kdfMemory = 16,
|
||||
kdfParallelism = 4,
|
||||
userDecryptionOptions = null,
|
||||
)
|
||||
val originalAccount = AccountJson(
|
||||
profile = originalProfile,
|
||||
tokens = null,
|
||||
settings = AccountJson.Settings(environmentUrlData = null),
|
||||
)
|
||||
val originalUserState = UserStateJson(
|
||||
activeUserId = "activeUserId",
|
||||
accounts = mapOf("activeUserId" to originalAccount),
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
UserStateJson(
|
||||
activeUserId = "activeUserId",
|
||||
accounts = mapOf(
|
||||
"activeUserId" to originalAccount.copy(
|
||||
profile = originalProfile.copy(
|
||||
userDecryptionOptions = UserDecryptionOptionsJson(
|
||||
hasMasterPassword = false,
|
||||
trustedDeviceUserDecryptionOptions = null,
|
||||
keyConnectorUserDecryptionOptions = null,
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
originalUserState.toRemovedPasswordUserStateJson(userId = "activeUserId"),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `toUpdatedUserStateJn should update user decryption options to not have a password`() {
|
||||
val originalProfile = AccountJson.Profile(
|
||||
userId = "activeUserId",
|
||||
email = "email",
|
||||
isEmailVerified = true,
|
||||
name = "name",
|
||||
stamp = null,
|
||||
organizationId = null,
|
||||
avatarColorHex = null,
|
||||
hasPremium = true,
|
||||
forcePasswordResetReason = null,
|
||||
kdfType = KdfTypeJson.ARGON2_ID,
|
||||
kdfIterations = 600000,
|
||||
kdfMemory = 16,
|
||||
kdfParallelism = 4,
|
||||
userDecryptionOptions = UserDecryptionOptionsJson(
|
||||
hasMasterPassword = true,
|
||||
trustedDeviceUserDecryptionOptions = null,
|
||||
keyConnectorUserDecryptionOptions = null,
|
||||
),
|
||||
)
|
||||
val originalAccount = AccountJson(
|
||||
profile = originalProfile,
|
||||
tokens = null,
|
||||
settings = AccountJson.Settings(environmentUrlData = null),
|
||||
)
|
||||
val originalUserState = UserStateJson(
|
||||
activeUserId = "activeUserId",
|
||||
accounts = mapOf("activeUserId" to originalAccount),
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
UserStateJson(
|
||||
activeUserId = "activeUserId",
|
||||
accounts = mapOf(
|
||||
"activeUserId" to originalAccount.copy(
|
||||
profile = originalProfile.copy(
|
||||
userDecryptionOptions = UserDecryptionOptionsJson(
|
||||
hasMasterPassword = false,
|
||||
trustedDeviceUserDecryptionOptions = null,
|
||||
keyConnectorUserDecryptionOptions = null,
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
originalUserState.toRemovedPasswordUserStateJson(userId = "activeUserId"),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `toUpdatedUserStateJson should do nothing for a non-matching account`() {
|
||||
val originalUserState = UserStateJson(
|
||||
|
|
|
@ -176,7 +176,11 @@ class SettingsRepositoryTest {
|
|||
)
|
||||
|
||||
// Updating the Vault settings values and calling setDefaultsIfNecessary again has no
|
||||
// effect on the currently stored values.
|
||||
// effect on the currently stored values since we have a way to unlock the vault.
|
||||
fakeAuthDiskSource.storePinProtectedUserKey(
|
||||
userId = USER_ID,
|
||||
pinProtectedUserKey = "pinProtectedKey",
|
||||
)
|
||||
fakeSettingsDiskSource.apply {
|
||||
storeVaultTimeoutInMinutes(
|
||||
userId = USER_ID,
|
||||
|
@ -195,6 +199,44 @@ class SettingsRepositoryTest {
|
|||
)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `setDefaultsIfNecessary should reset default values to LOGOUT for the given user without a password if necessary`() {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
assertNull(fakeSettingsDiskSource.getVaultTimeoutInMinutes(userId = USER_ID))
|
||||
assertNull(fakeSettingsDiskSource.getVaultTimeoutAction(userId = USER_ID))
|
||||
|
||||
settingsRepository.setDefaultsIfNecessary(userId = USER_ID)
|
||||
|
||||
// Calling once sets values
|
||||
assertEquals(15, fakeSettingsDiskSource.getVaultTimeoutInMinutes(userId = USER_ID))
|
||||
assertEquals(
|
||||
VaultTimeoutAction.LOGOUT,
|
||||
fakeSettingsDiskSource.getVaultTimeoutAction(userId = USER_ID),
|
||||
)
|
||||
|
||||
// Updating the Vault settings values and calling setDefaultsIfNecessary again has no
|
||||
// effect on the currently stored values.
|
||||
fakeSettingsDiskSource.apply {
|
||||
storeVaultTimeoutInMinutes(
|
||||
userId = USER_ID,
|
||||
vaultTimeoutInMinutes = 240,
|
||||
)
|
||||
storeVaultTimeoutAction(
|
||||
userId = USER_ID,
|
||||
vaultTimeoutAction = VaultTimeoutAction.LOCK,
|
||||
)
|
||||
}
|
||||
// This will reset the setting because the user does not have a method to unlock the vault
|
||||
// so you cannot use the "lock" timeout action, it must be "logout".
|
||||
settingsRepository.setDefaultsIfNecessary(userId = USER_ID)
|
||||
assertEquals(15, fakeSettingsDiskSource.getVaultTimeoutInMinutes(userId = USER_ID))
|
||||
assertEquals(
|
||||
VaultTimeoutAction.LOGOUT,
|
||||
fakeSettingsDiskSource.getVaultTimeoutAction(userId = USER_ID),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `appLanguage should pull from and update SettingsDiskSource`() {
|
||||
assertEquals(
|
||||
|
|
Loading…
Reference in a new issue