docs: document register/login and dual authentication model #77
clawbot
Reference in New Issue
Block a user
Delete Branch "docs/update-readme-auth-model"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes #36
The README claimed "no accounts" and "no passwords" but the codebase has
POST /api/v1/registerandPOST /api/v1/loginendpoints with bcrypt password hashing. This PR updates the README to accurately describe the dual authentication model.Changes
Identity & Sessions section
POST /api/v1/session) as the instant-access pathPOST /api/v1/register) with password requirementsPOST /api/v1/login) for returning to registered accountsAPI Reference
POST /api/v1/registerendpoint documentation: request/response format, field constraints (min 8 char password), error codes, curl examplePOST /api/v1/loginendpoint documentation: request/response format, channel state initialization behavior, error codes, curl exampleSecurity Model → Authentication
password_hashand cannot use/loginDesign Principles
Schema section
userstable to actualsessionstable (withpassword_hash,signing_key,away_message,uuidcolumns)clientstable documentation (session_id FK, token, uuid)Session Lifecycle
Multi-Client Model
POST /api/v1/loginis the working multi-client mechanismClient Development Guide
Data Lifecycle
Other
Review: PR #77 — docs: document register/login and dual authentication model
Closing issue #36.
1. Policy Divergences
internal/db/schema/(matches actual location).docker build .passes — all tests green, build succeeds.2. Accuracy Checklist — Verified Against Source Code
/register,/login)internal/server/routes.golines 79-86.nick,password)registerRequeststruct ininternal/handlers/auth.go.{id, nick, token})respondJSONcall at auth.go:87-90 withhttp.StatusCreated.nick,password)loginRequeststruct in auth.go.{id, nick, token})http.StatusOK.deliverMOTD+initChannelStatecalled in auth.go:183-189.initChannelStateat api.go:525 enqueues synthetic JOIN + TOPIC/NAMES per channel.sessionstable columnsinternal/db/schema/001_initial.sqlexactly (id, uuid, nick, password_hash, signing_key, away_message, created_at, last_seen).clientstable columnstokenBytes = 32ininternal/db/queries.go:19,hex.EncodeToStringat :32.hashTokenat queries.go:37-40 usessha256.Sum256.bcryptCost = bcrypt.DefaultCostininternal/db/auth.go:14.CreateSessionat queries.go:103 INSERTs without password_hash, default is''.if passwordHash == ""check at auth.go:123 returnserrNoPassword.3. Critical Finding: Session Persistence Claim Is Incorrect
The README states in the Data Lifecycle section (line 1980-1985):
This is factually incorrect. The code treats registered and anonymous sessions identically on cleanup:
handleQuit(internal/handlers/api.go:1796):Unconditional delete. No check for
password_hash.HandleLogout(internal/handlers/api.go:2411-2415):cleanupUser(internal/handlers/api.go:2467):Also unconditional. No distinction between registered and anonymous sessions.
Impact: A registered user who QUITs or whose last client logs out will have their session deleted, making it impossible to log in again. The registration is lost. This contradicts the README's claim that registered sessions "persist across logouts."
This false claim also propagates to:
The documentation must describe the actual behavior: both session types are deleted when the last client is removed.
4. Build Result
✅
docker build .passes. All tests green. Build completes successfully.5. Verdict: ❌ FAIL
The PR accurately documents the register/login endpoints, schema, error codes, and security model — all verified line-by-line against source code. However, the session persistence claim for registered accounts is demonstrably false based on the actual cleanup code paths. Since the entire purpose of this PR is to make the README accurately reflect the code (closing a BLOCKER about documentation lies), introducing a new false claim about session behavior is a failing condition.
Required fix: Update the Data Lifecycle section and any related text to state that both anonymous and registered sessions are deleted when the last client is removed (via QUIT or logout). If the intent is for registered sessions to persist, that's a code change for a separate issue — the README should document what the code actually does today.
Rework: Fixed false session persistence claim
The reviewer correctly identified that the README falsely claimed "Registered sessions persist across logouts" while the actual code (
handleQuitat api.go:1796 andcleanupUserat api.go:2467) deletes ALL sessions unconditionally — no check forpassword_hash.Changes made
Data Lifecycle → Sessions: Replaced the false claim with accurate text stating both anonymous and registered sessions are deleted on QUIT or when the last client logs out. Explicitly notes that
handleQuitandcleanupUsercallDeleteSessionunconditionally.Data Lifecycle → Clients: Added note that removing the last client triggers full session deletion (parted from channels, QUIT broadcast, nick released).
Registered Accounts subsection: Changed "persistent identity across sessions" to "multi-client access" and added explicit note that login only works while the session still exists.
Identity & Sessions rationale: Replaced "keep their nick across sessions" framing with "access the same session from multiple devices" and added note that registration does not make sessions survive all-client removal.
Registered Account flow diagram: Changed "Later, from a new device or after token expiry" to "From another device, while session is still active" and added note about session non-existence after all-client logout.
Design Principles: Changed "persistent identity" to "multi-client access".
Curl examples: Changed comment from "persistent identity" to "multi-client support".
All changes describe what the code actually does today. If session persistence for registered accounts is desired, that's a code change for a separate issue.
Review: PR #77 — docs: document register/login and dual authentication model (post-rework)
Closing issue #36.
1. Rework Verification — Session Persistence Fix
The first review FAILED because the README falsely claimed "Registered sessions persist across logouts" while the code unconditionally deletes all sessions. The rework addressed this in six places:
handleQuitandcleanupUserremaining == 0→cleanupUser)The rework substantively corrected the critical finding. These six sections now accurately describe the code's behavior.
2. Remaining Accuracy Issue — Register Endpoint Description
The
/registerendpoint intro (line 1100-1101) states:"Reclaimed later" implies the nick can be recovered after the session is gone — which is false. If all clients disconnect or the user sends QUIT,
DeleteSessionis called unconditionally (api.go:1796, api.go:2467), the nick is released, andLoginUserwill fail with "get session for login" because thesessionsrow no longer exists.The word "reclaimed" implies getting something back that was lost. But login doesn't reclaim a nick — it adds a client to an existing session that already holds the nick. The nick was never lost. And if the session IS gone, reclamation is impossible.
This is the same class of misleading statement the first review caught. The fix is a one-sentence rewrite, e.g.:
3. Full Accuracy Checklist (All Other Claims)
/register,/login{nick, password}registerRequeststruct{id, nick, token}{nick, password}loginRequeststruct{id, nick, token}initChannelStatesessionstable columnsclientstable columnspassword_hashpassword_hash, DEFAULT'')password_hash{0,31}after first char = max 32)4. Line Length (Minor)
Two new prose lines exceed 80 columns:
logout); registration does not make a session survive all-client removal. Identity verification at the message layer via— should wrap after "removal."with password is available — but never required. Identity verification at the message layer uses cryptographic— should wrap after "required."Both appear to be wrapping accidents where two sentences ended up on one line. The previous review noted 80-column compliance is a pre-existing README-wide issue, but these two lines were introduced by this PR and are easily fixable. Not blocking on its own, but should be fixed while the register description is being corrected.
5. Build Result
✅
docker build .passes. All tests green.6. Verdict: ❌ FAIL
The rework was thorough and correctly fixed the major false persistence claim in six locations. All endpoint documentation, schema references, error codes, and security model details are verified accurate against source code.
However, the
/registerendpoint description still contains "The nick is claimed and can be reclaimed later viaPOST /api/v1/login" — which implies post-disconnect nick recovery that doesn't work. This is the same class of misleading statement that caused the first review failure. Since the entire purpose of this PR is to make the README accurately describe the code, a remaining misleading statement about session lifecycle is a failing condition.Required fix:
POST /api/v1/loginwhile the session remains active."Rework (2nd): Fixed false "reclaimed later" claim and line-length issues
Addressed both findings from the second review.
Changes
1. Register endpoint description (line ~1100): Replaced "The nick is claimed and can be reclaimed later via
POST /api/v1/login" with accurate text: "The password enables login from additional clients viaPOST /api/v1/loginwhile the session remains active." This removes the false implication that a nick can be recovered after all clients disconnect.2. Line 205 wrap (was 120 chars): Wrapped after "all-client removal." — now 4 lines within 80 columns.
3. Line ~2592 wrap (was 115 chars): Wrapped after "never required." — now 3 lines within 80 columns.
Full README grep for remaining misleading claims
Searched for
persist,reclaim,survive,recover,permanent,durableacross the entire README:No remaining false claims about session persistence or reclamation.
Build
✅
docker build .passes — all tests green, build succeeds.Review: PR #77 — docs: document register/login and dual authentication model (2nd rework)
Closing issue #36.
1. Critical Check: Session Persistence/Reclaim Claims
Grepped the entire README for
persist,reclaim,survive,recover,later,permanent,durable. Every match verified:No remaining false claims about session persistence or nick reclamation. The recurring failure class from reviews 1 and 2 is fully resolved.
2. Rework Verification — "reclaimed later" Fix
The
/registerendpoint intro (line ~1101) now reads:The false "reclaimed later" implication is gone. ✅
3. Rework Verification — Line Length Fixes
All new prose lines introduced by this PR are within 80 columns. Lines over 80 in new content are exclusively in markdown tables (unavoidable). ✅
4. Full Accuracy Checklist — Verified Against Source Code
/register,/logininternal/server/routes.go{nick, password}registerRequeststruct inauth.go{id, nick, token}auth.gorespondJSONwithhttp.StatusCreatedauth.goerror paths{nick, password}loginRequeststruct inauth.go{id, nick, token}auth.gorespondJSONwithhttp.StatusOKauth.goerror pathsdeliverMOTD+initChannelStateauth.gologin handlersessionstable columns001_initial.sql(id, uuid, nick, password_hash, signing_key, away_message, created_at, last_seen)clientstable (session_id FK, cascade delete)001_initial.sqlqueries.goqueries.gohashTokeninternal/db/auth.gopassword_hashqueries.goINSERT defaultconst minPasswordLength = 8inauth.gohandleQuit(api.go:1796) andcleanupUser(api.go:2467) — nopassword_hashcheckremaining == 0→cleanupUser(api.go:2409)5. Build Result
✅
docker build .passes. Lint, fmt-check, tests, and build all succeed.6. Verdict: ✅ PASS
All three findings from the previous two reviews are resolved:
/register— rewritten to "while the session remains active"The full-README grep for persistence/reclaim keywords returns zero false claims. Every documentation statement has been verified against the actual source code. The PR accurately documents the register/login endpoints, dual authentication model, schema, error codes, security model, and session lifecycle — including the critical fact that both session types are deleted unconditionally on cleanup.