Local auth: users, sessions, register/login/logout/me (#4) #23
@@ -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
|
||||
|
||||
|
||||
@@ -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:
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,24 @@ func New(cfg *config.Config) *gin.Engine {
|
||||
_ = r.SetTrustedProxies(nil)
|
||||
}
|
||||
|
||||
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);
|
||||
// /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
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,224 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"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"`
|
||||
|
gitea-actions
commented
🟡 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>
|
||||
Password string `json:"password" binding:"required,max=1024"`
|
||||
}
|
||||
|
||||
// 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 {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
|
||||
|
gitea-actions
commented
🟠 startSession error in register/login is unlogged; operator-invisible, leaves half-created user on register correctness, error-handling, maintainability · flagged by 3 models
🪰 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>
|
||||
h.startSessionAndRespond(c, 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")
|
||||
|
gitea-actions
commented
🟡 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
🪰 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>
|
||||
return
|
||||
}
|
||||
|
||||
user, err := h.svc.Login(c.Request.Context(), req.Email, req.Password)
|
||||
if err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
|
||||
h.startSessionAndRespond(c, 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 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 == "" {
|
||||
abortUnauthenticated(c)
|
||||
|
gitea-actions
commented
🔴 Sliding session expiry (server-side) is never propagated to the client cookie's Max-Age, so active users are logged out after 30 days from login regardless of activity correctness · flagged by 2 models
🪰 Gadfly · advisory 🔴 **Sliding session expiry (server-side) is never propagated to the client cookie's Max-Age, so active users are logged out after 30 days from login regardless of activity**
_correctness · flagged by 2 models_
- **`internal/api/auth.go:107-126,145-148` — Sliding session expiry never reaches the browser cookie, so "sliding 30-day expiry" doesn't actually work.** `setSessionCookie` (sets `Max-Age`) is only called from `startSession` (`auth.go:129-136`), which is only invoked by `register`/`login` (`auth.go:54,75`). `requireAuth` (`auth.go:107-126`) calls `svc.ResolveSession`, which slides `sessions.expires_at` forward in the DB via `TouchSession` (`internal/service/auth.go:159-161`) but never re-issues…
<sub>🪰 Gadfly · advisory</sub>
|
||||
return
|
||||
}
|
||||
user, expiresAt, err := h.svc.ResolveSession(c.Request.Context(), token)
|
||||
if err != nil {
|
||||
|
gitea-actions
commented
⚪ requireAuth repeats an identical unauthorized-response triplet in two branches maintainability · flagged by 1 model
🪰 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>
|
||||
// Any resolution failure (missing/expired/tampered) is 401, never 404:
|
||||
// the client should log in, not think a resource is gone.
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
gitea-actions
commented
🟡 Session cookie Secure attribute defaults to false when PANSY_BASE_URL unset, risking token leak if deployed behind a proxy without setting the env var security · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Session cookie Secure attribute defaults to false when PANSY_BASE_URL unset, risking token leak if deployed behind a proxy without setting the env var**
_security · flagged by 1 model_
- **Session cookie defaults to non-Secure when `PANSY_BASE_URL` is unset** (`internal/api/auth.go:141-143`, `internal/config/config.go:70`). `cookieSecure()` only returns true when `BaseURL` starts with `https://`, and the default `BaseURL` is `""`. A self-hoster who puts pansy behind an HTTPS-terminating reverse proxy but forgets to set `PANSY_BASE_URL` gets a session cookie without `Secure`; any same-host HTTP request (or downgrade/MITM) then leaks the bearer token. This is documented as inten…
<sub>🪰 Gadfly · advisory</sub>
|
||||
return func(c *gin.Context) {
|
||||
if wantHost == "" {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
|
gitea-actions
commented
⚪ setSessionCookie MaxAge floor of 1 masks clock-skew/negative-duration bugs instead of clearing the cookie error-handling · flagged by 1 model
🪰 Gadfly · advisory ⚪ **setSessionCookie MaxAge floor of 1 masks clock-skew/negative-duration bugs instead of clearing the cookie**
_error-handling · flagged by 1 model_
- **`internal/api/auth.go:146` — `maxAge` floor of 1 masks clock-skew bugs.** `max(int(time.Until(expiresAt).Seconds()), 1)` clamps a negative duration to a 1-second cookie instead of clearing it. Not currently reachable (`expiresAt = now + 30d`), but the floor silently swallows a clock-skew class of bug. Trivial; leave or change to clear the cookie when `time.Until(expiresAt) <= 0`.
<sub>🪰 Gadfly · advisory</sub>
|
||||
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.
|
||||
|
gitea-actions
commented
🟡 writeServiceError method has unused receiver maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **writeServiceError method has unused receiver**
_maintainability · flagged by 1 model_
- **`internal/api/auth.go:164`** `writeServiceError` method has unused receiver — Declared as `func (h *handlers) writeServiceError(...)`, but `h` is never referenced inside the body, which is a small readability misdirection. **Fix:** Change it to a package-level function: `func writeServiceError(c *gin.Context, err error)`.
<sub>🪰 Gadfly · advisory</sub>
|
||||
func (h *handlers) startSessionAndRespond(c *gin.Context, user *domain.User) {
|
||||
token, expiresAt, err := h.svc.CreateSession(c.Request.Context(), user.ID)
|
||||
if err != nil {
|
||||
// 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)
|
||||
c.JSON(http.StatusOK, user)
|
||||
}
|
||||
|
||||
// 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) {
|
||||
// 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)
|
||||
}
|
||||
|
||||
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 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")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,239 @@
|
||||
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}
|
||||
}
|
||||
|
||||
// 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 v != nil {
|
||||
if err := json.NewEncoder(&buf).Encode(v); err != nil {
|
||||
t.Fatalf("encode body: %v", err)
|
||||
}
|
||||
}
|
||||
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)
|
||||
}
|
||||
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 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": "[email protected]", "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": "[email protected]", "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": "[email protected]", "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"
|
||||
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")
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
@@ -99,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.
|
||||
@@ -139,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"`
|
||||
}
|
||||
|
||||
@@ -0,0 +1,210 @@
|
||||
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
|
||||
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.
|
||||
|
gitea-actions
commented
🟠 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
🪰 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>
|
||||
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)
|
||||
|
gitea-actions
commented
🔴 TOCTOU race in Register allows multiple bootstrap admins correctness, error-handling, security · flagged by 3 models
🪰 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 email == "" || displayName == "" || in.Password == "" || len(in.Password) > maxPasswordLen {
|
||||
return nil, domain.ErrInvalidInput
|
||||
}
|
||||
|
||||
// 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)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return s.store.CreateUser(ctx, &domain.User{
|
||||
|
gitea-actions
commented
🟡 First-user-is-admin decided by CountUsers+CreateUser without a transaction; concurrent first registrations can both become admin correctness · flagged by 1 model
🪰 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,
|
||||
}, s.cfg.RegistrationOpen())
|
||||
|
gitea-actions
commented
🟠 First-user-is-admin is read-then-write TOCTOU: two concurrent first registrations with different emails both become admin correctness · flagged by 1 model 🪰 Gadfly · advisory 🟠 **First-user-is-admin is read-then-write TOCTOU: two concurrent first registrations with different emails both become admin**
_correctness · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
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 {
|
||||
|
gitea-actions
commented
🟠 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
🪰 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>
|
||||
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
|
||||
|
gitea-actions
commented
🟠 Malformed password hash silently returned as wrong-password with no log correctness, error-handling · flagged by 1 model
🪰 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>
|
||||
}
|
||||
|
||||
ok, err := verifyPassword(*u.PasswordHash, password)
|
||||
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
|
||||
}
|
||||
|
||||
// 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 and the
|
||||
|
gitea-actions
commented
🟡 Two DB round-trips on every authenticated request performance · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Two DB round-trips on every authenticated request**
_performance · flagged by 1 model_
- `internal/service/auth.go:141` — **Two DB round-trips on every authenticated request** (observation, not blocking). `ResolveSession` calls `GetSession` (by PK) and then `GetUserByID` (by PK). Both are indexed and fast, but every protected endpoint pays two query overheads. A single JOIN query would halve the DB work on the hot path. Verified by reading `store/sessions.go` and `store/users.go` — neither currently exposes a joined fetch. **Fix:** Consider adding `GetSessionWithUser` in a follow-…
<sub>🪰 Gadfly · advisory</sub>
|
||||
// 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, time.Time{}, domain.ErrNotFound
|
||||
|
gitea-actions
commented
🟡 DeleteSession error silently discarded on corrupt expiry error-handling · flagged by 3 models
🪰 Gadfly · advisory 🟡 **DeleteSession error silently discarded on corrupt expiry**
_error-handling · flagged by 3 models_
- **`internal/service/auth.go:149`** — `ResolveSession` also silently discards errors from `s.store.DeleteSession(...)` on the corrupt-expiry 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>
|
||||
}
|
||||
hash := hashToken(rawToken)
|
||||
sess, err := s.store.GetSession(ctx, hash)
|
||||
if err != nil {
|
||||
return nil, time.Time{}, err
|
||||
}
|
||||
|
gitea-actions
commented
🟡 DeleteSession error silently discarded on expired session error-handling · flagged by 2 models
🪰 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>
|
||||
|
||||
exp, err := parseTime(sess.ExpiresAt)
|
||||
if err != nil {
|
||||
// A corrupt expiry means we can't trust the session; drop it.
|
||||
|
gitea-actions
commented
🟠 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
🪰 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>
|
||||
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) {
|
||||
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 {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
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.
|
||||
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))
|
||||
}
|
||||
@@ -0,0 +1,294 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"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 TestRejectsOverlongPassword(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
long := strings.Repeat("a", maxPasswordLen+1)
|
||||
|
||||
if _, err := s.Register(context.Background(), RegisterInput{Email: "[email protected]", DisplayName: "A", Password: long}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("register overlong err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
|
||||
mustRegister(t, s, "[email protected]", "Bob", "password123")
|
||||
if _, err := s.Login(context.Background(), "[email protected]", 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
|
||||
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,
|
||||
}, true); 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, 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) {
|
||||
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) }
|
||||
_, 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)
|
||||
}
|
||||
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")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/subtle"
|
||||
"encoding/base64"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/crypto/argon2"
|
||||
)
|
||||
|
||||
// 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 = 3
|
||||
|
gitea-actions
commented
🟡 Argon2id t=1 with m=64MiB/p=4 doesn't match either RFC 9106 recommended parameter profile, weakening password hash resistance security · flagged by 1 model 🪰 Gadfly · advisory 🟡 **Argon2id t=1 with m=64MiB/p=4 doesn't match either RFC 9106 recommended parameter profile, weakening password hash resistance**
_security · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
||||
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
|
||||
|
gitea-actions
commented
🟡 errBadHash doc says callers should log it but no caller does maintainability · flagged by 1 model
🪰 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>
|
||||
// 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) {
|
||||
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
|
||||
}
|
||||
|
gitea-actions
commented
⚪ 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>
|
||||
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. 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) {
|
||||
fail := func() (uint32, uint32, uint8, []byte, []byte, error) {
|
||||
return 0, 0, 0, nil, nil, errBadHash
|
||||
}
|
||||
|
||||
parts := strings.Split(encoded, "$")
|
||||
// ["", "argon2id", "v=19", "m=..,t=..,p=..", "<salt>", "<hash>"]
|
||||
if len(parts) != 6 || parts[1] != "argon2id" {
|
||||
return fail()
|
||||
}
|
||||
|
||||
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 fail()
|
||||
}
|
||||
if key, err = b64.DecodeString(parts[5]); err != nil || len(key) == 0 {
|
||||
return fail()
|
||||
}
|
||||
return mem, t, threads, salt, key, nil
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,75 @@
|
||||
// Package service is pansy's business-logic seam: all permission checks and
|
||||
// invariants live here rather than in the HTTP handlers. Resource operations
|
||||
|
gitea-actions
commented
🟡 Package doc claims methods take (ctx, actor, args) but no method in this PR takes an actor maintainability · flagged by 1 model 🪰 Gadfly · advisory 🟡 **Package doc claims methods take (ctx, actor, args) but no method in this PR takes an actor**
_maintainability · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
||||
// 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 (
|
||||
"crypto/rand"
|
||||
"crypto/sha256"
|
||||
"encoding/base64"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"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 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.
|
||||
func New(st *store.DB, cfg *config.Config) *Service {
|
||||
|
gitea-actions
commented
🔴 Empty dummyHash breaks login timing equalization and enables account enumeration correctness, error-handling, security · flagged by 2 models
🪰 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>
|
||||
return &Service{
|
||||
store: st,
|
||||
cfg: cfg,
|
||||
now: time.Now,
|
||||
dummyHash: timingHash(),
|
||||
}
|
||||
}
|
||||
|
||||
// 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[:])
|
||||
}
|
||||
@@ -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);
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"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
|
||||
|
gitea-actions
commented
🟡 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>
|
||||
// returns it as int64, which does not convert straight to bool), so it is read
|
||||
// into an int64 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.
|
||||
//
|
||||
// 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)
|
||||
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)
|
||||
}
|
||||
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
|
||||
}
|
||||
|
||||
// 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: <table>.<column>").
|
||||
func isUniqueViolation(err error) bool {
|
||||
return err != nil && strings.Contains(err.Error(), "UNIQUE constraint failed")
|
||||
}
|
||||
🟠 Password length validation only in HTTP binding tags, not service layer
maintainability · flagged by 1 model
internal/api/auth.go:25-28Password length validation only in HTTP binding tags, not service layer —registerRequestenforcesmin=8,max=1024via gin binding tags, butservice.Register(internal/service/auth.go:48-50) only checksin.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