From d5f8eabf31a33a1ed59866d292b0a8ecf97177cf Mon Sep 17 00:00:00 2001 From: Oleg Semenenko <146032743+oleg-livefront@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:36:37 -0600 Subject: [PATCH] Logout a user on sync if the security stamp does not match (#1002) --- .../data/auth/manager/UserLogoutManager.kt | 4 +- .../auth/manager/UserLogoutManagerImpl.kt | 13 ++++- .../data/auth/manager/di/AuthManagerModule.kt | 2 + .../vault/repository/VaultRepositoryImpl.kt | 15 ++++++ .../repository/di/VaultRepositoryModule.kt | 3 ++ .../auth/manager/UserLogoutManagerTest.kt | 1 + .../auth/repository/AuthRepositoryTest.kt | 2 +- .../vault/repository/VaultRepositoryTest.kt | 49 ++++++++++++++++++- 8 files changed, 85 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManager.kt index 8eddce8da..e283d770c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManager.kt @@ -6,8 +6,10 @@ package com.x8bit.bitwarden.data.auth.manager interface UserLogoutManager { /** * Completely logs out the given [userId], removing all data. + * If [isExpired] is true, a toast will be displayed + * letting the user know the session has expired. */ - fun logout(userId: String) + fun logout(userId: String, isExpired: Boolean = false) /** * Partially logs out the given [userId]. All data for the given [userId] will be removed with diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt index 3217651b8..27613c950 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt @@ -1,5 +1,8 @@ package com.x8bit.bitwarden.data.auth.manager +import android.content.Context +import android.widget.Toast +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource @@ -16,6 +19,7 @@ import kotlinx.coroutines.launch */ @Suppress("LongParameterList") class UserLogoutManagerImpl( + private val context: Context, private val authDiskSource: AuthDiskSource, private val generatorDiskSource: GeneratorDiskSource, private val passwordHistoryDiskSource: PasswordHistoryDiskSource, @@ -25,10 +29,17 @@ class UserLogoutManagerImpl( private val dispatcherManager: DispatcherManager, ) : UserLogoutManager { private val scope = CoroutineScope(dispatcherManager.unconfined) + private val mainScope = CoroutineScope(dispatcherManager.main) - override fun logout(userId: String) { + override fun logout(userId: String, isExpired: Boolean) { val currentUserState = authDiskSource.userState ?: return + if (isExpired) { + mainScope.launch { + Toast.makeText(context, R.string.login_expired, Toast.LENGTH_SHORT).show() + } + } + // Remove the active user from the accounts map val updatedAccounts = currentUserState .accounts diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt index 19d0a29d7..a01052703 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt @@ -45,6 +45,7 @@ object AuthManagerModule { @Provides @Singleton fun provideUserLogoutManager( + @ApplicationContext context: Context, authDiskSource: AuthDiskSource, generatorDiskSource: GeneratorDiskSource, passwordHistoryDiskSource: PasswordHistoryDiskSource, @@ -54,6 +55,7 @@ object AuthManagerModule { dispatcherManager: DispatcherManager, ): UserLogoutManager = UserLogoutManagerImpl( + context = context, authDiskSource = authDiskSource, generatorDiskSource = generatorDiskSource, passwordHistoryDiskSource = passwordHistoryDiskSource, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 142509fbc..1a8e8c9fb 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -13,6 +13,7 @@ import com.bitwarden.core.SendType import com.bitwarden.core.SendView import com.bitwarden.crypto.Kdf import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource +import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.auth.repository.util.toUpdatedUserStateJson import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow @@ -139,6 +140,7 @@ class VaultRepositoryImpl( private val fileManager: FileManager, private val vaultLockManager: VaultLockManager, private val totpCodeManager: TotpCodeManager, + private val userLogoutManager: UserLogoutManager, private val pushManager: PushManager, private val clock: Clock, dispatcherManager: DispatcherManager, @@ -305,6 +307,7 @@ class VaultRepositoryImpl( } } + @Suppress("LongMethod") override fun sync() { val userId = activeUserId ?: return if (!syncJob.isCompleted) return @@ -318,6 +321,18 @@ class VaultRepositoryImpl( .sync() .fold( onSuccess = { syncResponse -> + val localSecurityStamp = + authDiskSource.userState?.activeAccount?.profile?.stamp + val serverSecurityStamp = syncResponse.profile.securityStamp + + // Log the user out if the stamps do not match + localSecurityStamp?.let { + if (serverSecurityStamp != localSecurityStamp) { + userLogoutManager.logout(userId = userId, isExpired = true) + return@launch + } + } + // Update user information with additional information from sync response authDiskSource.userState = authDiskSource .userState diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt index 2012d2111..d096dfd87 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.vault.repository.di import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource +import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager @@ -45,6 +46,7 @@ object VaultRepositoryModule { dispatcherManager: DispatcherManager, totpCodeManager: TotpCodeManager, pushManager: PushManager, + userLogoutManager: UserLogoutManager, clock: Clock, ): VaultRepository = VaultRepositoryImpl( syncService = syncService, @@ -60,6 +62,7 @@ object VaultRepositoryModule { dispatcherManager = dispatcherManager, totpCodeManager = totpCodeManager, pushManager = pushManager, + userLogoutManager = userLogoutManager, clock = clock, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt index 6b5440ade..7910ebca7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt @@ -45,6 +45,7 @@ class UserLogoutManagerTest { private val userLogoutManager: UserLogoutManager = UserLogoutManagerImpl( + context = mockk(), authDiskSource = authDiskSource, generatorDiskSource = generatorDiskSource, passwordHistoryDiskSource = passwordHistoryDiskSource, 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 c586ee4f2..621cd17cf 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 @@ -195,7 +195,7 @@ class AuthRepositoryTest { } returns "AsymmetricEncString".asSuccess() } private val userLogoutManager: UserLogoutManager = mockk { - every { logout(any()) } just runs + every { logout(any(), any()) } just runs } private val mutableLogoutFlow = bufferedMutableSharedFlow() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 09a809c2d..a01a5d7e1 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -18,6 +18,7 @@ import com.bitwarden.core.TotpResponse import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource +import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource @@ -56,6 +57,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockFolder import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockOrganization import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockOrganizationKeys import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockPolicy +import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockProfile import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockSend import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockSendJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockSyncResponse @@ -148,6 +150,9 @@ class VaultRepositoryTest { ZoneOffset.UTC, ) private val dispatcherManager: DispatcherManager = FakeDispatcherManager() + private val userLogoutManager: UserLogoutManager = mockk() { + every { logout(any(), any()) } just runs + } private val fileManager: FileManager = mockk() private val fakeAuthDiskSource = FakeAuthDiskSource() private val settingsDiskSource = mockk() @@ -215,6 +220,7 @@ class VaultRepositoryTest { pushManager = pushManager, fileManager = fileManager, clock = clock, + userLogoutManager = userLogoutManager, ) @BeforeEach @@ -713,6 +719,47 @@ class VaultRepositoryTest { } } + @Suppress("MaxLineLength") + @Test + fun `sync with syncService Success with a different security stamp should logout and return early`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val userId = "mockId-1" + val mockSyncResponse = createMockSyncResponse(number = 1) + coEvery { syncService.sync() } returns mockSyncResponse.copy( + profile = createMockProfile(number = 1).copy(securityStamp = "newStamp"), + ) + .asSuccess() + + coEvery { + vaultSdkSource.initializeOrganizationCrypto( + userId = userId, + request = InitOrgCryptoRequest( + organizationKeys = createMockOrganizationKeys(1), + ), + ) + } returns InitializeCryptoResult.Success.asSuccess() + + vaultRepository.sync() + + coVerify { + userLogoutManager.logout(userId = userId, isExpired = true) + } + + coVerify(exactly = 0) { + vaultDiskSource.replaceVaultData( + userId = MOCK_USER_STATE.activeUserId, + vault = any(), + ) + vaultSdkSource.initializeOrganizationCrypto( + userId = userId, + request = InitOrgCryptoRequest( + organizationKeys = createMockOrganizationKeys(1), + ), + ) + } + } + @Test fun `sync with syncService Failure should update DataStateFlow with an Error`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE @@ -5456,7 +5503,7 @@ private val MOCK_PROFILE = AccountJson.Profile( email = "email", isEmailVerified = true, name = null, - stamp = null, + stamp = "mockSecurityStamp-1", organizationId = null, avatarColorHex = null, hasPremium = false,