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.

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

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: cf1b8d78-023e-4c7f-beb6-eea222e0e233

📥 Commits

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

📒 Files selected for processing (2)

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

🚧 Files skipped from review as they are similar to previous changes (1)

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

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

📝 Walkthrough

Walkthrough

The account-removal worker now handles push-unregistration failures by logging the outcome and initiating user deletion. During login, the repository waits for scheduled account removal before checking whether the account exists. If removal is still pending after the wait, the login flow maps that result to a view state and displays a snackbar. Tests cover removal polling, login outcomes, and push-proxy completion.

Priority: ➖ Normal

Merge Risk | ⚪ Minimal · up to ee0d1

Merge Risk: ⚪ Minimal · up to ee0d1

The change makes login wait for a pending account removal to finish, and the supplied context shows no concrete merge-blocking risk.

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		Docstring coverage is 29.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: waiting for pending account removal before logging in again.
Description check		The description is complete and covers the symptom, cause, implementation, testing steps, related issues, screenshots, TODO status, and checklist. The backport and milestone items remain unchecked, bu…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

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).

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>

…PDX headers The CI checks detectNewJavaFiles and reuse-compliance-check failed on the pull request: new files must be written in Kotlin, and the two new test files had no SPDX-FileCopyrightText line. Rewrite AccountRemovalWorkerTest in Kotlin with the same two cases, remove the Java file and add the copyright line to both new test files. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>

github-actions · 2026-10-19T06:22:46Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

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

AndyScherzinger removed the 2. developing Work in progress label Oct 10, 2026

ToteMeiSter added 3 commits October 18, 2026 12:40

ToteMeiSter force-pushed the fix/account-removal-before-login branch from ee0d1a6 to 75c7d2c Compare October 18, 2026 12:42

github-actions Bot added the feedback-requested label Oct 19, 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 3 commits into
nextcloud:masterfrom
ToteMeiSter:fix/account-removal-before-login

ToteMeiSter commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

mahibi commented Oct 5, 2026

Uh oh!

ToteMeiSter commented Oct 6, 2026

Uh oh!

github-actions Bot commented Oct 19, 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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

mahibi commented Oct 5, 2026

Uh oh!

ToteMeiSter commented Oct 6, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading