Reject wildcard CORS origin when credentials are enabled

README.md

@@ -242,18 +242,43 @@ router.Use(rest.CORS(
 Features:
 - Automatic preflight (OPTIONS) handling
 - Origin validation with case-insensitive matching
-- Credentials support (reflects origin instead of `*`)
+- Credentials support (reflects the request origin instead of `*`)
 - Configurable cache duration for preflight results
 - Cache-correct `Vary` headers (adds `Access-Control-Request-Method` and `Access-Control-Request-Headers` on preflight)
 
 Available options:
-- `CorsAllowedOrigins(origins...)` - allowed origins (default: `*`)
+- `CorsAllowedOrigins(origins...)` - allowed origins (default: `*`), can't include `*` with credentials enabled
 - `CorsAllowedMethods(methods...)` - allowed HTTP methods (default: GET, POST, PUT, PATCH, DELETE, OPTIONS, HEAD)
 - `CorsAllowedHeaders(headers...)` - allowed request headers (default: Accept, Content-Type, Authorization, X-Requested-With)
 - `CorsExposedHeaders(headers...)` - headers exposed to client
 - `CorsAllowCredentials(bool)` - enable credentials (cookies, auth headers)
+- `CorsUnsafeAnyOriginWithCredentials(bool)` - allow `*` together with credentials, see below
 - `CorsMaxAge(seconds)` - preflight cache duration
 
+`CORS` panics if credentials are enabled while `*` is among the allowed origins, the default list included.
+That combination reflects any origin back together with `Access-Control-Allow-Credentials: true`, which lets
+any site a signed-in user visits read authenticated responses, so it should not be reached by accident.
+Name the origins instead:
+
+```go
+router.Use(rest.CORS(
+    rest.CorsAllowedOrigins("https://app.example.com"),
+    rest.CorsAllowCredentials(true),
+))
+```
+
+A service that genuinely has to accept credentialed requests from arbitrary third-party origins, such as an
+embeddable widget, can opt back in explicitly. Do this only when state-changing requests are protected by
+something other than the origin:
+
+```go
+router.Use(rest.CORS(
+    rest.CorsAllowedOrigins("*"),
+    rest.CorsAllowCredentials(true),
+    rest.CorsUnsafeAnyOriginWithCredentials(true),
+))
+```
+
 ### Secure middleware
 
 Adds security headers to responses. By default sets: `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, `X-XSS-Protection`, and `Strict-Transport-Security` (for HTTPS only).

cors.go

@@ -2,6 +2,7 @@ package rest
 
 import (
 	"net/http"
+	"slices"
 	"strconv"
 	"strings"
 )
@@ -10,7 +11,7 @@ import (
 // Use CorsOpt functions to customize.
 type CORSConfig struct {
 	// AllowedOrigins is a list of origins that may access the resource.
-	// use "*" to allow all origins (not recommended with credentials).
+	// use "*" to allow all origins, rejected by CORS when combined with credentials.
 	// default: ["*"]
 	AllowedOrigins []string
 	// AllowedMethods is a list of methods the client is allowed to use.
@@ -23,9 +24,14 @@ type CORSConfig struct {
 	// default: empty
 	ExposedHeaders []string
 	// AllowCredentials indicates whether the request can include credentials.
-	// when true, AllowedOrigins cannot be "*" (browser security restriction).
+	// when true, AllowedOrigins cannot contain "*" unless UnsafeAnyOriginWithCredentials is set,
+	// and CORS panics otherwise.
 	// default: false
 	AllowCredentials bool
+	// UnsafeAnyOriginWithCredentials permits "*" together with credentials, making the middleware
+	// reflect whatever Origin the request carries alongside Access-Control-Allow-Credentials.
+	// default: false
+	UnsafeAnyOriginWithCredentials bool
 	// MaxAge indicates how long (in seconds) the results of a preflight can be cached.
 	// default: 0 (no caching)
 	MaxAge int
@@ -47,7 +53,7 @@ func defaultCORSConfig() CORSConfig {
 }
 
 // CorsAllowedOrigins sets the list of allowed origins.
-// Use "*" to allow all origins (not recommended with credentials).
+// Use "*" to allow all origins, which CORS rejects when credentials are enabled.
 func CorsAllowedOrigins(origins ...string) CorsOpt {
 	return func(c *CORSConfig) {
 		c.AllowedOrigins = origins
@@ -76,13 +82,24 @@ func CorsExposedHeaders(headers ...string) CorsOpt {
 }
 
 // CorsAllowCredentials enables or disables credentials.
-// When true, AllowedOrigins cannot be "*".
+// When true, AllowedOrigins cannot contain "*" and CORS panics if it does.
 func CorsAllowCredentials(allow bool) CorsOpt {
 	return func(c *CORSConfig) {
 		c.AllowCredentials = allow
 	}
 }
 
+// CorsUnsafeAnyOriginWithCredentials permits "*" among the allowed origins together with credentials.
+// The middleware then reflects whatever Origin the request carries and sends
+// Access-Control-Allow-Credentials: true with it, so any site a signed-in user visits can read
+// authenticated responses. Only use it for a service meant to be embedded on arbitrary third-party
+// origins, and make sure state-changing requests are protected by something other than the origin.
+func CorsUnsafeAnyOriginWithCredentials(allow bool) CorsOpt {
+	return func(c *CORSConfig) {
+		c.UnsafeAnyOriginWithCredentials = allow
+	}
+}
+
 // CorsMaxAge sets how long (in seconds) preflight results can be cached.
 func CorsMaxAge(seconds int) CorsOpt {
 	return func(c *CORSConfig) {
@@ -93,12 +110,21 @@ func CorsMaxAge(seconds int) CorsOpt {
 // CORS is middleware that handles Cross-Origin Resource Sharing.
 // It handles preflight OPTIONS requests and sets appropriate headers.
 // By default allows all origins with common methods and headers.
+//
+// Panics if credentials are enabled while "*" is among the allowed origins, including the default
+// origin list. Such a configuration reflects any origin back with Access-Control-Allow-Credentials,
+// which lets any site read authenticated responses. Enumerate the origins instead.
 func CORS(opts ...CorsOpt) func(http.Handler) http.Handler {
 	cfg := defaultCORSConfig()
 	for _, opt := range opts {
 		opt(&cfg)
 	}
 
+	if cfg.AllowCredentials && !cfg.UnsafeAnyOriginWithCredentials && slices.Contains(cfg.AllowedOrigins, "*") {
+		panic(`rest: CORS with credentials can't allow "*" as an origin, list the allowed origins explicitly ` +
+			`or opt in with CorsUnsafeAnyOriginWithCredentials`)
+	}
+
 	// pre-compute joined strings for performance
 	methodsStr := strings.Join(cfg.AllowedMethods, ", ")
 	headersStr := strings.Join(cfg.AllowedHeaders, ", ")

