diff --git a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt index 435058f14f..7d2e05346e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt @@ -30,10 +30,12 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import java.time.Clock import javax.inject.Inject @@ -126,6 +128,19 @@ class MainViewModel @Inject constructor( } } .launchIn(viewModelScope) + + // On app launch, mark all active users as having previously logged in. + // This covers any users who are active prior to this value being recorded. + viewModelScope.launch { + val userState = authRepository + .userStateFlow + .first() + userState + ?.accounts + ?.forEach { + settingsRepository.storeUserHasLoggedInValue(it.userId) + } + } } override fun handleAction(action: MainAction) { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index ca05cb3b5b..e969c1fed1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -553,7 +553,7 @@ class AuthRepositoryImpl( ), ) } - + settingsRepository.storeUserHasLoggedInValue(userId) vaultRepository.syncIfNecessary() return LoginResult.Success } @@ -1560,6 +1560,7 @@ class AuthRepositoryImpl( resendEmailRequestJson = null twoFactorDeviceData = null settingsRepository.setDefaultsIfNecessary(userId = userId) + settingsRepository.storeUserHasLoggedInValue(userId) vaultRepository.syncIfNecessary() hasPendingAccountAddition = false LoginResult.Success diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt index 72d4ede447..ffbea55653 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt @@ -245,4 +245,17 @@ interface SettingsDiskSource { * Stores whether or not [isScreenCaptureAllowed] for the given [userId]. */ fun storeScreenCaptureAllowed(userId: String, isScreenCaptureAllowed: Boolean?) + + /** + * Records a user sign in for the given [userId]. This data is expected to remain on + * disk until storage is cleared or the app is uninstalled. + */ + fun storeUseHasLoggedInPreviously(userId: String) + + /** + * Checks if a user has signed in previously for the given [userId]. + * + * @see [storeUseHasLoggedInPreviously] + */ + fun getUserHasSignedInPreviously(userId: String): Boolean } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt index f37cb0c8b7..b8de4ec0a0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt @@ -383,4 +383,16 @@ class SettingsDiskSourceImpl( ) getMutableScreenCaptureAllowedFlow(userId).tryEmit(isScreenCaptureAllowed) } + + override fun storeUseHasLoggedInPreviously(userId: String) { + putBoolean( + key = HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY.appendIdentifier(userId), + value = true, + ) + } + + override fun getUserHasSignedInPreviously(userId: String): Boolean = + getBoolean( + key = HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY.appendIdentifier(userId), + ) == true } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt index c80de3671f..f6c6a42c7d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt @@ -233,4 +233,16 @@ interface SettingsRepository { * Clears any previously set unlock PIN for the current user. */ fun clearUnlockPin() + + /** + * Returns true if the given [userId] has previously logged in on this device. + * + * This assumes the device storage has not been cleared since installation. + */ + fun getUserHasLoggedInValue(userId: String): Boolean + + /** + * Record that a user has logged in on this device. + */ + fun storeUserHasLoggedInValue(userId: String) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt index 32a841bd61..23b99ef7a4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt @@ -494,6 +494,13 @@ class SettingsRepositoryImpl( } } + override fun getUserHasLoggedInValue(userId: String): Boolean = + settingsDiskSource.getUserHasSignedInPreviously(userId) + + override fun storeUserHasLoggedInValue(userId: String) { + settingsDiskSource.storeUseHasLoggedInPreviously(userId) + } + /** * Check the parameters of the vault unlock policy against the user's * settings to determine whether to update the user's settings. diff --git a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt index d4413b510b..99af228b36 100644 --- a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt @@ -75,6 +75,7 @@ class MainViewModelTest : BaseViewModelTest() { every { appThemeStateFlow } returns mutableAppThemeFlow every { isScreenCaptureAllowed } returns true every { isScreenCaptureAllowedStateFlow } returns mutableScreenCaptureAllowedFlow + every { storeUserHasLoggedInValue(any()) } just runs } private val authRepository = mockk { every { activeUserId } returns DEFAULT_USER_STATE.activeUserId @@ -756,6 +757,41 @@ class MainViewModelTest : BaseViewModelTest() { } } + @Test + fun `store logged in user status of the any active users on startup if they exist`() = runTest { + mutableUserStateFlow.value = DEFAULT_USER_STATE + createViewModel() + verify(exactly = 1) { + settingsRepository.storeUserHasLoggedInValue(userId = DEFAULT_USER_STATE.activeUserId) + } + } + + @Test + fun `store logged in user should recorded each active user`() = runTest { + val userId2 = "activeUserId2" + val multipleUserState = DEFAULT_USER_STATE.copy( + accounts = listOf( + DEFAULT_ACCOUNT, + DEFAULT_ACCOUNT.copy(userId = userId2), + ), + ) + mutableUserStateFlow.value = multipleUserState + createViewModel() + verify(exactly = 1) { + settingsRepository.storeUserHasLoggedInValue(userId = DEFAULT_USER_STATE.activeUserId) + settingsRepository.storeUserHasLoggedInValue(userId = userId2) + } + } + + @Test + fun `store logged in should not be called when there are no active users`() = runTest { + mutableUserStateFlow.value = null + createViewModel() + verify(exactly = 0) { + settingsRepository.storeUserHasLoggedInValue(userId = DEFAULT_USER_STATE.activeUserId) + } + } + private fun createViewModel( initialSpecialCircumstance: SpecialCircumstance? = null, ) = MainViewModel( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index f09a992d15..35c1d41982 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -164,6 +164,7 @@ class AuthRepositoryTest { private val settingsRepository: SettingsRepository = mockk { every { setDefaultsIfNecessary(any()) } just runs every { hasUserLoggedInOrCreatedAccount = true } just runs + every { storeUserHasLoggedInValue(any()) } just runs } private val authSdkSource = mockk { coEvery { @@ -1345,6 +1346,7 @@ class AuthRepositoryTest { organizationKeys = orgKeys, ) vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals(LoginResult.Success, result) } @@ -1393,6 +1395,7 @@ class AuthRepositoryTest { } coVerify(exactly = 0) { vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals(LoginResult.Error(errorMessage = null), result) } @@ -1558,6 +1561,7 @@ class AuthRepositoryTest { organizationKeys = null, ) vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( SINGLE_USER_STATE_1, @@ -1650,6 +1654,7 @@ class AuthRepositoryTest { coVerify(exactly = 0) { vaultRepository.syncIfNecessary() settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( @@ -1715,6 +1720,7 @@ class AuthRepositoryTest { uniqueAppId = UNIQUE_APP_ID, ) vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( SINGLE_USER_STATE_1, @@ -1813,6 +1819,7 @@ class AuthRepositoryTest { organizationKeys = null, ) vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( MULTI_USER_STATE, @@ -2083,6 +2090,7 @@ class AuthRepositoryTest { coVerify(exactly = 0) { vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } } @@ -2170,7 +2178,10 @@ class AuthRepositoryTest { SINGLE_USER_STATE_1, fakeAuthDiskSource.userState, ) - verify { settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } + verify { + settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) + } } @Test @@ -2355,12 +2366,13 @@ class AuthRepositoryTest { ), ), ) + settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( SINGLE_USER_STATE_1, fakeAuthDiskSource.userState, ) - verify { settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } } @Test @@ -2789,6 +2801,7 @@ class AuthRepositoryTest { uniqueAppId = UNIQUE_APP_ID, ) vaultRepository.syncIfNecessary() + settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } assertEquals( SINGLE_USER_STATE_1, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt index deaf2a224d..ceeebf2604 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt @@ -1000,4 +1000,33 @@ class SettingsDiskSourceTest { assertFalse(fakeSharedPreferences.contains(initialAutofillDialogShownKey)) } + + @Test + fun `record user sign in adds value of true to shared preferences`() { + val keyPrefix = "bwPreferencesStorage:hasUserLoggedInOrCreatedAccount" + val mockUserId = "mockUserId" + settingsDiskSource.storeUseHasLoggedInPreviously(userId = mockUserId) + + val actual = fakeSharedPreferences.getBoolean( + key = "${keyPrefix}_$mockUserId", + defaultValue = false, + ) + + assertTrue(actual) + } + + @Test + fun `hasUserSignedInPreviously returns true if value is present in shared preferences`() { + val mockUserId = "mockUserId" + fakeSharedPreferences.edit { + putBoolean("bwPreferencesStorage:hasUserLoggedInOrCreatedAccount_$mockUserId", true) + } + + assertTrue(settingsDiskSource.getUserHasSignedInPreviously(userId = mockUserId)) + } + + @Test + fun `hasUserSignedInPreviously returns false if value is not present in shared preferences`() { + assertFalse(settingsDiskSource.getUserHasSignedInPreviously(userId = "haveNotSignedIn")) + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index 1e63e04bcf..6cd29b1af8 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -60,6 +60,7 @@ class FakeSettingsDiskSource : SettingsDiskSource { private val storedScreenCaptureAllowed = mutableMapOf() private var storedSystemBiometricIntegritySource: String? = null private val storedAccountBiometricIntegrityValidity = mutableMapOf() + private val userSignIns = mutableMapOf() override var appLanguage: AppLanguage? = null @@ -277,6 +278,12 @@ class FakeSettingsDiskSource : SettingsDiskSource { getMutableScreenCaptureAllowedFlow(userId).tryEmit(isScreenCaptureAllowed) } + override fun storeUseHasLoggedInPreviously(userId: String) { + userSignIns[userId] = true + } + + override fun getUserHasSignedInPreviously(userId: String): Boolean = userSignIns[userId] == true + private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow { return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) { bufferedMutableSharedFlow(replay = 1) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt index 6bc0858e92..25d1ad3c0a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt @@ -1026,6 +1026,25 @@ class SettingsRepositoryTest { settingsRepository.isAuthenticatorSyncEnabled = true assertTrue(settingsRepository.isAuthenticatorSyncEnabled) } + + @Test + fun `getUserHasLoggedInValue should default to false if no value exists`() { + assertFalse(settingsRepository.getUserHasLoggedInValue(userId = "userId")) + } + + @Test + fun `getUserHasLoggedInValue should return true if it exists`() { + val userId = "userId" + fakeSettingsDiskSource.storeUseHasLoggedInPreviously(userId = userId) + assertTrue(settingsRepository.getUserHasLoggedInValue(userId = userId)) + } + + @Test + fun `storeUserHasLoggedInValue should store value of true to disk`() { + val userId = "userId" + settingsRepository.storeUserHasLoggedInValue(userId = userId) + assertTrue(fakeSettingsDiskSource.getUserHasSignedInPreviously(userId = userId)) + } } private const val USER_ID: String = "userId"