fix: add new auth method to update ownership within acl

Add helper util auth.UpdateBucketACLOwner() that sets new
default ACL based on new owner and removes old bucket policy.

The ChangeBucketOwner() remains in the backend.Backend
interface in case there is ever a backend that needs to manage
ownership in some other way than with bucket ACLs. The arguments
are changing to clarify the updated owner. This will break any
plugins implementing the old interface. They should use the new
auth.UpdateBucketACLOwner() or implement the corresponding
change specific for the backend.

auth/acl.go

@@ -466,3 +466,30 @@ func VerifyPublicBucketACL(ctx context.Context, be backend.Backend, bucket strin
 
 	return nil
 }
+
+// UpdateBucketACLOwner sets default ACL with new owner and removes
+// any previous bucket policy that was in place
+func UpdateBucketACLOwner(ctx context.Context, be backend.Backend, bucket, newOwner string) error {
+	acl := ACL{
+		Owner: newOwner,
+		Grantees: []Grantee{
+			{
+				Permission: PermissionFullControl,
+				Access:     newOwner,
+				Type:       types.TypeCanonicalUser,
+			},
+		},
+	}
+
+	result, err := json.Marshal(acl)
+	if err != nil {
+		return fmt.Errorf("marshal ACL: %w", err)
+	}
+
+	err = be.PutBucketAcl(ctx, bucket, result)
+	if err != nil {
+		return err
+	}
+
+	return be.DeleteBucketPolicy(ctx, bucket)
+}

backend/azure/azure.go

@@ -1683,8 +1683,8 @@ func (az *Azure) GetObjectLegalHold(ctx context.Context, bucket, object, version
 	return &status, nil
 }
 
-func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
+func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket, owner string) error {
-	return az.PutBucketAcl(ctx, bucket, acl)
+	return auth.UpdateBucketACLOwner(ctx, az, bucket, owner)
 }
 
 // The action actually returns the containers owned by the user, who initialized the gateway

backend/backend.go

@@ -96,7 +96,7 @@ type Backend interface {
 	GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error)
 
 	// non AWS actions
-	ChangeBucketOwner(_ context.Context, bucket string, acl []byte) error
+	ChangeBucketOwner(_ context.Context, bucket, owner string) error
 	ListBucketsAndOwners(context.Context) ([]s3response.Bucket, error)
 }
 
@@ -280,7 +280,7 @@ func (BackendUnsupported) GetObjectLegalHold(_ context.Context, bucket, object,
 	return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
 }
 
-func (BackendUnsupported) ChangeBucketOwner(_ context.Context, bucket string, acl []byte) error {
+func (BackendUnsupported) ChangeBucketOwner(_ context.Context, bucket, owner string) error {
 	return s3err.GetAPIError(s3err.ErrNotImplemented)
 }
 func (BackendUnsupported) ListBucketsAndOwners(context.Context) ([]s3response.Bucket, error) {

backend/posix/posix.go

@@ -4888,8 +4888,8 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId
 	return data, nil
 }
 
-func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
+func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, owner string) error {
-	return p.PutBucketAcl(ctx, bucket, acl)
+	return auth.UpdateBucketACLOwner(ctx, p, bucket, owner)
 }
 
 func listBucketFileInfos(bucketlinks bool) ([]fs.FileInfo, error) {

backend/s3proxy/s3.go

@@ -1547,8 +1547,8 @@ func (s *S3Proxy) GetObjectLegalHold(ctx context.Context, bucket, object, versio
 	return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
 }
 
-func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
+func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket, owner string) error {
-	return s.PutBucketAcl(ctx, bucket, acl)
+	return auth.UpdateBucketACLOwner(ctx, s, bucket, owner)
 }
 
 func (s *S3Proxy) ListBucketsAndOwners(ctx context.Context) ([]s3response.Bucket, error) {

s3api/controllers/admin.go

@@ -15,13 +15,10 @@
 package controllers
 
 import (
-	"encoding/json"
 	"encoding/xml"
-	"fmt"
 	"net/http"
 	"strings"
 
-	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/gofiber/fiber/v2"
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/backend"
@@ -172,27 +169,7 @@ func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error {
 			})
 	}
 
