diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index 628fa9551d..1a3b24b410 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -115,6 +115,7 @@ var baseS3ActionMap = map[string]string{ "DeleteBucketTagging": s3_constants.ACTION_TAGGING, // ACL operations "PutObjectAcl": s3_constants.ACTION_WRITE_ACP, + "PutObjectVersionAcl": s3_constants.ACTION_WRITE_ACP, "GetObjectAcl": s3_constants.ACTION_READ_ACP, "GetObjectVersionAcl": s3_constants.ACTION_READ_ACP, "PutBucketAcl": s3_constants.ACTION_WRITE_ACP, diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go index 7c228ba2ab..bc90c17ad1 100644 --- a/weed/s3api/s3_action_resolver.go +++ b/weed/s3api/s3_action_resolver.go @@ -166,15 +166,23 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool) } } - // ACL operations + // ACL operations. A versionId retargets the request at a specific + // version, so the version ACL action applies, not the current-object one. if query.Has("acl") { + versioned := hasObject && query.Get("versionId") != "" switch method { case http.MethodGet, http.MethodHead: + if versioned { + return s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL + } if hasObject { return s3_constants.S3_ACTION_GET_OBJECT_ACL } return s3_constants.S3_ACTION_GET_BUCKET_ACL case http.MethodPut: + if versioned { + return s3_constants.S3_ACTION_PUT_OBJECT_VERSION_ACL + } if hasObject { return s3_constants.S3_ACTION_PUT_OBJECT_ACL } diff --git a/weed/s3api/s3_action_resolver_test.go b/weed/s3api/s3_action_resolver_test.go index e46c1c8b6a..70dc53166e 100644 --- a/weed/s3api/s3_action_resolver_test.go +++ b/weed/s3api/s3_action_resolver_test.go @@ -146,6 +146,35 @@ func TestResolveS3Action_VersionTagging(t *testing.T) { } } +// An ACL request carrying versionId targets a specific version and must +// resolve to the version ACL action so a grant limited to the current-object +// action cannot reach historical versions. +func TestResolveS3Action_VersionAcl(t *testing.T) { + tests := []struct { + name string + method string + object string + query string + want string + }{ + {"get current acl", http.MethodGet, "key", "acl", s3_constants.S3_ACTION_GET_OBJECT_ACL}, + {"put current acl", http.MethodPut, "key", "acl", s3_constants.S3_ACTION_PUT_OBJECT_ACL}, + {"get version acl", http.MethodGet, "key", "acl&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL}, + {"head version acl", http.MethodHead, "key", "acl&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL}, + {"put version acl", http.MethodPut, "key", "acl&versionId=v1", s3_constants.S3_ACTION_PUT_OBJECT_VERSION_ACL}, + {"empty versionId stays current", http.MethodGet, "key", "acl&versionId=", s3_constants.S3_ACTION_GET_OBJECT_ACL}, + {"bucket acl ignores versionId", http.MethodGet, "", "acl&versionId=v1", s3_constants.S3_ACTION_GET_BUCKET_ACL}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil) + if got := ResolveS3Action(r, s3_constants.ACTION_READ, "bucket", tt.object); got != tt.want { + t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want) + } + }) + } +} + // Bucket subresources registered with ACTION_ADMIN must resolve to their own // S3 actions so a policy granting one of them does not need s3:*, and so no // broader grant sweeps them in. diff --git a/weed/s3api/s3_constants/s3_action_strings.go b/weed/s3api/s3_constants/s3_action_strings.go index 059c6e1dd8..8230516c58 100644 --- a/weed/s3api/s3_constants/s3_action_strings.go +++ b/weed/s3api/s3_constants/s3_action_strings.go @@ -15,6 +15,10 @@ const ( S3_ACTION_GET_OBJECT_ACL = "s3:GetObjectAcl" S3_ACTION_PUT_OBJECT_ACL = "s3:PutObjectAcl" + // Version-specific ACL operations (?acl&versionId) + S3_ACTION_GET_OBJECT_VERSION_ACL = "s3:GetObjectVersionAcl" + S3_ACTION_PUT_OBJECT_VERSION_ACL = "s3:PutObjectVersionAcl" + // Object tagging operations S3_ACTION_GET_OBJECT_TAGGING = "s3:GetObjectTagging" S3_ACTION_PUT_OBJECT_TAGGING = "s3:PutObjectTagging"