Skip to content

fix(ui): stop reading a credential 401 as a session expiry - #245

Merged
argszero merged 1 commit into
mainfrom
fix/credential-401-is-not-session-expired
Sep 14, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/credential-401-is-not-session-expired

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

A 401 from POST /api/auth/login means "the credentials you just sent are wrong", but ui/js/api.js read every 401 as "your session expired": it cleared the token, returned to the login page and toasted login.session.expired. Typing a wrong password therefore produced that toast on top of the inline "Incorrect email or password" — telling somebody who had never signed in that their session had expired.

The two meanings are indistinguishable in the request throat, so the caller now declares which one applies:

api.post("/api/auth/login", { email, password: pass }, { on401: api.CREDENTIAL_401 })

Anything else — including an unknown value — keeps the session-expiry reading, so a forgotten declaration fails towards "sign the user out once too often", never towards "leave a dead session standing". With the flag set the 401 falls through to the ordinary error construction and keeps status === 401, so the login form's inline error branch still fires.

POST /api/auth/change-password has the same 401 semantics (wrong old password) and the same shape would clear a still-valid token and throw the user out of the app. It has no front-end consumer today, so only the rule is recorded — no call site is invented.

Folded in (same axis): the login flow stops hand-rolling its bring-in. Once the credentials are accepted it reuses the boot entry point restoreSession(), so a token holder can no longer be stranded on the login page when the session load fails for a non-auth reason — the invariant from #244, reached through a different door (before: saveToken() followed by a failing loadSession() left a saved token sitting behind a login form, visually identical to "you were logged out").

Not a host rant: found by this task's own recon (the previous round's instrument disproved the original write-up of this defect — the inline error is reachable, the spurious toast is the real fault).

Related Issue

None — no open issue covers this.

