aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--weed/s3api/s3_bucket_policy_simple_test.go177
-rw-r--r--weed/s3api/s3api_bucket_policy_handlers.go44
2 files changed, 204 insertions, 17 deletions
diff --git a/weed/s3api/s3_bucket_policy_simple_test.go b/weed/s3api/s3_bucket_policy_simple_test.go
index 025b44900..5188779ff 100644
--- a/weed/s3api/s3_bucket_policy_simple_test.go
+++ b/weed/s3api/s3_bucket_policy_simple_test.go
@@ -143,42 +143,106 @@ func TestBucketResourceValidation(t *testing.T) {
bucket string
valid bool
}{
+ // SeaweedFS ARN format
{
- name: "Exact bucket ARN",
+ name: "Exact bucket ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket",
bucket: "test-bucket",
valid: true,
},
{
- name: "Bucket wildcard ARN",
+ name: "Bucket wildcard ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket/*",
bucket: "test-bucket",
valid: true,
},
{
- name: "Specific object ARN",
+ name: "Specific object ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket/path/to/object.txt",
bucket: "test-bucket",
valid: true,
},
+ // AWS ARN format (compatibility)
{
- name: "Different bucket ARN",
+ name: "Exact bucket ARN (AWS)",
+ resource: "arn:aws:s3:::test-bucket",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ {
+ name: "Bucket wildcard ARN (AWS)",
+ resource: "arn:aws:s3:::test-bucket/*",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ {
+ name: "Specific object ARN (AWS)",
+ resource: "arn:aws:s3:::test-bucket/path/to/object.txt",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ // Simplified format (without ARN prefix)
+ {
+ name: "Simplified bucket name",
+ resource: "test-bucket",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ {
+ name: "Simplified bucket wildcard",
+ resource: "test-bucket/*",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ {
+ name: "Simplified specific object",
+ resource: "test-bucket/path/to/object.txt",
+ bucket: "test-bucket",
+ valid: true,
+ },
+ // Invalid cases
+ {
+ name: "Different bucket ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::other-bucket/*",
bucket: "test-bucket",
valid: false,
},
{
- name: "Global S3 wildcard",
+ name: "Different bucket ARN (AWS)",
+ resource: "arn:aws:s3:::other-bucket/*",
+ bucket: "test-bucket",
+ valid: false,
+ },
+ {
+ name: "Different bucket simplified",
+ resource: "other-bucket/*",
+ bucket: "test-bucket",
+ valid: false,
+ },
+ {
+ name: "Global S3 wildcard (SeaweedFS)",
resource: "arn:seaweed:s3:::*",
bucket: "test-bucket",
valid: false,
},
{
+ name: "Global S3 wildcard (AWS)",
+ resource: "arn:aws:s3:::*",
+ bucket: "test-bucket",
+ valid: false,
+ },
+ {
name: "Invalid ARN format",
resource: "invalid-arn",
bucket: "test-bucket",
valid: false,
},
+ {
+ name: "Bucket name prefix match but different bucket",
+ resource: "test-bucket-different/*",
+ bucket: "test-bucket",
+ valid: false,
+ },
}
for _, tt := range tests {
@@ -226,3 +290,106 @@ func marshalPolicy(t *testing.T, policyDoc *policy.PolicyDocument) []byte {
require.NoError(t, err)
return data
}
+
+// TestIssue7252Examples tests the specific examples from GitHub issue #7252
+func TestIssue7252Examples(t *testing.T) {
+ s3Server := &S3ApiServer{}
+
+ tests := []struct {
+ name string
+ policy *policy.PolicyDocument
+ bucket string
+ expectedValid bool
+ description string
+ }{
+ {
+ name: "Issue #7252 - Standard ARN with wildcard",
+ policy: &policy.PolicyDocument{
+ Version: "2012-10-17",
+ Statement: []policy.Statement{
+ {
+ Effect: "Allow",
+ Principal: map[string]interface{}{
+ "AWS": "*",
+ },
+ Action: []string{"s3:GetObject"},
+ Resource: []string{"arn:aws:s3:::main-bucket/*"},
+ },
+ },
+ },
+ bucket: "main-bucket",
+ expectedValid: true,
+ description: "AWS ARN format should be accepted",
+ },
+ {
+ name: "Issue #7252 - Simplified resource with wildcard",
+ policy: &policy.PolicyDocument{
+ Version: "2012-10-17",
+ Statement: []policy.Statement{
+ {
+ Effect: "Allow",
+ Principal: map[string]interface{}{
+ "AWS": "*",
+ },
+ Action: []string{"s3:GetObject"},
+ Resource: []string{"main-bucket/*"},
+ },
+ },
+ },
+ bucket: "main-bucket",
+ expectedValid: true,
+ description: "Simplified format with wildcard should be accepted",
+ },
+ {
+ name: "Issue #7252 - Resource as exact bucket name",
+ policy: &policy.PolicyDocument{
+ Version: "2012-10-17",
+ Statement: []policy.Statement{
+ {
+ Effect: "Allow",
+ Principal: map[string]interface{}{
+ "AWS": "*",
+ },
+ Action: []string{"s3:GetObject"},
+ Resource: []string{"main-bucket"},
+ },
+ },
+ },
+ bucket: "main-bucket",
+ expectedValid: true,
+ description: "Exact bucket name should be accepted",
+ },
+ {
+ name: "Public read policy with AWS ARN",
+ policy: &policy.PolicyDocument{
+ Version: "2012-10-17",
+ Statement: []policy.Statement{
+ {
+ Sid: "PublicReadGetObject",
+ Effect: "Allow",
+ Principal: map[string]interface{}{
+ "AWS": "*",
+ },
+ Action: []string{"s3:GetObject"},
+ Resource: []string{"arn:aws:s3:::my-public-bucket/*"},
+ },
+ },
+ },
+ bucket: "my-public-bucket",
+ expectedValid: true,
+ description: "Standard public read policy with AWS ARN should work",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ err := s3Server.validateBucketPolicy(tt.policy, tt.bucket)
+
+ if tt.expectedValid {
+ assert.NoError(t, err, "Policy should be valid: %s", tt.description)
+ } else {
+ assert.Error(t, err, "Policy should be invalid: %s", tt.description)
+ }
+ })
+ }
+}
diff --git a/weed/s3api/s3api_bucket_policy_handlers.go b/weed/s3api/s3api_bucket_policy_handlers.go
index e079eb53e..4a83f0da4 100644
--- a/weed/s3api/s3api_bucket_policy_handlers.go
+++ b/weed/s3api/s3api_bucket_policy_handlers.go
@@ -274,18 +274,38 @@ func (s3a *S3ApiServer) validateBucketPolicy(policyDoc *policy.PolicyDocument, b
// validateResourceForBucket checks if a resource ARN is valid for the given bucket
func (s3a *S3ApiServer) validateResourceForBucket(resource, bucket string) bool {
- // Expected formats:
- // arn:seaweed:s3:::bucket-name
- // arn:seaweed:s3:::bucket-name/*
- // arn:seaweed:s3:::bucket-name/path/to/object
-
- expectedBucketArn := fmt.Sprintf("arn:seaweed:s3:::%s", bucket)
- expectedBucketWildcard := fmt.Sprintf("arn:seaweed:s3:::%s/*", bucket)
- expectedBucketPath := fmt.Sprintf("arn:seaweed:s3:::%s/", bucket)
-
- return resource == expectedBucketArn ||
- resource == expectedBucketWildcard ||
- strings.HasPrefix(resource, expectedBucketPath)
+ // Accepted formats for S3 bucket policies:
+ // AWS-style ARNs:
+ // arn:aws:s3:::bucket-name
+ // arn:aws:s3:::bucket-name/*
+ // arn:aws:s3:::bucket-name/path/to/object
+ // SeaweedFS ARNs:
+ // arn:seaweed:s3:::bucket-name
+ // arn:seaweed:s3:::bucket-name/*
+ // arn:seaweed:s3:::bucket-name/path/to/object
+ // Simplified formats (for convenience):
+ // bucket-name
+ // bucket-name/*
+ // bucket-name/path/to/object
+
+ var resourcePath string
+ const awsPrefix = "arn:aws:s3:::"
+ const seaweedPrefix = "arn:seaweed:s3:::"
+
+ // Strip the optional ARN prefix to get the resource path
+ if path, ok := strings.CutPrefix(resource, awsPrefix); ok {
+ resourcePath = path
+ } else if path, ok := strings.CutPrefix(resource, seaweedPrefix); ok {
+ resourcePath = path
+ } else {
+ resourcePath = resource
+ }
+
+ // After stripping the optional ARN prefix, the resource path must
+ // either match the bucket name exactly, or be a path within the bucket.
+ return resourcePath == bucket ||
+ resourcePath == bucket+"/*" ||
+ strings.HasPrefix(resourcePath, bucket+"/")
}
// IAM integration functions
generated by cgit v1.2.3 (git 2.25.1) at 2025-12-20 22:38:05 +0000