diff --git a/docs/services/routing.md b/docs/services/routing.md index de727ac..5817bf3 100644 --- a/docs/services/routing.md +++ b/docs/services/routing.md @@ -221,3 +221,5 @@ cors: ``` This fragment belongs in `config/http.yaml`. List the frontend's exact origin; `*` is for public, credential-free APIs only. + +surf answers every `OPTIONS` request on a CORS path itself, with 204, before any route runs, so a plugin never registers an `OPTIONS` route. A preflight gets the same headers Laravel's `HandleCors` sends: with `allowed_methods: ["*"]` the `Access-Control-Allow-Methods` value is the requested method, upper-cased, and with `allowed_headers: ["*"]` the `Access-Control-Allow-Headers` value is the requested header list. diff --git a/modules/surf/README.md b/modules/surf/README.md index 319f11e..045b531 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -19,7 +19,7 @@ surf turns the routes that plugins declare through `pact.HasRoutes` into one `ht - Fixed-window rate limiting (`surf.FixedWindowLimiter`): named buckets from plugins that implement `surf.BucketProvider`, or inline limits keyed by the signed-in user, or by client IP for guests. Rejected requests get a 429 with `Retry-After` and `X-RateLimit-*` headers. The in-process `surf.MemoryStore` sits behind the `surf.Store` interface. - Client IP resolution for limiter keys (`surf.ClientIP`) that only trusts `X-Forwarded-For` hops when the direct peer is inside a configured trusted proxy range (`surf.TrustedProxies`). - Every non-raw route runs inside JSON panic recovery (an opaque 500 via [wire](../wire/README.md)), gets the request locale from the `Accept-Language` header (see [towel](../towel/README.md)) and a request body cap. Responses are buffered until the handler returns, so a panic never leaves a half-written body. -- Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. +- Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. Every `OPTIONS` request on a CORS path is answered with 204 before routing, with the headers Laravel's `HandleCors` sends: `Cache-Control: no-cache, private` always and, on a preflight (an `Origin` and an `Access-Control-Request-Method`), the requested method (upper-cased) and headers echoed in `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` when `*` allows any, `Vary` on the request headers and PHP's default `Content-Type: text/html; charset=UTF-8`. - `surf.LocaleFromPrincipal` switches the request locale to the signed-in user's preferred locale. - A read-only route table (`surf.Router.Routes`) and the `serve` and `route:list` commands. diff --git a/modules/surf/cors.go b/modules/surf/cors.go index 8fd113d..3324017 100644 --- a/modules/surf/cors.go +++ b/modules/surf/cors.go @@ -94,13 +94,43 @@ func pathScopedCORS(cfg CORSConfig, next http.Handler) http.Handler { } } if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) + writeOptions(w, r, allowed, allowAnyMethod, allowAnyHeader) return } next.ServeHTTP(w, r) }) } +// writeOptions answers an OPTIONS request on a CORS path with 204 and the +// headers Laravel's HandleCors (fruitcake/php-cors) sends, so recorded PHP +// preflights replay unchanged. Every answer carries Symfony's default +// Cache-Control "no-cache, private". A preflight (an Origin and an +// Access-Control-Request-Method) also carries the Content-Type PHP's SAPI +// adds to a response that sets none ("text/html; charset=UTF-8") and Vary +// on the two request headers; when any method or header is allowed, the +// Allow-Methods and Allow-Headers values echo the requested method +// (upper-cased) and headers, as php-cors does, instead of "*". +func writeOptions(w http.ResponseWriter, r *http.Request, allowed, allowAnyMethod, allowAnyHeader bool) { + h := w.Header() + h.Set("Cache-Control", "no-cache, private") + method := r.Header.Get("Access-Control-Request-Method") + if r.Header.Get("Origin") != "" && method != "" { + h.Set("Content-Type", "text/html; charset=UTF-8") + h.Add("Vary", "Access-Control-Request-Method, Access-Control-Request-Headers") + if allowed && allowAnyMethod { + h.Set("Access-Control-Allow-Methods", strings.ToUpper(method)) + } + if allowed && allowAnyHeader { + if requested := r.Header.Get("Access-Control-Request-Headers"); requested != "" { + h.Set("Access-Control-Allow-Headers", requested) + } + } + } else { + h.Add("Vary", "Access-Control-Request-Method") + } + w.WriteHeader(http.StatusNoContent) +} + func corsAllowOrigin(allowAny bool, origins map[string]struct{}, pats []*regexp.Regexp, origin string) (bool, string) { if allowAny { return true, "*" diff --git a/modules/surf/cors_coverage_test.go b/modules/surf/cors_coverage_test.go index 1f6fd6a..bed8ff1 100644 --- a/modules/surf/cors_coverage_test.go +++ b/modules/surf/cors_coverage_test.go @@ -90,3 +90,75 @@ func TestCORSAllowOriginExactAndPattern(t *testing.T) { } }) } + +// TestCORSOptionsMatchesLaravel pins the OPTIONS answers Laravel's +// HandleCors gives (recorded from PHP for the feedback widget): a +// preflight echoes the requested method and headers when any is allowed, +// with PHP's default Content-Type and Symfony's Cache-Control; a plain +// OPTIONS is a bare 204 with the Cache-Control. +func TestCORSOptionsMatchesLaravel(t *testing.T) { + cfg := CORSConfig{ + Paths: []string{"_feedback/api/*"}, + AllowedMethods: []string{"*"}, + AllowedOrigins: []string{"*"}, + AllowedHeaders: []string{"*"}, + } + called := false + h := pathScopedCORS(cfg, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true })) + + req := httptest.NewRequest(http.MethodOptions, "/_feedback/api/v1/wk_key/submit", nil) + req.Header.Set("Origin", "https://app.example.test") + req.Header.Set("Access-Control-Request-Method", "post") + req.Header.Set("Access-Control-Request-Headers", "content-type") + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + want := map[string]string{ + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Methods": "POST", + "Access-Control-Allow-Headers": "content-type", + "Cache-Control": "no-cache, private", + "Content-Type": "text/html; charset=UTF-8", + "Vary": "Access-Control-Request-Method, Access-Control-Request-Headers", + } + if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 { + t.Fatalf("preflight = %d %q", rec.Code, rec.Body.String()) + } + for k, v := range want { + if got := rec.Header().Get(k); got != v { + t.Errorf("preflight %s = %q, want %q", k, got, v) + } + } + + rec = httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodOptions, "/_feedback/api/v1/anything/else", nil)) + if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 { + t.Fatalf("plain OPTIONS = %d %q", rec.Code, rec.Body.String()) + } + if got := rec.Header().Get("Cache-Control"); got != "no-cache, private" { + t.Errorf("plain Cache-Control = %q", got) + } + if got := rec.Header().Get("Content-Type"); got != "" { + t.Errorf("plain Content-Type = %q", got) + } + if got := rec.Header().Get("Vary"); got != "Access-Control-Request-Method" { + t.Errorf("plain Vary = %q", got) + } + if called { + t.Fatal("an OPTIONS request reached the route handler") + } + + // A listed method set is sent as configured, not echoed. + listed := pathScopedCORS(CORSConfig{Paths: []string{"api/*"}, AllowedMethods: []string{"GET", "POST"}, AllowedOrigins: []string{"*"}, AllowedHeaders: []string{"Authorization"}}, http.NotFoundHandler()) + req = httptest.NewRequest(http.MethodOptions, "/api/items", nil) + req.Header.Set("Origin", "https://app.example.test") + req.Header.Set("Access-Control-Request-Method", "DELETE") + req.Header.Set("Access-Control-Request-Headers", "x-custom") + rec = httptest.NewRecorder() + listed.ServeHTTP(rec, req) + if got := rec.Header().Get("Access-Control-Allow-Methods"); got != "GET, POST" { + t.Errorf("listed methods = %q", got) + } + if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "Authorization" { + t.Errorf("listed headers = %q", got) + } +}