Files
versitygw/s3api/controllers/object-delete_test.go
T
niksis02 008b8a49ba fix: return NoSuchUpload for malformed upload IDs ahead of access and request checks
Fixes #2533

`UploadPartCopy` and `CopyObject` returned `500 InternalError` when a non-root user copied from a source bucket with no stored ACL (such as a posix bucket directory created outside the gateway): `VerifyObjectCopyAccess` ran `json.Unmarshal` on the empty ACL bytes posix returns for such a bucket. It now parses them with `ParseACL`, like every other ACL reader.

The source checks also ran before the backend looked at the upload ID, so a malformed `uploadId` got `AccessDenied`, `NoSuchBucket` or `InvalidArgument` instead of `NoSuchUpload`. S3 rejects an upload ID that is malformed, or bound to another key, right after the bucket existence and expected-bucket-owner checks, before authorization and nearly all request validation. That applies to every multipart operation that takes an upload ID.

`Backend` gains `ValidateUploadID`, a format-only check that doesn't look up the upload. `BackendUnsupported` accepts every ID, so s3proxy and azure are unchanged, and posix's existing canonical UUID check becomes the method. `UploadPart`, `UploadPartCopy` and `AbortMultipartUpload` call it first. `ListParts` calls it after rejecting a `part-number-marker` or `max-parts` that isn't an integer (new `ValidateLimiterInteger`); negative values are still rejected after it. `CompleteMultipartUpload` calls it after rejecting unsupported `If-Match`/`If-None-Match` combinations, which are split out of `EvaluateObjectPutPreconditions` as `ValidateObjectPutPreconditionHeaders` and now checked before access verification and body parsing, as on S3.
2026-10-21 19:55:15 +04:00

390 lines
9.1 KiB
Go

// Copyright 2023 Versity Software
// This file is licensed under the Apache License, Version 2.0
// (the "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package controllers
import (
"context"
"net/http"
"testing"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/oklog/ulid/v2"
"github.com/versity/versitygw/s3api/utils"
"github.com/versity/versitygw/s3err"
"github.com/versity/versitygw/s3event"
)
func TestS3ApiController_DeleteObjectTagging(t *testing.T) {
versionId := ulid.Make().String()
tests := []struct {
name string
input testInput
output testOutput
}{
{
name: "empty versionId query param",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
},
},
{
name: "verify access fails",
input: testInput{
locals: accessDeniedLocals,
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "backend returns error",
input: testInput{
queries: map[string]string{
"versionId": versionId,
},
locals: defaultLocals,
beErr: s3err.GetAPIError(s3err.ErrInvalidRequest),
},
output: testOutput{
response: &Response{
Headers: map[string]*string{
"x-amz-version-id": &versionId,
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
EventName: s3event.EventObjectTaggingDelete,
},
},
err: s3err.GetAPIError(s3err.ErrInvalidRequest),
},
},
{
name: "successful response",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": versionId,
},
},
output: testOutput{
response: &Response{
Headers: map[string]*string{
"x-amz-version-id": &versionId,
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
EventName: s3event.EventObjectTaggingDelete,
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
be := &BackendMock{
DeleteObjectTaggingFunc: func(contextMoqParam context.Context, bucket, object, versionId string) error {
return tt.input.beErr
},
GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
},
}
ctrl := S3ApiController{
be: be,
}
testController(
t,
ctrl.DeleteObjectTagging,
tt.output.response,
tt.output.err,
ctxInputs{
locals: tt.input.locals,
queries: tt.input.queries,
})
})
}
}
func TestS3ApiController_AbortMultipartUpload(t *testing.T) {
tests := []struct {
name string
input testInput
output testOutput
}{
{
name: "malformed upload id",
input: testInput{
locals: accessDeniedLocals,
extraMockErr: s3err.GetNoSuchUploadErr("bad"),
queries: map[string]string{
"uploadId": "bad",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetNoSuchUploadErr("bad"),
},
},
{
name: "verify access fails",
input: testInput{
locals: accessDeniedLocals,
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "backend returns error",
input: testInput{
locals: defaultLocals,
beErr: s3err.GetAPIError(s3err.ErrInvalidRequest),
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
},
},
err: s3err.GetAPIError(s3err.ErrInvalidRequest),
},
},
{
name: "successful response",
input: testInput{
locals: defaultLocals,
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
be := &BackendMock{
ValidateUploadIDFunc: func(uploadID string) error {
return tt.input.extraMockErr
},
AbortMultipartUploadFunc: func(contextMoqParam context.Context, abortMultipartUploadInput *s3.AbortMultipartUploadInput) error {
return tt.input.beErr
},
GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
},
}
ctrl := S3ApiController{
be: be,
}
testController(
t,
ctrl.AbortMultipartUpload,
tt.output.response,
tt.output.err,
ctxInputs{
locals: tt.input.locals,
queries: tt.input.queries,
})
})
}
}
func TestS3ApiController_DeleteObject(t *testing.T) {
delMarker, versionId := true, "versionId"
var emptyRes *s3.DeleteObjectOutput
tests := []struct {
name string
input testInput
output testOutput
}{
{
name: "empty versionId query param",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
},
},
{
name: "If-Match with a versionId is rejected ahead of authorization",
input: testInput{
locals: accessDeniedLocals,
queries: map[string]string{
"versionId": "versionId",
},
headers: map[string]string{
"If-Match": "etag",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete),
},
},
{
name: "verify access fails",
input: testInput{
locals: accessDeniedLocals,
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "object locked",
input: testInput{
locals: defaultLocals,
extraMockErr: s3err.GetAPIError(s3err.ErrObjectLocked),
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrObjectLocked),
},
},
{
name: "backend returns error",
input: testInput{
locals: defaultLocals,
beErr: s3err.GetAPIError(s3err.ErrInvalidRequest),
extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
beRes: emptyRes,
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
EventName: s3event.EventObjectRemovedDelete,
},
},
err: s3err.GetAPIError(s3err.ErrInvalidRequest),
},
},
{
name: "successful response",
input: testInput{
locals: defaultLocals,
extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
beRes: &s3.DeleteObjectOutput{
DeleteMarker: &delMarker,
VersionId: &versionId,
},
},
output: testOutput{
response: &Response{
Headers: map[string]*string{
"x-amz-delete-marker": utils.GetStringPtr("true"),
"x-amz-version-id": &versionId,
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
Status: http.StatusNoContent,
EventName: s3event.EventObjectRemovedDelete,
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
be := &BackendMock{
DeleteObjectFunc: func(contextMoqParam context.Context, deleteObjectInput *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
return tt.input.beRes.(*s3.DeleteObjectOutput), tt.input.beErr
},
GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
},
GetObjectLockConfigurationFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return nil, tt.input.extraMockErr
},
}
ctrl := S3ApiController{
be: be,
}
testController(
t,
ctrl.DeleteObject,
tt.output.response,
tt.output.err,
ctxInputs{
locals: tt.input.locals,
queries: tt.input.queries,
headers: tt.input.headers,
})
})
}
}