From 16e8134e8021cef1ffdb04bfd633df7e4b0c0e88 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 14 Oct 2024 15:04:11 -0400 Subject: [PATCH] fix: Adds bucket object lock status check in GetObjectLegalHold and GetObjectRetention actions --- backend/azure/azure.go | 66 ++++++++++-------- backend/posix/posix.go | 114 +++++++++++++++++-------------- s3err/s3err.go | 9 +-- tests/integration/group-tests.go | 4 ++ tests/integration/tests.go | 50 +++++++++++++- 5 files changed, 157 insertions(+), 86 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 0f788f9..f8cd941 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -1300,22 +1300,9 @@ func (az *Azure) GetObjectLockConfiguration(ctx context.Context, bucket string) } func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { - cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock)) + err := az.isBucketObjectLockEnabled(ctx, bucket) if err != nil { - return azureErrToS3Err(err) - } - - if len(cfg) == 0 { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) - } - - var bucketLockConfig auth.BucketLockConfig - if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { - return fmt.Errorf("parse bucket lock config: %w", err) - } - - if !bucketLockConfig.Enabled { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } blobClient, err := az.getBlobClient(bucket, object) @@ -1376,6 +1363,11 @@ func (az *Azure) GetObjectRetention(ctx context.Context, bucket, object, version return nil, azureErrToS3Err(err) } + err = az.isBucketObjectLockEnabled(ctx, bucket) + if err != nil { + return nil, err + } + retentionPtr, ok := props.Metadata[string(keyObjRetention)] if !ok { return nil, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration) @@ -1385,22 +1377,9 @@ func (az *Azure) GetObjectRetention(ctx context.Context, bucket, object, version } func (az *Azure) PutObjectLegalHold(ctx context.Context, bucket, object, versionId string, status bool) error { - cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock)) + err := az.isBucketObjectLockEnabled(ctx, bucket) if err != nil { - return azureErrToS3Err(err) - } - - if len(cfg) == 0 { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) - } - - var bucketLockConfig auth.BucketLockConfig - if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { - return fmt.Errorf("parse bucket lock config: %w", err) - } - - if !bucketLockConfig.Enabled { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } blobClient, err := az.getBlobClient(bucket, object) @@ -1447,6 +1426,11 @@ func (az *Azure) GetObjectLegalHold(ctx context.Context, bucket, object, version return nil, azureErrToS3Err(err) } + err = az.isBucketObjectLockEnabled(ctx, bucket) + if err != nil { + return nil, err + } + retentionPtr, ok := props.Metadata[string(keyObjLegalHold)] if !ok { return nil, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration) @@ -1486,6 +1470,28 @@ func (az *Azure) ListBucketsAndOwners(ctx context.Context) (buckets []s3response return buckets, nil } +func (az *Azure) isBucketObjectLockEnabled(ctx context.Context, bucket string) error { + cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock)) + if err != nil { + return azureErrToS3Err(err) + } + + if len(cfg) == 0 { + return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + } + + var bucketLockConfig auth.BucketLockConfig + if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { + return fmt.Errorf("parse bucket lock config: %w", err) + } + + if !bucketLockConfig.Enabled { + return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + } + + return nil +} + func (az *Azure) getContainerURL(cntr string) string { return fmt.Sprintf("%v/%v", strings.TrimRight(az.serviceURL, "/"), cntr) } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 146bd54..5ed7873 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -183,6 +183,26 @@ func (p *Posix) versioningEnabled() bool { return p.versioningDir != "" } +func (p *Posix) doesBucketAndObjectExist(bucket, object 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) + } + + _, err = os.Stat(filepath.Join(bucket, object)) + if errors.Is(err, fs.ErrNotExist) { + return s3err.GetAPIError(s3err.ErrNoSuchKey) + } + if err != nil { + return fmt.Errorf("stat object: %w", err) + } + + return nil +} + func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { entries, err := os.ReadDir(".") if err != nil { @@ -3626,6 +3646,30 @@ func (p *Posix) DeleteBucketPolicy(ctx context.Context, bucket string) error { return p.PutBucketPolicy(ctx, bucket, nil) } +func (p *Posix) isBucketObjectLockEnabled(bucket string) error { + cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey) + if errors.Is(err, fs.ErrNotExist) { + return s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if errors.Is(err, meta.ErrNoSuchKey) { + return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + } + if err != nil { + return fmt.Errorf("get object lock config: %w", err) + } + + var bucketLockConfig auth.BucketLockConfig + if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { + return fmt.Errorf("parse bucket lock config: %w", err) + } + + if !bucketLockConfig.Enabled { + return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + } + + return nil +} + func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, config []byte) error { _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { @@ -3681,29 +3725,13 @@ func (p *Posix) GetObjectLockConfiguration(_ context.Context, bucket string) ([] } func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId string, status bool) error { - _, err := os.Stat(bucket) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err := p.doesBucketAndObjectExist(bucket, object) if err != nil { - return fmt.Errorf("stat bucket: %w", err) - } - - cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey) - if errors.Is(err, meta.ErrNoSuchKey) { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } + err = p.isBucketObjectLockEnabled(bucket) if err != nil { - return fmt.Errorf("get object lock config: %w", err) - } - - var bucketLockConfig auth.BucketLockConfig - if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { - return fmt.Errorf("parse bucket lock config: %w", err) - } - - if !bucketLockConfig.Enabled { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } var statusData []byte @@ -3747,12 +3775,13 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId } func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error) { - _, err := os.Stat(bucket) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err := p.doesBucketAndObjectExist(bucket, object) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err + } + err = p.isBucketObjectLockEnabled(bucket) + if err != nil { + return nil, err } if versionId != "" { @@ -3794,29 +3823,13 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId } func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { - _, err := os.Stat(bucket) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err := p.doesBucketAndObjectExist(bucket, object) if err != nil { - return fmt.Errorf("stat bucket: %w", err) - } - - cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey) - if errors.Is(err, meta.ErrNoSuchKey) { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } + err = p.isBucketObjectLockEnabled(bucket) if err != nil { - return fmt.Errorf("get object lock config: %w", err) - } - - var bucketLockConfig auth.BucketLockConfig - if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil { - return fmt.Errorf("parse bucket lock config: %w", err) - } - - if !bucketLockConfig.Enabled { - return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration) + return err } if versionId != "" { @@ -3882,12 +3895,13 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId } func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId string) ([]byte, error) { - _, err := os.Stat(bucket) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err := p.doesBucketAndObjectExist(bucket, object) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err + } + err = p.isBucketObjectLockEnabled(bucket) + if err != nil { + return nil, err } if versionId != "" { diff --git a/s3err/s3err.go b/s3err/s3err.go index 12e2136..e5499ed 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -437,12 +437,12 @@ var errorCodeResponse = map[ErrorCode]APIError{ }, ErrNoSuchObjectLockConfiguration: { Code: "NoSuchObjectLockConfiguration", - Description: "The specified object does not have an ObjectLock configuration.", + Description: "The specified object does not have a ObjectLock configuration.", HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidBucketObjectLockConfiguration: { Code: "InvalidRequest", - Description: "Bucket is missing ObjectLockConfiguration.", + Description: "Bucket is missing Object Lock Configuration.", HTTPStatusCode: http.StatusBadRequest, }, ErrObjectLockConfigurationNotAllowed: { @@ -521,8 +521,9 @@ var errorCodeResponse = map[ErrorCode]APIError{ HTTPStatusCode: http.StatusNotFound, }, ErrInvalidMetadataDirective: { - Code: "InvalidArgument", - Description: "Unknown metadata directive.", + Code: "InvalidArgument", + Description: "Unknown metadata directive.", + HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidVersionId: { Code: "InvalidArgument", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 301b885..dacdfba 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -407,6 +407,7 @@ func TestPutObjectRetention(s *S3Conf) { func TestGetObjectRetention(s *S3Conf) { GetObjectRetention_non_existing_bucket(s) GetObjectRetention_non_existing_object(s) + GetObjectRetention_disabled_lock(s) GetObjectRetention_unset_config(s) GetObjectRetention_success(s) } @@ -424,6 +425,7 @@ func TestPutObjectLegalHold(s *S3Conf) { func TestGetObjectLegalHold(s *S3Conf) { GetObjectLegalHold_non_existing_bucket(s) GetObjectLegalHold_non_existing_object(s) + GetObjectLegalHold_disabled_lock(s) GetObjectLegalHold_unset_config(s) GetObjectLegalHold_success(s) } @@ -877,6 +879,7 @@ func GetIntTests() IntTests { "PutObjectRetention_success": PutObjectRetention_success, "GetObjectRetention_non_existing_bucket": GetObjectRetention_non_existing_bucket, "GetObjectRetention_non_existing_object": GetObjectRetention_non_existing_object, + "GetObjectRetention_disabled_lock": GetObjectRetention_disabled_lock, "GetObjectRetention_unset_config": GetObjectRetention_unset_config, "GetObjectRetention_success": GetObjectRetention_success, "PutObjectLegalHold_non_existing_bucket": PutObjectLegalHold_non_existing_bucket, @@ -888,6 +891,7 @@ func GetIntTests() IntTests { "PutObjectLegalHold_success": PutObjectLegalHold_success, "GetObjectLegalHold_non_existing_bucket": GetObjectLegalHold_non_existing_bucket, "GetObjectLegalHold_non_existing_object": GetObjectLegalHold_non_existing_object, + "GetObjectLegalHold_disabled_lock": GetObjectLegalHold_disabled_lock, "GetObjectLegalHold_unset_config": GetObjectLegalHold_unset_config, "GetObjectLegalHold_success": GetObjectLegalHold_success, "WORMProtection_bucket_object_lock_configuration_compliance_mode": WORMProtection_bucket_object_lock_configuration_compliance_mode, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index f789351..0885ef6 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -8930,6 +8930,29 @@ func GetObjectRetention_non_existing_object(s *S3Conf) error { }) } +func GetObjectRetention_disabled_lock(s *S3Conf) error { + testName := "GetObjectRetention_disabled_lock" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + key := "my-obj" + _, err := putObjects(s3client, []string{key}, bucket) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectRetention(ctx, &s3.GetObjectRetentionInput{ + Bucket: &bucket, + Key: &key, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)); err != nil { + return err + } + + return nil + }) +} + func GetObjectRetention_unset_config(s *S3Conf) error { testName := "GetObjectRetention_unset_config" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -8950,7 +8973,7 @@ func GetObjectRetention_unset_config(s *S3Conf) error { } return nil - }) + }, withLock()) } func GetObjectRetention_success(s *S3Conf) error { @@ -9225,6 +9248,29 @@ func GetObjectLegalHold_non_existing_object(s *S3Conf) error { }) } +func GetObjectLegalHold_disabled_lock(s *S3Conf) error { + testName := "GetObjectLegalHold_disabled_lock" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + key := "my-obj" + _, err := putObjects(s3client, []string{key}, bucket) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectLegalHold(ctx, &s3.GetObjectLegalHoldInput{ + Bucket: &bucket, + Key: &key, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)); err != nil { + return err + } + + return nil + }) +} + func GetObjectLegalHold_unset_config(s *S3Conf) error { testName := "GetObjectLegalHold_unset_config" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -9245,7 +9291,7 @@ func GetObjectLegalHold_unset_config(s *S3Conf) error { } return nil - }) + }, withLock()) } func GetObjectLegalHold_success(s *S3Conf) error {