fix(login): wait for the pending account removal before logging in again by ToteMeiSter · Pull Request #6810 · nextcloud/talk-android

ToteMeiSter · 2026-10-04T07:49:02Z

Symptom

Remove an account and log in again with the same account right away, without restarting the app. After the browser login, Talk returns to the server address screen. There is no error message. After an app restart, the login works.

Cause

AccountRemovalWorker sent the push proxy unregister request with .subscribe(...). The Retrofit adapter runs on Schedulers.io(), so the call is asynchronous. doWork() returned Result.success() before the user row was deleted.
The UI restarts the app on SUCCEEDED. A new login with the same account then reaches LoginRepository.parseAndLogin. The user is still scheduledForDeletion, so it returns ExistingAccount. The app restarts with no accounts and shows the server address screen.
When the Nextcloud unregister request answered with a status other than 200/202, the user was not deleted at all.
On the next app start the worker finishes the removal, so a restart "fixes" it.

Change

AccountRemovalWorker waits for the push proxy request with ignoreElements().blockingGet(). blockingSubscribe cannot be used: the response body of Observable<Void> is null, and the blocking observer of RxJava 2 throws an NPE on it.
The notification channel group is deleted when the request succeeds. The user is deleted in every case, also after a status other than 200/202.
parseAndLogin: for an account that is still pending removal, it starts the worker again and waits up to 30 s for the removal (LocalLoginDataSource.awaitUserRemoval). After the removal, the account is set up as new. On timeout, the new LoginResult.RemovalPending shows a Snackbar (nc_account_removal_pending) instead of the silent return to the server address screen.

How to test

Log in with an account.
Settings → Remove account.
Right after the restart, log in with the same account.
Before: the server address screen is shown again. After: the conversation list opens, or a Snackbar asks to try again if the removal is not finished.

Unit tests: AccountRemovalWorkerTest (a successful request with a null body is success; a failed request returns its error), LocalLoginDataSourceTest, LoginRepositoryTest.

Tested by the reporter on a Huawei DEL-LX9.

🖼️ Screenshots

Not applicable.

🚧 TODO

None

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not needed
🔖 Capability is checked or not needed
🔙 Backport requests are created or not needed: /backport to stable-xx.x
📅 Milestone is set
🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

The content of this PR was partly or fully generated using AI

This change was prepared with Claude Code (claude-opus-5-5). The author reviewed the code and tested it on a device. The commits carry the Assisted-by: Claude-Code:claude-opus-5-5 trailer.

After "Remove account" the app restarted as soon as AccountRemovalWorker was SUCCEEDED, but the worker only started the push proxy request (async) and returned. The user row was still scheduled for deletion, so logging in with the same account ended in ExistingAccount and an app restart to the server address screen, without any message. The worker now blocks on the proxy request, and also deletes the user when the Nextcloud unregistration answers with another status than 200/202. When a login finds an account still scheduled for deletion, LoginRepository starts the removal and waits for it (30 s), then sets the account up as a new one. If the removal does not finish in time, an error is shown instead of silently restarting. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>

…nt removal The proxy answer has no body, so Retrofit emits null. A blocking subscriber queues the element and fails with a NullPointerException on it, which ended in onError for a successful request and skipped deleting the notification channel group. Wait for the request with ignoreElements() instead, which does not take the element, and delete the channel group when there is no error. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>

coderabbitai · 2026-10-04T07:57:14Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 84d7d393-94d3-46ee-ba6c-87fdad833d90

📥 Commits

Reviewing files that changed from the base of the PR and between 1e9c2f1 and 2ea65fc.

📒 Files selected for processing (9)

app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt
app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt
app/src/main/java/com/nextcloud/talk/account/data/io/LocalLoginDataSource.kt
app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt
app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java
app/src/main/res/values/strings.xml
app/src/test/java/com/nextcloud/talk/account/data/io/LocalLoginDataSourceTest.kt
app/src/test/java/com/nextcloud/talk/jobs/AccountRemovalWorkerTest.java
app/src/test/java/com/nextcloud/talk/login/data/LoginRepositoryTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The account-removal worker now handles unsuccessful push-unregistration outcomes and initiates user deletion. During login, the repository waits for scheduled account removal before checking for an existing account. If removal remains pending, the login flow displays a snackbar asking the user to retry shortly. Tests cover removal polling, repository outcomes, and push-proxy completion.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 2ea65

