diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index ad3cd1f0f2..628fa9551d 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -104,13 +104,15 @@ var baseS3ActionMap = map[string]string{ "DeleteObjectVersion": s3_constants.ACTION_WRITE, "GetObjectVersion": s3_constants.ACTION_READ, // Tagging operations - "GetObjectTagging": s3_constants.ACTION_TAGGING, - "GetObjectVersionTagging": s3_constants.ACTION_TAGGING, - "PutObjectTagging": s3_constants.ACTION_TAGGING, - "DeleteObjectTagging": s3_constants.ACTION_TAGGING, - "GetBucketTagging": s3_constants.ACTION_TAGGING, - "PutBucketTagging": s3_constants.ACTION_TAGGING, - "DeleteBucketTagging": s3_constants.ACTION_TAGGING, + "GetObjectTagging": s3_constants.ACTION_TAGGING, + "GetObjectVersionTagging": s3_constants.ACTION_TAGGING, + "PutObjectTagging": s3_constants.ACTION_TAGGING, + "PutObjectVersionTagging": s3_constants.ACTION_TAGGING, + "DeleteObjectTagging": s3_constants.ACTION_TAGGING, + "DeleteObjectVersionTagging": s3_constants.ACTION_TAGGING, + "GetBucketTagging": s3_constants.ACTION_TAGGING, + "PutBucketTagging": s3_constants.ACTION_TAGGING, + "DeleteBucketTagging": s3_constants.ACTION_TAGGING, // ACL operations "PutObjectAcl": s3_constants.ACTION_WRITE_ACP, "GetObjectAcl": s3_constants.ACTION_READ_ACP, diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go index 04dd8769a0..7c228ba2ab 100644 --- a/weed/s3api/s3_action_resolver.go +++ b/weed/s3api/s3_action_resolver.go @@ -182,20 +182,33 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool) } } - // Tagging operations + // Tagging operations. The routes match ?tagging whenever the key is + // present (gorilla/mux does not compare the value), so any value counts. + // A versionId retargets the request at a specific version, so the + // version-tagging action applies, not the current-object one. if query.Has("tagging") { + versioned := hasObject && query.Get("versionId") != "" switch method { case http.MethodGet: + if versioned { + return s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING + } if hasObject { return s3_constants.S3_ACTION_GET_OBJECT_TAGGING } return s3_constants.S3_ACTION_GET_BUCKET_TAGGING case http.MethodPut: + if versioned { + return s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING + } if hasObject { return s3_constants.S3_ACTION_PUT_OBJECT_TAGGING } return s3_constants.S3_ACTION_PUT_BUCKET_TAGGING case http.MethodDelete: + if versioned { + return s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING + } if hasObject { return s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING } diff --git a/weed/s3api/s3_action_resolver_test.go b/weed/s3api/s3_action_resolver_test.go index f11f372166..e46c1c8b6a 100644 --- a/weed/s3api/s3_action_resolver_test.go +++ b/weed/s3api/s3_action_resolver_test.go @@ -114,6 +114,38 @@ func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) { } } +// A tagging request carrying versionId targets a specific version and must +// resolve to the version-tagging action so a grant limited to current-object +// tagging cannot reach historical versions. +func TestResolveS3Action_VersionTagging(t *testing.T) { + tests := []struct { + name string + method string + object string + query string + want string + }{ + {"get current tags", http.MethodGet, "key", "tagging", s3_constants.S3_ACTION_GET_OBJECT_TAGGING}, + {"put current tags", http.MethodPut, "key", "tagging", s3_constants.S3_ACTION_PUT_OBJECT_TAGGING}, + {"delete current tags", http.MethodDelete, "key", "tagging", s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING}, + {"get version tags", http.MethodGet, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING}, + {"put version tags", http.MethodPut, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING}, + {"delete version tags", http.MethodDelete, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING}, + {"empty versionId stays current", http.MethodGet, "key", "tagging&versionId=", s3_constants.S3_ACTION_GET_OBJECT_TAGGING}, + {"bucket tagging ignores versionId", http.MethodGet, "", "tagging&versionId=v1", s3_constants.S3_ACTION_GET_BUCKET_TAGGING}, + {"non-empty tagging is still tagging", http.MethodGet, "key", "tagging=x&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING}, + {"non-empty tagging plain", http.MethodGet, "key", "tagging=x", s3_constants.S3_ACTION_GET_OBJECT_TAGGING}, + } + 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 af14ba5e95..059c6e1dd8 100644 --- a/weed/s3api/s3_constants/s3_action_strings.go +++ b/weed/s3api/s3_constants/s3_action_strings.go @@ -20,6 +20,11 @@ const ( S3_ACTION_PUT_OBJECT_TAGGING = "s3:PutObjectTagging" S3_ACTION_DELETE_OBJECT_TAGGING = "s3:DeleteObjectTagging" + // Version-specific tagging operations (?tagging&versionId) + S3_ACTION_GET_OBJECT_VERSION_TAGGING = "s3:GetObjectVersionTagging" + S3_ACTION_PUT_OBJECT_VERSION_TAGGING = "s3:PutObjectVersionTagging" + S3_ACTION_DELETE_OBJECT_VERSION_TAGGING = "s3:DeleteObjectVersionTagging" + // Object retention and legal hold S3_ACTION_GET_OBJECT_RETENTION = "s3:GetObjectRetention" S3_ACTION_PUT_OBJECT_RETENTION = "s3:PutObjectRetention"