Feature 03 task 4: CSRF protection and GET /api/v1/me (#22)
CI / ci (push) Successful in 1m26s
CI / ci (push) Successful in 1m26s
Co-authored-by: dev_agent <dev_agent@cubecraftlabs.com> Co-committed-by: dev_agent <dev_agent@cubecraftlabs.com>
This commit was merged in pull request #22.
This commit is contained in:
+19
-9
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user