Login now waits for a pending account removal to finish, or shows a retry message if it takes too long. Nothing in the supplied evidence shows a concrete problem that would block merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 2ea65

The change improves removal ordering, but immediate re-login can still overlap another removal attempt, allowing delayed cleanup to affect the replacement account. Existing login verification remains in place; no account takeover is demonstrated.

Retained concerns

Medium · architecture · inferred: The new login gate can admit a replacement account while another removal attempt still owns a stale account snapshot. Requests are independently enqueued, and the wait ends when the scheduled row disappears. Under overlapping execution, a delayed attempt can subsequently remove the notification group identified by the same server account identity; push or cookie effects additionally depend on identifier or credential reuse. Independent requests predate this PR, but the new in-process continuation admits account recreation without establishing that all old cleanup has finished.

Security review details

Security Blast Radius

observed — The worker processes all locally scheduled accounts, not only the account whose login triggered it. This selection scope is unchanged from the canonical base; the PR does not establish broader service or tenant authority.

Security Findings and Attack Paths

inferred — The supported concern is overlapping lifecycle cleanup, not a verified authentication bypass. It requires a locally scheduled account, overlapping removal attempts, and login continuation after one attempt deletes the row. Remote exploitation, replacement-row deletion, and confidential-data disclosure were not established.

Trust Boundaries and Controls

observed — The changed branch preserves existing account-existence and reauthorization handling. NewAccount still passes through server credential/profile verification before persistence. Canonical base comparison confirms that the reauthorization identity checks are existing controls, not newly introduced by this PR.

Resilience and Maintainability Implications

observed — Single-worker ordering is improved: proxy completion precedes notification cleanup and local deletion. However, the 30-second limit bounds the login wait, not removal execution, and timeout does not cancel independently enqueued cleanup.

Hardening Proposals

proposed — Give removal an explicit ownership/completion fence shared with account recreation, such as coordinated unique work plus account-generation checks, so replacement accounts are admitted only after old cleanup can no longer affect their state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: waiting for pending account removal before allowing the same account to log in again.
Description check	✅ Passed	The description covers the symptom, cause, change, test steps, related issues, and reported testing. It also includes the template sections. The backport and milestone checklist items remain unchecked…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

mahibi · 2026-10-05T08:20:52Z

Hi @ToteMeiSter thank you for your contributions.
We will review them soon

ToteMeiSter · 2026-10-06T12:53:59Z

This is a bug fix and all CodeRabbit review threads are resolved. Once it is approved and merged, would it be possible to backport it to stable-25.0.x (e.g. for 25.0.3)? The bug is present in v25.0.2.

Suggested command for a maintainer after merge: /backport to stable-25.0.x

This comment was drafted with AI assistance (Claude Code).

ToteMeiSter added 2 commits October 4, 2026 07:45

AndyScherzinger added 2. developing Work in progress AI assisted labels Oct 4, 2026

AndyScherzinger added the 3. to review Waiting for reviews label Oct 5, 2026

Navigation Menu

Repository navigation

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

fix(login): wait for the pending account removal before logging in again#6810
ToteMeiSter wants to merge 2 commits into
nextcloud:masterfrom
ToteMeiSter:fix/account-removal-before-login

ToteMeiSter commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

mahibi commented Oct 5, 2026

Uh oh!

ToteMeiSter commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ToteMeiSter commented Oct 4, 2026

Symptom

Cause

Change

How to test

Related

🖼️ Screenshots

🚧 TODO

🏁 Checklist

🤖 AI (if applicable)

Uh oh!

coderabbitai Bot commented Oct 4, 2026

Walkthrough

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

mahibi commented Oct 5, 2026

Uh oh!

ToteMeiSter commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants