mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-21 22:12:36 +00:00
s3: resolve versionId ACL requests to the version ACL actions
A request for an object's ACL carrying versionId acted on the historical version but authorized against the current-object action (s3:GetObjectAcl/s3:PutObjectAcl), because the acl block in resolveFromQueryParameters returned before versionId was considered - the same gap closed for tagging. An identity granted only current-object ACL access - or explicitly denied s3:*ObjectVersionAcl - could still read and modify ACLs on noncurrent versions. ?acl&versionId=<id> on an object now resolves to s3:GetObjectVersionAcl/s3:PutObjectVersionAcl; ?acl alone keeps the current-object actions and bucket ACL is unaffected. s3:PutObjectVersionAcl is mapped onto the WriteAcp identity action so policies can name it.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user