Changes

  • ui/js/api.js — add CREDENTIAL_401 and the opts.on401 guard on the 401 branch; the verbs pass opts through
  • ui/js/app.js — the login call declares the endpoint's 401 semantics; bring-in goes through restoreSession()
  • src/i18n_pack.rscredential_401_is_not_a_session_expiry pins the shape in CI (the call site declares it, the throat's 401 branch is guarded by it, and the global sign-out still exists), with detector controls. The control flow itself has no JS runner in CI, so the runtime half is documented in ui/README.md
  • ui/README.md — record the rule and the smoke-test shape
  • ui/index.html — cache-bust api.js / app.js
  • No config / schema / data-structure change, so no example file to sync

Tests

  • cargo test — 258 passed, 0 failed (257 before; +1 for the new gate)
  • cargo fmt --check — clean
  • cargo clippy --all-targets — only the pre-existing collapsible_match suggestion at src/protocol.rs:662 (unrelated; CI runs stable clippy and is green)
  • New unit test added

Runtime verification (jsdom, not in CI)

Instrument boots the real ui/index.html + the four real scripts, stubs only fetch (reproducing the backend's real answers), and drives the real login form. Four legs: login401 / login500 / boot401 / loginok.

Variant Red assertions
origin/main (pre-fix) login401: B1, login500: E1,E3
fixed tree — (green)
guard removed from api.js B1
declaration removed from the call site B1
bring-in reverted to loadSession()+enterApp() E1,E3
global sign-out deleted (the tempting half-fix) C1,C3 — disjoint from the axis
inline credential error dropped B3

Where B1 = "no login.session.expired toast after a wrong password", B3 = "the inline login.err.bad is shown instead" (positive control — already green before the fix), C1/C3 = "a business 401 still clears the token and says session-expired", E1/E3 = "credentials accepted but /api/me failing still enters the app". The pre-fix red set equals the union of the first three mutation legs. The C leg is what makes the instrument pin the direction rather than merely "a change": deleting the global sign-out turns the axis green while silently disabling sign-out everywhere else.

Checklist

  • Branch name follows the convention (fix/…)
  • Commit message uses Conventional Commits (fix(ui): …)
  • Single responsibility, minimal diff (5 files, +149/−13; 99 of those lines are the new gate and its comments)

`ui/js/api.js` answered *every* 401 with the global sign-out (clear the token, bounce to the login
page, `toast(login.session.expired)`), but `POST /api/auth/login` deliberately answers 401 for an
unknown email or a wrong password (`src/routes/mod.rs::login`). Typing a wrong password therefore
drew "Session expired, please sign in again" **on top of** the inline "Incorrect email or
password" — telling somebody who had never signed in that their session had expired. The same
shape is worse for `POST /api/auth/change-password`, whose 401 means "wrong old password": it
would clear a still-valid token and throw the user out of the app (that endpoint has no front-end
consumer today, so only the rule is recorded).

The two meanings of 401 are indistinguishable in the throat, so the caller now declares which one
applies:

    api.post("/api/auth/login", { email, password: pass }, { on401: api.CREDENTIAL_401 })

Anything else — including an unknown value — keeps the session-expiry reading, so a forgotten
declaration fails towards "sign the user out once too often", never towards "leave a dead session
standing". With the flag set, the 401 falls through to the ordinary error construction, keeping
`status === 401` so the login form's inline error branch still fires.

The login flow also stops hand-rolling its bring-in: once the credentials are accepted it reuses
the boot entry point `restoreSession()`, so a token holder can no longer be stranded on the login
page when the session load fails for a non-auth reason — the same invariant as #244, reached
through a different door (before: `saveToken()` then a failing `loadSession()` left a saved token
behind a login form).

`src/i18n_pack.rs::credential_401_is_not_a_session_expiry` pins the shape in CI: the login call
site declares the semantics, the throat's 401 branch is guarded by that declaration, and the
global sign-out still exists (deleting it would make the login page quiet while silently disabling
sign-out for every business endpoint). The control flow itself has no JS runner in CI, so the
runtime half is carried by `ui/README.md` and its smoke-test notes.

Verified with a jsdom instrument over the real `ui/index.html` + four real scripts (only `fetch`
stubbed) driving the real login form, four legs: login401 / login500 / boot401 / loginok.
Pre-fix tree (pinned to `origin/main`): red {login401: B1, login500: E1,E3}. Fixed tree: green.
Mutation legs, each turning exactly its own assertions red: guard removed -> {B1}; call-site
declaration removed -> {B1}; bring-in reverted -> {E1,E3}; global sign-out deleted -> {C1,C3}
(disjoint from the axis, which is the point — it pins the direction, not just "a change");
inline credential error dropped -> {B3}. The pre-fix red set equals the union of the first three.

- ui/js/api.js    : `CREDENTIAL_401` + `opts.on401` guard; verbs take opts
- ui/js/app.js    : declare the login endpoint's 401 semantics; bring-in via `restoreSession()`
- src/i18n_pack.rs: CI tripwire for the shape, with detector controls
- ui/README.md    : record the rule and the smoke-test shape
- ui/index.html   : cache-bust api.js / app.js

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
@argszero
argszero merged commit 301ca4d into main Sep 14, 2026
1 check passed
@argszero
argszero deleted the fix/credential-401-is-not-session-expired branch September 14, 2026 16:06
@argszero argszero mentioned this pull request Sep 15, 2026
10 tasks
argszero added a commit that referenced this pull request Sep 15, 2026
Ships the 18 PRs merged since v0.7.24 (#242-#259). Schema 14 -> 15 (two
covering indexes, applied at startup). No config change, so no deployment-side
config.toml edit is needed.

Two themes:

Perf on the NFS dev database
- #259: stop mapping the db (PRAGMA mmap_size 64MB -> 0) and stop a real write
  per request (dao::touch_api_key gains a 60s guard). Measured on the live dev
  db: mmap=64MB 1.7-3.1s per COUNT / 250 MiB read vs mmap=0 ~10.5ms / 80 KiB;
  mmap=0 alone still leaves ~1.2s behind any write, so the pair is required.
- #242: codify the two emergency indexes in a v15 migration and gate the
  conditional joins at the plan level.
- #243: read the sharing page's earn total from one batched aggregate.

Frontend: display must equal what it filters on, and one fact, one source
- #250 one writer for the transaction cache; #251 clear every session slot at
  the identity boundary and give the wallet view a loader; #253 one shared
  writer for the wallet/dashboard month-changes; #254 boot loads only the
  destination view; #255 a model row's identity is the model, not its index;
  #256 the marketplace source follows the session, not whether data arrived;
  #257 the sidebar advertises only digits that work; #258 the admin
  total-balance card sums the gift amount its caption names.

i18n
- #249 every backend error reaches the wordlist, and the comment stripper stops
  mangling UTF-8; #252 the backend stops inventing Chinese display labels in
  response data fields.

Forms and robustness
- #244 a non-auth boot failure no longer looks like being logged out; #245 a
  credential 401 is no longer read as a session expiry; #246 wire timestamps
  reach the renderer unsliced; #247 inline cards submit from every field; #248
  a market row's availability label comes from that row.

- Cargo.toml / Cargo.lock: 0.7.24 -> 0.7.25.
- CHANGELOG.md: v0.7.25 entry.
- ui/index.html: cache-bust left as-is; the UI PRs in this release already
  advanced it past the value deployed with v0.7.24 (app.js 20260915-13,
  i18n.js 20260915-3).

cargo test 288 passed; cargo fmt --check clean; clippy unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant