Skip to content

Commit 4e014c6

Browse files
tmacrotmacro
committed
impr(CLDSRV-621): Add tests for bucketOwnerId
1 parent 914d8d2 commit 4e014c6

3 files changed

Lines changed: 194 additions & 2 deletions

File tree

‎tests/unit/api/objectCopy.js‎

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const sinon = require('sinon');
55

66
const { bucketPut } = require('../../../lib/api/bucketPut');
77
const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning');
8+
const bucketPutPolicy = require('../../../lib/api/bucketPutPolicy');
89
const objectPut = require('../../../lib/api/objectPut');
910
const objectCopy = require('../../../lib/api/objectCopy');
1011
const DummyRequest = require('../DummyRequest');
@@ -45,6 +46,7 @@ function _createObjectCopyRequest(destBucketName) {
4546
objectKey,
4647
headers: {},
4748
url: `/${destBucketName}/${objectKey}`,
49+
socket: {},
4850
};
4951
return new DummyRequest(params);
5052
}
@@ -68,6 +70,7 @@ describe('objectCopy with versioning', () => {
6870

6971
before(done => {
7072
cleanup();
73+
sinon.spy(metadata, 'putObjectMD');
7174
async.series([
7275
callback => bucketPut(authInfo, putDestBucketRequest, log,
7376
callback),
@@ -96,7 +99,10 @@ describe('objectCopy with versioning', () => {
9699
});
97100
});
98101

99-
after(() => cleanup());
102+
after(() => {
103+
metadata.putObjectMD.restore();
104+
cleanup();
105+
});
100106

101107
it('should delete null version when creating new null version, ' +
102108
'even when null version is not the latest version', done => {
@@ -125,6 +131,94 @@ describe('objectCopy with versioning', () => {
125131
});
126132
});
127133
});
134+
135+
it('should not set bucketOwnerId if requesting account owns dest bucket', done => {
136+
const testObjectCopyRequest = _createObjectCopyRequest(destBucketName);
137+
objectCopy(authInfo, testObjectCopyRequest, sourceBucketName, objectKey,
138+
undefined, log, err => {
139+
assert.ifError(err);
140+
sinon.assert.calledWith(
141+
metadata.putObjectMD.lastCall,
142+
destBucketName,
143+
objectKey,
144+
sinon.match({ _data: { bucketOwnerId: sinon.match.typeOf('undefined') }}),
145+
sinon.match.any,
146+
sinon.match.any,
147+
sinon.match.any
148+
);
149+
done();
150+
});
151+
});
152+
153+
// TODO: S3C-9965
154+
// Skipped because the policy is not checked correctly
155+
// When source bucket policy is checked destination arn is used
156+
it.skip('should set bucketOwnerId if requesting account differs from dest bucket owner', done => {
157+
const authInfo2 = makeAuthInfo('accessKey2');
158+
const testObjectCopyRequest = _createObjectCopyRequest(destBucketName);
159+
const testPutSrcPolicyRequest = new DummyRequest({
160+
bucketName: sourceBucketName,
161+
namespace,
162+
headers: { host: `${sourceBucketName}.s3.amazonaws.com` },
163+
url: '/',
164+
socket: {},
165+
post: JSON.stringify({
166+
Version: '2012-10-17',
167+
Statement: [
168+
{
169+
Sid: 'AllowCrossAccountRead',
170+
Effect: 'Allow',
171+
Principal: { AWS: `arn:aws:iam::${authInfo2.shortid}:root` },
172+
Action: ['s3:GetObject'],
173+
Resource: [
174+
`arn:aws:s3:::${sourceBucketName}/*`
175+
],
176+
},
177+
],
178+
}),
179+
});
180+
const testPutDestPolicyRequest = new DummyRequest({
181+
bucketName: destBucketName,
182+
namespace,
183+
headers: { host: `${destBucketName}.s3.amazonaws.com` },
184+
url: '/',
185+
socket: {},
186+
post: JSON.stringify({
187+
Version: '2012-10-17',
188+
Statement: [
189+
{
190+
Sid: 'AllowCrossAccountWrite',
191+
Effect: 'Allow',
192+
Principal: { AWS: `arn:aws:iam::${authInfo2.shortid}:root` },
193+
Action: ['s3:PutObject'],
194+
Resource: [
195+
`arn:aws:s3:::${destBucketName}/*`
196+
],
197+
},
198+
],
199+
}),
200+
});
201+
bucketPutPolicy(authInfo, testPutSrcPolicyRequest, log, err => {
202+
assert.ifError(err);
203+
bucketPutPolicy(authInfo, testPutDestPolicyRequest, log, err => {
204+
assert.ifError(err);
205+
objectCopy(authInfo2, testObjectCopyRequest, sourceBucketName, objectKey,
206+
undefined, log, err => {
207+
sinon.assert.calledWith(
208+
metadata.putObjectMD.lastCall,
209+
destBucketName,
210+
objectKey,
211+
sinon.match({_data: { bucketOwnerId: authInfo.canonicalID }}),
212+
sinon.match.any,
213+
sinon.match.any,
214+
sinon.match.any
215+
);
216+
assert.ifError(err);
217+
done();
218+
});
219+
});
220+
});
221+
});
128222
});
129223

