From d2d89b5a0f6b3abcd255cfaee5aaa55511203a67 Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 26 Aug 2025 13:28:51 -0500 Subject: [PATCH] PM-25193: Clear last sync time on push notification for inactive user (#5784) --- .../auth/repository/AuthRepositoryImpl.kt | 17 +++++++---- .../repository/di/AuthRepositoryModule.kt | 3 ++ .../data/platform/manager/PushManager.kt | 2 +- .../data/platform/manager/PushManagerImpl.kt | 14 +++++---- .../manager/model/NotificationPayload.kt | 9 ++++++ .../auth/repository/AuthRepositoryTest.kt | 29 +++++++++++++++---- .../disk/util/FakeSettingsDiskSource.kt | 7 +++++ .../data/platform/manager/PushManagerTest.kt | 18 ++++++++---- 8 files changed, 78 insertions(+), 21 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 8b8e0af347..ad210fdcbc 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -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 diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt index 17ddca4cb0..fb42b48da6 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt @@ -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, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt index c5a205199d..d050fd6545 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt @@ -52,7 +52,7 @@ interface PushManager { /** * Flow that represents requests intended to trigger syncing organization keys. */ - val syncOrgKeysFlow: Flow + val syncOrgKeysFlow: Flow /** * Flow that represents requests intended to trigger a sync send delete. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt index 561039c404..5551c49b4d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt @@ -66,7 +66,7 @@ class PushManagerImpl @Inject constructor( bufferedMutableSharedFlow() private val mutableSyncFolderUpsertSharedFlow = bufferedMutableSharedFlow() - private val mutableSyncOrgKeysSharedFlow = bufferedMutableSharedFlow() + private val mutableSyncOrgKeysSharedFlow = bufferedMutableSharedFlow() private val mutableSyncSendDeleteSharedFlow = bufferedMutableSharedFlow() private val mutableSyncSendUpsertSharedFlow = @@ -93,7 +93,7 @@ class PushManagerImpl @Inject constructor( override val syncFolderUpsertFlow: SharedFlow get() = mutableSyncFolderUpsertSharedFlow.asSharedFlow() - override val syncOrgKeysFlow: SharedFlow + override val syncOrgKeysFlow: SharedFlow get() = mutableSyncOrgKeysSharedFlow.asSharedFlow() override val syncSendDeleteFlow: SharedFlow @@ -232,9 +232,13 @@ class PushManagerImpl @Inject constructor( } NotificationType.SYNC_ORG_KEYS -> { - if (isLoggedIn(userId)) { - mutableSyncOrgKeysSharedFlow.tryEmit(Unit) - } + json + .decodeFromString( + string = notification.payload, + ) + .userId + .takeIf { authDiskSource.userState?.accounts.orEmpty().containsKey(it) } + ?.let { mutableSyncOrgKeysSharedFlow.tryEmit(it) } } NotificationType.SYNC_SEND_CREATE, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt index 53ad2fb4e6..5ca4855340 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt @@ -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() } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 202d5456e1..800435db30 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -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() - private val mutableSyncOrgKeysFlow = bufferedMutableSharedFlow() + private val mutableSyncOrgKeysFlow = bufferedMutableSharedFlow() private val mutableActivePolicyFlow = bufferedMutableSharedFlow>() 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 diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index 0d22292bd7..376ba57d50 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -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, diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt index dac34de958..f8e7ac2695 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt @@ -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() } }