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