130224
describe('non-versioned objectCopy', () => {

‎tests/unit/api/objectCopyPart.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,21 @@ describe('objectCopyPart', () => {
137137
done();
138138
});
139139
});
140+
141+
it('should set owner-id to the canonicalId of the dest bucket owner', done => {
142+
const testObjectCopyRequest = _createObjectCopyPartRequest(destBucketName, uploadId);
143+
objectPutCopyPart(authInfo, testObjectCopyRequest, sourceBucketName, objectKey, undefined, log, err => {
144+
assert.ifError(err);
145+
sinon.assert.calledWith(
146+
metadataswitch.putObjectMD.lastCall,
147+
sinon.match.string, // MPU shadow bucket
148+
`${uploadId}..|..00001`,
149+
sinon.match({ 'owner-id': authInfo.canonicalID }),
150+
sinon.match.any,
151+
sinon.match.any,
152+
sinon.match.any
153+
);
154+
done();
155+
});
156+
});
140157
});

‎tests/unit/api/objectPut.js‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { bucketPut } = require('../../../lib/api/bucketPut');
88
const bucketPutObjectLock = require('../../../lib/api/bucketPutObjectLock');
99
const bucketPutACL = require('../../../lib/api/bucketPutACL');
1010
const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning');
11+
const bucketPutPolicy = require('../../../lib/api/bucketPutPolicy');
1112
const { parseTagFromQuery } = s3middleware.tagging;
1213
const { cleanup, DummyRequestLogger, makeAuthInfo, versioningTestUtils }
1314
= require('../helpers');
@@ -582,7 +583,7 @@ describe('objectPut API', () => {
582583
data.implName = 'multipleBackends';
583584

584585
const originalPut = data.client.put;
585-
data.client.put = (hashedStream, valueSize, keyContext, backendInfo, log, cb) =>
586+
data.client.put = (hashedStream, valueSize, keyContext, backendInfo, log, cb) =>
586587
cb({ httpCode: 408 });
587588
bucketPut(authInfo, testPutBucketRequest, log, () => {
588589
objectPut(authInfo, testPutObjectRequest, undefined, log,
@@ -762,6 +763,86 @@ describe('objectPut API', () => {
762763
},
763764
], done);
764765
});
766+
767+
it('should not set bucketOwnerId if requester owns the bucket', done => {
768+
const testPutObjectRequest = new DummyRequest({
769+
bucketName,
770+
namespace,
771+
objectKey: objectName,
772+
headers: {},
773+
url: `/${bucketName}/${objectName}`,
774+
calculatedHash: 'vnR+tLdVF79rPPfF+7YvOg==',
775+
}, postBody);
776+
777+
bucketPut(authInfo, testPutBucketRequest, log, () => {
778+
objectPut(authInfo, testPutObjectRequest, undefined, log,
779+
err => {
780+
assert.ifError(err);
781+
sinon.assert.calledWith(metadata.putObjectMD.lastCall,
782+
bucketName,
783+
objectName,
784+
sinon.match({ bucketOwnerId: sinon.match.typeOf('undefined') }),
785+
any,
786+
any,
787+
any
788+
);
789+
done();
790+
}
791+
);
792+
});
793+
});
794+
795+
it('should set bucketOwnerId if requester does not own the bucket', done => {
796+
const authInfo2 = makeAuthInfo('accessKey2');
797+
798+
const testPutObjectRequest = new DummyRequest({
799+
bucketName,
800+
namespace,
801+
objectKey: objectName,
802+
headers: {},
803+
url: `/${bucketName}/${objectName}`,
804+
calculatedHash: 'vnR+tLdVF79rPPfF+7YvOg==',
805+
}, postBody);
806+
807+
const testPutPolicyRequest = new DummyRequest({
808+
bucketName,
809+
namespace,
810+
headers: { host: `${bucketName}.s3.amazonaws.com` },
811+
url: '/',
812+
post: JSON.stringify({
813+
Version: '2012-10-17',
814+
Statement: [
815+
{
816+
Sid: 'AllowCrossAccountReadWrite',
817+
Effect: 'Allow',
818+
Principal: { AWS: `arn:aws:iam::${authInfo2.shortid}:root` },
819+
Action: ['s3:PutObject'],
820+
Resource: [`arn:aws:s3:::${bucketName}/*`],
821+
},
822+
],
823+
}),
824+
});
825+
826+
bucketPut(authInfo, testPutBucketRequest, log, () => {
827+
bucketPutPolicy(authInfo, testPutPolicyRequest, log, err => {
828+
assert.ifError(err);
829+
objectPut(authInfo, testPutObjectRequest, undefined, log,
830+
err => {
831+
assert.ifError(err);
832+
sinon.assert.calledWith(metadata.putObjectMD.lastCall,
833+
bucketName,
834+
objectName,
835+
sinon.match({ bucketOwnerId: authInfo.canonicalId }),
836+
any,
837+
any,
838+
any
839+
);
840+
done();
841+
}
842+
);
843+
});
844+
});
845+
});
765846
});
766847

767848
describe('objectPut API with versioning', () => {

0 commit comments

Comments
 (0)