Skip to content

Commit bd3c40f

Browse files
committed
Merge remote-tracking branch 'origin/bugfix/CLDSRV-1040-part-copy-destination-authorization' into w/9.4/bugfix/CLDSRV-1040-part-copy-destination-authorization
# Conflicts: # lib/api/apiUtils/authorization/prepareRequestContexts.js # tests/unit/api/apiUtils/authorization/prepareRequestContexts.js # tests/unit/api/objectCopyPart.js
2 parents d7f7551 + 29b6231 commit bd3c40f

3 files changed

Lines changed: 141 additions & 4 deletions

File tree

‎lib/api/apiUtils/authorization/prepareRequestContexts.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject,
9898
objectGetAction,
9999
's3',
100100
);
101-
const putRequestContext = generateRequestContext('objectPut');
101+
// Same API method as the one checked on the destination by the API
102+
const objectPutAction = apiMethodAfterVersionCheck === 'objectPutCopyPart' ? 'objectPutPart' : 'objectPut';
103+
const putRequestContext = generateRequestContext(objectPutAction);
102104
requestContexts.push(getRequestContext, putRequestContext);
103105
if (apiMethodAfterVersionCheck === 'objectCopy') {
104106
// if tagging directive is COPY, "s3:PutObjectTagging" don't need

‎tests/unit/api/apiUtils/authorization/prepareRequestContexts.js‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ describe('prepareRequestContexts', () => {
4242
);
4343

4444
it(
45-
'should return s3:PutObjectVersion request context action for objectPut method with empty x-scal-s3-version-id' +
46-
' header',
45+
'should return s3:PutObjectVersion request context action for objectPut method' +
46+
' with empty x-scal-s3-version-id header',
4747
() => {
4848
const apiMethod = 'objectPut';
4949
const request = makeRequest({
@@ -154,6 +154,38 @@ describe('prepareRequestContexts', () => {
154154
},
155155
);
156156

157+
// The API checks the authorization results by API method, so the request
158+
// contexts must carry the API methods it checks, not only the right actions
159+
[
160+
{ apiMethod: 'objectCopy', destApiMethod: 'objectPut' },
161+
{ apiMethod: 'objectPutCopyPart', destApiMethod: 'objectPutPart' },
162+
].forEach(({ apiMethod, destApiMethod }) => {
163+
it(`should authorize the destination of ${apiMethod} as ${destApiMethod}`, () => {
164+
const request = makeRequest({});
165+
const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject);
166+
167+
assert.deepStrictEqual(
168+
results.map(rc => rc._apiMethod),
169+
['objectGet', destApiMethod],
170+
);
171+
assert.deepStrictEqual(
172+
results.map(rc => rc.getAction()),
173+
['s3:GetObject', 's3:PutObject'],
174+
);
175+
});
176+
177+
it(`should authorize a specific source version of ${apiMethod} as objectGetVersion`, () => {
178+
const request = makeRequest({});
179+
const results = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId);
180+
181+
assert.deepStrictEqual(
182+
results.map(rc => rc._apiMethod),
183+
['objectGetVersion', destApiMethod],
184+
);
185+
assert.strictEqual(results[0].getQuery().versionId, sourceVersionId);
186+
});
187+
});
188+
157189
it('should return s3:PutObjectRetention with header x-amz-object-lock-mode', () => {
158190
const apiMethod = 'objectPut';
159191
const request = makeRequest({

‎tests/unit/api/objectCopyPart.js‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ const assert = require('assert');
22
const async = require('async');
33
const sinon = require('sinon');
44
const { parseString } = require('xml2js');
5-
const { storage } = require('arsenal');
5+
const { auth, storage } = require('arsenal');
6+
const api = require('../../../lib/api/api');
67
const { bucketPut } = require('../../../lib/api/bucketPut');
78
const objectPut = require('../../../lib/api/objectPut');
89
const objectPutCopyPart = require('../../../lib/api/objectPutCopyPart');
@@ -564,3 +565,105 @@ describe('objectPutCopyPart with checksums disabled', () => {
564565
});
565566
});
566567
});
568+
569+
describe('objectCopyPart through api.callApiMethod', () => {
570+
const userAuthInfo = makeAuthInfo(canonicalID, 'user');
571+
const objData = Buffer.from('foo', 'utf8');
572+
const sandbox = sinon.createSandbox();
573+
let uploadId;
574+
575+
// Vault returns one result per request context, keyed by its API method
576+
function stubDoAuth(deniedAction) {
577+
sandbox.stub(auth, 'server').value({
578+
doAuth: (req, reqLog, cb, awsService, requestContexts) =>
579+
cb(
580+
null,
581+
userAuthInfo,
582+
requestContexts.map(rc => {
583+
const denied = rc.getAction() === deniedAction;
584+
return { isAllowed: !denied, isImplicit: denied, action: rc._apiMethod };
585+
}),
586+
),
587+
});
588+
}
589+
590+
function _createCopyPartApiRequest() {
591+
return new DummyRequest({
592+
bucketName: destBucketName,
593+
namespace,
594+
objectKey,
595+
headers: {
596+
host: `${destBucketName}.s3.amazonaws.com`,
597+
'x-amz-copy-source': `${sourceBucketName}/${objectKey}`,
598+
},
599+
url: `/${objectKey}?partNumber=1&uploadId=${uploadId}`,
600+
query: {
601+
partNumber: '1',
602+
uploadId,
603+
},
604+
});
605+
}
606+
607+
before(done => {
608+
cleanup();
609+
async.waterfall(
610+
[
611+
callback => bucketPut(authInfo, _createBucketPutRequest(destBucketName), log, err => callback(err)),
612+
callback => bucketPut(authInfo, _createBucketPutRequest(sourceBucketName), log, err => callback(err)),
613+
callback =>
614+
objectPut(
615+
authInfo,
616+
versioningTestUtils.createPutObjectRequest(sourceBucketName, objectKey, objData),
617+
undefined,
618+
log,
619+
err => callback(err),
620+
),
621+
callback =>
622+
objectPut(
623+
authInfo,
624+
versioningTestUtils.createPutObjectRequest(destBucketName, objectKey, objData),
625+
undefined,
626+
log,
627+
err => callback(err),
628+
),
629+
callback =>
630+
initiateMultipartUpload(userAuthInfo, _createInitiateRequest(destBucketName), log, (err, res) =>
631+
callback(err, res),
632+
),
633+
],
634+
(err, res) => {
635+
if (err) {
636+
return done(err);
637+
}
638+
return parseString(res, (err, json) => {
639+
uploadId = json.InitiateMultipartUploadResult.UploadId[0];
640+
return done(err);
641+
});
642+
},
643+
);
644+
});
645+
646+
afterEach(() => {
647+
sandbox.restore();
648+
});
649+
650+
after(() => {
651+
cleanup();
652+
});
653+
654+
it('should copy the part when IAM allows every action', done => {
655+
stubDoAuth(null);
656+
api.callApiMethod('objectPutCopyPart', _createCopyPartApiRequest(), {}, log, err => {
657+
assert.ifError(err);
658+
done();
659+
});
660+
});
661+
662+
it('should deny the copy into an existing key when IAM denies s3:PutObject', done => {
663+
stubDoAuth('s3:PutObject');
664+
api.callApiMethod('objectPutCopyPart', _createCopyPartApiRequest(), {}, log, err => {
665+
assert(err && err.is.AccessDenied, `expected AccessDenied, got ${err}`);
666+
done();
667+
});
668+
});
669+
});

0 commit comments

Comments
 (0)