From 083c4312621f5378556ff09036409b10765263f9 Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Wed, 29 Nov 2023 16:59:06 -0600 Subject: [PATCH] BIT-325: Create and persist device identifier (#298) Co-authored-by: Oleg Semenenko --- .../auth/datasource/disk/AuthDiskSource.kt | 7 +++++ .../datasource/disk/AuthDiskSourceImpl.kt | 13 +++++++++ .../network/service/IdentityService.kt | 2 ++ .../network/service/IdentityServiceImpl.kt | 5 ++-- .../auth/repository/AuthRepositoryImpl.kt | 1 + .../datasource/disk/AuthDiskSourceTest.kt | 28 +++++++++++++++++++ .../disk/util/FakeAuthDiskSource.kt | 2 ++ .../network/service/IdentityServiceTest.kt | 5 ++++ .../auth/repository/AuthRepositoryTest.kt | 11 ++++++++ 9 files changed, 71 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt index 5fc777c8a1..b4ad02bd71 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt @@ -7,6 +7,13 @@ import kotlinx.coroutines.flow.Flow * Primary access point for disk information. */ interface AuthDiskSource { + /** + * Retrieves a unique ID for the application that is stored locally. This will generate a new + * one if it does not yet exist and it will only be reset for new installs or when clearing + * application data. + */ + val uniqueAppId: String + /** * The currently persisted saved email address (or `null` if not set). */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt index c2b6eadf14..0569abd9d2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt @@ -9,7 +9,9 @@ import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.onSubscription import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json +import java.util.UUID +private const val UNIQUE_APP_ID_KEY = "$BASE_KEY:appId" private const val REMEMBERED_EMAIL_ADDRESS_KEY = "$BASE_KEY:rememberedEmail" private const val STATE_KEY = "$BASE_KEY:state" private const val MASTER_KEY_ENCRYPTION_USER_KEY = "masterKeyEncryptedUserKey" @@ -23,6 +25,9 @@ class AuthDiskSourceImpl( private val json: Json, ) : BaseDiskSource(sharedPreferences = sharedPreferences), AuthDiskSource { + override val uniqueAppId: String + get() = getString(key = UNIQUE_APP_ID_KEY) ?: generateAndStoreUniqueAppId() + override var rememberedEmailAddress: String? get() = getString(key = REMEMBERED_EMAIL_ADDRESS_KEY) set(value) { @@ -70,4 +75,12 @@ class AuthDiskSourceImpl( value = privateKey, ) } + + private fun generateAndStoreUniqueAppId(): String = + UUID + .randomUUID() + .toString() + .also { + putString(key = UNIQUE_APP_ID_KEY, value = it) + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityService.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityService.kt index 226f3d0061..bb7c6424b2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityService.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityService.kt @@ -11,11 +11,13 @@ interface IdentityService { /** * Make request to get an access token. * + * @param uniqueAppId applications unique identifier. * @param email user's email address. * @param passwordHash password hashed with the Bitwarden SDK. * @param captchaToken captcha token to be passed to the API (nullable). */ suspend fun getToken( + uniqueAppId: String, email: String, passwordHash: String, captchaToken: String?, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt index 6a25036b4e..6a073ed7ca 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt @@ -9,7 +9,6 @@ import com.x8bit.bitwarden.data.platform.datasource.network.util.executeForResul import com.x8bit.bitwarden.data.platform.datasource.network.util.parseErrorBodyOrNull import com.x8bit.bitwarden.data.platform.util.DeviceModelProvider import kotlinx.serialization.json.Json -import java.util.UUID class IdentityServiceImpl constructor( private val api: IdentityApi, @@ -19,6 +18,7 @@ class IdentityServiceImpl constructor( @Suppress("MagicNumber") override suspend fun getToken( + uniqueAppId: String, email: String, passwordHash: String, captchaToken: String?, @@ -27,8 +27,7 @@ class IdentityServiceImpl constructor( scope = "api+offline_access", clientId = "mobile", authEmail = email.base64UrlEncode(), - // TODO: use correct device identifier here BIT-325 - deviceIdentifier = UUID.randomUUID().toString(), + deviceIdentifier = uniqueAppId, deviceName = deviceModelProvider.deviceModel, deviceType = "0", grantType = "password", diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 4cd08a7979..eca8ecb502 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -135,6 +135,7 @@ class AuthRepositoryImpl constructor( } .flatMap { passwordHash -> identityService.getToken( + uniqueAppId = authDiskSource.uniqueAppId, email = email, passwordHash = passwordHash, captchaToken = captchaToken, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index cbcbf2cf5f..da6316a3e2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -31,6 +31,34 @@ class AuthDiskSourceTest { json = json, ) + @Test + fun `uniqueAppId should generate a new ID and update SharedPreferences if none exists`() { + val rememberedUniqueAppIdKey = "bwPreferencesStorage:appId" + + // Assert that the SharedPreferences are empty + assertNull(fakeSharedPreferences.getString(rememberedUniqueAppIdKey, null)) + + // Generate a new uniqueAppId and retrieve it + val newId = authDiskSource.uniqueAppId + + // Ensure that the SharedPreferences were updated + assertEquals( + newId, + fakeSharedPreferences.getString(rememberedUniqueAppIdKey, null), + ) + } + + @Test + fun `uniqueAppId should not generate a new ID if one exists`() { + val rememberedUniqueAppIdKey = "bwPreferencesStorage:appId" + val testId = "testId" + + // Update preferences to hold test value + fakeSharedPreferences.edit().putString(rememberedUniqueAppIdKey, testId).apply() + + assertEquals(testId, authDiskSource.uniqueAppId) + } + @Test fun `rememberedEmailAddress should pull from and update SharedPreferences`() { val rememberedEmailKey = "bwPreferencesStorage:rememberedEmail" diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt index 4cc592ee05..10a5570b11 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt @@ -8,6 +8,8 @@ import kotlinx.coroutines.flow.onSubscription import org.junit.Assert.assertEquals class FakeAuthDiskSource : AuthDiskSource { + override val uniqueAppId: String = "testUniqueAppId" + override var rememberedEmailAddress: String? = null override var userState: UserStateJson? = null diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt index 63f2754d2a..b6adeef949 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt @@ -41,6 +41,7 @@ class IdentityServiceTest : BaseServiceTest() { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) assertEquals(Result.success(LOGIN_SUCCESS), result) } @@ -52,6 +53,7 @@ class IdentityServiceTest : BaseServiceTest() { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) assertTrue(result.isFailure) } @@ -63,6 +65,7 @@ class IdentityServiceTest : BaseServiceTest() { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) assertEquals(Result.success(CAPTCHA_BODY), result) } @@ -74,6 +77,7 @@ class IdentityServiceTest : BaseServiceTest() { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) assertEquals(Result.success(INVALID_LOGIN), result) } @@ -94,6 +98,7 @@ class IdentityServiceTest : BaseServiceTest() { } companion object { + private const val UNIQUE_APP_ID = "testUniqueAppId" private const val REFRESH_TOKEN = "refreshToken" private const val EMAIL = "email" private const val PASSWORD_HASH = "passwordHash" 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 8c8e4ff218..075598d638 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 @@ -320,6 +320,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } .returns(Result.failure(RuntimeException())) @@ -332,6 +333,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } } @@ -346,6 +348,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } returns Result.success( GetTokenResponseJson.Invalid( @@ -364,6 +367,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } } @@ -381,6 +385,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } .returns(Result.success(successResponse)) @@ -419,6 +424,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) vaultRepository.unlockVault( userId = USER_ID_1, @@ -441,6 +447,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } .returns(Result.success(GetTokenResponseJson.CaptchaRequired(CAPTCHA_KEY))) @@ -453,6 +460,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } } @@ -807,6 +815,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } returns Result.success(successResponse) coEvery { @@ -870,6 +879,7 @@ class AuthRepositoryTest { email = EMAIL, passwordHash = PASSWORD_HASH, captchaToken = null, + uniqueAppId = UNIQUE_APP_ID, ) } returns Result.success(successResponse) coEvery { @@ -975,6 +985,7 @@ class AuthRepositoryTest { "com.x8bit.bitwarden.data.auth.repository.util.GetTokenResponseExtensionsKt" private const val REFRESH_TOKEN_RESPONSE_EXTENSIONS_PATH = "com.x8bit.bitwarden.data.auth.repository.util.RefreshTokenResponseExtensionsKt" + private const val UNIQUE_APP_ID = "testUniqueAppId" private const val EMAIL = "test@bitwarden.com" private const val EMAIL_2 = "test2@bitwarden.com" private const val PASSWORD = "password"