Merge pull request #5600 from element-hq/feature/bma/deletePinCode

Delete pin code only when the last session is deleted
This commit is contained in:
Benoit Marty 2025-10-24 09:47:57 +02:00 committed by GitHub
commit f1b8f878de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 181 additions and 67 deletions

View file

@ -33,8 +33,7 @@ class DefaultSeenInvitesStore(
) : SeenInvitesStore {
init {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
if (sessionId.value == userId) {
clear()
}

View file

@ -35,12 +35,6 @@ interface LockScreenService {
fun isPinSetup(): Flow<Boolean>
}
/**
* Check if the app is currently locked.
*/
val LockScreenService.isLocked: Boolean
get() = lockState.value == LockScreenLockState.Locked
/**
* Makes sure the secure flag is set on the activity if the pin is setup.
* @param activity the activity to set the flag on.

View file

@ -73,15 +73,14 @@ class DefaultLockScreenService(
}
/**
* Makes sure to delete the pin code when the session is deleted.
* Makes sure to delete the pin code when the last session is deleted.
*/
private fun observeSessionsState() {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
override suspend fun onSessionDeleted(userId: String) {
// TODO handle multi session at some point
pinCodeManager.deletePinCode()
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
if (wasLastSession) {
pinCodeManager.deletePinCode()
}
}
})
}

View file

@ -0,0 +1,95 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.lockscreen.impl
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.lockscreen.impl.biometric.BiometricAuthenticatorManager
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.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.sessionstorage.api.observer.SessionObserver
import io.element.android.libraries.sessionstorage.test.observer.FakeSessionObserver
import io.element.android.services.appnavstate.api.AppForegroundStateService
import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultLockScreenServiceTest {
@Test
fun `when the pin is not mandatory and no pin is configured isSetupRequired emits false`() = runTest {
val sut = createDefaultLockScreenService(
lockScreenConfig = aLockScreenConfig(isPinMandatory = false)
)
sut.isSetupRequired().test {
assertThat(awaitItem()).isFalse()
}
}
@Test
fun `when the pin is mandatory, isSetupRequired emits true`() = runTest {
val lockScreenStore = InMemoryLockScreenStore()
val sut = createDefaultLockScreenService(
lockScreenConfig = aLockScreenConfig(isPinMandatory = true),
lockScreenStore = lockScreenStore,
)
sut.isSetupRequired().test {
assertThat(awaitItem()).isTrue()
// When the user configures the pin code, the setup is not required anymore
lockScreenStore.saveEncryptedPinCode("encryptedCode")
assertThat(awaitItem()).isFalse()
// Users deletes the pin code
lockScreenStore.deleteEncryptedPinCode()
assertThat(awaitItem()).isTrue()
}
}
@Test
fun `when the last session is deleted, the pin code is removed`() = runTest {
val sessionObserver = FakeSessionObserver()
val lockScreenStore = InMemoryLockScreenStore()
val sut = createDefaultLockScreenService(
lockScreenConfig = aLockScreenConfig(isPinMandatory = true),
lockScreenStore = lockScreenStore,
sessionObserver = sessionObserver,
)
sut.isPinSetup().test {
assertThat(awaitItem()).isFalse()
// When the user configure the pin code, the setup is not required anymore
lockScreenStore.saveEncryptedPinCode("encryptedCode")
assertThat(awaitItem()).isTrue()
sessionObserver.onSessionDeleted("userId", wasLastSession = false)
expectNoEvents()
sessionObserver.onSessionDeleted("userId", wasLastSession = true)
assertThat(awaitItem()).isFalse()
}
}
}
private fun TestScope.createDefaultLockScreenService(
lockScreenConfig: LockScreenConfig = aLockScreenConfig(),
lockScreenStore: LockScreenStore = InMemoryLockScreenStore(),
pinCodeManager: PinCodeManager = createDefaultPinCodeManager(
lockScreenStore = lockScreenStore,
),
sessionObserver: SessionObserver = FakeSessionObserver(),
appForegroundStateService: AppForegroundStateService = FakeAppForegroundStateService(),
biometricAuthenticatorManager: BiometricAuthenticatorManager = FakeBiometricAuthenticatorManager(),
) = DefaultLockScreenService(
lockScreenConfig = lockScreenConfig,
lockScreenStore = lockScreenStore,
pinCodeManager = pinCodeManager,
coroutineScope = backgroundScope,
sessionObserver = sessionObserver,
appForegroundStateService = appForegroundStateService,
biometricAuthenticatorManager = biometricAuthenticatorManager,
)

