bvandeusen
d5ab3b0764
Third attempt at the same class of mistake, so worth naming precisely. TestRoutesRegisteredInMount calls Mount() from INSIDE package api, so the call reads `Mount(...)` unqualified. My verification grep was `api.Mount(`, which cannot match it. Same shape as the previous failure, where I grepped `auth.ClientIP(` and missed nothing — but only because those callers happened to be in other packages. The lesson generalises: after changing an exported signature, search for the bare identifier, not the package-qualified form. In-package callers — which in Go means most tests — are invisible to the qualified pattern. This time I swept every signature I touched (Mount, RequireUser, ClientIP, TouchSessionLastSeen) with an unqualified pattern before pushing, rather than letting CI enumerate them one per run. Passing h.netSettings (nil in test handlers) is deliberate, not a placeholder: this test asserts route registration, and Hops() is nil-safe by design so the middleware reads "trust nothing" rather than panicking. Also gave netsettings' logger field a use — it was assigned and never read, which staticcheck's unused pass can flag. A hop-count change alters how much of a client-supplied header the server believes, so it earns a log line for anyone later debugging odd addresses in the sessions list.
105 lines
3.4 KiB
Go
105 lines
3.4 KiB
Go
// Package netsettings holds the DB-backed network settings the request path
|
|
// needs. Today that's the trusted reverse-proxy depth used to pull a real
|
|
// client address out of X-Forwarded-For (#2453).
|
|
//
|
|
// Values are cached under an RWMutex and refreshed on write. That isn't an
|
|
// optimisation: auth.ClientIP runs in the RequireUser middleware for every
|
|
// authenticated request, so a per-request query here would put the database
|
|
// on the critical path of the entire API.
|
|
package netsettings
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"log/slog"
|
|
"sync"
|
|
|
|
"github.com/jackc/pgx/v5/pgxpool"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
const (
|
|
// DefaultTrustedProxyHops mirrors migration 0053's column default. One
|
|
// proxy, because anything publicly reachable needs a TLS terminator in
|
|
// front of it.
|
|
DefaultTrustedProxyHops = 1
|
|
// MaxTrustedProxyHops mirrors the CHECK in migration 0053.
|
|
MaxTrustedProxyHops = 10
|
|
)
|
|
|
|
// ErrHopsOutOfRange is returned by SetHops for values the CHECK would reject,
|
|
// so the API layer can answer 400 instead of surfacing a constraint violation.
|
|
var ErrHopsOutOfRange = errors.New("trusted proxy hops must be between 0 and 10")
|
|
|
|
// Service caches the network settings and owns their persistence.
|
|
type Service struct {
|
|
pool *pgxpool.Pool
|
|
logger *slog.Logger
|
|
|
|
mu sync.RWMutex
|
|
hops int
|
|
}
|
|
|
|
// New loads the settings once and caches them.
|
|
//
|
|
// It ALWAYS returns a usable Service, even alongside a non-nil error. The
|
|
// value it holds sits on the authenticated request path, so a boot-time
|
|
// database hiccup must degrade to the default rather than take every request
|
|
// down with it (rule #131). The error is returned so the caller can log that
|
|
// the cache holds a default rather than stored state.
|
|
func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service, error) {
|
|
s := &Service{pool: pool, logger: logger, hops: DefaultTrustedProxyHops}
|
|
if pool == nil {
|
|
return s, nil
|
|
}
|
|
row, err := dbq.New(pool).GetNetworkSettings(ctx)
|
|
if err != nil {
|
|
return s, err
|
|
}
|
|
s.hops = int(row.TrustedProxyHops)
|
|
return s, nil
|
|
}
|
|
|
|
// Hops returns the cached trusted-proxy depth.
|
|
//
|
|
// Nil-safe: test contexts construct routers without this service, and a
|
|
// missing setting should mean "trust nothing" rather than a panic in
|
|
// middleware.
|
|
func (s *Service) Hops() int {
|
|
if s == nil {
|
|
return 0
|
|
}
|
|
s.mu.RLock()
|
|
defer s.mu.RUnlock()
|
|
return s.hops
|
|
}
|
|
|
|
// SetHops persists a new depth and refreshes the cache, so an admin change
|
|
// takes effect on the next request with no restart (rule #25).
|
|
func (s *Service) SetHops(ctx context.Context, hops int) error {
|
|
if s == nil || s.pool == nil {
|
|
// Mirrors Hops()'s nil-tolerance: handlers can be constructed without
|
|
// this service in tests, and a write attempt there should be an error
|
|
// rather than a panic in an HTTP handler.
|
|
return errors.New("network settings unavailable")
|
|
}
|
|
if hops < 0 || hops > MaxTrustedProxyHops {
|
|
return ErrHopsOutOfRange
|
|
}
|
|
row, err := dbq.New(s.pool).UpdateTrustedProxyHops(ctx, int32(hops))
|
|
if err != nil {
|
|
return err
|
|
}
|
|
s.mu.Lock()
|
|
s.hops = int(row.TrustedProxyHops)
|
|
s.mu.Unlock()
|
|
// Worth a line in the log: this changes how much of a client-supplied
|
|
// header the server believes, so an operator debugging odd addresses in
|
|
// the sessions list wants to see when it last moved.
|
|
if s.logger != nil {
|
|
s.logger.Info("netsettings: trusted proxy hops updated", "hops", hops)
|
|
}
|
|
return nil
|
|
}
|