Skip to content

Commit cd1ea94

Browse files
ichuckyiichuckyi
authored
Merge pull request #516 from awslabs/bucket-sniping-fix
Backend fix
2 parents d7099e4 + 031ccd6 commit cd1ea94

File tree

13 files changed

+107
-46
lines changed

13 files changed

+107
-46
lines changed

‎cloudformation/template.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,6 +1253,7 @@ Resources:
12531253
AdminsGroupName: !Join ['', [!Ref 'AWS::StackName', 'AdminsGroup']]
12541254
RegisteredGroupName: !Sub '${AWS::StackName}-RegisteredGroup'
12551255
DevelopmentMode: !Ref DevelopmentMode
1256+
SourceAccount: !Ref AWS::AccountId
12561257
# Adds the API as a trigger
12571258
Events:
12581259
ProxyApiRoot:
@@ -1714,6 +1715,7 @@ Resources:
17141715
Environment:
17151716
Variables:
17161717
BucketName: !Ref ArtifactsS3BucketName
1718+
SourceAccount: !Ref AWS::AccountId
17171719
Layers:
17181720
- !Ref LambdaCommonLayer
17191721

@@ -1738,6 +1740,7 @@ Resources:
17381740
Environment:
17391741
Variables:
17401742
StaticBucketName: !Ref ArtifactsS3BucketName
1743+
SourceAccount: !Ref AWS::AccountId
17411744
Layers:
17421745
- !Ref LambdaCommonLayer
17431746

@@ -2074,6 +2077,7 @@ Resources:
20742077
RegisteredGroup: !Ref CognitoRegisteredGroup
20752078
CustomersTable: !Ref CustomersTable
20762079
FeedbackTable: !If [EnableFeedbackSubmission, !Ref FeedbackTable, !Ref AWS::NoValue]
2080+
SourceAccount: !Ref AWS::AccountId
20772081

20782082

20792083
Outputs:

‎lambdas/backend/__tests__/routes/admin/catalog/sdkGeneration.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,15 @@ describe('idempotentSdkGenerationUpdate', () => {
6767

6868
process.env.StaticBucketName = 'staticBucketName'
6969
process.env.CatalogUpdaterFunctionArn = 'somebigfunctionarn'
70+
process.env.SourceAccount = '123412341234'
7071

7172
await sdkGeneration.idempotentSdkGenerationUpdate(true, 'apiid_stagename', res)
7273

7374
expect(util.s3.getObject).toHaveBeenCalledTimes(1)
7475
expect(util.s3.getObject).toHaveBeenCalledWith({
7576
Bucket: 'staticBucketName',
76-
Key: 'sdkGeneration.json'
77+
Key: 'sdkGeneration.json',
78+
ExpectedBucketOwner: '123412341234'
7779
})
7880

7981
expect(util.s3.upload).toHaveBeenCalledTimes(1)
@@ -83,7 +85,8 @@ describe('idempotentSdkGenerationUpdate', () => {
8385
Body: JSON.stringify({
8486
apiid_stagename: true,
8587
otherapiid_otherstagename: false
86-
})
88+
}),
89+
ExpectedBucketOwner: '123412341234'
8790
})
8891

8992
expect(util.lambda.invoke).toHaveBeenCalledTimes(1)
@@ -111,13 +114,15 @@ describe('idempotentSdkGenerationUpdate', () => {
111114

112115
process.env.StaticBucketName = 'staticBucketName'
113116
process.env.CatalogUpdaterFunctionArn = 'somebigfunctionarn'
117+
process.env.SourceAccount = '123412341234'
114118

115119
await sdkGeneration.idempotentSdkGenerationUpdate(true, 'apiid_stagename', res)
116120

117121
expect(util.s3.getObject).toHaveBeenCalledTimes(1)
118122
expect(util.s3.getObject).toHaveBeenCalledWith({
119123
Bucket: 'staticBucketName',
120-
Key: 'sdkGeneration.json'
124+
Key: 'sdkGeneration.json',
125+
ExpectedBucketOwner: '123412341234'
121126
})
122127

123128
expect(util.s3.upload).toHaveBeenCalledTimes(1)
@@ -127,7 +132,8 @@ describe('idempotentSdkGenerationUpdate', () => {
127132
Body: JSON.stringify({
128133
otherapiid_otherstagename: false,
129134
apiid_stagename: true
130-
})
135+
}),
136+
ExpectedBucketOwner: '123412341234'
131137
})
132138

133139
expect(util.lambda.invoke).toHaveBeenCalledTimes(1)
@@ -156,13 +162,15 @@ describe('idempotentSdkGenerationUpdate', () => {
156162

157163
process.env.StaticBucketName = 'staticBucketName'
158164
process.env.CatalogUpdaterFunctionArn = 'somebigfunctionarn'
165+
process.env.SourceAccount = '123412341234'
159166

160167
await sdkGeneration.idempotentSdkGenerationUpdate(false, 'apiid_stagename', res)
161168

162169
expect(util.s3.getObject).toHaveBeenCalledTimes(1)
163170
expect(util.s3.getObject).toHaveBeenCalledWith({
164171
Bucket: 'staticBucketName',
165-
Key: 'sdkGeneration.json'
172+
Key: 'sdkGeneration.json',
173+
ExpectedBucketOwner: '123412341234'
166174
})
167175

168176
expect(util.s3.upload).toHaveBeenCalledTimes(0)

‎lambdas/backend/__tests__/routes/admin/catalog/visibility.js‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ describe('POST /admin/catalog/visibility', () => {
405405
util.lambda.invoke = jest.fn().mockReturnValue(promiser())
406406

407407
process.env.StaticBucketName = 'myBucket'
408+
process.env.SourceAccount = '123412341234'
408409

409410
await adminCatalogVisibility.post(req, mockResponseObject)
410411

@@ -422,7 +423,8 @@ describe('POST /admin/catalog/visibility', () => {
422423
Key: 'catalog/a1b2c3_prod.json',
423424
Body: Buffer.from(JSON.stringify({
424425
info: { title: 'swagger document' }
425-
}))
426+
})),
427+
ExpectedBucketOwner: '123412341234'
426428
})
427429

428430
expect(mockResponseObject.status).toHaveBeenCalledWith(200)
@@ -443,6 +445,7 @@ describe('POST /admin/catalog/visibility', () => {
443445
util.lambda.invoke = jest.fn().mockReturnValue(promiser())
444446

445447
process.env.StaticBucketName = 'myBucket'
448+
process.env.SourceAccount = '123412341234'
446449

447450
await adminCatalogVisibility.post(req, mockResponseObject)
448451

@@ -460,7 +463,8 @@ describe('POST /admin/catalog/visibility', () => {
460463
Key: 'catalog/unsubscribable_a1b2c3_prod.json',
461464
Body: Buffer.from(JSON.stringify({
462465
info: { title: 'swagger document' }
463-
}))
466+
})),
467+
ExpectedBucketOwner: '123412341234'
464468
})
465469

466470
expect(mockResponseObject.status).toHaveBeenCalledWith(200)
@@ -475,13 +479,15 @@ describe('POST /admin/catalog/visibility', () => {
475479
util.lambda.invoke = jest.fn().mockReturnValue(promiser())
476480

477481
process.env.StaticBucketName = 'myPail'
482+
process.env.SourceAccount = '123412341234'
478483

479484
await adminCatalogVisibility.post(req, mockResponseObject)
480485

481486
expect(util.s3.upload).toHaveBeenCalledWith({
482487
Bucket: 'myPail',
483488
Key: `catalog/${hash({ info: { title: 'swagger document' } })}.json`,
484-
Body: Buffer.from(JSON.stringify({ info: { title: 'swagger document' } }))
489+
Body: Buffer.from(JSON.stringify({ info: { title: 'swagger document' } })),
490+
ExpectedBucketOwner: '123412341234'
485491
})
486492

487493
expect(mockResponseObject.status).toHaveBeenCalledWith(200)
@@ -531,16 +537,19 @@ describe('DELETE /admin/catalog/visibility/:id', () => {
531537
util.s3.deleteObject = jest.fn().mockReturnValue(promiser())
532538

533539
process.env.StaticBucketName = 'myOtherBucket'
540+
process.env.SourceAccount = '123412341234'
534541

535542
await adminCatalogVisibility.delete(req, mockResponseObject)
536543

537544
expect(util.s3.deleteObject).toHaveBeenCalledWith({
538545
Bucket: 'myOtherBucket',
539-
Key: 'catalog/unsubscribable_a1b2c3_prod.json'
546+
Key: 'catalog/unsubscribable_a1b2c3_prod.json',
547+
ExpectedBucketOwner: '123412341234'
540548
})
541549
expect(util.s3.deleteObject).toHaveBeenCalledWith({
542550
Bucket: 'myOtherBucket',
543-
Key: 'catalog/a1b2c3_prod.json'
551+
Key: 'catalog/a1b2c3_prod.json',
552+
ExpectedBucketOwner: '123412341234'
544553
})
545554

546555
expect(mockResponseObject.status).toHaveBeenCalledWith(200)
@@ -554,16 +563,19 @@ describe('DELETE /admin/catalog/visibility/:id', () => {
554563
util.s3.deleteObject = jest.fn().mockReturnValue(promiser())
555564

556565
process.env.StaticBucketName = 'myOtherBucket'
566+
process.env.SourceAccount = '123412341234'
557567

558568
await adminCatalogVisibility.delete(req, mockResponseObject)
559569

560570
expect(util.s3.deleteObject).toHaveBeenCalledWith({
561571
Bucket: 'myOtherBucket',
562-
Key: 'catalog/unsubscribable_a1b2c3_unmatched.json'
572+
Key: 'catalog/unsubscribable_a1b2c3_unmatched.json',
573+
ExpectedBucketOwner: '123412341234'
563574
})
564575
expect(util.s3.deleteObject).toHaveBeenCalledWith({
565576
Bucket: 'myOtherBucket',
566-
Key: 'catalog/a1b2c3_unmatched.json'
577+
Key: 'catalog/a1b2c3_unmatched.json',
578+
ExpectedBucketOwner: '123412341234'
567579
})
568580

569581
expect(mockResponseObject.status).toHaveBeenCalledWith(200)
@@ -577,12 +589,14 @@ describe('DELETE /admin/catalog/visibility/:id', () => {
577589
util.s3.deleteObject = jest.fn().mockReturnValue(promiser())
578590

579591
process.env.StaticBucketName = 'anotherBucket'
592+
process.env.SourceAccount = '123412341234'
580593

581594
await adminCatalogVisibility.delete(req, mockResponseObject)
582595

583596
expect(util.s3.deleteObject).toHaveBeenCalledWith({
584597
Bucket: 'anotherBucket',
585-
Key: 'catalog/somebighash123456.json'
598+
Key: 'catalog/somebighash123456.json',
599+
ExpectedBucketOwner: '123412341234'
586600
})
587601

588602
expect(mockResponseObject.status).toHaveBeenCalledWith(200)

‎lambdas/backend/__tests__/util.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,13 @@ describe('backend/util', () => {
160160
util.s3.getObject = jest.fn().mockReturnValue(promiser(response))
161161

162162
setEnv('StaticBucketName', 'test-bucket')
163+
setEnv('SourceAccount', '123412341234')
163164

164165
const result = await util.catalog()
165166

166167
expect(util.s3.getObject).toBeCalledTimes(1)
167168
expect(util.s3.getObject).toBeCalledWith(
168-
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json' })
169+
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json', ExpectedBucketOwner: '123412341234' })
169170
)
170171
expect(result).toMatchObject(catalog)
171172
})
@@ -182,6 +183,7 @@ describe('backend/util', () => {
182183
util.s3.getObject = jest.fn().mockReturnValue(promiser(null, response))
183184

184185
setEnv('StaticBucketName', 'test-bucket')
186+
setEnv('SourceAccount', '123412341234')
185187

186188
// Suppress error messages - we're expecting an error here.
187189
console.error = () => {}
@@ -190,7 +192,7 @@ describe('backend/util', () => {
190192

191193
expect(util.s3.getObject).toBeCalledTimes(1)
192194
expect(util.s3.getObject).toBeCalledWith(
193-
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json' })
195+
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json', ExpectedBucketOwner: '123412341234' })
194196
)
195197
expect(result).toMatchObject(catalog)
196198
})
@@ -203,6 +205,7 @@ describe('backend/util', () => {
203205
util.s3.getObject = jest.fn().mockReturnValue(promiser(null, response))
204206

205207
setEnv('StaticBucketName', 'test-bucket')
208+
setEnv('SourceAccount', '123412341234')
206209

207210
// Suppress error messages - we're expecting an error here.
208211
console.error = () => {}
@@ -211,7 +214,7 @@ describe('backend/util', () => {
211214

212215
expect(util.s3.getObject).toBeCalledTimes(1)
213216
expect(util.s3.getObject).toBeCalledWith(
214-
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json' })
217+
expect.objectContaining({ Bucket: 'test-bucket', Key: 'catalog.json', ExpectedBucketOwner: '123412341234' })
215218
)
216219
})
217220
})

‎lambdas/backend/routes/admin/catalog/sdkGeneration.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ exports.idempotentSdkGenerationUpdate = async (parity, id, res) => {
1919
const sdkGeneration =
2020
JSON.parse((await util.s3.getObject({
2121
Bucket: process.env.StaticBucketName,
22-
Key: 'sdkGeneration.json'
22+
Key: 'sdkGeneration.json',
23+
ExpectedBucketOwner: process.env.SourceAccount
2324
}).promise()).Body)
2425

2526
if (sdkGeneration[id] !== parity) {
@@ -28,7 +29,8 @@ exports.idempotentSdkGenerationUpdate = async (parity, id, res) => {
2829
await util.s3.upload({
2930
Bucket: process.env.StaticBucketName,
3031
Key: 'sdkGeneration.json',
31-
Body: JSON.stringify(sdkGeneration)
32+
Body: JSON.stringify(sdkGeneration),
33+
ExpectedBucketOwner: process.env.SourceAccount
3234
}).promise()
3335

3436
// call catalogUpdater to build a fresh catalog.json that includes changes from sdkGeneration.json

‎lambdas/backend/routes/admin/catalog/visibility.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,14 @@ async function uploadFile (file, body) {
156156
console.log('upload bucket: ', process.env.StaticBucketName)
157157
console.log('upload key: ', file)
158158
console.log('upload body length: ', body.byteLength)
159-
await util.s3.upload({ Bucket: process.env.StaticBucketName, Key: file, Body: body }).promise()
159+
await util.s3.upload({ Bucket: process.env.StaticBucketName, Key: file, Body: body, ExpectedBucketOwner: process.env.SourceAccount }).promise()
160160
await catalogUpdate()
161161
}
162162

163163
async function deleteFile (file) {
164164
console.log('remove bucket: ', process.env.StaticBucketName)
165165
console.log('remove key: ', file)
166-
await util.s3.deleteObject({ Bucket: process.env.StaticBucketName, Key: file }).promise()
166+
await util.s3.deleteObject({ Bucket: process.env.StaticBucketName, Key: file, ExpectedBucketOwner: process.env.SourceAccount }).promise()
167167
await catalogUpdate()
168168
}
169169

‎lambdas/backend/util.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ exports.catalog = () => {
7777
// function when the user updates the catalog. This led to confusing behavior, so I removed it.
7878
const params = {
7979
Bucket: process.env.StaticBucketName,
80-
Key: 'catalog.json'
80+
Key: 'catalog.json',
81+
ExpectedBucketOwner: process.env.SourceAccount
8182
}
8283

8384
console.log(`params: ${JSON.stringify(params, null, 4)}`)

‎lambdas/catalog-updater/__tests__/catalog-updater-test.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,15 @@ describe('handler', () => {
203203
setMock(index.s3, 'getObject', opts => promiser({ Body: Buffer.from(JSON.stringify(files[opts.Key])) }))
204204
setMock(index.s3, 'upload', () => promiser())
205205
setEnv('BucketName', 'TestBucket')
206+
setEnv('SourceAccount', '123412341234')
206207

207208
await index.handler({})
208209
expect(index.s3.upload).toBeCalledWith({
209210
Bucket: 'TestBucket',
210211
Key: 'catalog.json',
211212
Body: JSON.stringify(expectedCatalog),
212-
ContentType: 'application/json'
213+
ContentType: 'application/json',
214+
ExpectedBucketOwner: '123412341234'
213215
})
214216
})
215217
})

‎lambdas/catalog-updater/index.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ function swaggerFileFilter (file) {
5050
async function getSwaggerFile (file) {
5151
const params = {
5252
Bucket: bucketName,
53-
Key: file.Key
53+
Key: file.Key,
54+
ExpectedBucketOwner: process.env.SourceAccount
5455
}
5556

5657
const s3Repr = await exports.s3.getObject(params).promise()
@@ -190,7 +191,7 @@ async function handler (event, context) {
190191
bucketName = process.env.BucketName
191192

192193
const sdkGeneration = JSON.parse(
193-
(await exports.s3.getObject({ Bucket: bucketName, Key: 'sdkGeneration.json' }).promise())
194+
(await exports.s3.getObject({ Bucket: bucketName, Key: 'sdkGeneration.json', ExpectedBucketOwner: process.env.SourceAccount }).promise())
194195
.Body.toString()
195196
)
196197
console.log(`sdkGeneration: ${inspectStringify(sdkGeneration)}`)
@@ -207,8 +208,8 @@ async function handler (event, context) {
207208
while (true) {
208209
const listObjectsResult = await exports.s3.listObjectsV2(
209210
token != null
210-
? { Bucket: bucketName, Prefix: 'catalog/', ContinuationToken: token }
211-
: { Bucket: bucketName, Prefix: 'catalog/' }
211+
? { Bucket: bucketName, Prefix: 'catalog/', ContinuationToken: token, ExpectedBucketOwner: process.env.SourceAccount }
212+
: { Bucket: bucketName, Prefix: 'catalog/', ExpectedBucketOwner: process.env.SourceAccount }
212213
).promise()
213214

214215
for (const file of listObjectsResult.Contents) {
@@ -234,7 +235,8 @@ async function handler (event, context) {
234235
Bucket: bucketName,
235236
Key: 'catalog.json',
236237
Body: JSON.stringify(catalog),
237-
ContentType: 'application/json'
238+
ContentType: 'application/json',
239+
ExpectedBucketOwner: process.env.SourceAccount
238240
}
239241

240242
await exports.s3.upload(params).promise()

0 commit comments

Comments
 (0)