PM-25193: Clear last sync time on push notification for inactive user (#5784)

This commit is contained in:
David Perez 2025-08-26 13:28:51 -05:00 committed by GitHub
parent ddadd0135f
commit d2d89b5a0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 78 additions and 21 deletions

View File

@ -95,6 +95,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow
import com.x8bit.bitwarden.data.auth.util.KdfParamsConstants.DEFAULT_PBKDF2_ITERATIONS
import com.x8bit.bitwarden.data.auth.util.YubiKeyResult
import com.x8bit.bitwarden.data.auth.util.toSdkParams
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.error.MissingPropertyException
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.LogsManager
@ -144,6 +145,7 @@ class AuthRepositoryImpl(
private val authSdkSource: AuthSdkSource,
private val vaultSdkSource: VaultSdkSource,
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
private val configDiskSource: ConfigDiskSource,
private val environmentRepository: EnvironmentRepository,
private val settingsRepository: SettingsRepository,
@ -297,11 +299,16 @@ class AuthRepositoryImpl(
.launchIn(unconfinedScope)
pushManager
.syncOrgKeysFlow
.onEach {
val userId = activeUserId ?: return@onEach
// TODO: [PM-20593] Investigate why tokens are explicitly refreshed.
refreshAccessTokenSynchronously(userId = userId)
vaultRepository.sync(forced = true)
.onEach { userId ->
if (userId == activeUserId) {
// TODO: [PM-20593] Investigate why tokens are explicitly refreshed.
refreshAccessTokenSynchronously(userId = userId)
// We just sync now to get the latest data
vaultRepository.sync(forced = true)
} else {
// We clear the last sync time to ensure we sync when we become the active user
settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = null)
}
}
// This requires the ioScope to ensure that refreshAccessTokenSynchronously
// happens on a background thread

View File

@ -17,6 +17,7 @@ import com.x8bit.bitwarden.data.auth.manager.UserStateManager
import com.x8bit.bitwarden.data.auth.manager.UserStateManagerImpl
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.AuthRepositoryImpl
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
import com.x8bit.bitwarden.data.platform.manager.LogsManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
@ -52,6 +53,7 @@ object AuthRepositoryModule {
authSdkSource: AuthSdkSource,
vaultSdkSource: VaultSdkSource,
authDiskSource: AuthDiskSource,
settingsDiskSource: SettingsDiskSource,
configDiskSource: ConfigDiskSource,
dispatcherManager: DispatcherManager,
environmentRepository: EnvironmentRepository,
@ -74,6 +76,7 @@ object AuthRepositoryModule {
authSdkSource = authSdkSource,
vaultSdkSource = vaultSdkSource,
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
configDiskSource = configDiskSource,
haveIBeenPwnedService = haveIBeenPwnedService,
dispatcherManager = dispatcherManager,

View File

@ -52,7 +52,7 @@ interface PushManager {
/**
* Flow that represents requests intended to trigger syncing organization keys.
*/
val syncOrgKeysFlow: Flow<Unit>
val syncOrgKeysFlow: Flow<String>
/**
* Flow that represents requests intended to trigger a sync send delete.

View File

@ -66,7 +66,7 @@ class PushManagerImpl @Inject constructor(
bufferedMutableSharedFlow<SyncFolderDeleteData>()
private val mutableSyncFolderUpsertSharedFlow =
bufferedMutableSharedFlow<SyncFolderUpsertData>()
private val mutableSyncOrgKeysSharedFlow = bufferedMutableSharedFlow<Unit>()
private val mutableSyncOrgKeysSharedFlow = bufferedMutableSharedFlow<String>()
private val mutableSyncSendDeleteSharedFlow =
bufferedMutableSharedFlow<SyncSendDeleteData>()
private val mutableSyncSendUpsertSharedFlow =
@ -93,7 +93,7 @@ class PushManagerImpl @Inject constructor(
override val syncFolderUpsertFlow: SharedFlow<SyncFolderUpsertData>
get() = mutableSyncFolderUpsertSharedFlow.asSharedFlow()
override val syncOrgKeysFlow: SharedFlow<Unit>
override val syncOrgKeysFlow: SharedFlow<String>
get() = mutableSyncOrgKeysSharedFlow.asSharedFlow()
override val syncSendDeleteFlow: SharedFlow<SyncSendDeleteData>
@ -232,9 +232,13 @@ class PushManagerImpl @Inject constructor(
}
NotificationType.SYNC_ORG_KEYS -> {
if (isLoggedIn(userId)) {
mutableSyncOrgKeysSharedFlow.tryEmit(Unit)
}
json
.decodeFromString<NotificationPayload.SynchronizeOrganizationKeysNotifications>(
string = notification.payload,
)
.userId
.takeIf { authDiskSource.userState?.accounts.orEmpty().containsKey(it) }
?.let { mutableSyncOrgKeysSharedFlow.tryEmit(it) }
}
NotificationType.SYNC_SEND_CREATE,

View File

@ -74,4 +74,13 @@ sealed class NotificationPayload {
@JsonNames("UserId", "userId") override val userId: String?,
@JsonNames("Id", "id") val loginRequestId: String?,
) : NotificationPayload()
/**
* A notification payload for resynchronizing organization keys.
*/
@Serializable
data class SynchronizeOrganizationKeysNotifications(
@JsonNames("UserId", "userId") override val userId: String?,
@JsonNames("Id", "id") val loginRequestId: String?,
) : NotificationPayload()
}

View File

@ -111,6 +111,7 @@ 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
import com.x8bit.bitwarden.data.auth.util.toSdkParams
import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeSettingsDiskSource
import com.x8bit.bitwarden.data.platform.error.MissingPropertyException
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.LogsManager
@ -173,6 +174,7 @@ class AuthRepositoryTest {
every { isActiveUserUnlockingFlow } returns mutableIsActiveUserUnlockingFlow
}
private val fakeAuthDiskSource = FakeAuthDiskSource()
private val fakeSettingsDiskSource = FakeSettingsDiskSource()
private val fakeEnvironmentRepository =
FakeEnvironmentRepository()
.apply {
@ -238,7 +240,7 @@ class AuthRepositoryTest {
}
private val mutableLogoutFlow = bufferedMutableSharedFlow<NotificationLogoutData>()
private val mutableSyncOrgKeysFlow = bufferedMutableSharedFlow<Unit>()
private val mutableSyncOrgKeysFlow = bufferedMutableSharedFlow<String>()
private val mutableActivePolicyFlow = bufferedMutableSharedFlow<List<SyncResponseJson.Policy>>()
private val pushManager: PushManager = mockk {
every { logoutFlow } returns mutableLogoutFlow
@ -274,6 +276,7 @@ class AuthRepositoryTest {
authSdkSource = authSdkSource,
vaultSdkSource = vaultSdkSource,
authDiskSource = fakeAuthDiskSource,
settingsDiskSource = fakeSettingsDiskSource,
configDiskSource = configDiskSource,
environmentRepository = fakeEnvironmentRepository,
settingsRepository = settingsRepository,
@ -6364,16 +6367,15 @@ class AuthRepositoryTest {
}
@Test
fun `syncOrgKeysFlow emissions should refresh access token and force sync`() {
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
fun `syncOrgKeysFlow emissions for active user should refresh access token and force sync`() {
fakeAuthDiskSource.userState = MULTI_USER_STATE
fakeAuthDiskSource.storeAccountTokens(userId = USER_ID_1, accountTokens = ACCOUNT_TOKENS_1)
coEvery {
identityService.refreshTokenSynchronously(REFRESH_TOKEN)
} returns REFRESH_TOKEN_RESPONSE_JSON.asSuccess()
coEvery { vaultRepository.sync(forced = true) } just runs
mutableSyncOrgKeysFlow.tryEmit(Unit)
mutableSyncOrgKeysFlow.tryEmit(USER_ID_1)
coVerify(exactly = 1) {
identityService.refreshTokenSynchronously(REFRESH_TOKEN)
@ -6381,6 +6383,23 @@ class AuthRepositoryTest {
}
}
@Test
fun `syncOrgKeysFlow emissions for inactive user should clear the last sync time`() {
fakeAuthDiskSource.userState = MULTI_USER_STATE
fakeSettingsDiskSource.storeLastSyncTime(
userId = USER_ID_2,
lastSyncTime = FIXED_CLOCK.instant(),
)
mutableSyncOrgKeysFlow.tryEmit(USER_ID_2)
coVerify(exactly = 0) {
identityService.refreshTokenSynchronously(REFRESH_TOKEN)
vaultRepository.sync(forced = true)
}
fakeSettingsDiskSource.assertLastSyncTime(userId = USER_ID_2, null)
}
@Test
fun `validatePasswordAgainstPolicy validates password against policy requirements`() = runTest {
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1

View File

@ -470,6 +470,13 @@ class FakeSettingsDiskSource : SettingsDiskSource {
assertEquals(expected, storedFlightRecorderData)
}
/**
* Asserts that the stored last sync time matches the [expected] one.
*/
fun assertLastSyncTime(userId: String, expected: Instant?) {
assertEquals(expected, storedLastSyncTime[userId])
}
//region Private helper functions
private fun getMutableLastSyncTimeFlow(
userId: String,

View File

@ -364,6 +364,17 @@ class PushManagerTest {
)
}
}
@Test
fun `onMessageReceived with sync org keys emits to syncOrgKeysFlow`() = runTest {
pushManager.syncOrgKeysFlow.test {
pushManager.onMessageReceived(SYNC_ORG_KEYS_NOTIFICATION_MAP)
assertEquals(
"078966a2-93c2-4618-ae2a-0a2394c88d37",
awaitItem(),
)
}
}
}
@Nested
@ -546,13 +557,10 @@ class PushManagerTest {
}
@Test
fun `onMessageReceived with sync org keys emits to syncOrgKeysFlow`() = runTest {
fun `onMessageReceived with sync org keys does not emit`() = runTest {
pushManager.syncOrgKeysFlow.test {
pushManager.onMessageReceived(SYNC_ORG_KEYS_NOTIFICATION_MAP)
assertEquals(
Unit,
awaitItem(),
)
expectNoEvents()
}
}