PM-10909: Add persistance layer for usersKeyConnector (#3740)

This commit is contained in:
David Perez 2024-08-15 10:34:30 -05:00 committed by GitHub
parent e2cd3867dd
commit 18b58e75f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 72 additions and 7 deletions

View file

@ -45,6 +45,16 @@ interface AuthDiskSource {
*/ */
fun clearData(userId: String) fun clearData(userId: String)
/**
* Retrieves the state indicating that the user should use a key connector.
*/
fun getShouldUseKeyConnector(userId: String): Boolean?
/**
* Stores the boolean indicating that the user should use a key connector.
*/
fun storeShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?)
/** /**
* Retrieves the state indicating that the user has chosen to trust this device. * Retrieves the state indicating that the user has chosen to trust this device.
* *

View file

@ -39,6 +39,7 @@ private const val TWO_FACTOR_TOKEN_KEY = "twoFactorToken"
private const val MASTER_PASSWORD_HASH_KEY = "keyHash" private const val MASTER_PASSWORD_HASH_KEY = "keyHash"
private const val POLICIES_KEY = "policies" private const val POLICIES_KEY = "policies"
private const val SHOULD_TRUST_DEVICE_KEY = "shouldTrustDevice" private const val SHOULD_TRUST_DEVICE_KEY = "shouldTrustDevice"
private const val USES_KEY_CONNECTOR = "usesKeyConnector"
/** /**
* Primary implementation of [AuthDiskSource]. * Primary implementation of [AuthDiskSource].
@ -122,11 +123,23 @@ class AuthDiskSourceImpl(
storeMasterPasswordHash(userId = userId, passwordHash = null) storeMasterPasswordHash(userId = userId, passwordHash = null)
storePolicies(userId = userId, policies = null) storePolicies(userId = userId, policies = null)
storeAccountTokens(userId = userId, accountTokens = null) storeAccountTokens(userId = userId, accountTokens = null)
storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = null)
// Do not remove the DeviceKey or PendingAuthRequest on logout, these are persisted // Do not remove the DeviceKey or PendingAuthRequest on logout, these are persisted
// indefinitely unless the TDE flow explicitly removes them. // indefinitely unless the TDE flow explicitly removes them.
} }
override fun getShouldUseKeyConnector(
userId: String,
): Boolean? = getBoolean(key = USES_KEY_CONNECTOR.appendIdentifier(userId))
override fun storeShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?) {
putBoolean(
key = USES_KEY_CONNECTOR.appendIdentifier(userId),
value = shouldUseKeyConnector,
)
}
override fun getShouldTrustDevice(userId: String): Boolean = override fun getShouldTrustDevice(userId: String): Boolean =
requireNotNull( requireNotNull(
getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), default = false), getBoolean(key = SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), default = false),

View file

@ -898,27 +898,29 @@ class VaultRepositoryImpl(
) { ) {
val profile = syncResponse.profile val profile = syncResponse.profile
val userId = profile.id val userId = profile.id
val userKey = profile.key
val privateKey = profile.privateKey
authDiskSource.apply { authDiskSource.apply {
storeUserKey( storeUserKey(
userId = userId, userId = userId,
userKey = userKey, userKey = profile.key,
) )
storePrivateKey( storePrivateKey(
userId = userId, userId = userId,
privateKey = privateKey, privateKey = profile.privateKey,
) )
storeOrganizationKeys( storeOrganizationKeys(
userId = profile.id, userId = userId,
organizationKeys = profile.organizations organizationKeys = profile.organizations
.orEmpty() .orEmpty()
.filter { it.key != null } .filter { it.key != null }
.associate { it.id to requireNotNull(it.key) }, .associate { it.id to requireNotNull(it.key) },
) )
storeShouldUseKeyConnector(
userId = userId,
shouldUseKeyConnector = profile.shouldUseKeyConnector,
)
storeOrganizations( storeOrganizations(
userId = profile.id, userId = userId,
organizations = syncResponse.profile.organizations, organizations = profile.organizations,
) )
} }
} }

View file

@ -121,6 +121,24 @@ class AuthDiskSourceTest {
assertNull(authDiskSource.rememberedOrgIdentifier) assertNull(authDiskSource.rememberedOrgIdentifier)
} }
@Test
fun `shouldUseKeyConnector should pull from and update SharedPreferences`() {
val userId = "userId"
val shouldUseKeyConnectorKey = "bwPreferencesStorage:usesKeyConnector_$userId"
// Shared preferences and the disk source start with the same value.
assertNull(authDiskSource.getShouldUseKeyConnector(userId = userId))
assertFalse(fakeSharedPreferences.getBoolean(shouldUseKeyConnectorKey, false))
// Updating the disk source updates shared preferences
authDiskSource.storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = true)
assertTrue(fakeSharedPreferences.getBoolean(shouldUseKeyConnectorKey, false))
// Update SharedPreferences updates the disk source
fakeSharedPreferences.edit { putBoolean(shouldUseKeyConnectorKey, false) }
assertFalse(authDiskSource.getShouldUseKeyConnector(userId = userId) ?: true)
}
@Test @Test
fun `shouldTrustDevice should pull from and update SharedPreferences`() { fun `shouldTrustDevice should pull from and update SharedPreferences`() {
val userId = "userId" val userId = "userId"
@ -191,6 +209,7 @@ class AuthDiskSourceTest {
userId = userId, userId = userId,
pendingAuthRequest = pendingAuthRequestJson, pendingAuthRequest = pendingAuthRequestJson,
) )
authDiskSource.storeShouldUseKeyConnector(userId = userId, shouldUseKeyConnector = true)
val shouldTrustDevice = true val shouldTrustDevice = true
authDiskSource.storeShouldTrustDevice( authDiskSource.storeShouldTrustDevice(
userId = userId, userId = userId,
@ -258,6 +277,7 @@ class AuthDiskSourceTest {
assertNull(authDiskSource.getAccountTokens(userId = userId)) assertNull(authDiskSource.getAccountTokens(userId = userId))
assertNull(authDiskSource.getEncryptedPin(userId = userId)) assertNull(authDiskSource.getEncryptedPin(userId = userId))
assertNull(authDiskSource.getMasterPasswordHash(userId = userId)) assertNull(authDiskSource.getMasterPasswordHash(userId = userId))
assertNull(authDiskSource.getShouldUseKeyConnector(userId = userId))
} }
@Test @Test

View file

@ -26,6 +26,7 @@ class FakeAuthDiskSource : AuthDiskSource {
mutableMapOf<String, MutableSharedFlow<AccountTokensJson?>>() mutableMapOf<String, MutableSharedFlow<AccountTokensJson?>>()
private val mutableUserStateFlow = bufferedMutableSharedFlow<UserStateJson?>(replay = 1) private val mutableUserStateFlow = bufferedMutableSharedFlow<UserStateJson?>(replay = 1)
private val storedShouldUseKeyConnector = mutableMapOf<String, Boolean?>()
private val storedShouldTrustDevice = mutableMapOf<String, Boolean?>() private val storedShouldTrustDevice = mutableMapOf<String, Boolean?>()
private val storedInvalidUnlockAttempts = mutableMapOf<String, Int?>() private val storedInvalidUnlockAttempts = mutableMapOf<String, Int?>()
private val storedUserKeys = mutableMapOf<String, String?>() private val storedUserKeys = mutableMapOf<String, String?>()
@ -72,6 +73,14 @@ class FakeAuthDiskSource : AuthDiskSource {
mutableAccountTokensFlowMap.remove(userId) mutableAccountTokensFlowMap.remove(userId)
} }
override fun getShouldUseKeyConnector(
userId: String,
): Boolean = storedShouldUseKeyConnector[userId] ?: false
override fun storeShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?) {
storedShouldUseKeyConnector[userId] = shouldUseKeyConnector
}
override fun getShouldTrustDevice(userId: String): Boolean = override fun getShouldTrustDevice(userId: String): Boolean =
storedShouldTrustDevice[userId] ?: false storedShouldTrustDevice[userId] ?: false
@ -214,6 +223,13 @@ class FakeAuthDiskSource : AuthDiskSource {
getMutableAccountTokensFlow(userId = userId).tryEmit(accountTokens) getMutableAccountTokensFlow(userId = userId).tryEmit(accountTokens)
} }
/**
* Assert the the [shouldUseKeyConnector] was stored successfully using the [userId].
*/
fun assertShouldUseKeyConnector(userId: String, shouldUseKeyConnector: Boolean?) {
assertEquals(shouldUseKeyConnector, storedShouldUseKeyConnector[userId])
}
/** /**
* Assert that the given [userState] matches the currently tracked value. * Assert that the given [userState] matches the currently tracked value.
*/ */

View file

@ -826,6 +826,10 @@ class VaultRepositoryTest {
userId = "mockId-1", userId = "mockId-1",
policies = listOf(createMockPolicy(number = 1)), policies = listOf(createMockPolicy(number = 1)),
) )
fakeAuthDiskSource.assertShouldUseKeyConnector(
userId = "mockId-1",
shouldUseKeyConnector = false,
)
coVerify { coVerify {
vaultDiskSource.replaceVaultData( vaultDiskSource.replaceVaultData(
userId = MOCK_USER_STATE.activeUserId, userId = MOCK_USER_STATE.activeUserId,