PM-10899: Only display the TDE UI if a selection has not yet been made (#3823)

This commit is contained in:
David Perez 2024-08-26 12:53:35 -05:00 committed by GitHub
parent c36d0851ca
commit e9a7136a9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 102 additions and 11 deletions

View file

@ -60,6 +60,16 @@ interface AuthDiskSource {
*/ */
fun storeShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?) fun storeShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?)
/**
* Retrieves the state indicating that the user has completed login with TDE.
*/
fun getIsTdeLoginComplete(userId: String): Boolean?
/**
* Stores the boolean indicating that the user has completed login with TDE.
*/
fun storeIsTdeLoginComplete(userId: String, isTdeLoginComplete: Boolean?)
/** /**
* Retrieves the state indicating that the user has chosen to trust this device. * Retrieves the state indicating that the user has chosen to trust this device.
* *

View file

@ -39,6 +39,7 @@ private const val TWO_FACTOR_TOKEN_KEY = "twoFactorToken"
private const val MASTER_PASSWORD_HASH_KEY = "keyHash" private const val MASTER_PASSWORD_HASH_KEY = "keyHash"
private const val POLICIES_KEY = "policies" private const val POLICIES_KEY = "policies"
private const val SHOULD_TRUST_DEVICE_KEY = "shouldTrustDevice" private const val SHOULD_TRUST_DEVICE_KEY = "shouldTrustDevice"
private const val TDE_LOGIN_COMPLETE = "tdeLoginComplete"
private const val USES_KEY_CONNECTOR = "usesKeyConnector" private const val USES_KEY_CONNECTOR = "usesKeyConnector"
/** /**
@ -126,6 +127,7 @@ class AuthDiskSourceImpl(
storePolicies(userId = userId, policies = null) storePolicies(userId = userId, policies = null)
storeAccountTokens(userId = userId, accountTokens = null) storeAccountTokens(userId = userId, accountTokens = null)
storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = null) storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = null)
storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = null)
// Do not remove the DeviceKey or PendingAuthRequest on logout, these are persisted // Do not remove the DeviceKey or PendingAuthRequest on logout, these are persisted
// indefinitely unless the TDE flow explicitly removes them. // indefinitely unless the TDE flow explicitly removes them.
@ -147,6 +149,14 @@ class AuthDiskSourceImpl(
getMutableShouldUseKeyConnectorFlowMap(userId = userId).tryEmit(shouldUseKeyConnector) getMutableShouldUseKeyConnectorFlowMap(userId = userId).tryEmit(shouldUseKeyConnector)
} }
override fun getIsTdeLoginComplete(
userId: String,
): Boolean? = getBoolean(key = TDE_LOGIN_COMPLETE.appendIdentifier(userId))
override fun storeIsTdeLoginComplete(userId: String, isTdeLoginComplete: Boolean?) {
putBoolean(TDE_LOGIN_COMPLETE.appendIdentifier(userId), isTdeLoginComplete)
}
override fun getShouldTrustDevice( override fun getShouldTrustDevice(
userId: String, userId: String,
): Boolean? = getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId)) ): Boolean? = getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId))

View file

@ -18,6 +18,7 @@ class TrustedDeviceManagerImpl(
) : TrustedDeviceManager { ) : TrustedDeviceManager {
override suspend fun trustThisDeviceIfNecessary(userId: String): Result<Boolean> = override suspend fun trustThisDeviceIfNecessary(userId: String): Result<Boolean> =
if (authDiskSource.getShouldTrustDevice(userId = userId) != true) { if (authDiskSource.getShouldTrustDevice(userId = userId) != true) {
authDiskSource.storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = true)
false.asSuccess() false.asSuccess()
} else { } else {
vaultSdkSource vaultSdkSource
@ -51,6 +52,7 @@ class TrustedDeviceManagerImpl(
userId = userId, userId = userId,
previousUserState = requireNotNull(authDiskSource.userState), previousUserState = requireNotNull(authDiskSource.userState),
) )
authDiskSource.storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = true)
} }
.also { authDiskSource.storeShouldTrustDevice(userId = userId, shouldTrustDevice = null) } .also { authDiskSource.storeShouldTrustDevice(userId = userId, shouldTrustDevice = null) }
.map { Unit } .map { Unit }

View file

@ -103,6 +103,11 @@ interface AuthRepository : AuthenticatorProvider, AuthRequestManager {
*/ */
var rememberedOrgIdentifier: String? var rememberedOrgIdentifier: String?
/**
* The currently persisted state indicating whether the user has completed login via TDE.
*/
val tdeLoginComplete: Boolean?
/** /**
* The currently persisted state indicating whether the user has trusted this device. * The currently persisted state indicating whether the user has trusted this device.
*/ */

