Improve detection of configure PIN code.

This commit is contained in:
Benoit Marty 2026-05-05 18:24:25 +02:00 committed by Benoit Marty
parent 2f45ca8835
commit 61374bca4e
13 changed files with 77 additions and 44 deletions

View file

@ -36,13 +36,13 @@ interface BiometricAuthenticator {
}
val isActive: Boolean
fun setup()
suspend fun setup()
suspend fun authenticate(): AuthenticationResult
}
class NoopBiometricAuthentication : BiometricAuthenticator {
override val isActive: Boolean = false
override fun setup() = Unit
override suspend fun setup() = Unit
override suspend fun authenticate() = BiometricAuthenticator.AuthenticationResult.Failure()
}
@ -58,7 +58,7 @@ class DefaultBiometricAuthentication(
private var cryptoObject: CryptoObject? = null
override fun setup() {
override suspend fun setup() {
try {
val secretKey = ensureKey()
val cipher = encryptionDecryptionService.createEncryptionCipher(secretKey)
@ -86,7 +86,7 @@ class DefaultBiometricAuthentication(
}
@Throws(KeyPermanentlyInvalidatedException::class)
private fun ensureKey() = secretKeyRepository.getOrCreateKey(keyAlias, true).also {
private suspend fun ensureKey() = secretKeyRepository.getOrCreateKey(keyAlias, true).also {
encryptionDecryptionService.createEncryptionCipher(it)
}
}

View file

@ -18,7 +18,7 @@ import io.element.android.libraries.cryptography.api.SecretKeyRepository
import kotlinx.coroutines.flow.Flow
import java.util.concurrent.CopyOnWriteArrayList
private const val SECRET_KEY_ALIAS = "elementx.SECRET_KEY_ALIAS_PIN_CODE"
internal const val SECRET_KEY_ALIAS = "elementx.SECRET_KEY_ALIAS_PIN_CODE"
@ContributesBinding(AppScope::class)
@SingleIn(AppScope::class)
@ -38,7 +38,7 @@ class DefaultPinCodeManager(
}
override fun hasPinCode(): Flow<Boolean> {
return lockScreenStore.hasPinCode()
return secretKeyRepository.hasKey(SECRET_KEY_ALIAS)
}
override suspend fun getPinCodeSize(): Int {
@ -79,6 +79,7 @@ class DefaultPinCodeManager(
override suspend fun deletePinCode() {
lockScreenStore.deleteEncryptedPinCode()
lockScreenStore.resetCounter()
secretKeyRepository.deleteKey(SECRET_KEY_ALIAS)
callbacks.forEach { it.onPinCodeRemoved() }
}

View file

@ -8,8 +8,6 @@
package io.element.android.features.lockscreen.impl.storage
import kotlinx.coroutines.flow.Flow
/**
* Should be implemented by any class that provides access to the encrypted PIN code.
* All methods are suspending in case there are async IO operations involved.
@ -29,9 +27,4 @@ interface EncryptedPinCodeStorage {
* Deletes the PIN code from some persistable storage.
*/
suspend fun deleteEncryptedPinCode()
/**
* Returns whether the PIN code is stored or not.
*/
fun hasPinCode(): Flow<Boolean>
}

View file

@ -70,12 +70,6 @@ class PreferencesLockScreenStore(
}
}
override fun hasPinCode(): Flow<Boolean> {
return dataStore.data.map { preferences ->
preferences[pinCodeKey] != null
}
}
override fun isBiometricUnlockAllowed(): Flow<Boolean> {
return dataStore.data.map { preferences ->
preferences[biometricUnlockKey] ?: false

View file

@ -14,9 +14,12 @@ import io.element.android.features.lockscreen.impl.biometric.BiometricAuthentica
import io.element.android.features.lockscreen.impl.biometric.FakeBiometricAuthenticatorManager
import io.element.android.features.lockscreen.impl.fixtures.aLockScreenConfig
import io.element.android.features.lockscreen.impl.pin.PinCodeManager
import io.element.android.features.lockscreen.impl.pin.SECRET_KEY_ALIAS
import io.element.android.features.lockscreen.impl.pin.createDefaultPinCodeManager
import io.element.android.features.lockscreen.impl.pin.storage.InMemoryLockScreenStore
import io.element.android.features.lockscreen.impl.storage.LockScreenStore
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import io.element.android.libraries.cryptography.test.SimpleSecretKeyRepository
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import io.element.android.libraries.sessionstorage.test.observer.FakeSessionObserver
import io.element.android.services.appnavstate.api.AppForegroundStateService
@ -38,18 +41,18 @@ class DefaultLockScreenServiceTest {
@Test
fun `when the pin is mandatory, isSetupRequired emits true`() = runTest {
val lockScreenStore = InMemoryLockScreenStore()
val secretKeyRepository = SimpleSecretKeyRepository()
val sut = createDefaultLockScreenService(
lockScreenConfig = aLockScreenConfig(isPinMandatory = true),
lockScreenStore = lockScreenStore,
secretKeyRepository = secretKeyRepository,
)
sut.isSetupRequired().test {
assertThat(awaitItem()).isTrue()
// When the user configures the pin code, the setup is not required anymore
lockScreenStore.saveEncryptedPinCode("encryptedCode")
secretKeyRepository.getOrCreateKey(SECRET_KEY_ALIAS, true)
assertThat(awaitItem()).isFalse()
// Users deletes the pin code
lockScreenStore.deleteEncryptedPinCode()
secretKeyRepository.deleteKey("elementx.SECRET_KEY_ALIAS_PIN_CODE")
assertThat(awaitItem()).isTrue()
}
}
@ -57,16 +60,16 @@ class DefaultLockScreenServiceTest {
@Test
fun `when the last session is deleted, the pin code is removed`() = runTest {
val sessionObserver = FakeSessionObserver()
val lockScreenStore = InMemoryLockScreenStore()
val secretKeyRepository = SimpleSecretKeyRepository()
val sut = createDefaultLockScreenService(
lockScreenConfig = aLockScreenConfig(isPinMandatory = true),
lockScreenStore = lockScreenStore,
secretKeyRepository = secretKeyRepository,
sessionObserver = sessionObserver,
)
sut.isPinSetup().test {
assertThat(awaitItem()).isFalse()
// When the user configure the pin code, the setup is not required anymore
lockScreenStore.saveEncryptedPinCode("encryptedCode")
secretKeyRepository.getOrCreateKey(SECRET_KEY_ALIAS, true)
assertThat(awaitItem()).isTrue()
sessionObserver.onSessionDeleted("userId", wasLastSession = false)
expectNoEvents()
@ -79,8 +82,10 @@ class DefaultLockScreenServiceTest {
private fun TestScope.createDefaultLockScreenService(
lockScreenConfig: LockScreenConfig = aLockScreenConfig(),
lockScreenStore: LockScreenStore = InMemoryLockScreenStore(),
secretKeyRepository: SecretKeyRepository = SimpleSecretKeyRepository(),
pinCodeManager: PinCodeManager = createDefaultPinCodeManager(
lockScreenStore = lockScreenStore,
secretKeyRepository = secretKeyRepository,
),
sessionObserver: SessionObserver = FakeSessionObserver(),
appForegroundStateService: AppForegroundStateService = FakeAppForegroundStateService(),

View file

@ -12,6 +12,6 @@ class FakeBiometricAuthenticator(
override val isActive: Boolean = false,
private val authenticateLambda: suspend () -> BiometricAuthenticator.AuthenticationResult = { BiometricAuthenticator.AuthenticationResult.Success },
) : BiometricAuthenticator {
override fun setup() = Unit
override suspend fun setup() = Unit
override suspend fun authenticate() = authenticateLambda()
}

View file

@ -15,12 +15,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
private const val DEFAULT_REMAINING_ATTEMPTS = 3
class InMemoryLockScreenStore : LockScreenStore {
private val hasPinCode = MutableStateFlow(false)
private var pinCode: String? = null
set(value) {
field = value
hasPinCode.value = value != null
}
private var remainingAttempts: Int = DEFAULT_REMAINING_ATTEMPTS
private var isBiometricUnlockAllowed = MutableStateFlow(false)
@ -48,10 +43,6 @@ class InMemoryLockScreenStore : LockScreenStore {
pinCode = null
}
override fun hasPinCode(): Flow<Boolean> {
return hasPinCode
}
override fun isBiometricUnlockAllowed(): Flow<Boolean> {
return isBiometricUnlockAllowed
}

View file

@ -13,3 +13,7 @@ plugins {
android {
namespace = "io.element.android.libraries.cryptography.api"
}
dependencies {
implementation(libs.coroutines.core)
}

View file

@ -8,6 +8,7 @@
package io.element.android.libraries.cryptography.api
import kotlinx.coroutines.flow.Flow
import javax.crypto.SecretKey
/**
@ -15,16 +16,18 @@ import javax.crypto.SecretKey
* Implementation should be able to store the generated key securely.
*/
interface SecretKeyRepository {
fun hasKey(alias: String): Flow<Boolean>
/**
* Get or create a secret key for a given alias.
* @param alias the alias to use
* @param requiresUserAuthentication true if the key should be protected by user authentication
*/
fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey
suspend fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey
/**
* Delete the secret key for a given alias.
* @param alias the alias to use
*/
fun deleteKey(alias: String)
suspend fun deleteKey(alias: String)
}

View file

@ -21,6 +21,7 @@ setupDependencyInjection()
dependencies {
implementation(projects.libraries.di)
implementation(libs.coroutines.core)
api(projects.libraries.cryptography.api)
testCommonDependencies(libs)

View file

@ -13,11 +13,16 @@ import android.security.keystore.KeyGenParameterSpec
import android.security.keystore.KeyProperties
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.cryptography.api.AESEncryptionSpecs
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import timber.log.Timber
import java.security.KeyStore
import java.security.KeyStoreException
import java.util.concurrent.ConcurrentHashMap
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey
@ -25,13 +30,22 @@ import javax.crypto.SecretKey
* Default implementation of [SecretKeyRepository] that uses the Android Keystore to store the keys.
* The generated key uses AES algorithm, with a key size of 128 bits, and the GCM block mode.
*/
@SingleIn(AppScope::class)
@ContributesBinding(AppScope::class)
class KeyStoreSecretKeyRepository(
private val keyStore: KeyStore,
) : SecretKeyRepository {
private val hasKeyMap = ConcurrentHashMap<String, MutableStateFlow<Boolean>>()
override fun hasKey(alias: String): Flow<Boolean> {
return hasKeyMap.getOrPut(alias) {
MutableStateFlow(runCatching { keyStore.containsAlias(alias) }.getOrDefault(false))
}.asStateFlow()
}
// False positive lint issue
@SuppressLint("WrongConstant")
override fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey {
override suspend fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey {
val secretKeyEntry = (keyStore.getEntry(alias, null) as? KeyStore.SecretKeyEntry)
?.secretKey
return if (secretKeyEntry == null) {
@ -46,15 +60,22 @@ class KeyStoreSecretKeyRepository(
.setUserAuthenticationRequired(requiresUserAuthentication)
.build()
generator.init(keyGenSpec)
generator.generateKey()
generator.generateKey().also {
hasKeyMap.getOrPut(alias) {
MutableStateFlow(true)
}.emit(true)
}
} else {
secretKeyEntry
}
}
override fun deleteKey(alias: String) {
override suspend fun deleteKey(alias: String) {
try {
keyStore.deleteEntry(alias)
hasKeyMap.getOrPut(alias) {
MutableStateFlow(false)
}.emit(false)
} catch (e: KeyStoreException) {
Timber.e(e)
}

View file

@ -16,4 +16,5 @@ android {
dependencies {
api(projects.libraries.cryptography.api)
implementation(libs.coroutines.core)
}

View file

@ -10,20 +10,39 @@ package io.element.android.libraries.cryptography.test
import io.element.android.libraries.cryptography.api.AESEncryptionSpecs
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import java.util.concurrent.ConcurrentHashMap
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey
class SimpleSecretKeyRepository : SecretKeyRepository {
private var secretKeyForAlias = HashMap<String, SecretKey>()
override fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey {
private val hasKeyMap = ConcurrentHashMap<String, MutableStateFlow<Boolean>>()
override fun hasKey(alias: String): Flow<Boolean> {
return hasKeyMap.getOrPut(alias) {
MutableStateFlow(false)
}.asStateFlow()
}
override suspend fun getOrCreateKey(alias: String, requiresUserAuthentication: Boolean): SecretKey {
return secretKeyForAlias.getOrPut(alias) {
generateKey()
generateKey().also {
hasKeyMap.getOrPut(alias) {
MutableStateFlow(true)
}.emit(true)
}
}
}
override fun deleteKey(alias: String) {
override suspend fun deleteKey(alias: String) {
secretKeyForAlias.remove(alias)
hasKeyMap.getOrPut(alias) {
MutableStateFlow(false)
}.emit(false)
}
private fun generateKey(): SecretKey {