Skip to content

fix(auth): confirm a missed session on the write host before 401ing (9893 root cause)#1055

Merged
edwh merged 5 commits into
masterfrom
fix/authmw-session-read-after-write-9893
Jul 14, 2026
Merged

fix(auth): confirm a missed session on the write host before 401ing (9893 root cause)#1055
edwh merged 5 commits into
masterfrom
fix/authmw-session-read-after-write-9893

Conversation

@edwh

@edwh edwh commented Jul 12, 2026

Copy link
Copy Markdown
Member

Follow-up to #1048 (merged). #1048 fixed the symptom — a stray 401 no longer wipes the whole session and forces a relogin. This fixes the root cause of the stray 401, which the adversarial review correctly flagged was left unaddressed.

Root cause

iznik-server-go/user/authMiddleware.go runs a per-request session-consistency check as a plain, replica-routed SELECT ... FROM sessions JOIN users. The session row is INSERTed on the write host at login (auth.CreateSessionAndJWT), so a Galera read replica can momentarily return zero rows right after login (apply lag). The middleware treats zero rows as an invalid session and overrides the response with a 401.

Right after login the ModTools client fires a burst of authenticated requests (chat poll, notification/work counts, …); any one landing in that replication window gets spuriously 401'd. On /session that still forces a logout even after #1048 — and each forced relogin mints a fresh session row, re-opening the window (the self-reinforcing loop heavy mods hit "several times a day").

Fix

On a zero-row miss, re-confirm the lookup against the write host before deciding the session is invalid:

if dbQueryErr == nil && userIdInDB.Id == 0 {
    result = db.Clauses(dbresolver.Write).Raw(q, sessionIdInJWT, userIdInJWT).Scan(&userIdInDB)
    dbQueryErr = result.Error
}

This is the same read-after-write pinning already used in authority/stats.go and ~10 message/chat sites. .Clauses(dbresolver.Write) is a documented no-op when no replica is configured. The happy path (session found on the replica) is unchanged and adds no extra query — only a genuine miss pays for the confirming read.

Testing

The race needs a lagging read replica, which the single-host test DB can't reproduce, so there's no new unit test; existing auth tests still cover the happy path (a valid session authenticates, and my change is a no-op there).

🤖 Generated with Claude Code

…ing (9893 root cause)

Follow-up to #1048, which fixed the symptom (a stray 401 no longer wipes the
whole session). This addresses the root cause of the stray 401 itself.

The auth middleware's per-request session-consistency check runs a plain,
replica-routed SELECT. The session row is INSERTed on the write host at login
(auth.CreateSessionAndJWT), so a read replica can momentarily return zero rows
right after login (Galera apply lag) - which the middleware treats as an invalid
session and overrides the response with a 401. Right after login a ModTools
client fires a burst of authenticated requests, so one landing in that window
gets spuriously 401'd; on /session that still forces a logout even after #1048.

On a zero-row miss, re-confirm against the write host (.Clauses(dbresolver.Write),
a no-op when no replica is configured) before deciding the session is invalid -
the same read-after-write pinning already used in authority/stats.go and ~10
message/chat sites. The happy path (session found on the replica) is unchanged
and adds no extra query.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VXKgf7Xquudc6CwP2RN6jX
@netlify

netlify Bot commented Jul 12, 2026

Copy link
Copy Markdown

Deploy Preview for golden-caramel-d2c3a7 ready!

Name Link
🔨 Latest commit 1d169e7
🔍 Latest deploy log https://app.netlify.com/projects/golden-caramel-d2c3a7/deploys/6a554cd1dec3840007b28a7d
😎 Deploy Preview https://deploy-preview-1055--golden-caramel-d2c3a7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@FreegleGeeks FreegleGeeks added the CI: Passed CI build passed label Jul 12, 2026
Resolve the only conflict (docs/members/assets/browse.png) by keeping master's
version - a generated screenshot unrelated to this auth fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VXKgf7Xquudc6CwP2RN6jX
@FreegleGeeks FreegleGeeks added CI: Running CI build is currently running and removed CI: Passed CI build passed labels Jul 12, 2026
@FreegleGeeks FreegleGeeks added CI: Passed CI build passed and removed CI: Running CI build is currently running labels Jul 13, 2026
# Conflicts:
#	docs/members/assets/browse.png
@FreegleGeeks FreegleGeeks added CI: Running CI build is currently running CI: Passed CI build passed and removed CI: Passed CI build passed CI: Running CI build is currently running labels Jul 13, 2026
@edwh edwh merged commit ab9b5cb into master Jul 14, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: Passed CI build passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants