Local auth: users, sessions, register/login/logout/me (#4) #23

Merged
steve merged 2 commits from phase-1-local-auth into main 2026-07-18 21:05:01 +00:00
14 changed files with 1275 additions and 7 deletions
Showing only changes of commit 0e41ccd95a - Show all commits
+1 -1
View File
@@ -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
+32 -1
View File
@@ -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:
}
}
}
+1 -1
View File
@@ -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
+24 -4
View File
@@ -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
}
+180
View File
@@ -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 {
Review

🟠 Password length validation only in HTTP binding tags, not service layer

maintainability · flagged by 1 model

  • internal/api/auth.go:25-28 Password length validation only in HTTP binding tags, not service layer — registerRequest enforces min=8,max=1024 via gin binding tags, but service.Register (internal/service/auth.go:48-50) only checks in.Password == "". The service layer is documented as the place where "all permission checks and invariants live" (internal/service/service.go:2-4), so a caller bypassing REST (e.g. an agent tool or internal caller) will skip the length check. **Fix:*…

🪰 Gadfly · advisory

🟠 **Password length validation only in HTTP binding tags, not service layer** _maintainability · flagged by 1 model_ - **`internal/api/auth.go:25-28`** Password length validation only in HTTP binding tags, not service layer — `registerRequest` enforces `min=8,max=1024` via gin binding tags, but `service.Register` (`internal/service/auth.go:48-50`) only checks `in.Password == ""`. The service layer is documented as the place where "all permission checks and invariants live" (`internal/service/service.go:2-4`), so a caller bypassing REST (e.g. an agent tool or internal caller) will skip the length check. **Fix:*… <sub>🪰 Gadfly · advisory</sub>
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"`
Review

🟡 loginRequest missing max password boundary

error-handling · flagged by 1 model

🪰 Gadfly · advisory

🟡 **loginRequest missing max password boundary** _error-handling · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
}
// 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 {
Review

🟠 startSession error in register/login is unlogged; operator-invisible, leaves half-created user on register

correctness, error-handling, maintainability · flagged by 3 models

  • internal/api/auth.go:54-57 & :75-78startSession failure is swallowed (unlogged). When CreateSession returns an error (DB write failure, disk full, unique constraint), the handler returns a generic 500 envelope without logging the underlying cause. The logout handler (auth.go:87) and writeServiceError's default branch (auth.go:177) both slog.Error the cause; these two paths bypass that. For register this is especially bad: the user row is already committed, so the client…

🪰 Gadfly · advisory

🟠 **startSession error in register/login is unlogged; operator-invisible, leaves half-created user on register** _correctness, error-handling, maintainability · flagged by 3 models_ - **`internal/api/auth.go:54-57` & `:75-78` — `startSession` failure is swallowed (unlogged).** When `CreateSession` returns an error (DB write failure, disk full, unique constraint), the handler returns a generic 500 envelope without logging the underlying cause. The `logout` handler (auth.go:87) and `writeServiceError`'s default branch (auth.go:177) both `slog.Error` the cause; these two paths bypass that. For `register` this is especially bad: the user row is already committed, so the client… <sub>🪰 Gadfly · advisory</sub>
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) {
Review

🟡 Login CSRF: no Origin/Referer validation on /auth/login (and other auth POSTs) with cookie-based sessions; SameSite=Lax does not block cross-site Set-Cookie on login

security · flagged by 1 model

  • Login CSRF via unvalidated Origin on auth POSTs (internal/api/auth.go:62-92, internal/api/auth.go:37-59). The session is a cookie (HttpOnly, SameSite=Lax), and SameSite=Lax blocks sending existing cookies on cross-site POSTs but does not block a cross-site POST to /auth/login from being processed and from Set-Cookie taking effect on the victim's browser. An attacker page can fetch('…/api/v1/auth/login', {method:'POST', credentials:'include', …}) with the attacker's own cred…

🪰 Gadfly · advisory

🟡 **Login CSRF: no Origin/Referer validation on /auth/login (and other auth POSTs) with cookie-based sessions; SameSite=Lax does not block cross-site Set-Cookie on login** _security · flagged by 1 model_ - **Login CSRF via unvalidated Origin on auth POSTs** (`internal/api/auth.go:62-92`, `internal/api/auth.go:37-59`). The session is a cookie (HttpOnly, SameSite=Lax), and SameSite=Lax blocks sending *existing* cookies on cross-site POSTs but does **not** block a cross-site POST to `/auth/login` from being processed and from `Set-Cookie` taking effect on the victim's browser. An attacker page can `fetch('…/api/v1/auth/login', {method:'POST', credentials:'include', …})` with the attacker's own cred… <sub>🪰 Gadfly · advisory</sub>
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")
Review

requireAuth repeats an identical unauthorized-response triplet in two branches

maintainability · flagged by 1 model

  • internal/api/auth.go:111 and internal/api/auth.go:119 — the two failure branches in requireAuth (missing cookie and ResolveSession error) write the exact same writeAPIError(...) / c.Abort() / return triplet. Verified by reading the middleware. Minor, but a one-line unauthorized(c) helper would remove the duplication and make it obvious both paths are intentionally treated the same (the comment already explains why, so the intent isn't lost — just the repetition).

🪰 Gadfly · advisory

⚪ **requireAuth repeats an identical unauthorized-response triplet in two branches** _maintainability · flagged by 1 model_ - `internal/api/auth.go:111` and `internal/api/auth.go:119` — the two failure branches in `requireAuth` (`missing cookie` and `ResolveSession error`) write the exact same `writeAPIError(...) / c.Abort() / return` triplet. Verified by reading the middleware. Minor, but a one-line `unauthorized(c)` helper would remove the duplication and make it obvious both paths are intentionally treated the same (the comment already explains why, so the intent isn't lost — just the repetition). <sub>🪰 Gadfly · advisory</sub>
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")
}
}
+168
View File
@@ -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": "[email protected]", "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": "[email protected]", "displayName": "Alice", "password": "password123"}, nil)
w := doJSON(t, r, http.MethodPost, "/api/v1/auth/login",
map[string]string{"email": "[email protected]", "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": "[email protected]", "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": "[email protected]", "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")
}
}
+18
View File
@@ -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.
+186
View File
@@ -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
}
Review

🟠 TOCTOU race between CountUsers and CreateUser in Register lets concurrent first-time registrations both become admin and both bypass PANSY_REGISTRATION=closed bootstrap gating

correctness · flagged by 2 models

  • internal/service/auth.go:49 — Service layer does not enforce the 1024-character password ceiling Register validates in.Password == "" but never checks len(in.Password) > 1024. The API binding catches it today, but the PR positions internal/service as the seam for “future agent tools”. A direct caller (CLI, agent, test helper) can pass an arbitrarily large password, and the service will hash it rather than reject it. Fix: Add len(in.Password) > 1024 to the service-level vali…

🪰 Gadfly · advisory

🟠 **TOCTOU race between CountUsers and CreateUser in Register lets concurrent first-time registrations both become admin and both bypass PANSY_REGISTRATION=closed bootstrap gating** _correctness · flagged by 2 models_ - **`internal/service/auth.go:49` — Service layer does not enforce the 1024-character password ceiling** `Register` validates `in.Password == ""` but never checks `len(in.Password) > 1024`. The API binding catches it today, but the PR positions `internal/service` as the seam for “future agent tools”. A direct caller (CLI, agent, test helper) can pass an arbitrarily large password, and the service will hash it rather than reject it. **Fix:** Add `len(in.Password) > 1024` to the service-level vali… <sub>🪰 Gadfly · advisory</sub>
email := normalizeEmail(in.Email)
displayName := strings.TrimSpace(in.DisplayName)
if email == "" || displayName == "" || in.Password == "" {
return nil, domain.ErrInvalidInput
}
count, err := s.store.CountUsers(ctx)
Review

🔴 TOCTOU race in Register allows multiple bootstrap admins

correctness, error-handling, security · flagged by 3 models

  • TOCTOU race allows multiple bootstrap admins on a fresh instance internal/service/auth.go:53-82Register reads CountUsers, checks count > 0 && !RegistrationOpen(), then calls CreateUser. Two concurrent requests on a fresh DB can both observe count == 0, pass the closed-registration gate, and both create accounts with IsAdmin: true (as long as they use different emails). This breaks the intended “exactly one bootstrap admin” security model and lets an attacker race the legit…

🪰 Gadfly · advisory

🔴 **TOCTOU race in Register allows multiple bootstrap admins** _correctness, error-handling, security · flagged by 3 models_ - **TOCTOU race allows multiple bootstrap admins on a fresh instance** `internal/service/auth.go:53-82` — `Register` reads `CountUsers`, checks `count > 0 && !RegistrationOpen()`, then calls `CreateUser`. Two concurrent requests on a fresh DB can both observe `count == 0`, pass the closed-registration gate, and both create accounts with `IsAdmin: true` (as long as they use different emails). This breaks the intended “exactly one bootstrap admin” security model and lets an attacker race the legit… <sub>🪰 Gadfly · advisory</sub>
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{
Review

🟡 First-user-is-admin decided by CountUsers+CreateUser without a transaction; concurrent first registrations can both become admin

correctness · flagged by 1 model

  • internal/service/auth.go:77 (Register) — first-user-is-admin is decided by CountUsers then CreateUser with IsAdmin: count == 0, with no transaction or post-insert guard. Two concurrent first registrations with different emails both observe count == 0 and both insert as admin (the UNIQUE index only guards email collisions, which the comment acknowledges; it does not guard the admin grant). Result: two admins on a fresh instance. Low likelihood at household scale, but it is a real…

🪰 Gadfly · advisory

🟡 **First-user-is-admin decided by CountUsers+CreateUser without a transaction; concurrent first registrations can both become admin** _correctness · flagged by 1 model_ - `internal/service/auth.go:77` (`Register`) — first-user-is-admin is decided by `CountUsers` then `CreateUser` with `IsAdmin: count == 0`, with no transaction or post-insert guard. Two concurrent first registrations with *different* emails both observe `count == 0` and both insert as admin (the UNIQUE index only guards email collisions, which the comment acknowledges; it does not guard the admin grant). Result: two admins on a fresh instance. Low likelihood at household scale, but it is a real… <sub>🪰 Gadfly · advisory</sub>
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)
Review

🟠 Empty dummyHash (rand failure in New) short-circuits verifyPassword with zero argon2 work, defeating the timing equalizer it claims to provide

error-handling · flagged by 2 models

  • internal/service/auth.go:97, 103 + internal/service/service.go:47-51 — empty dummyHash defeats the timing equalizer with no runtime signal. If hashPassword fails in New (only path: crypto/rand.Read failure), s.dummyHash stays "". verifyPassword("", password) short-circuits in decodeHash (verified: strings.Split("", "$")[""], length 1 ≠ 6 → errBadHash) doing zero argon2 work. Then unknown-email returns ~instantly while wrong-password pays full argon2 cost —…

🪰 Gadfly · advisory

🟠 **Empty dummyHash (rand failure in New) short-circuits verifyPassword with zero argon2 work, defeating the timing equalizer it claims to provide** _error-handling · flagged by 2 models_ - **`internal/service/auth.go:97, 103` + `internal/service/service.go:47-51` — empty `dummyHash` defeats the timing equalizer with no runtime signal.** If `hashPassword` fails in `New` (only path: `crypto/rand.Read` failure), `s.dummyHash` stays `""`. `verifyPassword("", password)` short-circuits in `decodeHash` (verified: `strings.Split("", "$")` → `[""]`, length 1 ≠ 6 → `errBadHash`) doing **zero** argon2 work. Then unknown-email returns ~instantly while wrong-password pays full argon2 cost —… <sub>🪰 Gadfly · advisory</sub>
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)
Review

🟠 Malformed password hash silently returned as wrong-password with no log

correctness, error-handling · flagged by 1 model

  • internal/service/auth.go:107-110 — Malformed password hash silently returned as wrong-password with no log verifyPassword returns errBadHash when a stored hash is malformed (corruption, partial write, etc.). The password.go comment explicitly says “Callers treat it as an auth failure but should log it”, yet Login maps it straight to domain.ErrInvalidCredentials with no slog.Error. An operator has no server-side signal that data corruption is locking a user out. Fix: Log…

🪰 Gadfly · advisory

