mirror of
https://github.com/bitwarden/android.git
synced 2025-03-16 11:18:45 +03:00
The stored 'shouldTrustDevice' boolean ise scoped to the userId (#1287)
This commit is contained in:
parent
1365a2a4fe
commit
2cf8b05a87
8 changed files with 91 additions and 39 deletions
|
@ -28,11 +28,6 @@ interface AuthDiskSource {
|
|||
*/
|
||||
var rememberedOrgIdentifier: String?
|
||||
|
||||
/**
|
||||
* The currently persisted state indicating that the user has trusted this device.
|
||||
*/
|
||||
var shouldTrustDevice: Boolean
|
||||
|
||||
/**
|
||||
* The currently persisted user state information (or `null` if not set).
|
||||
*/
|
||||
|
@ -50,6 +45,21 @@ interface AuthDiskSource {
|
|||
*/
|
||||
fun clearData(userId: String)
|
||||
|
||||
/**
|
||||
* Retrieves the state indicating that the user has chosen to trust this device.
|
||||
*
|
||||
* Note: This indicates intent to trust the device, the device may not be trusted yet.
|
||||
*/
|
||||
fun getShouldTrustDevice(userId: String): Boolean
|
||||
|
||||
/**
|
||||
* Stores the boolean indicating that the user has chosen to trust this device for the given
|
||||
* [userId].
|
||||
*
|
||||
* Note: This indicates intent to trust the device, the device may not be trusted yet.
|
||||
*/
|
||||
fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?)
|
||||
|
||||
/**
|
||||
* Retrieves the "last active time" for the given [userId], in milliseconds.
|
||||
*
|
||||
|
|
|
@ -106,12 +106,6 @@ class AuthDiskSourceImpl(
|
|||
)
|
||||
}
|
||||
|
||||
override var shouldTrustDevice: Boolean
|
||||
get() = requireNotNull(getBoolean(key = SHOULD_TRUST_DEVICE_KEY, default = false))
|
||||
set(value) {
|
||||
putBoolean(key = SHOULD_TRUST_DEVICE_KEY, value = value)
|
||||
}
|
||||
|
||||
override val userStateFlow: Flow<UserStateJson?>
|
||||
get() = mutableUserStateFlow
|
||||
.onSubscription { emit(userState) }
|
||||
|
@ -135,6 +129,13 @@ class AuthDiskSourceImpl(
|
|||
// indefinitely unless the TDE flow explicitly removes them.
|
||||
}
|
||||
|
||||
override fun getShouldTrustDevice(userId: String): Boolean =
|
||||
requireNotNull(getBoolean(key = "${SHOULD_TRUST_DEVICE_KEY}_$userId", default = false))
|
||||
|
||||
override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) {
|
||||
putBoolean("${SHOULD_TRUST_DEVICE_KEY}_$userId", shouldTrustDevice)
|
||||
}
|
||||
|
||||
override fun getLastActiveTimeMillis(userId: String): Long? =
|
||||
getLong(key = "${LAST_ACTIVE_TIME_KEY}_$userId")
|
||||
|
||||
|
|
|
@ -17,13 +17,18 @@ class TrustedDeviceManagerImpl(
|
|||
private val devicesService: DevicesService,
|
||||
) : TrustedDeviceManager {
|
||||
override suspend fun trustThisDeviceIfNecessary(userId: String): Result<Boolean> =
|
||||
if (!authDiskSource.shouldTrustDevice) {
|
||||
if (!authDiskSource.getShouldTrustDevice(userId = userId)) {
|
||||
false.asSuccess()
|
||||
} else {
|
||||
vaultSdkSource
|
||||
.getTrustDevice(userId = userId)
|
||||
.flatMap { trustThisDevice(userId = userId, trustDeviceResponse = it) }
|
||||
.also { authDiskSource.shouldTrustDevice = false }
|
||||
.also {
|
||||
authDiskSource.storeShouldTrustDevice(
|
||||
userId = userId,
|
||||
shouldTrustDevice = null,
|
||||
)
|
||||
}
|
||||
.map { true }
|
||||
}
|
||||
|
||||
|
@ -47,6 +52,6 @@ class TrustedDeviceManagerImpl(
|
|||
previousUserState = requireNotNull(authDiskSource.userState),
|
||||
)
|
||||
}
|
||||
.also { authDiskSource.shouldTrustDevice = false }
|
||||
.also { authDiskSource.storeShouldTrustDevice(userId = userId, shouldTrustDevice = null) }
|
||||
.map { Unit }
|
||||
}
|
||||
|
|
|
@ -277,7 +277,13 @@ class AuthRepositoryImpl(
|
|||
|
||||
override var rememberedOrgIdentifier: String? by authDiskSource::rememberedOrgIdentifier
|
||||
|
||||
override var shouldTrustDevice: Boolean by authDiskSource::shouldTrustDevice
|
||||
override var shouldTrustDevice: Boolean
|
||||
get() = activeUserId?.let { authDiskSource.getShouldTrustDevice(userId = it) } ?: false
|
||||
set(value) {
|
||||
activeUserId?.let {
|
||||
authDiskSource.storeShouldTrustDevice(userId = it, shouldTrustDevice = value)
|
||||
}
|
||||
}
|
||||
|
||||
override var hasPendingAccountAddition: Boolean
|
||||
by mutableHasPendingAccountAdditionStateFlow::value
|
||||
|
@ -371,7 +377,7 @@ class AuthRepositoryImpl(
|
|||
userId = userId,
|
||||
email = account.profile.email,
|
||||
orgPublicKey = organizationKeys.publicKey,
|
||||
rememberDevice = authDiskSource.shouldTrustDevice,
|
||||
rememberDevice = authDiskSource.getShouldTrustDevice(userId = userId),
|
||||
)
|
||||
}
|
||||
.flatMap { keys ->
|
||||
|
|
|
@ -123,19 +123,20 @@ class AuthDiskSourceTest {
|
|||
|
||||
@Test
|
||||
fun `shouldTrustDevice should pull from and update SharedPreferences`() {
|
||||
val shouldTrustDeviceKey = "bwPreferencesStorage:shouldTrustDevice"
|
||||
val userId = "userId"
|
||||
val shouldTrustDeviceKey = "bwPreferencesStorage:shouldTrustDevice_$userId"
|
||||
|
||||
// Shared preferences and the disk source start with the same value.
|
||||
assertFalse(authDiskSource.shouldTrustDevice)
|
||||
assertFalse(authDiskSource.getShouldTrustDevice(userId = userId))
|
||||
assertFalse(fakeSharedPreferences.getBoolean(shouldTrustDeviceKey, false))
|
||||
|
||||
// Updating the disk source updates shared preferences
|
||||
authDiskSource.shouldTrustDevice = true
|
||||
authDiskSource.storeShouldTrustDevice(userId = userId, shouldTrustDevice = true)
|
||||
assertTrue(fakeSharedPreferences.getBoolean(shouldTrustDeviceKey, false))
|
||||
|
||||
// Update SharedPreferences updates the disk source
|
||||
fakeSharedPreferences.edit { putBoolean(shouldTrustDeviceKey, false) }
|
||||
assertFalse(authDiskSource.shouldTrustDevice)
|
||||
assertFalse(authDiskSource.getShouldTrustDevice(userId = userId))
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -190,6 +191,11 @@ class AuthDiskSourceTest {
|
|||
userId = userId,
|
||||
pendingAuthRequest = pendingAuthRequestJson,
|
||||
)
|
||||
val shouldTrustDevice = true
|
||||
authDiskSource.storeShouldTrustDevice(
|
||||
userId = userId,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
val deviceKey = "deviceKey"
|
||||
authDiskSource.storeDeviceKey(userId = userId, deviceKey = deviceKey)
|
||||
authDiskSource.storeUserBiometricUnlockKey(
|
||||
|
@ -241,6 +247,7 @@ class AuthDiskSourceTest {
|
|||
// We do not clear these even when you call clear storage
|
||||
assertEquals(pendingAuthRequestJson, authDiskSource.getPendingAuthRequest(userId = userId))
|
||||
assertEquals(deviceKey, authDiskSource.getDeviceKey(userId = userId))
|
||||
assertEquals(shouldTrustDevice, authDiskSource.getShouldTrustDevice(userId = userId))
|
||||
|
||||
// These should be cleared
|
||||
assertNull(authDiskSource.getUserBiometricUnlockKey(userId = userId))
|
||||
|
|
|
@ -26,6 +26,7 @@ class FakeAuthDiskSource : AuthDiskSource {
|
|||
mutableMapOf<String, MutableSharedFlow<AccountTokensJson?>>()
|
||||
private val mutableUserStateFlow = bufferedMutableSharedFlow<UserStateJson?>(replay = 1)
|
||||
|
||||
private val storedShouldTrustDevice = mutableMapOf<String, Boolean?>()
|
||||
private val storedLastActiveTimeMillis = mutableMapOf<String, Long?>()
|
||||
private val storedInvalidUnlockAttempts = mutableMapOf<String, Int?>()
|
||||
private val storedUserKeys = mutableMapOf<String, String?>()
|
||||
|
@ -44,8 +45,6 @@ class FakeAuthDiskSource : AuthDiskSource {
|
|||
private val storedMasterPasswordHashes = mutableMapOf<String, String?>()
|
||||
private val storedPolicies = mutableMapOf<String, List<SyncResponseJson.Policy>?>()
|
||||
|
||||
override var shouldTrustDevice: Boolean = false
|
||||
|
||||
override var userState: UserStateJson? = null
|
||||
set(value) {
|
||||
field = value
|
||||
|
@ -75,6 +74,13 @@ class FakeAuthDiskSource : AuthDiskSource {
|
|||
mutableAccountTokensFlowMap.remove(userId)
|
||||
}
|
||||
|
||||
override fun getShouldTrustDevice(userId: String): Boolean =
|
||||
storedShouldTrustDevice[userId] ?: false
|
||||
|
||||
override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) {
|
||||
storedShouldTrustDevice[userId] = shouldTrustDevice
|
||||
}
|
||||
|
||||
override fun getLastActiveTimeMillis(userId: String): Long? =
|
||||
storedLastActiveTimeMillis[userId]
|
||||
|
||||
|
|
|
@ -53,7 +53,7 @@ class TrustedDeviceManagerTests {
|
|||
@Test
|
||||
fun `trustThisDeviceIfNecessary when shouldTrustDevice false should return success with false`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.shouldTrustDevice = false
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID, shouldTrustDevice = false)
|
||||
|
||||
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
|
||||
|
||||
|
@ -71,7 +71,7 @@ class TrustedDeviceManagerTests {
|
|||
|
||||
@Test
|
||||
fun `trustThisDeviceIfNecessary when getTrustDevice fails should return failure`() = runTest {
|
||||
fakeAuthDiskSource.shouldTrustDevice = true
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID, shouldTrustDevice = true)
|
||||
val error = Throwable("Fail")
|
||||
coEvery {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
|
@ -80,7 +80,7 @@ class TrustedDeviceManagerTests {
|
|||
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
|
||||
|
||||
assertEquals(error.asFailure(), result)
|
||||
assertFalse(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID))
|
||||
coVerify(exactly = 1) {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
}
|
||||
|
@ -107,7 +107,7 @@ class TrustedDeviceManagerTests {
|
|||
protectedDevicePublicKey = protectedDevicePublicKey,
|
||||
)
|
||||
val error = Throwable("Fail")
|
||||
fakeAuthDiskSource.shouldTrustDevice = true
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID, shouldTrustDevice = true)
|
||||
coEvery {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
} returns trustedDeviceResponse.asSuccess()
|
||||
|
@ -123,7 +123,7 @@ class TrustedDeviceManagerTests {
|
|||
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
|
||||
|
||||
assertEquals(error.asFailure(), result)
|
||||
assertFalse(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID))
|
||||
coVerify(exactly = 1) {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
devicesService.trustDevice(
|
||||
|
@ -155,7 +155,7 @@ class TrustedDeviceManagerTests {
|
|||
creationDate = ZonedDateTime.parse("2024-09-13T01:00:00.00Z"),
|
||||
)
|
||||
fakeAuthDiskSource.userState = DEFAULT_USER_STATE
|
||||
fakeAuthDiskSource.shouldTrustDevice = true
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID, shouldTrustDevice = true)
|
||||
coEvery {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
} returns trustedDeviceResponse.asSuccess()
|
||||
|
@ -178,7 +178,7 @@ class TrustedDeviceManagerTests {
|
|||
|
||||
assertEquals(true.asSuccess(), result)
|
||||
fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey)
|
||||
assertFalse(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID))
|
||||
fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE)
|
||||
coVerify(exactly = 1) {
|
||||
vaultSdkSource.getTrustDevice(userId = USER_ID)
|
||||
|
@ -211,7 +211,7 @@ class TrustedDeviceManagerTests {
|
|||
creationDate = ZonedDateTime.parse("2024-09-13T01:00:00.00Z"),
|
||||
)
|
||||
fakeAuthDiskSource.userState = DEFAULT_USER_STATE
|
||||
fakeAuthDiskSource.shouldTrustDevice = true
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID, shouldTrustDevice = true)
|
||||
coEvery {
|
||||
devicesService.trustDevice(
|
||||
appId = "testUniqueAppId",
|
||||
|
@ -234,7 +234,7 @@ class TrustedDeviceManagerTests {
|
|||
|
||||
assertEquals(Unit.asSuccess(), result)
|
||||
fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey)
|
||||
assertFalse(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID))
|
||||
fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE)
|
||||
coVerify(exactly = 1) {
|
||||
devicesService.trustDevice(
|
||||
|
|
|
@ -535,16 +535,17 @@ class AuthRepositoryTest {
|
|||
|
||||
@Test
|
||||
fun `shouldTrustDevice should directly access the authDiskSource`() {
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
// AuthDiskSource and the repository start with the same default value.
|
||||
assertFalse(repository.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1))
|
||||
|
||||
// Updating the repository updates AuthDiskSource
|
||||
repository.shouldTrustDevice = true
|
||||
assertTrue(fakeAuthDiskSource.shouldTrustDevice)
|
||||
assertTrue(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1))
|
||||
|
||||
// Updating AuthDiskSource updates the repository
|
||||
fakeAuthDiskSource.shouldTrustDevice = false
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID_1, shouldTrustDevice = false)
|
||||
assertFalse(repository.shouldTrustDevice)
|
||||
}
|
||||
|
||||
|
@ -815,7 +816,11 @@ class AuthRepositoryTest {
|
|||
)
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.rememberedOrgIdentifier = orgIdentifier
|
||||
fakeAuthDiskSource.shouldTrustDevice = shouldTrustDevice
|
||||
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(
|
||||
userId = USER_ID_1,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
coEvery {
|
||||
organizationService.getOrganizationAutoEnrollStatus(orgIdentifier)
|
||||
} returns orgAutoEnrollStatusResponse.asSuccess()
|
||||
|
@ -871,7 +876,10 @@ class AuthRepositoryTest {
|
|||
)
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.rememberedOrgIdentifier = orgIdentifier
|
||||
fakeAuthDiskSource.shouldTrustDevice = shouldTrustDevice
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(
|
||||
userId = USER_ID_1,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
coEvery {
|
||||
organizationService.getOrganizationAutoEnrollStatus(orgIdentifier)
|
||||
} returns orgAutoEnrollStatusResponse.asSuccess()
|
||||
|
@ -937,7 +945,10 @@ class AuthRepositoryTest {
|
|||
)
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.rememberedOrgIdentifier = orgIdentifier
|
||||
fakeAuthDiskSource.shouldTrustDevice = shouldTrustDevice
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(
|
||||
userId = USER_ID_1,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
coEvery {
|
||||
organizationService.getOrganizationAutoEnrollStatus(orgIdentifier)
|
||||
} returns orgAutoEnrollStatusResponse.asSuccess()
|
||||
|
@ -1019,7 +1030,10 @@ class AuthRepositoryTest {
|
|||
)
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.rememberedOrgIdentifier = orgIdentifier
|
||||
fakeAuthDiskSource.shouldTrustDevice = shouldTrustDevice
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(
|
||||
userId = USER_ID_1,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
coEvery {
|
||||
organizationService.getOrganizationAutoEnrollStatus(orgIdentifier)
|
||||
} returns orgAutoEnrollStatusResponse.asSuccess()
|
||||
|
@ -1104,7 +1118,10 @@ class AuthRepositoryTest {
|
|||
)
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.rememberedOrgIdentifier = orgIdentifier
|
||||
fakeAuthDiskSource.shouldTrustDevice = shouldTrustDevice
|
||||
fakeAuthDiskSource.storeShouldTrustDevice(
|
||||
userId = USER_ID_1,
|
||||
shouldTrustDevice = shouldTrustDevice,
|
||||
)
|
||||
coEvery {
|
||||
organizationService.getOrganizationAutoEnrollStatus(orgIdentifier)
|
||||
} returns orgAutoEnrollStatusResponse.asSuccess()
|
||||
|
|
Loading…
Add table
Reference in a new issue