diff --git a/auth/acl.go b/auth/acl.go index 3b32cda..e900324 100644 --- a/auth/acl.go +++ b/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) +} diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 5cfa638..9934a5b 100644 --- a/backend/azure/azure.go +++ b/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 { - return az.PutBucketAcl(ctx, bucket, acl) +func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket, owner string) error { + return auth.UpdateBucketACLOwner(ctx, az, bucket, owner) } // The action actually returns the containers owned by the user, who initialized the gateway diff --git a/backend/backend.go b/backend/backend.go index de9ea2b..946ee73 100644 --- a/backend/backend.go +++ b/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) { diff --git a/backend/posix/posix.go b/backend/posix/posix.go index bb99ebe..3c96989 100644 --- a/backend/posix/posix.go +++ b/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 { - return p.PutBucketAcl(ctx, bucket, acl) +func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, owner string) error { + return auth.UpdateBucketACLOwner(ctx, p, bucket, owner) } func listBucketFileInfos(bucketlinks bool) ([]fs.FileInfo, error) { diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 2620ddf..d29db3d 100644 --- a/backend/s3proxy/s3.go +++ b/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 { - return s.PutBucketAcl(ctx, bucket, acl) +func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket, owner string) error { + return auth.UpdateBucketACLOwner(ctx, s, bucket, owner) } func (s *S3Proxy) ListBucketsAndOwners(ctx context.Context) ([]s3response.Bucket, error) { diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index 9aada79..d10dae1 100644 --- a/s3api/controllers/admin.go +++ b/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{ - 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) + err = c.be.ChangeBucketOwner(ctx.Context(), bucket, owner) return SendResponse(ctx, err, &MetaOpts{ Logger: c.l, diff --git a/s3api/controllers/admin_test.go b/s3api/controllers/admin_test.go index 3382461..aaa82a0 100644 --- a/s3api/controllers/admin_test.go +++ b/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 }, }, diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 5d2e9a2..d0ef8d7 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/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. - ACL []byte + // Owner is the owner argument value. + 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