Merge pull request #1575 from versity/sis/locked-objects-overwrite

fix: fixes locked objects overwrite in versioning-enabled buckets
This commit is contained in:
Ben McClelland
2025-10-09 10:55:47 -07:00
committed by GitHub
8 changed files with 275 additions and 14 deletions
+10 -1
View File
@@ -210,7 +210,16 @@ func ParseObjectLegalHoldOutput(status *bool) *s3response.GetObjectLegalHoldResu
}
}
func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects []types.ObjectIdentifier, bypass, isBucketPublic bool, be backend.Backend) error {
func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects []types.ObjectIdentifier, bypass, isBucketPublic bool, be backend.Backend, isOverwrite bool) error {
if isOverwrite {
// if bucket versioning is enabled, any overwrite request
// should be enabled, as it leads to a new object version
// creation
res, err := be.GetBucketVersioning(ctx, bucket)
if err == nil && res.Status != nil && *res.Status == types.BucketVersioningStatusEnabled {
return nil
}
}
data, err := be.GetObjectLockConfiguration(ctx, bucket)
if err != nil {
if errors.Is(err, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound)) {
+1 -1
View File
@@ -67,7 +67,7 @@ func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) (*Response, error) {
}, s3err.GetAPIError(s3err.ErrInvalidRequest)
}
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, dObj.Objects, bypass, IsBucketPublic, c.be)
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, dObj.Objects, bypass, IsBucketPublic, c.be, false)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
+1
View File
@@ -159,6 +159,7 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
bypass,
isBucketPublic,
c.be,
false,
)
if err != nil {
return &Response{
+1 -1
View File
@@ -629,7 +629,7 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []types.ObjectIdentifier{{Key: &key}}, true, IsBucketPublic, c.be)
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []types.ObjectIdentifier{{Key: &key}}, true, IsBucketPublic, c.be, true)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
+3
View File
@@ -1174,6 +1174,9 @@ func TestS3ApiController_PutObject(t *testing.T) {
GetObjectLockConfigurationFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return nil, tt.input.extraMockErr
},
GetBucketVersioningFunc: func(contextMoqParam context.Context, bucket string) (s3response.GetBucketVersioningOutput, error) {
return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented)
},
}
ctrl := S3ApiController{
+6
View File
@@ -1017,6 +1017,9 @@ func TestVersioning(ts *TestState) {
ts.Run(Versioning_WORM_obj_version_locked_with_legal_hold)
ts.Run(Versioning_WORM_obj_version_locked_with_governance_retention)
ts.Run(Versioning_WORM_obj_version_locked_with_compliance_retention)
ts.Run(Versioning_WORM_PutObject_overwrite_locked_object)
ts.Run(Versioning_WORM_CopyObject_overwrite_locked_object)
ts.Run(Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object)
// Concurrent requests
// Versioninig_concurrent_upload_object
ts.Run(Versioning_AccessControl_GetObjectVersion)
@@ -1612,6 +1615,9 @@ func GetIntTests() IntTests {
"Versioning_WORM_obj_version_locked_with_legal_hold": Versioning_WORM_obj_version_locked_with_legal_hold,
"Versioning_WORM_obj_version_locked_with_governance_retention": Versioning_WORM_obj_version_locked_with_governance_retention,
"Versioning_WORM_obj_version_locked_with_compliance_retention": Versioning_WORM_obj_version_locked_with_compliance_retention,
"Versioning_WORM_PutObject_overwrite_locked_object": Versioning_WORM_PutObject_overwrite_locked_object,
"Versioning_WORM_CopyObject_overwrite_locked_object": Versioning_WORM_CopyObject_overwrite_locked_object,
"Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object": Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object,
"Versioning_AccessControl_GetObjectVersion": Versioning_AccessControl_GetObjectVersion,
"Versioning_AccessControl_HeadObjectVersion": Versioning_AccessControl_HeadObjectVersion,
"Versioning_concurrent_upload_object": Versioning_concurrent_upload_object,
+249
View File
@@ -24060,6 +24060,255 @@ func Versioning_WORM_obj_version_locked_with_compliance_retention(s *S3Conf) err
}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
}
func Versioning_WORM_PutObject_overwrite_locked_object(s *S3Conf) error {
testName := "Versioning_WORM_PutObject_overwrite_locked_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
versions, err := createObjVersions(s3client, bucket, obj, 1)
if err != nil {
return err
}
v := versions[0]
v.IsLatest = getPtr(false)
// lock the object with legal hold
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
Bucket: &bucket,
Key: &obj,
LegalHold: &types.ObjectLockLegalHold{
Status: types.ObjectLockLegalHoldStatusOn,
},
})
cancel()
if err != nil {
return err
}
dataLen := int64(10)
// overwrite the locked object with a new version
r, err := putObjectWithData(dataLen, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
if err != nil {
return err
}
version := types.ObjectVersion{
ETag: r.res.ETag,
IsLatest: getPtr(true),
Key: &obj,
Size: &dataLen,
VersionId: r.res.VersionId,
StorageClass: types.ObjectVersionStorageClassStandard,
ChecksumType: r.res.ChecksumType,
}
result := []types.ObjectVersion{version, v}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
if !compareVersions(result, out.Versions) {
return fmt.Errorf("expected the object versions to be %v, instead got %v", result, out.Versions)
}
return cleanupLockedObjects(s3client, bucket, []objToDelete{
{
key: obj,
versionId: getString(v.VersionId),
removeOnlyLeglHold: true,
},
})
}, withLock())
}
func Versioning_WORM_CopyObject_overwrite_locked_object(s *S3Conf) error {
testName := "Versioning_WORM_CopyObject_overwrite_locked_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
versions, err := createObjVersions(s3client, bucket, obj, 1)
if err != nil {
return err
}
v := versions[0]
v.IsLatest = getPtr(false)
// lock the object with legal hold
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
Bucket: &bucket,
Key: &obj,
LegalHold: &types.ObjectLockLegalHold{
Status: types.ObjectLockLegalHoldStatusOn,
},
})
cancel()
if err != nil {
return err
}
// create a source object version
srcObj := "source-object"
srcVersions, err := createObjVersions(s3client, bucket, srcObj, 1)
if err != nil {
return err
}
srcVersion := srcVersions[0]
// overwrite the locked object with a new version with CopyObject
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
copyResult, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &bucket,
Key: &obj,
CopySource: getPtr(fmt.Sprintf("%s/%s", bucket, srcObj)),
})
cancel()
if err != nil {
return err
}
version := types.ObjectVersion{
ETag: copyResult.CopyObjectResult.ETag,
IsLatest: getPtr(true),
Key: &obj,
Size: srcVersion.Size,
VersionId: copyResult.VersionId,
StorageClass: types.ObjectVersionStorageClassStandard,
ChecksumType: copyResult.CopyObjectResult.ChecksumType,
}
result := []types.ObjectVersion{version, v, srcVersion}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
if !compareVersions(result, out.Versions) {
return fmt.Errorf("expected the object versions to be %v, instead got %v", result, out.Versions)
}
return cleanupLockedObjects(s3client, bucket, []objToDelete{
{
key: obj,
versionId: getString(v.VersionId),
removeOnlyLeglHold: true,
},
})
}, withLock())
}
func Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object(s *S3Conf) error {
testName := "Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
versions, err := createObjVersions(s3client, bucket, obj, 1)
if err != nil {
return err
}
v := versions[0]
v.IsLatest = getPtr(false)
// lock the object with legal hold
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
Bucket: &bucket,
Key: &obj,
LegalHold: &types.ObjectLockLegalHold{
Status: types.ObjectLockLegalHoldStatusOn,
},
})
cancel()
if err != nil {
return err
}
dataLen := int64(5 * 1024 * 1024)
// overwrite the locked object with a new version
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
parts, _, err := uploadParts(s3client, dataLen, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
part := parts[0]
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
Bucket: &bucket,
Key: &obj,
MultipartUpload: &types.CompletedMultipartUpload{
Parts: []types.CompletedPart{
{
ETag: part.ETag,
PartNumber: part.PartNumber,
ChecksumCRC64NVME: part.ChecksumCRC64NVME,
},
},
},
UploadId: mp.UploadId,
})
cancel()
if err != nil {
return err
}
version := types.ObjectVersion{
ETag: res.ETag,
IsLatest: getPtr(true),
Key: &obj,
Size: &dataLen,
VersionId: res.VersionId,
StorageClass: types.ObjectVersionStorageClassStandard,
ChecksumType: res.ChecksumType,
}
result := []types.ObjectVersion{version, v}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
if !compareVersions(result, out.Versions) {
return fmt.Errorf("expected the object versions to be %v, instead got %v", result, out.Versions)
}
return cleanupLockedObjects(s3client, bucket, []objToDelete{
{
key: obj,
versionId: getString(v.VersionId),
removeOnlyLeglHold: true,
},
})
}, withLock())
}
func Versioning_AccessControl_GetObjectVersion(s *S3Conf) error {
testName := "Versioning_AccessControl_GetObjectVersion"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+4 -11
View File
@@ -127,13 +127,10 @@ test_get_put_object_legal_hold_s3api_root() {
echo "fdkljafajkfs" > "$TEST_FILE_FOLDER/$bucket_file"
run put_object_with_user "s3api" "$TEST_FILE_FOLDER/$bucket_file" "$BUCKET_ONE_NAME" "$bucket_file" "$username" "$password"
assert_failure
assert_output --partial "object protected by object lock"
assert_success
run delete_object_with_user "s3api" "$BUCKET_ONE_NAME" "$bucket_file" "$username" "$password"
assert_failure 1
# shellcheck disable=SC2154
assert_output --partial "object protected by object lock"
assert_success
run put_object_legal_hold "s3api" "$BUCKET_ONE_NAME" "$bucket_file" "OFF"
assert_success
@@ -171,14 +168,10 @@ test_get_put_object_retention_s3api_root() {
echo "fdkljafajkfs" > "$TEST_FILE_FOLDER/$bucket_file"
run put_object_with_user "s3api" "$TEST_FILE_FOLDER/$bucket_file" "$BUCKET_ONE_NAME" "$bucket_file" "$username" "$secret_key"
assert_failure
# shellcheck disable=SC2154
assert_output --partial "object protected by object lock"
assert_success
run delete_object_with_user "s3api" "$BUCKET_ONE_NAME" "$bucket_file" "$username" "$secret_key"
assert_failure
# shellcheck disable=SC2154
assert_output --partial "object protected by object lock"
assert_success
}
test_retention_bypass_s3api_root() {