aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChris Lu <chrislusf@users.noreply.github.com>2025-10-29 13:43:27 -0700
committerGitHub <noreply@github.com>2025-10-29 13:43:27 -0700
commiteaecd8328b6e76068381d86098647decda11d8c1 (patch)
tree425a37699326bbd83a3621dcb04908bb18719717
parentd4790cb8e60d61c7b5c7e19304356b30f7b1bdc4 (diff)
downloadseaweedfs-eaecd8328b6e76068381d86098647decda11d8c1.tar.xz
seaweedfs-eaecd8328b6e76068381d86098647decda11d8c1.zip
S3: add fallback for CORS (#7404)
* add fallback for cors * refactor * expose aws headers * add fallback to test * refactor * Only falls back to global config when there's explicitly no bucket-level config. * fmt * Update s3_cors_http_test.go * refactoring
-rw-r--r--test/s3/cors/s3_cors_http_test.go15
-rw-r--r--weed/s3api/cors/middleware.go51
-rw-r--r--weed/s3api/cors/middleware_test.go405
-rw-r--r--weed/s3api/s3api_bucket_cors_handlers.go41
4 files changed, 494 insertions, 18 deletions
diff --git a/test/s3/cors/s3_cors_http_test.go b/test/s3/cors/s3_cors_http_test.go
index 872831a2a..8244e2f03 100644
--- a/test/s3/cors/s3_cors_http_test.go
+++ b/test/s3/cors/s3_cors_http_test.go
@@ -398,13 +398,15 @@ func TestCORSHeaderMatching(t *testing.T) {
}
}
-// TestCORSWithoutConfiguration tests CORS behavior when no configuration is set
+// TestCORSWithoutConfiguration tests CORS behavior when no bucket-level configuration is set
+// With the fallback feature, buckets without explicit CORS config will use the global CORS settings
func TestCORSWithoutConfiguration(t *testing.T) {
client := getS3Client(t)
bucketName := createTestBucket(t, client)
defer cleanupTestBucket(t, client, bucketName)
- // Test preflight request without CORS configuration
+ // Test preflight request without bucket-level CORS configuration
+ // The global CORS fallback (default: "*") should be used
httpClient := &http.Client{Timeout: 10 * time.Second}
req, err := http.NewRequest("OPTIONS", fmt.Sprintf("%s/%s/test-object", getDefaultConfig().Endpoint, bucketName), nil)
@@ -412,15 +414,16 @@ func TestCORSWithoutConfiguration(t *testing.T) {
req.Header.Set("Origin", "https://example.com")
req.Header.Set("Access-Control-Request-Method", "GET")
+ req.Header.Set("Access-Control-Request-Headers", "Content-Type")
resp, err := httpClient.Do(req)
require.NoError(t, err, "Should be able to send OPTIONS request")
defer resp.Body.Close()
- // Without CORS configuration, CORS headers should not be present
- assert.Empty(t, resp.Header.Get("Access-Control-Allow-Origin"), "Should not have Allow-Origin header without CORS config")
- assert.Empty(t, resp.Header.Get("Access-Control-Allow-Methods"), "Should not have Allow-Methods header without CORS config")
- assert.Empty(t, resp.Header.Get("Access-Control-Allow-Headers"), "Should not have Allow-Headers header without CORS config")
+ // With fallback CORS (global default: "*"), CORS headers should be present
+ assert.Equal(t, "https://example.com", resp.Header.Get("Access-Control-Allow-Origin"), "Should have Allow-Origin header from global fallback")
+ assert.Contains(t, resp.Header.Get("Access-Control-Allow-Methods"), "GET", "Should have GET in Allow-Methods from global fallback")
+ assert.Contains(t, resp.Header.Get("Access-Control-Allow-Headers"), "Content-Type", "Should have requested headers in Allow-Headers from global fallback")
}
// TestCORSMethodMatching tests method matching
diff --git a/weed/s3api/cors/middleware.go b/weed/s3api/cors/middleware.go
index c9cd0e19e..7aa40e84f 100644
--- a/weed/s3api/cors/middleware.go
+++ b/weed/s3api/cors/middleware.go
@@ -22,16 +22,47 @@ type CORSConfigGetter interface {
type Middleware struct {
bucketChecker BucketChecker
corsConfigGetter CORSConfigGetter
+ fallbackConfig *CORSConfiguration // Global CORS configuration as fallback
}
-// NewMiddleware creates a new CORS middleware instance
-func NewMiddleware(bucketChecker BucketChecker, corsConfigGetter CORSConfigGetter) *Middleware {
+// NewMiddleware creates a new CORS middleware instance with optional global fallback config
+func NewMiddleware(bucketChecker BucketChecker, corsConfigGetter CORSConfigGetter, fallbackConfig *CORSConfiguration) *Middleware {
return &Middleware{
bucketChecker: bucketChecker,
corsConfigGetter: corsConfigGetter,
+ fallbackConfig: fallbackConfig,
}
}
+// getCORSConfig retrieves the applicable CORS configuration, trying bucket-specific first, then fallback.
+// Returns the configuration and a boolean indicating if any configuration was found.
+// Only falls back to global config when there's explicitly no bucket-level config.
+// For other errors (e.g., access denied), returns false to let the handler deny the request.
+func (m *Middleware) getCORSConfig(bucket string) (*CORSConfiguration, bool) {
+ config, errCode := m.corsConfigGetter.GetCORSConfiguration(bucket)
+
+ switch errCode {
+ case s3err.ErrNone:
+ if config != nil {
+ // Found a bucket-specific config, use it.
+ return config, true
+ }
+ // No bucket config, proceed to fallback.
+ case s3err.ErrNoSuchCORSConfiguration:
+ // No bucket config, proceed to fallback.
+ default:
+ // Any other error means we should not proceed.
+ return nil, false
+ }
+
+ // No bucket-level config found, try global fallback
+ if m.fallbackConfig != nil {
+ return m.fallbackConfig, true
+ }
+
+ return nil, false
+}
+
// Handler returns the CORS middleware handler
func (m *Middleware) Handler(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -58,10 +89,10 @@ func (m *Middleware) Handler(next http.Handler) http.Handler {
return
}
- // Load CORS configuration from cache
- config, errCode := m.corsConfigGetter.GetCORSConfiguration(bucket)
- if errCode != s3err.ErrNone || config == nil {
- // No CORS configuration, handle based on request type
+ // Get CORS configuration (bucket-specific or fallback)
+ config, found := m.getCORSConfig(bucket)
+ if !found {
+ // No CORS configuration at all, handle based on request type
if corsReq.IsPreflightRequest {
// Preflight request without CORS config should fail
s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
@@ -126,10 +157,10 @@ func (m *Middleware) HandleOptionsRequest(w http.ResponseWriter, r *http.Request
return
}
- // Load CORS configuration from cache
- config, errCode := m.corsConfigGetter.GetCORSConfiguration(bucket)
- if errCode != s3err.ErrNone || config == nil {
- // No CORS configuration for OPTIONS request should return access denied
+ // Get CORS configuration (bucket-specific or fallback)
+ config, found := m.getCORSConfig(bucket)
+ if !found {
+ // No CORS configuration at all for OPTIONS request should return access denied
if corsReq.IsPreflightRequest {
s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
return
diff --git a/weed/s3api/cors/middleware_test.go b/weed/s3api/cors/middleware_test.go
new file mode 100644
index 000000000..e9f89a038
--- /dev/null
+++ b/weed/s3api/cors/middleware_test.go
@@ -0,0 +1,405 @@
+package cors
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/gorilla/mux"
+ "github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+)
+
+// Mock implementations for testing
+
+type mockBucketChecker struct {
+ bucketExists bool
+}
+
+func (m *mockBucketChecker) CheckBucket(r *http.Request, bucket string) s3err.ErrorCode {
+ if m.bucketExists {
+ return s3err.ErrNone
+ }
+ return s3err.ErrNoSuchBucket
+}
+
+type mockCORSConfigGetter struct {
+ config *CORSConfiguration
+ errCode s3err.ErrorCode
+}
+
+func (m *mockCORSConfigGetter) GetCORSConfiguration(bucket string) (*CORSConfiguration, s3err.ErrorCode) {
+ return m.config, m.errCode
+}
+
+// TestMiddlewareFallbackConfig tests that the middleware uses fallback config when bucket-level config is not available
+func TestMiddlewareFallbackConfig(t *testing.T) {
+ tests := []struct {
+ name string
+ bucketConfig *CORSConfiguration
+ fallbackConfig *CORSConfiguration
+ requestOrigin string
+ requestMethod string
+ isOptions bool
+ expectedStatus int
+ expectedOriginHeader string
+ description string
+ }{
+ {
+ name: "No bucket config, fallback to global config with wildcard",
+ bucketConfig: nil,
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"*"},
+ AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "HEAD"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://example.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "https://example.com",
+ description: "Should use fallback global config when no bucket config exists",
+ },
+ {
+ name: "No bucket config, fallback to global config with specific origin",
+ bucketConfig: nil,
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://example.com"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://example.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "https://example.com",
+ description: "Should use fallback config with specific origin match",
+ },
+ {
+ name: "No bucket config, fallback rejects non-matching origin",
+ bucketConfig: nil,
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://allowed.com"},
+ AllowedMethods: []string{"GET"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://notallowed.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "",
+ description: "Should not apply CORS headers when origin doesn't match fallback config",
+ },
+ {
+ name: "Bucket config takes precedence over fallback",
+ bucketConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://bucket-specific.com"},
+ AllowedMethods: []string{"GET"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"*"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://bucket-specific.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "https://bucket-specific.com",
+ description: "Bucket-level config should be used instead of fallback",
+ },
+ {
+ name: "Bucket config rejects, even though fallback would allow",
+ bucketConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://restricted.com"},
+ AllowedMethods: []string{"GET"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"*"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://example.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "",
+ description: "Bucket-level config is authoritative, fallback should not apply",
+ },
+ {
+ name: "No config at all, no CORS headers",
+ bucketConfig: nil,
+ fallbackConfig: nil,
+ requestOrigin: "https://example.com",
+ requestMethod: "GET",
+ isOptions: false,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "",
+ description: "Without any config, no CORS headers should be applied",
+ },
+ {
+ name: "OPTIONS preflight with fallback config",
+ bucketConfig: nil,
+ fallbackConfig: &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://example.com"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ },
+ requestOrigin: "https://example.com",
+ requestMethod: "OPTIONS",
+ isOptions: true,
+ expectedStatus: http.StatusOK,
+ expectedOriginHeader: "https://example.com",
+ description: "OPTIONS preflight should work with fallback config",
+ },
+ {
+ name: "OPTIONS preflight without any config should fail",
+ bucketConfig: nil,
+ fallbackConfig: nil,
+ requestOrigin: "https://example.com",
+ requestMethod: "OPTIONS",
+ isOptions: true,
+ expectedStatus: http.StatusForbidden,
+ expectedOriginHeader: "",
+ description: "OPTIONS preflight without config should return 403",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Setup mocks
+ bucketChecker := &mockBucketChecker{bucketExists: true}
+ configGetter := &mockCORSConfigGetter{
+ config: tt.bucketConfig,
+ errCode: s3err.ErrNone,
+ }
+
+ // Create middleware with optional fallback
+ middleware := NewMiddleware(bucketChecker, configGetter, tt.fallbackConfig)
+
+ // Create request with mux variables
+ req := httptest.NewRequest(tt.requestMethod, "/testbucket/testobject", nil)
+ req = mux.SetURLVars(req, map[string]string{
+ "bucket": "testbucket",
+ "object": "testobject",
+ })
+ if tt.requestOrigin != "" {
+ req.Header.Set("Origin", tt.requestOrigin)
+ }
+ if tt.isOptions {
+ req.Header.Set("Access-Control-Request-Method", "GET")
+ }
+
+ // Create response recorder
+ w := httptest.NewRecorder()
+
+ // Create a simple handler that returns 200 OK
+ nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusOK)
+ })
+
+ // Execute middleware
+ if tt.isOptions {
+ middleware.HandleOptionsRequest(w, req)
+ } else {
+ middleware.Handler(nextHandler).ServeHTTP(w, req)
+ }
+
+ // Check status code
+ if w.Code != tt.expectedStatus {
+ t.Errorf("%s: expected status %d, got %d", tt.description, tt.expectedStatus, w.Code)
+ }
+
+ // Check CORS header
+ actualOrigin := w.Header().Get("Access-Control-Allow-Origin")
+ if actualOrigin != tt.expectedOriginHeader {
+ t.Errorf("%s: expected Access-Control-Allow-Origin='%s', got '%s'",
+ tt.description, tt.expectedOriginHeader, actualOrigin)
+ }
+ })
+ }
+}
+
+// TestMiddlewareFallbackConfigWithMultipleOrigins tests fallback with multiple allowed origins
+func TestMiddlewareFallbackConfigWithMultipleOrigins(t *testing.T) {
+ fallbackConfig := &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"https://example1.com", "https://example2.com"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ }
+
+ bucketChecker := &mockBucketChecker{bucketExists: true}
+ configGetter := &mockCORSConfigGetter{
+ config: nil, // No bucket config
+ errCode: s3err.ErrNone,
+ }
+
+ middleware := NewMiddleware(bucketChecker, configGetter, fallbackConfig)
+
+ tests := []struct {
+ origin string
+ shouldMatch bool
+ description string
+ }{
+ {
+ origin: "https://example1.com",
+ shouldMatch: true,
+ description: "First allowed origin should match",
+ },
+ {
+ origin: "https://example2.com",
+ shouldMatch: true,
+ description: "Second allowed origin should match",
+ },
+ {
+ origin: "https://example3.com",
+ shouldMatch: false,
+ description: "Non-allowed origin should not match",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.description, func(t *testing.T) {
+ req := httptest.NewRequest("GET", "/testbucket/testobject", nil)
+ req = mux.SetURLVars(req, map[string]string{
+ "bucket": "testbucket",
+ "object": "testobject",
+ })
+ req.Header.Set("Origin", tt.origin)
+
+ w := httptest.NewRecorder()
+ nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusOK)
+ })
+
+ middleware.Handler(nextHandler).ServeHTTP(w, req)
+
+ actualOrigin := w.Header().Get("Access-Control-Allow-Origin")
+ if tt.shouldMatch {
+ if actualOrigin != tt.origin {
+ t.Errorf("%s: expected Access-Control-Allow-Origin='%s', got '%s'",
+ tt.description, tt.origin, actualOrigin)
+ }
+ } else {
+ if actualOrigin != "" {
+ t.Errorf("%s: expected no Access-Control-Allow-Origin header, got '%s'",
+ tt.description, actualOrigin)
+ }
+ }
+ })
+ }
+}
+
+// TestMiddlewareFallbackWithError tests that real errors (not "no config") don't trigger fallback
+func TestMiddlewareFallbackWithError(t *testing.T) {
+ fallbackConfig := &CORSConfiguration{
+ CORSRules: []CORSRule{
+ {
+ AllowedOrigins: []string{"*"},
+ AllowedMethods: []string{"GET", "POST"},
+ AllowedHeaders: []string{"*"},
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ errCode s3err.ErrorCode
+ expectedOriginHeader string
+ description string
+ }{
+ {
+ name: "ErrAccessDenied should not trigger fallback",
+ errCode: s3err.ErrAccessDenied,
+ expectedOriginHeader: "",
+ description: "Access denied errors should not expose CORS headers",
+ },
+ {
+ name: "ErrInternalError should not trigger fallback",
+ errCode: s3err.ErrInternalError,
+ expectedOriginHeader: "",
+ description: "Internal errors should not expose CORS headers",
+ },
+ {
+ name: "ErrNoSuchBucket should not trigger fallback",
+ errCode: s3err.ErrNoSuchBucket,
+ expectedOriginHeader: "",
+ description: "Bucket not found errors should not expose CORS headers",
+ },
+ {
+ name: "ErrNoSuchCORSConfiguration should trigger fallback",
+ errCode: s3err.ErrNoSuchCORSConfiguration,
+ expectedOriginHeader: "https://example.com",
+ description: "Explicit no CORS config should use fallback",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ bucketChecker := &mockBucketChecker{bucketExists: true}
+ configGetter := &mockCORSConfigGetter{
+ config: nil,
+ errCode: tt.errCode,
+ }
+
+ middleware := NewMiddleware(bucketChecker, configGetter, fallbackConfig)
+
+ req := httptest.NewRequest("GET", "/testbucket/testobject", nil)
+ req = mux.SetURLVars(req, map[string]string{
+ "bucket": "testbucket",
+ "object": "testobject",
+ })
+ req.Header.Set("Origin", "https://example.com")
+
+ w := httptest.NewRecorder()
+ nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusOK)
+ })
+
+ middleware.Handler(nextHandler).ServeHTTP(w, req)
+
+ actualOrigin := w.Header().Get("Access-Control-Allow-Origin")
+ if actualOrigin != tt.expectedOriginHeader {
+ t.Errorf("%s: expected Access-Control-Allow-Origin='%s', got '%s'",
+ tt.description, tt.expectedOriginHeader, actualOrigin)
+ }
+ })
+ }
+}
diff --git a/weed/s3api/s3api_bucket_cors_handlers.go b/weed/s3api/s3api_bucket_cors_handlers.go
index bd27785e2..c45f86014 100644
--- a/weed/s3api/s3api_bucket_cors_handlers.go
+++ b/weed/s3api/s3api_bucket_cors_handlers.go
@@ -10,6 +10,19 @@ import (
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
)
+// Default CORS configuration for global fallback
+var (
+ defaultFallbackAllowedMethods = []string{"GET", "PUT", "POST", "DELETE", "HEAD"}
+ defaultFallbackExposeHeaders = []string{
+ "ETag",
+ "Content-Length",
+ "Content-Type",
+ "Last-Modified",
+ "x-amz-request-id",
+ "x-amz-version-id",
+ }
+)
+
// S3BucketChecker implements cors.BucketChecker interface
type S3BucketChecker struct {
server *S3ApiServer
@@ -28,12 +41,36 @@ func (g *S3CORSConfigGetter) GetCORSConfiguration(bucket string) (*cors.CORSConf
return g.server.getCORSConfiguration(bucket)
}
-// getCORSMiddleware returns a CORS middleware instance with caching
+// getCORSMiddleware returns a CORS middleware instance with global fallback config
func (s3a *S3ApiServer) getCORSMiddleware() *cors.Middleware {
bucketChecker := &S3BucketChecker{server: s3a}
corsConfigGetter := &S3CORSConfigGetter{server: s3a}
- return cors.NewMiddleware(bucketChecker, corsConfigGetter)
+ // Create fallback CORS configuration from global AllowedOrigins setting
+ fallbackConfig := s3a.createFallbackCORSConfig()
+
+ return cors.NewMiddleware(bucketChecker, corsConfigGetter, fallbackConfig)
+}
+
+// createFallbackCORSConfig creates a CORS configuration from global AllowedOrigins
+func (s3a *S3ApiServer) createFallbackCORSConfig() *cors.CORSConfiguration {
+ if len(s3a.option.AllowedOrigins) == 0 {
+ return nil
+ }
+
+ // Create a permissive CORS rule based on global allowed origins
+ // This matches the behavior of handleCORSOriginValidation
+ rule := cors.CORSRule{
+ AllowedOrigins: s3a.option.AllowedOrigins,
+ AllowedMethods: defaultFallbackAllowedMethods,
+ AllowedHeaders: []string{"*"},
+ ExposeHeaders: defaultFallbackExposeHeaders,
+ MaxAgeSeconds: nil, // No max age by default
+ }
+
+ return &cors.CORSConfiguration{
+ CORSRules: []cors.CORSRule{rule},
+ }
}
// GetBucketCorsHandler handles Get bucket CORS configuration