mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-24 23:10:41 +02:00
s3: evaluate bucket CORS for path-style OPTIONS on the bucket root (#12028)
* s3: evaluate bucket CORS for path-style OPTIONS on the bucket root
A path-style preflight for /bucket (no trailing slash) missed the bucket
OPTIONS route, whose template is /{bucket}/, and was answered by the global
OPTIONS handler from the global AllowedOrigins instead of the bucket CORS
configuration. Register the bucket OPTIONS route without a path matcher, and
keep OPTIONS for the health probe paths on the service-level handler ahead of
each path-style router.
Fixes #12025
* test: drop the issue reference from the bucket-root preflight test comment
---------
Co-authored-by: Wleeaf <179125857+wleeaf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
2 files changed
+104
-5
No files matched your search
@@ -947,6 +947,17 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
|
||||
// Object path pattern with (?s) flag to match newlines in object keys
|
||||
const objectPath = "/{object:(?s).+}"
|
||||
|
||||
// CORS preflight for service-level requests, answered from the global
|
||||
// AllowedOrigins rather than a bucket's CORS configuration
|
||||
serviceOptionsHandler := func(w http.ResponseWriter, r *http.Request) {
|
||||
if s3a.handleCORSOriginValidation(w, r) {
|
||||
writeSuccessResponseEmpty(w, r)
|
||||
}
|
||||
}
|
||||
// A path-style bucket router would take the health probe paths as bucket
|
||||
// names, so their preflight is registered ahead of each one
|
||||
const healthProbePath = "/{probe:status|healthz|readyz}"
|
||||
|
||||
var routers []*mux.Router
|
||||
if s3a.option.DomainName != "" {
|
||||
domainNames := strings.Split(s3a.option.DomainName, ",")
|
||||
@@ -954,6 +965,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
|
||||
|
||||
// Register path-style domains
|
||||
for _, domain := range pathStyleDomains {
|
||||
apiRouter.Host(domain).Methods(http.MethodOptions).Path(healthProbePath).HandlerFunc(serviceOptionsHandler)
|
||||
routers = append(routers, apiRouter.Host(domain).PathPrefix("/{bucket}").Subrouter())
|
||||
apiRouter.Host(domain).
|
||||
Methods(http.MethodGet).
|
||||
@@ -971,6 +983,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
|
||||
// arrive via an IP, an unlisted hostname, or a reverse proxy that rewrites
|
||||
// the Host header still match bucket routes. Host-specific routers above
|
||||
// take precedence because they were registered first.
|
||||
apiRouter.Methods(http.MethodOptions).Path(healthProbePath).HandlerFunc(serviceOptionsHandler)
|
||||
routers = append(routers, apiRouter.PathPrefix("/{bucket}").Subrouter())
|
||||
|
||||
// Get CORS middleware instance with caching
|
||||
@@ -986,8 +999,9 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
|
||||
bucket.Use(corsMiddleware.Handler)
|
||||
|
||||
// Bucket-specific OPTIONS handler for CORS preflight requests
|
||||
// Use PathPrefix to catch all bucket-level preflight routes including /bucket/object
|
||||
bucket.PathPrefix("/").Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest)
|
||||
// No path matcher, so it also catches the path-style bucket root (/bucket),
|
||||
// which PathPrefix("/") would miss, as well as /bucket/ and /bucket/object
|
||||
bucket.Methods(http.MethodOptions).HandlerFunc(corsMiddleware.HandleOptionsRequest)
|
||||
|
||||
// each case should follow the next rule:
|
||||
// - requesting object with query must precede any other methods
|
||||
@@ -1222,9 +1236,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
|
||||
return
|
||||
}
|
||||
|
||||
if s3a.handleCORSOriginValidation(w, r) {
|
||||
writeSuccessResponseEmpty(w, r)
|
||||
}
|
||||
serviceOptionsHandler(w, r)
|
||||
})
|
||||
|
||||
// STS API endpoint for AssumeRoleWithWebIdentity
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
|
||||
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/cors"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -442,3 +443,89 @@ func TestRouting_BucketRouteMatchesAnyHost(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouting_BucketRootPreflightUsesBucketCORS is a regression test: a
|
||||
// path-style preflight for the bucket root without a trailing
|
||||
// slash must be evaluated against the bucket CORS rules like /bucket/, not
|
||||
// answered from the global AllowedOrigins. Service-level paths keep the
|
||||
// global answer, and virtual-host routing is unchanged.
|
||||
func TestRouting_BucketRootPreflightUsesBucketCORS(t *testing.T) {
|
||||
const (
|
||||
bucket = "cors-bucket"
|
||||
allowedOrigin = "https://app.example.com"
|
||||
foreignOrigin = "https://untrusted.example"
|
||||
)
|
||||
pathStyleBucketPaths := []string{"/" + bucket, "/" + bucket + "/", "/" + bucket + "?list-type=2", "/" + bucket + "/key"}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
domainName string
|
||||
host string
|
||||
bucketPaths []string // evaluated against the bucket CORS rules
|
||||
servicePaths []string // answered from the global AllowedOrigins
|
||||
}{
|
||||
{
|
||||
name: "path-style",
|
||||
host: "10.0.0.5:8333",
|
||||
bucketPaths: pathStyleBucketPaths,
|
||||
servicePaths: []string{"/", "/status", "/healthz", "/readyz"},
|
||||
},
|
||||
{
|
||||
name: "path-style domain",
|
||||
domainName: "s3.example.com,api.s3.example.com",
|
||||
host: "api.s3.example.com",
|
||||
bucketPaths: pathStyleBucketPaths,
|
||||
servicePaths: []string{"/status", "/healthz", "/readyz"},
|
||||
},
|
||||
{
|
||||
name: "virtual-host domain",
|
||||
domainName: "s3.example.com,api.s3.example.com",
|
||||
host: bucket + ".s3.example.com",
|
||||
bucketPaths: []string{"/", "/?list-type=2", "/key", "/status"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
s3a := setupRoutingTestServer(t)
|
||||
s3a.option.DomainName = tc.domainName
|
||||
s3a.option.AllowedOrigins = []string{"*"}
|
||||
s3a.bucketConfigCache = NewBucketConfigCache(time.Minute)
|
||||
s3a.bucketConfigCache.Set(bucket, &BucketConfig{
|
||||
Name: bucket,
|
||||
CORS: &cors.CORSConfiguration{CORSRules: []cors.CORSRule{{
|
||||
AllowedOrigins: []string{allowedOrigin},
|
||||
AllowedMethods: []string{http.MethodGet, http.MethodPut},
|
||||
AllowedHeaders: []string{"*"},
|
||||
}}},
|
||||
})
|
||||
router := mux.NewRouter()
|
||||
s3a.registerRouter(router)
|
||||
|
||||
preflight := func(path, origin string) *httptest.ResponseRecorder {
|
||||
req := httptest.NewRequest(http.MethodOptions, "http://"+tc.host+path, nil)
|
||||
req.Header.Set("Origin", origin)
|
||||
req.Header.Set("Access-Control-Request-Method", http.MethodPut)
|
||||
rr := httptest.NewRecorder()
|
||||
router.ServeHTTP(rr, req)
|
||||
return rr
|
||||
}
|
||||
|
||||
for _, path := range tc.bucketPaths {
|
||||
rr := preflight(path, foreignOrigin)
|
||||
assert.Equal(t, http.StatusForbidden, rr.Code, "foreign origin on %s", path)
|
||||
assert.Empty(t, rr.Header().Get("Access-Control-Allow-Origin"), "foreign origin on %s", path)
|
||||
|
||||
rr = preflight(path, allowedOrigin)
|
||||
assert.Equal(t, http.StatusOK, rr.Code, "allowed origin on %s", path)
|
||||
assert.Equal(t, allowedOrigin, rr.Header().Get("Access-Control-Allow-Origin"), "allowed origin on %s", path)
|
||||
}
|
||||
|
||||
for _, path := range tc.servicePaths {
|
||||
rr := preflight(path, foreignOrigin)
|
||||
assert.Equal(t, http.StatusOK, rr.Code, "service-level %s", path)
|
||||
assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin"), "service-level %s", path)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user