Skip to content

Commit

Permalink
fix: s3 delete objects improve validation
Browse files Browse the repository at this point in the history
  • Loading branch information
fenos committed Jun 20, 2024
1 parent 8483e05 commit ac6b230
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 22 deletions.
9 changes: 9 additions & 0 deletions src/storage/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export enum ErrorCode {
DatabaseError = 'DatabaseError',
MissingContentLength = 'MissingContentLength',
MissingParameter = 'MissingParameter',
InvalidParameter = 'InvalidParameter',
InvalidUploadSignature = 'InvalidUploadSignature',
LockTimeout = 'LockTimeout',
S3Error = 'S3Error',
Expand Down Expand Up @@ -89,6 +90,14 @@ export const ERRORS = {
originalError: e,
}),

InvalidParameter: (parameter: string, e?: Error) =>
new StorageBackendError({
code: ErrorCode.MissingParameter,
httpStatusCode: 400,
message: `Invalid Parameter ${parameter}`,
originalError: e,
}),

InvalidJWT: (e?: Error) =>
new StorageBackendError({
code: ErrorCode.InvalidJWT,
Expand Down
37 changes: 16 additions & 21 deletions src/storage/protocols/s3/s3-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,8 @@ export class S3ProtocolHandler {
throw ERRORS.MissingParameter('Delete')
}

if (!Delete.Objects) {
throw ERRORS.MissingParameter('Objects')
if (!Array.isArray(Delete.Objects)) {
throw ERRORS.InvalidParameter('Objects')
}

if (Delete.Objects.length === 0) {
Expand All @@ -903,28 +903,23 @@ export class S3ProtocolHandler {
.from(Bucket)
.deleteObjects(Delete.Objects.map((o) => o.Key || ''))

const deleted = Delete.Objects.filter((o) => deletedResult.find((d) => d.name === o.Key)).map(
(o) => ({ Key: o.Key })
)

const errors = Delete.Objects.filter((o) => !deletedResult.find((d) => d.name === o.Key)).map(
(o) => ({
Key: o.Key,
Code: 'AccessDenied',
Message: "You do not have permission to delete this object or the object doesn't exists",
})
)

return {
responseBody: {
DeletedResult: {
Deleted: Delete.Objects.map((o) => {
const isDeleted = deletedResult.find((d) => d.name === o.Key)
if (isDeleted) {
return {
Deleted: {
Key: o.Key,
},
}
}

return {
Error: {
Key: o.Key,
Code: 'AccessDenied',
Message:
"You do not have permission to delete this object or the object doesn't exists",
},
}
}),
Deleted: deleted,
Error: errors,
},
},
}
Expand Down
65 changes: 64 additions & 1 deletion src/test/s3-protocol.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,70 @@ describe('S3 Protocol', () => {
},
})

await client.send(deleteObjectsCommand)
const deleteResp = await client.send(deleteObjectsCommand)

expect(deleteResp.Deleted).toEqual([
{
Key: 'test-1.jpg',
},
{
Key: 'test-2.jpg',
},
{
Key: 'test-3.jpg',
},
])

const listObjectsCommand = new ListObjectsV2Command({
Bucket: bucketName,
})

const resp = await client.send(listObjectsCommand)
expect(resp.Contents).toBe(undefined)
})

it('try to delete multiple objects that dont exists', async () => {
const bucketName = await createBucket(client)

await uploadFile(client, bucketName, 'test-1.jpg', 1)

const deleteObjectsCommand = new DeleteObjectsCommand({
Bucket: bucketName,
Delete: {
Objects: [
{
Key: 'test-1.jpg',
},
{
Key: 'test-2.jpg',
},
{
Key: 'test-3.jpg',
},
],
},
})

const deleteResp = await client.send(deleteObjectsCommand)
expect(deleteResp.Deleted).toEqual([
{
Key: 'test-1.jpg',
},
])
expect(deleteResp.Errors).toEqual([
{
Key: 'test-2.jpg',
Code: 'AccessDenied',
Message:
"You do not have permission to delete this object or the object doesn't exists",
},
{
Key: 'test-3.jpg',
Code: 'AccessDenied',
Message:
"You do not have permission to delete this object or the object doesn't exists",
},
])

const listObjectsCommand = new ListObjectsV2Command({
Bucket: bucketName,
Expand Down

0 comments on commit ac6b230

Please sign in to comment.