From 0e41ccd95a2fb9fcfa9566cd73ef6f9fb5a9aada Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 18 Jul 2026 16:38:03 -0400 Subject: [PATCH 1/2] Add local auth: users, sessions, register/login/logout/me (#4) Implements pansy's local (email + password) authentication and the session layer that OIDC (#5) will also reuse. - store: users.go (create/get-by-id/get-by-email/count) and sessions.go (create/get/touch/delete/delete-expired), scanning the existing 0001 schema. - service: the business-logic seam. auth.go (Register/Login/session lifecycle/Providers) + password.go (argon2id, 64 MiB/1/4, PHC-encoded, constant-time verify) + service.go (Service, clock injection, token hashing). First user is admin; closed registration still allows the bootstrap user; unknown-email and wrong-password are indistinguishable (same error, same argon2 work via a dummy hash). - api: POST /auth/register|login|logout, GET /auth/me|providers, plus a requireAuth middleware that resolves the HttpOnly session cookie (SameSite=Lax, Secure under https) to the actor. Handlers stay thin. - main: wires the service and a periodic expired-session sweep; sessions are also dropped lazily on access. Sliding 30-day expiry. - tests: service (register/login/expiry/renewal/cleanup, password) and api (cookie flow, middleware, validation, providers). Verified end-to-end via curl: register -> me -> restart -> session persists -> logout -> 401. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi --- README.md | 2 +- cmd/pansy/main.go | 33 +++- go.mod | 2 +- internal/api/api.go | 28 ++- internal/api/auth.go | 180 ++++++++++++++++++++ internal/api/auth_test.go | 168 ++++++++++++++++++ internal/domain/domain.go | 18 ++ internal/service/auth.go | 186 ++++++++++++++++++++ internal/service/auth_test.go | 272 ++++++++++++++++++++++++++++++ internal/service/password.go | 82 +++++++++ internal/service/password_test.go | 54 ++++++ internal/service/service.go | 76 +++++++++ internal/store/sessions.go | 80 +++++++++ internal/store/users.go | 101 +++++++++++ 14 files changed, 1275 insertions(+), 7 deletions(-) create mode 100644 internal/api/auth.go create mode 100644 internal/api/auth_test.go create mode 100644 internal/service/auth.go create mode 100644 internal/service/auth_test.go create mode 100644 internal/service/password.go create mode 100644 internal/service/password_test.go create mode 100644 internal/service/service.go create mode 100644 internal/store/sessions.go create mode 100644 internal/store/users.go diff --git a/README.md b/README.md index e4ffd6d..dd79f6d 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ All configuration is via environment variables; every value has a default, so `. | `PANSY_OIDC_BUTTON_LABEL` | `Sign in with SSO` | Label for the OIDC button on the login page. | | `PANSY_TRUSTED_PROXIES` | *(none)* | Comma-separated proxy CIDRs/IPs to trust for client-IP resolution. | -Auth variables are read at startup now and consumed as the auth features land (issues #4/#5). +Local email/password auth is live (`POST /api/v1/auth/register`, `/auth/login`, `/auth/logout`, `GET /auth/me`, `GET /auth/providers`); the session is an HttpOnly cookie (`Secure` when `PANSY_BASE_URL` is https). The first account registered becomes admin, and it may register even when `PANSY_REGISTRATION=closed` to bootstrap the instance. OIDC (`PANSY_OIDC_*`) lands in #5. ## Docker & deployment diff --git a/cmd/pansy/main.go b/cmd/pansy/main.go index f4f92ac..1a980de 100644 --- a/cmd/pansy/main.go +++ b/cmd/pansy/main.go @@ -16,10 +16,15 @@ import ( "gitea.stevedudenhoeffer.com/steve/pansy/internal/api" "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/service" "gitea.stevedudenhoeffer.com/steve/pansy/internal/store" "gitea.stevedudenhoeffer.com/steve/pansy/internal/webdist" ) +// sessionSweepInterval is how often expired sessions are purged in the +// background (they are also dropped lazily on access). +const sessionSweepInterval = 6 * time.Hour + func main() { slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelInfo}))) @@ -47,7 +52,9 @@ func run() error { } slog.Info("pansy: database ready", "path", cfg.DBPath) - r := api.New(cfg) + svc := service.New(db, cfg) + + r := api.New(cfg, svc) api.RegisterSPA(r, webdist.Dist()) srv := &http.Server{ @@ -71,6 +78,8 @@ func run() error { ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() + go sweepSessions(ctx, svc) + select { case err := <-serveErr: return fmt.Errorf("http server: %w", err) @@ -85,3 +94,25 @@ func run() error { } return nil } + +// sweepSessions periodically purges expired sessions until ctx is cancelled. It +// runs one sweep immediately so a long-lived process doesn't wait a full +// interval after startup. Failures are logged, never fatal. +func sweepSessions(ctx context.Context, svc *service.Service) { + ticker := time.NewTicker(sessionSweepInterval) + defer ticker.Stop() + for { + if n, err := svc.CleanupExpiredSessions(ctx); err != nil { + if ctx.Err() == nil { + slog.Warn("pansy: session sweep failed", "error", err) + } + } else if n > 0 { + slog.Info("pansy: purged expired sessions", "count", n) + } + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + } +} diff --git a/go.mod b/go.mod index c595a2f..51d77d4 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.26.2 require ( github.com/gin-gonic/gin v1.10.1 github.com/samber/slog-gin v1.15.0 + golang.org/x/crypto v0.31.0 modernc.org/sqlite v1.34.4 ) @@ -36,7 +37,6 @@ require ( go.opentelemetry.io/otel v1.29.0 // indirect go.opentelemetry.io/otel/trace v1.29.0 // indirect golang.org/x/arch v0.8.0 // indirect - golang.org/x/crypto v0.31.0 // indirect golang.org/x/net v0.33.0 // indirect golang.org/x/sys v0.28.0 // indirect golang.org/x/text v0.21.0 // indirect diff --git a/internal/api/api.go b/internal/api/api.go index 721dcac..be57af0 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -13,13 +13,21 @@ import ( sloggin "github.com/samber/slog-gin" "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/service" ) +// handlers carries the dependencies shared by every HTTP handler. Handlers stay +// thin: decode the request, call a service method, encode the result. +type handlers struct { + cfg *config.Config + svc *service.Service +} + // New builds the gin engine with the standard middleware stack and registers the -// API routes. The embedded SPA fallback is registered separately by the caller -// via RegisterSPA (see spa.go) so the API can be built and tested without a web -// build present. -func New(cfg *config.Config) *gin.Engine { +// API routes against the given service. The embedded SPA fallback is registered +// separately by the caller via RegisterSPA (see spa.go) so the API can be built +// and tested without a web build present. +func New(cfg *config.Config, svc *service.Service) *gin.Engine { gin.SetMode(gin.ReleaseMode) r := gin.New() @@ -33,9 +41,21 @@ func New(cfg *config.Config) *gin.Engine { _ = r.SetTrustedProxies(nil) } + h := &handlers{cfg: cfg, svc: svc} + v1 := r.Group("/api/v1") v1.GET("/healthz", healthz) + // Auth endpoints are exempt from requireAuth (you can't be logged in yet); + // /me is the one that needs a session. Feature routers in later issues attach + // h.requireAuth() to their own protected groups. + auth := v1.Group("/auth") + auth.POST("/register", h.register) + auth.POST("/login", h.login) + auth.POST("/logout", h.logout) + auth.GET("/providers", h.providers) + auth.GET("/me", h.requireAuth(), h.me) + return r } diff --git a/internal/api/auth.go b/internal/api/auth.go new file mode 100644 index 0000000..69a7e0a --- /dev/null +++ b/internal/api/auth.go @@ -0,0 +1,180 @@ +package api + +import ( + "errors" + "log/slog" + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/service" +) + +// sessionCookie is the name of the HttpOnly cookie carrying the raw session token. +const sessionCookie = "pansy_session" + +// actorKey is the gin-context key under which requireAuth stashes the +// authenticated *domain.User. +const actorKey = "actor" + +// registerRequest / loginRequest are the JSON bodies for the local-auth +// endpoints. gin's binding validates them before the service is called. +type registerRequest struct { + Email string `json:"email" binding:"required,email"` + DisplayName string `json:"displayName" binding:"required"` + Password string `json:"password" binding:"required,min=8,max=1024"` +} + +type loginRequest struct { + Email string `json:"email" binding:"required,email"` + Password string `json:"password" binding:"required"` +} + +// register creates a local account and logs it in (sets the session cookie). +func (h *handlers) register(c *gin.Context) { + var req registerRequest + if err := c.ShouldBindJSON(&req); err != nil { + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "email, display name, and an 8+ character password are required") + return + } + + user, err := h.svc.Register(c.Request.Context(), service.RegisterInput{ + Email: req.Email, + DisplayName: req.DisplayName, + Password: req.Password, + }) + if err != nil { + h.writeServiceError(c, err) + return + } + + if err := h.startSession(c, user.ID); err != nil { + writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "could not start session") + return + } + c.JSON(http.StatusOK, user) +} + +// login verifies credentials and sets the session cookie. +func (h *handlers) login(c *gin.Context) { + var req loginRequest + if err := c.ShouldBindJSON(&req); err != nil { + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "email and password are required") + return + } + + user, err := h.svc.Login(c.Request.Context(), req.Email, req.Password) + if err != nil { + h.writeServiceError(c, err) + return + } + + if err := h.startSession(c, user.ID); err != nil { + writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "could not start session") + return + } + c.JSON(http.StatusOK, user) +} + +// logout deletes the current session (if any) and clears the cookie. It is +// idempotent and never requires a valid session. +func (h *handlers) logout(c *gin.Context) { + if token, err := c.Cookie(sessionCookie); err == nil && token != "" { + if err := h.svc.Logout(c.Request.Context(), token); err != nil { + slog.Error("api: logout", "error", err) + } + } + h.clearSessionCookie(c) + c.JSON(http.StatusOK, gin.H{"ok": true}) +} + +// me returns the authenticated user. It sits behind requireAuth. +func (h *handlers) me(c *gin.Context) { + c.JSON(http.StatusOK, mustActor(c)) +} + +// providers reports which login methods to render on the login page. +func (h *handlers) providers(c *gin.Context) { + c.JSON(http.StatusOK, h.svc.Providers()) +} + +// requireAuth is middleware that rejects requests without a valid session and, +// on success, stashes the resolved user in the context. Feature routers added by +// later issues (gardens, objects, …) attach this to their protected groups. +func (h *handlers) requireAuth() gin.HandlerFunc { + return func(c *gin.Context) { + token, err := c.Cookie(sessionCookie) + if err != nil || token == "" { + writeAPIError(c, http.StatusUnauthorized, "UNAUTHENTICATED", "authentication required") + c.Abort() + return + } + user, err := h.svc.ResolveSession(c.Request.Context(), token) + if err != nil { + // Any resolution failure (missing/expired/tampered) is 401, never 404: + // the client should log in, not think a resource is gone. + writeAPIError(c, http.StatusUnauthorized, "UNAUTHENTICATED", "authentication required") + c.Abort() + return + } + c.Set(actorKey, user) + c.Next() + } +} + +// startSession issues a session and writes the cookie. +func (h *handlers) startSession(c *gin.Context, userID int64) error { + token, expiresAt, err := h.svc.CreateSession(c.Request.Context(), userID) + if err != nil { + return err + } + h.setSessionCookie(c, token, expiresAt) + return nil +} + +// cookieSecure reports whether the session cookie should carry the Secure +// attribute: only when the instance is served over HTTPS (PANSY_BASE_URL). +// Marking it Secure under plain-http local dev would make the browser drop it. +func (h *handlers) cookieSecure() bool { + return strings.HasPrefix(h.cfg.BaseURL, "https://") +} + +func (h *handlers) setSessionCookie(c *gin.Context, token string, expiresAt time.Time) { + maxAge := max(int(time.Until(expiresAt).Seconds()), 1) + c.SetSameSite(http.SameSiteLaxMode) + c.SetCookie(sessionCookie, token, maxAge, "/", "", h.cookieSecure(), true) +} + +func (h *handlers) clearSessionCookie(c *gin.Context) { + c.SetSameSite(http.SameSiteLaxMode) + c.SetCookie(sessionCookie, "", -1, "/", "", h.cookieSecure(), true) +} + +// mustActor returns the user stashed by requireAuth. It panics if called on a +// route that isn't behind requireAuth — a programming error. +func mustActor(c *gin.Context) *domain.User { + return c.MustGet(actorKey).(*domain.User) +} + +// writeServiceError maps a service-layer sentinel error to pansy's JSON error +// envelope. Login failures never leak which of email/password was wrong. +func (h *handlers) writeServiceError(c *gin.Context, err error) { + switch { + case errors.Is(err, domain.ErrInvalidCredentials): + writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password") + case errors.Is(err, domain.ErrEmailTaken): + writeAPIError(c, http.StatusConflict, "EMAIL_TAKEN", "an account with that email already exists") + case errors.Is(err, domain.ErrRegistrationClosed): + writeAPIError(c, http.StatusForbidden, "REGISTRATION_CLOSED", "registration is closed") + case errors.Is(err, domain.ErrLocalAuthDisabled): + writeAPIError(c, http.StatusForbidden, "LOCAL_AUTH_DISABLED", "local authentication is disabled") + case errors.Is(err, domain.ErrInvalidInput): + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid input") + default: + slog.Error("api: unhandled service error", "error", err) + writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "internal error") + } +} diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go new file mode 100644 index 0000000..60215d9 --- /dev/null +++ b/internal/api/auth_test.go @@ -0,0 +1,168 @@ +package api + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/service" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/store" +) + +// authEngine builds an API engine backed by a fresh in-memory database. +func authEngine(t *testing.T, cfg *config.Config) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + db, err := store.Open(":memory:") + if err != nil { + t.Fatalf("store.Open: %v", err) + } + t.Cleanup(func() { db.Close() }) + if err := db.Migrate(context.Background()); err != nil { + t.Fatalf("Migrate: %v", err) + } + return New(cfg, service.New(db, cfg)) +} + +func localCfg() *config.Config { + return &config.Config{Registration: config.RegistrationOpen, LocalAuth: true} +} + +// doJSON issues a JSON request, optionally carrying a session cookie. +func doJSON(t *testing.T, r *gin.Engine, method, path string, body any, cookie *http.Cookie) *httptest.ResponseRecorder { + t.Helper() + var buf bytes.Buffer + if body != nil { + if err := json.NewEncoder(&buf).Encode(body); err != nil { + t.Fatalf("encode body: %v", err) + } + } + req := httptest.NewRequest(method, path, &buf) + req.Header.Set("Content-Type", "application/json") + if cookie != nil { + req.AddCookie(cookie) + } + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + return w +} + +// sessionCookieFrom extracts the pansy_session cookie from a response. +func sessionCookieFrom(t *testing.T, w *httptest.ResponseRecorder) *http.Cookie { + t.Helper() + for _, ck := range w.Result().Cookies() { + if ck.Name == sessionCookie { + return ck + } + } + t.Fatalf("no %s cookie set (status %d, body %s)", sessionCookie, w.Code, w.Body.String()) + return nil +} + +func TestRegisterLoginMeLogoutFlow(t *testing.T) { + r := authEngine(t, localCfg()) + + // Register auto-logs-in and sets a cookie. + w := doJSON(t, r, http.MethodPost, "/api/v1/auth/register", + map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"}, nil) + if w.Code != http.StatusOK { + t.Fatalf("register status = %d, body %s", w.Code, w.Body.String()) + } + cookie := sessionCookieFrom(t, w) + if !cookie.HttpOnly { + t.Error("session cookie must be HttpOnly") + } + if cookie.SameSite != http.SameSiteLaxMode { + t.Errorf("session cookie SameSite = %v, want Lax", cookie.SameSite) + } + if cookie.Secure { + t.Error("session cookie must not be Secure without an https base URL") + } + + // /me with the cookie returns the user; password hash is never serialized. + w = doJSON(t, r, http.MethodGet, "/api/v1/auth/me", nil, cookie) + if w.Code != http.StatusOK { + t.Fatalf("me status = %d, body %s", w.Code, w.Body.String()) + } + if strings.Contains(w.Body.String(), "password") { + t.Errorf("/me leaked a password field: %s", w.Body.String()) + } + + // Logout clears the cookie and invalidates the session. + w = doJSON(t, r, http.MethodPost, "/api/v1/auth/logout", nil, cookie) + if w.Code != http.StatusOK { + t.Fatalf("logout status = %d", w.Code) + } + w = doJSON(t, r, http.MethodGet, "/api/v1/auth/me", nil, cookie) + if w.Code != http.StatusUnauthorized { + t.Errorf("me after logout status = %d, want 401", w.Code) + } +} + +func TestMeRequiresAuth(t *testing.T) { + r := authEngine(t, localCfg()) + w := doJSON(t, r, http.MethodGet, "/api/v1/auth/me", nil, nil) + if w.Code != http.StatusUnauthorized { + t.Errorf("me without cookie status = %d, want 401", w.Code) + } +} + +func TestLoginWrongPasswordIs401(t *testing.T) { + r := authEngine(t, localCfg()) + doJSON(t, r, http.MethodPost, "/api/v1/auth/register", + map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"}, nil) + + w := doJSON(t, r, http.MethodPost, "/api/v1/auth/login", + map[string]string{"email": "a@example.com", "password": "wrong"}, nil) + if w.Code != http.StatusUnauthorized { + t.Errorf("bad login status = %d, want 401", w.Code) + } +} + +func TestRegisterValidationRejectsShortPassword(t *testing.T) { + r := authEngine(t, localCfg()) + w := doJSON(t, r, http.MethodPost, "/api/v1/auth/register", + map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "short"}, nil) + if w.Code != http.StatusBadRequest { + t.Errorf("short-password register status = %d, want 400", w.Code) + } +} + +func TestProvidersEndpoint(t *testing.T) { + r := authEngine(t, localCfg()) + w := doJSON(t, r, http.MethodGet, "/api/v1/auth/providers", nil, nil) + if w.Code != http.StatusOK { + t.Fatalf("providers status = %d", w.Code) + } + var got struct { + Local bool `json:"local"` + OIDC bool `json:"oidc"` + } + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode providers: %v", err) + } + if !got.Local || got.OIDC { + t.Errorf("providers = %+v, want local=true oidc=false", got) + } +} + +func TestSecureCookieWithHTTPSBaseURL(t *testing.T) { + cfg := localCfg() + cfg.BaseURL = "https://pansy.example.com" + r := authEngine(t, cfg) + w := doJSON(t, r, http.MethodPost, "/api/v1/auth/register", + map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"}, nil) + if w.Code != http.StatusOK { + t.Fatalf("register status = %d", w.Code) + } + if !sessionCookieFrom(t, w).Secure { + t.Error("session cookie should be Secure when base URL is https") + } +} diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 85ce397..7561146 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -17,6 +17,24 @@ var ( // ErrVersionConflict means the row's version did not match the one supplied // with a PATCH/DELETE; the caller should refetch and retry. ErrVersionConflict = errors.New("version conflict") + + // ErrInvalidInput means the caller supplied structurally invalid data (empty + // required field, malformed value). Mapped to 400. + ErrInvalidInput = errors.New("invalid input") + // ErrInvalidCredentials means a login attempt failed. It is deliberately + // identical for an unknown email and a wrong password so neither can be + // enumerated. Mapped to 401. + ErrInvalidCredentials = errors.New("invalid credentials") + // ErrEmailTaken means registration collided with an existing account's email. + // Mapped to 409. + ErrEmailTaken = errors.New("email already registered") + // ErrRegistrationClosed means local self-service signup is disabled and at + // least one user already exists (the very first user may always register to + // bootstrap the instance). Mapped to 403. + ErrRegistrationClosed = errors.New("registration closed") + // ErrLocalAuthDisabled means PANSY_LOCAL_AUTH=false, so local register/login + // are rejected in favor of OIDC. Mapped to 403. + ErrLocalAuthDisabled = errors.New("local authentication disabled") ) // Enumerated string values mirrored from the schema CHECK constraints. diff --git a/internal/service/auth.go b/internal/service/auth.go new file mode 100644 index 0000000..c5da735 --- /dev/null +++ b/internal/service/auth.go @@ -0,0 +1,186 @@ +package service + +import ( + "context" + "errors" + "strings" + "time" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// RegisterInput is the payload for local self-service signup. +type RegisterInput struct { + Email string + DisplayName string + Password string +} + +// Providers reports which login methods the server offers, so the login page +// (#6) can render the right controls. OIDCLabel is only meaningful when OIDC is +// true. +type Providers struct { + Local bool `json:"local"` + OIDC bool `json:"oidc"` + OIDCLabel string `json:"oidcLabel"` +} + +// Providers returns the enabled auth methods. OIDC is always false until #5 +// wires the endpoints; advertising it before then would point the UI at routes +// that don't exist. +func (s *Service) Providers() Providers { + return Providers{ + Local: s.cfg.LocalAuth, + OIDC: false, + OIDCLabel: s.cfg.OIDC.ButtonLabel, + } +} + +// Register creates a local (password) account and returns it. The first user on +// a fresh instance becomes admin and may always register (bootstrap), even when +// PANSY_REGISTRATION=closed; afterward, closed registration is enforced. +func (s *Service) Register(ctx context.Context, in RegisterInput) (*domain.User, error) { + if !s.cfg.LocalAuth { + return nil, domain.ErrLocalAuthDisabled + } + + email := normalizeEmail(in.Email) + displayName := strings.TrimSpace(in.DisplayName) + if email == "" || displayName == "" || in.Password == "" { + return nil, domain.ErrInvalidInput + } + + count, err := s.store.CountUsers(ctx) + if err != nil { + return nil, err + } + // Closed registration still allows the very first account so a locked-down + // instance can be bootstrapped without editing config. + if count > 0 && !s.cfg.RegistrationOpen() { + return nil, domain.ErrRegistrationClosed + } + + // Friendly duplicate check. The UNIQUE index is the real guard against a + // race; that path surfaces as a generic insert error (500), acceptable at + // household scale. + if _, err := s.store.GetUserByEmail(ctx, email); err == nil { + return nil, domain.ErrEmailTaken + } else if !errors.Is(err, domain.ErrNotFound) { + return nil, err + } + + hash, err := hashPassword(in.Password) + if err != nil { + return nil, err + } + + return s.store.CreateUser(ctx, &domain.User{ + Email: email, + DisplayName: displayName, + PasswordHash: &hash, + IsAdmin: count == 0, + }) +} + +// Login verifies an email/password pair and returns the user. Unknown-email and +// wrong-password both return domain.ErrInvalidCredentials, and both spend the +// same argon2 work, so neither the error nor the timing reveals which failed. +func (s *Service) Login(ctx context.Context, email, password string) (*domain.User, error) { + if !s.cfg.LocalAuth { + return nil, domain.ErrLocalAuthDisabled + } + + u, err := s.store.GetUserByEmail(ctx, normalizeEmail(email)) + switch { + case errors.Is(err, domain.ErrNotFound): + // Spend comparable time so timing can't distinguish a missing account. + _, _ = verifyPassword(s.dummyHash, password) + return nil, domain.ErrInvalidCredentials + case err != nil: + return nil, err + case u.PasswordHash == nil: + // OIDC-only account: no local password to check, but equalize timing. + _, _ = verifyPassword(s.dummyHash, password) + return nil, domain.ErrInvalidCredentials + } + + ok, err := verifyPassword(*u.PasswordHash, password) + if err != nil || !ok { + return nil, domain.ErrInvalidCredentials + } + return u, nil +} + +// CreateSession issues a new session for a user and returns the raw bearer token +// (to place in the cookie) and its expiry. +func (s *Service) CreateSession(ctx context.Context, userID int64) (token string, expiresAt time.Time, err error) { + raw, err := newSessionToken() + if err != nil { + return "", time.Time{}, err + } + exp := s.now().Add(sessionTTL) + if err := s.store.CreateSession(ctx, &domain.Session{ + TokenHash: hashToken(raw), + UserID: userID, + ExpiresAt: formatTime(exp), + }); err != nil { + return "", time.Time{}, err + } + return raw, exp, nil +} + +// ResolveSession validates a raw bearer token and returns its user. An expired +// session is deleted and treated as absent (domain.ErrNotFound). A still-valid +// session has its expiry slid forward, but only when that moves it by more than +// an hour, so a busy client doesn't write on every request. +func (s *Service) ResolveSession(ctx context.Context, rawToken string) (*domain.User, error) { + if rawToken == "" { + return nil, domain.ErrNotFound + } + hash := hashToken(rawToken) + sess, err := s.store.GetSession(ctx, hash) + if err != nil { + return nil, err + } + + exp, err := parseTime(sess.ExpiresAt) + if err != nil { + // A corrupt expiry means we can't trust the session; drop it. + _ = s.store.DeleteSession(ctx, hash) + return nil, domain.ErrNotFound + } + + now := s.now() + if !now.Before(exp) { + _ = s.store.DeleteSession(ctx, hash) + return nil, domain.ErrNotFound + } + + if newExp := now.Add(sessionTTL); newExp.Sub(exp) > time.Hour { + _ = s.store.TouchSession(ctx, hash, formatTime(newExp)) + } + + return s.store.GetUserByID(ctx, sess.UserID) +} + +// Logout deletes the session behind a raw bearer token. It is idempotent. +func (s *Service) Logout(ctx context.Context, rawToken string) error { + if rawToken == "" { + return nil + } + return s.store.DeleteSession(ctx, hashToken(rawToken)) +} + +// CleanupExpiredSessions deletes all sessions that have passed their expiry and +// returns the count. Called periodically; expired sessions are also dropped +// lazily on access by ResolveSession. +func (s *Service) CleanupExpiredSessions(ctx context.Context) (int64, error) { + return s.store.DeleteExpiredSessions(ctx, formatTime(s.now())) +} + +// normalizeEmail trims and lowercases an email for consistent storage and +// lookup. (The users.email column is also NOCASE, so lookups are robust either +// way; normalizing keeps stored values tidy.) +func normalizeEmail(email string) string { + return strings.ToLower(strings.TrimSpace(email)) +} diff --git a/internal/service/auth_test.go b/internal/service/auth_test.go new file mode 100644 index 0000000..f583452 --- /dev/null +++ b/internal/service/auth_test.go @@ -0,0 +1,272 @@ +package service + +import ( + "context" + "errors" + "testing" + "time" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/store" +) + +// newTestService builds a Service over a fresh in-memory database. +func newTestService(t *testing.T, cfg *config.Config) *Service { + t.Helper() + db, err := store.Open(":memory:") + if err != nil { + t.Fatalf("store.Open: %v", err) + } + t.Cleanup(func() { db.Close() }) + if err := db.Migrate(context.Background()); err != nil { + t.Fatalf("Migrate: %v", err) + } + return New(db, cfg) +} + +func openConfig() *config.Config { + return &config.Config{Registration: config.RegistrationOpen, LocalAuth: true} +} + +func mustRegister(t *testing.T, s *Service, email, name, pw string) *domain.User { + t.Helper() + u, err := s.Register(context.Background(), RegisterInput{Email: email, DisplayName: name, Password: pw}) + if err != nil { + t.Fatalf("Register(%s): %v", email, err) + } + return u +} + +func TestRegisterFirstUserIsAdmin(t *testing.T) { + s := newTestService(t, openConfig()) + + first := mustRegister(t, s, "a@example.com", "Alice", "password123") + if !first.IsAdmin { + t.Error("first user should be admin") + } + + second := mustRegister(t, s, "b@example.com", "Bob", "password123") + if second.IsAdmin { + t.Error("second user should not be admin") + } +} + +func TestRegisterNormalizesAndRejectsDuplicateEmail(t *testing.T) { + s := newTestService(t, openConfig()) + mustRegister(t, s, "Alice@Example.com", "Alice", "password123") + + // Stored normalized (lowercased). + u, err := s.store.GetUserByEmail(context.Background(), "alice@example.com") + if err != nil { + t.Fatalf("lookup normalized email: %v", err) + } + if u.Email != "alice@example.com" { + t.Errorf("stored email = %q, want lowercased", u.Email) + } + + // A different-cased duplicate is rejected. + _, err = s.Register(context.Background(), RegisterInput{Email: "ALICE@example.com", DisplayName: "A2", Password: "password123"}) + if !errors.Is(err, domain.ErrEmailTaken) { + t.Errorf("duplicate register err = %v, want ErrEmailTaken", err) + } +} + +func TestRegisterRejectsBlankFields(t *testing.T) { + s := newTestService(t, openConfig()) + _, err := s.Register(context.Background(), RegisterInput{Email: " ", DisplayName: "", Password: ""}) + if !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("blank register err = %v, want ErrInvalidInput", err) + } +} + +func TestRegistrationClosedAllowsBootstrapThenBlocks(t *testing.T) { + cfg := openConfig() + cfg.Registration = config.RegistrationClosed + s := newTestService(t, cfg) + + // The very first user may register even when closed (bootstrap). + first := mustRegister(t, s, "admin@example.com", "Admin", "password123") + if !first.IsAdmin { + t.Error("bootstrap user should be admin") + } + + // Subsequent signups are blocked. + _, err := s.Register(context.Background(), RegisterInput{Email: "b@example.com", DisplayName: "Bob", Password: "password123"}) + if !errors.Is(err, domain.ErrRegistrationClosed) { + t.Errorf("closed register err = %v, want ErrRegistrationClosed", err) + } +} + +func TestLocalAuthDisabledRejectsRegisterAndLogin(t *testing.T) { + cfg := openConfig() + cfg.LocalAuth = false + s := newTestService(t, cfg) + + if _, err := s.Register(context.Background(), RegisterInput{Email: "a@example.com", DisplayName: "A", Password: "password123"}); !errors.Is(err, domain.ErrLocalAuthDisabled) { + t.Errorf("register err = %v, want ErrLocalAuthDisabled", err) + } + if _, err := s.Login(context.Background(), "a@example.com", "password123"); !errors.Is(err, domain.ErrLocalAuthDisabled) { + t.Errorf("login err = %v, want ErrLocalAuthDisabled", err) + } +} + +func TestLoginSucceedsAndFailsIndistinguishably(t *testing.T) { + s := newTestService(t, openConfig()) + mustRegister(t, s, "a@example.com", "Alice", "correct-horse") + + // Correct credentials (case-insensitive email). + u, err := s.Login(context.Background(), "A@example.com", "correct-horse") + if err != nil { + t.Fatalf("login correct: %v", err) + } + if u.Email != "a@example.com" { + t.Errorf("logged-in user = %q", u.Email) + } + + // Wrong password and unknown email both yield the same sentinel. + if _, err := s.Login(context.Background(), "a@example.com", "wrong"); !errors.Is(err, domain.ErrInvalidCredentials) { + t.Errorf("wrong-password err = %v, want ErrInvalidCredentials", err) + } + if _, err := s.Login(context.Background(), "nobody@example.com", "whatever"); !errors.Is(err, domain.ErrInvalidCredentials) { + t.Errorf("unknown-email err = %v, want ErrInvalidCredentials", err) + } +} + +func TestLoginRejectsOIDCOnlyUser(t *testing.T) { + s := newTestService(t, openConfig()) + // Simulate an OIDC-only account (no password hash) directly in the store. + iss, sub := "https://idp.example", "subject-1" + if _, err := s.store.CreateUser(context.Background(), &domain.User{ + Email: "oidc@example.com", DisplayName: "O", OIDCIssuer: &iss, OIDCSubject: &sub, + }); err != nil { + t.Fatalf("seed oidc user: %v", err) + } + if _, err := s.Login(context.Background(), "oidc@example.com", "anything"); !errors.Is(err, domain.ErrInvalidCredentials) { + t.Errorf("oidc-only login err = %v, want ErrInvalidCredentials", err) + } +} + +func TestSessionLifecycle(t *testing.T) { + s := newTestService(t, openConfig()) + u := mustRegister(t, s, "a@example.com", "Alice", "password123") + + token, _, err := s.CreateSession(context.Background(), u.ID) + if err != nil { + t.Fatalf("CreateSession: %v", err) + } + + got, err := s.ResolveSession(context.Background(), token) + if err != nil { + t.Fatalf("ResolveSession: %v", err) + } + if got.ID != u.ID { + t.Errorf("resolved user %d, want %d", got.ID, u.ID) + } + + // Logout invalidates it. + if err := s.Logout(context.Background(), token); err != nil { + t.Fatalf("Logout: %v", err) + } + if _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("resolve after logout err = %v, want ErrNotFound", err) + } +} + +func TestResolveSessionRejectsGarbageToken(t *testing.T) { + s := newTestService(t, openConfig()) + if _, err := s.ResolveSession(context.Background(), "not-a-real-token"); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("garbage token err = %v, want ErrNotFound", err) + } + if _, err := s.ResolveSession(context.Background(), ""); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("empty token err = %v, want ErrNotFound", err) + } +} + +func TestSessionExpiryAndLazyDeletion(t *testing.T) { + s := newTestService(t, openConfig()) + u := mustRegister(t, s, "a@example.com", "Alice", "password123") + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + s.now = func() time.Time { return base } + + token, _, err := s.CreateSession(context.Background(), u.ID) + if err != nil { + t.Fatalf("CreateSession: %v", err) + } + + // Jump past the 30-day TTL: the session must be treated as gone... + s.now = func() time.Time { return base.Add(sessionTTL + time.Hour) } + if _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { + t.Fatalf("expired resolve err = %v, want ErrNotFound", err) + } + // ...and lazily deleted from the store. + if _, err := s.store.GetSession(context.Background(), hashToken(token)); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("expired session row still present: %v", err) + } +} + +func TestSessionSlidingRenewal(t *testing.T) { + s := newTestService(t, openConfig()) + u := mustRegister(t, s, "a@example.com", "Alice", "password123") + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + s.now = func() time.Time { return base } + token, firstExp, err := s.CreateSession(context.Background(), u.ID) + if err != nil { + t.Fatalf("CreateSession: %v", err) + } + + // Use it 10 days later; expiry should slide forward. + s.now = func() time.Time { return base.Add(10 * 24 * time.Hour) } + if _, err := s.ResolveSession(context.Background(), token); err != nil { + t.Fatalf("ResolveSession: %v", err) + } + sess, err := s.store.GetSession(context.Background(), hashToken(token)) + if err != nil { + t.Fatalf("GetSession: %v", err) + } + newExp, err := parseTime(sess.ExpiresAt) + if err != nil { + t.Fatalf("parse expiry: %v", err) + } + if !newExp.After(firstExp) { + t.Errorf("expiry did not slide: new %v not after first %v", newExp, firstExp) + } +} + +func TestCleanupExpiredSessions(t *testing.T) { + s := newTestService(t, openConfig()) + u := mustRegister(t, s, "a@example.com", "Alice", "password123") + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + s.now = func() time.Time { return base } + if _, _, err := s.CreateSession(context.Background(), u.ID); err != nil { + t.Fatalf("CreateSession: %v", err) + } + + // Before expiry: nothing to clean. + if n, err := s.CleanupExpiredSessions(context.Background()); err != nil || n != 0 { + t.Fatalf("early cleanup = (%d, %v), want (0, nil)", n, err) + } + + // After expiry: the one session is purged. + s.now = func() time.Time { return base.Add(sessionTTL + time.Hour) } + if n, err := s.CleanupExpiredSessions(context.Background()); err != nil || n != 1 { + t.Fatalf("late cleanup = (%d, %v), want (1, nil)", n, err) + } +} + +func TestProvidersReflectsConfig(t *testing.T) { + cfg := openConfig() + cfg.OIDC.ButtonLabel = "Sign in with Authentik" + s := newTestService(t, cfg) + + p := s.Providers() + if !p.Local { + t.Error("expected local=true") + } + if p.OIDC { + t.Error("OIDC must be false until #5 wires it") + } +} diff --git a/internal/service/password.go b/internal/service/password.go new file mode 100644 index 0000000..fa34ae6 --- /dev/null +++ b/internal/service/password.go @@ -0,0 +1,82 @@ +package service + +import ( + "crypto/rand" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "strings" + + "golang.org/x/crypto/argon2" +) + +// argon2id parameters. ~64 MiB memory / 1 pass / 4 lanes is the interactive +// profile recommended by the argon2 authors and is comfortable on a +// self-hosted box. They are encoded into every stored hash, so raising them +// later leaves old hashes verifiable. +const ( + argonMemKiB = 64 * 1024 // 64 MiB + argonTime = 1 + argonThreads = 4 + argonKeyLen = 32 + argonSaltLen = 16 +) + +// errBadHash marks a stored hash string that could not be parsed — a data or +// programming error, not a wrong password. Callers treat it as an auth failure +// but should log it. +var errBadHash = errors.New("service: malformed password hash") + +// b64 is the padding-free base64 used inside the PHC-style hash string. +var b64 = base64.RawStdEncoding + +// hashPassword returns a self-describing argon2id hash in the PHC string format +// "$argon2id$v=19$m=...,t=...,p=...$salt$hash" (all base64, no padding). +func hashPassword(password string) (string, error) { + salt := make([]byte, argonSaltLen) + if _, err := rand.Read(salt); err != nil { + return "", fmt.Errorf("service: generate salt: %w", err) + } + key := argon2.IDKey([]byte(password), salt, argonTime, argonMemKiB, argonThreads, argonKeyLen) + return fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", + argon2.Version, argonMemKiB, argonTime, argonThreads, + b64.EncodeToString(salt), b64.EncodeToString(key), + ), nil +} + +// verifyPassword reports whether password matches the encoded argon2id hash. The +// comparison is constant-time. A malformed encoded value returns errBadHash. +func verifyPassword(encoded, password string) (bool, error) { + mem, t, threads, salt, want, err := decodeHash(encoded) + if err != nil { + return false, err + } + got := argon2.IDKey([]byte(password), salt, t, mem, threads, uint32(len(want))) + return subtle.ConstantTimeCompare(got, want) == 1, nil +} + +// decodeHash parses a PHC-format argon2id string into its parameters, salt, and +// derived key. +func decodeHash(encoded string) (mem, t uint32, threads uint8, salt, key []byte, err error) { + parts := strings.Split(encoded, "$") + // ["", "argon2id", "v=19", "m=..,t=..,p=..", "", ""] + if len(parts) != 6 || parts[1] != "argon2id" { + return 0, 0, 0, nil, nil, errBadHash + } + + var version int + if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil || version != argon2.Version { + return 0, 0, 0, nil, nil, errBadHash + } + if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &mem, &t, &threads); err != nil { + return 0, 0, 0, nil, nil, errBadHash + } + if salt, err = b64.DecodeString(parts[4]); err != nil { + return 0, 0, 0, nil, nil, errBadHash + } + if key, err = b64.DecodeString(parts[5]); err != nil || len(key) == 0 { + return 0, 0, 0, nil, nil, errBadHash + } + return mem, t, threads, salt, key, nil +} diff --git a/internal/service/password_test.go b/internal/service/password_test.go new file mode 100644 index 0000000..8470555 --- /dev/null +++ b/internal/service/password_test.go @@ -0,0 +1,54 @@ +package service + +import ( + "strings" + "testing" +) + +func TestHashAndVerifyPassword(t *testing.T) { + hash, err := hashPassword("correct-horse-battery-staple") + if err != nil { + t.Fatalf("hashPassword: %v", err) + } + if !strings.HasPrefix(hash, "$argon2id$v=19$") { + t.Errorf("hash %q lacks argon2id PHC prefix", hash) + } + + ok, err := verifyPassword(hash, "correct-horse-battery-staple") + if err != nil || !ok { + t.Errorf("verify correct = (%v, %v), want (true, nil)", ok, err) + } + + ok, err = verifyPassword(hash, "wrong-password") + if err != nil || ok { + t.Errorf("verify wrong = (%v, %v), want (false, nil)", ok, err) + } +} + +func TestHashPasswordIsSalted(t *testing.T) { + // Two hashes of the same password must differ (random salt), yet both verify. + h1, _ := hashPassword("same-password") + h2, _ := hashPassword("same-password") + if h1 == h2 { + t.Error("two hashes of the same password are identical; salt not random") + } + for _, h := range []string{h1, h2} { + if ok, err := verifyPassword(h, "same-password"); err != nil || !ok { + t.Errorf("verify(%q) = (%v, %v), want (true, nil)", h, ok, err) + } + } +} + +func TestVerifyPasswordRejectsMalformedHash(t *testing.T) { + for _, bad := range []string{ + "", + "not-a-hash", + "$argon2id$v=19$m=65536,t=1,p=4$onlyfourparts", + "$argon2i$v=19$m=65536,t=1,p=4$c2FsdA$aGFzaA", // wrong variant + "$argon2id$v=1$m=65536,t=1,p=4$c2FsdA$aGFzaA", // wrong version + } { + if _, err := verifyPassword(bad, "whatever"); err == nil { + t.Errorf("verifyPassword(%q) err = nil, want errBadHash", bad) + } + } +} diff --git a/internal/service/service.go b/internal/service/service.go new file mode 100644 index 0000000..bf6aadc --- /dev/null +++ b/internal/service/service.go @@ -0,0 +1,76 @@ +// Package service is pansy's business-logic seam: every operation is a method on +// *Service taking (ctx, actor, args), and all permission checks and invariants +// live here rather than in the HTTP handlers. REST handlers (internal/api) and, +// later, agent tools (internal/agent) are thin adapters over these methods, so +// both inherit the same rules. This file holds the shared plumbing; feature +// methods live alongside it (auth.go, and gardens/objects/… in later issues). +package service + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "fmt" + "log/slog" + "time" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" + "gitea.stevedudenhoeffer.com/steve/pansy/internal/store" +) + +// timeLayout is the ISO-8601 UTC format used for every timestamp pansy stores. +// It matches the schema's strftime('%Y-%m-%dT%H:%M:%SZ') so string comparison +// (e.g. session expiry) is equivalent to time comparison. +const timeLayout = "2006-01-02T15:04:05Z" + +// sessionTTL is how long a session lives from its last use (sliding expiry). +const sessionTTL = 30 * 24 * time.Hour + +// Service holds the dependencies shared by every operation. +type Service struct { + store *store.DB + cfg *config.Config + // now is the clock, injectable so tests can advance time (session expiry). + now func() time.Time + // dummyHash is a valid argon2id hash of a throwaway password. Login verifies + // against it when an email is unknown so the response time doesn't reveal + // whether an account exists. + dummyHash string +} + +// New constructs a Service. It precomputes a dummy password hash used to +// equalize login timing; if that fails (it shouldn't), login still works but +// loses the timing defense. +func New(st *store.DB, cfg *config.Config) *Service { + s := &Service{store: st, cfg: cfg, now: time.Now} + if h, err := hashPassword("pansy-timing-equalizer-not-a-real-password"); err != nil { + slog.Warn("service: could not precompute login timing hash", "error", err) + } else { + s.dummyHash = h + } + return s +} + +// formatTime renders a time as pansy's canonical UTC string. +func formatTime(t time.Time) string { return t.UTC().Format(timeLayout) } + +// parseTime parses a canonical pansy timestamp. +func parseTime(s string) (time.Time, error) { return time.Parse(timeLayout, s) } + +// newSessionToken returns a fresh URL-safe random bearer token (32 bytes of +// entropy). The raw token goes in the cookie; only its hash is persisted. +func newSessionToken() (string, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return "", fmt.Errorf("service: generate session token: %w", err) + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// hashToken maps a raw bearer token to the hex sha256 stored as the session's +// primary key. +func hashToken(raw string) string { + sum := sha256.Sum256([]byte(raw)) + return hex.EncodeToString(sum[:]) +} diff --git a/internal/store/sessions.go b/internal/store/sessions.go new file mode 100644 index 0000000..7ef20dc --- /dev/null +++ b/internal/store/sessions.go @@ -0,0 +1,80 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// CreateSession inserts a session row. The raw bearer token is never stored; +// s.TokenHash is its sha256 (computed by the service layer). ExpiresAt is an +// ISO-8601 UTC string, lexicographically comparable to the schema's timestamps. +func (d *DB) CreateSession(ctx context.Context, s *domain.Session) error { + _, err := d.sql.ExecContext(ctx, + `INSERT INTO sessions (token_hash, user_id, expires_at) VALUES (?, ?, ?)`, + s.TokenHash, s.UserID, s.ExpiresAt, + ) + if err != nil { + return fmt.Errorf("store: insert session: %w", err) + } + return nil +} + +// GetSession returns the session for a token hash, or domain.ErrNotFound. It +// does not check expiry — that is the service layer's concern (which also +// implements sliding renewal). +func (d *DB) GetSession(ctx context.Context, tokenHash string) (*domain.Session, error) { + var s domain.Session + err := d.sql.QueryRowContext(ctx, + `SELECT token_hash, user_id, expires_at, created_at FROM sessions WHERE token_hash = ?`, + tokenHash, + ).Scan(&s.TokenHash, &s.UserID, &s.ExpiresAt, &s.CreatedAt) + if errors.Is(err, sql.ErrNoRows) { + return nil, domain.ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("store: get session: %w", err) + } + return &s, nil +} + +// TouchSession moves a session's expiry forward (sliding-expiry renewal). +func (d *DB) TouchSession(ctx context.Context, tokenHash, expiresAt string) error { + _, err := d.sql.ExecContext(ctx, + `UPDATE sessions SET expires_at = ? WHERE token_hash = ?`, expiresAt, tokenHash, + ) + if err != nil { + return fmt.Errorf("store: touch session: %w", err) + } + return nil +} + +// DeleteSession removes a session (logout, or lazy cleanup of an expired one). +// Deleting a nonexistent session is not an error. +func (d *DB) DeleteSession(ctx context.Context, tokenHash string) error { + if _, err := d.sql.ExecContext(ctx, + `DELETE FROM sessions WHERE token_hash = ?`, tokenHash, + ); err != nil { + return fmt.Errorf("store: delete session: %w", err) + } + return nil +} + +// DeleteExpiredSessions removes every session that expired at or before now +// (an ISO-8601 UTC string) and returns how many rows were deleted. +func (d *DB) DeleteExpiredSessions(ctx context.Context, now string) (int64, error) { + res, err := d.sql.ExecContext(ctx, + `DELETE FROM sessions WHERE expires_at <= ?`, now, + ) + if err != nil { + return 0, fmt.Errorf("store: delete expired sessions: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return 0, fmt.Errorf("store: expired sessions affected: %w", err) + } + return n, nil +} diff --git a/internal/store/users.go b/internal/store/users.go new file mode 100644 index 0000000..e92a36b --- /dev/null +++ b/internal/store/users.go @@ -0,0 +1,101 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// userColumns lists the users columns in the fixed order scanUser expects. +const userColumns = `id, email, display_name, password_hash, oidc_issuer, oidc_subject, is_admin, version, created_at, updated_at` + +// scanner is satisfied by both *sql.Row and *sql.Rows, so scanUser works for +// single-row and multi-row queries alike. +type scanner interface { + Scan(dest ...any) error +} + +// scanUser reads one users row. is_admin is stored as INTEGER 0/1 (the driver +// returns it as int64, which does not convert straight to bool), so it is read +// into an int and mapped. +func scanUser(s scanner) (*domain.User, error) { + var ( + u domain.User + isAdmin int64 + ) + if err := s.Scan( + &u.ID, &u.Email, &u.DisplayName, &u.PasswordHash, + &u.OIDCIssuer, &u.OIDCSubject, &isAdmin, &u.Version, + &u.CreatedAt, &u.UpdatedAt, + ); err != nil { + return nil, err + } + u.IsAdmin = isAdmin != 0 + return &u, nil +} + +// CreateUser inserts a new user and returns the stored row (with generated id +// and timestamps). PasswordHash/OIDCIssuer/OIDCSubject may be nil. +func (d *DB) CreateUser(ctx context.Context, u *domain.User) (*domain.User, error) { + res, err := d.sql.ExecContext(ctx, + `INSERT INTO users (email, display_name, password_hash, oidc_issuer, oidc_subject, is_admin) + VALUES (?, ?, ?, ?, ?, ?)`, + u.Email, u.DisplayName, u.PasswordHash, u.OIDCIssuer, u.OIDCSubject, boolToInt(u.IsAdmin), + ) + if err != nil { + return nil, fmt.Errorf("store: insert user: %w", err) + } + id, err := res.LastInsertId() + if err != nil { + return nil, fmt.Errorf("store: user insert id: %w", err) + } + return d.GetUserByID(ctx, id) +} + +// GetUserByID returns the user with the given id, or domain.ErrNotFound. +func (d *DB) GetUserByID(ctx context.Context, id int64) (*domain.User, error) { + u, err := scanUser(d.sql.QueryRowContext(ctx, + `SELECT `+userColumns+` FROM users WHERE id = ?`, id)) + if errors.Is(err, sql.ErrNoRows) { + return nil, domain.ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("store: get user by id: %w", err) + } + return u, nil +} + +// GetUserByEmail returns the user with the given email (case-insensitive via the +// column's NOCASE collation), or domain.ErrNotFound. +func (d *DB) GetUserByEmail(ctx context.Context, email string) (*domain.User, error) { + u, err := scanUser(d.sql.QueryRowContext(ctx, + `SELECT `+userColumns+` FROM users WHERE email = ?`, email)) + if errors.Is(err, sql.ErrNoRows) { + return nil, domain.ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("store: get user by email: %w", err) + } + return u, nil +} + +// CountUsers returns the number of user rows. Used to decide first-user-is-admin +// and to allow bootstrap registration when signup is otherwise closed. +func (d *DB) CountUsers(ctx context.Context) (int, error) { + var n int + if err := d.sql.QueryRowContext(ctx, `SELECT count(*) FROM users`).Scan(&n); err != nil { + return 0, fmt.Errorf("store: count users: %w", err) + } + return n, nil +} + +// boolToInt maps a Go bool to the 0/1 SQLite stores for INTEGER "boolean" columns. +func boolToInt(b bool) int { + if b { + return 1 + } + return 0 +} From 02c928ac6da44c1b2c4da80c579351cd03e31496 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 18 Jul 2026 17:04:35 -0400 Subject: [PATCH 2/2] Address Gadfly review on #4: TOCTOU, sliding cookie, CSRF, argon2 Fixes from the PR #23 adversarial review (graded 35 real / 1 false positive): Security / correctness - Race-free registration: is_admin and the registration gate are now computed atomically inside a single INSERT...SELECT, so concurrent first registrations can't both become admin or bypass closed registration (fixed the whole TOCTOU cluster). - Sliding session now reaches the browser: ResolveSession returns the current expiry and requireAuth re-sets the cookie, so active users aren't logged out 30 days after login regardless of activity. - Login CSRF: csrfGuard rejects state-changing requests whose Origin doesn't match PANSY_BASE_URL (no-op when unset, so the dev proxy is unaffected). SameSite=Lax alone didn't cover this. - argon2id tuned to RFC 9106's second recommended profile (t=3). - Timing equalizer can't fail open: the dummy hash is derived deterministically (fixed salt, no RNG) so it's always present. - Password length (<=1024) enforced in the service for both register and login, not just HTTP binding tags; login rejects over-long input before spending argon2 work. Error handling / robustness - Login logs a malformed stored hash instead of silently treating it as a wrong password. - Best-effort session writes (Touch/Delete during renewal, expiry, and corrupt-expiry cleanup) now log on failure. - index sessions.expires_at via new migration 0002 (0001 is immutable). Maintainability - Extract startSessionAndRespond and abortUnauthenticated; make writeServiceError a free function; consistent error handling in decodeHash; doc/comment fixes. Tests: over-long password, CSRF guard (cross-origin/same-origin/dev no-op), and cookie refresh on authenticated requests; migration-version assertions bumped to 2. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi --- internal/api/api.go | 3 + internal/api/auth.go | 96 ++++++++++++++----- internal/api/auth_test.go | 81 +++++++++++++++- internal/domain/domain.go | 64 ++++++------- internal/service/auth.go | 92 +++++++++++------- internal/service/auth_test.go | 36 +++++-- internal/service/password.go | 54 +++++++---- internal/service/password_test.go | 2 +- internal/service/service.go | 37 ++++--- .../0002_sessions_expires_index.sql | 7 ++ internal/store/store_test.go | 10 +- internal/store/users.go | 38 +++++++- 12 files changed, 370 insertions(+), 150 deletions(-) create mode 100644 internal/store/migrations/0002_sessions_expires_index.sql diff --git a/internal/api/api.go b/internal/api/api.go index be57af0..87ef607 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -44,6 +44,9 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine { h := &handlers{cfg: cfg, svc: svc} v1 := r.Group("/api/v1") + // CSRF defense for every state-changing API call (no-op unless PANSY_BASE_URL + // is set; see csrfGuard). + v1.Use(h.csrfGuard()) v1.GET("/healthz", healthz) // Auth endpoints are exempt from requireAuth (you can't be logged in yet); diff --git a/internal/api/auth.go b/internal/api/auth.go index 69a7e0a..ca06dec 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -4,6 +4,7 @@ import ( "errors" "log/slog" "net/http" + "net/url" "strings" "time" @@ -30,7 +31,7 @@ type registerRequest struct { type loginRequest struct { Email string `json:"email" binding:"required,email"` - Password string `json:"password" binding:"required"` + Password string `json:"password" binding:"required,max=1024"` } // register creates a local account and logs it in (sets the session cookie). @@ -47,15 +48,11 @@ func (h *handlers) register(c *gin.Context) { Password: req.Password, }) if err != nil { - h.writeServiceError(c, err) + writeServiceError(c, err) return } - if err := h.startSession(c, user.ID); err != nil { - writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "could not start session") - return - } - c.JSON(http.StatusOK, user) + h.startSessionAndRespond(c, user) } // login verifies credentials and sets the session cookie. @@ -68,15 +65,11 @@ func (h *handlers) login(c *gin.Context) { user, err := h.svc.Login(c.Request.Context(), req.Email, req.Password) if err != nil { - h.writeServiceError(c, err) + writeServiceError(c, err) return } - if err := h.startSession(c, user.ID); err != nil { - writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "could not start session") - return - } - c.JSON(http.StatusOK, user) + h.startSessionAndRespond(c, user) } // logout deletes the current session (if any) and clears the cookie. It is @@ -102,37 +95,84 @@ func (h *handlers) providers(c *gin.Context) { } // requireAuth is middleware that rejects requests without a valid session and, -// on success, stashes the resolved user in the context. Feature routers added by -// later issues (gardens, objects, …) attach this to their protected groups. +// on success, stashes the resolved user in the context and slides the cookie's +// lifetime forward in step with the server-side session (otherwise an active +// user would be logged out 30 days after login regardless of activity). Feature +// routers added by later issues (gardens, objects, …) attach this to their +// protected groups. func (h *handlers) requireAuth() gin.HandlerFunc { return func(c *gin.Context) { token, err := c.Cookie(sessionCookie) if err != nil || token == "" { - writeAPIError(c, http.StatusUnauthorized, "UNAUTHENTICATED", "authentication required") - c.Abort() + abortUnauthenticated(c) return } - user, err := h.svc.ResolveSession(c.Request.Context(), token) + user, expiresAt, err := h.svc.ResolveSession(c.Request.Context(), token) if err != nil { // Any resolution failure (missing/expired/tampered) is 401, never 404: // the client should log in, not think a resource is gone. - writeAPIError(c, http.StatusUnauthorized, "UNAUTHENTICATED", "authentication required") - c.Abort() + abortUnauthenticated(c) return } c.Set(actorKey, user) + // Refresh the client cookie to the session's current expiry so browser and + // server slide together. + h.setSessionCookie(c, token, expiresAt) c.Next() } } -// startSession issues a session and writes the cookie. -func (h *handlers) startSession(c *gin.Context, userID int64) error { - token, expiresAt, err := h.svc.CreateSession(c.Request.Context(), userID) +// abortUnauthenticated writes the standard 401 and stops the handler chain. +func abortUnauthenticated(c *gin.Context) { + writeAPIError(c, http.StatusUnauthorized, "UNAUTHENTICATED", "authentication required") + c.Abort() +} + +// csrfGuard rejects state-changing requests whose Origin header doesn't match +// the instance's own origin — a defense against login/logout CSRF, which +// SameSite=Lax cookies alone do not prevent. It is a no-op unless PANSY_BASE_URL +// is set (so it never interferes with the local Vite dev proxy), and it allows +// requests with no Origin (non-browser clients like curl, which aren't a CSRF +// vector). Safe methods are always allowed. +func (h *handlers) csrfGuard() gin.HandlerFunc { + var wantHost string + if u, err := url.Parse(h.cfg.BaseURL); err == nil { + wantHost = u.Host + } + return func(c *gin.Context) { + if wantHost == "" { + c.Next() + return + } + switch c.Request.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + c.Next() + return + } + if origin := c.GetHeader("Origin"); origin != "" { + if u, err := url.Parse(origin); err != nil || !strings.EqualFold(u.Host, wantHost) { + writeAPIError(c, http.StatusForbidden, "CROSS_ORIGIN", "cross-origin request rejected") + c.Abort() + return + } + } + c.Next() + } +} + +// startSessionAndRespond issues a session, sets the cookie, and returns the user +// as JSON — the shared tail of register and login. +func (h *handlers) startSessionAndRespond(c *gin.Context, user *domain.User) { + token, expiresAt, err := h.svc.CreateSession(c.Request.Context(), user.ID) if err != nil { - return err + // The account exists and is usable via login; only the auto-login cookie + // failed. Log it so the operator can see the underlying DB problem. + slog.Error("api: could not start session", "user_id", user.ID, "error", err) + writeAPIError(c, http.StatusInternalServerError, "INTERNAL", "could not start session") + return } h.setSessionCookie(c, token, expiresAt) - return nil + c.JSON(http.StatusOK, user) } // cookieSecure reports whether the session cookie should carry the Secure @@ -143,6 +183,10 @@ func (h *handlers) cookieSecure() bool { } func (h *handlers) setSessionCookie(c *gin.Context, token string, expiresAt time.Time) { + // Callers only ever pass a future expiry (login/register set now+TTL; + // requireAuth uses a session already checked to be unexpired), so this is + // always well above the floor; the floor merely avoids emitting a delete + // cookie (MaxAge<=0) from a pathological clock skew. maxAge := max(int(time.Until(expiresAt).Seconds()), 1) c.SetSameSite(http.SameSiteLaxMode) c.SetCookie(sessionCookie, token, maxAge, "/", "", h.cookieSecure(), true) @@ -161,7 +205,7 @@ func mustActor(c *gin.Context) *domain.User { // writeServiceError maps a service-layer sentinel error to pansy's JSON error // envelope. Login failures never leak which of email/password was wrong. -func (h *handlers) writeServiceError(c *gin.Context, err error) { +func writeServiceError(c *gin.Context, err error) { switch { case errors.Is(err, domain.ErrInvalidCredentials): writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password") diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 60215d9..af0b338 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -35,16 +35,22 @@ func localCfg() *config.Config { return &config.Config{Registration: config.RegistrationOpen, LocalAuth: true} } -// doJSON issues a JSON request, optionally carrying a session cookie. -func doJSON(t *testing.T, r *gin.Engine, method, path string, body any, cookie *http.Cookie) *httptest.ResponseRecorder { +// encodeBody JSON-encodes v into a reader for a request body. +func encodeBody(t *testing.T, v any) *bytes.Buffer { t.Helper() var buf bytes.Buffer - if body != nil { - if err := json.NewEncoder(&buf).Encode(body); err != nil { + if v != nil { + if err := json.NewEncoder(&buf).Encode(v); err != nil { t.Fatalf("encode body: %v", err) } } - req := httptest.NewRequest(method, path, &buf) + return &buf +} + +// doJSON issues a JSON request, optionally carrying a session cookie. +func doJSON(t *testing.T, r *gin.Engine, method, path string, body any, cookie *http.Cookie) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(method, path, encodeBody(t, body)) req.Header.Set("Content-Type", "application/json") if cookie != nil { req.AddCookie(cookie) @@ -153,6 +159,71 @@ func TestProvidersEndpoint(t *testing.T) { } } +func TestAuthenticatedRequestRefreshesCookie(t *testing.T) { + // requireAuth must re-set the session cookie so the browser's Max-Age slides + // with the server session rather than expiring 30 days after login. + r := authEngine(t, localCfg()) + w := doJSON(t, r, http.MethodPost, "/api/v1/auth/register", + map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"}, nil) + cookie := sessionCookieFrom(t, w) + + w = doJSON(t, r, http.MethodGet, "/api/v1/auth/me", nil, cookie) + if w.Code != http.StatusOK { + t.Fatalf("me status = %d", w.Code) + } + // A fresh session cookie should be present on the /me response. + refreshed := sessionCookieFrom(t, w) + if refreshed.Value != cookie.Value { + t.Errorf("refreshed cookie value changed: got %q want same token", refreshed.Value) + } + if refreshed.MaxAge <= 0 { + t.Errorf("refreshed cookie Max-Age = %d, want > 0", refreshed.MaxAge) + } +} + +func TestCSRFGuardRejectsCrossOrigin(t *testing.T) { + cfg := localCfg() + cfg.BaseURL = "https://pansy.example.com" + r := authEngine(t, cfg) + + body := map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"} + + // Cross-origin POST is rejected before touching the service. + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/register", encodeBody(t, body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Origin", "https://evil.example.com") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("cross-origin register status = %d, want 403", w.Code) + } + + // Same-origin POST is allowed. + req = httptest.NewRequest(http.MethodPost, "/api/v1/auth/register", encodeBody(t, body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Origin", "https://pansy.example.com") + w = httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("same-origin register status = %d, want 200 (body %s)", w.Code, w.Body.String()) + } +} + +func TestCSRFGuardNoopWithoutBaseURL(t *testing.T) { + // In local dev (no PANSY_BASE_URL) the guard must not interfere, even with a + // foreign Origin (the Vite dev proxy forwards the browser's origin). + r := authEngine(t, localCfg()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/register", + encodeBody(t, map[string]string{"email": "a@example.com", "displayName": "Alice", "password": "password123"})) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Origin", "http://localhost:5173") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("dev cross-origin register status = %d, want 200", w.Code) + } +} + func TestSecureCookieWithHTTPSBaseURL(t *testing.T) { cfg := localCfg() cfg.BaseURL = "https://pansy.example.com" diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 7561146..57e4766 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -117,25 +117,25 @@ type GardenShare struct { // GardenObject is any placeable object in a garden (bed, container, tree, path…). // Positioned by center point + rotation about center, in garden space. type GardenObject struct { - ID int64 `json:"id"` - GardenID int64 `json:"gardenId"` - Kind string `json:"kind"` - Name string `json:"name"` - Shape string `json:"shape"` - Points *string `json:"points,omitempty"` // reserved: JSON for polygons - XCM float64 `json:"xCm"` - YCM float64 `json:"yCm"` - WidthCM float64 `json:"widthCm"` - HeightCM float64 `json:"heightCm"` - RotationDeg float64 `json:"rotationDeg"` - ZIndex int `json:"zIndex"` - Plantable bool `json:"plantable"` - Color *string `json:"color,omitempty"` - Props *string `json:"props,omitempty"` // kind-specific JSON - Notes string `json:"notes"` - Version int64 `json:"version"` - CreatedAt string `json:"createdAt"` - UpdatedAt string `json:"updatedAt"` + ID int64 `json:"id"` + GardenID int64 `json:"gardenId"` + Kind string `json:"kind"` + Name string `json:"name"` + Shape string `json:"shape"` + Points *string `json:"points,omitempty"` // reserved: JSON for polygons + XCM float64 `json:"xCm"` + YCM float64 `json:"yCm"` + WidthCM float64 `json:"widthCm"` + HeightCM float64 `json:"heightCm"` + RotationDeg float64 `json:"rotationDeg"` + ZIndex int `json:"zIndex"` + Plantable bool `json:"plantable"` + Color *string `json:"color,omitempty"` + Props *string `json:"props,omitempty"` // kind-specific JSON + Notes string `json:"notes"` + Version int64 `json:"version"` + CreatedAt string `json:"createdAt"` + UpdatedAt string `json:"updatedAt"` } // Plant is a catalog entry. OwnerID nil means a read-only seeded built-in. @@ -157,17 +157,17 @@ type Plant struct { // Planting ("plop") is a circular patch of one plant, positioned in its parent // object's local frame. Count nil means it is derived from area / spacing². type Planting struct { - ID int64 `json:"id"` - ObjectID int64 `json:"objectId"` - PlantID int64 `json:"plantId"` - XCM float64 `json:"xCm"` - YCM float64 `json:"yCm"` - RadiusCM float64 `json:"radiusCm"` - Count *int `json:"count,omitempty"` // nil = derived - Label *string `json:"label,omitempty"` - PlantedAt *string `json:"plantedAt,omitempty"` - RemovedAt *string `json:"removedAt,omitempty"` - Version int64 `json:"version"` - CreatedAt string `json:"createdAt"` - UpdatedAt string `json:"updatedAt"` + ID int64 `json:"id"` + ObjectID int64 `json:"objectId"` + PlantID int64 `json:"plantId"` + XCM float64 `json:"xCm"` + YCM float64 `json:"yCm"` + RadiusCM float64 `json:"radiusCm"` + Count *int `json:"count,omitempty"` // nil = derived + Label *string `json:"label,omitempty"` + PlantedAt *string `json:"plantedAt,omitempty"` + RemovedAt *string `json:"removedAt,omitempty"` + Version int64 `json:"version"` + CreatedAt string `json:"createdAt"` + UpdatedAt string `json:"updatedAt"` } diff --git a/internal/service/auth.go b/internal/service/auth.go index c5da735..6d284b2 100644 --- a/internal/service/auth.go +++ b/internal/service/auth.go @@ -3,12 +3,17 @@ package service import ( "context" "errors" + "log/slog" "strings" "time" "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" ) +// maxPasswordLen caps accepted password length. It bounds argon2 input and +// request work, and sits far above any real password. +const maxPasswordLen = 1024 + // RegisterInput is the payload for local self-service signup. type RegisterInput struct { Email string @@ -46,27 +51,22 @@ func (s *Service) Register(ctx context.Context, in RegisterInput) (*domain.User, email := normalizeEmail(in.Email) displayName := strings.TrimSpace(in.DisplayName) - if email == "" || displayName == "" || in.Password == "" { + if email == "" || displayName == "" || in.Password == "" || len(in.Password) > maxPasswordLen { return nil, domain.ErrInvalidInput } - count, err := s.store.CountUsers(ctx) - if err != nil { - return nil, err - } - // Closed registration still allows the very first account so a locked-down - // instance can be bootstrapped without editing config. - if count > 0 && !s.cfg.RegistrationOpen() { - return nil, domain.ErrRegistrationClosed - } - - // Friendly duplicate check. The UNIQUE index is the real guard against a - // race; that path surfaces as a generic insert error (500), acceptable at - // household scale. - if _, err := s.store.GetUserByEmail(ctx, email); err == nil { - return nil, domain.ErrEmailTaken - } else if !errors.Is(err, domain.ErrNotFound) { - return nil, err + // Cheap best-effort gate so a closed instance doesn't burn argon2 work on + // signups it will reject anyway. The authoritative, race-free gate — plus + // atomic first-user-is-admin assignment and duplicate-email detection — lives + // in store.CreateUser's single INSERT. + if !s.cfg.RegistrationOpen() { + n, err := s.store.CountUsers(ctx) + if err != nil { + return nil, err + } + if n > 0 { + return nil, domain.ErrRegistrationClosed + } } hash, err := hashPassword(in.Password) @@ -78,8 +78,7 @@ func (s *Service) Register(ctx context.Context, in RegisterInput) (*domain.User, Email: email, DisplayName: displayName, PasswordHash: &hash, - IsAdmin: count == 0, - }) + }, s.cfg.RegistrationOpen()) } // Login verifies an email/password pair and returns the user. Unknown-email and @@ -89,6 +88,10 @@ func (s *Service) Login(ctx context.Context, email, password string) (*domain.Us if !s.cfg.LocalAuth { return nil, domain.ErrLocalAuthDisabled } + if len(password) > maxPasswordLen { + // No stored password is this long; reject without spending argon2 work. + return nil, domain.ErrInvalidCredentials + } u, err := s.store.GetUserByEmail(ctx, normalizeEmail(email)) switch { @@ -105,7 +108,13 @@ func (s *Service) Login(ctx context.Context, email, password string) (*domain.Us } ok, err := verifyPassword(*u.PasswordHash, password) - if err != nil || !ok { + if err != nil { + // A stored hash we can't parse is a data problem, not a wrong password; + // surface it so a corrupt row doesn't silently lock a user out unnoticed. + slog.Error("service: malformed stored password hash", "user_id", u.ID, "error", err) + return nil, domain.ErrInvalidCredentials + } + if !ok { return nil, domain.ErrInvalidCredentials } return u, nil @@ -129,38 +138,53 @@ func (s *Service) CreateSession(ctx context.Context, userID int64) (token string return raw, exp, nil } -// ResolveSession validates a raw bearer token and returns its user. An expired -// session is deleted and treated as absent (domain.ErrNotFound). A still-valid -// session has its expiry slid forward, but only when that moves it by more than -// an hour, so a busy client doesn't write on every request. -func (s *Service) ResolveSession(ctx context.Context, rawToken string) (*domain.User, error) { +// ResolveSession validates a raw bearer token and returns its user and the +// session's current expiry (so the caller can slide the client cookie in +// lockstep with the server). An expired session is deleted and treated as absent +// (domain.ErrNotFound). A still-valid session has its expiry slid forward, but +// only when that moves it by more than an hour, so a busy client doesn't write +// on every request. +func (s *Service) ResolveSession(ctx context.Context, rawToken string) (*domain.User, time.Time, error) { if rawToken == "" { - return nil, domain.ErrNotFound + return nil, time.Time{}, domain.ErrNotFound } hash := hashToken(rawToken) sess, err := s.store.GetSession(ctx, hash) if err != nil { - return nil, err + return nil, time.Time{}, err } exp, err := parseTime(sess.ExpiresAt) if err != nil { // A corrupt expiry means we can't trust the session; drop it. - _ = s.store.DeleteSession(ctx, hash) - return nil, domain.ErrNotFound + if delErr := s.store.DeleteSession(ctx, hash); delErr != nil { + slog.Warn("service: deleting session with corrupt expiry", "error", delErr) + } + return nil, time.Time{}, domain.ErrNotFound } now := s.now() if !now.Before(exp) { - _ = s.store.DeleteSession(ctx, hash) - return nil, domain.ErrNotFound + if delErr := s.store.DeleteSession(ctx, hash); delErr != nil { + slog.Warn("service: deleting expired session", "error", delErr) + } + return nil, time.Time{}, domain.ErrNotFound } if newExp := now.Add(sessionTTL); newExp.Sub(exp) > time.Hour { - _ = s.store.TouchSession(ctx, hash, formatTime(newExp)) + if err := s.store.TouchSession(ctx, hash, formatTime(newExp)); err != nil { + // Non-fatal: the session is still valid at its current expiry. + slog.Warn("service: sliding session expiry", "error", err) + } else { + exp = newExp + } } - return s.store.GetUserByID(ctx, sess.UserID) + user, err := s.store.GetUserByID(ctx, sess.UserID) + if err != nil { + return nil, time.Time{}, err + } + return user, exp, nil } // Logout deletes the session behind a raw bearer token. It is idempotent. diff --git a/internal/service/auth_test.go b/internal/service/auth_test.go index f583452..5a85bde 100644 --- a/internal/service/auth_test.go +++ b/internal/service/auth_test.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "strings" "testing" "time" @@ -98,6 +99,20 @@ func TestRegistrationClosedAllowsBootstrapThenBlocks(t *testing.T) { } } +func TestRejectsOverlongPassword(t *testing.T) { + s := newTestService(t, openConfig()) + long := strings.Repeat("a", maxPasswordLen+1) + + if _, err := s.Register(context.Background(), RegisterInput{Email: "a@example.com", DisplayName: "A", Password: long}); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("register overlong err = %v, want ErrInvalidInput", err) + } + + mustRegister(t, s, "b@example.com", "Bob", "password123") + if _, err := s.Login(context.Background(), "b@example.com", long); !errors.Is(err, domain.ErrInvalidCredentials) { + t.Errorf("login overlong err = %v, want ErrInvalidCredentials", err) + } +} + func TestLocalAuthDisabledRejectsRegisterAndLogin(t *testing.T) { cfg := openConfig() cfg.LocalAuth = false @@ -139,7 +154,7 @@ func TestLoginRejectsOIDCOnlyUser(t *testing.T) { iss, sub := "https://idp.example", "subject-1" if _, err := s.store.CreateUser(context.Background(), &domain.User{ Email: "oidc@example.com", DisplayName: "O", OIDCIssuer: &iss, OIDCSubject: &sub, - }); err != nil { + }, true); err != nil { t.Fatalf("seed oidc user: %v", err) } if _, err := s.Login(context.Background(), "oidc@example.com", "anything"); !errors.Is(err, domain.ErrInvalidCredentials) { @@ -156,29 +171,32 @@ func TestSessionLifecycle(t *testing.T) { t.Fatalf("CreateSession: %v", err) } - got, err := s.ResolveSession(context.Background(), token) + got, exp, err := s.ResolveSession(context.Background(), token) if err != nil { t.Fatalf("ResolveSession: %v", err) } if got.ID != u.ID { t.Errorf("resolved user %d, want %d", got.ID, u.ID) } + if !exp.After(s.now()) { + t.Errorf("resolved expiry %v is not in the future", exp) + } // Logout invalidates it. if err := s.Logout(context.Background(), token); err != nil { t.Fatalf("Logout: %v", err) } - if _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { + if _, _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { t.Errorf("resolve after logout err = %v, want ErrNotFound", err) } } func TestResolveSessionRejectsGarbageToken(t *testing.T) { s := newTestService(t, openConfig()) - if _, err := s.ResolveSession(context.Background(), "not-a-real-token"); !errors.Is(err, domain.ErrNotFound) { + if _, _, err := s.ResolveSession(context.Background(), "not-a-real-token"); !errors.Is(err, domain.ErrNotFound) { t.Errorf("garbage token err = %v, want ErrNotFound", err) } - if _, err := s.ResolveSession(context.Background(), ""); !errors.Is(err, domain.ErrNotFound) { + if _, _, err := s.ResolveSession(context.Background(), ""); !errors.Is(err, domain.ErrNotFound) { t.Errorf("empty token err = %v, want ErrNotFound", err) } } @@ -197,7 +215,7 @@ func TestSessionExpiryAndLazyDeletion(t *testing.T) { // Jump past the 30-day TTL: the session must be treated as gone... s.now = func() time.Time { return base.Add(sessionTTL + time.Hour) } - if _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { + if _, _, err := s.ResolveSession(context.Background(), token); !errors.Is(err, domain.ErrNotFound) { t.Fatalf("expired resolve err = %v, want ErrNotFound", err) } // ...and lazily deleted from the store. @@ -219,9 +237,13 @@ func TestSessionSlidingRenewal(t *testing.T) { // Use it 10 days later; expiry should slide forward. s.now = func() time.Time { return base.Add(10 * 24 * time.Hour) } - if _, err := s.ResolveSession(context.Background(), token); err != nil { + _, resolvedExp, err := s.ResolveSession(context.Background(), token) + if err != nil { t.Fatalf("ResolveSession: %v", err) } + if !resolvedExp.After(firstExp) { + t.Errorf("returned expiry did not slide: %v not after %v", resolvedExp, firstExp) + } sess, err := s.store.GetSession(context.Background(), hashToken(token)) if err != nil { t.Fatalf("GetSession: %v", err) diff --git a/internal/service/password.go b/internal/service/password.go index fa34ae6..634b554 100644 --- a/internal/service/password.go +++ b/internal/service/password.go @@ -11,13 +11,13 @@ import ( "golang.org/x/crypto/argon2" ) -// argon2id parameters. ~64 MiB memory / 1 pass / 4 lanes is the interactive -// profile recommended by the argon2 authors and is comfortable on a -// self-hosted box. They are encoded into every stored hash, so raising them -// later leaves old hashes verifiable. +// argon2id parameters: RFC 9106's second recommended profile (m=64 MiB, t=3, +// p=4) — the memory-constrained option, appropriate for a self-hosted box where +// the 2 GiB first profile is too heavy. They are encoded into every stored hash, +// so raising them later leaves old hashes verifiable. const ( argonMemKiB = 64 * 1024 // 64 MiB - argonTime = 1 + argonTime = 3 argonThreads = 4 argonKeyLen = 32 argonSaltLen = 16 @@ -25,12 +25,27 @@ const ( // errBadHash marks a stored hash string that could not be parsed — a data or // programming error, not a wrong password. Callers treat it as an auth failure -// but should log it. +// and log it (Login does). var errBadHash = errors.New("service: malformed password hash") // b64 is the padding-free base64 used inside the PHC-style hash string. var b64 = base64.RawStdEncoding +// timingHash is a valid argon2id hash used to equalize login response time when +// an email is unknown (see Service.dummyHash). It uses a fixed salt rather than +// crypto/rand so it can never fail to be produced at startup — an all-important +// property, since a missing timing hash would silently re-open account +// enumeration. It guards nothing, so a static salt is safe; deriving it from the +// live argon parameters keeps its cost matched to a real verify. +func timingHash() string { + salt := []byte("pansy-timing-eq!") // exactly argonSaltLen (16) bytes + key := argon2.IDKey([]byte("x"), salt, argonTime, argonMemKiB, argonThreads, argonKeyLen) + return fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", + argon2.Version, argonMemKiB, argonTime, argonThreads, + b64.EncodeToString(salt), b64.EncodeToString(key), + ) +} + // hashPassword returns a self-describing argon2id hash in the PHC string format // "$argon2id$v=19$m=...,t=...,p=...$salt$hash" (all base64, no padding). func hashPassword(password string) (string, error) { @@ -57,26 +72,31 @@ func verifyPassword(encoded, password string) (bool, error) { } // decodeHash parses a PHC-format argon2id string into its parameters, salt, and -// derived key. +// derived key. Any structural problem returns errBadHash (with zero values) so +// the caller never has to distinguish parse failures. func decodeHash(encoded string) (mem, t uint32, threads uint8, salt, key []byte, err error) { - parts := strings.Split(encoded, "$") - // ["", "argon2id", "v=19", "m=..,t=..,p=..", "", ""] - if len(parts) != 6 || parts[1] != "argon2id" { + fail := func() (uint32, uint32, uint8, []byte, []byte, error) { return 0, 0, 0, nil, nil, errBadHash } - var version int - if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil || version != argon2.Version { - return 0, 0, 0, nil, nil, errBadHash + parts := strings.Split(encoded, "$") + // ["", "argon2id", "v=19", "m=..,t=..,p=..", "", ""] + if len(parts) != 6 || parts[1] != "argon2id" { + return fail() } - if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &mem, &t, &threads); err != nil { - return 0, 0, 0, nil, nil, errBadHash + + var version int + if _, e := fmt.Sscanf(parts[2], "v=%d", &version); e != nil || version != argon2.Version { + return fail() + } + if _, e := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &mem, &t, &threads); e != nil { + return fail() } if salt, err = b64.DecodeString(parts[4]); err != nil { - return 0, 0, 0, nil, nil, errBadHash + return fail() } if key, err = b64.DecodeString(parts[5]); err != nil || len(key) == 0 { - return 0, 0, 0, nil, nil, errBadHash + return fail() } return mem, t, threads, salt, key, nil } diff --git a/internal/service/password_test.go b/internal/service/password_test.go index 8470555..8e6b486 100644 --- a/internal/service/password_test.go +++ b/internal/service/password_test.go @@ -45,7 +45,7 @@ func TestVerifyPasswordRejectsMalformedHash(t *testing.T) { "not-a-hash", "$argon2id$v=19$m=65536,t=1,p=4$onlyfourparts", "$argon2i$v=19$m=65536,t=1,p=4$c2FsdA$aGFzaA", // wrong variant - "$argon2id$v=1$m=65536,t=1,p=4$c2FsdA$aGFzaA", // wrong version + "$argon2id$v=1$m=65536,t=1,p=4$c2FsdA$aGFzaA", // wrong version } { if _, err := verifyPassword(bad, "whatever"); err == nil { t.Errorf("verifyPassword(%q) err = nil, want errBadHash", bad) diff --git a/internal/service/service.go b/internal/service/service.go index bf6aadc..b33cedd 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -1,9 +1,11 @@ -// Package service is pansy's business-logic seam: every operation is a method on -// *Service taking (ctx, actor, args), and all permission checks and invariants -// live here rather than in the HTTP handlers. REST handlers (internal/api) and, -// later, agent tools (internal/agent) are thin adapters over these methods, so -// both inherit the same rules. This file holds the shared plumbing; feature -// methods live alongside it (auth.go, and gardens/objects/… in later issues). +// Package service is pansy's business-logic seam: all permission checks and +// invariants live here rather than in the HTTP handlers. Resource operations +// take (ctx, actor, args) so every rule is enforced regardless of caller; the +// auth operations here are the exception — they establish the actor, so they +// take credentials rather than one. REST handlers (internal/api) and, later, +// agent tools (internal/agent) are thin adapters over these methods, so both +// inherit the same rules. This file holds the shared plumbing; feature methods +// live alongside it (auth.go, and gardens/objects/… in later issues). package service import ( @@ -12,7 +14,6 @@ import ( "encoding/base64" "encoding/hex" "fmt" - "log/slog" "time" "gitea.stevedudenhoeffer.com/steve/pansy/internal/config" @@ -33,23 +34,21 @@ type Service struct { cfg *config.Config // now is the clock, injectable so tests can advance time (session expiry). now func() time.Time - // dummyHash is a valid argon2id hash of a throwaway password. Login verifies - // against it when an email is unknown so the response time doesn't reveal - // whether an account exists. + // dummyHash is a valid argon2id hash Login verifies against when an email is + // unknown, so response time doesn't reveal whether an account exists. It is + // produced by timingHash (fixed salt, no RNG) so it is always present — an + // empty one would silently re-open account enumeration. dummyHash string } -// New constructs a Service. It precomputes a dummy password hash used to -// equalize login timing; if that fails (it shouldn't), login still works but -// loses the timing defense. +// New constructs a Service. func New(st *store.DB, cfg *config.Config) *Service { - s := &Service{store: st, cfg: cfg, now: time.Now} - if h, err := hashPassword("pansy-timing-equalizer-not-a-real-password"); err != nil { - slog.Warn("service: could not precompute login timing hash", "error", err) - } else { - s.dummyHash = h + return &Service{ + store: st, + cfg: cfg, + now: time.Now, + dummyHash: timingHash(), } - return s } // formatTime renders a time as pansy's canonical UTC string. diff --git a/internal/store/migrations/0002_sessions_expires_index.sql b/internal/store/migrations/0002_sessions_expires_index.sql new file mode 100644 index 0000000..a956310 --- /dev/null +++ b/internal/store/migrations/0002_sessions_expires_index.sql @@ -0,0 +1,7 @@ +-- 0002_sessions_expires_index.sql — index sessions.expires_at. +-- +-- The periodic sweep (and lazy cleanup) run `DELETE FROM sessions WHERE +-- expires_at <= ?`; without this index that is a full table scan. The table is +-- small at household scale, but the index is cheap insurance and keeps the sweep +-- O(expired) rather than O(all sessions). +CREATE INDEX idx_sessions_expires ON sessions (expires_at); diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 9cd5154..9c5e4a3 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -42,15 +42,15 @@ func TestMigrateCreatesSchema(t *testing.T) { if err := db.SQL().QueryRow(`SELECT max(version) FROM schema_migrations`).Scan(&version); err != nil { t.Fatalf("read schema_migrations: %v", err) } - if version != 1 { - t.Errorf("schema version = %d, want 1", version) + if version != 2 { + t.Errorf("schema version = %d, want 2", version) } } func TestMigrateIsIdempotent(t *testing.T) { db := openTestDB(t) - // A second Migrate must be a no-op and leave exactly one recorded version. + // A second Migrate must be a no-op and leave exactly one row per migration. if err := db.Migrate(context.Background()); err != nil { t.Fatalf("second Migrate: %v", err) } @@ -58,8 +58,8 @@ func TestMigrateIsIdempotent(t *testing.T) { if err := db.SQL().QueryRow(`SELECT count(*) FROM schema_migrations`).Scan(&count); err != nil { t.Fatalf("count schema_migrations: %v", err) } - if count != 1 { - t.Errorf("schema_migrations rows = %d, want 1", count) + if count != 2 { + t.Errorf("schema_migrations rows = %d, want 2", count) } } diff --git a/internal/store/users.go b/internal/store/users.go index e92a36b..a7e3548 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" ) @@ -20,7 +21,7 @@ type scanner interface { // scanUser reads one users row. is_admin is stored as INTEGER 0/1 (the driver // returns it as int64, which does not convert straight to bool), so it is read -// into an int and mapped. +// into an int64 and mapped. func scanUser(s scanner) (*domain.User, error) { var ( u domain.User @@ -39,15 +40,37 @@ func scanUser(s scanner) (*domain.User, error) { // CreateUser inserts a new user and returns the stored row (with generated id // and timestamps). PasswordHash/OIDCIssuer/OIDCSubject may be nil. -func (d *DB) CreateUser(ctx context.Context, u *domain.User) (*domain.User, error) { +// +// Two invariants are enforced inside the single INSERT statement so concurrent +// registrations can't violate them (SQLite serializes writers, and the COUNT +// subqueries see the latest committed state): +// - is_admin is set iff this is the first user — no read-then-write window in +// which two "first" registrations both become admin. +// - the row is inserted only when the table is empty (bootstrap) or allowSignup +// is true; otherwise zero rows are affected and ErrRegistrationClosed is +// returned. This is the authoritative registration gate. +// +// A duplicate email trips the UNIQUE index and maps to ErrEmailTaken. +func (d *DB) CreateUser(ctx context.Context, u *domain.User, allowSignup bool) (*domain.User, error) { res, err := d.sql.ExecContext(ctx, `INSERT INTO users (email, display_name, password_hash, oidc_issuer, oidc_subject, is_admin) - VALUES (?, ?, ?, ?, ?, ?)`, - u.Email, u.DisplayName, u.PasswordHash, u.OIDCIssuer, u.OIDCSubject, boolToInt(u.IsAdmin), + SELECT ?, ?, ?, ?, ?, (SELECT count(*) FROM users) = 0 + WHERE (SELECT count(*) FROM users) = 0 OR ?`, + u.Email, u.DisplayName, u.PasswordHash, u.OIDCIssuer, u.OIDCSubject, boolToInt(allowSignup), ) if err != nil { + if isUniqueViolation(err) { + return nil, domain.ErrEmailTaken + } return nil, fmt.Errorf("store: insert user: %w", err) } + n, err := res.RowsAffected() + if err != nil { + return nil, fmt.Errorf("store: user insert rows: %w", err) + } + if n == 0 { + return nil, domain.ErrRegistrationClosed + } id, err := res.LastInsertId() if err != nil { return nil, fmt.Errorf("store: user insert id: %w", err) @@ -99,3 +122,10 @@ func boolToInt(b bool) int { } return 0 } + +// isUniqueViolation reports whether err is a SQLite UNIQUE-constraint failure. +// modernc surfaces these in the error text; the message is stable across SQLite +// versions ("UNIQUE constraint failed: ."). +func isUniqueViolation(err error) bool { + return err != nil && strings.Contains(err.Error(), "UNIQUE constraint failed") +}