From de39f76627584c1aa567ded3ec3ee3329b712fc4 Mon Sep 17 00:00:00 2001 From: David Perez Date: Mon, 8 Apr 2024 16:47:25 -0500 Subject: [PATCH] Make SdkClientManager the single source of the Bitwarden SDK Client (#1242) --- .../auth/datasource/sdk/AuthSdkSourceImpl.kt | 87 ++++++++++--------- .../auth/datasource/sdk/di/AuthSdkModule.kt | 13 +-- .../datasource/di/EncryptionModule.kt | 22 ----- .../data/platform/manager/SdkClientManager.kt | 4 +- .../platform/manager/SdkClientManagerImpl.kt | 14 +-- .../datasource/sdk/GeneratorSdkSourceImpl.kt | 23 +++-- .../datasource/sdk/di/GeneratorSdkModule.kt | 6 +- .../auth/datasource/sdk/AuthSdkSourceTest.kt | 28 +++--- .../datasource/sdk/GeneratorSdkSourceTest.kt | 11 ++- 9 files changed, 94 insertions(+), 114 deletions(-) delete mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/di/EncryptionModule.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceImpl.kt index a8e7b243b..e2557519e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceImpl.kt @@ -6,53 +6,43 @@ import com.bitwarden.core.MasterPasswordPolicyOptions import com.bitwarden.core.RegisterKeyResponse import com.bitwarden.crypto.HashPurpose import com.bitwarden.crypto.Kdf +import com.bitwarden.sdk.Client import com.bitwarden.sdk.ClientAuth -import com.bitwarden.sdk.ClientPlatform import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toPasswordStrengthOrNull import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toUByte -import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager -import com.x8bit.bitwarden.data.vault.datasource.sdk.BitwardenFeatureFlagManager -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager /** * Primary implementation of [AuthSdkSource] that serves as a convenience wrapper around a * [ClientAuth]. */ class AuthSdkSourceImpl( - private val clientAuth: ClientAuth, - private val clientPlatform: ClientPlatform, - dispatcherManager: DispatcherManager, - featureFlagManager: BitwardenFeatureFlagManager, + private val sdkClientManager: SdkClientManager, ) : AuthSdkSource { - private val ioScope = CoroutineScope(dispatcherManager.io) - - init { - ioScope.launch { - clientPlatform.loadFlags(featureFlagManager.featureFlags) - } - } - override suspend fun getNewAuthRequest( email: String, ): Result = runCatching { - clientAuth.newAuthRequest( - email = email, - ) + getClient() + .auth() + .newAuthRequest( + email = email, + ) } override suspend fun getUserFingerprint( email: String, publicKey: String, ): Result = runCatching { - clientPlatform.fingerprint( - req = FingerprintRequest( - fingerprintMaterial = email, - publicKey = publicKey, - ), - ) + getClient() + .platform() + .fingerprint( + req = FingerprintRequest( + fingerprintMaterial = email, + publicKey = publicKey, + ), + ) } override suspend fun hashPassword( @@ -61,12 +51,14 @@ class AuthSdkSourceImpl( kdf: Kdf, purpose: HashPurpose, ): Result = runCatching { - clientAuth.hashPassword( - email = email, - password = password, - kdfParams = kdf, - purpose = purpose, - ) + getClient() + .auth() + .hashPassword( + email = email, + password = password, + kdfParams = kdf, + purpose = purpose, + ) } override suspend fun makeRegisterKeys( @@ -74,11 +66,13 @@ class AuthSdkSourceImpl( password: String, kdf: Kdf, ): Result = runCatching { - clientAuth.makeRegisterKeys( - email = email, - password = password, - kdf = kdf, - ) + getClient() + .auth() + .makeRegisterKeys( + email = email, + password = password, + kdf = kdf, + ) } override suspend fun passwordStrength( @@ -87,7 +81,8 @@ class AuthSdkSourceImpl( additionalInputs: List, ): Result = runCatching { @Suppress("UnsafeCallOnNullableType") - clientAuth + getClient() + .auth() .passwordStrength( password = password, email = email, @@ -101,10 +96,16 @@ class AuthSdkSourceImpl( passwordStrength: PasswordStrength, policy: MasterPasswordPolicyOptions, ): Result = runCatching { - clientAuth.satisfiesPolicy( - password = password, - strength = passwordStrength.toUByte(), - policy = policy, - ) + getClient() + .auth() + .satisfiesPolicy( + password = password, + strength = passwordStrength.toUByte(), + policy = policy, + ) } + + private suspend fun getClient( + userId: String? = null, + ): Client = sdkClientManager.getOrCreateClient(userId = userId) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/di/AuthSdkModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/di/AuthSdkModule.kt index 37399d56a..f2fe30e62 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/di/AuthSdkModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/sdk/di/AuthSdkModule.kt @@ -1,10 +1,8 @@ package com.x8bit.bitwarden.data.auth.datasource.sdk.di -import com.bitwarden.sdk.Client import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSourceImpl -import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager -import com.x8bit.bitwarden.data.vault.datasource.sdk.BitwardenFeatureFlagManager +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -21,13 +19,8 @@ object AuthSdkModule { @Provides @Singleton fun provideAuthSdkSource( - client: Client, - featureFlagManager: BitwardenFeatureFlagManager, - dispatcherManager: DispatcherManager, + sdkClientManager: SdkClientManager, ): AuthSdkSource = AuthSdkSourceImpl( - clientAuth = client.auth(), - clientPlatform = client.platform(), - featureFlagManager = featureFlagManager, - dispatcherManager = dispatcherManager, + sdkClientManager = sdkClientManager, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/di/EncryptionModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/di/EncryptionModule.kt deleted file mode 100644 index 4a6d57cba..000000000 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/di/EncryptionModule.kt +++ /dev/null @@ -1,22 +0,0 @@ -package com.x8bit.bitwarden.data.platform.datasource.di - -import com.bitwarden.sdk.Client -import dagger.Module -import dagger.Provides -import dagger.hilt.InstallIn -import dagger.hilt.components.SingletonComponent -import javax.inject.Singleton - -/** - * Provides dependencies related to encryption / decryption / secure generation. - */ -@Module -@InstallIn(SingletonComponent::class) -object EncryptionModule { - - @Provides - @Singleton - fun provideBitwardenClient(): Client { - return Client(null) - } -} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManager.kt index b40b1f752..9b3df862d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManager.kt @@ -11,11 +11,11 @@ interface SdkClientManager { * Returns the cached [Client] instance for the given [userId], otherwise creates and caches * a new one and returns it. */ - suspend fun getOrCreateClient(userId: String): Client + suspend fun getOrCreateClient(userId: String?): Client /** * Clears any resources from the [Client] associated with the given [userId] and removes it * from the internal cache. */ - fun destroyClient(userId: String) + fun destroyClient(userId: String?) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManagerImpl.kt index 28d4e5b1d..482668a45 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/SdkClientManagerImpl.kt @@ -9,19 +9,19 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.BitwardenFeatureFlagManager class SdkClientManagerImpl( private val featureFlagManager: BitwardenFeatureFlagManager, private val clientProvider: suspend () -> Client = { - Client(null) - .apply { platform().loadFlags(featureFlagManager.featureFlags) } + Client(settings = null).apply { + platform().loadFlags(featureFlagManager.featureFlags) + } }, ) : SdkClientManager { - private val userIdToClientMap = mutableMapOf() + private val userIdToClientMap = mutableMapOf() override suspend fun getOrCreateClient( - userId: String, - ): Client = - userIdToClientMap.getOrPut(key = userId) { clientProvider() } + userId: String?, + ): Client = userIdToClientMap.getOrPut(key = userId) { clientProvider() } override fun destroyClient( - userId: String, + userId: String?, ) { userIdToClientMap .remove(key = userId) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceImpl.kt index 597b14b85..6df839a8c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceImpl.kt @@ -3,50 +3,57 @@ package com.x8bit.bitwarden.data.tools.generator.datasource.sdk import com.bitwarden.generators.PassphraseGeneratorRequest import com.bitwarden.generators.PasswordGeneratorRequest import com.bitwarden.generators.UsernameGeneratorRequest +import com.bitwarden.sdk.Client import com.bitwarden.sdk.ClientGenerators +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager /** * Implementation of [GeneratorSdkSource] that delegates password generation. * - * @property clientGenerator An instance of [ClientGenerators] provided by the Bitwarden SDK. + * @property sdkClientManager The [SdkClientManager] used to retrieve an instance of the + * [ClientGenerators] provided by the Bitwarden SDK. */ class GeneratorSdkSourceImpl( - private val clientGenerator: ClientGenerators, + private val sdkClientManager: SdkClientManager, ) : GeneratorSdkSource { override suspend fun generatePassword( request: PasswordGeneratorRequest, ): Result = runCatching { - clientGenerator.password(request) + getClient().generators().password(request) } override suspend fun generatePassphrase( request: PassphraseGeneratorRequest, ): Result = runCatching { - clientGenerator.passphrase(request) + getClient().generators().passphrase(request) } override suspend fun generatePlusAddressedEmail( request: UsernameGeneratorRequest.Subaddress, ): Result = runCatching { - clientGenerator.username(request) + getClient().generators().username(request) } override suspend fun generateCatchAllEmail( request: UsernameGeneratorRequest.Catchall, ): Result = runCatching { - clientGenerator.username(request) + getClient().generators().username(request) } override suspend fun generateRandomWord( request: UsernameGeneratorRequest.Word, ): Result = runCatching { - clientGenerator.username(request) + getClient().generators().username(request) } override suspend fun generateForwardedServiceEmail( request: UsernameGeneratorRequest.Forwarded, ): Result = runCatching { - clientGenerator.username(request) + getClient().generators().username(request) } + + private suspend fun getClient( + userId: String? = null, + ): Client = sdkClientManager.getOrCreateClient(userId = userId) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/di/GeneratorSdkModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/di/GeneratorSdkModule.kt index e608ef87c..dbb1ae4dd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/di/GeneratorSdkModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/di/GeneratorSdkModule.kt @@ -1,6 +1,6 @@ package com.x8bit.bitwarden.data.tools.generator.datasource.sdk.di -import com.bitwarden.sdk.Client +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager import com.x8bit.bitwarden.data.tools.generator.datasource.sdk.GeneratorSdkSource import com.x8bit.bitwarden.data.tools.generator.datasource.sdk.GeneratorSdkSourceImpl import dagger.Module @@ -19,6 +19,6 @@ object GeneratorSdkModule { @Provides @Singleton fun provideGeneratorSdkSource( - client: Client, - ): GeneratorSdkSource = GeneratorSdkSourceImpl(clientGenerator = client.generators()) + sdkClientManager: SdkClientManager, + ): GeneratorSdkSource = GeneratorSdkSourceImpl(sdkClientManager = sdkClientManager) } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceTest.kt index 14e1d0481..dc2caca7b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/sdk/AuthSdkSourceTest.kt @@ -6,20 +6,20 @@ import com.bitwarden.core.MasterPasswordPolicyOptions import com.bitwarden.core.RegisterKeyResponse import com.bitwarden.crypto.HashPurpose import com.bitwarden.crypto.Kdf +import com.bitwarden.sdk.Client import com.bitwarden.sdk.ClientAuth import com.bitwarden.sdk.ClientPlatform import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength -import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager import com.x8bit.bitwarden.data.platform.util.asSuccess -import com.x8bit.bitwarden.data.vault.datasource.sdk.BitwardenFeatureFlagManager import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test @@ -28,26 +28,18 @@ class AuthSdkSourceTest { private val clientPlatform = mockk { coEvery { loadFlags(any()) } just runs } - private val featureFlagManager = mockk { - coEvery { featureFlags } returns emptyMap() + private val client = mockk { + every { auth() } returns clientAuth + every { platform() } returns clientPlatform + } + private val sdkClientManager = mockk { + coEvery { getOrCreateClient(userId = null) } returns client } - private val dispatcherManager = FakeDispatcherManager() private val authSkdSource: AuthSdkSource = AuthSdkSourceImpl( - clientAuth = clientAuth, - clientPlatform = clientPlatform, - featureFlagManager = featureFlagManager, - dispatcherManager = dispatcherManager, + sdkClientManager = sdkClientManager, ) - @BeforeEach - fun setup() { - coVerify(exactly = 1) { - featureFlagManager.featureFlags - clientPlatform.loadFlags(any()) - } - } - @Test fun `getNewAuthRequest should call SDK and return a Result with correct data`() = runBlocking { val email = "test@gmail.com" diff --git a/app/src/test/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceTest.kt index 56e43557c..d62ed2f8d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/tools/generator/datasource/sdk/GeneratorSdkSourceTest.kt @@ -5,10 +5,13 @@ import com.bitwarden.generators.ForwarderServiceType import com.bitwarden.generators.PassphraseGeneratorRequest import com.bitwarden.generators.PasswordGeneratorRequest import com.bitwarden.generators.UsernameGeneratorRequest +import com.bitwarden.sdk.Client import com.bitwarden.sdk.ClientGenerators +import com.x8bit.bitwarden.data.platform.manager.SdkClientManager import com.x8bit.bitwarden.data.platform.util.asSuccess import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals @@ -16,7 +19,13 @@ import org.junit.Test class GeneratorSdkSourceTest { private val clientGenerators = mockk() - private val generatorSdkSource: GeneratorSdkSource = GeneratorSdkSourceImpl(clientGenerators) + private val client = mockk { + every { generators() } returns clientGenerators + } + private val sdkClientManager = mockk { + coEvery { getOrCreateClient(userId = null) } returns client + } + private val generatorSdkSource: GeneratorSdkSource = GeneratorSdkSourceImpl(sdkClientManager) @Test fun `generatePassword should call SDK and return a Result with the generated password`() =