feat(server): log session IP changes, add optional PAD_IP_CHANGE_ENFORCE=strict (TASK-666) (#191)

* feat(server): log session IP changes, add optional PAD_IP_CHANGE_ENFORCE=strict (TASK-666)

Sessions stored a client IP at creation but never rechecked it. A stolen
cookie could be used from anywhere with no signal to the owner. This
change adds mid-lifetime IP-change detection without breaking legitimate
mobility (mobile roaming, VPN toggles, carrier NAT) by default.

- New audit action ActionSessionIPChanged captures {old_ip, new_ip} in
  the audit metadata. Visible via the existing /api/v1/admin/audit-log.
- handleSessionIPChange wired into both SessionAuth (cookies) and
  TokenAuth (padsess_ bearer). After UA check passes, compares stored
  session IP to clientIP(r). On mismatch:
    - log one audit row
    - update the stored session IP so we don't spam the log
    - strict mode: DeleteSession + 401 "session_ip_changed"
    - default mode: let the request through
- Store.UpdateSessionIP lets middleware refresh the recorded IP without
  tearing down the session.
- PAD_IP_CHANGE_ENFORCE=strict env var + ip_change_enforce TOML key +
  Server.SetIPChangeEnforce setter (case-insensitive, trims whitespace).
- Table-driven tests cover log-only, strict rejection with session
  destruction, and setter parsing edge cases.

Parent: PLAN-643 (OSS Security Hardening).

* fix(server): dedupe session-IP-change audit via CAS, handle browser vs API paths per Codex review

Addresses two P2 comments on PR #191:

1. Race: parallel requests after an IP change could each emit
   ActionSessionIPChanged before any of them updated the stored IP,
   producing duplicate audit rows for a single transition.
   - Replace UpdateSessionIP with UpdateSessionIPIfEquals (compare-and-set
     on ip_address). Only the request that actually rotates the stored
     value logs; concurrent siblings lose the CAS and skip logging.
   - New test TestSessionIPChange_CASDedupesRace fires 20 concurrent
     requests from the new IP and asserts exactly 1 audit row.

2. Strict-mode 401 on non-API paths:
   - In current routing the SPA is mounted on the root router outside
     the auth Group, so SessionAuth only fires for /api/* in practice.
     The original concern about JSON 401s on browser navigation doesn't
     surface today, but defense-in-depth keeps the code forward-safe:
     restructure handleSessionIPChange to return a four-state outcome
     (Continue / AllowedLogged / Revoked / Terminated) and only write
     the JSON 401 on /api/* paths. Revoked + non-API falls through
     unauthenticated so a future SPA-in-group configuration would still
     render a login screen instead of raw JSON.
   - Clear the session cookie (MaxAge=-1) in strict rejection so the
     browser stops sending the now-revoked token on the next request.
     TestSessionIPChange_StrictClearsCookies verifies the Set-Cookie.

Parent: PLAN-643 (OSS Security Hardening), TASK-666.

* fix(server): strict mode destroys session atomically, never rotate stored IP when destroying (TASK-666)

Addresses Codex P1 on PR #191: previously we rotated the session's stored
ip_address via UpdateSessionIPIfEquals BEFORE attempting DeleteSession.
If the DELETE failed (transient DB error) the row remained alive —
rebound to the attacker's new IP — so follow-up requests saw stored IP
== client IP and passed handleSessionIPChange's "match, no-op" branch.
That silently defeated strict enforcement.

- New Store.DeleteSessionIfExists returns (bool, error) to serve as the
  CAS primitive for strict mode: only the caller whose DELETE affected a
  row emits the audit entry, and a DB error fails closed (500 — "Unable
  to validate session") rather than letting the request through.
- handleSessionIPChange splits into two paths:
    * log-only mode: UpdateSessionIPIfEquals for CAS dedup (unchanged)
    * strict mode: DeleteSessionIfExists is the CAS; stored IP is NEVER
      rotated so any failure leaves the session bound to the OLD IP and
      subsequent requests from the new IP still mismatch + still reject.
- TestSessionIPChange_StrictDestroysSessionAtomically regression test
  verifies a second request from the new IP with the same token still
  fails after the first strict-mode rejection.

Parent: PLAN-643 (OSS Security Hardening).

* fix(server): exempt public API paths from strict IP-change termination (TASK-666)

Addresses Codex P2 on PR #191: SessionAuth runs for every /api/* path,
including public endpoints like /api/v1/auth/login, /api/v1/auth/register,
/api/v1/health, /api/v1/s/* (share links), and /api/v1/plan-limits. In
strict mode, a stale session cookie on those requests was rejected with
a 401 session_ip_changed BEFORE the public handler could run — the user
literally couldn't log back in because their own stale cookie blocked
the login call.

- Extract isPublicAPIPath as a shared helper between RequireAuth and
  handleSessionIPChange so they can't drift out of sync.
- handleSessionIPChange strict-mode flow now: destroy session + clear
  cookies + audit log (unchanged), then for public API paths return
  Revoked so the handler still runs. For authenticated-only API paths
  still return Terminated (401). For non-API paths return Revoked for
  the SPA fallback.
- Updated TokenAuth Revoked handler to match: pass through unauth on
  public paths, 401 on authenticated-only.
- TestSessionIPChange_StrictAllowsPublicAPIPaths regression test:
  a stale session cookie on /api/v1/auth/login must NOT produce
  session_ip_changed; /api/v1/plan-limits must still return 200.

Parent: PLAN-643 (OSS Security Hardening).

* fix(server): short-circuit SessionAuth on token auth + fix IPv6 clientIP parsing (TASK-666)

Addresses two more Codex comments on PR #191:

P1 — SessionAuth 401'd API-token-authenticated requests:
TokenAuth sets currentUser for user-owned tokens AND tokenWorkspaceID
for legacy workspace-scoped tokens. SessionAuth short-circuited only on
currentUser, so a workspace-scoped-token request that happened to carry
a stale session cookie with a mismatched IP would be rejected by the
IP-change strict path before RequireAuth could honor the token. Extend
the short-circuit to also check tokenWorkspaceID; either signal is
enough to say "token auth already succeeded, skip cookie validation".

P2 — clientIP mangled IPv6 addresses:
clientIP used strings.LastIndex(":") on RemoteAddr. For bare IPv6
addresses like "2001:db8::1" (which TrustedProxyRealIP writes verbatim
from X-Forwarded-For, no brackets/port), that strips the final hextet
to "2001:db8:" — unusable for comparison in the new IP-change audit
path and incorrect for rate-limit keys too. Switch to net.SplitHostPort
which handles both "host:port" and "[ipv6]:port", falling back to the
raw RemoteAddr when no port is present (the trusted-proxy rewrite
case).

Tests:
- TestClientIP_IPv6NotMangled covers IPv4 w/wo port, bracketed IPv6,
  bare IPv6 (no port, no brackets), and loopback forms.
- TestSessionAuth_ShortCircuitsOnAPITokenAuth exercises the worst case:
  strict mode + valid API token + stale session cookie + new client IP.
  Request must succeed (token wins) and NO new session_ip_changed audit
  row must appear.

Parent: PLAN-643 (OSS Security Hardening).

* fix(server): canonicalize IPs before session-IP-change comparison (TASK-666)

Addresses Codex P2 on PR #191: raw-string comparison of session.IPAddress
vs clientIP(r) would fire session_ip_changed spuriously when the same
IPv6 address arrived in different valid textual representations (the
trusted-proxy path writes X-Forwarded-For verbatim, and different hops
normalize differently — "2001:0db8::1" vs "2001:db8::1" etc.).

- canonicalIP helper: net.ParseIP + stringify to collapse equivalent
  IPv6 forms (compressed vs expanded, case, leading zeros) and IPv4-in-
  IPv6 into a single canonical string. Non-parseable inputs pass through
  unchanged so debug/malformed values behave predictably.
- handleSessionIPChange compares and logs the canonical forms. The CAS
  still passes session.IPAddress (the raw stored value) to the DB — the
  compare-and-set is about row identity — but the new IP written in is
  the canonical form so future comparisons are stable.
- TestCanonicalIP covers empty, IPv4, shorthand "::1", expanded 8-group
  equivalent, mixed-case 2001:DB8::1, fully expanded 2001:0db8:…:0001,
  and non-IP fallback.

Parent: PLAN-643 (OSS Security Hardening).
This commit is contained in:
xarmian
2026-04-22 10:43:22 -04:00
committed by GitHub
parent 0a24078554
commit 46fa72ca0f
9 changed files with 763 additions and 21 deletions
+8
View File
@@ -49,3 +49,11 @@ PAD_ENCRYPTION_KEY=
# "Authorization: Bearer $PAD_METRICS_TOKEN".
# openssl rand -hex 32
# PAD_METRICS_TOKEN=
# OPTIONAL — Session IP-change policy.
# Default (unset) logs an ActionSessionIPChanged audit entry when a session
# presents a new client IP and updates the stored IP. "strict" additionally
# destroys the session and forces re-login. Strict mode breaks legitimate
# mobility (mobile roaming, VPN toggles, carrier NAT); enable only for
# high-sensitivity deployments.
# PAD_IP_CHANGE_ENFORCE=strict
+1
View File
@@ -275,6 +275,7 @@ func serveCmd() *cobra.Command {
srv.SetSecureCookies(cfg.SecureCookies)
srv.SetTrustedProxies(cfg.TrustedProxies)
srv.SetMetricsToken(cfg.MetricsToken)
srv.SetIPChangeEnforce(cfg.IPChangeEnforce)
srv.SetSSELimits(cfg.SSEMaxConnections, cfg.SSEMaxPerWorkspace)
// Cloud mode: enable cloud-specific endpoints and behavior
+8 -4
View File
@@ -48,10 +48,11 @@ type Config struct {
EncryptionKeySource string `toml:"-"` // "env", "file", "generated", or "" (unset); populated by EnsureEncryptionKey
// Security
CORSOrigins string `toml:"cors_origins"` // Comma-separated allowed origins (e.g. "https://app.pad.dev,https://admin.pad.dev")
SecureCookies bool `toml:"secure_cookies"` // Set Secure flag on cookies (requires TLS)
TrustedProxies string `toml:"trusted_proxies"` // Comma-separated CIDRs whose X-Forwarded-For is trusted. Empty = ignore proxy headers.
MetricsToken string `toml:"metrics_token"` // Shared Bearer token required to scrape /metrics. Empty = loopback-only.
CORSOrigins string `toml:"cors_origins"` // Comma-separated allowed origins (e.g. "https://app.pad.dev,https://admin.pad.dev")
SecureCookies bool `toml:"secure_cookies"` // Set Secure flag on cookies (requires TLS)
TrustedProxies string `toml:"trusted_proxies"` // Comma-separated CIDRs whose X-Forwarded-For is trusted. Empty = ignore proxy headers.
MetricsToken string `toml:"metrics_token"` // Shared Bearer token required to scrape /metrics. Empty = loopback-only.
IPChangeEnforce string `toml:"ip_change_enforce"` // "" (log only) or "strict" (reject session when client IP differs from the one recorded at session creation).
// SSE limits
SSEMaxConnections int `toml:"sse_max_connections"` // Global max SSE connections (0 = unlimited)
@@ -167,6 +168,9 @@ func Load() (*Config, error) {
if v := os.Getenv("PAD_METRICS_TOKEN"); v != "" {
cfg.MetricsToken = v
}
if v := os.Getenv("PAD_IP_CHANGE_ENFORCE"); v != "" {
cfg.IPChangeEnforce = v
}
if v := os.Getenv("PAD_SSE_MAX_CONNECTIONS"); v != "" {
if max, err := strconv.Atoi(v); err == nil {
cfg.SSEMaxConnections = max
+7
View File
@@ -32,6 +32,13 @@ const (
ActionUserDisabled = "user_disabled"
ActionUserEnabled = "user_enabled"
ActionAccountDeleted = "account_deleted"
// ActionSessionIPChanged is logged when a session presents a different
// client IP than the one recorded at creation. We don't strict-check IP
// by default (that breaks legitimate geo shifts — VPN toggle, mobile
// roaming) but surface the change to the audit log for detection. In
// deployments configured with PAD_IP_CHANGE_ENFORCE=strict the middleware
// additionally rejects the request.
ActionSessionIPChanged = "session_ip_changed"
)
type Activity struct {
+228 -4
View File
@@ -6,12 +6,14 @@ import (
"encoding/hex"
"encoding/json"
"log/slog"
"net"
"net/http"
"strings"
"time"
"github.com/go-chi/chi/v5"
"github.com/xarmian/pad/internal/models"
"github.com/xarmian/pad/internal/store"
)
type contextKey string
@@ -73,6 +75,25 @@ func (s *Server) TokenAuth(next http.Handler) http.Handler {
writeError(w, http.StatusUnauthorized, "unauthorized", "Session expired")
return
}
// Session binding: detect IP changes. Log to audit log in all modes;
// reject only when PAD_IP_CHANGE_ENFORCE=strict.
switch s.handleSessionIPChange(w, r, session, token) {
case sessionIPChangeTerminated:
return
case sessionIPChangeRevoked:
// Session was destroyed. For public API paths (login,
// password reset, health, share links) pass through
// unauthenticated so the caller can recover. For
// authenticated-only paths, the Bearer flow has no SPA
// fallback — treat revocation as a hard 401.
if isPublicAPIPath(r.URL.Path) {
next.ServeHTTP(w, r)
return
}
writeError(w, http.StatusUnauthorized, "session_ip_changed",
"Session client IP changed — please log in again.")
return
}
ctx := context.WithValue(r.Context(), ctxCurrentUser, session.User)
next.ServeHTTP(w, r.WithContext(ctx))
return
@@ -126,8 +147,13 @@ func (s *Server) TokenAuth(next http.Handler) http.Handler {
// If a user was already resolved by TokenAuth, this is a no-op.
func (s *Server) SessionAuth(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Already authenticated by TokenAuth
if currentUser(r) != nil {
// Already authenticated by TokenAuth — user-bound API tokens set
// currentUser, legacy workspace-scoped tokens set tokenWorkspaceID
// with no user. In either case we must short-circuit: otherwise a
// stale session cookie on the same request could trigger the
// IP-change strict-mode path and 401 the request even though the
// API token itself is valid.
if currentUser(r) != nil || tokenWorkspaceID(r) != "" {
next.ServeHTTP(w, r)
return
}
@@ -157,6 +183,18 @@ func (s *Server) SessionAuth(next http.Handler) http.Handler {
return
}
// Session binding: detect IP changes. Log to audit log in all modes;
// reject only when PAD_IP_CHANGE_ENFORCE=strict.
switch s.handleSessionIPChange(w, r, session, cookie.Value) {
case sessionIPChangeTerminated:
return
case sessionIPChangeRevoked:
// Browser path: let the request through unauthenticated so
// the SPA renders its login flow instead of a JSON error.
next.ServeHTTP(w, r)
return
}
// Re-issue CSRF cookie if the session is valid but the cookie is missing.
// This can happen when cookies expire at different times or are selectively cleared.
// Skip for auth endpoints — they manage their own CSRF cookies (login sets, logout clears).
@@ -171,6 +209,22 @@ func (s *Server) SessionAuth(next http.Handler) http.Handler {
})
}
// isPublicAPIPath reports whether the given request path is an API
// endpoint that bypasses authentication entirely. These handlers must
// work even when the caller has no valid session (login, registration,
// password reset, health probes, public plan limits, share-link tokens).
// Shared between RequireAuth and the session-IP-change path so both stay
// in sync — in particular, strict IP-change enforcement must NOT 401
// these endpoints, otherwise a user with a stale cookie can't even
// recover by logging in again.
func isPublicAPIPath(path string) bool {
return strings.HasPrefix(path, "/api/v1/auth/") ||
path == "/api/v1/health" ||
strings.HasPrefix(path, "/api/v1/health/") ||
strings.HasPrefix(path, "/api/v1/s/") ||
path == "/api/v1/plan-limits"
}
// RequireAuth middleware blocks unauthenticated requests when users exist
// in the system. When no users exist (fresh install), all requests pass
// through to allow the setup flow.
@@ -180,8 +234,7 @@ func (s *Server) RequireAuth(next http.Handler) http.Handler {
// Auth endpoints, share link resolution, health, and the public
// plan-limits endpoint are always exempt from auth.
if strings.HasPrefix(path, "/api/v1/auth/") || path == "/api/v1/health" || strings.HasPrefix(path, "/api/v1/health/") || strings.HasPrefix(path, "/api/v1/s/") ||
path == "/api/v1/plan-limits" {
if isPublicAPIPath(path) {
next.ServeHTTP(w, r)
return
}
@@ -441,6 +494,177 @@ func sha256hex(s string) string {
return hex.EncodeToString(h[:])
}
// canonicalIP returns the semantic (canonical text) form of an IP
// address string. For IPv6 this collapses different valid spellings
// (compressed vs expanded, uppercase vs lowercase, leading zeroes) to
// a single representation so comparing two strings by == reflects
// actual IP equality. For IPv4 it accepts both 4-byte and IPv4-in-IPv6
// forms. For inputs that don't parse as an IP the original string is
// returned so behavior stays predictable on malformed/debug values.
func canonicalIP(s string) string {
if s == "" {
return s
}
if ip := net.ParseIP(s); ip != nil {
return ip.String()
}
return s
}
// sessionIPChangeOutcome captures what the caller (TokenAuth / SessionAuth)
// should do after handleSessionIPChange inspected the session.
type sessionIPChangeOutcome int
const (
// sessionIPChangeContinue — happy path. IP matches (or no recorded IP).
// Caller proceeds to install the session user in context and call next.
sessionIPChangeContinue sessionIPChangeOutcome = iota
// sessionIPChangeAllowedLogged — IP differs, log-only mode. Session
// remains valid, caller proceeds as usual.
sessionIPChangeAllowedLogged
// sessionIPChangeRevoked — strict mode. The session was destroyed and
// the caller must NOT install the user in context. For browser (non-API)
// paths the request should continue unauthenticated so the SPA can
// render its login screen instead of a raw JSON error.
sessionIPChangeRevoked
// sessionIPChangeTerminated — strict mode + API path. A 401 has already
// been written; caller must return immediately.
sessionIPChangeTerminated
)
// handleSessionIPChange compares the client IP on this request to the IP
// recorded when the session was created.
//
// Behavior:
// - Session has no recorded IP (legacy pre-migration row): continue.
// - IP matches: continue.
// - Log-only mode (default): atomically rotate the stored IP via
// compare-and-set. The race winner emits exactly one
// ActionSessionIPChanged audit row per transition — concurrent
// requests that lose the CAS skip logging to avoid duplicate rows.
// - Strict mode (PAD_IP_CHANGE_ENFORCE=strict): do NOT rotate the
// stored IP. Instead use DeleteSessionIfExists as the CAS primitive
// — the request that actually deletes the row (a) logs the audit
// entry once, (b) returns 401 for API / revoked for browser paths.
// Rotating the IP first would be unsafe: if the subsequent
// DeleteSession failed (transient DB error) the session would remain
// valid rebound to the new IP, defeating strict enforcement. By
// deleting atomically, any failure leaves the session bound to the
// original IP so a follow-up request from the new IP still mismatches
// and is still rejected. If the delete errors outright, the request
// is rejected with 500 so the client can't proceed.
//
// Log-only mode breaks fewer legitimate clients (mobile roaming, VPN
// toggles, carrier NAT) while still giving operators a visible signal.
func (s *Server) handleSessionIPChange(w http.ResponseWriter, r *http.Request, session *store.SessionInfo, plainToken string) sessionIPChangeOutcome {
// Canonicalize both sides so equivalent IPv6 representations
// (compressed vs expanded, case, leading zeros) don't register as a
// spurious change. Raw trusted-proxy X-Forwarded-For values can
// arrive in any valid form.
storedIP := canonicalIP(session.IPAddress)
newIP := canonicalIP(clientIP(r))
if storedIP == "" || newIP == "" || storedIP == newIP {
return sessionIPChangeContinue
}
userID := ""
if session.User != nil {
userID = session.User.ID
}
if s.ipChangeEnforceStrict {
// Destroy the session atomically. Only the caller whose DELETE
// affected a row logs and issues the "session_ip_changed" response
// body. A failed delete means the session is still valid AND still
// bound to the old IP (we skipped the rotation), so a follow-up
// request from the new IP will hit this path again — safe.
deleted, err := s.store.DeleteSessionIfExists(plainToken)
if err != nil {
// Fail closed: we can't prove the session is gone, so refuse
// to let the request through. A retry will converge once the
// DB recovers.
slog.Error("failed to destroy session on IP-change in strict mode; failing closed",
"session_ip", storedIP,
"client_ip", newIP,
"user_id", userID,
"error", err)
writeError(w, http.StatusInternalServerError, "internal_error",
"Unable to validate session. Please try again.")
return sessionIPChangeTerminated
}
if deleted {
s.logAuditEventForUser(models.ActionSessionIPChanged, r, userID, auditMeta(map[string]string{
"old_ip": storedIP,
"new_ip": newIP,
}))
slog.Warn("session destroyed: IP changed (strict enforcement)",
"session_ip", storedIP,
"client_ip", newIP,
"user_id", userID)
}
// Clear client-side cookies regardless — even if another request
// already deleted the row, this client is still holding the now-
// invalid token.
clearSessionCookie(w, s.secureCookies)
clearCSRFCookie(w)
// Public API paths (login/register/password-reset, health, share
// links, plan-limits) must STILL work after the session was
// destroyed — a user with a stale cookie needs a way back in, and
// Prometheus health probes shouldn't 401 because some other tab
// left a stale session. Pass the request through unauthenticated.
if isPublicAPIPath(r.URL.Path) {
return sessionIPChangeRevoked
}
if strings.HasPrefix(r.URL.Path, "/api/") {
writeError(w, http.StatusUnauthorized, "session_ip_changed",
"Session client IP changed — please log in again.")
return sessionIPChangeTerminated
}
// Non-API (browser) path: caller must NOT install the destroyed
// session's user into the request context. The SPA will render its
// unauth state and the user will redirect to login.
return sessionIPChangeRevoked
}
// Log-only mode: CAS-rotate the stored IP so only the winner logs.
// The CAS compares against session.IPAddress (the raw value we read
// from the DB) and writes the canonical newIP so future comparisons
// are consistent. Err on the side of "winner" when the CAS errors
// — we'd rather log an extra row than miss the signal entirely.
// A failed rotate here is non-fatal: worst case the next request
// also gets an audit row.
won, err := s.store.UpdateSessionIPIfEquals(plainToken, session.IPAddress, newIP)
if err != nil {
slog.Warn("failed to rotate session ip after change", "error", err)
won = true
}
if won {
s.logAuditEventForUser(models.ActionSessionIPChanged, r, userID, auditMeta(map[string]string{
"old_ip": storedIP,
"new_ip": newIP,
}))
slog.Info("session client IP changed (logged, request allowed)",
"session_ip", storedIP,
"client_ip", newIP,
"user_id", userID)
}
return sessionIPChangeAllowedLogged
}
// clearSessionCookie expires the session cookie on the client so a
// subsequent request doesn't keep presenting a now-revoked token.
func clearSessionCookie(w http.ResponseWriter, secure bool) {
http.SetCookie(w, &http.Cookie{
Name: sessionCookieName(secure),
Value: "",
Path: "/",
MaxAge: -1,
HttpOnly: true,
Secure: secure,
SameSite: http.SameSiteLaxMode,
})
}
// tokenScopeAllows checks if the token's scopes permit the given HTTP method
// and path. Scopes are stored as a JSON array of strings.
//
+16 -8
View File
@@ -3,6 +3,7 @@ package server
import (
"log/slog"
"math"
"net"
"net/http"
"strconv"
"strings"
@@ -279,16 +280,23 @@ func rateLimitKey(r *http.Request, ip string) string {
}
// clientIP extracts the client IP from RemoteAddr. This is safe because
// chimiddleware.RealIP runs earlier in the chain and overwrites RemoteAddr
// with the trusted value from X-Real-IP / X-Forwarded-For. We deliberately
// do NOT read proxy headers here to prevent clients from spoofing their IP
// to bypass rate limits.
// TrustedProxyRealIP runs earlier in the chain and — when a trusted
// proxy is configured — overwrites RemoteAddr with the trusted value
// from X-Real-IP / X-Forwarded-For. We deliberately do NOT read proxy
// headers here to prevent clients from spoofing their IP to bypass
// rate limits.
//
// Uses net.SplitHostPort so IPv6 addresses are handled correctly.
// A naive LastIndex(":") strips the final hextet of a bare IPv6 address
// like "2001:db8::1" — TrustedProxyRealIP writes the X-Forwarded-For
// value verbatim (no port, no brackets), so a LastIndex-based parse
// would mangle it. For bare IPs without a port SplitHostPort returns
// an error and we return the address as-is.
func clientIP(r *http.Request) string {
host := r.RemoteAddr
if idx := strings.LastIndex(host, ":"); idx != -1 {
return host[:idx]
if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
return host
}
return host
return r.RemoteAddr
}
// writeRateLimitResponse sends a 429 response with Retry-After and X-RateLimit-* headers.
@@ -0,0 +1,425 @@
package server
import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/xarmian/pad/internal/models"
)
// doRequestWithCookieFrom is like doRequestWithCookie but lets the caller
// set the request RemoteAddr so tests can simulate a session jumping to a
// different client IP mid-lifetime.
func doRequestWithCookieFrom(srv *Server, method, path string, body interface{}, token, remoteAddr string) *httptest.ResponseRecorder {
var bodyReader io.Reader
if body != nil {
data, _ := json.Marshal(body)
bodyReader = bytes.NewReader(data)
}
req := httptest.NewRequest(method, path, bodyReader)
if body != nil {
req.Header.Set("Content-Type", "application/json")
}
req.RemoteAddr = remoteAddr
req.AddCookie(&http.Cookie{
Name: "pad_session",
Value: token,
})
const testCSRF = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
req.AddCookie(&http.Cookie{
Name: "pad_csrf",
Value: testCSRF,
})
req.Header.Set("X-CSRF-Token", testCSRF)
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, req)
return rr
}
// TestSessionIPChange_LogOnlyDefault verifies that without
// PAD_IP_CHANGE_ENFORCE=strict, a session that presents a different client
// IP is allowed through but an ActionSessionIPChanged audit row is written.
func TestSessionIPChange_LogOnlyDefault(t *testing.T) {
srv := testServer(t)
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin") // bootstrap creates from 127.0.0.1
// First request from the original IP — no change, no audit row.
rr := doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "127.0.0.1:2222")
if rr.Code != http.StatusOK {
t.Fatalf("same-IP request: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
if got := countIPChangeEvents(t, srv); got != 0 {
t.Fatalf("no audit row expected before IP change, got %d", got)
}
// Request from a brand-new IP — request still succeeds (log-only).
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code != http.StatusOK {
t.Fatalf("IP-changed request in log-only mode: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("expected 1 ActionSessionIPChanged audit row after IP change, got %d", got)
}
// Subsequent request from the same new IP must NOT re-log (we updated
// the stored IP in-place on the first hit).
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code != http.StatusOK {
t.Fatalf("same-new-IP repeat: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("no additional audit row expected after IP settles, got %d", got)
}
}
// TestSessionIPChange_StrictRejects verifies that with strict enforcement
// enabled, a session presenting a different client IP is rejected AND the
// session is destroyed (so a stolen token can't be retried from the same
// new IP).
func TestSessionIPChange_StrictRejects(t *testing.T) {
srv := testServer(t)
srv.SetIPChangeEnforce("strict")
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
// Sanity: original IP still works.
rr := doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "127.0.0.1:2222")
if rr.Code != http.StatusOK {
t.Fatalf("same-IP request: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
// Jump to a new IP — must be rejected with 401 in strict mode.
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code != http.StatusUnauthorized {
t.Fatalf("IP-changed request in strict mode: expected 401, got %d: %s", rr.Code, rr.Body.String())
}
// Audit row still gets written.
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("expected 1 ActionSessionIPChanged audit row in strict mode, got %d", got)
}
// Retrying with the same token — even from the new IP — must fail
// (session was destroyed).
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code == http.StatusOK {
t.Fatal("session should have been destroyed after strict rejection")
}
}
// TestSessionIPChange_StrictDestroysSessionAtomically verifies that in
// strict mode the session is destroyed as part of the IP-mismatch check
// and does NOT survive with a rebound IP. A regression would mean that
// if DeleteSession were ever separated from the rotation, a follow-up
// request from the new IP could pass authentication.
func TestSessionIPChange_StrictDestroysSessionAtomically(t *testing.T) {
srv := testServer(t)
srv.SetIPChangeEnforce("strict")
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
// First IP-mismatch request → 401.
rr := doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code != http.StatusUnauthorized {
t.Fatalf("expected 401 on IP mismatch, got %d: %s", rr.Code, rr.Body.String())
}
// Second request from the NEW IP must also fail — the session must
// be gone, not rebound to 198.51.100.7. A regression that rotated
// the stored IP before destroying would leak through here.
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code == http.StatusOK {
t.Fatal("session must be destroyed in strict mode, not rebound to new IP")
}
// Single audit row for the whole transition (the second request hits
// the ValidateSession-returns-nil branch and never re-logs).
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("expected exactly 1 audit row for the strict-mode transition, got %d", got)
}
}
// TestSessionIPChange_StrictClearsCookies verifies that in strict mode
// the session cookie is explicitly cleared on the response (MaxAge=-1) so
// the browser doesn't keep re-sending a now-revoked token on the next
// navigation. Only the API 401 path reaches SessionAuth in the current
// routing (the SPA catch-all is mounted on the root router outside the
// auth group), but cookie hygiene still applies for API clients that
// present cookies (e.g. the web UI's fetch calls).
func TestSessionIPChange_StrictClearsCookies(t *testing.T) {
srv := testServer(t)
srv.SetIPChangeEnforce("strict")
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
rr := doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code != http.StatusUnauthorized {
t.Fatalf("expected 401, got %d: %s", rr.Code, rr.Body.String())
}
// Session cookie must have been cleared by Set-Cookie: MaxAge=-1.
foundClear := false
for _, c := range rr.Result().Cookies() {
if strings.HasPrefix(c.Name, "pad_session") && c.MaxAge < 0 {
foundClear = true
break
}
}
if !foundClear {
t.Fatalf("expected session cookie to be cleared in strict mode; cookies=%v", rr.Result().Cookies())
}
}
// TestSessionIPChange_StrictAllowsPublicAPIPaths verifies that a stale
// session cookie on a PUBLIC API path (login, password reset, health,
// share links, plan-limits) does NOT produce a 401 in strict mode. The
// session still gets destroyed and the cookie cleared so the stale token
// can't be reused, but the request falls through so the user can log in
// again or the health probe succeeds.
func TestSessionIPChange_StrictAllowsPublicAPIPaths(t *testing.T) {
srv := testServer(t)
srv.SetIPChangeEnforce("strict")
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
// POST /api/v1/auth/login with the stale session cookie AND a
// different client IP. The handler expects email+password in the body
// and returns 400 "bad_request" on invalid JSON, so we send an empty
// body to keep the assertion simple. The important thing is: we
// don't get 401 session_ip_changed — the stale cookie didn't block
// the user from re-authenticating.
rr := doRequestWithCookieFrom(srv, "POST", "/api/v1/auth/login", map[string]string{
"email": "admin@example.com",
"password": "wrong-password-on-purpose",
}, token, "198.51.100.7:5555")
if rr.Code == http.StatusUnauthorized {
// Specifically, must NOT be the IP-change error code.
if strings.Contains(rr.Body.String(), "session_ip_changed") {
t.Fatalf("stale cookie on login endpoint blocked user with session_ip_changed: %s", rr.Body.String())
}
}
// Session must still be destroyed + audit row written.
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("expected 1 ActionSessionIPChanged audit row, got %d", got)
}
// A subsequent authenticated API call with the same stale token must
// now fail — session is gone.
rr = doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
if rr.Code == http.StatusOK {
t.Fatal("revoked session must not authenticate after strict IP-change destroy")
}
// Plan-limits is another public path — same treatment.
srv2 := testServer(t)
srv2.SetIPChangeEnforce("strict")
token2 := bootstrapFirstUser(t, srv2, "admin2@example.com", "Admin2")
rr = doRequestWithCookieFrom(srv2, "GET", "/api/v1/plan-limits", nil, token2, "198.51.100.7:5555")
if rr.Code == http.StatusUnauthorized && strings.Contains(rr.Body.String(), "session_ip_changed") {
t.Fatalf("stale cookie on plan-limits blocked with session_ip_changed: %s", rr.Body.String())
}
}
// TestSessionIPChange_CASDedupesRace verifies that the compare-and-set
// update scheme produces at most one audit row per IP-change transition,
// even when many concurrent requests arrive from the new IP before any
// of them has observed the rotated value.
func TestSessionIPChange_CASDedupesRace(t *testing.T) {
srv := testServer(t)
token := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
// Warm the stored IP with an initial request from the bootstrap source.
rr := doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "127.0.0.1:2222")
if rr.Code != http.StatusOK {
t.Fatalf("warmup: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
// Fire N concurrent requests from the new IP. Only the request whose
// CAS actually rotated the stored IP should have logged an audit row.
const n = 20
done := make(chan struct{}, n)
for i := 0; i < n; i++ {
go func() {
doRequestWithCookieFrom(srv, "GET", "/api/v1/auth/me", nil, token, "198.51.100.7:5555")
done <- struct{}{}
}()
}
for i := 0; i < n; i++ {
<-done
}
if got := countIPChangeEvents(t, srv); got != 1 {
t.Fatalf("expected exactly 1 audit row from %d concurrent requests, got %d", n, got)
}
}
// TestClientIP_IPv6NotMangled verifies the clientIP helper uses
// net.SplitHostPort correctly so IPv6 addresses aren't truncated at an
// internal colon. After TrustedProxyRealIP writes X-Forwarded-For
// verbatim into RemoteAddr, a bare IPv6 like "2001:db8::1" has no port
// and the old LastIndex(":") approach would return "2001:db8:" —
// mangled and unusable for session-IP audits.
func TestClientIP_IPv6NotMangled(t *testing.T) {
cases := []struct {
remoteAddr string
want string
}{
{"192.0.2.1:1234", "192.0.2.1"},
{"192.0.2.1", "192.0.2.1"},
{"[2001:db8::1]:8080", "2001:db8::1"},
{"2001:db8::1", "2001:db8::1"}, // bare IPv6 (no port, no brackets)
{"127.0.0.1:1234", "127.0.0.1"},
{"::1", "::1"},
}
for _, tc := range cases {
req, _ := http.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = tc.remoteAddr
if got := clientIP(req); got != tc.want {
t.Errorf("clientIP(%q) = %q, want %q", tc.remoteAddr, got, tc.want)
}
}
}
// TestSessionAuth_ShortCircuitsOnAPITokenAuth verifies that when a
// request carries BOTH a valid API token (Authorization: Bearer pad_...)
// AND a stale session cookie with a different client IP, SessionAuth
// short-circuits and lets the token-authenticated request through
// instead of 401-ing the whole thing in strict mode. Without this,
// valid token calls could be rejected purely because an unrelated
// browser tab dropped a stale cookie into the request.
func TestSessionAuth_ShortCircuitsOnAPITokenAuth(t *testing.T) {
srv := testServer(t)
// Bootstrap + create the token BEFORE enabling strict mode, so the
// setup calls (which come from 127.0.0.1 vs doRequestWithCookie's
// 192.0.2.1) don't trip the IP-change check.
sessionToken := bootstrapFirstUser(t, srv, "admin@example.com", "Admin")
// api_tokens.workspace_id is NOT NULL on the legacy schema, so create
// a workspace first and scope the token to it.
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces",
map[string]string{"name": "IP-Test"}, sessionToken)
if rr.Code != http.StatusCreated {
t.Fatalf("create workspace: expected 201, got %d: %s", rr.Code, rr.Body.String())
}
var ws struct {
ID string `json:"id"`
Slug string `json:"slug"`
}
parseJSON(t, rr, &ws)
// Create a user-scoped API token for the workspace. TokenAuth will
// set currentUser from the user_id on the token.
rr = doRequestWithCookie(srv, "POST", "/api/v1/auth/tokens", map[string]interface{}{
"name": "integration-test-token",
"workspace_id": ws.ID,
}, sessionToken)
if rr.Code != http.StatusCreated {
t.Fatalf("create token: expected 201, got %d: %s", rr.Code, rr.Body.String())
}
var tokResp map[string]interface{}
parseJSON(t, rr, &tokResp)
apiToken, _ := tokResp["token"].(string)
if apiToken == "" {
t.Fatal("expected token in response")
}
// Snapshot audit count AFTER setup so we can isolate the effect of
// the final request (setup calls from the 192.0.2.1 test remote will
// have emitted their own audit rows in log-only mode before strict
// is enabled).
srv.SetIPChangeEnforce("strict")
before := countIPChangeEvents(t, srv)
// Request with BOTH the API token AND a stale session cookie, from a
// new client IP. Strict mode would 401 the cookie path — but the
// token-auth short-circuit must kick in first.
req, _ := http.NewRequest(http.MethodGet, "/api/v1/auth/me", nil)
req.RemoteAddr = "203.0.113.9:5555" // different IP from both bootstrap (127.0.0.1) and setup (192.0.2.1)
req.Header.Set("Authorization", "Bearer "+apiToken)
req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken})
const testCSRF = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
req.AddCookie(&http.Cookie{Name: "pad_csrf", Value: testCSRF})
req.Header.Set("X-CSRF-Token", testCSRF)
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("token + stale cookie in strict mode: expected 200 (token auth wins), got %d: %s", rec.Code, rec.Body.String())
}
// The session-IP-change path MUST NOT have fired — no new audit rows
// should appear for this request.
after := countIPChangeEvents(t, srv)
if after != before {
t.Fatalf("expected 0 new ActionSessionIPChanged audit rows when token auth short-circuits; before=%d after=%d", before, after)
}
}
// TestCanonicalIP verifies equivalent IPv6 representations collapse to a
// single canonical form so the session-IP-change comparison doesn't
// spuriously fire on "0000:0000:0000:0000:0000:0000:0000:0001" vs "::1".
func TestCanonicalIP(t *testing.T) {
cases := []struct {
in, want string
}{
{"", ""},
{"127.0.0.1", "127.0.0.1"},
{"192.0.2.1", "192.0.2.1"},
{"::1", "::1"},
{"0000:0000:0000:0000:0000:0000:0000:0001", "::1"},
{"2001:DB8::1", "2001:db8::1"},
{"2001:0db8:0000:0000:0000:0000:0000:0001", "2001:db8::1"},
{"not-an-ip", "not-an-ip"}, // non-parseable stays as-is
}
for _, tc := range cases {
if got := canonicalIP(tc.in); got != tc.want {
t.Errorf("canonicalIP(%q) = %q, want %q", tc.in, got, tc.want)
}
}
}
// TestSetIPChangeEnforce_CaseInsensitive verifies the setter accepts
// common variants without surprises.
func TestSetIPChangeEnforce_CaseInsensitive(t *testing.T) {
srv := testServer(t)
cases := []struct {
in string
want bool
}{
{"", false},
{"strict", true},
{"STRICT", true},
{" Strict ", true},
{"log", false},
{"yes", false},
}
for _, tc := range cases {
srv.SetIPChangeEnforce(tc.in)
if srv.ipChangeEnforceStrict != tc.want {
t.Errorf("SetIPChangeEnforce(%q) → ipChangeEnforceStrict=%v, want %v",
tc.in, srv.ipChangeEnforceStrict, tc.want)
}
}
}
// countIPChangeEvents returns the number of session_ip_changed rows in the
// audit log.
func countIPChangeEvents(t *testing.T, srv *Server) int {
t.Helper()
acts, err := srv.store.ListAuditLog(models.AuditLogParams{
Action: models.ActionSessionIPChanged,
Limit: 100,
})
if err != nil {
t.Fatalf("list audit log: %v", err)
}
// Guard against accidental matches on unrelated actions.
n := 0
for _, a := range acts {
if a.Action == models.ActionSessionIPChanged {
n++
}
}
return n
}
// keep strings reference in case future cases need it; harmless no-op.
var _ = strings.EqualFold
+17 -5
View File
@@ -42,11 +42,12 @@ type Server struct {
baseURL string // public base URL for generating links (e.g. invite URLs)
corsOrigins string // comma-separated CORS origins (empty = localhost defaults)
secureCookies bool // set Secure flag on cookies (for TLS deployments)
metrics *metrics.Metrics // Prometheus metrics (optional)
metricsToken string // shared bearer token for /metrics scrapes ("" = loopback-only)
trustedProxyCIDRs []*net.IPNet // CIDRs allowed to set X-Forwarded-For (nil = proxy headers untrusted)
sseMaxConnections int // global SSE connection limit (0 = unlimited)
sseMaxPerWorkspace int // per-workspace SSE connection limit (0 = unlimited)
metrics *metrics.Metrics // Prometheus metrics (optional)
metricsToken string // shared bearer token for /metrics scrapes ("" = loopback-only)
trustedProxyCIDRs []*net.IPNet // CIDRs allowed to set X-Forwarded-For (nil = proxy headers untrusted)
ipChangeEnforceStrict bool // when true, reject sessions whose client IP differs from the one recorded at session creation
sseMaxConnections int // global SSE connection limit (0 = unlimited)
sseMaxPerWorkspace int // per-workspace SSE connection limit (0 = unlimited)
cloudMode bool // true when running as Pad Cloud (PAD_CLOUD=true or PAD_MODE=cloud)
cloudSecrets []string // shared secrets for sidecar ↔ pad communication (supports rotation)
version string // release version (e.g. "dev", "1.2.3")
@@ -237,6 +238,17 @@ func (s *Server) SetTrustedProxies(spec string) {
s.trustedProxyCIDRs = ParseTrustedProxyCIDRs(spec)
}
// SetIPChangeEnforce controls how the auth middleware reacts when a
// session's client IP changes mid-lifetime:
// - mode == "strict": reject the request (session treated as possibly stolen)
// - anything else (default): log to the audit log, update the stored IP,
// and let the request through. Strict mode breaks legitimate mobility
// (mobile roaming, VPN toggles) so it is opt-in for high-sensitivity
// deployments via the PAD_IP_CHANGE_ENFORCE env var.
func (s *Server) SetIPChangeEnforce(mode string) {
s.ipChangeEnforceStrict = strings.EqualFold(strings.TrimSpace(mode), "strict")
}
// reconfigureEmail reads email settings from the platform_settings table
// and updates (or creates) the email sender. Called after admin settings change.
func (s *Server) reconfigureEmail() {
+53
View File
@@ -94,6 +94,35 @@ func (s *Store) ValidateSession(token string) (*SessionInfo, error) {
}, nil
}
// UpdateSessionIPIfEquals atomically rotates the stored IP on a session
// from expectedOldIP to newIP. Returns true if a row was updated (i.e. the
// caller is the race winner and should write the audit row); false when
// some other concurrent request already rotated the IP ahead of us.
//
// This compare-and-set behavior is what lets the auth middleware emit
// exactly one ActionSessionIPChanged audit row per transition even when
// multiple requests race after the client's IP flips.
func (s *Store) UpdateSessionIPIfEquals(token, expectedOldIP, newIP string) (bool, error) {
hash := sha256.Sum256([]byte(token))
tokenHash := hex.EncodeToString(hash[:])
res, err := s.db.Exec(s.q(`
UPDATE sessions
SET ip_address = ?
WHERE token_hash = ? AND ip_address = ?
`), newIP, tokenHash, expectedOldIP)
if err != nil {
return false, fmt.Errorf("update session ip: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
// Some drivers don't report RowsAffected reliably. Err on the safe
// side: treat as "not the winner" so we don't log when unsure.
return false, nil
}
return n > 0, nil
}
// DeleteSession destroys a session by its plaintext token.
func (s *Store) DeleteSession(token string) error {
hash := sha256.Sum256([]byte(token))
@@ -106,6 +135,30 @@ func (s *Store) DeleteSession(token string) error {
return nil
}
// DeleteSessionIfExists destroys a session by plaintext token and reports
// whether a row was actually deleted. Used by the IP-change strict-mode
// path as a compare-and-set primitive: only the caller that actually
// deletes the row emits the ActionSessionIPChanged audit entry so
// concurrent requests don't write duplicate audit rows, and only the
// caller whose DELETE succeeded on the DB gets a clean "session is now
// gone" guarantee.
func (s *Store) DeleteSessionIfExists(token string) (bool, error) {
hash := sha256.Sum256([]byte(token))
tokenHash := hex.EncodeToString(hash[:])
res, err := s.db.Exec(s.q("DELETE FROM sessions WHERE token_hash = ?"), tokenHash)
if err != nil {
return false, fmt.Errorf("delete session: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
// Drivers that don't report affected rows → treat as "existed" so
// callers don't skip their post-delete work.
return true, nil
}
return n > 0, nil
}
// DeleteUserSessions destroys all sessions for a user (logout everywhere).
func (s *Store) DeleteUserSessions(userID string) error {
_, err := s.db.Exec(s.q("DELETE FROM sessions WHERE user_id = ?"), userID)