Nothing pins the login() disabled guard, or SetUserDisabled at all #156

closed cmc opened this on 2026-09-05 04:10 UTC · security · milestone v1.14.0

Discussion

cmc 2026-09-05 04:10 UTC

login() re-reads the user after ConsumeLoginToken and refuses a disabled account (#155). That guard is the load-bearing half of the suspension fix, and no test pins it alone.

SetUserDisabled also deletes login_tokens, so the two e2e cases added in !261 still pass with the guard removed — the store-side delete masks it. Mutation testing confirmed this: dropping either layer alone keeps the tests green, dropping both fails.

The two are not equivalent. SetUserDisabled is not transactional (internal/store/users.go:174-193): it runs UPDATE users, then separate DELETE statements. With the guard gone, a request in flight between the UPDATE and DELETE FROM login_tokens consumes a still-present token, and its CreateWebSession insert can land after DELETE FROM web_sessions — a live session on a suspended account. The guard closes that window because it re-reads users after the consume. The delete is defense in depth and hygiene.

SetUserDisabled has no unit test at all; it is reached only transitively through the admin e2e cases.

Fix: a unit test in internal/httpd that inserts a login token through the store, sets disabled directly, and calls login(). Bypassing SetUserDisabled is the point — it pins the guard alone, at unit speed.

closed by commit e3cd3c4236 by cmc: httpd: pin the login() disabled guard and the checkOrigin invariant

2026-09-05 18:22 UTC