-	acl := auth.ACL{
+	err = c.be.ChangeBucketOwner(ctx.Context(), bucket, owner)
-		Owner: owner,
-		Grantees: []auth.Grantee{
-			{
-				Permission: auth.PermissionFullControl,
-				Access:     owner,
-				Type:       types.TypeCanonicalUser,
-			},
-		},
-	}
-
-	aclParsed, err := json.Marshal(acl)
-	if err != nil {
-		return SendResponse(ctx, fmt.Errorf("failed to marshal the bucket acl: %w", err),
-			&MetaOpts{
-				Logger: c.l,
-				Action: metrics.ActionAdminChangeBucketOwner,
-			})
-	}
-
-	err = c.be.ChangeBucketOwner(ctx.Context(), bucket, aclParsed)
 	return SendResponse(ctx, err,
 		&MetaOpts{
 			Logger: c.l,

s3api/controllers/admin_test.go

@@ -324,7 +324,7 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) {
 	}
 	adminController := AdminController{
 		be: &BackendMock{
-			ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, acl []byte) error {
+			ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket, owner string) error {
 				return nil
 			},
 		},

s3api/controllers/backend_moq_test.go

@@ -26,7 +26,7 @@ var _ backend.Backend = &BackendMock{}
 //			AbortMultipartUploadFunc: func(contextMoqParam context.Context, abortMultipartUploadInput *s3.AbortMultipartUploadInput) error {
 //				panic("mock out the AbortMultipartUpload method")
 //			},
-//			ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, acl []byte) error {
+//			ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, owner string) error {
 //				panic("mock out the ChangeBucketOwner method")
 //			},
 //			CompleteMultipartUploadFunc: func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (s3response.CompleteMultipartUploadResult, string, error) {
@@ -196,7 +196,7 @@ type BackendMock struct {
 	AbortMultipartUploadFunc func(contextMoqParam context.Context, abortMultipartUploadInput *s3.AbortMultipartUploadInput) error
 
 	// ChangeBucketOwnerFunc mocks the ChangeBucketOwner method.
-	ChangeBucketOwnerFunc func(contextMoqParam context.Context, bucket string, acl []byte) error
+	ChangeBucketOwnerFunc func(contextMoqParam context.Context, bucket string, owner string) error
 
 	// CompleteMultipartUploadFunc mocks the CompleteMultipartUpload method.
 	CompleteMultipartUploadFunc func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (s3response.CompleteMultipartUploadResult, string, error)
@@ -369,8 +369,8 @@ type BackendMock struct {
 			ContextMoqParam context.Context
 			// Bucket is the bucket argument value.
 			Bucket string
-			// ACL is the acl argument value.
+			// Owner is the owner argument value.
-			ACL []byte
+			Owner string
 		}
 		// CompleteMultipartUpload holds details about calls to the CompleteMultipartUpload method.
 		CompleteMultipartUpload []struct {
@@ -864,23 +864,23 @@ func (mock *BackendMock) AbortMultipartUploadCalls() []struct {
 }
 
 // ChangeBucketOwner calls ChangeBucketOwnerFunc.
-func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, bucket string, acl []byte) error {
+func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, bucket string, owner string) error {
 	if mock.ChangeBucketOwnerFunc == nil {
 		panic("BackendMock.ChangeBucketOwnerFunc: method is nil but Backend.ChangeBucketOwner was just called")
 	}
 	callInfo := struct {
 		ContextMoqParam context.Context
 		Bucket          string
-		ACL             []byte
+		Owner           string
 	}{
 		ContextMoqParam: contextMoqParam,
 		Bucket:          bucket,
-		ACL:             acl,
+		Owner:           owner,
 	}
 	mock.lockChangeBucketOwner.Lock()
 	mock.calls.ChangeBucketOwner = append(mock.calls.ChangeBucketOwner, callInfo)
 	mock.lockChangeBucketOwner.Unlock()
-	return mock.ChangeBucketOwnerFunc(contextMoqParam, bucket, acl)
+	return mock.ChangeBucketOwnerFunc(contextMoqParam, bucket, owner)
 }
 
 // ChangeBucketOwnerCalls gets all the calls that were made to ChangeBucketOwner.
@@ -890,12 +890,12 @@ func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, buck
 func (mock *BackendMock) ChangeBucketOwnerCalls() []struct {
 	ContextMoqParam context.Context
 	Bucket          string
-	ACL             []byte
+	Owner           string
 } {
 	var calls []struct {
 		ContextMoqParam context.Context
 		Bucket          string
-		ACL             []byte
+		Owner           string
 	}
 	mock.lockChangeBucketOwner.RLock()
 	calls = mock.calls.ChangeBucketOwner