fix(login): wait for the pending account removal before logging in again

app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt

@@ -16,6 +16,7 @@ import android.os.Bundle
 import android.widget.Toast
 import androidx.activity.OnBackPressedCallback
 import androidx.activity.viewModels
+import androidx.annotation.StringRes
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.ViewModelProvider
@@ -115,8 +116,11 @@ class BrowserLoginActivity : BaseActivity() {
                         }
                         BrowserLoginActivityViewModel.PostLoginViewState.PostLoginTooManyLoginAttempts -> {
                             logger.e(TAG, "Login refused because of too many failed logins")
-                            Snackbar.make(binding.root, R.string.nc_login_too_many_attempts, Snackbar.LENGTH_LONG)
-                                .show()
+                            showLongSnackbar(R.string.nc_login_too_many_attempts)
+                        }
+                        BrowserLoginActivityViewModel.PostLoginViewState.PostLoginRemovalPending -> {
+                            logger.e(TAG, "Account removal did not finish in time")
+                            showLongSnackbar(R.string.nc_account_removal_pending)
                         }
                         BrowserLoginActivityViewModel.PostLoginViewState.PostLoginRestartApp -> {
                             restartApp()
@@ -179,6 +183,10 @@ class BrowserLoginActivity : BaseActivity() {
         startActivity(intent)
     }
 
+    private fun showLongSnackbar(@StringRes message: Int) {
+        Snackbar.make(binding.root, message, Snackbar.LENGTH_LONG).show()
+    }
+
     private fun restartApp() {
         val intent = Intent(context, MainActivity::class.java)
         intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)

app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt

@@ -52,6 +52,9 @@ class LoginRepository(val network: NetworkLoginDataSource, val local: LocalLogin
         /** An existing account, reauthorized or not, or an account that is being removed. */
         data object ExistingAccount : LoginResult
 
+        /** The account is still being removed, so it can't be set up again yet. */
+        data object RemovalPending : LoginResult
+
         /** A reauthorization in which a different account than the one to reauthorize logged in. */
         data object DifferentAccount : LoginResult
     }
@@ -211,16 +214,22 @@ class LoginRepository(val network: NetworkLoginDataSource, val local: LocalLogin
     }
 
     /**
-     * Returns [LoginResult.NewAccount] if the account is not scheduled for deletion and doesn't exist yet. During a
+     * Returns [LoginResult.NewAccount] if the account doesn't exist yet. An account that is scheduled for deletion is
+     * waited for first, [LoginResult.RemovalPending] if its removal does not finish in time. During a
      * reauthorization, [LoginResult.DifferentAccount] if another account than the one to reauthorize logged in, set
      * up or not, which is then left unchanged.
      */
     suspend fun parseAndLogin(loginData: LoginCompletion): LoginResult {
         if (local.checkIfUserIsScheduledForDeletion(loginData)) {
-            // however the user is not yet deleted, just start AccountRemovalWorker again to make sure to delete it.
+            // The account was removed just before and the removal did not finish yet. Make sure it runs and wait for
+            // it, as the account can then be set up again. Otherwise the login would end without any account.
             local.startAccountRemovalWorker()
-            return LoginResult.ExistingAccount
-        } else if (local.checkIfUserExists(loginData)) {
+            if (!local.awaitUserRemoval(loginData)) {
+                return LoginResult.RemovalPending
+            }
+        }
+
+        if (local.checkIfUserExists(loginData)) {
             if (shouldReauthorizeUser) {
                 if (!local.updateUser(loginData, accountToReauthorize)) {
                     Log.w(TAG, "Logged in with a different account than the one to reauthorize. Skipped update.")

app/src/main/java/com/nextcloud/talk/account/data/io/LocalLoginDataSource.kt

@@ -17,11 +17,18 @@ import com.nextcloud.talk.account.data.model.LoginCompletion
 import com.nextcloud.talk.jobs.AccountRemovalWorker
 import com.nextcloud.talk.users.UserManager
 import com.nextcloud.talk.utils.preferences.AppPreferences
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.withTimeoutOrNull
 
 // local datasource for communicating with room through account manager
 // crucial for making sure the login process interacts with the db as expected.
 class LocalLoginDataSource(val userManager: UserManager, val appPreferences: AppPreferences, val context: Context) {
 
+    companion object {
+        private const val REMOVAL_TIMEOUT_MILLIS = 30_000L
+        private const val REMOVAL_POLL_INTERVAL_MILLIS = 200L
+    }
+
     /**
      * Stores the new credentials of a reauthorized account. That is the account that logged in, found by its login
      * name and server, as the browser login flow grants an app password for whoever is logged in there. Returns false
@@ -48,6 +55,19 @@ class LocalLoginDataSource(val userManager: UserManager, val appPreferences: App
         return WorkManager.getInstance(context).getWorkInfoByIdLiveData(accountRemovalWork.id)
     }
 
+    /**
+     * Waits until the account of [data] is not scheduled for deletion anymore, i.e. the account removal finished, for
+     * at most [timeoutMillis]. Returns false if it still is. The removal also contacts the server and the push proxy,
+     * so it can take a while.
+     */
+    suspend fun awaitUserRemoval(data: LoginCompletion, timeoutMillis: Long = REMOVAL_TIMEOUT_MILLIS): Boolean =
+        withTimeoutOrNull(timeoutMillis) {
+            while (checkIfUserIsScheduledForDeletion(data)) {
+                delay(REMOVAL_POLL_INTERVAL_MILLIS)
+            }
+            true
+        } ?: false
+
     suspend fun checkIfUserIsScheduledForDeletion(data: LoginCompletion): Boolean =
         userManager.checkIfUserIsScheduledForDeletion(data.loginName, data.server)
 

app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt

@@ -41,6 +41,7 @@ class BrowserLoginActivityViewModel @Inject constructor(val repository: LoginRep
         data object PostLoginTooManyLoginAttempts : PostLoginViewState()
         data class PostLoginContinue(val data: Bundle) : PostLoginViewState()
         data object PostLoginDifferentAccount : PostLoginViewState()
+        data object PostLoginRemovalPending : PostLoginViewState()
     }
 
     private val _postLoginState = MutableStateFlow<PostLoginViewState>(PostLoginViewState.None)
@@ -121,6 +122,7 @@ class BrowserLoginActivityViewModel @Inject constructor(val repository: LoginRep
             is LoginRepository.LoginResult.NewAccount -> PostLoginViewState.PostLoginContinue(result.bundle)
             LoginRepository.LoginResult.ExistingAccount -> PostLoginViewState.PostLoginRestartApp
             LoginRepository.LoginResult.DifferentAccount -> PostLoginViewState.PostLoginDifferentAccount
+            LoginRepository.LoginResult.RemovalPending -> PostLoginViewState.PostLoginRemovalPending
         }
 
     fun cancelLogin() = repository.cancelLoginFlow()

app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java

@@ -42,6 +42,7 @@
 import androidx.work.Worker;
 import androidx.work.WorkerParameters;
 import autodagger.AutoInjector;
+import io.reactivex.Observable;
 import io.reactivex.Observer;
 import io.reactivex.disposables.Disposable;
 import kotlin.coroutines.EmptyCoroutineContext;
@@ -129,6 +130,9 @@ public void onNext(@io.reactivex.annotations.NonNull GenericOverall genericOvera
                                 queryMap.put("deviceIdentifierSignature",
                                              finalPushConfigurationState.getDeviceIdentifierSignature());
                                 unregisterDeviceForNotificationWithProxy(queryMap, user);
+                            } else {
+                                Log.e(TAG, "unregister Device For Notifications failed with status " + statusCode);
+                                initiateUserDeletion(user);
                             }
                         }
 
@@ -152,45 +156,40 @@ public void onComplete() {
     }
 
     private void unregisterDeviceForNotificationWithProxy(HashMap<String, String> queryMap, User user) {
-        ncApi.unregisterDeviceForNotificationsWithProxy
-                (ApiUtils.getUrlPushProxy(), queryMap)
-                .subscribe(new Observer<Void>() {
-                    @Override
-                    public void onSubscribe(Disposable d) {
-                        // unused atm
-                    }
-
-                    @Override
-                    public void onNext(Void aVoid) {
-                            String groupName = String.format(
-                                getApplicationContext()
-                                    .getResources()
-                                    .getString(R.string.nc_notification_channel), user.getUserId(), user.getBaseUrl());
-                            CRC32 crc32 = new CRC32();
-                            crc32.update(groupName.getBytes());
-                            NotificationManager notificationManager =
-                                    (NotificationManager) getApplicationContext()
-                                        .getSystemService(Context.NOTIFICATION_SERVICE);
-
-                            if (notificationManager != null) {
-                                notificationManager.deleteNotificationChannelGroup(
-                                    Long.toString(crc32.getValue()));
-                            }
-
-                        initiateUserDeletion(user);
-                    }
+        Throwable error = awaitCompletion(
+            ncApi.unregisterDeviceForNotificationsWithProxy(ApiUtils.getUrlPushProxy(), queryMap));
+
+        if (error == null) {
+            String groupName = String.format(
+                getApplicationContext()
+                    .getResources()
+                    .getString(R.string.nc_notification_channel), user.getUserId(), user.getBaseUrl());
+            CRC32 crc32 = new CRC32();
+            crc32.update(groupName.getBytes());
+            NotificationManager notificationManager =
+                (NotificationManager) getApplicationContext()
+                    .getSystemService(Context.NOTIFICATION_SERVICE);
+
+            if (notificationManager != null) {
+                notificationManager.deleteNotificationChannelGroup(
+                    Long.toString(crc32.getValue()));
+            }
+        } else {
+            Log.e(TAG, "error while trying to unregister Device For Notification With Proxy", error);
+        }
 
-                    @Override
-                    public void onError(Throwable e) {
-                        Log.e(TAG, "error while trying to unregister Device For Notification With Proxy", e);
-                        initiateUserDeletion(user);
-                    }
+        initiateUserDeletion(user);
+    }
 
-                    @Override
-                    public void onComplete() {
-                        // unused atm
-                    }
-                });
+    /**
+     * Blocks until the request ended, so that doWork() returns only after the user is deleted. Callers restart the
+     * app or log in again as soon as the work is SUCCEEDED, and the user must be gone by then.
+     * The elements are ignored: the body of the proxy answer is null, which a blocking subscriber can't take.
+     *
+     * @return the error, or null if the request succeeded
+     */
+    static Throwable awaitCompletion(Observable<?> request) {
+        return request.ignoreElements().blockingGet();
     }
 
     private void initiateUserDeletion(User user) {

app/src/main/res/values/strings.xml

@@ -33,6 +33,7 @@ How to translate with transifex:
     <string name="nc_common_dismiss">Dismiss</string>
     <string name="nc_common_error_sorry">Sorry, something went wrong!</string>
     <string name="nc_login_too_many_attempts">Too many failed login attempts from this network. Try again later.</string>
+    <string name="nc_account_removal_pending">The removal of this account is not finished yet. Please try again in a moment.</string>
     <string name="nc_reauthorize_different_account">You logged in with a different account. Log in with the account you want to reauthorize.</string>
     <string name="nc_common_create">Create</string>
     <string name="nc_common_unknown">Unknown</string>

app/src/test/java/com/nextcloud/talk/account/data/io/LocalLoginDataSourceTest.kt

@@ -0,0 +1,46 @@
+/*
+ * Nextcloud Talk - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.nextcloud.talk.account.data.io
+
+import com.nextcloud.talk.account.data.model.LoginCompletion
+import com.nextcloud.talk.users.UserManager
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.times
+import org.mockito.kotlin.verify
+import org.mockito.kotlin.whenever
+
+@OptIn(ExperimentalCoroutinesApi::class)
+class LocalLoginDataSourceTest {
+
+    private val userManager: UserManager = mock()
+    private val dataSource = LocalLoginDataSource(userManager, mock(), mock())
+    private val loginData = LoginCompletion(200, "https://example.com", "user", "pass")
+
+    @Test
+    fun `awaitUserRemoval returns once the user is not scheduled for deletion anymore`() =
+        runTest {
+            whenever(userManager.checkIfUserIsScheduledForDeletion("user", "https://example.com"))
+                .thenReturn(true, true, false)
+
+            assertTrue(dataSource.awaitUserRemoval(loginData, 10_000L))
+            verify(userManager, times(3)).checkIfUserIsScheduledForDeletion("user", "https://example.com")
+        }
+
+    @Test
+    fun `awaitUserRemoval gives up when the removal does not finish in time`() =
+        runTest {
+            whenever(userManager.checkIfUserIsScheduledForDeletion("user", "https://example.com"))
+                .thenReturn(true)
+
+            assertFalse(dataSource.awaitUserRemoval(loginData, 1_000L))
+        }
+}

app/src/test/java/com/nextcloud/talk/jobs/AccountRemovalWorkerTest.kt

@@ -0,0 +1,39 @@
+/*
+ * Nextcloud Talk - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.nextcloud.talk.jobs
+
+import io.reactivex.Observable
+import io.reactivex.disposables.Disposables
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertSame
+import org.junit.Test
+
+class AccountRemovalWorkerTest {
+
+    @Test
+    fun aSuccessfulRequestWithANullBodyIsRecognizedAsSuccess() {
+        // The push proxy answers without a body, Retrofit emits null.
+        val request = Observable.unsafeCreate<Void> { observer ->
+            observer.onSubscribe(Disposables.empty())
+            observer.onNext(unsafeNull())
+            observer.onComplete()
+        }
+
+        assertNull(AccountRemovalWorker.awaitCompletion(request))
+    }
+
+    @Test
+    fun aFailedRequestReturnsItsError() {
+        val error = RuntimeException("proxy unreachable")
+
+        assertSame(error, AccountRemovalWorker.awaitCompletion(Observable.error<Void>(error)))
+    }
+
+    // Kotlin refuses a literal null for the non-null RxJava parameter; the erased generic cast lets it through.
+    @Suppress("UNCHECKED_CAST")
+    private fun <T> unsafeNull(): T = null as T
+}

app/src/test/java/com/nextcloud/talk/login/data/LoginRepositoryTest.kt

@@ -389,21 +389,39 @@ class LoginRepositoryTest {
     // ========== parseAndLogin() Tests ==========
 
     @Test
-    fun `parseAndLogin returns null when user is scheduled for deletion`() =
+    fun `parseAndLogin sets the account up again once its pending removal finished`() =
         runTest {
             // Arrange
             val loginData = LoginCompletion(200, "https://server.com", "testuser", "apppass123")
             whenever(localLoginDataSource.checkIfUserIsScheduledForDeletion(loginData))
                 .thenReturn(true)
             whenever(localLoginDataSource.startAccountRemovalWorker())
                 .thenReturn(liveData)
+            whenever(localLoginDataSource.awaitUserRemoval(any(), any())).thenReturn(true)
+
+            whenever(localLoginDataSource.checkIfUserExists(loginData)).thenReturn(false)
 
             // Act
             val result = repo.parseAndLogin(loginData)
 
             // Assert
-            assertEquals(LoginRepository.LoginResult.ExistingAccount, result)
+            assertTrue(result is LoginRepository.LoginResult.NewAccount)
             verify(localLoginDataSource).startAccountRemovalWorker()
+            verify(localLoginDataSource).awaitUserRemoval(any(), any())
+        }
+
+    @Test
+    fun `parseAndLogin reports a pending removal that does not finish in time`() =
+        runTest {
+            val loginData = LoginCompletion(200, "https://server.com", "testuser", "apppass123")
+            whenever(localLoginDataSource.checkIfUserIsScheduledForDeletion(loginData)).thenReturn(true)
+            whenever(localLoginDataSource.startAccountRemovalWorker()).thenReturn(liveData)
+            whenever(localLoginDataSource.awaitUserRemoval(any(), any())).thenReturn(false)
+
+            val result = repo.parseAndLogin(loginData)
+
+            assertEquals(LoginRepository.LoginResult.RemovalPending, result)
+            verify(localLoginDataSource, never()).checkIfUserExists(any())
         }
 
     @Test
@@ -530,6 +548,8 @@ class LoginRepositoryTest {
                 .thenReturn(true)
             whenever(localLoginDataSource.startAccountRemovalWorker())
                 .thenReturn(liveData)
+            whenever(localLoginDataSource.awaitUserRemoval(any(), any())).thenReturn(true)
+            whenever(localLoginDataSource.checkIfUserExists(loginData)).thenReturn(false)
 
             // Act
             repo.parseAndLogin(loginData)