Compare commits

...
Author SHA1 Message Date
dev_agentandClaude Opus 5.5 15cc3c5ce4 Add CSRF protection and GET /api/v1/me
Independent review Codex gpt-6-sol high: APPROVED (0 findings)
CI / ci (pull_request) Successful in 1m27s
CI / ci (push) Successful in 1m23s
RequireSession now stores the validated session in the request context
(auth.WithSession / auth.SessionFrom); auth.ActorFrom derives the actor
from it, so there is one context value.

Every /api/v1 request other than GET, HEAD and OPTIONS must carry
X-MealQuest-CSRF equal to the session's csrf_token (constant-time
compare), else 403 {"error":{"code":"csrf"}}. The check runs after
RequireSession, so unauthenticated requests still get 401. A session
with no token is a 500, never a match against a missing header.

GET /api/v1/me returns {name, email, household: {label: "Our kitchen"},
csrfToken} with Cache-Control: no-store.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 10:32:31 -04:00
9 changed files with 306 additions and 23 deletions
+19 -9
View File
@@ -19,17 +19,27 @@ func (s Session) Actor() Actor {
return Actor{Subject: s.Subject, Email: s.Email, Name: s.Name, Groups: slices.Clone(s.Groups)}
}
type actorKey struct{}
type sessionKey struct{}
// WithActor returns a copy of ctx carrying actor. The session middleware
// calls it for every authenticated request.
func WithActor(ctx context.Context, actor Actor) context.Context {
return context.WithValue(ctx, actorKey{}, actor)
// WithSession returns a copy of ctx carrying s. The session middleware calls
// it for every authenticated request.
func WithSession(ctx context.Context, s Session) context.Context {
return context.WithValue(ctx, sessionKey{}, s)
}
// ActorFrom returns the actor WithActor stored in ctx. ok is false when ctx
// has none, which behind the session middleware means a wiring bug.
// SessionFrom returns the session WithSession stored in ctx. ok is false when
// ctx has none, which behind the session middleware means a wiring bug.
func SessionFrom(ctx context.Context) (s Session, ok bool) {
s, ok = ctx.Value(sessionKey{}).(Session)
return s, ok
}
// ActorFrom returns the actor of the session WithSession stored in ctx. ok is
// false when ctx has none.
func ActorFrom(ctx context.Context) (actor Actor, ok bool) {
actor, ok = ctx.Value(actorKey{}).(Actor)
return actor, ok
s, ok := SessionFrom(ctx)
if !ok {
return Actor{}, false
}
return s.Actor(), true
}
+12 -4
View File
@@ -11,10 +11,18 @@ func TestActorContext(t *testing.T) {
if _, ok := ActorFrom(context.Background()); ok {
t.Error("ActorFrom found an actor in an empty context")
}
want := Actor{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}}
got, ok := ActorFrom(WithActor(context.Background(), want))
if !ok || got.Subject != want.Subject || got.Email != want.Email || got.Name != want.Name || !slices.Equal(got.Groups, want.Groups) {
t.Errorf("ActorFrom = %+v (ok=%v), want %+v", got, ok, want)
if _, ok := SessionFrom(context.Background()); ok {
t.Error("SessionFrom found a session in an empty context")
}
s := Session{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}, CSRFToken: "t"}
ctx := WithSession(context.Background(), s)
gotSession, ok := SessionFrom(ctx)
if !ok || gotSession.Subject != s.Subject || gotSession.CSRFToken != s.CSRFToken {
t.Errorf("SessionFrom = %+v (ok=%v), want %+v", gotSession, ok, s)
}
got, ok := ActorFrom(ctx)
if !ok || got.Subject != s.Subject || got.Email != s.Email || got.Name != s.Name || !slices.Equal(got.Groups, s.Groups) {
t.Errorf("ActorFrom = %+v (ok=%v), want the session's actor", got, ok)
}
}
+42
View File
@@ -0,0 +1,42 @@
package server
import (
"crypto/subtle"
"log/slog"
"net/http"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth"
)
// CSRFHeader carries the session's CSRF token, which the client reads from
// GET /api/v1/me.
const CSRFHeader = "X-MealQuest-CSRF"
// requireCSRF passes GET, HEAD and OPTIONS requests to next; every other
// request must carry CSRFHeader equal to its session's CSRF token or get 403
// csrf. It runs behind requireSession, which puts the session in the context.
func requireCSRF(logger *slog.Logger) func(http.Handler) http.Handler {
forbidden := apiErrorHandler(logger, http.StatusForbidden, "csrf", "missing or invalid CSRF token")
internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error")
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodGet, http.MethodHead, http.MethodOptions:
next.ServeHTTP(w, r)
return
}
s, ok := auth.SessionFrom(r.Context())
if !ok || s.CSRFToken == "" {
// An empty token would match a missing header.
logger.ErrorContext(r.Context(), "csrf check without a session token", "session", ok)
internal(w, r)
return
}
if subtle.ConstantTimeCompare([]byte(r.Header.Get(CSRFHeader)), []byte(s.CSRFToken)) != 1 {
forbidden(w, r)
return
}
next.ServeHTTP(w, r)
})
}
}
+165
View File
@@ -0,0 +1,165 @@
package server
import (
"context"
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth"
)
const (
csrfJSON = `{"error":{"code":"csrf","message":"missing or invalid CSRF token","retryable":false}}`
methodNotAllowedJSON = `{"error":{"code":"method_not_allowed","message":"method not allowed","retryable":false}}`
)
func TestCSRFThroughRouter(t *testing.T) {
t.Parallel()
router := newTestRouter(t, "dev")
for _, tc := range []struct {
name, method, path string
header []string // nil sends no CSRF header
wantStatus int
wantBody string
}{
{"POST without header", http.MethodPost, "/api/v1/me", nil, http.StatusForbidden, csrfJSON},
{"POST with wrong header", http.MethodPost, "/api/v1/me", []string{"wrong-token"}, http.StatusForbidden, csrfJSON},
{"POST with empty header", http.MethodPost, "/api/v1/me", []string{""}, http.StatusForbidden, csrfJSON},
{"POST with token prefix", http.MethodPost, "/api/v1/me", []string{"csrf-token"}, http.StatusForbidden, csrfJSON},
{"PATCH without header", http.MethodPatch, "/api/v1/settings/household", nil, http.StatusForbidden, csrfJSON},
{"DELETE with wrong header", http.MethodDelete, "/api/v1/me", []string{"wrong-token"}, http.StatusForbidden, csrfJSON},
// The right token passes the check and reaches the router, which has
// no POST /api/v1/me.
{"POST with right header", http.MethodPost, "/api/v1/me", []string{testSession.CSRFToken}, http.StatusMethodNotAllowed, methodNotAllowedJSON},
{"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusNotFound, notFoundJSON},
{"GET without header", http.MethodGet, "/api/v1/me", nil, http.StatusOK, ""},
// HEAD and OPTIONS pass the check and reach the router, which
// answers neither for /api/v1/me.
{"HEAD without header", http.MethodHead, "/api/v1/me", nil, http.StatusMethodNotAllowed, ""},
{"OPTIONS without header", http.MethodOptions, "/api/v1/me", nil, http.StatusMethodNotAllowed, methodNotAllowedJSON},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest(tc.method, tc.path, nil)
req.AddCookie(validSessionCookie)
if tc.header != nil {
req.Header.Set(CSRFHeader, tc.header[0])
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != tc.wantStatus {
t.Errorf("status = %d, want %d (body %s)", rec.Code, tc.wantStatus, rec.Body.String())
}
if tc.wantBody != "" && rec.Body.String() != tc.wantBody {
t.Errorf("body = %s, want %s", rec.Body.String(), tc.wantBody)
}
})
}
}
func TestCSRFUnauthenticatedGets401(t *testing.T) {
t.Parallel()
req := httptest.NewRequest(http.MethodPost, "/api/v1/me", nil)
rec := httptest.NewRecorder()
newTestRouter(t, "dev").ServeHTTP(rec, req)
if rec.Code != http.StatusUnauthorized || rec.Body.String() != unauthenticatedJSON {
t.Errorf("response = %d %s, want 401 %s", rec.Code, rec.Body.String(), unauthenticatedJSON)
}
}
func TestRequireCSRF(t *testing.T) {
t.Parallel()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
for _, tc := range []struct {
name string
method string
session *auth.Session
header string
wantStatus int
}{
{"right header reaches handler", http.MethodPost, &testSession, testSession.CSRFToken, http.StatusNoContent},
{"wrong header", http.MethodPut, &testSession, "nope", http.StatusForbidden},
{"GET needs no header", http.MethodGet, &testSession, "", http.StatusNoContent},
{"no session in context", http.MethodPost, nil, "", http.StatusInternalServerError},
{"session without token", http.MethodPost, &auth.Session{Subject: "user-123"}, "", http.StatusInternalServerError},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
called := false
h := requireCSRF(logger)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
called = true
w.WriteHeader(http.StatusNoContent)
}))
ctx := context.Background()
if tc.session != nil {
ctx = auth.WithSession(ctx, *tc.session)
}
req := httptest.NewRequestWithContext(ctx, tc.method, "/api/v1/anything", nil)
if tc.header != "" {
req.Header.Set(CSRFHeader, tc.header)
}
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)
if rec.Code != tc.wantStatus {
t.Errorf("status = %d, want %d", rec.Code, tc.wantStatus)
}
if called != (tc.wantStatus == http.StatusNoContent) {
t.Errorf("handler called = %v with status %d", called, rec.Code)
}
})
}
}
func TestMe(t *testing.T) {
t.Parallel()
req := httptest.NewRequest(http.MethodGet, "/api/v1/me", nil)
req.AddCookie(validSessionCookie)
rec := httptest.NewRecorder()
newTestRouter(t, "dev").ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String())
}
if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
t.Errorf("Content-Type = %q, want application/json", ct)
}
if cc := rec.Header().Get("Cache-Control"); cc != "no-store" {
t.Errorf("Cache-Control = %q, want no-store", cc)
}
var got map[string]any
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
t.Fatalf("decode %s: %v", rec.Body.String(), err)
}
want := map[string]any{
"name": "Home Cook",
"email": "cook@example.com",
"household": map[string]any{"label": "Our kitchen"},
"csrfToken": "csrf-token-123",
}
gotJSON, _ := json.Marshal(got)
wantJSON, _ := json.Marshal(want)
if string(gotJSON) != string(wantJSON) {
t.Errorf("body = %s, want %s", gotJSON, wantJSON)
}
}
func TestMeNeedsSession(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
newTestRouter(t, "dev").ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/v1/me", nil))
if rec.Code != http.StatusUnauthorized || rec.Body.String() != unauthenticatedJSON {
t.Errorf("response = %d %s, want 401 %s", rec.Code, rec.Body.String(), unauthenticatedJSON)
}
}
+52
View File
@@ -0,0 +1,52 @@
package server
import (
"encoding/json"
"log/slog"
"net/http"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth"
)
// householdLabel is the single shared household identity.
const householdLabel = "Our kitchen"
type meResponse struct {
Name string `json:"name"`
Email string `json:"email"`
Household householdInfo `json:"household"`
CSRFToken string `json:"csrfToken"`
}
type householdInfo struct {
Label string `json:"label"`
}
// me answers GET /api/v1/me with the signed-in member, the household and the
// session's CSRF token for CSRFHeader.
func me(logger *slog.Logger) http.HandlerFunc {
internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error")
return func(w http.ResponseWriter, r *http.Request) {
s, ok := auth.SessionFrom(r.Context())
if !ok {
logger.ErrorContext(r.Context(), "GET /api/v1/me without a session")
internal(w, r)
return
}
body, err := json.Marshal(meResponse{
Name: s.Name,
Email: s.Email,
Household: householdInfo{Label: householdLabel},
CSRFToken: s.CSRFToken,
})
if err != nil {
panic(err) // string fields cannot fail to marshal
}
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Cache-Control", "no-store")
w.WriteHeader(http.StatusOK)
if _, err := w.Write(body); err != nil {
logger.WarnContext(r.Context(), "write me response", "error", err)
}
}
}
+5 -2
View File
@@ -24,7 +24,8 @@ type RouteMounter interface {
// NewRouter returns the HTTP handler for the service. version is reported by
// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and
// /media; assets is the web build served for every other GET. /health, /auth
// and the web build are public.
// and the web build are public. Requests to /api/v1 other than GET, HEAD and
// OPTIONS also need the session's CSRF token in CSRFHeader.
func NewRouter(logger *slog.Logger, version string, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found")
methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed")
@@ -44,15 +45,17 @@ func NewRouter(logger *slog.Logger, version string, auth RouteMounter, sessions
// rather than being added with Use: chi skips a subrouter's middleware
// when it has no routes, which would leave it unguarded.
signedIn := requireSession(logger, sessions)
csrf := requireCSRF(logger)
apiV1, media := chi.NewRouter(), chi.NewRouter()
for _, sub := range []*chi.Mux{apiV1, media} {
sub.MethodNotAllowed(methodNotAllowed)
sub.NotFound(notFound)
}
apiV1.Get("/me", me(logger))
r.Route("/api", func(r chi.Router) {
r.MethodNotAllowed(methodNotAllowed)
r.NotFound(notFound)
r.Mount("/v1", signedIn(apiV1))
r.Mount("/v1", signedIn(csrf(apiV1)))
})
r.Mount("/media", signedIn(media))
// Catch-all for the PWA. Unknown /auth paths reach the frontend handler,
+3 -1
View File
@@ -139,9 +139,11 @@ func TestReservedPathsNotFound(t *testing.T) {
t.Run(tt.method+" "+tt.path, func(t *testing.T) {
t.Parallel()
// Signed in, so /api/v1 and /media reach their routers.
// Signed in with the CSRF token, so /api/v1 and /media reach
// their routers.
req := httptest.NewRequest(tt.method, tt.path, nil)
req.AddCookie(validSessionCookie)
req.Header.Set(CSRFHeader, testSession.CSRFToken)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
+3 -3
View File
@@ -15,8 +15,8 @@ type SessionLookup interface {
}
// requireSession is the RequireSession middleware. It passes requests with a
// live session to next with the session's actor in the context (read it with
// auth.ActorFrom); others get 401 unauthenticated.
// live session to next with the session in the context (read it with
// auth.SessionFrom or auth.ActorFrom); others get 401 unauthenticated.
func requireSession(logger *slog.Logger, sessions SessionLookup) func(http.Handler) http.Handler {
unauthenticated := apiErrorHandler(logger, http.StatusUnauthorized, "unauthenticated", "sign in required")
internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error")
@@ -30,7 +30,7 @@ func requireSession(logger *slog.Logger, sessions SessionLookup) func(http.Handl
logger.ErrorContext(r.Context(), "look up session", "error", err)
internal(w, r)
default:
next.ServeHTTP(w, r.WithContext(auth.WithActor(r.Context(), s.Actor())))
next.ServeHTTP(w, r.WithContext(auth.WithSession(r.Context(), s)))
}
})
}
+5 -4
View File
@@ -21,10 +21,11 @@ var (
validSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "valid"}
brokenSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "broken"}
testSession = auth.Session{
Subject: "user-123",
Email: "cook@example.com",
Name: "Home Cook",
Groups: []string{"mealquest-admin"},
Subject: "user-123",
Email: "cook@example.com",
Name: "Home Cook",
Groups: []string{"mealquest-admin"},
CSRFToken: "csrf-token-123",
}
)