From fcb84016ca660959cadaf7f75ba2ddecb211fcfe Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Tue, 16 Jan 2024 14:34:06 +0100
Subject: [PATCH] Ensure client secret is deleted when the user signs out.

---
 .../api/clientsecret/PushClientSecret.kt      |  5 ----
 libraries/pushstore/impl/build.gradle.kts     |  1 +
 .../impl/clientsecret/PushClientSecretImpl.kt | 24 +++++++++++++++----
 .../clientsecret/PushClientSecretImplTest.kt  |  5 ++--
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/clientsecret/PushClientSecret.kt b/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/clientsecret/PushClientSecret.kt
index dbdd22ce07..1700d41181 100644
--- a/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/clientsecret/PushClientSecret.kt
+++ b/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/clientsecret/PushClientSecret.kt
@@ -29,9 +29,4 @@ interface PushClientSecret {
      * Return null if not found.
      */
     suspend fun getUserIdFromSecret(clientSecret: String): SessionId?
-
-    /**
-     * To call when the user signs out.
-     */
-    suspend fun resetSecretForUser(userId: SessionId)
 }
diff --git a/libraries/pushstore/impl/build.gradle.kts b/libraries/pushstore/impl/build.gradle.kts
index 9b01588090..28e53e011c 100644
--- a/libraries/pushstore/impl/build.gradle.kts
+++ b/libraries/pushstore/impl/build.gradle.kts
@@ -49,6 +49,7 @@ dependencies {
     testImplementation(libs.coroutines.test)
     testImplementation(projects.libraries.matrix.test)
     testImplementation(projects.services.appnavstate.test)
+    testImplementation(projects.libraries.sessionStorage.test)
 
     androidTestImplementation(libs.coroutines.test)
     androidTestImplementation(libs.test.core)
diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImpl.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImpl.kt
index ca0ed14e33..25c0332b43 100644
--- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImpl.kt
+++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImpl.kt
@@ -22,13 +22,20 @@ import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret
 import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretFactory
 import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore
+import io.element.android.libraries.sessionstorage.api.observer.SessionListener
+import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
 import javax.inject.Inject
 
-@ContributesBinding(AppScope::class)
+@ContributesBinding(AppScope::class, boundType = PushClientSecret::class)
 class PushClientSecretImpl @Inject constructor(
     private val pushClientSecretFactory: PushClientSecretFactory,
     private val pushClientSecretStore: PushClientSecretStore,
-) : PushClientSecret {
+    private val sessionObserver: SessionObserver,
+) : PushClientSecret, SessionListener {
+    init {
+        observeSessions()
+    }
+
     override suspend fun getSecretForUser(userId: SessionId): String {
         val existingSecret = pushClientSecretStore.getSecret(userId)
         if (existingSecret != null) {
@@ -43,7 +50,16 @@ class PushClientSecretImpl @Inject constructor(
         return pushClientSecretStore.getUserIdFromSecret(clientSecret)
     }
 
-    override suspend fun resetSecretForUser(userId: SessionId) {
-        pushClientSecretStore.resetSecret(userId)
+    private fun observeSessions() {
+        sessionObserver.addListener(this)
+    }
+
+    override suspend fun onSessionCreated(userId: String) {
+        // Nothing to do
+    }
+
+    override suspend fun onSessionDeleted(userId: String) {
+        // Delete the secret
+        pushClientSecretStore.resetSecret(SessionId(userId))
     }
 }
diff --git a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImplTest.kt b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImplTest.kt
index 5af4877ed3..dc0e5b3651 100644
--- a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImplTest.kt
+++ b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImplTest.kt
@@ -18,6 +18,7 @@ package io.element.android.libraries.pushstore.impl.clientsecret
 
 import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.matrix.api.core.SessionId
+import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
@@ -31,7 +32,7 @@ internal class PushClientSecretImplTest {
     fun test() = runTest {
         val factory = FakePushClientSecretFactory()
         val store = InMemoryPushClientSecretStore()
-        val sut = PushClientSecretImpl(factory, store)
+        val sut = PushClientSecretImpl(factory, store, NoOpSessionObserver())
 
         val secret0 = factory.getSecretForUser(0)
         val secret1 = factory.getSecretForUser(1)
@@ -56,7 +57,7 @@ internal class PushClientSecretImplTest {
         assertThat(sut.getUserIdFromSecret(A_UNKNOWN_SECRET)).isNull()
 
         // User signs out
-        sut.resetSecretForUser(A_USER_ID_0)
+        sut.onSessionDeleted(A_USER_ID_0.value)
         assertThat(store.getSecrets()).hasSize(1)
         // Create a new secret after reset
         assertThat(sut.getSecretForUser(A_USER_ID_0)).isEqualTo(secret2)