Skip to content

fix(account): don't delete an account when the server refuses API calls temporarily - #6728

Draft
AndyScherzinger wants to merge 2 commits into
masterfrom
bugfix/noid/no-account-deletion-on-transient-401
Draft

AndyScherzinger wants to merge 2 commits into
masterfrom
bugfix/noid/no-account-deletion-on-transient-401

Conversation

@AndyScherzinger

@AndyScherzinger AndyScherzinger commented Sep 19, 2026

Copy link
Copy Markdown
Member

🖼️ Screenshots

The login error text changes, so a screenshot of the new message on the server selection screen is still owed.

🏚️ Before 🏡 After
"Talk not available (not installed or restricted by admin)" "Could not reach the server. It might be temporarily unavailable or blocking requests. Please try again later."

A server that temporarily refuses API calls — brute force protection kicking in is the case that was reported, but a timeout or a short outage behaves the same — was read as "this server has no Talk", and in some flows that cost the user their account locally. Two independent paths did that.

The remote wipe interceptor removed the account on every HTTP 401, including when /core/wipe/check answered that no wipe was requested and when the check itself could not be performed. A 401 on its own is not a wipe: an expired session, a throttling server, or a proxy in front of Nextcloud all produce one while the account is perfectly valid, and removing it takes the local message cache with it. It now wipes only on a confirmed wipe request and passes every other 401 through to the caller, which already offers re-authentication. This also covers 401s seen by background workers — NotificationWorker builds its API client from the shared OkHttp client, so a single unlucky 401 while handling a push could delete the account with no user present.

During account verification, any failure of the capabilities request was reported as "Talk not available" and recorded as SERVER_WITHOUT_TALK. A server without Talk answers that endpoint normally, just without a spreed entry, and that case is already handled in onNext — so an error there never means Talk is missing. It is now reported as a temporary failure. In the same flow, a failed capabilities refresh aborted verification, which deletes the account that was stored moments earlier; since the capabilities were already stored together with the profile, the login now continues and only reports the failed refresh, the same way a failed push registration already does.

🚧 TODO

  • Screenshot of the new error message on the server selection screen

🏁 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

@AndyScherzinger AndyScherzinger added this to the 25.1.0 milestone Sep 19, 2026
@AndyScherzinger

AndyScherzinger commented Sep 19, 2026

Copy link
Copy Markdown
Member Author

/backport to stable-25.0.x

@AndyScherzinger
AndyScherzinger force-pushed the bugfix/noid/no-account-deletion-on-transient-401 branch from 75cfe91 to 0c81760 Compare September 20, 2026 06:15
The remote wipe interceptor removed the account on every HTTP 401,
including when /core/wipe/check answered that no wipe was requested and
when the check could not be performed at all. A 401 on its own is not a
wipe: an expired session, a server that rate limits or throttles the
client, or a proxy in front of Nextcloud all produce one while the
account is perfectly valid, and losing it takes the local message cache
with it.

Wipe only on a confirmed wipe request and pass every other 401 through
to the caller, which already offers re-authentication. The de-duplication
entry is released again when no wipe was requested so a later genuine
wipe is still honoured.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A server that cannot be asked for its capabilities - because it rate
limits the client through brute force protection, is briefly
unavailable, or the request times out - was reported to the user as
"Talk not available (not installed or restricted by admin)" and recorded
as SERVER_WITHOUT_TALK. A server without Talk answers the capabilities
endpoint normally, just without a spreed entry, and that case is already
handled in onNext, so an error there never means Talk is missing.

Report such a failure as a temporary one instead, and let a failed
capabilities refresh during verification continue the login rather than
abort it: the capabilities were already stored together with the
profile, so aborting only served to delete an account that is fine.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
AndyScherzinger force-pushed the bugfix/noid/no-account-deletion-on-transient-401 branch from 0c81760 to 1406c03 Compare September 20, 2026 12:23
@mahibi

mahibi commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

@AndyScherzinger regarding
"stop calling abortVerification() when the capabilities-refresh step fails during verification, since capabilities were already stored with the profile."
i also address this in
#6736
Just to avoid you put further effort in the AccountVerificationActivity here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants