PM-11714 record if a user has signed in on device before. (#3876)

Co-authored-by: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com>
This commit is contained in:
Dave Severns 2024-09-06 13:18:35 -04:00 committed by GitHub
parent e039d5c3fb
commit ae349183e8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 167 additions and 3 deletions

View file

@ -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) {

View file

@ -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

View file

@ -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
}

View file

@ -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
}

View file

@ -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)
}

View file

@ -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.

View file

@ -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<AuthRepository> {
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(

View file

@ -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<AuthSdkSource> {
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,

View file

@ -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"))
}
}

View file

@ -60,6 +60,7 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val storedScreenCaptureAllowed = mutableMapOf<String, Boolean?>()
private var storedSystemBiometricIntegritySource: String? = null
private val storedAccountBiometricIntegrityValidity = mutableMapOf<String, Boolean?>()
private val userSignIns = mutableMapOf<String, Boolean>()
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<Boolean?> {
return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) {
bufferedMutableSharedFlow(replay = 1)

View file

@ -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"