From b971467446ae0a8ecdd77db4411fc57a6edf91eb Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 20 May 2024 12:07:33 -0400 Subject: [PATCH] fix: Changed the logic to return BucketAlreadyOwnedByYou error when user tries to create an existing bucket owned by him --- backend/azure/azure.go | 29 ++++++++++++++++++++++++++++ backend/posix/posix.go | 12 ++++++++++++ tests/integration/group-tests.go | 2 ++ tests/integration/tests.go | 33 +++++++++++++++++++++++++++++++- 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index e5de128..6e06eab 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -127,6 +127,11 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, string(keyAclCapital): backend.GetStringPtr(string(acl)), } + acct, ok := ctx.Value("account").(auth.Account) + if !ok { + acct = auth.Account{} + } + if input.ObjectLockEnabledForBucket != nil && *input.ObjectLockEnabledForBucket { now := time.Now() defaultLock := auth.BucketLockConfig{ @@ -142,6 +147,30 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, meta[string(keyBucketLock)] = backend.GetStringPtr(string(defaultLockParsed)) } _, err := az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) + if errors.Is(s3err.GetAPIError(s3err.ErrBucketAlreadyExists), azureErrToS3Err(err)) { + client, err := az.getContainerClient(*input.Bucket) + if err != nil { + return err + } + + props, err := client.GetProperties(ctx, nil) + if err != nil { + return azureErrToS3Err(err) + } + + aclPtr, ok := props.Metadata[string(keyAclCapital)] + if !ok { + return fmt.Errorf("missing acl in the bucket") + } + + var acl auth.ACL + if err := json.Unmarshal([]byte(*aclPtr), &acl); err != nil { + return fmt.Errorf("unmarshal bucket acl: %w", err) + } + if acl.Owner == acct.Access { + return s3err.GetAPIError(s3err.ErrBucketAlreadyOwnedByYou) + } + } return azureErrToS3Err(err) } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 2d41803..ca14dde 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -213,6 +213,18 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a err := os.Mkdir(bucket, defaultDirPerm) if err != nil && os.IsExist(err) { + aclJSON, err := p.meta.RetrieveAttribute(bucket, "", aclkey) + if err != nil { + return fmt.Errorf("get bucket acl: %w", err) + } + var acl auth.ACL + if err := json.Unmarshal(aclJSON, &acl); err != nil { + return fmt.Errorf("unmarshal acl: %w", err) + } + + if acl.Owner == acct.Access { + return s3err.GetAPIError(s3err.ErrBucketAlreadyOwnedByYou) + } return s3err.GetAPIError(s3err.ErrBucketAlreadyExists) } if err != nil { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index a070f10..e05ea26 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -64,6 +64,7 @@ func TestPresignedAuthentication(s *S3Conf) { func TestCreateBucket(s *S3Conf) { CreateBucket_invalid_bucket_name(s) CreateBucket_existing_bucket(s) + CreateBucket_owned_by_you(s) CreateBucket_as_user(s) CreateBucket_default_acl(s) CreateBucket_non_default_acl(s) @@ -484,6 +485,7 @@ func GetIntTests() IntTests { "PresignedAuth_UploadPart": PresignedAuth_UploadPart, "CreateBucket_invalid_bucket_name": CreateBucket_invalid_bucket_name, "CreateBucket_existing_bucket": CreateBucket_existing_bucket, + "CreateBucket_owned_by_you": CreateBucket_owned_by_you, "CreateBucket_as_user": CreateBucket_as_user, "CreateDeleteBucket_success": CreateDeleteBucket_success, "CreateBucket_default_acl": CreateBucket_default_acl, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 2d87370..6cd044e 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -1667,7 +1667,21 @@ func CreateBucket_existing_bucket(s *S3Conf) error { testName := "CreateBucket_existing_bucket" runF(testName) bucket := getBucketName() - err := setup(s, bucket) + admin := user{ + access: "admin1", + secret: "admin1secret", + role: "admin", + } + if err := createUsers(s, []user{admin}); err != nil { + failF("%v: %v", testName, err) + return fmt.Errorf("%v: %w", testName, err) + } + + adminCfg := *s + adminCfg.awsID = admin.access + adminCfg.awsSecret = admin.secret + + err := setup(&adminCfg, bucket) if err != nil { failF("%v: %v", testName, err) return fmt.Errorf("%v: %w", testName, err) @@ -1688,6 +1702,23 @@ func CreateBucket_existing_bucket(s *S3Conf) error { return nil } +func CreateBucket_owned_by_you(s *S3Conf) error { + testName := "CreateBucket_owned_by_you" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + }) + cancel() + var bErr *types.BucketAlreadyOwnedByYou + if !errors.As(err, &bErr) { + return fmt.Errorf("expected error to be %w, instead got %w", s3err.GetAPIError(s3err.ErrBucketAlreadyOwnedByYou), err) + } + + return nil + }) +} + func CreateBucket_default_acl(s *S3Conf) error { testName := "CreateBucket_default_acl" runF(testName)