cors_test.go

@@ -142,18 +142,36 @@ func TestCORS_Credentials(t *testing.T) {
 		assert.Equal(t, "true", resp.Header.Get("Access-Control-Allow-Credentials"))
 	})
 
-	t.Run("wildcard with credentials reflects origin", func(t *testing.T) {
+	t.Run("credentials with wildcard origins rejected", func(t *testing.T) {
+		tbl := []struct {
+			name string
+			opts []CorsOpt
+		}{
+			{"default origins", []CorsOpt{CorsAllowCredentials(true)}},
+			{"explicit wildcard", []CorsOpt{CorsAllowedOrigins("*"), CorsAllowCredentials(true)}},
+			{"wildcard among others", []CorsOpt{CorsAllowedOrigins("https://app.example.com", "*"), CorsAllowCredentials(true)}},
+		}
+		for _, tt := range tbl {
+			t.Run(tt.name, func(t *testing.T) {
+				assert.PanicsWithValue(t,
+					`rest: CORS with credentials can't allow "*" as an origin, list the allowed origins explicitly `+
+						`or opt in with CorsUnsafeAnyOriginWithCredentials`,
+					func() { CORS(tt.opts...) })
+			})
+		}
+	})
+
+	t.Run("wildcard without credentials allowed", func(t *testing.T) {
 		req := httptest.NewRequest("GET", "/test", http.NoBody)
 		req.Header.Set("Origin", "https://any.example.com")
 		w := httptest.NewRecorder()
 
-		CORS(CorsAllowCredentials(true))(handler).ServeHTTP(w, req)
+		CORS()(handler).ServeHTTP(w, req)
 		resp := w.Result()
 		defer resp.Body.Close()
 
-		// with credentials, must reflect origin, not "*"
-		assert.Equal(t, "https://any.example.com", resp.Header.Get("Access-Control-Allow-Origin"))
-		assert.Equal(t, "true", resp.Header.Get("Access-Control-Allow-Credentials"))
+		assert.Equal(t, "*", resp.Header.Get("Access-Control-Allow-Origin"))
+		assert.Empty(t, resp.Header.Get("Access-Control-Allow-Credentials"))
 	})
 }
 
@@ -346,3 +364,47 @@ func TestCORS_Integration(t *testing.T) {
 		assert.Contains(t, resp.Header.Get("Access-Control-Expose-Headers"), "X-Request-Id")
 	})
 }
+
+func TestCORS_UnsafeAnyOriginWithCredentials(t *testing.T) {
+	handler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+
+	t.Run("opting in keeps the wildcard working", func(t *testing.T) {
+		tbl := []struct {
+			name string
+			opts []CorsOpt
+		}{
+			{"default origins", []CorsOpt{
+				CorsAllowCredentials(true), CorsUnsafeAnyOriginWithCredentials(true)}},
+			{"explicit wildcard", []CorsOpt{
+				CorsAllowedOrigins("*"), CorsAllowCredentials(true), CorsUnsafeAnyOriginWithCredentials(true)}},
+		}
+		for _, tt := range tbl {
+			t.Run(tt.name, func(t *testing.T) {
+				req := httptest.NewRequest("GET", "/test", http.NoBody)
+				req.Header.Set("Origin", "https://any.example.com")
+				w := httptest.NewRecorder()
+
+				require.NotPanics(t, func() { CORS(tt.opts...)(handler).ServeHTTP(w, req) })
+				resp := w.Result()
+				defer resp.Body.Close()
+
+				// the origin is reflected, not "*", which is what credentials require
+				assert.Equal(t, "https://any.example.com", resp.Header.Get("Access-Control-Allow-Origin"))
+				assert.Equal(t, "true", resp.Header.Get("Access-Control-Allow-Credentials"))
+			})
+		}
+	})
+
+	t.Run("opting in without credentials changes nothing", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/test", http.NoBody)
+		req.Header.Set("Origin", "https://any.example.com")
+		w := httptest.NewRecorder()
+
+		CORS(CorsUnsafeAnyOriginWithCredentials(true))(handler).ServeHTTP(w, req)
+		resp := w.Result()
+		defer resp.Body.Close()
+
+		assert.Equal(t, "*", resp.Header.Get("Access-Control-Allow-Origin"))
+		assert.Empty(t, resp.Header.Get("Access-Control-Allow-Credentials"))
+	})
+}