View file

@ -312,6 +312,9 @@ class AuthRepositoryImpl(
override var rememberedOrgIdentifier: String? by authDiskSource::rememberedOrgIdentifier override var rememberedOrgIdentifier: String? by authDiskSource::rememberedOrgIdentifier
override val tdeLoginComplete: Boolean?
get() = activeUserId?.let { authDiskSource.getIsTdeLoginComplete(userId = it) }
override var shouldTrustDevice: Boolean override var shouldTrustDevice: Boolean
get() = activeUserId?.let { authDiskSource.getShouldTrustDevice(userId = it) } ?: false get() = activeUserId?.let { authDiskSource.getShouldTrustDevice(userId = it) } ?: false
set(value) { set(value) {

View file

@ -63,6 +63,7 @@ class RootNavViewModel @Inject constructor(
val specialCircumstance = action.specialCircumstance val specialCircumstance = action.specialCircumstance
val updatedRootNavState = when { val updatedRootNavState = when {
userState?.activeAccount?.trustedDevice?.isDeviceTrusted == false && userState?.activeAccount?.trustedDevice?.isDeviceTrusted == false &&
authRepository.tdeLoginComplete != true &&
!userState.activeAccount.isVaultUnlocked -> RootNavState.TrustedDevice !userState.activeAccount.isVaultUnlocked -> RootNavState.TrustedDevice
userState?.activeAccount?.needsMasterPassword == true -> RootNavState.SetPassword userState?.activeAccount?.needsMasterPassword == true -> RootNavState.SetPassword

View file

@ -159,6 +159,24 @@ class AuthDiskSourceTest {
} }
} }
@Test
fun `tdeLoginComplete should pull from and update SharedPreferences`() {
val userId = "userId"
val isTdeLoginComplete = "bwPreferencesStorage:tdeLoginComplete_$userId"
// Shared preferences and the disk source start with the same value.
assertNull(authDiskSource.getIsTdeLoginComplete(userId = userId))
assertFalse(fakeSharedPreferences.getBoolean(isTdeLoginComplete, false))
// Updating the disk source updates shared preferences
authDiskSource.storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = true)
assertTrue(fakeSharedPreferences.getBoolean(isTdeLoginComplete, false))
// Update SharedPreferences updates the disk source
fakeSharedPreferences.edit { putBoolean(isTdeLoginComplete, false) }
assertFalse(authDiskSource.getIsTdeLoginComplete(userId = userId) ?: true)
}
@Test @Test
fun `shouldTrustDevice should pull from and update SharedPreferences`() { fun `shouldTrustDevice should pull from and update SharedPreferences`() {
val userId = "userId" val userId = "userId"
@ -235,6 +253,7 @@ class AuthDiskSourceTest {
userId = userId, userId = userId,
shouldTrustDevice = shouldTrustDevice, shouldTrustDevice = shouldTrustDevice,
) )
authDiskSource.storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = true)
val deviceKey = "deviceKey" val deviceKey = "deviceKey"
authDiskSource.storeDeviceKey(userId = userId, deviceKey = deviceKey) authDiskSource.storeDeviceKey(userId = userId, deviceKey = deviceKey)
authDiskSource.storeUserBiometricUnlockKey( authDiskSource.storeUserBiometricUnlockKey(
@ -298,6 +317,7 @@ class AuthDiskSourceTest {
assertNull(authDiskSource.getEncryptedPin(userId = userId)) assertNull(authDiskSource.getEncryptedPin(userId = userId))
assertNull(authDiskSource.getMasterPasswordHash(userId = userId)) assertNull(authDiskSource.getMasterPasswordHash(userId = userId))
assertNull(authDiskSource.getShouldUseKeyConnector(userId = userId)) assertNull(authDiskSource.getShouldUseKeyConnector(userId = userId))
assertNull(authDiskSource.getIsTdeLoginComplete(userId = userId))
} }
@Test @Test

View file

@ -29,6 +29,7 @@ class FakeAuthDiskSource : AuthDiskSource {
private val mutableUserStateFlow = bufferedMutableSharedFlow<UserStateJson?>(replay = 1) private val mutableUserStateFlow = bufferedMutableSharedFlow<UserStateJson?>(replay = 1)
private val storedShouldUseKeyConnector = mutableMapOf<String, Boolean?>() private val storedShouldUseKeyConnector = mutableMapOf<String, Boolean?>()
private val storedIsTdeLoginComplete = mutableMapOf<String, Boolean?>()
private val storedShouldTrustDevice = mutableMapOf<String, Boolean?>() private val storedShouldTrustDevice = mutableMapOf<String, Boolean?>()
private val storedInvalidUnlockAttempts = mutableMapOf<String, Int?>() private val storedInvalidUnlockAttempts = mutableMapOf<String, Int?>()
private val storedUserKeys = mutableMapOf<String, String?>() private val storedUserKeys = mutableMapOf<String, String?>()
@ -90,8 +91,14 @@ class FakeAuthDiskSource : AuthDiskSource {
getMutableShouldUseKeyConnectorFlow(userId = userId).tryEmit(shouldUseKeyConnector) getMutableShouldUseKeyConnectorFlow(userId = userId).tryEmit(shouldUseKeyConnector)
} }
override fun getShouldTrustDevice(userId: String): Boolean = override fun getIsTdeLoginComplete(userId: String): Boolean? = storedIsTdeLoginComplete[userId]
storedShouldTrustDevice[userId] ?: false
override fun storeIsTdeLoginComplete(userId: String, isTdeLoginComplete: Boolean?) {
storedIsTdeLoginComplete[userId] = isTdeLoginComplete
}
override fun getShouldTrustDevice(userId: String): Boolean? =
storedShouldTrustDevice[userId]
override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) { override fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) {
storedShouldTrustDevice[userId] = shouldTrustDevice storedShouldTrustDevice[userId] = shouldTrustDevice
@ -232,6 +239,20 @@ class FakeAuthDiskSource : AuthDiskSource {
getMutableAccountTokensFlow(userId = userId).tryEmit(accountTokens) getMutableAccountTokensFlow(userId = userId).tryEmit(accountTokens)
} }
/**
* Assert the the [isTdeLoginComplete] was stored successfully using the [userId].
*/
fun assertIsTdeLoginComplete(userId: String, isTdeLoginComplete: Boolean?) {
assertEquals(isTdeLoginComplete, storedIsTdeLoginComplete[userId])
}
/**
* Assert the the [shouldTrustDevice] was stored successfully using the [userId].
*/
fun assertShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) {
assertEquals(shouldTrustDevice, storedShouldTrustDevice[userId])
}
/** /**
* Assert the the [shouldUseKeyConnector] was stored successfully using the [userId]. * Assert the the [shouldUseKeyConnector] was stored successfully using the [userId].
*/ */

View file

@ -23,7 +23,6 @@ import io.mockk.unmockkStatic
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test import org.junit.jupiter.api.Test
import java.time.ZonedDateTime import java.time.ZonedDateTime
@ -58,6 +57,7 @@ class TrustedDeviceManagerTests {
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID) val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
assertEquals(false.asSuccess(), result) assertEquals(false.asSuccess(), result)
fakeAuthDiskSource.assertIsTdeLoginComplete(userId = USER_ID, isTdeLoginComplete = true)
coVerify(exactly = 0) { coVerify(exactly = 0) {
vaultSdkSource.getTrustDevice(userId = USER_ID) vaultSdkSource.getTrustDevice(userId = USER_ID)
devicesService.trustDevice( devicesService.trustDevice(
@ -80,7 +80,8 @@ class TrustedDeviceManagerTests {
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID) val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
assertEquals(error.asFailure(), result) assertEquals(error.asFailure(), result)
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID)) fakeAuthDiskSource.assertShouldTrustDevice(userId = USER_ID, shouldTrustDevice = null)
fakeAuthDiskSource.assertIsTdeLoginComplete(userId = USER_ID, isTdeLoginComplete = null)
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultSdkSource.getTrustDevice(userId = USER_ID) vaultSdkSource.getTrustDevice(userId = USER_ID)
} }
@ -123,7 +124,8 @@ class TrustedDeviceManagerTests {
val result = manager.trustThisDeviceIfNecessary(userId = USER_ID) val result = manager.trustThisDeviceIfNecessary(userId = USER_ID)
assertEquals(error.asFailure(), result) assertEquals(error.asFailure(), result)
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID)) fakeAuthDiskSource.assertShouldTrustDevice(userId = USER_ID, shouldTrustDevice = null)
fakeAuthDiskSource.assertIsTdeLoginComplete(userId = USER_ID, isTdeLoginComplete = null)
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultSdkSource.getTrustDevice(userId = USER_ID) vaultSdkSource.getTrustDevice(userId = USER_ID)
devicesService.trustDevice( devicesService.trustDevice(
@ -178,8 +180,9 @@ class TrustedDeviceManagerTests {
assertEquals(true.asSuccess(), result) assertEquals(true.asSuccess(), result)
fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey) fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey)
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID)) fakeAuthDiskSource.assertShouldTrustDevice(userId = USER_ID, shouldTrustDevice = null)
fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE) fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE)
fakeAuthDiskSource.assertIsTdeLoginComplete(userId = USER_ID, isTdeLoginComplete = true)
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultSdkSource.getTrustDevice(userId = USER_ID) vaultSdkSource.getTrustDevice(userId = USER_ID)
devicesService.trustDevice( devicesService.trustDevice(
@ -234,8 +237,9 @@ class TrustedDeviceManagerTests {
assertEquals(Unit.asSuccess(), result) assertEquals(Unit.asSuccess(), result)
fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey) fakeAuthDiskSource.assertDeviceKey(userId = USER_ID, deviceKey = deviceKey)
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID)) fakeAuthDiskSource.assertShouldTrustDevice(userId = USER_ID, shouldTrustDevice = null)
fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE) fakeAuthDiskSource.assertUserState(UPDATED_USER_STATE)
fakeAuthDiskSource.assertIsTdeLoginComplete(userId = USER_ID, isTdeLoginComplete = true)
coVerify(exactly = 1) { coVerify(exactly = 1) {
devicesService.trustDevice( devicesService.trustDevice(
appId = "testUniqueAppId", appId = "testUniqueAppId",

View file

@ -558,20 +558,32 @@ class AuthRepositoryTest {
assertNull(repository.rememberedOrgIdentifier) assertNull(repository.rememberedOrgIdentifier)
} }
@Test
fun `tdeLoginComplete should directly access the authDiskSource`() {
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
// AuthDiskSource and the repository start with the same default value.
assertNull(repository.tdeLoginComplete)
assertNull(fakeAuthDiskSource.getIsTdeLoginComplete(userId = USER_ID_1))
// Updating AuthDiskSource updates the repository
fakeAuthDiskSource.storeIsTdeLoginComplete(userId = USER_ID_1, isTdeLoginComplete = true)
assertEquals(true, repository.tdeLoginComplete)
}
@Test @Test
fun `shouldTrustDevice should directly access the authDiskSource`() { fun `shouldTrustDevice should directly access the authDiskSource`() {
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
// AuthDiskSource and the repository start with the same default value. // AuthDiskSource and the repository start with the same default value.
assertFalse(repository.shouldTrustDevice) assertFalse(repository.shouldTrustDevice)
assertFalse(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1)) assertNull(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1))
// Updating the repository updates AuthDiskSource // Updating the repository updates AuthDiskSource
repository.shouldTrustDevice = true repository.shouldTrustDevice = true
assertTrue(fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1)) assertEquals(true, fakeAuthDiskSource.getShouldTrustDevice(userId = USER_ID_1))
// Updating AuthDiskSource updates the repository // Updating AuthDiskSource updates the repository
fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID_1, shouldTrustDevice = false) fakeAuthDiskSource.storeShouldTrustDevice(userId = USER_ID_1, shouldTrustDevice = false)
assertFalse(repository.shouldTrustDevice) assertEquals(false, repository.shouldTrustDevice)
} }
@Test @Test

