End any existing session when a sign-in is refused

A browser that still holds a session and signs in as an account outside
the household's group got the 403 page but kept its old session. The
refusal now ends that session and clears its cookie first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/auth/actor.go

@@ -19,27 +19,17 @@ func (s Session) Actor() Actor {
 	return Actor{Subject: s.Subject, Email: s.Email, Name: s.Name, Groups: slices.Clone(s.Groups)}
 }
 
-type sessionKey struct{}
+type actorKey struct{}
 
-// 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)
+// 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)
 }
 
-// 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.
+// ActorFrom returns the actor WithActor stored in ctx. ok is false when ctx
+// has none, which behind the session middleware means a wiring bug.
 func ActorFrom(ctx context.Context) (actor Actor, ok bool) {
-	s, ok := SessionFrom(ctx)
-	if !ok {
-		return Actor{}, false
-	}
-	return s.Actor(), true
+	actor, ok = ctx.Value(actorKey{}).(Actor)
+	return actor, ok
 }

internal/auth/actor_test.go

@@ -11,18 +11,10 @@ func TestActorContext(t *testing.T) {
 	if _, ok := ActorFrom(context.Background()); ok {
 		t.Error("ActorFrom found an actor in an empty context")
 	}
-	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)
+	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)
 	}
 }
 

internal/server/csrf.go

@@ -1,42 +0,0 @@
-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)
-		})
-	}
-}

internal/server/csrf_test.go

@@ -1,165 +0,0 @@
-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)
-	}
-}

internal/server/me.go

@@ -1,52 +0,0 @@
-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)
-		}
-	}
-}

internal/server/server.go

@@ -24,8 +24,7 @@ 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. Requests to /api/v1 other than GET, HEAD and
-// OPTIONS also need the session's CSRF token in CSRFHeader.
+// and the web build are public.
 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")
@@ -45,17 +44,15 @@ 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(csrf(apiV1)))
+		r.Mount("/v1", signedIn(apiV1))
 	})
 	r.Mount("/media", signedIn(media))
 	// Catch-all for the PWA. Unknown /auth paths reach the frontend handler,

internal/server/server_test.go

@@ -139,11 +139,9 @@ func TestReservedPathsNotFound(t *testing.T) {
 		t.Run(tt.method+" "+tt.path, func(t *testing.T) {
 			t.Parallel()
 
-			// Signed in with the CSRF token, so /api/v1 and /media reach
-			// their routers.
+			// Signed in, 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)
 

internal/server/session.go

@@ -15,8 +15,8 @@ type SessionLookup interface {
 }
 
 // requireSession is the RequireSession middleware. It passes requests with a
-// live session to next with the session in the context (read it with
-// auth.SessionFrom or auth.ActorFrom); others get 401 unauthenticated.
+// live session to next with the session's actor in the context (read it with
+// 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.WithSession(r.Context(), s)))
+				next.ServeHTTP(w, r.WithContext(auth.WithActor(r.Context(), s.Actor())))
 			}
 		})
 	}

internal/server/session_test.go

@@ -21,11 +21,10 @@ 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"},
-		CSRFToken: "csrf-token-123",
+		Subject: "user-123",
+		Email:   "cook@example.com",
+		Name:    "Home Cook",
+		Groups:  []string{"mealquest-admin"},
 	}
 )