View file

@ -10,19 +10,18 @@ package io.element.android.features.lockscreen.impl.pin
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
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.EncryptionDecryptionService
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import io.element.android.libraries.cryptography.impl.AESEncryptionDecryptionService
import io.element.android.libraries.cryptography.test.SimpleSecretKeyRepository
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultPinCodeManagerTest {
private val lockScreenStore = InMemoryLockScreenStore()
private val secretKeyRepository = SimpleSecretKeyRepository()
private val encryptionDecryptionService = AESEncryptionDecryptionService()
private val pinCodeManager = DefaultPinCodeManager(secretKeyRepository, encryptionDecryptionService, lockScreenStore)
@Test
fun `given a pin code when create and delete assert no pin code left`() = runTest {
val pinCodeManager = createDefaultPinCodeManager()
pinCodeManager.hasPinCode().test {
assertThat(awaitItem()).isFalse()
pinCodeManager.createPinCode("1234")
@ -34,6 +33,7 @@ class DefaultPinCodeManagerTest {
@Test
fun `given a pin code when create and verify with the same pin succeed`() = runTest {
val pinCodeManager = createDefaultPinCodeManager()
val pinCode = "1234"
pinCodeManager.createPinCode(pinCode)
assertThat(pinCodeManager.verifyPinCode(pinCode)).isTrue()
@ -41,7 +41,18 @@ class DefaultPinCodeManagerTest {
@Test
fun `given a pin code when create and verify with a different pin fails`() = runTest {
val pinCodeManager = createDefaultPinCodeManager()
pinCodeManager.createPinCode("1234")
assertThat(pinCodeManager.verifyPinCode("1235")).isFalse()
}
}
fun createDefaultPinCodeManager(
lockScreenStore: LockScreenStore = InMemoryLockScreenStore(),
secretKeyRepository: SecretKeyRepository = SimpleSecretKeyRepository(),
encryptionDecryptionService: EncryptionDecryptionService = AESEncryptionDecryptionService(),
) = DefaultPinCodeManager(
lockScreenStore = lockScreenStore,
secretKeyRepository = secretKeyRepository,
encryptionDecryptionService = encryptionDecryptionService,
)

View file

@ -35,9 +35,7 @@ class DefaultImageLoaderHolder(
private fun observeSessions() {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
remove(SessionId(userId))
}
})

View file

@ -30,8 +30,7 @@ class DefaultSessionPreferencesStoreFactory(
init {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
val sessionPreferences = cache.remove(SessionId(userId))
sessionPreferences?.clear()
}

View file

@ -108,10 +108,6 @@ class DefaultPushService(
sessionObserver.addListener(this)
}
override suspend fun onSessionCreated(userId: String) {
// Nothing to do
}
/**
* The session has been deleted.
* In this case, this is not necessary to unregister the pusher from the homeserver,
@ -119,7 +115,7 @@ class DefaultPushService(
* The current push provider may want to take action, and we need to
* cleanup the stores.
*/
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
val sessionId = SessionId(userId)
val userPushStore = userPushStoreFactory.getOrCreate(sessionId)
val currentPushProviderName = userPushStore.getPushProviderName()

View file

@ -59,9 +59,7 @@ class DefaultNotificationConversationService(
init {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
onSessionLogOut(SessionId(userId))
}
})

View file

@ -248,7 +248,7 @@ class DefaultPushServiceTest {
),
pushClientSecretStore = pushClientSecretStore,
)
defaultPushService.onSessionDeleted(A_SESSION_ID.value)
defaultPushService.onSessionDeleted(A_SESSION_ID.value, false)
assertThat(userPushStore.getPushProviderName()).isNull()
assertThat(pushClientSecretStore.getSecret(A_SESSION_ID)).isNull()
onSessionDeletedLambda.assertions().isCalledOnce().with(value(A_SESSION_ID))
@ -268,7 +268,7 @@ class DefaultPushServiceTest {
),
pushClientSecretStore = pushClientSecretStore,
)
defaultPushService.onSessionDeleted(A_SESSION_ID.value)
defaultPushService.onSessionDeleted(A_SESSION_ID.value, false)
assertThat(userPushStore.getPushProviderName()).isNull()
assertThat(pushClientSecretStore.getSecret(A_SESSION_ID)).isNull()
}

View file

@ -8,6 +8,6 @@
package io.element.android.libraries.sessionstorage.api.observer
interface SessionListener {
suspend fun onSessionCreated(userId: String)
suspend fun onSessionDeleted(userId: String)
suspend fun onSessionCreated(userId: String) {}
suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {}
}

View file

@ -60,9 +60,10 @@ class DefaultSessionObserver(
// Compute diff
// Removed user
val removedUsers = currentUserSet - newUserSet
val wasLastSession = newUserSet.isEmpty()
removedUsers.forEach { removedUser ->
listeners.onEach { listener ->
listener.onSessionDeleted(removedUser)
listener.onSessionDeleted(removedUser, wasLastSession)
}
}
// Added user

View file

@ -24,7 +24,7 @@ class DatabaseSessionStoreTest {
private lateinit var database: SessionDatabase
private lateinit var databaseSessionStore: DatabaseSessionStore
private val aSessionData = aSessionData()
private val aSessionData = aDbSessionData()
@OptIn(ExperimentalCoroutinesApi::class)
@Before

View file

@ -10,8 +10,10 @@ package io.element.android.libraries.sessionstorage.impl
import io.element.android.libraries.matrix.session.SessionData
import io.element.android.libraries.sessionstorage.api.LoginType
internal fun aSessionData() = SessionData(
userId = "userId",
internal fun aDbSessionData(
userId: String = "userId",
) = SessionData(
userId = userId,
deviceId = "deviceId",
accessToken = "accessToken",
refreshToken = "refreshToken",

View file

@ -11,11 +11,10 @@ import app.cash.sqldelight.driver.jdbc.sqlite.JdbcSqliteDriver
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.sessionstorage.impl.DatabaseSessionStore
import io.element.android.libraries.sessionstorage.impl.SessionDatabase
import io.element.android.libraries.sessionstorage.impl.aSessionData
import io.element.android.libraries.sessionstorage.impl.aDbSessionData
import io.element.android.libraries.sessionstorage.impl.toApiModel
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancelChildren
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runCurrent
@ -23,7 +22,8 @@ import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Test
@OptIn(ExperimentalCoroutinesApi::class) class DefaultSessionObserverTest {
@OptIn(ExperimentalCoroutinesApi::class)
class DefaultSessionObserverTest {
private lateinit var database: SessionDatabase
private lateinit var databaseSessionStore: DatabaseSessionStore
@ -46,7 +46,7 @@ import org.junit.Test
@Test
fun `adding data invokes onSessionCreated`() = runTest {
val sessionData = aSessionData()
val sessionData = aDbSessionData()
val sut = createDefaultSessionObserver()
runCurrent()
val listener = TestSessionListener()
@ -54,12 +54,11 @@ import org.junit.Test
databaseSessionStore.addSession(sessionData.toApiModel())
listener.assertEvents(TestSessionListener.Event.Created(sessionData.userId))
sut.removeListener(listener)
coroutineContext.cancelChildren()
}
@Test
fun `adding and deleting data invokes onSessionCreated and onSessionDeleted`() = runTest {
val sessionData = aSessionData()
val sessionData = aDbSessionData()
val sut = createDefaultSessionObserver()
runCurrent()
val listener = TestSessionListener()
@ -69,15 +68,34 @@ import org.junit.Test
databaseSessionStore.removeSession(sessionData.userId)
listener.assertEvents(
TestSessionListener.Event.Created(sessionData.userId),
TestSessionListener.Event.Deleted(sessionData.userId),
TestSessionListener.Event.Deleted(sessionData.userId, true),
)
}
@Test
fun `adding and deleting data twice invokes onSessionCreated and onSessionDeleted`() = runTest {
val sessionData1 = aDbSessionData(userId = "user1")
val sessionData2 = aDbSessionData(userId = "user2")
val sut = createDefaultSessionObserver()
runCurrent()
val listener = TestSessionListener()
sut.addListener(listener)
databaseSessionStore.addSession(sessionData1.toApiModel())
databaseSessionStore.addSession(sessionData2.toApiModel())
databaseSessionStore.removeSession(sessionData2.userId)
databaseSessionStore.removeSession(sessionData1.userId)
listener.assertEvents(
TestSessionListener.Event.Created(sessionData1.userId),
TestSessionListener.Event.Created(sessionData2.userId),
TestSessionListener.Event.Deleted(sessionData2.userId, wasLastSession = false),
TestSessionListener.Event.Deleted(sessionData1.userId, wasLastSession = true),
)
coroutineContext.cancelChildren()
}
private fun TestScope.createDefaultSessionObserver(): DefaultSessionObserver {
return DefaultSessionObserver(
sessionStore = databaseSessionStore,
coroutineScope = this,
coroutineScope = backgroundScope,
dispatchers = testCoroutineDispatchers(useUnconfinedTestDispatcher = true),
)
}

View file

@ -13,7 +13,7 @@ import io.element.android.libraries.sessionstorage.api.observer.SessionListener
class TestSessionListener : SessionListener {
sealed interface Event {
data class Created(val userId: String) : Event
data class Deleted(val userId: String) : Event
data class Deleted(val userId: String, val wasLastSession: Boolean) : Event
}
private val trackRecord: MutableList<Event> = mutableListOf()
@ -22,8 +22,8 @@ class TestSessionListener : SessionListener {
trackRecord.add(Event.Created(userId))
}
override suspend fun onSessionDeleted(userId: String) {
trackRecord.add(Event.Deleted(userId))
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
trackRecord.add(Event.Deleted(userId, wasLastSession))
}
fun assertEvents(vararg events: Event) {

View file

@ -28,7 +28,7 @@ class FakeSessionObserver : SessionObserver {
listeners.forEach { it.onSessionCreated(userId) }
}
suspend fun onSessionDeleted(userId: String) {
listeners.forEach { it.onSessionDeleted(userId) }
suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean = true) {
listeners.forEach { it.onSessionDeleted(userId, wasLastSession = wasLastSession) }
}
}

View file

@ -16,7 +16,6 @@ import im.vector.app.features.analytics.itf.VectorAnalyticsScreen
import im.vector.app.features.analytics.plan.SuperProperties
import im.vector.app.features.analytics.plan.UserProperties
import io.element.android.libraries.di.annotations.AppCoroutineScope
import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.libraries.sessionstorage.api.observer.SessionListener
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import io.element.android.services.analytics.api.AnalyticsService
@ -39,7 +38,6 @@ class DefaultAnalyticsService(
@AppCoroutineScope
private val coroutineScope: CoroutineScope,
private val sessionObserver: SessionObserver,
private val sessionStore: SessionStore,
) : AnalyticsService, SessionListener {
// Cache for the store values
private val userConsent = AtomicBoolean(false)
@ -75,13 +73,9 @@ class DefaultAnalyticsService(
analyticsStore.setAnalyticsId(analyticsId)
}
override suspend fun onSessionCreated(userId: String) {
// Nothing to do
}
override suspend fun onSessionDeleted(userId: String) {
override suspend fun onSessionDeleted(userId: String, wasLastSession: Boolean) {
// Delete the store when the last session is deleted
if (sessionStore.getAllSessions().isEmpty()) {
if (wasLastSession) {
analyticsStore.reset()
}
}

View file

@ -16,9 +16,7 @@ import im.vector.app.features.analytics.plan.MobileScreen
import im.vector.app.features.analytics.plan.PollEnd
import im.vector.app.features.analytics.plan.SuperProperties
import im.vector.app.features.analytics.plan.UserProperties
import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver
import io.element.android.services.analytics.impl.store.AnalyticsStore
import io.element.android.services.analytics.impl.store.FakeAnalyticsStore
@ -178,10 +176,24 @@ class DefaultAnalyticsServiceTest {
coroutineScope = backgroundScope,
analyticsStore = store,
)
sut.onSessionDeleted("userId")
sut.onSessionDeleted("userId", true)
resetLambda.assertions().isCalledOnce()
}
@Test
fun `when a session is deleted, the store is not reset if it was not the last session`() = runTest {
val resetLambda = lambdaRecorder<Unit> { }
val store = FakeAnalyticsStore(
resetLambda = resetLambda,
)
val sut = createDefaultAnalyticsService(
coroutineScope = backgroundScope,
analyticsStore = store,
)
sut.onSessionDeleted("userId", false)
resetLambda.assertions().isNeverCalled()
}
@Test
fun `when a session is added, nothing happen`() = runTest {
val sut = createDefaultAnalyticsService(
@ -260,13 +272,11 @@ class DefaultAnalyticsServiceTest {
),
analyticsStore: AnalyticsStore = FakeAnalyticsStore(),
sessionObserver: SessionObserver = NoOpSessionObserver(),
sessionStore: SessionStore = InMemorySessionStore(),
) = DefaultAnalyticsService(
analyticsProviders = analyticsProviders,
analyticsStore = analyticsStore,
coroutineScope = coroutineScope,
sessionObserver = sessionObserver,
sessionStore = sessionStore,
).also {
// Wait for the service to be ready
delay(1)