🟠 **Malformed password hash silently returned as wrong-password with no log** _correctness, error-handling · flagged by 1 model_ - **`internal/service/auth.go:107-110` — Malformed password hash silently returned as wrong-password with no log** `verifyPassword` returns `errBadHash` when a stored hash is malformed (corruption, partial write, etc.). The `password.go` comment explicitly says *“Callers treat it as an auth failure but should log it”*, yet `Login` maps it straight to `domain.ErrInvalidCredentials` with no `slog.Error`. An operator has no server-side signal that data corruption is locking a user out. **Fix:** Log… <sub>🪰 Gadfly · advisory</sub>
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)
Review

🟡 DeleteSession error silently discarded on expired session

error-handling · flagged by 2 models

  • internal/service/auth.go:155ResolveSession also silently discards errors from s.store.DeleteSession(...) on the already-expired path. If the lazy delete fails, the stale/corrupt row stays in the database until the background sweeper eventually removes it. These errors should be logged rather than thrown away.

🪰 Gadfly · advisory

🟡 **DeleteSession error silently discarded on expired session** _error-handling · flagged by 2 models_ - **`internal/service/auth.go:155`** — `ResolveSession` also silently discards errors from `s.store.DeleteSession(...)` on the already-expired path. If the lazy delete fails, the stale/corrupt row stays in the database until the background sweeper eventually removes it. These errors should be logged rather than thrown away. <sub>🪰 Gadfly · advisory</sub>
return nil, domain.ErrNotFound
}
if newExp := now.Add(sessionTTL); newExp.Sub(exp) > time.Hour {
Review

🟠 Sliding session renewal bumps DB expiry but never refreshes the cookie Max-Age — browser logs the user out ~30 days after the original login regardless of activity

correctness, error-handling · flagged by 3 models

  • internal/service/auth.go:159 / internal/api/auth.go:145-149 — sliding renewal extends the DB session but never refreshes the cookie's Max-Age. ResolveSession calls s.store.TouchSession(...) to push expires_at forward, but the cookie set at login (setSessionCookie) computes maxAge once from expiresAt and is never re-issued on /me or any subsequent request. So a user who keeps using the app will have their server-side session slid to "now + 30 days" indefinitely, but the br…

🪰 Gadfly · advisory

🟠 **Sliding session renewal bumps DB expiry but never refreshes the cookie Max-Age — browser logs the user out ~30 days after the original login regardless of activity** _correctness, error-handling · flagged by 3 models_ - **`internal/service/auth.go:159` / `internal/api/auth.go:145-149` — sliding renewal extends the DB session but never refreshes the cookie's Max-Age.** `ResolveSession` calls `s.store.TouchSession(...)` to push `expires_at` forward, but the cookie set at login (`setSessionCookie`) computes `maxAge` once from `expiresAt` and is never re-issued on `/me` or any subsequent request. So a user who keeps using the app will have their server-side session slid to "now + 30 days" indefinitely, but the br… <sub>🪰 Gadfly · advisory</sub>
_ = 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))
}
+272
View File
@@ -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, "[email protected]", "Alice", "password123")
if !first.IsAdmin {
t.Error("first user should be admin")
}
second := mustRegister(t, s, "[email protected]", "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, "[email protected]", "Alice", "password123")
// Stored normalized (lowercased).
u, err := s.store.GetUserByEmail(context.Background(), "[email protected]")
if err != nil {
t.Fatalf("lookup normalized email: %v", err)
}
if u.Email != "[email protected]" {
t.Errorf("stored email = %q, want lowercased", u.Email)
}
// A different-cased duplicate is rejected.
_, err = s.Register(context.Background(), RegisterInput{Email: "[email protected]", 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, "[email protected]", "Admin", "password123")
if !first.IsAdmin {
t.Error("bootstrap user should be admin")
}
// Subsequent signups are blocked.
_, err := s.Register(context.Background(), RegisterInput{Email: "[email protected]", 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: "[email protected]", DisplayName: "A", Password: "password123"}); !errors.Is(err, domain.ErrLocalAuthDisabled) {
t.Errorf("register err = %v, want ErrLocalAuthDisabled", err)
}
if _, err := s.Login(context.Background(), "[email protected]", "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, "[email protected]", "Alice", "correct-horse")
// Correct credentials (case-insensitive email).
u, err := s.Login(context.Background(), "[email protected]", "correct-horse")
if err != nil {
t.Fatalf("login correct: %v", err)
}
if u.Email != "[email protected]" {
t.Errorf("logged-in user = %q", u.Email)
}
// Wrong password and unknown email both yield the same sentinel.
if _, err := s.Login(context.Background(), "[email protected]", "wrong"); !errors.Is(err, domain.ErrInvalidCredentials) {
t.Errorf("wrong-password err = %v, want ErrInvalidCredentials", err)
}
if _, err := s.Login(context.Background(), "[email protected]", "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: "[email protected]", DisplayName: "O", OIDCIssuer: &iss, OIDCSubject: &sub,
}); err != nil {
t.Fatalf("seed oidc user: %v", err)
}
if _, err := s.Login(context.Background(), "[email protected]", "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, "[email protected]", "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, "[email protected]", "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, "[email protected]", "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, "[email protected]", "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")
}
}
+82
View File
@@ -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
Review

🟡 errBadHash doc says callers should log it but no caller does

maintainability · flagged by 1 model

  • internal/service/password.go:27-28 — The errBadHash doc says "Callers treat it as an auth failure but should log it." No caller logs it: service.Login (auth.go:107-110) folds errBadHash into ErrInvalidCredentials via if err != nil || !ok with no log, and the only other caller in tests asserts the error. So the "should log it" guidance is unfulfilled — either drop the prescription from the comment or actually log it in Login (a stored hash that won't parse is a real data/ops s…

🪰 Gadfly · advisory

🟡 **errBadHash doc says callers should log it but no caller does** _maintainability · flagged by 1 model_ - **`internal/service/password.go:27-28`** — The `errBadHash` doc says "Callers treat it as an auth failure but should log it." No caller logs it: `service.Login` (auth.go:107-110) folds `errBadHash` into `ErrInvalidCredentials` via `if err != nil || !ok` with no log, and the only other caller in tests asserts the error. So the "should log it" guidance is unfulfilled — either drop the prescription from the comment or actually log it in `Login` (a stored hash that won't parse is a real data/ops s… <sub>🪰 Gadfly · advisory</sub>
// 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=..", "<salt>", "<hash>"]
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 {
Review

decodeHash inconsistently shadows vs. assigns the named err return

maintainability · flagged by 1 model

🪰 Gadfly · advisory

⚪ **decodeHash inconsistently shadows vs. assigns the named err return** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
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
}
+54
View File
@@ -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)
}
}
}
+76
View File
@@ -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 {
Review

🔴 Empty dummyHash breaks login timing equalization and enables account enumeration

correctness, error-handling, security · flagged by 2 models

  • Empty dummyHash nullifies the login timing defense. In Service.New (internal/service/service.go:47-51), if hashPassword fails, s.dummyHash remains "". In Login (internal/service/auth.go:95-104), the unknown-email and OIDC-only paths call verifyPassword(s.dummyHash, password), which returns errBadHash instantly without touching argon2. The wrong-password path on a real account, however, performs the full 64 MiB argon2 computation. The resulting timing gap is large and me…

🪰 Gadfly · advisory

🔴 **Empty dummyHash breaks login timing equalization and enables account enumeration** _correctness, error-handling, security · flagged by 2 models_ - **Empty `dummyHash` nullifies the login timing defense.** In `Service.New` (`internal/service/service.go:47-51`), if `hashPassword` fails, `s.dummyHash` remains `""`. In `Login` (`internal/service/auth.go:95-104`), the unknown-email and OIDC-only paths call `verifyPassword(s.dummyHash, password)`, which returns `errBadHash` instantly without touching argon2. The wrong-password path on a real account, however, performs the full 64 MiB argon2 computation. The resulting timing gap is large and me… <sub>🪰 Gadfly · advisory</sub>
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[:])
}
+80
View File
@@ -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
}
+101
View File
@@ -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
Review

🟡 scanUser comment says is_admin read into an int but code declares int64

maintainability · flagged by 1 model

🪰 Gadfly · advisory

🟡 **scanUser comment says is_admin read into an int but code declares int64** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
// 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
}