View file

@ -173,6 +173,7 @@ class RootNavViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength") @Suppress("MaxLineLength")
@Test @Test
fun `when the active user has an untrusted device without password the nav state should be TrustedDevice`() { fun `when the active user has an untrusted device without password the nav state should be TrustedDevice`() {
every { authRepository.tdeLoginComplete } returns null
mutableUserStateFlow.tryEmit( mutableUserStateFlow.tryEmit(
UserState( UserState(
activeUserId = "activeUserId", activeUserId = "activeUserId",
@ -209,6 +210,7 @@ class RootNavViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength") @Suppress("MaxLineLength")
@Test @Test
fun `when the active user has an untrusted device with password the nav state should be TrustedDevice`() { fun `when the active user has an untrusted device with password the nav state should be TrustedDevice`() {
every { authRepository.tdeLoginComplete } returns null
mutableUserStateFlow.tryEmit( mutableUserStateFlow.tryEmit(
UserState( UserState(
activeUserId = "activeUserId", activeUserId = "activeUserId",
@ -244,7 +246,8 @@ class RootNavViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength") @Suppress("MaxLineLength")
@Test @Test
fun `when the active user has an untrusted device but an unlocked vault the nav state should be Auth`() { fun `when the active user has an untrusted device but has completed TDE login the nav state should be Auth`() {
every { authRepository.tdeLoginComplete } returns true
mutableUserStateFlow.tryEmit( mutableUserStateFlow.tryEmit(
UserState( UserState(
activeUserId = "activeUserId", activeUserId = "activeUserId",