Merge pull request #1911 from versity/sis/revert-acl-headers-validation

feat: revert ignore object ACL behavior

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,

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,

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
-}

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)
 			}
 		}

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)
 			}
 		}

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))
+	})
+}

tests/integration/PutObject.go

@@ -1130,7 +1130,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)
 			}
 		}

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,
 	}
 }

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))
-	})
-}