diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 50fa52c..c01b86a 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -1142,7 +1142,6 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("acl") { var input *s3.PutBucketAclInput - var accessControlPolicy auth.AccessControlPolicy parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) err := auth.VerifyAccess(ctx.Context(), c.be, @@ -1165,21 +1164,21 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { }) } - err = xml.Unmarshal(ctx.Body(), &accessControlPolicy) - if err != nil { - if c.debug { - log.Printf("error unmarshalling access control policy: %v", err) + if len(ctx.Body()) > 0 { + var accessControlPolicy auth.AccessControlPolicy + err := xml.Unmarshal(ctx.Body(), &accessControlPolicy) + if err != nil { + if c.debug { + log.Printf("error unmarshalling access control policy: %v", err) + } + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrMalformedXML), + &MetaOpts{ + Logger: c.logger, + Action: metrics.ActionPutBucketAcl, + BucketOwner: parsedAcl.Owner, + }) } - return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest), - &MetaOpts{ - Logger: c.logger, - MetricsMng: c.mm, - Action: metrics.ActionPutBucketAcl, - BucketOwner: parsedAcl.Owner, - }) - } - if len(accessControlPolicy.AccessControlList.Grants) > 0 { if grants+acl != "" { if c.debug { log.Printf("invalid request: %q (grants) %q (acl)", @@ -1218,7 +1217,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { BucketOwner: parsedAcl.Owner, }) } - if len(accessControlPolicy.AccessControlList.Grants) > 0 || grants != "" { + if len(ctx.Body()) > 0 || grants != "" { if c.debug { log.Printf("invalid request: %q (grants) %q (acl)", grants, acl) @@ -1237,7 +1236,9 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { Bucket: &bucket, ACL: types.BucketCannedACL(acl), AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &accessControlPolicy.Owner, + Owner: &types.Owner{ + ID: &acct.Access, + }, }, } } @@ -1250,12 +1251,15 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { GrantWrite: &granWrite, GrantWriteACP: &grantWriteACP, AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &accessControlPolicy.Owner, + Owner: &types.Owner{ + ID: &acct.Access, + }, }, ACL: "", } } + fmt.Println(*input, parsedAcl) updAcl, err := auth.UpdateACL(input, parsedAcl, c.iam) if err != nil { return SendResponse(ctx, err, diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index cca0f9b..5a40ef5 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -593,14 +593,6 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { ` - succBody := ` - - - valid access - - - ` - tagBody := ` @@ -690,10 +682,9 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { // PutBucketAcl incorrect bucket owner case incorrectBucketOwner := httptest.NewRequest(http.MethodPut, "/my-bucket?acl", strings.NewReader(invOwnerBody)) - incorrectBucketOwner.Header.Set("X-Amz-Acl", "private") // PutBucketAcl acl success - aclSuccReq := httptest.NewRequest(http.MethodPut, "/my-bucket?acl", strings.NewReader(succBody)) + aclSuccReq := httptest.NewRequest(http.MethodPut, "/my-bucket?acl", nil) aclSuccReq.Header.Set("X-Amz-Acl", "private") // Invalid acl body case diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 8118b6c..98dd7f0 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -270,6 +270,7 @@ func TestPutBucketAcl(s *S3Conf) { PutBucketAcl_invalid_acl_canned_and_grants(s) PutBucketAcl_invalid_acl_acp_and_grants(s) PutBucketAcl_invalid_owner(s) + PutBucketAcl_invalid_owner_not_in_body(s) PutBucketAcl_success_access_denied(s) PutBucketAcl_success_grants(s) PutBucketAcl_success_canned_acl(s) @@ -626,6 +627,7 @@ func GetIntTests() IntTests { "PutBucketAcl_invalid_acl_canned_and_grants": PutBucketAcl_invalid_acl_canned_and_grants, "PutBucketAcl_invalid_acl_acp_and_grants": PutBucketAcl_invalid_acl_acp_and_grants, "PutBucketAcl_invalid_owner": PutBucketAcl_invalid_owner, + "PutBucketAcl_invalid_owner_not_in_body": PutBucketAcl_invalid_owner_not_in_body, "PutBucketAcl_success_access_denied": PutBucketAcl_success_access_denied, "PutBucketAcl_success_grants": PutBucketAcl_success_grants, "PutBucketAcl_success_canned_acl": PutBucketAcl_success_canned_acl, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 4015a5c..093711c 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -5903,7 +5903,7 @@ func PutBucketAcl_invalid_acl_acp_and_grants(s *S3Conf) error { } func PutBucketAcl_invalid_owner(s *S3Conf) error { - testName := "PutBucketAcl_invalid_acl_acp_and_grants" + testName := "PutBucketAcl_invalid_owner" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{ @@ -5931,6 +5931,32 @@ func PutBucketAcl_invalid_owner(s *S3Conf) error { }) } +func PutBucketAcl_invalid_owner_not_in_body(s *S3Conf) error { + testName := "PutBucketAcl_invalid_owner_not_in_body" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + if err := createUsers(s, []user{{"grt1", "grt1secret", "user"}}); err != nil { + return err + } + + newConf := *s + newConf.awsID = "grt1" + newConf.awsSecret = "grt1secret" + userClient := s3.NewFromConfig(newConf.Config()) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := userClient.PutBucketAcl(ctx, &s3.PutBucketAclInput{ + Bucket: &bucket, + ACL: types.BucketCannedACLPublicRead, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil { + return err + } + + return nil + }) +} + func PutBucketAcl_success_access_denied(s *S3Conf) error { testName := "PutBucketAcl_success_access_denied" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -5987,12 +6013,7 @@ func PutBucketAcl_success_canned_acl(s *S3Conf) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err = s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{ Bucket: &bucket, - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &types.Owner{ - ID: &s.awsID, - }, - }, - ACL: types.BucketCannedACLPublicReadWrite, + ACL: types.BucketCannedACLPublicReadWrite, }) cancel() if err != nil { @@ -6023,12 +6044,7 @@ func PutBucketAcl_success_acp(s *S3Conf) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err = s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{ - Bucket: &bucket, - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &types.Owner{ - ID: &s.awsID, - }, - }, + Bucket: &bucket, GrantRead: getPtr("grt1"), }) cancel()