mirror of
https://github.com/bitwarden/android.git
synced 2024-10-31 07:05:35 +03:00
BITAU-108 Store Authenticator Sync Key (#3873)
This commit is contained in:
parent
eb4e2ab31f
commit
c817253760
6 changed files with 174 additions and 9 deletions
|
@ -45,6 +45,18 @@ interface AuthDiskSource {
|
|||
*/
|
||||
fun clearData(userId: String)
|
||||
|
||||
/**
|
||||
* Get the authenticator sync unlock key. Null means there is no key, which means the user
|
||||
* has not enabled authenticator syncing
|
||||
*/
|
||||
fun getAuthenticatorSyncUnlockKey(userId: String): String?
|
||||
|
||||
/**
|
||||
* Store the authenticator sync unlock key. Storing a null key effectively disables
|
||||
* authenticator syncing.
|
||||
*/
|
||||
fun storeAuthenticatorSyncUnlockKey(userId: String, authenticatorSyncUnlockKey: String?)
|
||||
|
||||
/**
|
||||
* Retrieves the state indicating that the user should use a key connector.
|
||||
*/
|
||||
|
|
|
@ -18,6 +18,7 @@ import java.util.UUID
|
|||
|
||||
// These keys should be encrypted
|
||||
private const val ACCOUNT_TOKENS_KEY = "accountTokens"
|
||||
private const val AUTHENTICATOR_SYNC_UNLOCK_KEY = "authenticatorSyncUnlock"
|
||||
private const val BIOMETRICS_UNLOCK_KEY = "userKeyBiometricUnlock"
|
||||
private const val USER_AUTO_UNLOCK_KEY_KEY = "userKeyAutoUnlock"
|
||||
private const val DEVICE_KEY_KEY = "deviceKey"
|
||||
|
@ -128,11 +129,25 @@ class AuthDiskSourceImpl(
|
|||
storeAccountTokens(userId = userId, accountTokens = null)
|
||||
storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = null)
|
||||
storeIsTdeLoginComplete(userId = userId, isTdeLoginComplete = null)
|
||||
storeAuthenticatorSyncUnlockKey(userId = userId, authenticatorSyncUnlockKey = null)
|
||||
|
||||
// Do not remove the DeviceKey or PendingAuthRequest on logout, these are persisted
|
||||
// indefinitely unless the TDE flow explicitly removes them.
|
||||
}
|
||||
|
||||
override fun getAuthenticatorSyncUnlockKey(userId: String): String? =
|
||||
getEncryptedString(AUTHENTICATOR_SYNC_UNLOCK_KEY.appendIdentifier(userId))
|
||||
|
||||
override fun storeAuthenticatorSyncUnlockKey(
|
||||
userId: String,
|
||||
authenticatorSyncUnlockKey: String?,
|
||||
) {
|
||||
putEncryptedString(
|
||||
key = AUTHENTICATOR_SYNC_UNLOCK_KEY.appendIdentifier(userId),
|
||||
value = authenticatorSyncUnlockKey,
|
||||
)
|
||||
}
|
||||
|
||||
override fun getShouldUseKeyConnectorFlow(userId: String): Flow<Boolean?> =
|
||||
getMutableShouldUseKeyConnectorFlowMap(userId = userId)
|
||||
.onSubscription { emit(getShouldUseKeyConnector(userId = userId)) }
|
||||
|
|
|
@ -99,8 +99,37 @@ class SettingsRepositoryImpl(
|
|||
}
|
||||
?: MutableStateFlow(value = null)
|
||||
|
||||
// TODO: this should be backed by disk and should set and clear the sync key (BITAU-103)
|
||||
override var isAuthenticatorSyncEnabled: Boolean = false
|
||||
override var isAuthenticatorSyncEnabled: Boolean
|
||||
// Authenticator sync is enabled if there is an authenticator sync unlock key for
|
||||
// the current active user:
|
||||
get() = activeUserId
|
||||
?.let { authDiskSource.getAuthenticatorSyncUnlockKey(userId = it) != null }
|
||||
?: false
|
||||
set(value) {
|
||||
val userId = activeUserId ?: return
|
||||
// When turning off authenticator sync, set authenticator sync unlock key to
|
||||
// null for the current active user:
|
||||
if (!value) {
|
||||
authDiskSource.storeAuthenticatorSyncUnlockKey(
|
||||
userId = userId,
|
||||
authenticatorSyncUnlockKey = null,
|
||||
)
|
||||
return
|
||||
}
|
||||
// When turning on authenticator sync, get a user encryption key from the vault SDK
|
||||
// and store it as a authenticator sync unlock key:
|
||||
unconfinedScope.launch {
|
||||
vaultSdkSource
|
||||
.getUserEncryptionKey(userId = userId)
|
||||
.getOrNull()
|
||||
?.let {
|
||||
authDiskSource.storeAuthenticatorSyncUnlockKey(
|
||||
userId = userId,
|
||||
authenticatorSyncUnlockKey = it,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override var isIconLoadingDisabled: Boolean
|
||||
get() = settingsDiskSource.isIconLoadingDisabled ?: false
|
||||
|
|
|
@ -295,6 +295,10 @@ class AuthDiskSourceTest {
|
|||
)
|
||||
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = "encryptedPin")
|
||||
authDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = "passwordHash")
|
||||
authDiskSource.storeAuthenticatorSyncUnlockKey(
|
||||
userId = userId,
|
||||
authenticatorSyncUnlockKey = "authenticatorSyncUnlockKey",
|
||||
)
|
||||
|
||||
authDiskSource.clearData(userId = userId)
|
||||
|
||||
|
@ -318,6 +322,7 @@ class AuthDiskSourceTest {
|
|||
assertNull(authDiskSource.getMasterPasswordHash(userId = userId))
|
||||
assertNull(authDiskSource.getShouldUseKeyConnector(userId = userId))
|
||||
assertNull(authDiskSource.getIsTdeLoginComplete(userId = userId))
|
||||
assertNull(authDiskSource.getAuthenticatorSyncUnlockKey(userId = userId))
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -1040,6 +1045,45 @@ class AuthDiskSourceTest {
|
|||
json.parseToJsonElement(requireNotNull(actual)),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `getAuthenticatorSyncUnlockKey should pull from SharedPreferences`() {
|
||||
val authenticatorSyncUnlockKey = "bwSecureStorage:authenticatorSyncUnlock"
|
||||
val mockUserId = "mockUserId"
|
||||
val mockAuthenticatorSyncUnlockKey = "mockAuthSyncUnlockKey"
|
||||
fakeEncryptedSharedPreferences
|
||||
.edit {
|
||||
putString(
|
||||
"${authenticatorSyncUnlockKey}_$mockUserId",
|
||||
mockAuthenticatorSyncUnlockKey,
|
||||
)
|
||||
}
|
||||
val actual = authDiskSource.getAuthenticatorSyncUnlockKey(userId = mockUserId)
|
||||
assertEquals(
|
||||
mockAuthenticatorSyncUnlockKey,
|
||||
actual,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `storeAuthenticatorSyncUnlockKey should update SharedPreferences`() {
|
||||
val authenticatorSyncUnlockKey = "bwSecureStorage:authenticatorSyncUnlock"
|
||||
val mockUserId = "mockUserId"
|
||||
val mockAuthenticatorSyncUnlockKey = "mockAuthSyncUnlockKey"
|
||||
authDiskSource.storeAuthenticatorSyncUnlockKey(
|
||||
userId = mockUserId,
|
||||
authenticatorSyncUnlockKey = mockAuthenticatorSyncUnlockKey,
|
||||
)
|
||||
|
||||
val actual = fakeEncryptedSharedPreferences.getString(
|
||||
key = "${authenticatorSyncUnlockKey}_$mockUserId",
|
||||
defaultValue = null,
|
||||
)
|
||||
assertEquals(
|
||||
mockAuthenticatorSyncUnlockKey,
|
||||
actual,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private const val USER_STATE_JSON = """
|
||||
|
|
|
@ -46,6 +46,7 @@ class FakeAuthDiskSource : AuthDiskSource {
|
|||
private val storedPendingAuthRequests = mutableMapOf<String, PendingAuthRequestJson?>()
|
||||
private val storedBiometricKeys = mutableMapOf<String, String?>()
|
||||
private val storedMasterPasswordHashes = mutableMapOf<String, String?>()
|
||||
private val storedAuthenticationSyncKeys = mutableMapOf<String, String?>()
|
||||
private val storedPolicies = mutableMapOf<String, List<SyncResponseJson.Policy>?>()
|
||||
|
||||
override var userState: UserStateJson? = null
|
||||
|
@ -215,6 +216,16 @@ class FakeAuthDiskSource : AuthDiskSource {
|
|||
storedMasterPasswordHashes[userId] = passwordHash
|
||||
}
|
||||
|
||||
override fun getAuthenticatorSyncUnlockKey(userId: String): String? =
|
||||
storedAuthenticationSyncKeys[userId]
|
||||
|
||||
override fun storeAuthenticatorSyncUnlockKey(
|
||||
userId: String,
|
||||
authenticatorSyncUnlockKey: String?,
|
||||
) {
|
||||
storedAuthenticationSyncKeys[userId] = authenticatorSyncUnlockKey
|
||||
}
|
||||
|
||||
override fun getPolicies(
|
||||
userId: String,
|
||||
): List<SyncResponseJson.Policy>? = storedPolicies[userId]
|
||||
|
|
|
@ -1020,13 +1020,6 @@ class SettingsRepositoryTest {
|
|||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isAuthenticatorSyncEnabled should be kept in memory and update accordingly`() = runTest {
|
||||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
settingsRepository.isAuthenticatorSyncEnabled = true
|
||||
assertTrue(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `getUserHasLoggedInValue should default to false if no value exists`() {
|
||||
assertFalse(settingsRepository.getUserHasLoggedInValue(userId = "userId"))
|
||||
|
@ -1045,9 +1038,70 @@ class SettingsRepositoryTest {
|
|||
settingsRepository.storeUserHasLoggedInValue(userId = userId)
|
||||
assertTrue(fakeSettingsDiskSource.getUserHasSignedInPreviously(userId = userId))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `setting isAuthenticatorSyncEnabled to true should generate an authenticator sync key`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
coEvery { vaultSdkSource.getUserEncryptionKey(USER_ID) }
|
||||
.returns(AUTHENTICATION_SYNC_KEY.asSuccess())
|
||||
|
||||
assertNull(fakeAuthDiskSource.getAuthenticatorSyncUnlockKey(USER_ID))
|
||||
|
||||
settingsRepository.isAuthenticatorSyncEnabled = true
|
||||
|
||||
assertTrue(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
assertEquals(
|
||||
AUTHENTICATION_SYNC_KEY,
|
||||
fakeAuthDiskSource.getAuthenticatorSyncUnlockKey(USER_ID),
|
||||
)
|
||||
coVerify { vaultSdkSource.getUserEncryptionKey(USER_ID) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `setting isAuthenticatorSyncEnabled to false should clear authenticator sync key`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
fakeAuthDiskSource.storeAuthenticatorSyncUnlockKey(USER_ID, AUTHENTICATION_SYNC_KEY)
|
||||
|
||||
assertTrue(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
|
||||
settingsRepository.isAuthenticatorSyncEnabled = false
|
||||
|
||||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
assertNull(fakeAuthDiskSource.getAuthenticatorSyncUnlockKey(USER_ID))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isAuthenticatorSyncEnabled should be true when there exists an authenticator sync key`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
fakeAuthDiskSource.storeAuthenticatorSyncUnlockKey(
|
||||
userId = USER_ID,
|
||||
authenticatorSyncUnlockKey = "fakeKey",
|
||||
)
|
||||
assertTrue(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `isAuthenticatorSyncEnabled should be false when there is no active user`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.userState = null
|
||||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `isAuthenticatorSyncEnabled should be false when the active user has no authenticator sync key set`() =
|
||||
runTest {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
assertFalse(settingsRepository.isAuthenticatorSyncEnabled)
|
||||
}
|
||||
}
|
||||
|
||||
private const val USER_ID: String = "userId"
|
||||
private const val AUTHENTICATION_SYNC_KEY = "authSyncKey"
|
||||
|
||||
private val MOCK_TRUSTED_DEVICE_USER_DECRYPTION_OPTIONS = TrustedDeviceUserDecryptionOptionsJson(
|
||||
encryptedPrivateKey = null,
|
||||
|
|
Loading…
Reference in a new issue