From 3264be998d18503c04bd0d7d0d23576213fd531c Mon Sep 17 00:00:00 2001 From: David Perez Date: Fri, 26 Jan 2024 00:22:07 -0600 Subject: [PATCH] Add storage for biometrics key (#798) --- .../auth/datasource/disk/AuthDiskSource.kt | 10 ++++ .../datasource/disk/AuthDiskSourceImpl.kt | 15 ++++++ .../datasource/disk/AuthDiskSourceTest.kt | 54 ++++++++++++++++++- .../disk/util/FakeAuthDiskSource.kt | 9 ++++ .../platform/base/FakeSharedPreferences.kt | 6 ++- 5 files changed, 92 insertions(+), 2 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 32ae2001e..82e1e1ebe 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 @@ -100,6 +100,16 @@ interface AuthDiskSource { */ fun storeUserAutoUnlockKey(userId: String, userAutoUnlockKey: String?) + /** + * Gets the biometrics key for the given [userId]. + */ + fun getUserBiometricUnlockKey(userId: String): String? + + /** + * Stores the biometrics key for the given [userId]. + */ + fun storeUserBiometricUnlockKey(userId: String, biometricsKey: String?) + /** * Retrieves a pin-protected user key for the given [userId]. */ 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 429701792..6e3ac77c9 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 @@ -15,6 +15,7 @@ import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json import java.util.UUID +private const val BIOMETRICS_UNLOCK_KEY = "$ENCRYPTED_BASE_KEY:userKeyBiometricUnlock" private const val USER_AUTO_UNLOCK_KEY_KEY = "$ENCRYPTED_BASE_KEY:userKeyAutoUnlock" private const val UNIQUE_APP_ID_KEY = "$BASE_KEY:appId" private const val REMEMBERED_EMAIL_ADDRESS_KEY = "$BASE_KEY:rememberedEmail" @@ -92,6 +93,7 @@ class AuthDiskSourceImpl( storePrivateKey(userId = userId, privateKey = null) storeOrganizationKeys(userId = userId, organizationKeys = null) storeOrganizations(userId = userId, organizations = null) + storeUserBiometricUnlockKey(userId = userId, biometricsKey = null) } override fun getLastActiveTimeMillis(userId: String): Long? = @@ -156,6 +158,19 @@ class AuthDiskSourceImpl( ) } + override fun getUserBiometricUnlockKey(userId: String): String? = + getEncryptedString(key = "${BIOMETRICS_UNLOCK_KEY}_$userId") + + override fun storeUserBiometricUnlockKey( + userId: String, + biometricsKey: String?, + ) { + putEncryptedString( + key = "${BIOMETRICS_UNLOCK_KEY}_$userId", + value = biometricsKey, + ) + } + override fun getPinProtectedUserKey(userId: String): String? = inMemoryPinProtectedUserKeys[userId] ?: getString(key = "${PIN_PROTECTED_USER_KEY_KEY}_$userId") 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 ffc40e157..ac3133f58 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 @@ -27,10 +27,11 @@ import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test +@Suppress("LargeClass") class AuthDiskSourceTest { private val fakeEncryptedSharedPreferences = FakeSharedPreferences() private val fakeSharedPreferences = FakeSharedPreferences() - private val legacySecureStorageMigrator = mockk() { + private val legacySecureStorageMigrator = mockk { every { migrateIfNecessary() } just runs } @@ -137,6 +138,10 @@ class AuthDiskSourceTest { fun `clearData should clear all necessary data for the given user`() { val userId = "userId" + authDiskSource.storeUserBiometricUnlockKey( + userId = userId, + biometricsKey = "1234-9876-0192", + ) authDiskSource.storeLastActiveTimeMillis( userId = userId, lastActiveTimeMillis = 123456789L, @@ -162,6 +167,7 @@ class AuthDiskSourceTest { authDiskSource.clearData(userId = userId) + assertNull(authDiskSource.getUserBiometricUnlockKey(userId = userId)) assertNull(authDiskSource.getLastActiveTimeMillis(userId = userId)) assertNull(authDiskSource.getInvalidUnlockAttempts(userId = userId)) assertNull(authDiskSource.getUserKey(userId = userId)) @@ -439,6 +445,52 @@ class AuthDiskSourceTest { ) } + @Test + fun `getUserBiometricUnlockKey should pull from SharedPreferences`() { + val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val mockUserId = "mockUserId" + val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" + val biometricsKey = "1234" + fakeEncryptedSharedPreferences.edit { + putString(biometricsKeyKey, biometricsKey) + } + val actual = authDiskSource.getUserBiometricUnlockKey(userId = mockUserId) + assertEquals(biometricsKey, actual) + } + + @Test + fun `storeUserBiometricUnlockKey for non-null values should update SharedPreferences`() { + val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val mockUserId = "mockUserId" + val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" + val biometricsKey = "1234" + authDiskSource.storeUserBiometricUnlockKey( + userId = mockUserId, + biometricsKey = biometricsKey, + ) + val actual = fakeEncryptedSharedPreferences.getString( + key = biometricsKeyKey, + defaultValue = null, + ) + assertEquals(biometricsKey, actual) + } + + @Test + fun `storeUserBiometricUnlockKey for null values should clear SharedPreferences`() { + val biometricsKeyBaseKey = "bwSecureStorage:userKeyBiometricUnlock" + val mockUserId = "mockUserId" + val biometricsKeyKey = "${biometricsKeyBaseKey}_$mockUserId" + val biometricsKey = "1234" + fakeEncryptedSharedPreferences.edit { + putString(biometricsKeyKey, biometricsKey) + } + authDiskSource.storeUserBiometricUnlockKey( + userId = mockUserId, + biometricsKey = null, + ) + assertFalse(fakeEncryptedSharedPreferences.contains(biometricsKeyKey)) + } + @Test fun `getPinProtectedUserKey should pull from SharedPreferences`() { val pinProtectedUserKeyBaseKey = "bwPreferencesStorage:pinKeyEncryptedUserKey" 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 660d480a2..b5464cbc6 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 @@ -30,6 +30,7 @@ class FakeAuthDiskSource : AuthDiskSource { private val storedOrganizations = mutableMapOf?>() private val storedOrganizationKeys = mutableMapOf?>() + private val storedBiometricKeys = mutableMapOf() override var userState: UserStateJson? = null set(value) { @@ -50,6 +51,7 @@ class FakeAuthDiskSource : AuthDiskSource { storedPinProtectedUserKeys.remove(userId) storedEncryptedPins.remove(userId) storedOrganizations.remove(userId) + storedBiometricKeys.remove(userId) storedOrganizationKeys.remove(userId) mutableOrganizationsFlowMap.remove(userId) @@ -146,6 +148,13 @@ class FakeAuthDiskSource : AuthDiskSource { getMutableOrganizationsFlow(userId = userId).tryEmit(organizations) } + override fun getUserBiometricUnlockKey(userId: String): String? = + storedBiometricKeys[userId] + + override fun storeUserBiometricUnlockKey(userId: String, biometricsKey: String?) { + storedBiometricKeys[userId] = biometricsKey + } + /** * Assert that the given [userState] matches the currently tracked value. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeSharedPreferences.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeSharedPreferences.kt index 34f479568..458bdc43f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeSharedPreferences.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeSharedPreferences.kt @@ -100,6 +100,10 @@ class FakeSharedPreferences : SharedPreferences { private inline fun putValue( key: String, value: T, - ): SharedPreferences.Editor = apply { pendingSharedPreferences[key] = value } + ): SharedPreferences.Editor = apply { + value + ?.let { pendingSharedPreferences[key] = it } + ?: pendingSharedPreferences.remove(key) + } } }