Merge pull request #671 from versity/fix/change-bucket-owner-ownership

ChangeBucketOwner acl update
This commit is contained in:
Ben McClelland
2024-07-11 14:02:09 -07:00
committed by GitHub
9 changed files with 81 additions and 77 deletions
+2 -28
View File
@@ -1368,34 +1368,8 @@ func (az *Azure) GetObjectLegalHold(ctx context.Context, bucket, object, version
return &status, nil
}
func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error {
client, err := az.getContainerClient(bucket)
if err != nil {
return err
}
props, err := client.GetProperties(ctx, nil)
if err != nil {
return azureErrToS3Err(err)
}
acl, err := getAclFromMetadata(props.Metadata, keyAclCapital)
if err != nil {
return err
}
acl.Owner = newOwner
newAcl, err := json.Marshal(acl)
if err != nil {
return fmt.Errorf("marshal acl: %w", err)
}
err = az.PutBucketAcl(ctx, bucket, newAcl)
if err != nil {
return err
}
return nil
func (az *Azure) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
return az.PutBucketAcl(ctx, bucket, acl)
}
// The action actually returns the containers owned by the user, who initialized the gateway
+2 -2
View File
@@ -93,7 +93,7 @@ type Backend interface {
GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error)
// non AWS actions
ChangeBucketOwner(_ context.Context, bucket, newOwner string) error
ChangeBucketOwner(_ context.Context, bucket string, acl []byte) error
ListBucketsAndOwners(context.Context) ([]s3response.Bucket, error)
}
@@ -268,7 +268,7 @@ func (BackendUnsupported) GetObjectLegalHold(_ context.Context, bucket, object,
return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
}
func (BackendUnsupported) ChangeBucketOwner(_ context.Context, bucket, newOwner string) error {
func (BackendUnsupported) ChangeBucketOwner(_ context.Context, bucket string, acl []byte) error {
return s3err.GetAPIError(s3err.ErrNotImplemented)
}
func (BackendUnsupported) ListBucketsAndOwners(context.Context) ([]s3response.Bucket, error) {
+2 -33
View File
@@ -2619,39 +2619,8 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId
return data, nil
}
func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
if err != nil {
return fmt.Errorf("stat bucket: %w", err)
}
aclTag, err := p.meta.RetrieveAttribute(bucket, "", aclkey)
if err != nil {
return fmt.Errorf("get acl: %w", err)
}
var acl auth.ACL
err = json.Unmarshal(aclTag, &acl)
if err != nil {
return fmt.Errorf("unmarshal acl: %w", err)
}
acl.Owner = newOwner
newAcl, err := json.Marshal(acl)
if err != nil {
return fmt.Errorf("marshal acl: %w", err)
}
err = p.meta.StoreAttribute(bucket, "", aclkey, newAcl)
if err != nil {
return fmt.Errorf("set acl: %w", err)
}
return nil
func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
return p.PutBucketAcl(ctx, bucket, acl)
}
func (p *Posix) ListBucketsAndOwners(ctx context.Context) (buckets []s3response.Bucket, err error) {
+6 -2
View File
@@ -619,8 +619,12 @@ func (s *S3Proxy) GetObjectLegalHold(ctx context.Context, bucket, object, versio
return &status, nil
}
func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error {
req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/change-bucket-owner/?bucket=%v&owner=%v", s.endpoint, bucket, newOwner), nil)
func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error {
var acll auth.ACL
if err := json.Unmarshal(acl, &acll); err != nil {
return fmt.Errorf("unmarshal acl: %w", err)
}
req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/change-bucket-owner/?bucket=%v&owner=%v", s.endpoint, bucket, acll.Owner), nil)
if err != nil {
return fmt.Errorf("failed to send the request: %w", err)
}
+17 -1
View File
@@ -19,6 +19,7 @@ import (
"fmt"
"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"
@@ -138,7 +139,22 @@ func (c AdminController) ChangeBucketOwner(ctx *fiber.Ctx) error {
return ctx.Status(fiber.StatusNotFound).SendString("user specified as the new bucket owner does not exist")
}
err = c.be.ChangeBucketOwner(ctx.Context(), bucket, owner)
acl := auth.ACL{
Owner: owner,
Grantees: []auth.Grantee{
{
Permission: types.PermissionFullControl,
Access: owner,
},
},
}
aclParsed, err := json.Marshal(acl)
if err != nil {
return fmt.Errorf("failed to marshal the bucket acl: %w", err)
}
err = c.be.ChangeBucketOwner(ctx.Context(), bucket, aclParsed)
if err != nil {
return err
}
+1 -1
View File
@@ -407,7 +407,7 @@ func TestAdminController_ChangeBucketOwner(t *testing.T) {
}
adminController := AdminController{
be: &BackendMock{
ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket, newOwner string) error {
ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, acl []byte) error {
return nil
},
},
+10 -10
View File
@@ -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, newOwner string) error {
// ChangeBucketOwnerFunc: func(contextMoqParam context.Context, bucket string, acl []byte) error {
// panic("mock out the ChangeBucketOwner method")
// },
// CompleteMultipartUploadFunc: func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) {
@@ -187,7 +187,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, newOwner string) error
ChangeBucketOwnerFunc func(contextMoqParam context.Context, bucket string, acl []byte) error
// CompleteMultipartUploadFunc mocks the CompleteMultipartUpload method.
CompleteMultipartUploadFunc func(contextMoqParam context.Context, completeMultipartUploadInput *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error)
@@ -351,8 +351,8 @@ type BackendMock struct {
ContextMoqParam context.Context
// Bucket is the bucket argument value.
Bucket string
// NewOwner is the newOwner argument value.
NewOwner string
// ACL is the acl argument value.
ACL []byte
}
// CompleteMultipartUpload holds details about calls to the CompleteMultipartUpload method.
CompleteMultipartUpload []struct {
@@ -822,23 +822,23 @@ func (mock *BackendMock) AbortMultipartUploadCalls() []struct {
}
// ChangeBucketOwner calls ChangeBucketOwnerFunc.
func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, bucket string, newOwner string) error {
func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, bucket string, acl []byte) error {
if mock.ChangeBucketOwnerFunc == nil {
panic("BackendMock.ChangeBucketOwnerFunc: method is nil but Backend.ChangeBucketOwner was just called")
}
callInfo := struct {
ContextMoqParam context.Context
Bucket string
NewOwner string
ACL []byte
}{
ContextMoqParam: contextMoqParam,
Bucket: bucket,
NewOwner: newOwner,
ACL: acl,
}
mock.lockChangeBucketOwner.Lock()
mock.calls.ChangeBucketOwner = append(mock.calls.ChangeBucketOwner, callInfo)
mock.lockChangeBucketOwner.Unlock()
return mock.ChangeBucketOwnerFunc(contextMoqParam, bucket, newOwner)
return mock.ChangeBucketOwnerFunc(contextMoqParam, bucket, acl)
}
// ChangeBucketOwnerCalls gets all the calls that were made to ChangeBucketOwner.
@@ -848,12 +848,12 @@ func (mock *BackendMock) ChangeBucketOwner(contextMoqParam context.Context, buck
func (mock *BackendMock) ChangeBucketOwnerCalls() []struct {
ContextMoqParam context.Context
Bucket string
NewOwner string
ACL []byte
} {
var calls []struct {
ContextMoqParam context.Context
Bucket string
NewOwner string
ACL []byte
}
mock.lockChangeBucketOwner.RLock()
calls = mock.calls.ChangeBucketOwner
+9
View File
@@ -480,6 +480,7 @@ func TestAccessControl(s *S3Conf) {
AccessControl_bucket_resource_all_action(s)
AccessControl_single_object_resource_actions(s)
AccessControl_multi_statement_policy(s)
AccessControl_bucket_ownership_to_user(s)
}
type IntTests map[string]func(s *S3Conf) error
@@ -762,5 +763,13 @@ func GetIntTests() IntTests {
"IAM_userplus_access_denied": IAM_userplus_access_denied,
"IAM_userplus_CreateBucket": IAM_userplus_CreateBucket,
"IAM_admin_ChangeBucketOwner": IAM_admin_ChangeBucketOwner,
"AccessControl_default_ACL_user_access_denied": AccessControl_default_ACL_user_access_denied,
"AccessControl_default_ACL_userplus_access_denied": AccessControl_default_ACL_userplus_access_denied,
"AccessControl_default_ACL_admin_successful_access": AccessControl_default_ACL_admin_successful_access,
"AccessControl_bucket_resource_single_action": AccessControl_bucket_resource_single_action,
"AccessControl_bucket_resource_all_action": AccessControl_bucket_resource_all_action,
"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,
}
}
+32
View File
@@ -9151,6 +9151,38 @@ func AccessControl_multi_statement_policy(s *S3Conf) error {
})
}
func AccessControl_bucket_ownership_to_user(s *S3Conf) error {
testName := "AccessControl_bucket_ownership_to_user"
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.HeadBucket(ctx, &s3.HeadBucketInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
return nil
})
}
// IAM related tests
// multi-user iam tests
func IAM_user_access_denied(s *S3Conf) error {