fix(push): stop pushes from a removed server on Android

androidApp/src/androidMain/kotlin/org/siloserver/silo/android/auth/SignOutTeardown.kt

@@ -74,10 +74,12 @@ class SignOutTeardown(
         fun stillActive() = serverRegistry.activeServerId.value == signedOutOf
         if (flushFirst) flushPendingSettings()
         if (!stillActive()) return false
-        // While the credentials removing it needs still exist. A registration
-        // left behind keeps the server pushing this device for an account it
-        // no longer holds, but failing to remove it never stops the sign-out.
-        // Cancellation still does, before anyone is signed out.
+        // Before the credentials are cleared: a server without the retirement
+        // route can only be told with them. A retirement the server has not
+        // answered stays queued and is sent again without them, as a
+        // registration left behind keeps the server pushing this device for an
+        // account it no longer holds. Failing to remove it never stops the
+        // sign-out; cancellation still does, before anyone is signed out.
         var removal: PushRemoval? = null
         if (signedOutOf != null) tolerateFailure { removal = unregisterPush(signedOutOf) }
         if (!stillActive()) {

androidApp/src/androidMain/kotlin/org/siloserver/silo/android/push/AndroidPushRegistrationStarter.kt

@@ -24,6 +24,9 @@ class AndroidPushRegistrationStarter(
         if (scope != null) return
         val nextScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
         scope = nextScope
+        // A removed or signed-out server keeps pushing until its retirement lands.
+        // It needs no login, so it goes out on app start and every foreground.
+        nextScope.launch { runCatching { registrar.retryPendingRetirements() } }
         nextScope.registerOnce()
         nextScope.launch {
             notificationsRepository.resetSignals.collect {

androidApp/src/androidMain/kotlin/org/siloserver/silo/android/push/PushInstallationStore.kt

@@ -28,8 +28,19 @@ internal data class PushInstallation(
      * registration supersedes it instead.
      */
     val ownerLeaving: Boolean = false,
+    /**
+     * The removal intent at this record's generation still has to reach the
+     * server as a retirement, which needs only [key]: it is sent again after
+     * the login that registered is gone, until the server answers for good.
+     * Any newer generation supersedes it.
+     */
+    val retirement: PushRetirement? = null,
 ) { override fun toString() = "PushInstallation(<redacted>)" }
 
+/** A queued retirement of [generation], first queued at [queuedAtMs] (epoch milliseconds). */
+@Serializable
+internal data class PushRetirement(val generation: Long, val queuedAtMs: Long)
+
 @Serializable
 internal data class PushInstallationIntent(
     val serverId: String,

androidApp/src/androidMain/kotlin/org/siloserver/silo/android/push/SiloFirebaseMessagingService.kt

@@ -3,6 +3,7 @@ package org.siloserver.silo.android.push
 import com.google.firebase.messaging.FirebaseMessagingService
 import com.google.firebase.messaging.RemoteMessage
 import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeoutOrNull
 import org.koin.java.KoinJavaComponent.get
@@ -116,18 +117,24 @@ class SiloFirebaseMessagingService : FirebaseMessagingService() {
 
     override fun onMessageReceived(message: RemoteMessage) {
         val handler = runCatching { get<PushMessageHandler>(PushMessageHandler::class.java) }.getOrNull()
-            ?: return
-        // Budgeted inside the presenter rather than here, so a stalled server
-        // still ends in a posted notification rather than none at all. The catch
-        // is a boundary net: nothing may escape into the Firebase SDK.
+        val registrar = runCatching { get<AndroidPushRegistrar>(AndroidPushRegistrar::class.java) }.getOrNull()
         runBlocking {
-            runCatching {
-                handler.handle(
-                    data = message.data,
-                    fallbackTitle = message.notification?.title,
-                    fallbackBody = message.notification?.body,
-                )
+            // The push may come from a server this device retired: its pending
+            // retirement goes out again beside the delivery, inside the same budget.
+            val retirements = registrar?.let { launch { withinPushBudget { it.retryPendingRetirements(afterPush = true) } } }
+            // Budgeted inside the presenter rather than here, so a stalled server
+            // still ends in a posted notification rather than none at all. The catch
+            // is a boundary net: nothing may escape into the Firebase SDK.
+            handler?.let {
+                runCatching {
+                    it.handle(
+                        data = message.data,
+                        fallbackTitle = message.notification?.title,
+                        fallbackBody = message.notification?.body,
+                    )
+                }
             }
+            retirements?.join()
         }
     }
 

androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/servers/ServerListViewModel.kt

@@ -124,9 +124,11 @@ class ServerListViewModel(
     fun onRemove(serverId: String) {
         viewModelScope.launch {
             val wasActive = serverRegistry.activeServerId.value == serverId
-            // While the server's credentials still exist: removal wipes them.
-            // Failing to remove the registration never stops the removal;
-            // cancellation still does, before anything is removed.
+            // Before removal wipes the server's credentials, which a server
+            // without the retirement route needs; a retirement it has not
+            // answered is sent again later without them. Failing to remove the
+            // registration never stops the removal; cancellation still does,
+            // before anything is removed.
             var removal: PushRemoval? = null
             tolerateFailure { removal = unregisterPush(serverId) }
             try {

androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/push/AndroidPushRegistrarTest.kt

@@ -1,13 +1,21 @@
 package org.siloserver.silo.android.push
 
+import android.app.Application
 import kotlinx.coroutines.*
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
 import org.siloserver.silo.model.notifications.*
 import org.siloserver.silo.network.*
 import org.siloserver.silo.network.api.PushRegistrationApi
 import org.siloserver.silo.repository.PushRegistrationRepository
 import kotlin.test.*
 
+// Robolectric for the diagnostics log lines the registrar writes.
+@RunWith(RobolectricTestRunner::class)
+@Config(application=Application::class, sdk=[28])
 class AndroidPushRegistrarTest {
     private var owner = DurableLoginAuthority("login", AuthScopeSnapshot("server","profile","https://example.invalid","pin",identityGeneration=1))
     private val token = "t".repeat(80)
@@ -44,6 +52,11 @@ class AndroidPushRegistrarTest {
     private var unreachableUrl: String?=null
     private var available=true
     private var onAvailable: suspend (AuthScopeSnapshot) -> Unit = {}
+    /** The retirement route's answer: 200 retires, null is no answer. A server without the route by default. */
+    private var retireStatus: Int? = 404
+    /** Each retirement sent: server URL, key and generation. It carries no login at all. */
+    private val retirements=mutableListOf<Triple<String,String,Long>>()
+    private var onRetire: suspend () -> Unit = {}
     private val api=object : PushRegistrationApi {
         override suspend fun available(owner: AuthScopeSnapshot): ApiResult<Boolean> { onAvailable(owner); return ApiResult.Success(available) }
         override suspend fun register(request: PushDeviceRegisterRequest,key:String,generation:Long,owner:AuthScopeSnapshot):ApiResult<PushDeviceRegisterResponse> {
@@ -61,6 +74,17 @@ class AndroidPushRegistrarTest {
             if(serverProfiles[owner.serverUrl].let { it!=null && it!=owner.profileId }) return ApiResult.Error(409,"conflict","")
             return if(uncertain) ApiResult.NetworkError(IllegalStateException("lost reply")) else ApiResult.Success(Unit)
         }
+        override suspend fun retire(serverUrl:String,deviceId:String,key:String,generation:Long):ApiResult<Unit> {
+            val record=store.entries.single { it.serverUrl==serverUrl }
+            assertEquals(generation,record.retirement?.generation) // durable before dispatch
+            assertNull(record.intent.registration); assertEquals("device",deviceId)
+            retirements+=Triple(serverUrl,key,generation); onRetire()
+            return when(val status=retireStatus) {
+                null -> ApiResult.NetworkError(IllegalStateException("no answer"))
+                200 -> ApiResult.Success(Unit)
+                else -> ApiResult.Error(status,"","")
+            }
+        }
     }
     private fun registrar(provider: AndroidPushTokenProvider=object:AndroidPushTokenProvider { override suspend fun token()=token }) =
         AndroidPushRegistrar(provider,PushRegistrationRepository(api),{"device"},authorities,store,{proof})
@@ -147,7 +171,8 @@ class AndroidPushRegistrarTest {
         available=true
         owner=owner.copy(loginId="next",scope=owner.scope.copy(identityGeneration=2))
         assertIs<ApiResult.Success<*>>(r.registerToken(token))
-        assertEquals(listOf(1L,2L),generations); assertEquals("next",store.entries.single().intent.loginId)
+        // The removal took generation 2 before anything was sent, so the next login registers above it.
+        assertEquals(listOf(1L,3L),generations); assertEquals("next",store.entries.single().intent.loginId)
     }
     @Test fun theNextForegroundGivesOtherServersARotatedTokenTheyMissed()=runTest {
         val r=registrar(); r.registerToken(token)
@@ -278,6 +303,140 @@ class AndroidPushRegistrarTest {
         savedLogins.remove("server") // the sign-out went through after all
         assertIs<ApiResult.Error>(r.restoreRegistration(removal)); assertNull(store.entries.single().intent.registration)
     }
+    @Test fun aRemovalIsRetiredWithoutALoginAtTheGenerationItPersisted()=runTest {
+        val r=registrar(); r.registerToken(token)
+        retireStatus=200
+        savedLogins.remove("server") // no login can act for that server any more
+        assertIs<ApiResult.Success<Unit>>(r.unregisterDevice("server").result)
+        assertEquals(listOf(Triple("https://example.invalid",proof,2L)),retirements)
+        assertEquals(listOf(1L),generations) // no session request
+        val record=store.entries.single()
+        assertNull(record.intent.registration); assertTrue(record.acknowledged); assertNull(record.retirement)
+    }
+    @Test fun aRemovalTheSessionCouldNotSendIsRetiredAfterARestartAtTheSameKeyAndGeneration()=runTest {
+        registrar().registerToken(token)
+        // A server without the retirement route, whose session removal gets no answer.
+        uncertain=true
+        assertFalse(registrar().unregisterDevice("server").result is ApiResult.Success)
+        assertEquals(listOf(1L,2L),generations)
+        savedLogins.remove("server") // the sign-out goes ahead and clears the login
+        // After a restart, the updated server answers.
+        uncertain=false; retireStatus=200
+        registrar().retryPendingRetirements()
+        assertEquals(listOf(2L,2L),retirements.map { it.third }); assertEquals(setOf(proof),retirements.map { it.second }.toSet())
+        assertEquals(listOf(1L,2L),generations)
+        val record=store.entries.single(); assertTrue(record.acknowledged); assertNull(record.retirement)
+        registrar().retryPendingRetirements(); assertEquals(2,retirements.size)
+    }
+    @Test fun onlyAFinalAnswerDropsAQueuedRetirement()=runTest {
+        val day=24*60*60*1000L
+        // status to whether the entry is queued 31 days ago, and whether it is then dropped.
+        val cases=listOf(Triple(200,false,true),Triple(403,false,true),Triple(409,false,true),Triple(503,false,false),
+            Triple(null,false,false),Triple(404,false,false),Triple(404,true,true))
+        for ((status,aged,dropped) in cases) {
+            store.entries=emptyList(); savedLogins["server"]="login"
+            retireStatus=503; registrar().registerToken(token)
+            uncertain=true; registrar().unregisterDevice("server"); uncertain=false // the session removal gets no answer either
+            savedLogins.remove("server")
+            if (aged) store.entries=store.entries.map { it.copy(retirement=it.retirement!!.copy(queuedAtMs=it.retirement!!.queuedAtMs-31*day)) }
+            retireStatus=status; val sent=retirements.size
+            registrar().retryPendingRetirements()
+            assertEquals(sent+1,retirements.size,"$status")
+            val record=store.entries.single()
+            assertEquals(dropped,record.retirement==null,"$status aged=$aged")
+            assertEquals(status==200,record.acknowledged,"$status")
+            assertNull(record.intent.registration)
+        }
+    }
+    @Test fun signingInAgainOrARestoreDropsTheRetirementAndRegistersAboveIt()=runTest {
+        val r=registrar(); r.registerToken(token)
+        retireStatus=null; uncertain=true // neither the retirement nor the session removal is answered
+        assertFalse(r.unregisterDevice("server").result is ApiResult.Success)
+        uncertain=false
+        assertEquals(2L,store.entries.single().retirement?.generation)
+        owner=owner.copy(loginId="next",scope=owner.scope.copy(identityGeneration=2)); savedLogins["server"]="next"
+        assertIs<ApiResult.Success<*>>(r.registerToken(token))
+        assertEquals(listOf(1L,2L,3L),generations)
+        assertNull(store.entries.single().retirement)
+        registrar().retryPendingRetirements(); assertEquals(1,retirements.size)
+
+        // A sign-out that stops undoes its removal, retirement included.
+        uncertain=true; val removal=r.unregisterDevice("server"); uncertain=false
+        assertEquals(4L,store.entries.single().retirement?.generation)
+        assertIs<ApiResult.Success<*>>(r.restoreRegistration(removal))
+        assertEquals(listOf(1L,2L,3L,4L,5L),generations)
+        val record=store.entries.single(); assertNull(record.retirement); assertTrue(record.acknowledged)
+        registrar().retryPendingRetirements(); assertEquals(2,retirements.size)
+    }
+    @Test fun aRemovalThatRanOutOfTimeWaitingOnARegistrationIsRetiredOnceTheLoginIsGone()=runTest {
+        val r=registrar(); r.registerToken(token)
+        // A registration holds the server's lock past the removal's budget.
+        val gate=CompletableDeferred<Unit>(); onAvailable={ gate.await() }
+        val registering=launch { r.registerToken(token) }; runCurrent()
+        assertFalse(r.unregisterDevice("server").result is ApiResult.Success)
+        assertNull(store.entries.single().retirement)
+        savedLogins.remove("server") // the sign-out goes ahead and clears the login
+        onAvailable={}; gate.complete(Unit); registering.join()
+        retireStatus=200
+        registrar().retryPendingRetirements()
+        assertEquals(listOf(Triple("https://example.invalid",proof,2L)),retirements)
+        val record=store.entries.single(); assertNull(record.intent.registration); assertTrue(record.acknowledged); assertNull(record.retirement)
+    }
+    @Test fun aRemovalAnOlderVersionCouldNotSendIsRetiredAtItsOwnGeneration()=runTest {
+        registrar().registerToken(token)
+        // What a failed session removal left behind before retirements existed.
+        store.entries=store.entries.map { it.copy(generation=2, intent=it.intent.copy(registration=null), acknowledged=false, receipt=null, ownerLeaving=true) }
+        savedLogins.remove("server")
+        retireStatus=200
+        registrar().retryPendingRetirements()
+        assertEquals(listOf(Triple("https://example.invalid",proof,2L)),retirements)
+        assertTrue(store.entries.single().acknowledged)
+    }
+    @Test fun aDroppedRetirementIsNotQueuedAgain()=runTest {
+        val day=24*60*60*1000L
+        for (aged in listOf(false,true)) {
+            store.entries=emptyList(); retirements.clear(); savedLogins["server"]="login"
+            registrar().registerToken(token)
+            retireStatus=if (aged) null else 403
+            uncertain=aged; registrar().unregisterDevice("server"); uncertain=false; savedLogins.remove("server")
+            if (aged) {
+                store.entries=store.entries.map { it.copy(retirement=it.retirement!!.copy(queuedAtMs=it.retirement!!.queuedAtMs-91*day)) }
+                registrar().retryPendingRetirements() // expires unsent
+            }
+            assertNull(store.entries.single().retirement,"aged=$aged")
+            val sent=retirements.size
+            registrar().retryPendingRetirements()
+            assertEquals(sent,retirements.size,"aged=$aged"); assertNull(store.entries.single().retirement,"aged=$aged")
+        }
+    }
+    @Test fun aRetirementThatStallsWaitsOutTheBackOffLikeOneThatFailed()=runTest {
+        registrar().registerToken(token)
+        retireStatus=null; uncertain=true; registrar().unregisterDevice("server"); uncertain=false; savedLogins.remove("server")
+        val r=registrar() // a restart: no failures remembered
+        onRetire={ awaitCancellation() }
+        r.retryPendingRetirements(afterPush=true)
+        r.retryPendingRetirements(afterPush=true)
+        assertEquals(2,retirements.size) // the removal's and the stalled retry's; the next push waits
+        assertEquals(2L,store.entries.single().retirement?.generation)
+    }
+    @Test fun aSlowRetirementLeavesTheSessionRemovalTimeBeforeTheLoginIsCleared()=runTest {
+        val r=registrar(); r.registerToken(token)
+        // A server without the route that takes 7 seconds to say so, and a session removal that takes 1.5.
+        onRetire={ delay(7_000) }; send={ delay(1_500) }
+        assertIs<ApiResult.Success<Unit>>(r.unregisterDevice("server").result)
+        assertEquals(listOf(1L,2L),generations)
+        val record=store.entries.single(); assertTrue(record.acknowledged); assertNull(record.retirement)
+    }
+    @Test fun aRetirementWithoutAnAnswerFallsBackToTheSessionRemoval()=runTest {
+        for (status in listOf(null,429,503)) {
+            store.entries=emptyList(); generations.clear()
+            registrar().registerToken(token)
+            retireStatus=status
+            assertIs<ApiResult.Success<Unit>>(registrar().unregisterDevice("server").result,"$status")
+            assertEquals(listOf(1L,2L),generations,"$status")
+            val record=store.entries.single(); assertTrue(record.acknowledged,"$status"); assertNull(record.retirement,"$status")
+        }
+    }
     @Test fun conflictStaysDurableAndNeverRotatesProofOrGeneration()=runTest {
         failCode=409; registrar().registerToken(token)
         failCode=null