From 75d64dd412b6e81a19ae0cb2e09ecfc84d6f644e Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sun, 30 Aug 2026 09:49:05 +0200 Subject: [PATCH] =?UTF-8?q?refactor(b3-2):=20auth=20vertical=20slice=20?= =?UTF-8?q?=E2=80=94=20service.AuthService=20behind=20a=20consumer-owned?= =?UTF-8?q?=20interface=20(S-10)=20+=20HP-3=20draft=20(#1450)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(b3-1): record PR #1449 = 71d867cb in the status line, step table and evidence block Pre-squash SHAs completed with the coverage commit a0356ee1 and the three Codex rounds (head 8614603b). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * refactor(b3-2): auth_deps.go — the consumer-owned AuthService interface Eight methods beside the handlers that need them: Register, Login, VerifyTOTP, Logout, DeleteAccount, EnableTOTP, ConfirmTOTP, DisableTOTP — fewer than the ten *db.DB methods the two handlers call today. The input and result types they name (Principal, RegisterInput, LoginInput, AuthResult, TOTPChangeResult) and the AuthBroadcaster the delete path needs live in service/auth.go. Nothing implements or calls the interface yet. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * refactor(b3-2): service.AuthService — the auth orchestration, moved verbatim Register, Login, VerifyTOTP, Logout, DeleteAccount, EnableTOTP, ConfirmTOTP, DisableTOTP and the RegistrationPolicy gate two characterization rows pin ahead of the body read. The enumeration guard, the F3 reserve-before-compare, the audit writes, the best-effort custom-status clear and the 200+warning partial-success contract move line for line; persistence stays in db behind Store. Each refusal is a named service.Err* whose Error() is the exact public message the handler wrote and whose category (ErrUnauthorized and ErrInvalidInput join the message.go set) the transport maps to a status. The auth rate multiplier moves to auth/ratescale.go so the route mounts and the login failure accounting read one value; api keeps its wrappers. Nothing calls the service yet — the handlers still own their copies. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * refactor(b3-2): thin auth handlers — decode, call AuthService, encode *db.DB leaves every handler signature in auth_handler.go and totp_handler.go; MountAuthRoutes takes the interface and the AuthMiddleware the caller builds, and router.go constructs the service after the hub. Each refusal is encoded by one writeAuthError switch on the service's error categories. The principal helper in middleware.go hands the handlers the caller as service.Principal, and userResponse moves next to the profile handler, so neither auth file names db any more: their two DBImportAllow rows go in this commit (TestDBImportAllowIsLive proves the rows could not outlive the import) and the boundary fixture points at middleware.go instead. The auth-slice limits leave api/constants.go with the code that reads them; profile_handler.go reads the shared pw_confirm budget from the service. Test files change only where they mount the routes (four helper lines + two direct mounts); no assertion or row moves. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * docs(b3-2): after-state boundary inventory — api db importers 12 → 10 Regenerated table (49 files; move 28 → 26), the auth slice's after-state dependency rows, and the honest reading of the plan's "neither db nor service" target: met for db, not for service — the handlers import service for the interface's types and Err* categories. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * docs(b3-2): evidence block — pre-squash SHAs, graph deltas, gates, coverage Characterization green at each SHA in a detached worktree with the frozen files byte-identical to 71d867cb; nine-method interface vs ten db methods; api db importers 12 → 10; slice coverage 392/433 = 90.5% → 392/427 = 91.8%; the five behaviour notes (decode-before-gate corner cases, shared AuthMiddleware, folded confirmation block, moved limits, moved converter). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * docs(hp-3): scorecard draft and the D4 vertical-slice pattern in server.md Five questions answered with commands and outputs at fe1d11b8/3f0d24ec; owner sign-off line left blank. server.md gains D4 — the eight-step interface/service/handler rule for B3-8 with the awkward step (gate-before-decode) named — and its D3 deviation note drops the auth routes. Plans README indexes the scorecard. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo * docs(b3-2): record PR #1450 in the evidence block and the HP-3 fetch line Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo --------- Co-authored-by: Claude Fable 5 --- Server/api/auth_deps.go | 53 ++ Server/api/auth_handler.go | 670 +++----------- .../api/auth_handler_delete_broadcast_test.go | 9 +- Server/api/auth_handler_test.go | 3 +- Server/api/constants.go | 96 +- Server/api/constants_test.go | 22 +- Server/api/coverage_push_test.go | 2 +- Server/api/invite_handler_test.go | 2 +- Server/api/middleware.go | 13 + Server/api/profile_handler.go | 46 +- Server/api/router.go | 11 +- .../router_delete_account_broadcast_test.go | 14 +- Server/api/totp_handler.go | 410 +-------- Server/auth/ratescale.go | 39 + Server/invariants/db_import_boundary.go | 2 - Server/invariants/db_import_boundary_test.go | 2 +- Server/service/auth.go | 854 ++++++++++++++++++ docs/architecture/server-boundaries.md | 148 +-- docs/architecture/server.md | 83 +- docs/plans/README.md | 3 +- ...rver-architecture-guardrails-2026-08-29.md | 123 ++- docs/plans/hp-3-scorecard-2026-08-29.md | 231 +++++ 22 files changed, 1739 insertions(+), 1097 deletions(-) create mode 100644 Server/api/auth_deps.go create mode 100644 Server/auth/ratescale.go create mode 100644 Server/service/auth.go create mode 100644 docs/plans/hp-3-scorecard-2026-08-29.md diff --git a/Server/api/auth_deps.go b/Server/api/auth_deps.go new file mode 100644 index 00000000..c3ff3f94 --- /dev/null +++ b/Server/api/auth_deps.go @@ -0,0 +1,53 @@ +package api + +import ( + "context" + + "github.com/J3vb/OwnCord/Server/service" +) + +// AuthService is the consumer-owned interface behind the auth routes: every +// call auth_handler.go and totp_handler.go make below the transport layer, and +// nothing more (layout-refactor supplement, "interface beside the consumer"). +// service.AuthService implements it. A handler decodes and validates the +// request, calls one method, and encodes either the result or the returned +// service.Err* value; every lockout, password compare, sentinel mapping, +// audit write and broadcast lives behind these nine methods. +// +// Nine methods stand in for the ten *db.DB methods, two db functions and two +// db sentinels the two handlers called directly at 71d867cb +// (docs/architecture/server-boundaries.md, "Auth slice"). +type AuthService interface { + // RegistrationPolicy reports whether registration is permitted right now. + // It is the one gate that runs before the body is read: two + // characterization rows pin a closed server's 403 ahead of any + // credential, malformed body included. + RegistrationPolicy(ctx context.Context) error + // Register consumes the invite, creates the account and issues a session. + // in is already validated (see service.RegisterInput). + Register(ctx context.Context, in service.RegisterInput) (*service.AuthResult, error) + // Login runs the lockout gates and the constant-time password check, then + // issues a session or, for an enrolled account, starts a two-factor + // challenge. + Login(ctx context.Context, in service.LoginInput) (*service.AuthResult, error) + // VerifyTOTP completes a challenge Login started and issues the session, + // bound to the login request's device and IP rather than this one's. + VerifyTOTP(ctx context.Context, partialToken, code string) (*service.AuthResult, error) + // Logout revokes p.Session server-side and clears the custom status. + Logout(ctx context.Context, p service.Principal) error + // DeleteAccount confirms the password, anonymises and bans the account and + // broadcasts member_ban. ip is only logged and audited. + DeleteAccount(ctx context.Context, p service.Principal, password, ip string) error + // EnableTOTP confirms the password and stages a pending secret; qrURI is + // the enrolment payload for the authenticator app. + EnableTOTP(ctx context.Context, p service.Principal, password string) (qrURI string, err error) + // ConfirmTOTP verifies code against the pending secret, persists it and + // revokes the caller's other sessions. + ConfirmTOTP(ctx context.Context, p service.Principal, password, code string) (*service.TOTPChangeResult, error) + // DisableTOTP confirms the password, refuses while the server requires + // 2FA, clears the secret and revokes the caller's other sessions. + DisableTOTP(ctx context.Context, p service.Principal, password string) (*service.TOTPChangeResult, error) +} + +// The production implementation satisfies the interface it was extracted for. +var _ AuthService = (*service.AuthService)(nil) diff --git a/Server/api/auth_handler.go b/Server/api/auth_handler.go index c5d33254..a2b81ee2 100644 --- a/Server/api/auth_handler.go +++ b/Server/api/auth_handler.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "log/slog" "net/http" "strings" @@ -12,8 +11,6 @@ import ( "unicode/utf8" "github.com/J3vb/OwnCord/Server/auth" - "github.com/J3vb/OwnCord/Server/db" - "github.com/J3vb/OwnCord/Server/permissions" "github.com/J3vb/OwnCord/Server/service" "github.com/go-chi/chi/v5" ) @@ -21,16 +18,9 @@ import ( // maxLoginUsernameLen bounds the username accepted by handleLogin, mirroring // auth.ValidateUsername's 32-rune cap on registered usernames. Enforced // before the value is ever used to build a RateLimiter map key — see the -// check in handleLogin for why. +// check in loginReadRequest for why. const maxLoginUsernameLen = 32 -// genericAuthError is returned for all login/register failures to avoid -// revealing whether a username exists. -var genericAuthError = errorResponse{ - Error: "INVALID_CREDENTIALS", - Message: "invalid invite or credentials", -} - // registerRequest is the JSON body for POST /api/v1/auth/register. type registerRequest struct { Username string `json:"username"` @@ -44,25 +34,6 @@ type loginRequest struct { Password string `json:"password"` } -// userResponse is the user shape included in auth responses. -type userResponse struct { - ID int64 `json:"id"` - Username string `json:"username"` - Avatar string `json:"avatar,omitempty"` - // DisplayName and About are always present (null = unset) so the settings - // form can tell "cleared" from "the server does not know this field". - DisplayName *string `json:"display_name"` - About *string `json:"about"` - // CustomStatus is the user's own free-text status line. - CustomStatus *string `json:"custom_status"` - // Status is the user's own true status, invisible included. This response - // only ever describes the caller, so there is nothing to hide from them. - Status string `json:"status"` - RoleID int64 `json:"role_id"` - TOTPEnabled bool `json:"totp_enabled"` - CreatedAt string `json:"created_at"` -} - // authSuccessResponse is returned on successful login/register. type authSuccessResponse struct { Token string `json:"token,omitempty"` @@ -71,77 +42,56 @@ type authSuccessResponse struct { User *userResponse `json:"user,omitempty"` } -// AuthBroadcaster is the interface handleDeleteAccount uses to notify -// connected WebSocket clients that an account is gone. Satisfied by *ws.Hub -// (which already implements BroadcastMemberBan for the admin ban path this -// mirrors). -type AuthBroadcaster interface { - BroadcastMemberBan(userID int64) -} - -// MountAuthRoutes registers all auth endpoints on the given router. -// Rate limiters are applied per-endpoint as specified. trustedProxies is the -// list of CIDRs whose X-Forwarded-For / X-Real-IP headers are honoured for -// rate-limiting IP resolution. totpKey is the AES-256 key used to encrypt -// TOTP secrets at rest (M1 security hardening). -// -// broadcaster is variadic and optional: MountAuthRoutes is called before the -// hub exists (router.go mounts auth routes first, and the hub needs the -// router to register its own webhook route), so a caller that cannot supply -// one yet may omit it entirely and self-deletion simply sends no event, -// exactly like today. A caller mounted after hub creation should pass it so -// DELETE /api/v1/auth/account can broadcast the same member_ban event the -// admin ban path already sends for the identical anonymise-and-ban DB state. -func MountAuthRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter, trustedProxies []string, totpKey []byte, broadcaster ...AuthBroadcaster) { - var ab AuthBroadcaster - if len(broadcaster) > 0 { - ab = broadcaster[0] - } - registerLimiter := limiter - loginLimiter := limiter - partialStore := auth.NewPartialAuthStore(partialAuthStoreTTL) - pendingTOTPStore := auth.NewPendingTOTPStore(pendingTOTPStoreTTL) - usedTOTPCodes := auth.NewUsedTOTPCodeStore() - +// MountAuthRoutes registers all auth endpoints on the given router. svc owns +// every decision below the transport (service.AuthService in production, see +// AuthService); requireAuth is the AuthMiddleware the authenticated routes +// mount, built by the caller because it needs the database handle this file +// no longer sees. Rate limiters are applied per-endpoint as specified; +// trustedProxies is the list of CIDRs whose X-Forwarded-For / X-Real-IP +// headers are honoured for rate-limiting IP resolution. +func MountAuthRoutes(r chi.Router, svc AuthService, requireAuth func(http.Handler) http.Handler, limiter *auth.RateLimiter, trustedProxies []string) { r.Route("/api/v1/auth", func(r chi.Router) { - r.With(RateLimitMiddleware(registerLimiter, "register:", scaledAuthLimit(registerRateLimitPerMinute), time.Minute, trustedProxies)). - Post("/register", handleRegister(database, trustedProxies)) + r.With(RateLimitMiddleware(limiter, "register:", scaledAuthLimit(registerRateLimitPerMinute), time.Minute, trustedProxies)). + Post("/register", handleRegister(svc, trustedProxies)) - r.With(RateLimitMiddleware(loginLimiter, "login:", scaledAuthLimit(loginRateLimitPerMinute), time.Minute, trustedProxies)). - Post("/login", handleLogin(database, limiter, partialStore, trustedProxies)) + r.With(RateLimitMiddleware(limiter, "login:", scaledAuthLimit(loginRateLimitPerMinute), time.Minute, trustedProxies)). + Post("/login", handleLogin(svc, trustedProxies)) r.With(RateLimitMiddleware(limiter, "totp_verify:", scaledAuthLimit(verifyTOTPRateLimitPerMinute), time.Minute, trustedProxies)). - Post("/verify-totp", handleVerifyTOTP(database, partialStore, limiter, usedTOTPCodes, totpKey)) + Post("/verify-totp", handleVerifyTOTP(svc)) - r.With(AuthMiddleware(database)). - Post("/logout", handleLogout(database)) + r.With(requireAuth). + Post("/logout", handleLogout(svc)) - r.With(AuthMiddleware(database)). + r.With(requireAuth). Get("/me", handleMe()) - r.With(AuthMiddleware(database), + r.With(requireAuth, RateLimitMiddleware(limiter, "del_account:", scaledAuthLimit(sensitiveEndpointRateLimitPerMinute), time.Minute, trustedProxies)). - Delete("/account", handleDeleteAccount(database, limiter, ab)) + Delete("/account", handleDeleteAccount(svc)) }) - r.With(AuthMiddleware(database), + r.With(requireAuth, RateLimitMiddleware(limiter, "totp:", scaledAuthLimit(sensitiveEndpointRateLimitPerMinute), time.Minute, trustedProxies)). - Post("/api/v1/users/me/totp/enable", handleEnableTOTP(pendingTOTPStore, limiter)) + Post("/api/v1/users/me/totp/enable", handleEnableTOTP(svc)) - r.With(AuthMiddleware(database), + r.With(requireAuth, RateLimitMiddleware(limiter, "totp:", scaledAuthLimit(sensitiveEndpointRateLimitPerMinute), time.Minute, trustedProxies)). - Post("/api/v1/users/me/totp/confirm", handleConfirmTOTP(database, pendingTOTPStore, usedTOTPCodes, limiter, totpKey)) + Post("/api/v1/users/me/totp/confirm", handleConfirmTOTP(svc)) - r.With(AuthMiddleware(database), + r.With(requireAuth, RateLimitMiddleware(limiter, "totp:", scaledAuthLimit(sensitiveEndpointRateLimitPerMinute), time.Minute, trustedProxies)). - Delete("/api/v1/users/me/totp", handleDisableTOTP(database, pendingTOTPStore, limiter)) + Delete("/api/v1/users/me/totp", handleDisableTOTP(svc)) } // handleRegister processes POST /api/v1/auth/register. -func handleRegister(database *db.DB, trustedProxies []string) http.HandlerFunc { +func handleRegister(svc AuthService, trustedProxies []string) http.HandlerFunc { proxyNets := parseCIDRList(trustedProxies) // W3-3a: parse once at construction return func(w http.ResponseWriter, r *http.Request) { - if !registerPolicyGate(w, r, database) { + // The policy gate runs before any credential is read: a closed + // server refuses even a malformed body with the policy's 403. + if err := svc.RegistrationPolicy(r.Context()); err != nil { + writeAuthError(r.Context(), w, err) return } @@ -150,116 +100,21 @@ func handleRegister(database *db.DB, trustedProxies []string) http.HandlerFunc { return } - // Hash password before consuming the invite so that a hashing failure - // does not burn a valid invite code. - hash, err := auth.HashPassword(req.Password) + res, err := svc.Register(r.Context(), service.RegisterInput{ + Username: req.Username, + Password: req.Password, + InviteCode: req.InviteCode, + Device: truncateDevice(r.Header.Get("User-Agent")), + IP: clientIPWithProxies(r, proxyNets), + }) if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to process registration", - }) + writeAuthError(r.Context(), w, err) return } - - // Atomically consume the invite and create the user so failed - // registrations do not burn a valid invite code. - uid, err := database.CreateUserWithInvite(r.Context(), req.Username, hash, int(permissions.MemberRoleID), req.InviteCode) - if err != nil { - // UNIQUE constraint violation → duplicate username → 400. - // Any other DB error → 500. - switch { - case db.IsUniqueConstraintError(err): - writeJSON(w, http.StatusBadRequest, genericAuthError) - case errors.Is(err, db.ErrNotFound): - writeJSON(w, http.StatusBadRequest, genericAuthError) - default: - slog.Error("CreateUserWithInvite failed", "err", err, "username", req.Username) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "registration failed — please try again", - }) - } - return - } - - ip := clientIPWithProxies(r, proxyNets) - slog.Info("user registered", "username", req.Username, "user_id", uid, "ip", ip) - db.WriteAudit(context.WithoutCancel(r.Context()), database, uid, "user_register", "user", uid, - "new account created via invite") - - // Issue session. - token, err := auth.GenerateToken() - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to create session", - }) - return - } - - device := truncateDevice(r.Header.Get("User-Agent")) - if _, err := database.CreateSession(r.Context(), uid, auth.HashToken(token), device, ip); err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to create session", - }) - return - } - - user, err := database.GetUserByID(r.Context(), uid) - if err != nil || user == nil { - slog.Error("failed to fetch user after registration", "user_id", uid, "error", err) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "registration succeeded but user fetch failed", - }) - return - } - writeJSON(w, http.StatusCreated, authSuccessResponse{ - Token: token, - Requires2FA: false, - User: toUserResponse(user), - }) + writeJSON(w, http.StatusCreated, authResponse(res)) } } -// registerPolicyGate reports whether registration is currently permitted, -// writing the refusal response itself when it is not. -func registerPolicyGate(w http.ResponseWriter, r *http.Request, database *db.DB) bool { - registrationOpen, err := isRegistrationOpen(r.Context(), database) - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to load registration policy", - }) - return false - } - if !registrationOpen { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "registration is currently closed", - }) - return false - } - - require2FA, err := isRequire2FAEnabled(r.Context(), database) - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to load registration policy", - }) - return false - } - if require2FA { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "registration is unavailable while two-factor authentication is required", - }) - return false - } - return true -} - // registerReadRequest decodes and validates the registration body, writing the // rejection response itself when the input cannot be used. func registerReadRequest(w http.ResponseWriter, r *http.Request) (registerRequest, bool) { @@ -327,7 +182,7 @@ func registerReadRequest(w http.ResponseWriter, r *http.Request) (registerReques } // handleLogin processes POST /api/v1/auth/login. -func handleLogin(database *db.DB, limiter *auth.RateLimiter, partialStore *auth.PartialAuthStore, trustedProxies []string) http.HandlerFunc { +func handleLogin(svc AuthService, trustedProxies []string) http.HandlerFunc { proxyNets := parseCIDRList(trustedProxies) // W3-3a: parse once at construction return func(w http.ResponseWriter, r *http.Request) { req, ok := loginReadRequest(w, r) @@ -335,77 +190,17 @@ func handleLogin(database *db.DB, limiter *auth.RateLimiter, partialStore *auth. return } - ip := clientIPWithProxies(r, proxyNets) - - user, ok := loginAuthenticate(w, r, database, limiter, req, ip) - if !ok { - return - } - - if auth.IsEffectivelyBanned(user) { - slog.Warn("banned user login attempt", "username", user.Username, "user_id", user.ID, "ip", ip) - db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "login_blocked_banned", "user", user.ID, - "banned user attempted login from "+ip) - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "your account has been suspended", - }) - return - } - - require2FA, err := isRequire2FAEnabled(r.Context(), database) - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to load authentication policy", - }) - return - } - if user.TOTPSecret != nil { - partialToken, err := partialStore.Issue(user.ID, truncateDevice(r.Header.Get("User-Agent")), ip) - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to start two-factor challenge", - }) - return - } - writeJSON(w, http.StatusOK, authSuccessResponse{ - PartialToken: partialToken, - Requires2FA: true, - }) - return - } - if require2FA { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "two-factor authentication must be enabled on this account before login", - }) - return - } - - // Issue session. - token, err := issueSession(r.Context(), database, user.ID, truncateDevice(r.Header.Get("User-Agent")), ip) - if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to create session", - }) - return - } - - // Don't set status to "online" here — the WebSocket connection in - // serve.go does that when the user actually connects. Setting it here - // would leave the user permanently "online" if they never open a WS - // connection or if the client crashes before connecting. - slog.Info("user logged in", "username", user.Username, "user_id", user.ID, "ip", ip) - db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "user_login", "user", user.ID, - "logged in from "+ip) - writeJSON(w, http.StatusOK, authSuccessResponse{ - Token: token, - Requires2FA: false, - User: toUserResponse(user), + res, err := svc.Login(r.Context(), service.LoginInput{ + Username: req.Username, + Password: req.Password, + Device: truncateDevice(r.Header.Get("User-Agent")), + IP: clientIPWithProxies(r, proxyNets), }) + if err != nil { + writeAuthError(r.Context(), w, err) + return + } + writeJSON(w, http.StatusOK, authResponse(res)) } } @@ -434,12 +229,13 @@ func loginReadRequest(w http.ResponseWriter, r *http.Request) (loginRequest, boo } // F: reject an over-long username before it is ever used to build a - // RateLimiter map key below (unameKey, failKey, userFailKey, lockout - // keys). Unlike registration, login has no account to validate - // against yet, so nothing else bounds this value — an unauthenticated - // caller could otherwise pin an arbitrarily large, body-sized string - // as a retained key (Cleanup only evicts it after hours). Mirrors the - // same 32-rune cap auth.ValidateUsername enforces at registration. + // RateLimiter map key (unameKey, failKey, userFailKey, lockout keys in + // service.AuthService). Unlike registration, login has no account to + // validate against yet, so nothing else bounds this value — an + // unauthenticated caller could otherwise pin an arbitrarily large, + // body-sized string as a retained key (Cleanup only evicts it after + // hours). Mirrors the same 32-rune cap auth.ValidateUsername enforces at + // registration. if utf8.RuneCountInString(req.Username) > maxLoginUsernameLen { writeJSON(w, http.StatusBadRequest, errorResponse{ Error: "INVALID_INPUT", @@ -450,144 +246,18 @@ func loginReadRequest(w http.ResponseWriter, r *http.Request) (loginRequest, boo return req, true } -// loginAuthenticate runs the lockout gates, the constant-time password compare -// and the failure accounting for one login attempt. It returns the -// authenticated user, or false after writing the rejection response itself. -func loginAuthenticate(w http.ResponseWriter, r *http.Request, database *db.DB, limiter *auth.RateLimiter, req loginRequest, ip string) (*db.User, bool) { - // Check per-IP lockout first. - lockKey := "login_lock:" + ip - if limiter.IsLockedOut(lockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "account temporarily locked due to too many failed attempts", - }) - return nil, false - } - - // BUG-110: Also check per-username lockout to prevent distributed brute force. - // F1: canonicalize the username the same way GetUserByUsername does (COLLATE - // NOCASE) before keying the lockout, so case variants of one account - // (admin/Admin/ADMIN) share a single bucket instead of each getting its own. - unameKey := strings.ToLower(req.Username) - userLockKey := "login_user_lock:" + unameKey - if limiter.IsLockedOut(userLockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "account temporarily locked due to too many failed attempts", - }) - return nil, false - } - - // Constant-time lookup: always attempt bcrypt compare even when user - // does not exist to prevent timing-based username enumeration. - user, err := database.GetUserByUsername(r.Context(), req.Username) - - // Distinguish DB errors from authentication failures. DB errors - // should NOT increment the rate limiter — otherwise a transient - // DB outage would lock out legitimate users. - if err != nil && user == nil { - // Could be a real DB error or simply "user not found". - // GetUserByUsername returns (nil, nil) for not-found, so a - // non-nil error here is a genuine DB failure. - slog.Error("login: GetUserByUsername failed", "err", err, "ip", ip) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "login temporarily unavailable", - }) - return nil, false - } - - failKey := "login_fail:" + ip - userFailKey := "login_user_fail:" + unameKey - // F3: atomically reserve this attempt BEFORE the bcrypt compare. The - // read-only IsLockedOut gates above are check-then-act: N concurrent - // requests all pass them before any failure is recorded below, so the - // per-username cap — the only cross-IP brute-force defence — bound - // only sequential attackers. Allow records the attempt under the - // limiter's lock, capping a concurrent burst at the same budget a - // sequential attacker gets. Sized at threshold+1 so the sequential - // accepted-input set is unchanged: failures 1–10 still land, the 10th - // still trips the lockout (via the Check below), and a correct - // password on attempt 10 still succeeds — successful logins reset - // both counters. The reservation sits after the DB-error return above - // so a transient DB outage still does not consume attempts. - if !limiter.Allow(failKey, scaledAuthLimit(loginFailureThreshold)+1, loginFailureWindow) || - !limiter.Allow(userFailKey, loginUserFailureThreshold+1, loginUserFailureWindow) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "account temporarily locked due to too many failed attempts", - }) - return nil, false - } - // Always run the password check — with an empty hash when the user does - // not exist. auth.CheckPassword performs a dummy bcrypt comparison for an - // empty hash, so bcrypt executes on every path and response time stays - // constant, preventing timing-based username enumeration. (A `user == nil - // || CheckPassword(...)` short-circuit would skip bcrypt entirely for - // unknown usernames, reintroducing the timing side-channel.) - storedHash := "" - if user != nil { - storedHash = user.PasswordHash - } - if !auth.CheckPassword(storedHash, req.Password) { - // The attempt was already recorded atomically up-front (F3); here - // only decide the lockouts, at the same boundary as before: the - // 10th in-window failure locks the key. Check is read-only, so - // the reservation is not double-counted. - if !limiter.Check(failKey, scaledAuthLimit(loginFailureThreshold)+1, loginFailureWindow) { - limiter.Lockout(r.Context(), lockKey, loginLockoutDuration) - } - // BUG-110: per-username lockout on threshold. - if !limiter.Check(userFailKey, loginUserFailureThreshold+1, loginUserFailureWindow) { - limiter.Lockout(r.Context(), userLockKey, loginUserLockoutDuration) - } - slog.Info("login failed", "ip", ip, "username_len", len(req.Username)) - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid credentials", - }) - return nil, false - } - - // Reset failure counters on success. - limiter.Reset(r.Context(), failKey) - limiter.Reset(r.Context(), userFailKey) - return user, true -} - // handleLogout processes POST /api/v1/auth/logout. -func handleLogout(database *db.DB) http.HandlerFunc { +func handleLogout(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - sess, ok := r.Context().Value(SessionKey).(*db.Session) - if !ok || sess == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) + p, ok := principal(r) + if !ok || p.Session == nil { + writeNotAuthenticated(w) return } - - // The client clears its token optimistically — once logout reaches the - // server, the revocation must not die with a dropped connection. - if err := database.DeleteSession(context.WithoutCancel(r.Context()), sess.TokenHash); err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to logout", - }) + if err := svc.Logout(r.Context(), p); err != nil { + writeAuthError(r.Context(), w, err) return } - - // A custom status is a "what I am doing right now" note. Leaving it - // standing after the user signed out states something about them that - // is no longer true, so logout clears it — unlike the chosen presence - // status, which is a preference and deliberately survives. - if err := database.UpdateUserCustomStatus(context.WithoutCancel(r.Context()), sess.UserID, nil); err != nil { - slog.Warn("failed to clear custom status on logout", "user_id", sess.UserID, "err", err) - } - - slog.Info("user logged out", "user_id", sess.UserID) - db.WriteAudit(context.WithoutCancel(r.Context()), database, sess.UserID, "user_logout", "user", sess.UserID, "") - w.WriteHeader(http.StatusNoContent) } } @@ -595,15 +265,12 @@ func handleLogout(database *db.DB) http.HandlerFunc { // handleMe processes GET /api/v1/auth/me. func handleMe() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user, ok := r.Context().Value(UserKey).(*db.User) - if !ok || user == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) + p, ok := principal(r) + if !ok { + writeNotAuthenticated(w) return } - writeJSON(w, http.StatusOK, toUserResponse(user)) + writeJSON(w, http.StatusOK, toUserResponse(p.User)) } } @@ -612,30 +279,14 @@ type deleteAccountRequest struct { Password string `json:"password"` } -// handleDeleteAccount processes DELETE /api/v1/auth/account. -// The caller must supply their current password for confirmation. -// Progressive lockout mirrors the login handler: 3 failures → 15-min lock. -// broadcaster may be nil, in which case no event is sent and other connected -// clients converge on their next reconnect instead (same fallback every -// other broadcaster-optional handler in this package uses). -func handleDeleteAccount(database *db.DB, limiter *auth.RateLimiter, broadcaster AuthBroadcaster) http.HandlerFunc { +// handleDeleteAccount processes DELETE /api/v1/auth/account. The caller must +// supply their current password for confirmation; the lockout, the compare +// and the member_ban broadcast are the service's. +func handleDeleteAccount(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user, ok := r.Context().Value(UserKey).(*db.User) - if !ok || user == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) - return - } - - // Per-user lockout to prevent password brute-force on this destructive endpoint. - lockKey := auth.Key("delete_lock", user.ID) - if limiter.IsLockedOut(lockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "too many failed attempts, try again later", - }) + p, ok := principal(r) + if !ok { + writeNotAuthenticated(w) return } @@ -648,80 +299,70 @@ func handleDeleteAccount(database *db.DB, limiter *auth.RateLimiter, broadcaster return } - if req.Password == "" { - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "INVALID_INPUT", - Message: "password is required", - }) + if err := svc.DeleteAccount(r.Context(), p, req.Password, clientIP(r)); err != nil { + writeAuthError(r.Context(), w, err) return } - - // Verify the supplied password matches the stored hash. - failKey := auth.Key("delete_fail", user.ID) - if !auth.CheckPassword(user.PasswordHash, req.Password) { - if !limiter.Allow(failKey, deleteAccountFailureThreshold, deleteAccountFailureWindow) { - limiter.Lockout(r.Context(), lockKey, deleteAccountLockoutDuration) - } - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "INVALID_INPUT", - Message: "incorrect password", - }) - return - } - limiter.Reset(r.Context(), failKey) - - if err := database.DeleteAccount(r.Context(), user.ID); err != nil { - if errors.Is(err, db.ErrLastAdmin) { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "cannot delete the last admin account", - }) - return - } - slog.Error("DeleteAccount failed", "err", err, "user_id", user.ID) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to delete account", - }) - return - } - - ip := clientIP(r) - slog.Info("account deleted", "username", user.Username, "user_id", user.ID, "ip", ip) - db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "account_deleted", "user", user.ID, - "account self-deleted from "+ip) - - // DeleteAccount left the row in exactly the state an admin ban does - // (anonymised, banned, sessions revoked) — broadcast the same event so - // every other connected client drops the deleted user immediately - // instead of keeping their pre-deletion username until it reconnects. - if broadcaster != nil { - broadcaster.BroadcastMemberBan(user.ID) - } - w.WriteHeader(http.StatusNoContent) } } -// toUserResponse converts a db.User to the API response shape. -func toUserResponse(u *db.User) *userResponse { - avatar := "" - if u.Avatar != nil { - avatar = *u.Avatar +// authResponse encodes a service result: a session, or the two-factor +// challenge Login started instead of one. +func authResponse(res *service.AuthResult) authSuccessResponse { + if res.Requires2FA { + return authSuccessResponse{ + PartialToken: res.PartialToken, + Requires2FA: true, + } } - resp := &userResponse{ - ID: u.ID, - Username: u.Username, - Avatar: avatar, - DisplayName: u.DisplayName, - About: u.About, - CustomStatus: u.CustomStatus, - Status: u.Status, - RoleID: u.RoleID, - TOTPEnabled: u.TOTPSecret != nil, - CreatedAt: u.CreatedAt, + return authSuccessResponse{ + Token: res.Token, + Requires2FA: false, + User: toUserResponse(res.User), } - return resp +} + +// writeNotAuthenticated is the refusal for a route mounted behind +// AuthMiddleware that still finds no usable principal on the request. +func writeNotAuthenticated(w http.ResponseWriter) { + writeJSON(w, http.StatusUnauthorized, errorResponse{ + Error: "UNAUTHORIZED", + Message: "not authenticated", + }) +} + +// writeAuthError encodes a service.Err* refusal from the auth slice. Each +// named value's Error() is the public message; its category picks the +// status and code, and two values carry a code of their own. Anything that +// is not an auth refusal is a contract bug in the service, logged and +// answered as a generic 500 so no cause leaks to the client. +func writeAuthError(ctx context.Context, w http.ResponseWriter, err error) { + var status int + var code string + switch { + case errors.Is(err, service.ErrRegistrationRejected): + status, code = http.StatusBadRequest, "INVALID_CREDENTIALS" + case errors.Is(err, service.ErrTOTPAlreadyEnabled): + status, code = http.StatusConflict, "TOTP_ALREADY_ENABLED" + case errors.Is(err, service.ErrRateLimited): + status, code = http.StatusTooManyRequests, "RATE_LIMITED" + case errors.Is(err, service.ErrUnauthorized): + status, code = http.StatusUnauthorized, "UNAUTHORIZED" + case errors.Is(err, service.ErrForbidden): + status, code = http.StatusForbidden, "FORBIDDEN" + case errors.Is(err, service.ErrInvalidInput): + status, code = http.StatusBadRequest, "INVALID_INPUT" + case errors.Is(err, service.ErrBadRequest): + status, code = http.StatusBadRequest, "BAD_REQUEST" + case errors.Is(err, service.ErrInternal): + status, code = http.StatusInternalServerError, "INTERNAL_ERROR" + default: + slog.ErrorContext(ctx, "auth service returned a non-refusal error", "error", err) + writeJSON(w, http.StatusInternalServerError, errorResponse{Error: "INTERNAL_ERROR", Message: "internal error"}) + return + } + writeJSON(w, status, errorResponse{Error: code, Message: err.Error()}) } // truncateDevice truncates the User-Agent to prevent oversized session records. @@ -733,54 +374,3 @@ func truncateDevice(ua string) string { } return ua } - -func issueSession(ctx context.Context, database *db.DB, userID int64, device, ip string) (string, error) { - token, err := auth.GenerateToken() - if err != nil { - return "", err - } - if _, err := database.CreateSession(ctx, userID, auth.HashToken(token), device, ip); err != nil { - return "", err - } - return token, nil -} - -func isRequire2FAEnabled(ctx context.Context, database *db.DB) (bool, error) { - return getBooleanSetting(ctx, database, "require_2fa", false) -} - -func isRegistrationOpen(ctx context.Context, database *db.DB) (bool, error) { - return getBooleanSetting(ctx, database, "registration_open", true) -} - -func getBooleanSetting(ctx context.Context, database *db.DB, key string, defaultValue bool) (bool, error) { - value, err := database.GetSetting(ctx, key) - if err != nil { - if errors.Is(err, db.ErrNotFound) { - return defaultValue, nil - } - return false, err - } - return parseBooleanSettingValue(value) -} - -func parseBooleanSettingValue(value string) (bool, error) { - switch strings.ToLower(strings.TrimSpace(value)) { - case "1", "true": - return true, nil - case "0", "false": - return false, nil - default: - return false, fmt.Errorf("invalid boolean setting value %q", value) - } -} - -func requirePasswordConfirmation(user *db.User, password string) error { - if password == "" { - return fmt.Errorf("password is required") - } - if !auth.CheckPassword(user.PasswordHash, password) { - return fmt.Errorf("password confirmation failed") - } - return nil -} diff --git a/Server/api/auth_handler_delete_broadcast_test.go b/Server/api/auth_handler_delete_broadcast_test.go index 8d43c6c9..e418fcc1 100644 --- a/Server/api/auth_handler_delete_broadcast_test.go +++ b/Server/api/auth_handler_delete_broadcast_test.go @@ -7,6 +7,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/auth" + "github.com/J3vb/OwnCord/Server/service" "github.com/go-chi/chi/v5" ) @@ -32,7 +33,7 @@ func TestDeleteAccount_BroadcastsMemberBan(t *testing.T) { broadcaster := &recordingAuthBroadcaster{} r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter, nil, testTOTPKey, broadcaster) + api.MountAuthRoutes(r, service.NewAuthService(database, limiter, testTOTPKey, broadcaster), api.AuthMiddleware(database), limiter, nil) hash, _ := auth.HashPassword("correctPass1") uid, _ := database.CreateUser(context.Background(), "deletebroadcast", hash, 4) @@ -51,14 +52,14 @@ func TestDeleteAccount_BroadcastsMemberBan(t *testing.T) { } } -// Omitting the broadcaster (the shape every existing MountAuthRoutes call -// site uses today) must keep working exactly as before: no event, no panic. +// A nil broadcaster (the shape every test mount uses) must keep working +// exactly as before: no event, no panic. func TestDeleteAccount_NoBroadcasterOmitted(t *testing.T) { database := newAuthTestDB(t) limiter := auth.NewRateLimiter() r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter, nil, testTOTPKey) + api.MountAuthRoutes(r, service.NewAuthService(database, limiter, testTOTPKey, nil), api.AuthMiddleware(database), limiter, nil) hash, _ := auth.HashPassword("correctPass1") uid, _ := database.CreateUser(context.Background(), "deletenobroadcast", hash, 4) diff --git a/Server/api/auth_handler_test.go b/Server/api/auth_handler_test.go index dba48a33..24d90d3f 100644 --- a/Server/api/auth_handler_test.go +++ b/Server/api/auth_handler_test.go @@ -17,6 +17,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/service" "github.com/go-chi/chi/v5" ) @@ -45,7 +46,7 @@ func buildAuthRouter(database *db.DB, limiter *auth.RateLimiter) http.Handler { func buildAuthRouterWithProxies(database *db.DB, limiter *auth.RateLimiter, trustedProxies []string) http.Handler { r := chi.NewRouter() - api.MountAuthRoutes(r, database, limiter, trustedProxies, testTOTPKey) + api.MountAuthRoutes(r, service.NewAuthService(database, limiter, testTOTPKey, nil), api.AuthMiddleware(database), limiter, trustedProxies) return r } diff --git a/Server/api/constants.go b/Server/api/constants.go index 4c2fa052..528d3e26 100644 --- a/Server/api/constants.go +++ b/Server/api/constants.go @@ -1,10 +1,9 @@ package api import ( - "math" - "sync/atomic" "time" + "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/config" ) @@ -13,36 +12,18 @@ import ( // Each constant defines either a request cap or a sliding-window duration used // by the per-endpoint rate limiters. -// authRateScaleBits holds the security.auth_rate_limit_multiplier as float -// bits. It scales the per-IP auth request caps and failure thresholds for -// deployments where many users share one IP (office/school NAT) — the -// compiled-in constants below assume roughly one person per address. Atomic -// because tests construct multiple routers concurrently. Set via -// setAuthRateScale in NewRouter; reads happen at mount time and on the login -// failure-count path. -var authRateScaleBits atomic.Uint64 - -func init() { authRateScaleBits.Store(math.Float64bits(1.0)) } +// The auth rate multiplier (security.auth_rate_limit_multiplier) lives in +// auth (auth/ratescale.go) since B3-2, so the route mounts here and the login +// failure accounting in service.AuthService read one value. These wrappers +// keep the mount sites and api/constants_test.go unchanged. // setAuthRateScale clamps and installs the auth rate multiplier. Zero or // negative (unset config) means 1.0. -func setAuthRateScale(m float64) { - if m <= 0 { - m = 1.0 - } - m = math.Min(math.Max(m, 0.1), 100) - authRateScaleBits.Store(math.Float64bits(m)) -} +func setAuthRateScale(m float64) { auth.SetRateScale(m) } // scaledAuthLimit applies the auth rate multiplier to a compiled-in limit, // never returning less than 1. -func scaledAuthLimit(n int) int { - scaled := int(math.Round(float64(n) * math.Float64frombits(authRateScaleBits.Load()))) - if scaled < 1 { - return 1 - } - return scaled -} +func scaledAuthLimit(n int) int { return auth.ScaledLimit(n) } const ( // registerRateLimitPerMinute is the maximum registration attempts per IP per minute. @@ -72,40 +53,6 @@ const ( // stays under this; it exists to bound abuse of the operator's Klipy quota. gifRateLimitPerMinute = 30 - // loginFailureThreshold is the number of failed login attempts (within - // loginFailureWindow) before the IP is locked out. - loginFailureThreshold = 9 - - // loginFailureWindow is the sliding window for counting login failures. - loginFailureWindow = 15 * time.Minute - - // loginLockoutDuration is how long an IP is locked out after exceeding - // loginFailureThreshold. - loginLockoutDuration = 15 * time.Minute - - // deleteAccountFailureThreshold is the number of wrong-password attempts - // before the per-user lockout kicks in. - deleteAccountFailureThreshold = 3 - - // deleteAccountFailureWindow is the sliding window for counting - // delete-account password failures. - deleteAccountFailureWindow = 15 * time.Minute - - // deleteAccountLockoutDuration is how long the account-deletion endpoint - // is locked after exceeding deleteAccountFailureThreshold. - deleteAccountLockoutDuration = 15 * time.Minute - - // totpFailureRateLimit is the maximum TOTP verification failures per user - // within totpFailureWindow before the user is rate-limited. - totpFailureRateLimit = 10 - - // totpFailureWindow is the sliding window for counting per-user TOTP failures. - totpFailureWindow = 15 * time.Minute - - // partialAuthMaxFailures is the number of failed TOTP attempts on a single - // partial-auth challenge before it is revoked. - partialAuthMaxFailures = 5 - // profilePasswordRateLimitPerMinute is the maximum password change attempts // per IP per minute. profilePasswordRateLimitPerMinute = 5 @@ -114,29 +61,6 @@ const ( // per user per minute. profileUpdateRateLimitPerMinute = 10 - // loginUserFailureThreshold is the number of failed login attempts for a - // specific username (regardless of source IP) before the account is locked. - loginUserFailureThreshold = 9 - - // loginUserFailureWindow is the sliding window for per-username login failures. - loginUserFailureWindow = 15 * time.Minute - - // loginUserLockoutDuration is how long a username is locked after exceeding - // loginUserFailureThreshold. - loginUserLockoutDuration = 15 * time.Minute - - // pwConfirmFailureThreshold is the number of wrong-password attempts on - // password-confirmation endpoints before per-user lockout kicks in. - pwConfirmFailureThreshold = 3 - - // pwConfirmFailureWindow is the sliding window for per-user password - // confirmation failures. - pwConfirmFailureWindow = 15 * time.Minute - - // pwConfirmLockoutDuration is how long password-confirmation endpoints are - // locked after exceeding pwConfirmFailureThreshold. - pwConfirmLockoutDuration = 15 * time.Minute - // uploadRateLimitPerMinute is the maximum file uploads per user per minute. uploadRateLimitPerMinute = 10 @@ -149,12 +73,6 @@ const ( // ─── Timeouts & TTLs ──────────────────────────────────────────────────────── const ( - // partialAuthStoreTTL is the lifetime of a partial-auth (2FA) challenge token. - partialAuthStoreTTL = 10 * time.Minute - - // pendingTOTPStoreTTL is the lifetime of a pending TOTP enrollment secret. - pendingTOTPStoreTTL = 10 * time.Minute - // rateLimiterCleanupInterval is how often stale rate-limiter entries are reaped. rateLimiterCleanupInterval = 5 * time.Minute diff --git a/Server/api/constants_test.go b/Server/api/constants_test.go index 44f98e73..01ac3cfc 100644 --- a/Server/api/constants_test.go +++ b/Server/api/constants_test.go @@ -27,8 +27,9 @@ func TestRateLimiterCleanupHorizon_CoversMaxSlowMode(t *testing.T) { } // setAuthRateScale/scaledAuthLimit gate every per-IP auth limit -// (auth_handler.go:107-136) and the per-IP login failure threshold that arms -// the lockout (auth_handler.go:514,537). The multiplier is operator-supplied +// (auth_handler.go MountAuthRoutes) and, through auth.ScaledLimit, the per-IP +// login failure threshold that arms the lockout (service/auth.go +// authenticate). The multiplier is operator-supplied // via security.auth_rate_limit_multiplier and config validates nothing, so // this clamp is all that stands between a typo and brute-force protection // disappearing. @@ -49,7 +50,7 @@ func TestSetAuthRateScale_ClampsMultiplier(t *testing.T) { {"below the floor clamps to 0.1x", 1e-9, verifyTOTPRateLimitPerMinute, 1}, {"at the floor is 0.1x", 0.1, verifyTOTPRateLimitPerMinute, 1}, {"in range scales and rounds", 0.5, loginRateLimitPerMinute, 3}, - {"in range scales the failure threshold", 2, loginFailureThreshold, 18}, + {"in range scales the failure threshold", 2, 9, 18}, // 9 = service/auth.go loginFailureThreshold } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -63,7 +64,7 @@ func TestSetAuthRateScale_ClampsMultiplier(t *testing.T) { } // A limit of 0 lets nothing through: on the login failure threshold -// (auth_handler.go:514) that locks every IP out on its first attempt. The +// (service/auth.go authenticate) that locks every IP out on its first attempt. The // smallest allowed multiplier must still leave every scaled limit usable. func TestScaledAuthLimit_NeverBelowOne(t *testing.T) { t.Cleanup(func() { setAuthRateScale(1.0) }) @@ -75,7 +76,7 @@ func TestScaledAuthLimit_NeverBelowOne(t *testing.T) { loginRateLimitPerMinute, verifyTOTPRateLimitPerMinute, sensitiveEndpointRateLimitPerMinute, - loginFailureThreshold, + 9, // service/auth.go loginFailureThreshold } { if got := scaledAuthLimit(n); got < 1 { t.Errorf("scaledAuthLimit(%d) = %d at the 0.1x floor, want >= 1", n, got) @@ -85,20 +86,19 @@ func TestScaledAuthLimit_NeverBelowOne(t *testing.T) { // The multiplier exists for shared-NAT *per-IP* limits. The per-user caps are // the only cross-IP brute-force defence, so scaling them would hand a -// distributed attacker up to 100x the guesses (totp_handler.go:76-80). Those -// caps are only observable through a limiter key inside the handler, so this -// pins the call site instead. +// distributed attacker up to 100x the guesses (service/auth.go VerifyTOTP). +// Those caps are only observable through a limiter key inside the service, so +// this pins the call site instead. func TestPerUserFailureCapsStayUnscaled(t *testing.T) { for file, constants := range map[string][]string{ - "totp_handler.go": {"totpFailureRateLimit"}, - "auth_handler.go": {"loginUserFailureThreshold"}, + "../service/auth.go": {"totpFailureRateLimit", "loginUserFailureThreshold"}, } { src, err := os.ReadFile(file) if err != nil { t.Fatalf("read %s: %v", file, err) } for _, c := range constants { - if strings.Contains(string(src), "scaledAuthLimit("+c) { + if strings.Contains(string(src), "ScaledLimit("+c) || strings.Contains(string(src), "scaledAuthLimit("+c) { t.Errorf("%s scales %s with the per-IP auth multiplier; per-user caps must stay unscaled", file, c) } diff --git a/Server/api/coverage_push_test.go b/Server/api/coverage_push_test.go index 546fb6b2..85cd90e5 100644 --- a/Server/api/coverage_push_test.go +++ b/Server/api/coverage_push_test.go @@ -750,7 +750,7 @@ func buildCombinedRouter(t *testing.T) (http.Handler, *auth.RateLimiter, string) r := chi.NewRouter() svc := service.New(database, limiter) - api.MountAuthRoutes(r, database, limiter, nil, testTOTPKey) + api.MountAuthRoutes(r, service.NewAuthService(database, limiter, testTOTPKey, nil), api.AuthMiddleware(database), limiter, nil) api.MountProfileRoutes(r, database, svc, nil, limiter, nil, nil) api.MountInviteRoutes(r, database, svc) diff --git a/Server/api/invite_handler_test.go b/Server/api/invite_handler_test.go index 031986f7..e530c3ec 100644 --- a/Server/api/invite_handler_test.go +++ b/Server/api/invite_handler_test.go @@ -19,7 +19,7 @@ import ( func buildInviteRouter(database *db.DB, limiter *auth.RateLimiter) http.Handler { r := chi.NewRouter() svc := service.New(database, limiter) - api.MountAuthRoutes(r, database, limiter, nil, testTOTPKey) + api.MountAuthRoutes(r, service.NewAuthService(database, limiter, testTOTPKey, nil), api.AuthMiddleware(database), limiter, nil) api.MountInviteRoutes(r, database, svc) return r } diff --git a/Server/api/middleware.go b/Server/api/middleware.go index dfbb0f3a..bedeabd3 100644 --- a/Server/api/middleware.go +++ b/Server/api/middleware.go @@ -14,6 +14,7 @@ import ( "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/db" "github.com/J3vb/OwnCord/Server/permissions" + "github.com/J3vb/OwnCord/Server/service" ) // contextKey is an unexported type for context keys in this package. @@ -67,6 +68,18 @@ func (t *touchThrottle) shouldTouch(hash string, now time.Time) bool { return true } +// principal returns the caller AuthMiddleware resolved for r as the shape +// the service layer takes. ok is false when the request carries no +// authenticated user; Session is nil for an API-token principal. +func principal(r *http.Request) (service.Principal, bool) { + user, ok := r.Context().Value(UserKey).(*db.User) + if !ok || user == nil { + return service.Principal{}, false + } + sess, _ := r.Context().Value(SessionKey).(*db.Session) + return service.Principal{User: user, Session: sess}, true +} + // AuthMiddleware reads the "Authorization: Bearer " header, validates // the session, and injects the user and session into the request context. // Returns 401 if the token is missing, invalid, or the session is expired. diff --git a/Server/api/profile_handler.go b/Server/api/profile_handler.go index 14f7c483..a9853bd8 100644 --- a/Server/api/profile_handler.go +++ b/Server/api/profile_handler.go @@ -23,6 +23,48 @@ import ( // ─── Request / Response types ──────────────────────────────────────────────── +// userResponse is the caller's own user record, the shape auth responses and +// PATCH /users/me return. It lives beside the profile handler because this is +// the file that still sees db.User; the auth handlers get it through +// toUserResponse without naming db (B3-2). +type userResponse struct { + ID int64 `json:"id"` + Username string `json:"username"` + Avatar string `json:"avatar,omitempty"` + // DisplayName and About are always present (null = unset) so the settings + // form can tell "cleared" from "the server does not know this field". + DisplayName *string `json:"display_name"` + About *string `json:"about"` + // CustomStatus is the user's own free-text status line. + CustomStatus *string `json:"custom_status"` + // Status is the user's own true status, invisible included. This response + // only ever describes the caller, so there is nothing to hide from them. + Status string `json:"status"` + RoleID int64 `json:"role_id"` + TOTPEnabled bool `json:"totp_enabled"` + CreatedAt string `json:"created_at"` +} + +// toUserResponse converts a db.User to the API response shape. +func toUserResponse(u *db.User) *userResponse { + avatar := "" + if u.Avatar != nil { + avatar = *u.Avatar + } + return &userResponse{ + ID: u.ID, + Username: u.Username, + Avatar: avatar, + DisplayName: u.DisplayName, + About: u.About, + CustomStatus: u.CustomStatus, + Status: u.Status, + RoleID: u.RoleID, + TOTPEnabled: u.TOTPSecret != nil, + CreatedAt: u.CreatedAt, + } +} + // updateProfileRequest is the JSON body for PATCH /api/v1/users/me. // identity_public_key, when present, publishes the client's long-term E2EE // identity public key (F3 voice E2EE TOFU); omitted = leave unchanged. @@ -392,8 +434,8 @@ func handleChangePassword(svc *service.Services, limiter *auth.RateLimiter) http // Verify old password using constant-time bcrypt comparison. failKey := auth.Key("pw_confirm_fail", user.ID) if !auth.CheckPassword(user.PasswordHash, req.OldPassword) { - if !limiter.Allow(failKey, pwConfirmFailureThreshold, pwConfirmFailureWindow) { - limiter.Lockout(r.Context(), lockKey, pwConfirmLockoutDuration) + if !limiter.Allow(failKey, service.PwConfirmFailureThreshold, service.PwConfirmFailureWindow) { + limiter.Lockout(r.Context(), lockKey, service.PwConfirmLockoutDuration) } writeJSON(w, http.StatusForbidden, errorResponse{ Error: "FORBIDDEN", Message: "incorrect password", diff --git a/Server/api/router.go b/Server/api/router.go index a39b5da9..dd877001 100644 --- a/Server/api/router.go +++ b/Server/api/router.go @@ -109,12 +109,11 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri getOnlineUsers = func() int { return hub.ClientCount() } hubAlive = func() bool { return hub.DispatchAlive() } - // Auth routes: register, login, logout, me. Mounted with the hub as the - // AuthBroadcaster so DELETE /api/v1/auth/account (self-service account - // deletion) fans out member_ban and force-disconnects the deleted user's - // own socket, exactly like the admin ban path does for the same - // anonymise-and-ban DB state. - MountAuthRoutes(r, database, limiter, cfg.Server.TrustedProxies, totpKey, hub) + // Auth routes. The service is built after the hub, with the hub as its + // AuthBroadcaster, so DELETE /api/v1/auth/account fans out member_ban and + // force-disconnects the deleted user's own socket exactly like the admin + // ban path does for the same DB state. B3-3 moves this to internal/app. + MountAuthRoutes(r, service.NewAuthService(database, limiter, totpKey, hub), AuthMiddleware(database), limiter, cfg.Server.TrustedProxies) routerPluginWiring(hub, pluginRegistry) diff --git a/Server/api/router_delete_account_broadcast_test.go b/Server/api/router_delete_account_broadcast_test.go index 39e5e9fa..a831bd52 100644 --- a/Server/api/router_delete_account_broadcast_test.go +++ b/Server/api/router_delete_account_broadcast_test.go @@ -1,11 +1,11 @@ package api_test // router_delete_account_broadcast_test.go pins the production wiring for -// OC-0048: NewRouter (router.go) must pass the WS hub to MountAuthRoutes as -// its optional AuthBroadcaster so self-service account deletion fans out -// member_ban exactly like the admin ban path does. MountAuthRoutes is called -// before the hub exists in router.go, so the only production call site used -// to omit the broadcaster entirely — handleDeleteAccount's +// OC-0048: NewRouter (router.go) must hand the WS hub to +// service.NewAuthService as its AuthBroadcaster so self-service account +// deletion fans out member_ban exactly like the admin ban path does. Auth +// routes were once mounted before the hub existed in router.go, so the only +// production call site used to omit the broadcaster entirely — handleDeleteAccount's // `if broadcaster != nil` guard was never taken outside tests that construct // their own fake broadcaster (see auth_handler_delete_broadcast_test.go, // which only proves the handler itself works when a broadcaster IS passed). @@ -169,7 +169,7 @@ func TestNewRouter_DeleteAccount_BroadcastsMemberBanOverWS(t *testing.T) { if !sawMemberBan { t.Fatal("no member_ban WS broadcast for the deleted user observed on a second client — " + - "router.go's MountAuthRoutes call must pass the hub as the optional " + - "AuthBroadcaster (mount it after ws.NewHub, not before)") + "router.go must build service.NewAuthService with the hub as its " + + "AuthBroadcaster (after ws.NewHub, not before)") } } diff --git a/Server/api/totp_handler.go b/Server/api/totp_handler.go index 9c0fd1d4..8cbe17d2 100644 --- a/Server/api/totp_handler.go +++ b/Server/api/totp_handler.go @@ -1,17 +1,13 @@ package api import ( - "context" "encoding/json" "errors" "io" - "log/slog" "net/http" - "strings" - "time" "github.com/J3vb/OwnCord/Server/auth" - "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/service" ) // ─── TOTP request/response types ───────────────────────────────────────────── @@ -36,7 +32,9 @@ type totpEnableResponse struct { // ─── Handlers ──────────────────────────────────────────────────────────────── -func handleVerifyTOTP(database *db.DB, partialStore *auth.PartialAuthStore, limiter *auth.RateLimiter, usedTOTPCodes *auth.UsedTOTPCodeStore, totpKey []byte) http.HandlerFunc { +// handleVerifyTOTP processes POST /api/v1/auth/verify-totp: the bearer token +// is the partial-login challenge Login issued. +func handleVerifyTOTP(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { partialToken, ok := auth.ExtractBearerToken(r) if !ok { @@ -47,15 +45,6 @@ func handleVerifyTOTP(database *db.DB, partialStore *auth.PartialAuthStore, limi return } - challenge, ok := partialStore.Lookup(partialToken) - if !ok { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid or expired two-factor challenge", - }) - return - } - var req verifyTotpRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { writeJSON(w, http.StatusBadRequest, errorResponse{ @@ -65,137 +54,21 @@ func handleVerifyTOTP(database *db.DB, partialStore *auth.PartialAuthStore, limi return } - totpRateLimitKey := auth.Key("totp_fail", challenge.UserID) - // Atomically record this attempt and reject once the per-user failure cap - // is reached. Recording up-front — rather than a read-only Check now and - // Allow only on failure — closes a TOCTOU where many concurrent requests - // reusing one valid partial token all pass the read-only check before any - // failure is recorded, defeating the per-user brute-force cap (the only - // cross-IP defence). A successful verification resets the counter below, - // so legitimate retries are not penalised. - // Deliberately NOT scaledAuthLimit: this cap is keyed per USER, and it - // is the only cross-IP brute-force defence on TOTP codes. The - // multiplier exists for shared-NAT per-IP limits; scaling a per-user - // threshold with it would hand a distributed attacker more guesses. - // Mirrors loginUserFailureThreshold staying unscaled in auth_handler. - if !limiter.Allow(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "too many failed attempts, try again later", - }) - return - } - - user, secret, ok := totpChallengeSecret(w, r, database, totpKey, challenge.UserID) - if !ok { - return - } - - if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(req.Code), time.Now().UTC(), user.ID, usedTOTPCodes) { - // The attempt was already recorded atomically up-front via - // limiter.Allow; only the per-partial-token counter is advanced here. - partialStore.RegisterFailure(partialToken, partialAuthMaxFailures) - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid two-factor code", - }) - return - } - - limiter.Reset(r.Context(), totpRateLimitKey) - - if _, ok := partialStore.Consume(partialToken); !ok { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid or expired two-factor challenge", - }) - return - } - - token, err := issueSession(r.Context(), database, user.ID, challenge.Device, challenge.IP) + res, err := svc.VerifyTOTP(r.Context(), partialToken, req.Code) if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to create session", - }) + writeAuthError(r.Context(), w, err) return } - - slog.Info("totp verified", "user_id", user.ID, "ip", challenge.IP) - db.WriteAudit(context.WithoutCancel(r.Context()), database, user.ID, "totp_verified", "user", user.ID, - "two-factor verification completed from "+challenge.IP) - - writeJSON(w, http.StatusOK, authSuccessResponse{ - Token: token, - Requires2FA: false, - User: toUserResponse(user), - }) + writeJSON(w, http.StatusOK, authResponse(res)) } } -// totpChallengeSecret resolves the user behind a partial-auth challenge and -// returns their decrypted TOTP secret. It writes its own refusal, so a false -// third result means the response is already complete. -func totpChallengeSecret(w http.ResponseWriter, r *http.Request, database *db.DB, totpKey []byte, challengeUserID int64) (*db.User, string, bool) { - user, err := database.GetUserByID(r.Context(), challengeUserID) - if err != nil || user == nil || user.TOTPSecret == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid or expired two-factor challenge", - }) - return nil, "", false - } - - // A ban can land inside the partial-token window; the login path - // refuses banned users right after the password compare, so the - // second factor must refuse them too. - if auth.IsEffectivelyBanned(user) { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "your account has been suspended", - }) - return nil, "", false - } - - secret, decErr := auth.DecryptTOTPSecret(totpKey, *user.TOTPSecret) - if decErr != nil { - slog.Error("failed to decrypt TOTP secret", "user_id", user.ID, "error", decErr) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to verify two-factor code", - }) - return nil, "", false - } - - return user, secret, true -} - -func handleEnableTOTP(pendingStore *auth.PendingTOTPStore, limiter *auth.RateLimiter) http.HandlerFunc { +// handleEnableTOTP processes POST /api/v1/users/me/totp/enable. +func handleEnableTOTP(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user, ok := r.Context().Value(UserKey).(*db.User) - if !ok || user == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) - return - } - - // BUG-111: Per-user lockout for password confirmation. - lockKey := auth.Key("pw_confirm_lock", user.ID) - if limiter.IsLockedOut(lockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "too many failed attempts, try again later", - }) - return - } - - if user.TOTPSecret != nil && *user.TOTPSecret != "" { - writeJSON(w, http.StatusConflict, errorResponse{ - Error: "TOTP_ALREADY_ENABLED", - Message: "disable 2FA before re-enabling", - }) + p, ok := principal(r) + if !ok { + writeNotAuthenticated(w) return } @@ -207,78 +80,25 @@ func handleEnableTOTP(pendingStore *auth.PendingTOTPStore, limiter *auth.RateLim }) return } - failKey := auth.Key("pw_confirm_fail", user.ID) - if err := requirePasswordConfirmation(user, req.Password); err != nil { - if !limiter.Allow(failKey, pwConfirmFailureThreshold, pwConfirmFailureWindow) { - limiter.Lockout(r.Context(), lockKey, pwConfirmLockoutDuration) - } - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "INVALID_INPUT", - Message: err.Error(), - }) - return - } - limiter.Reset(r.Context(), failKey) - secret, err := auth.GenerateTOTPSecret() + qrURI, err := svc.EnableTOTP(r.Context(), p, req.Password) if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to generate two-factor secret", - }) + writeAuthError(r.Context(), w, err) return } - - pendingStore.Put(user.ID, secret) writeJSON(w, http.StatusOK, totpEnableResponse{ - QRURI: auth.BuildTOTPURI(user.Username, secret, "OwnCord"), + QRURI: qrURI, BackupCodes: []string{}, }) } } -// revokeOtherSessionsAfterAuthChange revokes every session for userID except -// keepSessionID as the security tail of a committed 2FA state change. It -// mirrors UserService.ChangePassword (service/user.go:262-274): a failure is -// logged and retried once (bounded compensating retry for transient write -// contention); if the retry also fails, revoked reports what did succeed and -// failed is true so the caller can report a partial success instead of -// silently claiming the other sessions were revoked when they were not. -func revokeOtherSessionsAfterAuthChange(ctx context.Context, database *db.DB, userID, keepSessionID int64, action string) (revoked int64, failed bool) { - revoked, err := database.DeleteOtherSessions(ctx, userID, keepSessionID) - if err != nil { - slog.Error("DeleteOtherSessions after "+action, "err", err, "user_id", userID) - revokedRetry, retryErr := database.DeleteOtherSessions(ctx, userID, keepSessionID) - if retryErr != nil { - slog.Error("DeleteOtherSessions retry after "+action, "err", retryErr, "user_id", userID) - return revoked, true - } - revoked += revokedRetry - } - if revoked > 0 { - slog.Info("revoked other sessions after "+action, "user_id", userID, "revoked", revoked) - } - return revoked, false -} - -func handleConfirmTOTP(database *db.DB, pendingStore *auth.PendingTOTPStore, usedTOTPCodes *auth.UsedTOTPCodeStore, limiter *auth.RateLimiter, totpKey []byte) http.HandlerFunc { +// handleConfirmTOTP processes POST /api/v1/users/me/totp/confirm. +func handleConfirmTOTP(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user, ok := r.Context().Value(UserKey).(*db.User) - if !ok || user == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) - return - } - - // BUG-111: Per-user lockout for password confirmation. - lockKey := auth.Key("pw_confirm_lock", user.ID) - if limiter.IsLockedOut(lockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "too many failed attempts, try again later", - }) + p, ok := principal(r) + if !ok { + writeNotAuthenticated(w) return } @@ -290,107 +110,24 @@ func handleConfirmTOTP(database *db.DB, pendingStore *auth.PendingTOTPStore, use }) return } - failKey := auth.Key("pw_confirm_fail", user.ID) - if err := requirePasswordConfirmation(user, req.Password); err != nil { - if !limiter.Allow(failKey, pwConfirmFailureThreshold, pwConfirmFailureWindow) { - limiter.Lockout(r.Context(), lockKey, pwConfirmLockoutDuration) - } - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "INVALID_INPUT", - Message: err.Error(), - }) + + res, err := svc.ConfirmTOTP(r.Context(), p, req.Password, req.Code) + if err != nil { + writeAuthError(r.Context(), w, err) return } - limiter.Reset(r.Context(), failKey) - - secret, ok := pendingStore.Lookup(user.ID) - if !ok { - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "BAD_REQUEST", - Message: "no pending two-factor enrollment found", - }) - return - } - - if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(req.Code), time.Now().UTC(), user.ID, usedTOTPCodes) { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "invalid two-factor code", - }) - return - } - - encryptedSecret, encErr := auth.EncryptTOTPSecret(totpKey, secret) - if encErr != nil { - slog.Error("failed to encrypt TOTP secret", "user_id", user.ID, "error", encErr) - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to enable two-factor authentication", - }) - return - } - - if err := database.UpdateUserTOTPSecret(r.Context(), user.ID, &encryptedSecret); err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to enable two-factor authentication", - }) - return - } - pendingStore.Delete(user.ID) - - // BUG-108: Revoke all other sessions after 2FA state change. An - // API-token principal has a nil session; keep=0 matches no row, so - // every login session is revoked — same semantics as change-password. - sess, _ := r.Context().Value(SessionKey).(*db.Session) - keepSessionID := int64(0) - if sess != nil { - keepSessionID = sess.ID - } - // Security tail of the 2FA change: once the secret update committed, - // revoking the other sessions must not be aborted by a dead request. - tailCtx := context.WithoutCancel(r.Context()) - revoked, revokeFailed := revokeOtherSessionsAfterAuthChange(tailCtx, database, user.ID, keepSessionID, "totp enable") - - slog.Info("totp enabled", "user_id", user.ID) - db.WriteAudit(tailCtx, database, user.ID, "totp_enabled", "user", user.ID, - "two-factor authentication enrolled") - - if revokeFailed { - // Partial success: 2FA IS enabled; only revoking the other - // sessions failed. A 5xx here would be a lie — the state change - // already committed — so mirror the ChangePassword contract - // (api/profile_handler.go) and report 200 with an explicit warning - // instead of a silent, unqualified 204. - writeJSON(w, http.StatusOK, map[string]any{ - "warning": "two-factor authentication enabled, but other sessions could not be revoked; revoke them from the sessions list", - "sessions_revoked": revoked, - }) - return - } - - w.WriteHeader(http.StatusNoContent) + writeTOTPChange(w, res) } } -func handleDisableTOTP(database *db.DB, pendingStore *auth.PendingTOTPStore, limiter *auth.RateLimiter) http.HandlerFunc { +// handleDisableTOTP processes DELETE /api/v1/users/me/totp. An empty body is +// accepted (and then refused by the service as a missing password); only a +// body that is present and not JSON is malformed. +func handleDisableTOTP(svc AuthService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - user, ok := r.Context().Value(UserKey).(*db.User) - if !ok || user == nil { - writeJSON(w, http.StatusUnauthorized, errorResponse{ - Error: "UNAUTHORIZED", - Message: "not authenticated", - }) - return - } - - // BUG-111: Per-user lockout for password confirmation. - lockKey := auth.Key("pw_confirm_lock", user.ID) - if limiter.IsLockedOut(lockKey) { - writeJSON(w, http.StatusTooManyRequests, errorResponse{ - Error: "RATE_LIMITED", - Message: "too many failed attempts, try again later", - }) + p, ok := principal(r) + if !ok { + writeNotAuthenticated(w) return } @@ -402,74 +139,27 @@ func handleDisableTOTP(database *db.DB, pendingStore *auth.PendingTOTPStore, lim }) return } - failKey := auth.Key("pw_confirm_fail", user.ID) - if err := requirePasswordConfirmation(user, req.Password); err != nil { - if !limiter.Allow(failKey, pwConfirmFailureThreshold, pwConfirmFailureWindow) { - limiter.Lockout(r.Context(), lockKey, pwConfirmLockoutDuration) - } - writeJSON(w, http.StatusBadRequest, errorResponse{ - Error: "INVALID_INPUT", - Message: err.Error(), - }) - return - } - limiter.Reset(r.Context(), failKey) - require2FA, err := isRequire2FAEnabled(r.Context(), database) + res, err := svc.DisableTOTP(r.Context(), p, req.Password) if err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to load authentication policy", - }) + writeAuthError(r.Context(), w, err) return } - if require2FA { - writeJSON(w, http.StatusForbidden, errorResponse{ - Error: "FORBIDDEN", - Message: "two-factor authentication is required for this server", - }) - return - } - - pendingStore.Delete(user.ID) - if err := database.UpdateUserTOTPSecret(r.Context(), user.ID, nil); err != nil { - writeJSON(w, http.StatusInternalServerError, errorResponse{ - Error: "INTERNAL_ERROR", - Message: "failed to disable two-factor authentication", - }) - return - } - - // BUG-108: Revoke all other sessions after 2FA state change. An - // API-token principal has a nil session; keep=0 matches no row, so - // every login session is revoked — same semantics as change-password. - sess, _ := r.Context().Value(SessionKey).(*db.Session) - keepSessionID := int64(0) - if sess != nil { - keepSessionID = sess.ID - } - // Security tail of the 2FA change: once the secret update committed, - // revoking the other sessions must not be aborted by a dead request. - tailCtx := context.WithoutCancel(r.Context()) - revoked, revokeFailed := revokeOtherSessionsAfterAuthChange(tailCtx, database, user.ID, keepSessionID, "totp disable") - - slog.Info("totp disabled", "user_id", user.ID) - db.WriteAudit(tailCtx, database, user.ID, "totp_disabled", "user", user.ID, - "two-factor authentication disabled") - - if revokeFailed { - // Partial success: 2FA IS disabled; only revoking the other - // sessions failed. A 5xx here would be a lie — the state change - // already committed — so mirror the ChangePassword contract - // (api/profile_handler.go) and report 200 with an explicit warning - // instead of a silent, unqualified 204. - writeJSON(w, http.StatusOK, map[string]any{ - "warning": "two-factor authentication disabled, but other sessions could not be revoked; revoke them from the sessions list", - "sessions_revoked": revoked, - }) - return - } - - w.WriteHeader(http.StatusNoContent) + writeTOTPChange(w, res) } } + +// writeTOTPChange answers a committed 2FA change: 204, or 200 with the +// warning when the caller's other sessions could not be revoked — a partial +// success the service reports instead of a 5xx, because the change is +// already durable. +func writeTOTPChange(w http.ResponseWriter, res *service.TOTPChangeResult) { + if res.Warning != "" { + writeJSON(w, http.StatusOK, map[string]any{ + "warning": res.Warning, + "sessions_revoked": res.SessionsRevoked, + }) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/Server/auth/ratescale.go b/Server/auth/ratescale.go new file mode 100644 index 00000000..d1283761 --- /dev/null +++ b/Server/auth/ratescale.go @@ -0,0 +1,39 @@ +package auth + +import ( + "math" + "sync/atomic" +) + +// rateScaleBits holds security.auth_rate_limit_multiplier as float bits. It +// scales the per-IP auth request caps and failure thresholds for deployments +// where many users share one IP (office/school NAT) — the compiled-in limits +// assume roughly one person per address. Atomic because tests construct +// multiple routers concurrently. Installed by api.NewRouter via SetRateScale; +// read at route-mount time and on the login failure-count path +// (service.AuthService). +var rateScaleBits atomic.Uint64 + +func init() { rateScaleBits.Store(math.Float64bits(1.0)) } + +// SetRateScale clamps and installs the auth rate multiplier. Zero or +// negative (unset config) means 1.0. +func SetRateScale(m float64) { + if m <= 0 { + m = 1.0 + } + m = math.Min(math.Max(m, 0.1), 100) + rateScaleBits.Store(math.Float64bits(m)) +} + +// ScaledLimit applies the auth rate multiplier to a compiled-in limit, never +// returning less than 1. Per-user caps must not go through it: they are the +// only cross-IP brute-force defence, and the multiplier exists for shared-NAT +// per-IP limits. +func ScaledLimit(n int) int { + scaled := int(math.Round(float64(n) * math.Float64frombits(rateScaleBits.Load()))) + if scaled < 1 { + return 1 + } + return scaled +} diff --git a/Server/invariants/db_import_boundary.go b/Server/invariants/db_import_boundary.go index d9650b4c..60c38a50 100644 --- a/Server/invariants/db_import_boundary.go +++ b/Server/invariants/db_import_boundary.go @@ -56,7 +56,6 @@ var DBImportAllow = map[string]DBImportEntry{ "admin/setup_wizard.go": {"move", "auth", "BeginTx for the wizard; setup sub-family"}, "admin/types.go": {"adapter", "", "response DTOs; the one GetRoleByID moves with handlers_users"}, // ── api ─────────────────────────────────────────────────────────────── - "api/auth_handler.go": {"move", "auth", "B3-2 slice: register/login/logout/delete own the DB"}, "api/channel_handler.go": {"adapter", "", "response types only; service owns the calls"}, "api/dm_handler.go": {"adapter", "", "DM response types + pure status helpers"}, "api/emoji_handler.go": {"adapter", "", "Emoji/User types only"}, @@ -66,7 +65,6 @@ var DBImportAllow = map[string]DBImportEntry{ "api/plugins_handler.go": {"adapter", "", "db.Auditor is the seam; WriteAudit only"}, "api/profile_handler.go": {"move", "upload", "avatar upload creates the attachment row"}, "api/router.go": {"boundary", "", "health probe (PingRead, SQLDb); hub construction leaves in B3-3"}, - "api/totp_handler.go": {"move", "auth", "B3-2 slice: TOTP enrol/verify write the user row"}, "api/upload_handler.go": {"move", "upload", "attachment access + a raw QueryRowContext"}, // ── auth ────────────────────────────────────────────────────────────── "auth/helpers.go": {"adapter", "", "db.User type in a helper signature"}, diff --git a/Server/invariants/db_import_boundary_test.go b/Server/invariants/db_import_boundary_test.go index 71a1f837..f887b1d3 100644 --- a/Server/invariants/db_import_boundary_test.go +++ b/Server/invariants/db_import_boundary_test.go @@ -30,7 +30,7 @@ func TestDBImportBoundary(t *testing.T) { }, { name: "listed file is allowed", - path: "api/auth_handler.go", + path: "api/middleware.go", src: "package api\n" + importDB + "\nvar _ *db.DB\n", want: 0, }, diff --git a/Server/service/auth.go b/Server/service/auth.go new file mode 100644 index 00000000..2da0c745 --- /dev/null +++ b/Server/service/auth.go @@ -0,0 +1,854 @@ +package service + +import ( + "context" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/J3vb/OwnCord/Server/auth" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/permissions" +) + +// ─── Collaborators and shapes ──────────────────────────────────────────────── + +// AuthBroadcaster is how the auth slice tells connected WebSocket clients that +// an account is gone. Satisfied by *ws.Hub, which already implements +// BroadcastMemberBan for the admin ban path that self-deletion mirrors. A nil +// broadcaster sends nothing and other clients converge on their next +// reconnect. +type AuthBroadcaster interface { + BroadcastMemberBan(userID int64) +} + +// Principal is the authenticated caller api.AuthMiddleware resolved for a +// request. Session is nil for an API-token principal. +type Principal struct { + User *db.User + Session *db.Session +} + +// RegisterInput is a validated registration: the transport has already +// trimmed, sanitized and format-checked Username, checked Password strength +// and trimmed InviteCode. Device and IP describe the request that will own +// the issued session. +type RegisterInput struct { + Username string + Password string + InviteCode string + Device string + IP string +} + +// LoginInput is one login attempt. Username is trimmed; Password is not — +// passwords may carry leading or trailing whitespace on purpose. +type LoginInput struct { + Username string + Password string + Device string + IP string +} + +// AuthResult is a successful register, login or second-factor step. Either a +// session was issued (Token and User) or a two-factor challenge was started +// (PartialToken with Requires2FA) — never both. +type AuthResult struct { + Token string + PartialToken string + Requires2FA bool + User *db.User +} + +// TOTPChangeResult reports a committed 2FA enable or disable. Warning is +// non-empty when the state change committed but revoking the caller's other +// sessions failed: a partial success the transport must answer with 200 and +// the warning, never a 5xx, because the change is already durable. +type TOTPChangeResult struct { + SessionsRevoked int64 + Warning string +} + +// ─── Limits ────────────────────────────────────────────────────────────────── +// +// Moved from api/constants.go with the orchestration that reads them (B3-2). + +const ( + // loginFailureThreshold is the number of failed login attempts (within + // loginFailureWindow) before the IP is locked out. + loginFailureThreshold = 9 + + // loginFailureWindow is the sliding window for counting login failures. + loginFailureWindow = 15 * time.Minute + + // loginLockoutDuration is how long an IP is locked out after exceeding + // loginFailureThreshold. + loginLockoutDuration = 15 * time.Minute + + // loginUserFailureThreshold is the number of failed login attempts for a + // specific username (regardless of source IP) before the account is locked. + loginUserFailureThreshold = 9 + + // loginUserFailureWindow is the sliding window for per-username login failures. + loginUserFailureWindow = 15 * time.Minute + + // loginUserLockoutDuration is how long a username is locked after exceeding + // loginUserFailureThreshold. + loginUserLockoutDuration = 15 * time.Minute + + // deleteAccountFailureThreshold is the number of wrong-password attempts + // before the per-user lockout kicks in. + deleteAccountFailureThreshold = 3 + + // deleteAccountFailureWindow is the sliding window for counting + // delete-account password failures. + deleteAccountFailureWindow = 15 * time.Minute + + // deleteAccountLockoutDuration is how long the account-deletion endpoint + // is locked after exceeding deleteAccountFailureThreshold. + deleteAccountLockoutDuration = 15 * time.Minute + + // totpFailureRateLimit is the maximum TOTP verification failures per user + // within totpFailureWindow before the user is rate-limited. + totpFailureRateLimit = 10 + + // totpFailureWindow is the sliding window for counting per-user TOTP failures. + totpFailureWindow = 15 * time.Minute + + // partialAuthMaxFailures is the number of failed TOTP attempts on a single + // partial-auth challenge before it is revoked. + partialAuthMaxFailures = 5 + + // partialAuthStoreTTL is the lifetime of a partial-auth (2FA) challenge token. + partialAuthStoreTTL = 10 * time.Minute + + // pendingTOTPStoreTTL is the lifetime of a pending TOTP enrollment secret. + pendingTOTPStoreTTL = 10 * time.Minute +) + +// The password-confirmation lockout is shared with the change-password route +// (api/profile_handler.go, the user family in B3-8), so one key space +// ("pw_confirm_fail", "pw_confirm_lock") and one budget cover every route +// that asks for the current password. +const ( + // PwConfirmFailureThreshold is the number of wrong-password attempts on + // password-confirmation endpoints before per-user lockout kicks in. + PwConfirmFailureThreshold = 3 + + // PwConfirmFailureWindow is the sliding window for per-user password + // confirmation failures. + PwConfirmFailureWindow = 15 * time.Minute + + // PwConfirmLockoutDuration is how long password-confirmation endpoints are + // locked after exceeding PwConfirmFailureThreshold. + PwConfirmLockoutDuration = 15 * time.Minute +) + +// ─── Errors ────────────────────────────────────────────────────────────────── + +// Category sentinels the transport maps to a status and code. ErrRateLimited, +// ErrForbidden, ErrBadRequest, ErrConflict and ErrInternal are the shared set +// in message.go; these two are what auth adds. +var ( + // ErrUnauthorized is a credential or challenge that does not authenticate + // (401). Deliberately generic: the enumeration guard depends on every + // failed login looking the same. + ErrUnauthorized = errors.New("unauthorized") + // ErrInvalidInput is a request the auth routes refuse as INVALID_INPUT + // (400) — the password-confirmation refusals. + ErrInvalidInput = errors.New("invalid input") +) + +// authError is one refusal the auth slice can return. Error() is the exact +// public message the pre-B3-2 handler wrote — B3-1's characterization rows +// pin it byte for byte — and Is reports the category sentinel the transport +// maps to a status and code, so errors.Is matches both the named value and +// its category. +type authError struct { + kind error + msg string +} + +func (e *authError) Error() string { return e.msg } +func (e *authError) Is(target error) bool { return target == e.kind } + +// Every refusal below is returned bare, never wrapped around the cause: the +// transport echoes Error() to the client, and the cause (a database error, a +// decrypt failure) is logged here instead. +var ( + // Registration. + ErrRegistrationPolicyUnavailable = &authError{ErrInternal, "failed to load registration policy"} + ErrRegistrationClosed = &authError{ErrForbidden, "registration is currently closed"} + ErrRegistrationRequires2FA = &authError{ErrForbidden, "registration is unavailable while two-factor authentication is required"} + ErrPasswordHash = &authError{ErrInternal, "failed to process registration"} + // ErrRegistrationRejected is the generic register refusal: an unknown, + // used-up or expired invite and a taken username share it so the response + // reveals neither. The transport writes it as 400 INVALID_CREDENTIALS. + ErrRegistrationRejected = &authError{ErrBadRequest, "invalid invite or credentials"} + ErrRegistrationFailed = &authError{ErrInternal, "registration failed — please try again"} + ErrSessionIssue = &authError{ErrInternal, "failed to create session"} + ErrRegisteredUserFetch = &authError{ErrInternal, "registration succeeded but user fetch failed"} + + // Login. + ErrLockedOut = &authError{ErrRateLimited, "account temporarily locked due to too many failed attempts"} + ErrLoginUnavailable = &authError{ErrInternal, "login temporarily unavailable"} + ErrInvalidCredentials = &authError{ErrUnauthorized, "invalid credentials"} + ErrBanned = &authError{ErrForbidden, "your account has been suspended"} + ErrAuthPolicyUnavailable = &authError{ErrInternal, "failed to load authentication policy"} + ErrTOTPChallengeStart = &authError{ErrInternal, "failed to start two-factor challenge"} + ErrRequire2FA = &authError{ErrForbidden, "two-factor authentication must be enabled on this account before login"} + + // Logout. + ErrLogoutFailed = &authError{ErrInternal, "failed to logout"} + + // Password confirmation (account deletion and TOTP management). + ErrTooManyAttempts = &authError{ErrRateLimited, "too many failed attempts, try again later"} + ErrPasswordRequired = &authError{ErrInvalidInput, "password is required"} + ErrIncorrectPassword = &authError{ErrInvalidInput, "incorrect password"} + ErrPasswordConfirmationFailed = &authError{ErrInvalidInput, "password confirmation failed"} + + // Account deletion. + ErrLastAdmin = &authError{ErrForbidden, "cannot delete the last admin account"} + ErrDeleteAccountFailed = &authError{ErrInternal, "failed to delete account"} + + // Second factor. + ErrTOTPChallengeInvalid = &authError{ErrUnauthorized, "invalid or expired two-factor challenge"} + ErrTOTPSecretUnreadable = &authError{ErrInternal, "failed to verify two-factor code"} + ErrTOTPCodeInvalid = &authError{ErrUnauthorized, "invalid two-factor code"} + // ErrTOTPAlreadyEnabled is written by the transport as 409 + // TOTP_ALREADY_ENABLED, not the generic CONFLICT code. + ErrTOTPAlreadyEnabled = &authError{ErrConflict, "disable 2FA before re-enabling"} + ErrTOTPSecretGenerate = &authError{ErrInternal, "failed to generate two-factor secret"} + ErrNoPendingTOTP = &authError{ErrBadRequest, "no pending two-factor enrollment found"} + ErrTOTPEnableFailed = &authError{ErrInternal, "failed to enable two-factor authentication"} + ErrTOTPRequiredByServer = &authError{ErrForbidden, "two-factor authentication is required for this server"} + ErrTOTPDisableFailed = &authError{ErrInternal, "failed to disable two-factor authentication"} +) + +// ─── Service ───────────────────────────────────────────────────────────────── + +// AuthService owns the auth slice's orchestration: the lockout and +// enumeration guards, the password and second-factor checks, session issue +// and revoke, the audit writes and the member_ban broadcast on self-deletion. +// Persistence stays in db behind Store. B3-2 moved every line here verbatim +// from api/auth_handler.go and api/totp_handler.go at 71d867cb; B3-1's +// characterization rows pin the behaviour. +type AuthService struct { + st Store + limiter *auth.RateLimiter + partial *auth.PartialAuthStore + pending *auth.PendingTOTPStore + usedCodes *auth.UsedTOTPCodeStore + totpKey []byte + broadcaster AuthBroadcaster +} + +// NewAuthService wires the auth slice. limiter is the shared auth rate +// limiter (lockouts are keyed inside it); totpKey is the AES-256 key that +// encrypts TOTP secrets at rest; broadcaster may be nil. The three in-memory +// stores — partial-login challenges, pending TOTP enrolments, used TOTP +// codes — are created here with the fixed TTLs the route mount used to own. +func NewAuthService(st Store, limiter *auth.RateLimiter, totpKey []byte, broadcaster AuthBroadcaster) *AuthService { + return &AuthService{ + st: st, + limiter: limiter, + partial: auth.NewPartialAuthStore(partialAuthStoreTTL), + pending: auth.NewPendingTOTPStore(pendingTOTPStoreTTL), + usedCodes: auth.NewUsedTOTPCodeStore(), + totpKey: totpKey, + broadcaster: broadcaster, + } +} + +// RegistrationPolicy reports whether registration is currently permitted. It +// runs before the transport reads any credential: a closed server refuses +// even a malformed body with the policy's 403. +func (s *AuthService) RegistrationPolicy(ctx context.Context) error { + registrationOpen, err := s.registrationOpen(ctx) + if err != nil { + return ErrRegistrationPolicyUnavailable + } + if !registrationOpen { + return ErrRegistrationClosed + } + + require2FA, err := s.require2FAEnabled(ctx) + if err != nil { + return ErrRegistrationPolicyUnavailable + } + if require2FA { + return ErrRegistrationRequires2FA + } + return nil +} + +// Register consumes the invite, creates the account and issues a session. +func (s *AuthService) Register(ctx context.Context, in RegisterInput) (*AuthResult, error) { + // Hash password before consuming the invite so that a hashing failure + // does not burn a valid invite code. + hash, err := auth.HashPassword(in.Password) + if err != nil { + return nil, ErrPasswordHash + } + + // Atomically consume the invite and create the user so failed + // registrations do not burn a valid invite code. + uid, err := s.st.CreateUserWithInvite(ctx, in.Username, hash, int(permissions.MemberRoleID), in.InviteCode) + if err != nil { + // UNIQUE constraint violation → duplicate username → 400. + // Any other DB error → 500. + switch { + case db.IsUniqueConstraintError(err): + return nil, ErrRegistrationRejected + case errors.Is(err, db.ErrNotFound): + return nil, ErrRegistrationRejected + default: + slog.Error("CreateUserWithInvite failed", "err", err, "username", in.Username) + return nil, ErrRegistrationFailed + } + } + + slog.Info("user registered", "username", in.Username, "user_id", uid, "ip", in.IP) + db.WriteAudit(context.WithoutCancel(ctx), s.st, uid, "user_register", "user", uid, + "new account created via invite") + + // Issue session. + token, err := issueSession(ctx, s.st, uid, in.Device, in.IP) + if err != nil { + return nil, ErrSessionIssue + } + + user, err := s.st.GetUserByID(ctx, uid) + if err != nil || user == nil { + slog.Error("failed to fetch user after registration", "user_id", uid, "error", err) + return nil, ErrRegisteredUserFetch + } + return &AuthResult{Token: token, User: user}, nil +} + +// Login runs the lockout gates and the constant-time password check, then +// issues a session or, for an enrolled account, starts a two-factor +// challenge. +func (s *AuthService) Login(ctx context.Context, in LoginInput) (*AuthResult, error) { + user, err := s.authenticate(ctx, in) + if err != nil { + return nil, err + } + + if auth.IsEffectivelyBanned(user) { + slog.Warn("banned user login attempt", "username", user.Username, "user_id", user.ID, "ip", in.IP) + db.WriteAudit(context.WithoutCancel(ctx), s.st, user.ID, "login_blocked_banned", "user", user.ID, + "banned user attempted login from "+in.IP) + return nil, ErrBanned + } + + require2FA, err := s.require2FAEnabled(ctx) + if err != nil { + return nil, ErrAuthPolicyUnavailable + } + if user.TOTPSecret != nil { + partialToken, err := s.partial.Issue(user.ID, in.Device, in.IP) + if err != nil { + return nil, ErrTOTPChallengeStart + } + return &AuthResult{PartialToken: partialToken, Requires2FA: true}, nil + } + if require2FA { + return nil, ErrRequire2FA + } + + // Issue session. + token, err := issueSession(ctx, s.st, user.ID, in.Device, in.IP) + if err != nil { + return nil, ErrSessionIssue + } + + // Don't set status to "online" here — the WebSocket connection in + // serve.go does that when the user actually connects. Setting it here + // would leave the user permanently "online" if they never open a WS + // connection or if the client crashes before connecting. + slog.Info("user logged in", "username", user.Username, "user_id", user.ID, "ip", in.IP) + db.WriteAudit(context.WithoutCancel(ctx), s.st, user.ID, "user_login", "user", user.ID, + "logged in from "+in.IP) + return &AuthResult{Token: token, User: user}, nil +} + +// authenticate runs the lockout gates, the constant-time password compare and +// the failure accounting for one login attempt, returning the authenticated +// user or the refusal. +func (s *AuthService) authenticate(ctx context.Context, in LoginInput) (*db.User, error) { + // Check per-IP lockout first. + lockKey := "login_lock:" + in.IP + if s.limiter.IsLockedOut(lockKey) { + return nil, ErrLockedOut + } + + // BUG-110: Also check per-username lockout to prevent distributed brute force. + // F1: canonicalize the username the same way GetUserByUsername does (COLLATE + // NOCASE) before keying the lockout, so case variants of one account + // (admin/Admin/ADMIN) share a single bucket instead of each getting its own. + unameKey := strings.ToLower(in.Username) + userLockKey := "login_user_lock:" + unameKey + if s.limiter.IsLockedOut(userLockKey) { + return nil, ErrLockedOut + } + + // Constant-time lookup: always attempt bcrypt compare even when user + // does not exist to prevent timing-based username enumeration. + user, err := s.st.GetUserByUsername(ctx, in.Username) + + // Distinguish DB errors from authentication failures. DB errors + // should NOT increment the rate limiter — otherwise a transient + // DB outage would lock out legitimate users. + if err != nil && user == nil { + // Could be a real DB error or simply "user not found". + // GetUserByUsername returns (nil, nil) for not-found, so a + // non-nil error here is a genuine DB failure. + slog.Error("login: GetUserByUsername failed", "err", err, "ip", in.IP) + return nil, ErrLoginUnavailable + } + + failKey := "login_fail:" + in.IP + userFailKey := "login_user_fail:" + unameKey + // F3: atomically reserve this attempt BEFORE the bcrypt compare. The + // read-only IsLockedOut gates above are check-then-act: N concurrent + // requests all pass them before any failure is recorded below, so the + // per-username cap — the only cross-IP brute-force defence — bound + // only sequential attackers. Allow records the attempt under the + // limiter's lock, capping a concurrent burst at the same budget a + // sequential attacker gets. Sized at threshold+1 so the sequential + // accepted-input set is unchanged: failures 1–10 still land, the 10th + // still trips the lockout (via the Check below), and a correct + // password on attempt 10 still succeeds — successful logins reset + // both counters. The reservation sits after the DB-error return above + // so a transient DB outage still does not consume attempts. + // Deliberately NOT auth.ScaledLimit for the per-username budget: that cap + // is keyed per USER and is the only cross-IP brute-force defence, so + // scaling it with the shared-NAT multiplier would hand a distributed + // attacker more guesses (api/constants_test.go pins this call site). + if !s.limiter.Allow(failKey, auth.ScaledLimit(loginFailureThreshold)+1, loginFailureWindow) || + !s.limiter.Allow(userFailKey, loginUserFailureThreshold+1, loginUserFailureWindow) { + return nil, ErrLockedOut + } + // Always run the password check — with an empty hash when the user does + // not exist. auth.CheckPassword performs a dummy bcrypt comparison for an + // empty hash, so bcrypt executes on every path and response time stays + // constant, preventing timing-based username enumeration. (A `user == nil + // || CheckPassword(...)` short-circuit would skip bcrypt entirely for + // unknown usernames, reintroducing the timing side-channel.) + storedHash := "" + if user != nil { + storedHash = user.PasswordHash + } + if !auth.CheckPassword(storedHash, in.Password) { + // The attempt was already recorded atomically up-front (F3); here + // only decide the lockouts, at the same boundary as before: the + // 10th in-window failure locks the key. Check is read-only, so + // the reservation is not double-counted. + if !s.limiter.Check(failKey, auth.ScaledLimit(loginFailureThreshold)+1, loginFailureWindow) { + s.limiter.Lockout(ctx, lockKey, loginLockoutDuration) + } + // BUG-110: per-username lockout on threshold. + if !s.limiter.Check(userFailKey, loginUserFailureThreshold+1, loginUserFailureWindow) { + s.limiter.Lockout(ctx, userLockKey, loginUserLockoutDuration) + } + slog.Info("login failed", "ip", in.IP, "username_len", len(in.Username)) + return nil, ErrInvalidCredentials + } + + // Reset failure counters on success. + s.limiter.Reset(ctx, failKey) + s.limiter.Reset(ctx, userFailKey) + return user, nil +} + +// VerifyTOTP completes a challenge Login started and issues the session, +// bound to the login request's device and IP rather than this one's. +func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string) (*AuthResult, error) { + challenge, ok := s.partial.Lookup(partialToken) + if !ok { + return nil, ErrTOTPChallengeInvalid + } + + totpRateLimitKey := auth.Key("totp_fail", challenge.UserID) + // Atomically record this attempt and reject once the per-user failure cap + // is reached. Recording up-front — rather than a read-only Check now and + // Allow only on failure — closes a TOCTOU where many concurrent requests + // reusing one valid partial token all pass the read-only check before any + // failure is recorded, defeating the per-user brute-force cap (the only + // cross-IP defence). A successful verification resets the counter below, + // so legitimate retries are not penalised. + // Deliberately NOT auth.ScaledLimit: this cap is keyed per USER, and it + // is the only cross-IP brute-force defence on TOTP codes. The + // multiplier exists for shared-NAT per-IP limits; scaling a per-user + // threshold with it would hand a distributed attacker more guesses. + // Mirrors loginUserFailureThreshold staying unscaled in authenticate. + if !s.limiter.Allow(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) { + return nil, ErrTooManyAttempts + } + + user, secret, err := s.challengeSecret(ctx, challenge.UserID) + if err != nil { + return nil, err + } + + if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(code), time.Now().UTC(), user.ID, s.usedCodes) { + // The attempt was already recorded atomically up-front via + // limiter.Allow; only the per-partial-token counter is advanced here. + s.partial.RegisterFailure(partialToken, partialAuthMaxFailures) + return nil, ErrTOTPCodeInvalid + } + + s.limiter.Reset(ctx, totpRateLimitKey) + + if _, ok := s.partial.Consume(partialToken); !ok { + return nil, ErrTOTPChallengeInvalid + } + + token, err := issueSession(ctx, s.st, user.ID, challenge.Device, challenge.IP) + if err != nil { + return nil, ErrSessionIssue + } + + slog.Info("totp verified", "user_id", user.ID, "ip", challenge.IP) + db.WriteAudit(context.WithoutCancel(ctx), s.st, user.ID, "totp_verified", "user", user.ID, + "two-factor verification completed from "+challenge.IP) + return &AuthResult{Token: token, User: user}, nil +} + +// challengeSecret resolves the user behind a partial-auth challenge and +// returns their decrypted TOTP secret. +func (s *AuthService) challengeSecret(ctx context.Context, challengeUserID int64) (*db.User, string, error) { + user, err := s.st.GetUserByID(ctx, challengeUserID) + if err != nil || user == nil || user.TOTPSecret == nil { + return nil, "", ErrTOTPChallengeInvalid + } + + // A ban can land inside the partial-token window; the login path + // refuses banned users right after the password compare, so the + // second factor must refuse them too. + if auth.IsEffectivelyBanned(user) { + return nil, "", ErrBanned + } + + secret, decErr := auth.DecryptTOTPSecret(s.totpKey, *user.TOTPSecret) + if decErr != nil { + slog.Error("failed to decrypt TOTP secret", "user_id", user.ID, "error", decErr) + return nil, "", ErrTOTPSecretUnreadable + } + + return user, secret, nil +} + +// Logout revokes p.Session server-side and clears the custom status. p.Session +// must be non-nil: the transport answers an API-token principal with 401 +// before calling. +func (s *AuthService) Logout(ctx context.Context, p Principal) error { + sess := p.Session + if sess == nil { + return errors.New("logout: principal has no session") + } + + // The client clears its token optimistically — once logout reaches the + // server, the revocation must not die with a dropped connection. + if err := s.st.DeleteSession(context.WithoutCancel(ctx), sess.TokenHash); err != nil { + return ErrLogoutFailed + } + + // A custom status is a "what I am doing right now" note. Leaving it + // standing after the user signed out states something about them that + // is no longer true, so logout clears it — unlike the chosen presence + // status, which is a preference and deliberately survives. + if err := s.st.UpdateUserCustomStatus(context.WithoutCancel(ctx), sess.UserID, nil); err != nil { + slog.Warn("failed to clear custom status on logout", "user_id", sess.UserID, "err", err) + } + + slog.Info("user logged out", "user_id", sess.UserID) + db.WriteAudit(context.WithoutCancel(ctx), s.st, sess.UserID, "user_logout", "user", sess.UserID, "") + return nil +} + +// DeleteAccount confirms the password, anonymises and bans the account and +// broadcasts member_ban. Progressive lockout mirrors login: 3 failures → +// 15-min lock. ip is only logged and audited. +func (s *AuthService) DeleteAccount(ctx context.Context, p Principal, password, ip string) error { + user := p.User + + // Per-user lockout to prevent password brute-force on this destructive endpoint. + lockKey := auth.Key("delete_lock", user.ID) + if s.limiter.IsLockedOut(lockKey) { + return ErrTooManyAttempts + } + + if password == "" { + return ErrPasswordRequired + } + + // Verify the supplied password matches the stored hash. + failKey := auth.Key("delete_fail", user.ID) + if !auth.CheckPassword(user.PasswordHash, password) { + if !s.limiter.Allow(failKey, deleteAccountFailureThreshold, deleteAccountFailureWindow) { + s.limiter.Lockout(ctx, lockKey, deleteAccountLockoutDuration) + } + return ErrIncorrectPassword + } + s.limiter.Reset(ctx, failKey) + + if err := s.st.DeleteAccount(ctx, user.ID); err != nil { + if errors.Is(err, db.ErrLastAdmin) { + return ErrLastAdmin + } + slog.Error("DeleteAccount failed", "err", err, "user_id", user.ID) + return ErrDeleteAccountFailed + } + + slog.Info("account deleted", "username", user.Username, "user_id", user.ID, "ip", ip) + db.WriteAudit(context.WithoutCancel(ctx), s.st, user.ID, "account_deleted", "user", user.ID, + "account self-deleted from "+ip) + + // DeleteAccount left the row in exactly the state an admin ban does + // (anonymised, banned, sessions revoked) — broadcast the same event so + // every other connected client drops the deleted user immediately + // instead of keeping their pre-deletion username until it reconnects. + if s.broadcaster != nil { + s.broadcaster.BroadcastMemberBan(user.ID) + } + return nil +} + +// EnableTOTP confirms the password and stages a pending secret; the returned +// URI is the enrolment payload for the authenticator app. +func (s *AuthService) EnableTOTP(ctx context.Context, p Principal, password string) (string, error) { + user := p.User + + // BUG-111: Per-user lockout for password confirmation. + lockKey := auth.Key("pw_confirm_lock", user.ID) + if s.limiter.IsLockedOut(lockKey) { + return "", ErrTooManyAttempts + } + + if user.TOTPSecret != nil && *user.TOTPSecret != "" { + return "", ErrTOTPAlreadyEnabled + } + + if err := s.confirmPassword(ctx, user, password, lockKey); err != nil { + return "", err + } + + secret, err := auth.GenerateTOTPSecret() + if err != nil { + return "", ErrTOTPSecretGenerate + } + + s.pending.Put(user.ID, secret) + return auth.BuildTOTPURI(user.Username, secret, "OwnCord"), nil +} + +// ConfirmTOTP verifies code against the pending secret, persists it and +// revokes the caller's other sessions. +func (s *AuthService) ConfirmTOTP(ctx context.Context, p Principal, password, code string) (*TOTPChangeResult, error) { + user := p.User + + // BUG-111: Per-user lockout for password confirmation. + lockKey := auth.Key("pw_confirm_lock", user.ID) + if s.limiter.IsLockedOut(lockKey) { + return nil, ErrTooManyAttempts + } + + if err := s.confirmPassword(ctx, user, password, lockKey); err != nil { + return nil, err + } + + secret, ok := s.pending.Lookup(user.ID) + if !ok { + return nil, ErrNoPendingTOTP + } + + if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(code), time.Now().UTC(), user.ID, s.usedCodes) { + return nil, ErrTOTPCodeInvalid + } + + encryptedSecret, encErr := auth.EncryptTOTPSecret(s.totpKey, secret) + if encErr != nil { + slog.Error("failed to encrypt TOTP secret", "user_id", user.ID, "error", encErr) + return nil, ErrTOTPEnableFailed + } + + if err := s.st.UpdateUserTOTPSecret(ctx, user.ID, &encryptedSecret); err != nil { + return nil, ErrTOTPEnableFailed + } + s.pending.Delete(user.ID) + + // Security tail of the 2FA change: once the secret update committed, + // revoking the other sessions must not be aborted by a dead request. + tailCtx := context.WithoutCancel(ctx) + revoked, revokeFailed := s.revokeOtherSessionsAfterAuthChange(tailCtx, user.ID, keepSessionID(p), "totp enable") + + slog.Info("totp enabled", "user_id", user.ID) + db.WriteAudit(tailCtx, s.st, user.ID, "totp_enabled", "user", user.ID, + "two-factor authentication enrolled") + + res := &TOTPChangeResult{SessionsRevoked: revoked} + if revokeFailed { + // Partial success: 2FA IS enabled; only revoking the other + // sessions failed. A 5xx here would be a lie — the state change + // already committed — so mirror the ChangePassword contract + // (api/profile_handler.go) and report 200 with an explicit warning + // instead of a silent, unqualified 204. + res.Warning = "two-factor authentication enabled, but other sessions could not be revoked; revoke them from the sessions list" + } + return res, nil +} + +// DisableTOTP confirms the password, refuses while the server requires 2FA, +// clears the secret and revokes the caller's other sessions. +func (s *AuthService) DisableTOTP(ctx context.Context, p Principal, password string) (*TOTPChangeResult, error) { + user := p.User + + // BUG-111: Per-user lockout for password confirmation. + lockKey := auth.Key("pw_confirm_lock", user.ID) + if s.limiter.IsLockedOut(lockKey) { + return nil, ErrTooManyAttempts + } + + if err := s.confirmPassword(ctx, user, password, lockKey); err != nil { + return nil, err + } + + require2FA, err := s.require2FAEnabled(ctx) + if err != nil { + return nil, ErrAuthPolicyUnavailable + } + if require2FA { + return nil, ErrTOTPRequiredByServer + } + + s.pending.Delete(user.ID) + if err := s.st.UpdateUserTOTPSecret(ctx, user.ID, nil); err != nil { + return nil, ErrTOTPDisableFailed + } + + // Security tail of the 2FA change: once the secret update committed, + // revoking the other sessions must not be aborted by a dead request. + tailCtx := context.WithoutCancel(ctx) + revoked, revokeFailed := s.revokeOtherSessionsAfterAuthChange(tailCtx, user.ID, keepSessionID(p), "totp disable") + + slog.Info("totp disabled", "user_id", user.ID) + db.WriteAudit(tailCtx, s.st, user.ID, "totp_disabled", "user", user.ID, + "two-factor authentication disabled") + + res := &TOTPChangeResult{SessionsRevoked: revoked} + if revokeFailed { + // Partial success: 2FA IS disabled; only revoking the other + // sessions failed. A 5xx here would be a lie — the state change + // already committed — so mirror the ChangePassword contract + // (api/profile_handler.go) and report 200 with an explicit warning + // instead of a silent, unqualified 204. + res.Warning = "two-factor authentication disabled, but other sessions could not be revoked; revoke them from the sessions list" + } + return res, nil +} + +// confirmPassword is the password-confirmation step the TOTP routes share: a +// missing or wrong password counts against the per-user pw_confirm budget +// and trips the lockout on the threshold; a correct one resets the counter. +func (s *AuthService) confirmPassword(ctx context.Context, user *db.User, password, lockKey string) error { + failKey := auth.Key("pw_confirm_fail", user.ID) + if err := requirePasswordConfirmation(user, password); err != nil { + if !s.limiter.Allow(failKey, PwConfirmFailureThreshold, PwConfirmFailureWindow) { + s.limiter.Lockout(ctx, lockKey, PwConfirmLockoutDuration) + } + return err + } + s.limiter.Reset(ctx, failKey) + return nil +} + +// keepSessionID is the session a 2FA state change keeps alive. BUG-108: an +// API-token principal has a nil session; keep=0 matches no row, so every +// login session is revoked — same semantics as change-password. +func keepSessionID(p Principal) int64 { + if p.Session != nil { + return p.Session.ID + } + return 0 +} + +// revokeOtherSessionsAfterAuthChange revokes every session for userID except +// keepSessionID as the security tail of a committed 2FA state change. It +// mirrors UserService.ChangePassword (service/user.go:262-274): a failure is +// logged and retried once (bounded compensating retry for transient write +// contention); if the retry also fails, revoked reports what did succeed and +// failed is true so the caller can report a partial success instead of +// silently claiming the other sessions were revoked when they were not. +func (s *AuthService) revokeOtherSessionsAfterAuthChange(ctx context.Context, userID, keepSessionID int64, action string) (revoked int64, failed bool) { + revoked, err := s.st.DeleteOtherSessions(ctx, userID, keepSessionID) + if err != nil { + slog.Error("DeleteOtherSessions after "+action, "err", err, "user_id", userID) + revokedRetry, retryErr := s.st.DeleteOtherSessions(ctx, userID, keepSessionID) + if retryErr != nil { + slog.Error("DeleteOtherSessions retry after "+action, "err", retryErr, "user_id", userID) + return revoked, true + } + revoked += revokedRetry + } + if revoked > 0 { + slog.Info("revoked other sessions after "+action, "user_id", userID, "revoked", revoked) + } + return revoked, false +} + +// ─── Helpers moved from api/auth_handler.go ────────────────────────────────── + +func issueSession(ctx context.Context, st Store, userID int64, device, ip string) (string, error) { + token, err := auth.GenerateToken() + if err != nil { + return "", err + } + if _, err := st.CreateSession(ctx, userID, auth.HashToken(token), device, ip); err != nil { + return "", err + } + return token, nil +} + +func (s *AuthService) require2FAEnabled(ctx context.Context) (bool, error) { + return getBooleanSetting(ctx, s.st, "require_2fa", false) +} + +func (s *AuthService) registrationOpen(ctx context.Context) (bool, error) { + return getBooleanSetting(ctx, s.st, "registration_open", true) +} + +func getBooleanSetting(ctx context.Context, st Store, key string, defaultValue bool) (bool, error) { + value, err := st.GetSetting(ctx, key) + if err != nil { + if errors.Is(err, db.ErrNotFound) { + return defaultValue, nil + } + return false, err + } + return parseBooleanSettingValue(value) +} + +func parseBooleanSettingValue(value string) (bool, error) { + switch strings.ToLower(strings.TrimSpace(value)) { + case "1", "true": + return true, nil + case "0", "false": + return false, nil + default: + return false, fmt.Errorf("invalid boolean setting value %q", value) + } +} + +func requirePasswordConfirmation(user *db.User, password string) error { + if password == "" { + return ErrPasswordRequired + } + if !auth.CheckPassword(user.PasswordHash, password) { + return ErrPasswordConfirmationFailed + } + return nil +} diff --git a/docs/architecture/server-boundaries.md b/docs/architecture/server-boundaries.md index 56f1a743..ecb9ef4f 100644 --- a/docs/architecture/server-boundaries.md +++ b/docs/architecture/server-boundaries.md @@ -1,6 +1,9 @@ # Server boundaries — database-call and lifecycle inventory **Written:** 2026-08-29 (B3-0), measured at `dev` `ad4defc2`. +**Re-measured:** 2026-08-30 (B3-2) — the first table and the auth slice's +after-state at `fe1d11b8` (pre-squash; the squash SHA is in the plan's B3-2 +evidence block). **Owner:** the B3 plan, [plans/b3-server-architecture-guardrails-2026-08-29.md](../plans/b3-server-architecture-guardrails-2026-08-29.md). **Regenerate the first table:** `cd Server && go run ./cmd/dbinventory` and @@ -18,7 +21,7 @@ happens to that use — one of four dispositions from the | Disposition | Meaning | Rows | | ----------- | --------------------------------------------------------------------------------------------------------------------- | ---: | -| `move` | persistence or a domain decision that belongs behind a service; **Family** names the service B3-8 (or B3-2) builds | 28 | +| `move` | persistence or a domain decision that belongs behind a service; **Family** names the service B3-8 (or B3-2) builds | 26 | | `adapter` | a transport adapter that uses `db` types or pure helpers only — response shapes, status helpers — no persistence call | 17 | | `boundary` | an explicit composition or transaction boundary that legitimately owns a handle (process entry, CLIs, health probe) | 6 | | `remove` | the import is unnecessary and goes | 0 | @@ -26,8 +29,8 @@ happens to that use — one of four dispositions from the The rows live in code, not only here: `Server/invariants/db_import_boundary.go` holds them as `DBImportAllow`, the `db-import-boundary` rule fails any new importer that has no row, and `TestDBImportAllowIsLive` fails any row whose -file stopped importing `db`. The list only shrinks — B3-2 deletes the two auth -handler rows, B3-8 deletes a family's rows as it moves. +file stopped importing `db`. The list only shrinks — B3-2 deleted the two auth +handler rows (28 → 26 `move`), B3-8 deletes a family's rows as it moves. ## How the measurement works @@ -55,62 +58,60 @@ which is a row worth reading, and none exists today. -| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | -| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | --------------------------------------------------------------- | -| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | -| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | -| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | -| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | -| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | -| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | -| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | -| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | -| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | -| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | -| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | -| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | -| `admin/middleware.go` | `DB×3` `Role` `User` | — | `GetRoleByID` | calls | move | auth | owner gate re-reads the role — OC-0345 | -| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | -| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | -| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | -| `api/auth_handler.go` | `DB×11` `Session` `User×5` | `ErrLastAdmin` `ErrNotFound×2` `IsUniqueConstraintError()` `WriteAudit()×5` | `CreateSession×2` `CreateUserWithInvite` `DeleteAccount` `DeleteSession` `GetSetting` `GetUserByID` `GetUserByUsername` `UpdateUserCustomStatus` | calls | move | auth | B3-2 slice: register/login/logout/delete own the DB | -| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | -| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | -| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | -| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | -| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | -| `api/middleware.go` | `DB` `Role` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | -| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | -| `api/profile_handler.go` | `DB×2` `Session×2` `User×6` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | -| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction leaves in B3-3 | -| `api/totp_handler.go` | `DB×5` `Session×2` `User×4` | `WriteAudit()×3` | `DeleteOtherSessions×2` `GetUserByID` `UpdateUserTOTPSecret×2` | calls | move | auth | B3-2 slice: TOTP enrol/verify write the user row | -| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | -| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | -| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | -| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | -| `main.go` | `AuditWriter×2` `DB×10` | `ErrNotFound` `Migrate()` `NewAuditWriter()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `Close` `DeleteExpiredSessions` `DeleteOrphanedAttachments` `GetMaxEventSeq` `GetSetting` `ResetAllUserStatuses` `SetAuditWriter` `SetSetting` | calls | boundary | — | process composition root; B3-3 moves it to internal/app | -| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | -| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | -| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | -| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | -| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | -| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | -| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | -| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | -| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | -| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | -| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | -| `ws/serve.go` | `ChannelOverride` `DB×9` `PersistedEvent` `User` `VoiceState` | `ConnectStatus()` `StatusOffline` `WriteAudit()` | `GetChannelOverridesFor` `GetRoleByID×4` `GetUserByID×2` `GetUserDMChannelIDs` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `MarkUserDisconnected` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | -| `ws/serve_auth.go` | `DB` `User` | — | `GetSessionByTokenHash` `GetUserByID` | calls | move | auth | session lookup on the WebSocket handshake | -| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | -| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×5` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×4` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetUserDMChannels` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot: channels, overrides, unreads, DMs, members | -| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | -| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | +| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | +| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | --------------------------------------------------------------- | +| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | +| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | +| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | +| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | +| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | +| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | +| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | +| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | +| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | +| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | +| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | +| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | +| `admin/middleware.go` | `DB×3` `Role` `User` | — | `GetRoleByID` | calls | move | auth | owner gate re-reads the role — OC-0345 | +| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | +| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | +| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | +| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | +| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | +| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | +| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | +| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | +| `api/middleware.go` | `DB` `Role` `Session` `User` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | +| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | +| `api/profile_handler.go` | `DB×2` `Session×2` `User×7` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | +| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction leaves in B3-3 | +| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | +| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | +| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | +| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | +| `main.go` | `AuditWriter×2` `DB×10` | `ErrNotFound` `Migrate()` `NewAuditWriter()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `Close` `DeleteExpiredSessions` `DeleteOrphanedAttachments` `GetMaxEventSeq` `GetSetting` `ResetAllUserStatuses` `SetAuditWriter` `SetSetting` | calls | boundary | — | process composition root; B3-3 moves it to internal/app | +| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | +| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | +| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | +| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | +| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | +| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | +| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | +| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | +| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | +| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | +| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | +| `ws/serve.go` | `ChannelOverride` `DB×9` `PersistedEvent` `User` `VoiceState` | `ConnectStatus()` `StatusOffline` `WriteAudit()` | `GetChannelOverridesFor` `GetRoleByID×4` `GetUserByID×2` `GetUserDMChannelIDs` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `MarkUserDisconnected` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | +| `ws/serve_auth.go` | `DB` `User` | — | `GetSessionByTokenHash` `GetUserByID` | calls | move | auth | session lookup on the WebSocket handshake | +| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | +| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×5` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×4` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetUserDMChannels` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot: channels, overrides, unreads, DMs, members | +| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | +| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | -51 files import `db` outside `db/` and `service/` (. 2, admin 16, api 12, auth 2, cmd/seed 1, plugin 1, ws 17); 14 are type-only; 0 unlisted. -Dispositions: adapter 17, boundary 6, move 28. Move targets: auth 9, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. +49 files import `db` outside `db/` and `service/` (. 2, admin 16, api 10, auth 2, cmd/seed 1, plugin 1, ws 17); 14 are type-only; 0 unlisted. +Dispositions: adapter 17, boundary 6, move 26. Move targets: auth 7, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. @@ -216,7 +217,7 @@ close function, which is exactly what B3-3's failure-injection test pins. ## Auth slice — before-state dependency graph The three files B3-2 moves, and what they depend on at `ad4defc2`. The -after-state table is appended by B3-2. +after-state table follows it. | File | Imports (module-internal) | `db` symbols used | | ---------------------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | @@ -224,9 +225,32 @@ after-state table is appended by B3-2. | `api/totp_handler.go` | `auth`, `db` | types `DB`, `Session`, `User`; func `WriteAudit`; methods `DeleteOtherSessions`, `GetUserByID`, `UpdateUserTOTPSecret` | | `auth/*.go` (10 files) | `db` (types only, in `helpers.go`, `resolve.go`), `config`, `syncutil` | types `User`, `Session`, `APIToken`, `Role` — no method calls; `auth` is a leaf that computes and does not persist | -Eleven distinct `*db.DB` methods across the two handlers. That is the upper -bound of the interface `api/auth_deps.go` declares in B3-2; the after-state -row must show the handlers importing neither `db` nor `service` directly. +Eleven distinct `*db.DB` methods across the two handlers (ten after +de-duplicating `GetUserByID`). That was the upper bound set for the interface +`api/auth_deps.go` declares in B3-2. + +### Auth slice — after-state dependency graph + +Measured at `fe1d11b8` (B3-2, pre-squash). The handlers import `db` nowhere; +`api` goes from 12 `db` importers to 10. + +| File | Imports (module-internal) | `db` symbols used | +| ------------------------ | -------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `api/auth_deps.go` | `service` | none — nine-method `AuthService` interface naming `service.Principal`, `RegisterInput`, `LoginInput`, `AuthResult`, `TOTPChangeResult` | +| `api/auth_handler.go` | `auth`, `service` | none — `auth` for `ValidateUsername`/`ValidatePasswordStrength`; `service` for the interface's types, the `Err*` categories `writeAuthError` switches on, and `SanitizeText` (imported before B3-2 too) | +| `api/totp_handler.go` | `auth`, `service` | none — `auth` for `ExtractBearerToken`; `service` for `TOTPChangeResult` and the `Err*` categories | +| `api/middleware.go` | `auth`, `db`, `permissions`, `service` (new) | unchanged row (`move`, auth): gained `principal(r)`, which reads the `*db.User`/`*db.Session` it already stores on the context and hands them to the handlers as `service.Principal` | +| `api/profile_handler.go` | unchanged | unchanged row (`move`, upload): now hosts `userResponse`/`toUserResponse`, the one converter that names `db.User`, which `/me` and the auth responses share with `PATCH /users/me` | +| `service/auth.go` | `auth`, `db`, `permissions` | funcs `WriteAudit`, `IsUniqueConstraintError`; sentinels `ErrLastAdmin`, `ErrNotFound`; the ten methods, through `service.Store`: `CreateSession`, `CreateUserWithInvite`, `DeleteAccount`, `DeleteOtherSessions`, `DeleteSession`, `GetSetting`, `GetUserByID`, `GetUserByUsername`, `UpdateUserCustomStatus`, `UpdateUserTOTPSecret` | +| `auth/ratescale.go` | — | none — the auth rate multiplier, moved from `api/constants.go` so the route mounts and the service's login accounting read one value | + +Honest reading of the plan's target ("handlers importing neither `db` nor +`service` directly"): met for `db`, not for `service`. Both handlers import +`service` because the consumer-owned interface is expressed in the service's +input and result types and its `Err*` values — the alternative, an `api`-side +copy of every type, would have been a second definition of the same shapes. +The dependency direction is still `api → service → db`; what the handlers no +longer see is the database. ## Client baselines are not here diff --git a/docs/architecture/server.md b/docs/architecture/server.md index e501a12b..755a3f10 100644 --- a/docs/architecture/server.md +++ b/docs/architecture/server.md @@ -1,6 +1,7 @@ # Server Architecture -**Verified against:** commit `5630aa1`, 2026-08-04 +**Verified against:** commit `5630aa1`, 2026-08-04; §D4 and the auth rows of +§D3 against `fe1d11b8`, 2026-08-30 (B3-2) Single Go binary (`github.com/J3vb/OwnCord/Server`, Go 1.26). Pure-Go SQLite (`modernc.org/sqlite`, no CGO), chi router, `github.com/coder/websocket`, @@ -121,7 +122,7 @@ sequenceDiagram DB-->>C: JSON response (errorResponse envelope on failure) rect rgba(200,120,120,0.15) - Note over H,DB: Deviation — auth routes: MountAuthRoutes(r, database, …)
bypasses the service layer and queries *db.DB directly.
Admin REST (Server/admin) does the same behind
AdminIPRestrict + RequireAdminAuth. + Note over H,DB: Deviation — Admin REST (Server/admin) queries *db.DB
directly behind AdminIPRestrict + RequireAdminAuth.
The auth routes did the same until B3-2 (see D4). end ``` @@ -136,9 +137,83 @@ channel-less routes on server-wide role permissions via per-channel allow must never open a server-wide gate), while anything channel-scoped is checked in the service layer through `svc.Permissions` / `permissions.Checker`, which resolves overrides and fails closed if they cannot -be fetched. The shaded region marks the two documented bypass paths of the -domain layer. +be fetched. The shaded region marks the remaining documented bypass of the +domain layer; the auth routes left it in B3-2 (`MountAuthRoutes(r, svc, +requireAuth, …)` → `service.AuthService` → `Store`). **Source of truth:** `Server/api/router.go`, `Server/api/middleware.go`, `Server/api/auth_handler.go`, `Server/admin/middleware.go`, `Server/permissions/`, `Server/service/`. + +## D4 — The vertical-slice pattern (B3-2; the rule for B3-8) + +The auth slice was the first domain family moved from "handler owns the +database" to `api → service → db` behind a consumer-owned interface, with a +frozen characterization set proving nothing changed. B3-8 repeats this per +family. The steps, in commit order, each its own commit so the reviewer can +diff the move separately from the rewrite: + +1. **Characterize first, in its own PR.** Table tests over the mounted + router pin today's behaviour per route: status, code and message of every + refusal, session shape, rate-limit accounting, DB-fault paths. A row that + finds a defect is pinned as-is with a `// known:` comment and a ledger + entry — the slice moves behaviour, it does not fix it. Record per-file + statement coverage of the handler files. +2. **List the gates, then grep the rows.** Before writing the interface, + list every handler statement that runs _before_ the body decode (policy + reads, per-user lockouts, "already enrolled", challenge lookups) and grep + the characterization file for malformed-body rows per route (`{not json`). + A route with such a row keeps its gate as a separate interface method, + ahead of the decode; a route without one decodes first, the service runs + the gate in the original order, and the corner case (garbage input behind + a gate now gets 400 instead of the gate's status) goes in the evidence + block. This was the awkward step on the auth slice: `/register` had the + rows (→ `RegistrationPolicy`), the password-confirmation routes did not. +3. **Interface beside the consumer** — `api/_deps.go`, exported, + one method per route call, named in the service's input/result types + (`service.Principal`, `service.Input`, `service.Result`), never in + `db` types, so the file adds no `db` importer. Method count ≤ the distinct + `*db.DB` methods the handlers called; the after-state table records both + numbers. `var _ Service = (*service.Service)(nil)` pins the + production implementation. +4. **Service owns every decision, verbatim.** `service/.go` takes + `Store` and the collaborators the handlers used (the shared + `auth.RateLimiter`, keys, broadcasters). Lockout keys, enumeration + guards, audit writes, best-effort side effects, partial-success contracts + and the limits that size them move line for line, comments included. Each + refusal is a named `Err*` value whose `Error()` is the public message and + whose category (`ErrRateLimited`, `ErrForbidden`, `ErrBadRequest`, + `ErrConflict`, `ErrInternal`, `ErrUnauthorized`, `ErrInvalidInput`) the + transport maps to a status; return them bare and log the cause in the + service, so nothing leaks through `Error()`. Expect one value per distinct + public message — the auth slice had thirty-one. +5. **Handler = decode, one call, encode.** Validation of the body shape and + format (`ValidateUsername`, size bounds, `SanitizeText`) stays in the + handler; everything that reads state moves. One `writeError` + switch on the categories, with the two or three values that carry their + own code listed first. The principal comes from `principal(r)` in + `middleware.go`, never from a `db` type assertion in the handler file. + `MountRoutes` takes the interface and the `AuthMiddleware` the + caller built; `*db.DB` leaves every signature in the file. +6. **The row leaves with the import.** Delete the family's + `invariants.DBImportAllow` rows in the same commit that removes the + import; `TestDBImportAllowIsLive` fails if a row outlives it, and + `db-import-boundary` fails if an import outlives its row. Constants move + with the code that reads them; a converter that still names a `db` type + (`toUserResponse`) moves to a file that legitimately imports `db`. +7. **Composition root builds the service after its collaborators.** The + auth service needs the hub (broadcast) that `service.New` runs before, so + `router.go` constructs it separately, after `ws.NewHub`. B3-3 moves that + into `internal/app`; until then it is one line in `NewRouter`, which sits + at the `funlen` limit — fold, do not add. +8. **Evidence block:** pre-squash SHAs with the characterization run against + each tree, before/after inventory rows, the full gate, and coverage of + the handler files plus the service (blocks merged per file, each counted + once) — the slice must not drop below its before-figure even when the + per-file handler numbers dip, because the unreachable branches are now a + larger share of a smaller file. + +What the pattern does not promise: the handlers import `service` (types and +categories); the crypto-failure paths remain uninjectable inside the service; +and a family whose gates all precede the decode will spend interface methods +on them. diff --git a/docs/plans/README.md b/docs/plans/README.md index 70bdf896..8d99f33d 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -22,7 +22,8 @@ authority**. | [hp-1-scorecard-2026-08-27](hp-1-scorecard-2026-08-27.md) | **HP-1 accepted 2026-08-27.** Structural-diff proofs for the flatten and module rename, plus the B1 exit gate. | | [b2-protocol-trust-compat-2026-08-28](b2-protocol-trust-compat-2026-08-28.md) | **B2 complete — HP-2 accepted 2026-08-29.** B2-0, B2-1, B2-8 done 2026-08-28; B2-2 (B2-3/B2-4 folded in), B2-5, B2-6, B2-7, B2-9 done 2026-08-29. Scorecard below. | | [hp-2-scorecard-2026-08-29](hp-2-scorecard-2026-08-29.md) | **HP-2 accepted 2026-08-29.** Seven questions answered with commands; B2 exit gate, nine conditions met (1 at the slim epoch scope, 4 with one E2EE gap disclaimed). Owner follow-ups that do not gate B3: BPR-051 reader line, SEC-01/SEC-04 advisory IDs. | -| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) in review 2026-08-29. | +| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) and B3-1 (auth characterization) merged 2026-08-29; B3-2 (auth vertical slice) in review 2026-08-30. | +| [hp-3-scorecard-2026-08-29](hp-3-scorecard-2026-08-29.md) | **HP-3 draft 2026-08-30, awaiting the owner's signature.** Five questions on the auth vertical slice answered with commands: frozen set green at every pre-squash SHA, `api` db importers 12 → 10, B2 contracts unchanged, the pattern written as D4 in server.md, guardrails as they exist. | | [audit-2026-08-19-remediation](audit-2026-08-19-remediation.md) | Phases 1–6 done 2026-08-20; **phase 7 pending**. Its header still reads "in progress 2026-08-19" — stale; the phase table is correct. | ## Partially implemented diff --git a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md index d57a5a4b..f90af996 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -6,7 +6,7 @@ verified at `bf7b886d` **Status:** in progress — plan merged 2026-08-29 (PR #1447 = `ad4defc2`); B3-0 merged 2026-08-29 (PR #1448 = `d383d8c7`; closes entry-gate item 3); -B3-1 in progress 2026-08-29. +B3-1 merged 2026-08-29 (PR #1449 = `71d867cb`); B3-2 in progress 2026-08-30. Update this line, not only the step table, when a step lands. Primary inputs: @@ -34,7 +34,7 @@ surface to it. | Step | What | Size | Parallel with | | -------- | ---------------------------------------------------------------------------------------------------------------------------------- | -------- | ---------------------------------------------------------------- | | **B3-0** | Boundary inventory: every upper-layer `db` import with a disposition; hub lifecycle; before-graph — **DONE 2026-08-29 (PR #1448)** | 1–2 days | B3-6, B3-7 | -| **B3-1** | Auth characterization tests — enumeration, sentinels, sessions, TOTP, rate limits, failure paths | 1 day | B3-6, B3-7 | +| **B3-1** | Auth characterization tests — enumeration, sentinels, sessions, TOTP, rate limits, failure paths — **DONE 2026-08-29 (PR #1449)** | 1 day | B3-6, B3-7 | | **B3-2** | The auth vertical slice (S-10): route → `service.AuthService` → `db`, behaviour-neutral | 2–3 days | B3-6, B3-7 | | **HP-3** | First vertical-slice review — scorecard | — | — | | **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract | 1–2 days | B3-4 | @@ -233,7 +233,7 @@ Exit: the characterization file is green on HEAD; its row count and the two files' coverage are in the evidence block. One PR. **Evidence, 2026-08-29** — branch `feat/b3-1-auth-characterization` from `dev` -`d383d8c7`; PR to `dev` recorded below. +`d383d8c7`; PR #1449 to `dev`, squash-merged 2026-08-29 as `71d867cb`. - **Inventory.** The three existing files hold **85** tests (`auth_handler_test.go` 61, `totp_handler_test.go` 22, `auth_handler_delete_broadcast_test.go` 2 — @@ -316,8 +316,8 @@ TO x_gone` for read faults, `RAISE(FAIL)` triggers for write faults), so no 83.3% (`GenerateToken` failure). - **Pre-squash SHAs:** `659c8cbd` (B3-0 SHA recorded), `0905a942` (inventory - table), `b7317d03` (characterization file + ledger + claim updates), plus the - coverage commit that carries this bullet. + table), `b7317d03` (characterization file + ledger + claim updates), `a0356ee1` + (this coverage bullet); Codex rounds `7c38c2bd`, `bf49453c`, `8614603b` (head). - Gates before every commit: `check:docs`, `check:hygiene`; from `Server/` the four build-tag variants, `go vet`, `go test -race ./...`, `go test -tags deadlock ./ws/`, `golangci-lint run` (first run tripped `prealloc` and @@ -359,6 +359,119 @@ rewrite, remove old path — with B3-1's tests green after every commit. Exit: HP-3. +**Evidence, 2026-08-30** — branch `feat/b3-2-auth-slice` from `dev` +`71d867cb`; PR #1450 to `dev` (squash SHA recorded at merge). + +- **Pre-squash SHAs**, one per numbered item. For each, the characterization + file was run against that exact tree in a detached worktree + (`go test -count=1 -run TestAuthCharacterization ./api/`) and the frozen + files (`auth_characterization_test.go`, `totp_handler_test.go`) diffed + against `71d867cb`: + + | Commit | Item | Characterization | Frozen files vs `71d867cb` | + | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------- | ---------------- | -------------------------- | + | `825b406f` | B3-1's merge SHA recorded in this plan | ok 1.860 s | identical | + | `448c50f6` | 1 — `api/auth_deps.go`: the consumer-owned interface, and the input/result types it names in `service/auth.go` | ok 1.852 s | identical | + | `24ed138d` | 2 — `service/auth.go`: orchestration moved verbatim, the `Err*` set; `auth/ratescale.go` | ok 1.854 s | identical | + | `fe1d11b8` | 3 — thin handlers; `MountAuthRoutes(r, svc, requireAuth, limiter, proxies)`; `router.go` builds the service; two `DBImportAllow` rows deleted | ok 1.849 s | identical (see wiring) | + | `3f0d24ec` | 4 — after-state inventory and graph rows in `server-boundaries.md` | ok 1.832 s | identical | + | this commit | 5 — this block | docs only | — | + +- **The interface.** Nine methods — `RegistrationPolicy`, `Register`, + `Login`, `VerifyTOTP`, `Logout`, `DeleteAccount`, `EnableTOTP`, + `ConfirmTOTP`, `DisableTOTP` — against the ten distinct `*db.DB` methods, + two `db` functions and two `db` sentinels the handlers called at + `71d867cb`. `RegistrationPolicy` is the ninth: two characterization rows + pin `/register`'s 403 "before any credential is read" (a malformed body + still receives the policy's 403), so that gate stays ahead of the body + decode as its own call. `service.AuthService` is constructed with the + `Store` (`*db.DB`), the shared `auth.RateLimiter`, the TOTP key and the + `AuthBroadcaster`; it creates the partial-login, pending-TOTP and used-code + stores itself with the fixed TTLs the route mount used to own. +- **Error semantics** (item 4): every refusal is a named `service.Err*` value + whose `Error()` is the exact public message the handler wrote and whose + category (`ErrRateLimited`, `ErrForbidden`, `ErrBadRequest`, `ErrConflict`, + `ErrInternal`, plus the new `ErrUnauthorized` and `ErrInvalidInput`) the one + `writeAuthError` switch maps to a status and code; two values carry their + own code (`INVALID_CREDENTIALS`, `TOTP_ALREADY_ENABLED`). Thirty-one values, + because the pre-slice handlers had thirty-one distinct refusal triples. No + sentinel-mapping row changed. +- **Behaviour notes** — no row changed; listed because HP-3 Q1 asks: + 1. _Decode before gate on five routes._ At `71d867cb`, `DELETE /account`, + the three TOTP-management routes and `/verify-totp` ran a gate (per-user + lockout; already-enrolled; challenge lookup) before decoding the body. A + thin handler decodes first and the service runs the same gates in the + same order. Observable only for a _malformed_ body behind a gate: + locked-out + malformed → 400 (was 429); enrolled + malformed enable → + 400 (was 409); unknown partial token + malformed verify body → 400 (was + 401). No frozen row exercises those; every well-formed request is + byte-identical. `/register` kept its gate first (above). + 2. _One `AuthMiddleware` for the six authenticated auth routes._ The caller + builds it once and passes it in; before, each `r.With(AuthMiddleware(database))` + had its own `last_used` touch throttle. Fewer writes, same semantics. + 3. `confirmPassword` folds the three identical password-confirmation blocks + of the TOTP routes into one method (verbatim otherwise; the accounting + — a missing or wrong password counts, a correct one resets — is + unchanged). + 4. The auth-slice limits left `api/constants.go` with the code that reads + them; the shared `pw_confirm` budget is exported as `service.PwConfirm*` + and `profile_handler.go` reads it there. The auth rate multiplier moved + to `auth/ratescale.go` (`SetRateScale`, `ScaledLimit`) so the route + mounts and the service's login accounting read one value; `api` keeps + `setAuthRateScale`/`scaledAuthLimit` as wrappers, and + `TestPerUserFailureCapsStayUnscaled` now pins `../service/auth.go`. + 5. `userResponse`/`toUserResponse` moved to `profile_handler.go` (the other + consumer, which still imports `db`); `middleware.go` gained + `principal(r)`, which hands the handlers the context's `*db.User` and + `*db.Session` as `service.Principal`. +- **Test files changed** — wiring only (`git diff -U0 71d867cb fe1d11b8 -- '*_test.go'`): + `auth_handler_test.go` (+1 import, the one mount line in + `buildAuthRouterWithProxies`), `auth_handler_delete_broadcast_test.go` (+1 + import, two mount lines, one comment), `coverage_push_test.go` and + `invite_handler_test.go` (one mount line each), `constants_test.go` (the + scale tests follow the constant), `router_delete_account_broadcast_test.go` + (comment and failure message), `invariants/db_import_boundary_test.go` + (fixture path). `auth_characterization_test.go` and `totp_handler_test.go`: + untouched. No assertion, row or expected value moved. +- **Graph tables:** [server-boundaries.md](../architecture/server-boundaries.md) + §"Auth slice" — before at `ad4defc2`, after at `fe1d11b8`. `api` `db` + importers **12 → 10**, `move` rows 28 → 26, 51 → 49 files. The handlers + import `service` (the interface's types, the `Err*` categories, + `SanitizeText`), so the plan's "neither `db` nor `service`" is met for + `db` only — stated there, not bent. +- **Gates**, before every commit per `ci-check`: from `Server/` the four + build-tag variants, `go vet ./...`, `go test -race ./...` (18 packages + ok), `go test -tags deadlock -count=1 ./ws/` (ok, 59.9 s), + `golangci-lint run` (0 issues — one `funlen` on `NewRouter` at 101 lines + during item 3, fixed by folding the constructor into the mount call), + `sqlc generate` and `genprotocol` drift clean, `go test ./invariants/` ok; + from the root `check:docs` and `check:hygiene`. + `go test -count=1 -race ./api/ ./service/ ./auth/` at `3f0d24ec`: + `ok api 51.447s`, `ok service 84.403s`, `ok auth 1.802s`. +- **Coverage** (statements; `go test -coverprofile` blocks merged per file — + before is `-coverprofile ./api/` at `71d867cb`, after is the same run for + the handler files plus `-coverpkg=./api/,./service/ ./api/ ./service/` for + the slice, each profile block counted once): + + | File | Before (`71d867cb`) | After (`fe1d11b8`) | + | ------------------------------ | ------------------- | ------------------- | + | `api/auth_handler.go` | 229/254 = 90.2% | 98/114 = 86.0% | + | `api/totp_handler.go` | 163/179 = 91.1% | 54/60 = 90.0% | + | `service/auth.go` | — | 240/253 = 94.9% | + | **slice (handlers + service)** | **392/433 = 90.5%** | **392/427 = 91.8%** | + + The same 392 statements are exercised after the move; the total shrank by + six (the folded confirmation blocks). The handler files' percentages dip + because their unreachable branches are now a larger share of a smaller + file: `writeNotAuthenticated` (the no-principal branch behind + `AuthMiddleware` — B3-1's `handleMe` note), `writeAuthError`'s default (a + service contract bug, unreachable by construction), and + `registerReadRequest`/`loginReadRequest`, unchanged at 78.9%/83.3% (B3-1's + note: those branches are `auth/` tests). The service's gaps are B3-1's + not-injectable set: `Register` 85.0% (hash failure, post-commit + `GetUserByID`), `issueSession` 83.3% (`GenerateToken`), `Logout` 90.0% (the + nil-session guard the handler answers first). + ## HP-3 — First vertical-slice review `docs/plans/hp-3-scorecard-.md`, in the HP-2 shape. Questions: diff --git a/docs/plans/hp-3-scorecard-2026-08-29.md b/docs/plans/hp-3-scorecard-2026-08-29.md new file mode 100644 index 00000000..9d221287 --- /dev/null +++ b/docs/plans/hp-3-scorecard-2026-08-29.md @@ -0,0 +1,231 @@ +# HP-3 — First vertical-slice review scorecard + +**Hold point:** HP-3, defined in +[b3-server-architecture-guardrails-2026-08-29.md](b3-server-architecture-guardrails-2026-08-29.md) +§HP-3 (roadmap +[repo-health-roadmap-2026-08-23.md](repo-health-roadmap-2026-08-23.md), B3) +**Commits reviewed:** the pre-squash commits of #1450 (B3-2; table below), on top of B3-0 (#1448 = `d383d8c7`) and B3-1 (#1449 = `71d867cb`) +**Measured at:** `fe1d11b8` (the handler-move commit) and `3f0d24ec` (the +after-state inventory), branch `feat/b3-2-auth-slice` off `dev` `71d867cb` +**Measured:** 2026-08-30 +**Evidence base:** the plan's B3-0, B3-1 and B3-2 evidence blocks; +[server-boundaries.md](../architecture/server-boundaries.md) §"Auth slice"; +[server.md](../architecture/server.md) §D4 + +**Decision: DRAFT — awaiting the owner's signature (line at the end).** + +HP-3 asks five questions. Each is answered below with the command that +produces the evidence and what it printed on the measured tree, not with an +assertion. It follows the shape of +[hp-2-scorecard-2026-08-29.md](hp-2-scorecard-2026-08-29.md). Acceptance +authorises B3-3 onward and B3-8's per-family repeats of the pattern; it +claims nothing about beta readiness. + +## The commits under review are not on `dev` + +`dev` is squash-merge only. The structure HP-3 reviews — one commit per +numbered item of §B3-2, characterization green after each — survives only on +the pull-request ref: + +```bash +git fetch origin 'refs/pull/1450/head:pr-1450' +``` + +| Commit | §B3-2 item | +| ----------- | -------------------------------------------------------------------------------------------------------------------------- | +| `825b406f` | — B3-1's merge SHA `71d867cb` recorded in the plan | +| `448c50f6` | 1 — `api/auth_deps.go`, the consumer-owned `AuthService` interface; its input/result types in `service/auth.go` | +| `24ed138d` | 2 — `service/auth.go`, the orchestration moved verbatim and the `Err*` set; `auth/ratescale.go` | +| `fe1d11b8` | 3 — thin handlers, `MountAuthRoutes` takes the interface, `router.go` builds the service, two `DBImportAllow` rows deleted | +| `3f0d24ec` | 4 — after-state inventory and graph rows in `server-boundaries.md` | +| `1077a992` | 5 — the evidence block | +| this commit | 6 — this scorecard and the pattern section in `server.md` | + +## Question 1 — did the slice move behaviour without changing it? + +**The proof is the frozen set.** `Server/api/auth_characterization_test.go` +(12 tests, 44 rows) and the 85 tests in `auth_handler_test.go`, +`totp_handler_test.go` and `auth_handler_delete_broadcast_test.go` were +written before a line of the slice moved (B3-1) and did not change here. + +```bash +# each pre-squash SHA, in a detached worktree of that exact tree +cd Server && go test -count=1 -run TestAuthCharacterization ./api/ +git diff --stat 71d867cb -- Server/api/auth_characterization_test.go Server/api/totp_handler_test.go +``` + +| SHA | Characterization | Frozen files vs `71d867cb` | +| ---------- | ---------------- | -------------------------- | +| `825b406f` | ok 1.860 s | identical | +| `448c50f6` | ok 1.852 s | identical | +| `24ed138d` | ok 1.854 s | identical | +| `fe1d11b8` | ok 1.849 s | identical | +| `3f0d24ec` | ok 1.832 s | identical | + +The 85 tests: `go test -count=1 -race ./api/` at `3f0d24ec` — `ok 51.447s`, +the whole package. Of the frozen files, `auth_handler_test.go` and +`auth_handler_delete_broadcast_test.go` changed only where they mount the +routes (one helper line, two direct mounts, one import each, one comment — +`git diff -U0 71d867cb fe1d11b8 -- Server/api/auth_handler_test.go Server/api/auth_handler_delete_broadcast_test.go`); +no assertion or expected value moved. + +**The sentinel-mapping table is byte-identical.** Every refusal the handlers +wrote at `71d867cb` — thirty-one distinct (status, code, message) triples — +is now a named `service.Err*` value whose `Error()` is that message and whose +category the one `writeAuthError` switch maps back to that status and code. +The rows that pin them (`RegisterRejectionsAreIndistinguishable`, +`LoginRejectionsAreIndistinguishable`, `LoginFailurePaths`, +`DeleteAccountFailurePaths`, `VerifyTOTPFailurePaths`, +`TOTPManagementFailurePaths`, `RouteRateLimits`) assert status, code and +message with `wantErr` and are green above. The three `// known:` rows +(OC-0376, OC-0377, OC-0378) are still pinned as defects — the slice moved +them, it did not fix them; B3-9 flips each row in the commit that fixes it. + +**What did change, recorded rather than hidden** (the plan's B3-2 evidence +block, "Behaviour notes"): five routes that ran a gate before decoding the +body now decode first and let the service run the same gates in the same +order. The only observable difference is a _malformed_ body arriving behind +a gate (locked-out, already-enrolled, unknown challenge): it is refused as +400 where it was refused as the gate's 429/409/401. No row exercises that +input; every well-formed request is byte-identical. `/register` kept its +gate first because two rows pin "403 before any credential is read" — that +is why the interface has a ninth method, `RegistrationPolicy`. The six +authenticated auth routes share one `AuthMiddleware` instance (one +`last_used` throttle instead of six). + +**Verdict: PASS** — the frozen behaviour set is unchanged and green at every +commit; the one ordering change is confined to malformed input behind a gate +and is written down. + +## Question 2 — did it reduce coupling? + +```bash +cd Server && go run ./cmd/dbinventory | tail -2 +# before (ad4defc2, B3-0): 51 files import db ... api 12 ...; Dispositions: adapter 17, boundary 6, move 28 +# after (fe1d11b8): 49 files import db ... api 10 ...; Dispositions: adapter 17, boundary 6, move 26 +go test -count=1 ./invariants/ # ok — db-import-boundary and TestDBImportAllowIsLive at the new list +``` + +| Measure | Before (`ad4defc2`) | After (`fe1d11b8`) | +| ------------------------------------------- | ------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | +| `db` importers in `api` | 12 | **10** | +| `DBImportAllow` `move` rows | 28 | 26 | +| `db` symbols the two handlers touched | 3 types, 2 funcs, 2 sentinels, **10 distinct `*db.DB` methods** (11 call sites) | **none** — `auth_handler.go` and `totp_handler.go` import `auth` and `service` only | +| Interface the handlers call | — | `api.AuthService`, **9 methods** (`api/auth_deps.go`) | +| Where the ten `db` methods live | the handlers | `service/auth.go`, through `service.Store` | +| Handler size (statements, `go test -cover`) | `auth_handler.go` 254, `totp_handler.go` 179 | 114 and 60; `service/auth.go` 253 | +| Direction | `api → db` (handlers) beside `api → service → db` (the rest) | `api → service → db` for the whole slice | + +The full before/after tables are `server-boundaries.md` §"Auth slice — +before-state dependency graph" and §"after-state dependency graph". The +after-state also records what is _not_ met: the plan hoped the handlers would +import neither `db` nor `service`; they import `service` for the interface's +input/result types, the `Err*` categories and `SanitizeText`. The +alternative — an `api`-side copy of every type — would be a second definition +of the same shapes, so the dependency is kept and named. + +**Verdict: PASS** — nine methods replace ten database methods plus four +symbols; two importers gone; the rule that proves it (`db-import-boundary`) +is green with the rows deleted in the same commit as the imports. + +## Question 3 — did it weaken a B2 contract? + +Nothing under `ws/`, `protocol/`, `permissions/` or the fixtures changed on +the branch (`git diff --stat 71d867cb 3f0d24ec -- Server/ws Server/permissions protocol` is empty). The +tests HP-2 accepted, run at `3f0d24ec`: + +```bash +cd Server && go test -count=1 -run 'TestEpoch1Fixtures|TestAuth_ProtocolEpoch|TestAbsenceContract_|Parity' -v ./ws/ ./api/ ./service/ +``` + +| Contract | Test | Result | +| ------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| Epoch-1 fixtures replay, `auth-failure` | `TestEpoch1Fixtures/auth-failure` | PASS | +| Epoch-1 fixtures replay, `fresh-connect` | `TestEpoch1Fixtures/fresh-connect` (and the other nine: ping, chat-send-fanout, chat-edit-delete, reaction-add-remove, typing, mark-read, dm-send, resume-replay, voice-join-e2ee-leave) | PASS ×11 | +| Epoch negotiation matrix | `TestAuth_ProtocolEpoch` | PASS | +| Absence (federation / directory / listing) | `TestAbsenceContract_NoFederationDirectoryOrListingRoutes`, `…WireTypes`, `…ConfigKeys` | PASS ×3 | +| Predicate parity | `TestSendPolicyParity`, `TestViewPolicyParity` (service); `TestRefreshChannelVisibilityCanSend_Parity`, `TestApplySetChannelID_Parity`, `TestChannelReadAudience_Parity`, `TestChannelSubject_Parity`, `TestVoiceJoinPrecheck_Parity`, `TestVoiceStillAllowed_Parity`, `TestRefreshChannelVisibility_Parity` (ws) | PASS ×9 | + +`ok ws 7.658s`, `ok api 1.423s`, `ok service 0.717s`. The auth rate +multiplier moved packages (`auth.SetRateScale`/`ScaledLimit`), and +`api/constants_test.go`'s clamp and floor tests still run through `api`'s +wrappers — `TestSetAuthRateScale_ClampsMultiplier`, +`TestScaledAuthLimit_NeverBelowOne`, `TestPerUserFailureCapsStayUnscaled` +green in the `-race` run above. + +**Verdict: PASS** — every B2 contract test is unchanged and green. + +## Question 4 — is the pattern repeatable? + +Written down as the rule for B3-8 in +[server.md](../architecture/server.md) §"D4 — The vertical-slice pattern": +interface beside the consumer → service owns every decision → handler is +decode, one call, encode → the allowlist row leaves with the import → the +limits and converters move with the code that reads them → the composition +root builds the service after the collaborators it needs. + +**The one thing that was awkward, named honestly:** _gate-before-decode._ +Six of the nine routes checked something before reading the body — a policy, +a per-user lockout, "already enrolled", a challenge lookup. A thin handler +cannot keep that order without a second interface method per route, and the +interface had a method-count cap. The characterization file settled it +route by route: `/register` had two rows pinning the gate ahead of the body +(a malformed body still gets the policy's 403), so its gate became the ninth +method; the other five had none, decode first, and the corner cases are in +the evidence block. The general rule (D4, step 2): _before writing the +interface, list every statement that precedes the body decode and grep the +characterization file for malformed-body rows per route._ B3-8's families +will meet the same shape (every "confirm your password" route has it). + +A smaller cost worth knowing before the next family: thirty-one named error +values, because the pre-slice handlers had thirty-one distinct public +messages and the rows pin them byte for byte. That is the price of +"verbatim"; it is also a list a later phase can consolidate on purpose. + +**Verdict: PASS** — the shape is written as steps with the awkward step +named, and every step was exercised once on this slice. + +## Question 5 — are the B3-6 guardrails green on the slice's SHA? + +B3-6 did not land in parallel: no guardrail PR merged to `dev` between +`71d867cb` and this branch. What exists today, run at `fe1d11b8`: + +| Guardrail | State | +| ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `db-import-boundary` (B3-0, `Server/invariants/`) | **green** — `go test -count=1 ./invariants/` ok with the two rows deleted; `go run ./cmd/dbinventory` exits 0, "0 unlisted" | +| Coverage floor (S-06, B3-6 item 1) | **not yet a gate** — measured by hand instead: slice 392/433 = 90.5% → 392/427 = 91.8% (B3-2 evidence block); the same 392 statements exercised. B3-6 turns the measurement into a check | +| Hub simulation / fault transport / fuzz seeds | **not landed** — nothing to run; the slice touches no `ws/` file, so no new surface for them either | +| The full server gate | **green** before every commit: four build-tag variants, vet, `-race` (18 packages), `-tags deadlock ./ws/`, `golangci-lint` 0 issues, sqlc/genprotocol drift clean, `check:docs`, `check:hygiene` | + +**Verdict: PASS for the guardrail that exists; the rest is B3-6's own exit, +not this slice's.** Recorded so the exit gate's condition 5 can point at +this table and at B3-6's later run on the same SHA. + +## Open items carried past HP-3 + +Recorded, not fixed. None blocks B3-3. + +| Item | State | +| -------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| OC-0376, OC-0377, OC-0378 (B3-1's `// known:` rows) | **B3-9, now unblocked** — the orchestration they live in is `service.AuthService`; each fix flips its row in the same commit | +| Decode-before-gate corner cases (malformed body behind a gate) | **recorded** in the B3-2 evidence block; not a defect, a shape. If a later phase wants the gate's status back for garbage input, it is one more interface method | +| Handlers import `service` | **accepted** — types and error categories, not persistence; `server-boundaries.md` says so | +| B3-1's not-injectable set (crypto failures, post-commit fetch) | **still not injectable** — they sit inside the service now; a `Store` double would reach the post-commit `GetUserByID`, `auth.GenerateToken` still has no seam | +| Thirty-one auth `Err*` values | **by design** (verbatim); consolidation would be a behaviour change and a later phase's decision | +| Coverage floor as a gate | **B3-6** — the per-file block-merge measurement in the evidence block is the method to encode | + +## What acceptance does and does not authorise + +Accepting HP-3 authorises B3-3 (lifecycle extraction), B3-4, B3-5 and the +per-family repeats in B3-8 to proceed with the D4 pattern. It does **not** +claim: + +- that the auth slice is bug-free — three known defects are pinned and + scheduled (B3-9); +- that every handler is thin — ten `api` files still import `db`, each with + a disposition in `server-boundaries.md`; +- that the coverage floor is enforced — it is measured, B3-6 makes it a + gate; +- that the error set is final — it is verbatim. + +**Signed:** \_\_\_\_ (repository owner), 2026-\_\_-\_\_ —