diff --git a/auth/acl.go b/auth/acl.go index be9d6871..a69c4270 100644 --- a/auth/acl.go +++ b/auth/acl.go @@ -496,7 +496,7 @@ func UpdateBucketACLOwner(ctx context.Context, be backend.Backend, bucket, newOw } // ValidateCannedACL validates bucket canned acl value -func ValidateCannedACL(acl string) error { +func ValidateCannedACL(acl types.BucketCannedACL) error { switch types.BucketCannedACL(acl) { case types.BucketCannedACLPrivate, types.BucketCannedACLPublicRead, types.BucketCannedACLPublicReadWrite, "": return nil diff --git a/s3api/controllers/bucket-put.go b/s3api/controllers/bucket-put.go index 3d1d262f..a568e1fa 100644 --- a/s3api/controllers/bucket-put.go +++ b/s3api/controllers/bucket-put.go @@ -321,7 +321,7 @@ func (c S3ApiController) PutBucketPolicy(ctx *fiber.Ctx) (*Response, error) { func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) { bucket := ctx.Params("bucket") - acl := ctx.Get("X-Amz-Acl") + acl := types.BucketCannedACL(ctx.Get("X-Amz-Acl")) grantFullControl := ctx.Get("X-Amz-Grant-Full-Control") grantRead := ctx.Get("X-Amz-Grant-Read") grantReadACP := ctx.Get("X-Amz-Grant-Read-Acp") @@ -414,7 +414,7 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) { } } - if grants+acl != "" { + if grants+string(acl) != "" { debuglogger.Logf("invalid request: %q (grants) %q (acl)", grants, acl) return &Response{ @@ -480,7 +480,7 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) { func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { bucket := ctx.Params("bucket") - acl := ctx.Get("X-Amz-Acl") + acl := types.BucketCannedACL(ctx.Get("X-Amz-Acl")) grantFullControl := ctx.Get("X-Amz-Grant-Full-Control") grantRead := ctx.Get("X-Amz-Grant-Read") grantReadACP := ctx.Get("X-Amz-Grant-Read-Acp") @@ -488,9 +488,7 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { grantWriteACP := ctx.Get("X-Amz-Grant-Write-Acp") lockEnabled := strings.EqualFold(ctx.Get("X-Amz-Bucket-Object-Lock-Enabled"), "true") grants := grantFullControl + grantRead + grantReadACP + grantWrite + grantWriteACP - objectOwnership := types.ObjectOwnership( - ctx.Get("X-Amz-Object-Ownership", string(types.ObjectOwnershipBucketOwnerEnforced)), - ) + objectOwnership := types.ObjectOwnership(ctx.Get("X-Amz-Object-Ownership")) if c.readonly { return &Response{ @@ -519,6 +517,16 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { }, s3err.GetAPIError(s3err.ErrInvalidBucketName) } + // both bucket canned ACL and acl grants is not allowed + if acl != "" && grants != "" { + debuglogger.Logf("invalid request: %q (grants) %q (acl)", grants, acl) + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: bucketOwner.Access, + }, + }, s3err.GetAPIError(s3err.ErrBothCannedAndHeaderGrants) + } + // validate bucket canned acl err := auth.ValidateCannedACL(acl) if err != nil { @@ -529,6 +537,16 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { }, err } + // if bucket acl is 'private', object ownership should default to 'BucketOwnerPreferred' + if acl == types.BucketCannedACLPrivate && objectOwnership == "" { + objectOwnership = types.ObjectOwnershipBucketOwnerPreferred + } + + // if object ownership is so far empty, it should default to BucketOwnerEnforced + if objectOwnership == "" { + objectOwnership = types.ObjectOwnershipBucketOwnerEnforced + } + // validate the object ownership value if ok := utils.IsValidOwnership(objectOwnership); !ok { return &Response{ @@ -542,7 +560,11 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { } } - if acl+grants != "" && objectOwnership == types.ObjectOwnershipBucketOwnerEnforced { + // any bucket ACL(canned, grants) is not allowed with object ownership 'BucketOwnerEnforced' + // but there's an exception for 'private' bucket canned ACL, + // which is the effective default for all buckets. In this case + // the ACL(private canned) is allowed with 'BucketOwnerEnforced' + if acl != types.BucketCannedACLPrivate && string(acl)+grants != "" && objectOwnership == types.ObjectOwnershipBucketOwnerEnforced { debuglogger.Logf("bucket acls are disabled for %v object ownership", objectOwnership) return &Response{ MetaOpts: &MetaOptions{ @@ -551,15 +573,6 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { }, s3err.GetAPIError(s3err.ErrInvalidBucketAclWithObjectOwnership) } - if acl != "" && grants != "" { - debuglogger.Logf("invalid request: %q (grants) %q (acl)", grants, acl) - return &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: bucketOwner.Access, - }, - }, s3err.GetAPIError(s3err.ErrBothCannedAndHeaderGrants) - } - var body s3response.CreateBucketConfiguration if len(ctx.Body()) != 0 { // request body is optional for CreateBucket diff --git a/tests/integration/CreateBucket.go b/tests/integration/CreateBucket.go index 0e91f53d..61e44d67 100644 --- a/tests/integration/CreateBucket.go +++ b/tests/integration/CreateBucket.go @@ -287,6 +287,133 @@ func CreateBucket_non_default_acl(s *S3Conf) error { return nil } +func CreateBucket_private_canned_acl(s *S3Conf) error { + testName := "CreateBucket_private_canned_acl" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + ACL: types.BucketCannedACLPrivate, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.GetBucketAcl(ctx, &s3.GetBucketAclInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if getString(res.Owner.ID) != s.awsID { + return fmt.Errorf("expected bucket owner to be %v, instead got %v", + s.awsID, getString(res.Owner.ID)) + } + if len(res.Grants) != 1 { + return fmt.Errorf("expected grants length to be 1, instead got %v", + len(res.Grants)) + } + grt := res.Grants[0] + if grt.Permission != types.PermissionFullControl { + return fmt.Errorf("expected the grantee to have full-control permission, instead got %v", + grt.Permission) + } + if getString(grt.Grantee.ID) != s.awsID { + return fmt.Errorf("expected the grantee id to be %v, instead got %v", + s.awsID, getString(grt.Grantee.ID)) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketOwnershipControls(ctx, &s3.GetBucketOwnershipControlsInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if out.OwnershipControls == nil { + return fmt.Errorf("unexpected nil OwnershipControls") + } + if len(out.OwnershipControls.Rules) != 1 { + return fmt.Errorf("expected the OwnershipControls rules length to be 1, instead got %v", len(out.OwnershipControls.Rules)) + } + if out.OwnershipControls.Rules[0].ObjectOwnership != types.ObjectOwnershipBucketOwnerPreferred { + return fmt.Errorf("expected the ObjectOwnership to be %s, instead got %s", types.ObjectOwnershipBucketOwnerPreferred, out.OwnershipControls.Rules[0].ObjectOwnership) + } + + return teardown(s, bucket) + }) +} + +func CreateBucket_private_canned_acl_bucket_owner_enforced_ownership(s *S3Conf) error { + testName := "CreateBucket_private_canned_acl_bucket_owner_enforced_ownership" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + ACL: types.BucketCannedACLPrivate, + ObjectOwnership: types.ObjectOwnershipBucketOwnerEnforced, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.GetBucketAcl(ctx, &s3.GetBucketAclInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if getString(res.Owner.ID) != s.awsID { + return fmt.Errorf("expected bucket owner to be %v, instead got %v", + s.awsID, getString(res.Owner.ID)) + } + if len(res.Grants) != 1 { + return fmt.Errorf("expected grants length to be 1, instead got %v", + len(res.Grants)) + } + grt := res.Grants[0] + if grt.Permission != types.PermissionFullControl { + return fmt.Errorf("expected the grantee to have full-control permission, instead got %v", + grt.Permission) + } + if getString(grt.Grantee.ID) != s.awsID { + return fmt.Errorf("expected the grantee id to be %v, instead got %v", + s.awsID, getString(grt.Grantee.ID)) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketOwnershipControls(ctx, &s3.GetBucketOwnershipControlsInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if out.OwnershipControls == nil { + return fmt.Errorf("unexpected nil OwnershipControls") + } + if len(out.OwnershipControls.Rules) != 1 { + return fmt.Errorf("expected the OwnershipControls rules length to be 1, instead got %v", len(out.OwnershipControls.Rules)) + } + if out.OwnershipControls.Rules[0].ObjectOwnership != types.ObjectOwnershipBucketOwnerEnforced { + return fmt.Errorf("expected the ObjectOwnership to be %s, instead got %s", types.ObjectOwnershipBucketOwnerEnforced, out.OwnershipControls.Rules[0].ObjectOwnership) + } + + return teardown(s, bucket) + }) +} + func CreateBucket_default_object_lock(s *S3Conf) error { testName := "CreateBucket_default_object_lock" runF(testName) diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 93ad0274..248b5a57 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -79,6 +79,8 @@ func TestCreateBucket(ts *TestState) { ts.Run(CreateBucket_as_user) ts.Run(CreateBucket_default_acl) ts.Run(CreateBucket_non_default_acl) + ts.Run(CreateBucket_private_canned_acl) + ts.Run(CreateBucket_private_canned_acl_bucket_owner_enforced_ownership) ts.Run(CreateDeleteBucket_success) ts.Run(CreateBucket_default_object_lock) ts.Run(CreateBucket_invalid_location_constraint) @@ -1247,6 +1249,8 @@ func GetIntTests() IntTests { "CreateDeleteBucket_success": CreateDeleteBucket_success, "CreateBucket_default_acl": CreateBucket_default_acl, "CreateBucket_non_default_acl": CreateBucket_non_default_acl, + "CreateBucket_private_canned_acl": CreateBucket_private_canned_acl, + "CreateBucket_private_canned_acl_bucket_owner_enforced_ownership": CreateBucket_private_canned_acl_bucket_owner_enforced_ownership, "CreateBucket_default_object_lock": CreateBucket_default_object_lock, "CreateBucket_invalid_location_constraint": CreateBucket_invalid_location_constraint, "CreateBucket_long_tags": CreateBucket_long_tags,