fix(middleware): return a JSON error for rejected CSRF requests (#18)

CSRF now rejects an untrusted cross-origin request with a structured
JSON 403, as CORS already does, instead of a `text/plain` body.

- `CSRF` calls `CrossOriginProtection.Check` and returns
`web.RespondError(ctx, w, errs.New(http.StatusForbidden, err))` on
rejection.
- The JSON `message` is the standard library's rejection reason.
- `CSRF` still panics at construction on an invalid trusted origin.
- `TestCSRF_Rejected` sends cross-origin POSTs with a cross-site
`Sec-Fetch-Site` and with an untrusted `Origin`; each gets 403 with a
JSON body.
- `TestCSRF_TrustedOriginAllowed` checks that a POST from a trusted
origin reaches the handler.

Closes #17

Author: adamwoolhether

web/middleware/csrf.go

@@ -2,20 +2,21 @@ package middleware
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
+	"github.com/adamwoolhether/httper/web"
+	"github.com/adamwoolhether/httper/web/errs"
 	"github.com/adamwoolhether/httper/web/mux"
 )
 
 // CSRF uses the standard library CrossOriginProtection to prevent CSRF attacks.
+// It rejects an untrusted cross-origin request with a 403 JSON error.
 // Each trusted origin must be an exact scheme://host[:port] value, such as
 // "https://app.example.com:8443", with no path, query, or trailing slash.
 // Wildcards are not supported: an entry such as "https://*.example.com" is
 // accepted but matches no origin. CSRF panics if a trusted origin is invalid.
 func CSRF(allowedOrigins ...string) mux.Middleware {
 	cop := http.NewCrossOriginProtection()
-	cop.SetDenyHandler(errHandler())
 	for _, origin := range allowedOrigins {
 		if err := cop.AddTrustedOrigin(origin); err != nil {
 			panic(err)
@@ -24,31 +25,15 @@ func CSRF(allowedOrigins ...string) mux.Middleware {
 
 	m := func(handler mux.Handler) mux.Handler {
 		h := func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
-			var err error
+			if err := cop.Check(r); err != nil {
+				return web.RespondError(ctx, w, errs.New(http.StatusForbidden, err))
+			}
 
-			std := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				ctx = r.Context()
-
-				err = handler(ctx, w, r)
-			})
-
-			cop.Handler(std).ServeHTTP(w, r.WithContext(ctx))
-
-			return err
+			return handler(ctx, w, r)
 		}
 
 		return h
 	}
 
 	return m
 }
-
-func errHandler() http.HandlerFunc {
-	f := func(w http.ResponseWriter, r *http.Request) {
-		mux.SetStatusCode(r.Context(), http.StatusForbidden)
-
-		http.Error(w, fmt.Errorf("csrf: %s", http.StatusText(http.StatusForbidden)).Error(), http.StatusForbidden)
-	}
-
-	return f
-}

web/middleware/csrf_test.go

@@ -1,11 +1,86 @@
 package middleware_test
 
 import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/adamwoolhether/httper/web/middleware"
 )
 
+func TestCSRF_Rejected(t *testing.T) {
+	tests := map[string]http.Header{
+		"cross-site fetch metadata": {"Sec-Fetch-Site": {"cross-site"}},
+		"untrusted origin":          {"Origin": {"https://evil.example.com"}},
+	}
+
+	for name, header := range tests {
+		t.Run(name, func(t *testing.T) {
+			called := false
+			handler := middleware.CSRF("https://app.example.com")(func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
+				called = true
+				return nil
+			})
+
+			w := httptest.NewRecorder()
+			r := httptest.NewRequest(http.MethodPost, "http://api.example.com/", nil)
+			r.Header = header
+
+			if err := handler(r.Context(), w, r); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			if w.Code != http.StatusForbidden {
+				t.Fatalf("status = %d, want %d", w.Code, http.StatusForbidden)
+			}
+			if called {
+				t.Fatal("handler should not be called for a rejected request")
+			}
+			if ct := w.Header().Get("Content-Type"); ct != "application/json" {
+				t.Fatalf("Content-Type = %q, want %q", ct, "application/json")
+			}
+
+			var body struct {
+				Code    int    `json:"code"`
+				Message string `json:"message"`
+			}
+			if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+				t.Fatalf("body %q should be JSON: %v", w.Body.String(), err)
+			}
+			if body.Code != http.StatusForbidden || body.Message == "" {
+				t.Fatalf("body = %+v, want code %d and a message", body, http.StatusForbidden)
+			}
+		})
+	}
+}
+
+func TestCSRF_TrustedOriginAllowed(t *testing.T) {
+	called := false
+	handler := middleware.CSRF("https://app.example.com")(func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
+		called = true
+		w.WriteHeader(http.StatusOK)
+		return nil
+	})
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest(http.MethodPost, "http://api.example.com/", nil)
+	r.Header.Set("Sec-Fetch-Site", "cross-site")
+	r.Header.Set("Origin", "https://app.example.com")
+
+	if err := handler(r.Context(), w, r); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if !called {
+		t.Fatal("handler should be called for a trusted origin")
+	}
+	if w.Code != http.StatusOK {
+		t.Fatalf("status = %d, want %d", w.Code, http.StatusOK)
+	}
+}
+
 func TestCSRF_TrustedOrigin(t *testing.T) {
 	tests := map[string]struct {
 		origin    string