Don't wipe local data on a transient 401 during session/request (#137)

[MiMoHo]

Summary

Addresses #137 (app logs itself out after a while and disables autofill).

Root cause

SessionApi.requestSession() threw ClientDeauthorizedException on both 401 and 403:

if (code == 403 || code == 401)
    throw ClientDeauthorizedException()

Unlike a Result.Error — which ApiController.openSession() already handles gracefully by falling back to the cached keychain and keeping the user logged in — this exception propagates up to PasswordsViewModel (_clientDeauthorized), which triggers a full logout: the password and folder databases, the stored credentials and the cached keychain are all cleared and the app restarts to the login screen.

A 401 is not necessarily a permanent deauthorization, and it is ambiguous. Verified against a live Nextcloud + Passwords server, GET /session/request returns:

Situation	HTTP	Body
Valid auth	200	{} / challenge
Wrong / expired / revoked app token	401	{"message":""}
No credentials	401	{"message":"Current user is not logged in"}
Core brute-force limit reached	429	429 page
Passwords-app login-attempt lockout	403	{"message":"Login not allowed"}

A revoked app token returns exactly the same 401 {"message":""} as a wrong password, so a 401 cannot be told apart from a transient failure by status code or body. Meanwhile Nextcloud's own brute-force protection surfaces as 429 (already handled here as a non-destructive API_BAD_RESPONSE), not 401. The destructive 401 therefore comes from a token invalidated server-side, or from a 401/403 injected by a captive portal / reverse proxy / WAF in front of Nextcloud on a flaky connection — which matches the #137 reports (recurs "after some time", on spotty mobile data, while the server had not revoked the session).

Change

requestSession() now classifies the response and only an explicit 403 (the Passwords app's deliberate login-attempt lockout) deauthorizes; a 401 is a recoverable auth error that returns Result.Error(API_AUTH_ERROR), so the existing cached-keychain fallback keeps the user logged in and the local vault intact. The classification is a pure function:

enum class SessionRequestOutcome { SUCCESS, RECOVERABLE_AUTH_ERROR, DEAUTHORIZED, BAD_RESPONSE }
fun classifySessionRequest(code: Int) = when (code) {
    200 -> SUCCESS; 401 -> RECOVERABLE_AUTH_ERROR; 403 -> DEAUTHORIZED; else -> BAD_RESPONSE
}

Files: SessionApi.kt (classifier + use it), Error.kt (API_AUTH_ERROR), ApiController.kt (log the new code alongside the existing ones).

Testing

• Unit tests (SessionRequestClassificationTest): 200→SUCCESS, 401→RECOVERABLE_AUTH_ERROR, 403→DEAUTHORIZED, other→BAD_RESPONSE. Green.
• Live behaviour: created / verified / revoked an app token against a real server and confirmed the status/body table above (revoked token → 401 {"message":""}).
• Emulator (Android 15): happy path unaffected — login, session open and sync work.

Trade-off worth your call

A genuinely revoked app password (also a 401) no longer auto-logs-out; the user keeps the cached, E2E-encrypted vault with a session error and logs out manually. For a password manager I think not destroying the local vault on an ambiguous 401 is the safer default, but an alternative is to retry a few times and only then deauthorize. Happy to adjust.

Note

This contribution was made with AI assistance (Claude). All generated code was proofread and validated by the committer against a live server and the source.

[hegocre]

Hello,

Have you been able to validate the diagnosis made by the AI? Is there any documentation that explains which situations can produce a 401 code? In that case, it is possible that by reading the body the reason can be determined accurately?

I don't like the idea of keeping a cached session when the token has been revoked, so I think we should be able to tell the situations appart.

Once we can do that, I totally agree that a transient 401 error should not erase all the data.

I will also take a look at this when I have more time, to see if I can find any more info.

Thank you!

[MiMoHo]

I validated this against a live Nextcloud + Passwords server and the server source, so I can now answer the "which situations produce a 401 / can the body tell them apart" question precisely.

What /session/request actually returns (checked with curl against a real server and by reading the code):

Situation	HTTP	Body
Valid auth	200	{} / challenge
Wrong / expired / revoked app token	401	{"message":""}
No credentials	401	{"message":"Current user is not logged in"}
Core brute-force limit reached	429	429 page
Passwords-app login-attempt lockout	403	{"message":"Login not allowed"}

The key point for your concern: I created an app token, confirmed it works (200), revoked it, and requested again — a revoked token returns 401 with an empty body, byte-for-byte identical to a wrong password. So the status code and the body cannot distinguish a revoked token from a transient failure; there is no reliable signal to tell them apart at this endpoint.

I also need to correct my own earlier wording: Nextcloud brute-force protection does not return 401 for valid credentials. Below the limit it only adds a delay (valid creds still return 200); at the limit it returns 429 (index.php catches MaxDelayReached). The app already treats 429 as a non-destructive API_BAD_RESPONSE. So the destructive 401 realistically comes from (a) a token invalidated server-side, or (b) a 401/403 injected by something in front of Nextcloud (captive portal, reverse proxy, WAF) on a flaky connection — which fits the reporter's "spotty mobile data" note and "the server did not revoke the session".

Given that a 401 is genuinely ambiguous, not wiping the local (E2E-encrypted, offline) vault on a 401 is the safer default. 403 — the Passwords app's explicit login-attempt lockout — stays a deliberate signal and still deauthorizes. The trade-off stands: a truly revoked token now leaves the encrypted vault on the device until the user logs out manually. For a password manager I think that's the right call, but I'm happy to switch to retry-a-few-times-then-deauthorize if you'd prefer.

I've updated the PR description and the code comment to reflect the accurate 429-vs-401 mechanism.

(Disclosure: this investigation and the code were done with AI assistance and verified by me against a live server and the source.)