From 5c918f3682f5d64f19e8706047fc94a37f79d3b3 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Mon, 2 Mar 2026 19:19:28 +0400 Subject: [PATCH] feat: revert ignore object ACL behavior The logic to return a `NotImplemented` error on object upload operations, when any ACL header is present has been removed. Now all object ACL headers are by default ignored. The `-noacl` flag is preserved to disabled bucket ACLs. **Testing** The Put/Get object ACL tests are moved to `NotImplemented` integration tests group as a default gateway behavior. The existing `_acl_not_supported` tests are modified to expect no error, when ACLs are used on object uploads. --- s3api/controllers/object-post.go | 11 +- s3api/controllers/object-put.go | 22 +-- s3api/utils/utils.go | 26 ---- tests/integration/CopyObject.go | 2 +- tests/integration/CreateMultipartUpload.go | 2 +- tests/integration/NotImplemented_actions.go | 47 ++++++ tests/integration/PutObject.go | 2 +- tests/integration/group-tests.go | 15 +- tests/integration/no_acl.go | 152 -------------------- 9 files changed, 58 insertions(+), 221 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index 3d34dbc3..ffd0d346 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -160,16 +160,7 @@ func (c S3ApiController) CreateMultipartUpload(ctx *fiber.Ctx) (*Response, error isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool) parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) - err := utils.ValidateNoACLHeaders(ctx, c.disableACL) - if err != nil { - return &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: parsedAcl.Owner, - }, - }, err - } - - err = auth.VerifyAccess(ctx.Context(), c.be, + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, Acl: parsedAcl, diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 7dc3d484..25120b94 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -515,16 +515,7 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) { isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool) parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) - err := utils.ValidateNoACLHeaders(ctx, c.disableACL) - if err != nil { - return &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: parsedAcl.Owner, - }, - }, err - } - - err = utils.ValidateCopySource(copySource) + err := utils.ValidateCopySource(copySource) if err != nil { return &Response{ MetaOpts: &MetaOptions{ @@ -673,15 +664,6 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) { parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx) - err := utils.ValidateNoACLHeaders(ctx, c.disableACL) - if err != nil { - return &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: parsedAcl.Owner, - }, - }, err - } - // Content Length contentLengthStr := ctx.Get("Content-Length") if contentLengthStr == "" { @@ -697,7 +679,7 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) { // load the meta headers metadata := utils.GetUserMetaData(&ctx.Request().Header) - err = auth.VerifyAccess(ctx.Context(), c.be, + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, Acl: parsedAcl, diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index da6dd201..59dc1dc6 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -1006,29 +1006,3 @@ func NewTLSListener(network string, address string, getCertificateFunc func(*tls } return tls.NewListener(ln, config), nil } - -// ValidateNoACLHeaders checks whether any ACL-related request headers are set. -// since ACL operations are not supported on objects, the presence of any ACL headers -// results in a NotImplemented error. It returns nil only when all ACL headers -// are absent. -func ValidateNoACLHeaders(ctx *fiber.Ctx, disableACL bool) error { - if disableACL { - return nil - } - - for _, header := range []string{ - "x-amz-acl", - "x-amz-grant-full-control", - "x-amz-grant-read", - "x-amz-grant-read-acp", - "x-amz-grant-write-acp", - } { - value := ctx.Request().Header.Peek(header) - if len(value) != 0 { - debuglogger.Logf("an unsupported object acl header present: %s:%s", header, value) - return s3err.GetAPIError(s3err.ErrNotImplemented) - } - } - - return nil -} diff --git a/tests/integration/CopyObject.go b/tests/integration/CopyObject.go index 87d145d4..8a4320d0 100644 --- a/tests/integration/CopyObject.go +++ b/tests/integration/CopyObject.go @@ -1667,7 +1667,7 @@ func CopyObject_object_acl_not_supported(s *S3Conf) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.CopyObject(ctx, input) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + if err != nil { return fmt.Errorf("test %v failed: %w", i+1, err) } } diff --git a/tests/integration/CreateMultipartUpload.go b/tests/integration/CreateMultipartUpload.go index c5aab224..63be4931 100644 --- a/tests/integration/CreateMultipartUpload.go +++ b/tests/integration/CreateMultipartUpload.go @@ -617,7 +617,7 @@ func CreateMultipartUpload_object_acl_not_supported(s *S3Conf) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.CreateMultipartUpload(ctx, input) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + if err != nil { return fmt.Errorf("test %v failed: %w", i+1, err) } } diff --git a/tests/integration/NotImplemented_actions.go b/tests/integration/NotImplemented_actions.go index c55b88d0..4beea7d6 100644 --- a/tests/integration/NotImplemented_actions.go +++ b/tests/integration/NotImplemented_actions.go @@ -701,3 +701,50 @@ func DeleteBucketWebsite_not_implemented(s *S3Conf) error { return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)) }) } + +func PutObjectAcl_not_implemented(s *S3Conf) error { + testName := "PutObjectAcl_not_implemented" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectAcl(ctx, &s3.PutObjectAclInput{ + Bucket: &bucket, + Key: &obj, + ACL: types.ObjectCannedACLAuthenticatedRead, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)) + }) +} + +func GetObjectAcl_not_implemented(s *S3Conf) error { + testName := "GetObjectAcl_not_implemented" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectAcl(ctx, &s3.GetObjectAclInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)) + }) +} diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 6b848f0a..5d99292c 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -1112,7 +1112,7 @@ func PutObject_object_acl_not_supported(s *S3Conf) error { modifyInput(input) _, err := putObjectWithData(0, input, s3client) - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + if err != nil { return fmt.Errorf("test %v failed: %w", i+1, err) } } diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 181f0beb..a53c962b 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -753,6 +753,9 @@ func TestNotImplementedActions(ts *TestState) { ts.Run(PutBucketWebsite_not_implemented) ts.Run(GetBucketWebsite_not_implemented) ts.Run(DeleteBucketWebsite_not_implemented) + // object acl actions + ts.Run(PutObjectAcl_not_implemented) + ts.Run(GetObjectAcl_not_implemented) } func TestWORMProtection(ts *TestState) { @@ -1176,12 +1179,7 @@ func TestSignedStreaminPayloadTrailer(ts *TestState) { func TestNoAclMode(ts *TestState) { ts.Run(NoAclMode_CreateBucket_with_acl) - ts.Run(NoAclMode_PutObject_with_acl) - ts.Run(NoAclMode_CopyObject_with_acl) - ts.Run(NoAclMode_multipart_upload_with_acl) ts.Run(NoAclMode_PutBucketAcl) - ts.Run(NoAclMode_PutObjectAcl_not_implemented) - ts.Run(NoAclMode_GetObjectAcl_not_implemented) } type IntTest func(s3 *S3Conf) error @@ -1709,6 +1707,8 @@ func GetIntTests() IntTests { "PutBucketWebsite_not_implemented": PutBucketWebsite_not_implemented, "GetBucketWebsite_not_implemented": GetBucketWebsite_not_implemented, "DeleteBucketWebsite_not_implemented": DeleteBucketWebsite_not_implemented, + "PutObjectAcl_not_implemented": PutObjectAcl_not_implemented, + "GetObjectAcl_not_implemented": GetObjectAcl_not_implemented, "WORMProtection_bucket_object_lock_configuration_compliance_mode": WORMProtection_bucket_object_lock_configuration_compliance_mode, "WORMProtection_bucket_object_lock_configuration_governance_mode": WORMProtection_bucket_object_lock_configuration_governance_mode, "WORMProtection_bucket_object_lock_governance_bypass_delete": WORMProtection_bucket_object_lock_governance_bypass_delete, @@ -1886,11 +1886,6 @@ func GetIntTests() IntTests { "SignedStreamingPayloadTrailer_bad_digest": SignedStreamingPayloadTrailer_bad_digest, "SignedStreamingPayloadTrailer_success": SignedStreamingPayloadTrailer_success, "NoAclMode_CreateBucket_with_acl": NoAclMode_CreateBucket_with_acl, - "NoAclMode_PutObject_with_acl": NoAclMode_PutObject_with_acl, - "NoAclMode_CopyObject_with_acl": NoAclMode_CopyObject_with_acl, - "NoAclMode_multipart_upload_with_acl": NoAclMode_multipart_upload_with_acl, "NoAclMode_PutBucketAcl": NoAclMode_PutBucketAcl, - "NoAclMode_PutObjectAcl_not_implemented": NoAclMode_PutObjectAcl_not_implemented, - "NoAclMode_GetObjectAcl_not_implemented": NoAclMode_GetObjectAcl_not_implemented, } } diff --git a/tests/integration/no_acl.go b/tests/integration/no_acl.go index 4039e6ab..19736679 100644 --- a/tests/integration/no_acl.go +++ b/tests/integration/no_acl.go @@ -17,7 +17,6 @@ package integration import ( "context" "fmt" - "strings" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -79,110 +78,6 @@ func NoAclMode_CreateBucket_with_acl(s *S3Conf) error { }) } -func NoAclMode_PutObject_with_acl(s *S3Conf) error { - testName := "NoAclMode_PutObject_with_acl" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - obj := "my-object" - u := getUser("user") - err := createUsers(s, []user{u}) - if err != nil { - return err - } - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.PutObject(ctx, &s3.PutObjectInput{ - Bucket: &bucket, - Key: &obj, - ACL: types.ObjectCannedACLBucketOwnerFullControl, - GrantFullControl: &u.access, - GrantRead: &u.access, - GrantReadACP: &u.access, - GrantWriteACP: &u.access, - Body: strings.NewReader("dummy data"), - }) - cancel() - - return err - }) -} - -func NoAclMode_CopyObject_with_acl(s *S3Conf) error { - testName := "NoAclMode_CopyObject_with_acl" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - u := getUser("user") - err := createUsers(s, []user{u}) - if err != nil { - return err - } - - srcObj, dstObj := "source-object", "destination-object" - _, err = putObjectWithData(10, &s3.PutObjectInput{ - Bucket: &bucket, - Key: &srcObj, - ACL: types.ObjectCannedACLAuthenticatedRead, - }, s3client) - if err != nil { - return err - } - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ - Bucket: &bucket, - Key: &dstObj, - CopySource: getPtr(fmt.Sprintf("%s/%s", bucket, srcObj)), - }) - cancel() - - return err - }) -} - -func NoAclMode_multipart_upload_with_acl(s *S3Conf) error { - testName := "NoAclMode_CreateMultipartUpload_with_acl" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - obj := "my-object" - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - mp, err := s3client.CreateMultipartUpload(ctx, &s3.CreateMultipartUploadInput{ - Bucket: &bucket, - Key: &obj, - ACL: types.ObjectCannedACLAuthenticatedRead, - GrantFullControl: getPtr("non_existing_user_1"), - GrantRead: getPtr("non_existing_user_2"), - GrantReadACP: getPtr("non_existing_user_3"), - GrantWriteACP: getPtr("non_existing_user_4"), - }) - cancel() - if err != nil { - return err - } - - parts, _, err := uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId) - if err != nil { - return err - } - - compParts := []types.CompletedPart{} - for _, el := range parts { - compParts = append(compParts, types.CompletedPart{ - ETag: el.ETag, - PartNumber: el.PartNumber, - }) - } - - ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ - Bucket: &bucket, - Key: &obj, - UploadId: mp.UploadId, - MultipartUpload: &types.CompletedMultipartUpload{ - Parts: compParts, - }, - }) - cancel() - - return err - }) -} - func NoAclMode_PutBucketAcl(s *S3Conf) error { testName := "NoAclMode_PutBucketAcl" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -196,50 +91,3 @@ func NoAclMode_PutBucketAcl(s *S3Conf) error { return checkApiErr(err, s3err.GetAPIError(s3err.ErrACLsDisabled)) }) } - -func NoAclMode_PutObjectAcl_not_implemented(s *S3Conf) error { - testName := "NoAclMode_PutObjectAcl_not_implemented" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - obj := "my-object" - _, err := putObjectWithData(10, &s3.PutObjectInput{ - Bucket: &bucket, - Key: &obj, - }, s3client) - if err != nil { - return err - } - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.PutObjectAcl(ctx, &s3.PutObjectAclInput{ - Bucket: &bucket, - Key: &obj, - ACL: types.ObjectCannedACLAuthenticatedRead, - }) - cancel() - - return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)) - }) -} - -func NoAclMode_GetObjectAcl_not_implemented(s *S3Conf) error { - testName := "NoAclMode_GetObjectAcl_not_implemented" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - obj := "my-object" - _, err := putObjectWithData(10, &s3.PutObjectInput{ - Bucket: &bucket, - Key: &obj, - }, s3client) - if err != nil { - return err - } - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.GetObjectAcl(ctx, &s3.GetObjectAclInput{ - Bucket: &bucket, - Key: &obj, - }) - cancel() - - return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)) - }) -}