fix: Removed the bucket ACL owner check for admin and root users

versity · Jul 17, 2024 · 23a40d8 · 23a40d8
1 parent ed9a10a
commit 23a40d8
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 6 deletions.
diff --git a/auth/acl.go b/auth/acl.go
@@ -118,11 +118,11 @@ func ParseACLOutput(data []byte) (GetBucketAclOutput, error) {
 	}, nil
 }
 
-func UpdateACL(input *PutBucketAclInput, acl ACL, iam IAMService) ([]byte, error) {
+func UpdateACL(input *PutBucketAclInput, acl ACL, iam IAMService, isAdmin bool) ([]byte, error) {
 	if input == nil {
 		return nil, s3err.GetAPIError(s3err.ErrInvalidRequest)
 	}
-	if acl.Owner != *input.AccessControlPolicy.Owner.ID {
+	if !isAdmin && acl.Owner != *input.AccessControlPolicy.Owner.ID {
 		return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
 	}
 

diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go
@@ -1361,7 +1361,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error {
 			}
 		}
 
-		updAcl, err := auth.UpdateACL(input, parsedAcl, c.iam)
+		updAcl, err := auth.UpdateACL(input, parsedAcl, c.iam, acct.Role == auth.RoleAdmin)
 		if err != nil {
 			return SendResponse(ctx, err,
 				&MetaOpts{
@@ -1448,7 +1448,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error {
 				ID: &acct.Access,
 			}},
 		ACL: types.BucketCannedACL(acl),
-	}, defACL, c.iam)
+	}, defACL, c.iam, acct.Role == auth.RoleAdmin)
 	if err != nil {
 		return SendResponse(ctx, err,
 			&MetaOpts{

diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go
@@ -481,6 +481,7 @@ func TestAccessControl(s *S3Conf) {
 	AccessControl_single_object_resource_actions(s)
 	AccessControl_multi_statement_policy(s)
 	AccessControl_bucket_ownership_to_user(s)
+	AccessControl_root_PutBucketAcl(s)
 }
 
 type IntTests map[string]func(s *S3Conf) error
@@ -771,5 +772,6 @@ func GetIntTests() IntTests {
 		"AccessControl_single_object_resource_actions":                        AccessControl_single_object_resource_actions,
 		"AccessControl_multi_statement_policy":                                AccessControl_multi_statement_policy,
 		"AccessControl_bucket_ownership_to_user":                              AccessControl_bucket_ownership_to_user,
+		"AccessControl_root_PutBucketAcl":                                     AccessControl_root_PutBucketAcl,
 	}
 }
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
@@ -6250,14 +6250,30 @@ func PutBucketAcl_invalid_acl_acp_and_grants(s *S3Conf) error {
 func PutBucketAcl_invalid_owner(s *S3Conf) error {
 	testName := "PutBucketAcl_invalid_owner"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		usr := user{
+			access: "grt1",
+			secret: "grt1secret",
+			role:   "user",
+		}
+
+		if err := createUsers(s, []user{usr}); err != nil {
+			return err
+		}
+
+		if err := changeBucketsOwner(s, []string{bucket}, usr.access); err != nil {
+			return err
+		}
+
+		userClient := getUserS3Client(usr, s)
+
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{
+		_, err := userClient.PutBucketAcl(ctx, &s3.PutBucketAclInput{
 			Bucket: &bucket,
 			AccessControlPolicy: &types.AccessControlPolicy{
 				Grants: []types.Grant{
 					{
 						Grantee: &types.Grantee{
-							ID:   getPtr("awsID"),
+							ID:   getPtr(usr.access),
 							Type: types.TypeCanonicalUser,
 						},
 					},
@@ -9188,6 +9204,39 @@ func AccessControl_bucket_ownership_to_user(s *S3Conf) error {
 	})
 }
 
+func AccessControl_root_PutBucketAcl(s *S3Conf) error {
+	testName := "AccessControl_root_PutBucketAcl"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		usr := user{
+			access: "grt1",
+			secret: "grt1secret",
+			role:   "user",
+		}
+
+		if err := createUsers(s, []user{usr}); err != nil {
+			return err
+		}
+
+		if err := changeBucketsOwner(s, []string{bucket}, usr.access); err != nil {
+			return err
+		}
+
+		userClient := getUserS3Client(usr, s)
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := userClient.PutBucketAcl(ctx, &s3.PutBucketAclInput{
+			Bucket: &bucket,
+			ACL:    types.BucketCannedACLPrivate,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}, withOwnership(types.ObjectOwnershipBucketOwnerPreferred))
+}
+
 // IAM related tests
 // multi-user iam tests
 func IAM_user_access_denied(s *S3Conf) error {