From 15ddc02c253cecef275edea94626dfc5293b4cb5 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Wed, 13 Sep 2023 18:14:05 +0200 Subject: [PATCH 1/8] CLDSRV-431: implment other API impDeny logic --- lib/api/apiUtils/object/abortMultipartUpload.js | 11 +++++++++-- lib/api/bucketHead.js | 2 +- lib/api/completeMultipartUpload.js | 7 ++++++- lib/api/initiateMultipartUpload.js | 2 +- lib/api/listMultipartUploads.js | 2 +- lib/api/listParts.js | 7 ++++++- lib/api/objectCopy.js | 9 +++++++-- lib/api/objectHead.js | 2 +- lib/api/websiteHead.js | 4 ++-- lib/routes/routeBackbeat.js | 4 +++- 10 files changed, 37 insertions(+), 13 deletions(-) diff --git a/lib/api/apiUtils/object/abortMultipartUpload.js b/lib/api/apiUtils/object/abortMultipartUpload.js index 8112c7e47d..24688ddb10 100644 --- a/lib/api/apiUtils/object/abortMultipartUpload.js +++ b/lib/api/apiUtils/object/abortMultipartUpload.js @@ -22,10 +22,11 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, // but the requestType is the more general 'objectDelete' const metadataValParams = Object.assign({}, metadataValMPUparams); metadataValParams.requestType = 'objectPut'; + const authzIdentityResult = request ? request.iamAuthzResults : true; async.waterfall([ function checkDestBucketVal(next) { - metadataValidateBucketAndObj(metadataValParams, log, + metadataValidateBucketAndObj(metadataValParams, authzIdentityResult, log, (err, destinationBucket) => { if (err) { return next(err, destinationBucket); @@ -56,9 +57,15 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket, next) { const location = mpuOverviewObj.controllingLocationConstraint; + const originalIdentityAuthzResults = request.iamAuthzResults; + // eslint-disable-next-line no-param-reassign + // eslint-disable-next-line no-param-reassign + delete request.iamAuthzResults; return data.abortMPU(objectKey, uploadId, location, bucketName, - request, destBucket, locationConstraintCheck, log, + request, destBucket, locationConstraintCheck, log, (err, skipDataDelete) => { + // eslint-disable-next-line no-param-reassign + request.iamAuthzResults = originalIdentityAuthzResults; if (err) { return next(err, destBucket); } diff --git a/lib/api/bucketHead.js b/lib/api/bucketHead.js index a4eb17e7d7..815689b527 100644 --- a/lib/api/bucketHead.js +++ b/lib/api/bucketHead.js @@ -21,7 +21,7 @@ function bucketHead(authInfo, request, log, callback) { requestType: 'bucketHead', request, }; - metadataValidateBucket(metadataValParams, log, (err, bucket) => { + metadataValidateBucket(metadataValParams, request.iamAuthzResults, log, (err, bucket) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); if (err) { diff --git a/lib/api/completeMultipartUpload.js b/lib/api/completeMultipartUpload.js index eb73dd53e8..fa0bcdfbcd 100644 --- a/lib/api/completeMultipartUpload.js +++ b/lib/api/completeMultipartUpload.js @@ -120,7 +120,7 @@ function completeMultipartUpload(authInfo, request, log, callback) { // at the destinationBucket level are same as objectPut requestType: 'objectPut', }; - metadataValidateBucketAndObj(metadataValParams, log, next); + metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, next); }, function validateMultipart(destBucket, objMD, next) { if (objMD) { @@ -190,9 +190,14 @@ function completeMultipartUpload(authInfo, request, log, callback) { const mdInfo = { storedParts, mpuOverviewKey, splitter }; const mpuInfo = { objectKey, uploadId, jsonList, bucketName, destBucket }; + const originalIdentityAuthzResults = request.iamAuthzResults; + // eslint-disable-next-line no-param-reassign + delete request.iamAuthzResults; return data.completeMPU(request, mpuInfo, mdInfo, location, null, null, null, locationConstraintCheck, log, (err, completeObjData) => { + // eslint-disable-next-line no-param-reassign + request.iamAuthzResults = originalIdentityAuthzResults; if (err) { return next(err, destBucket); } diff --git a/lib/api/initiateMultipartUpload.js b/lib/api/initiateMultipartUpload.js index 7dd7c01785..2947f9dc16 100644 --- a/lib/api/initiateMultipartUpload.js +++ b/lib/api/initiateMultipartUpload.js @@ -253,7 +253,7 @@ function initiateMultipartUpload(authInfo, request, log, callback) { } async.waterfall([ - next => metadataValidateBucketAndObj(metadataValParams, log, + next => metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, (error, destinationBucket) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destinationBucket); diff --git a/lib/api/listMultipartUploads.js b/lib/api/listMultipartUploads.js index 502363dfb0..471d2b72d0 100644 --- a/lib/api/listMultipartUploads.js +++ b/lib/api/listMultipartUploads.js @@ -104,7 +104,7 @@ function listMultipartUploads(authInfo, request, log, callback) { function waterfall1(next) { // Check final destination bucket for authorization rather // than multipart upload bucket - metadataValidateBucket(metadataValParams, log, + metadataValidateBucket(metadataValParams, request.iamAuthzResults, log, (err, bucket) => next(err, bucket)); }, function getMPUBucket(bucket, next) { diff --git a/lib/api/listParts.js b/lib/api/listParts.js index 44cb5c53e2..3a66808b8a 100644 --- a/lib/api/listParts.js +++ b/lib/api/listParts.js @@ -112,7 +112,7 @@ function listParts(authInfo, request, log, callback) { async.waterfall([ function checkDestBucketVal(next) { - metadataValidateBucketAndObj(metadataValParams, log, + metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, (err, destinationBucket) => { if (err) { return next(err, destinationBucket, null); @@ -150,8 +150,13 @@ function listParts(authInfo, request, log, callback) { mpuOverviewObj, destBucket, }; + const originalIdentityAuthzResults = request.iamAuthzResults; + // eslint-disable-next-line no-param-reassign + delete request.iamAuthzResults; return data.listParts(mpuInfo, request, locationConstraintCheck, log, (err, backendPartList) => { + // eslint-disable-next-line no-param-reassign + request.iamAuthzResults = originalIdentityAuthzResults; if (err) { return next(err, destBucket); } diff --git a/lib/api/objectCopy.js b/lib/api/objectCopy.js index 9e0302d46a..56fef30863 100644 --- a/lib/api/objectCopy.js +++ b/lib/api/objectCopy.js @@ -245,7 +245,7 @@ function objectCopy(authInfo, request, sourceBucket, } return async.waterfall([ function checkDestAuth(next) { - return metadataValidateBucketAndObj(valPutParams, log, + return metadataValidateBucketAndObj(valPutParams, request.iamAuthzResults, log, (err, destBucketMD, destObjMD) => { if (err) { log.debug('error validating put part of request', @@ -263,7 +263,7 @@ function objectCopy(authInfo, request, sourceBucket, }); }, function checkSourceAuthorization(destBucketMD, destObjMD, next) { - return metadataValidateBucketAndObj(valGetParams, log, + return metadataValidateBucketAndObj(valGetParams, request.iamAuthzResults, log, (err, sourceBucketMD, sourceObjMD) => { if (err) { log.debug('error validating get part of request', @@ -408,10 +408,15 @@ function objectCopy(authInfo, request, sourceBucket, return next(null, storeMetadataParams, dataLocator, destObjMD, serverSideEncryption, destBucketMD); } + const originalIdentityAuthzResults = request.iamAuthzResults; + // eslint-disable-next-line no-param-reassign + delete request.iamAuthzResults; return data.copyObject(request, sourceLocationConstraintName, storeMetadataParams, dataLocator, dataStoreContext, backendInfoDest, sourceBucketMD, destBucketMD, serverSideEncryption, log, (err, results) => { + // eslint-disable-next-line no-param-reassign + request.iamAuthzResults = originalIdentityAuthzResults; if (err) { return next(err, destBucketMD); } diff --git a/lib/api/objectHead.js b/lib/api/objectHead.js index 738b9df259..362b828e8d 100644 --- a/lib/api/objectHead.js +++ b/lib/api/objectHead.js @@ -48,7 +48,7 @@ function objectHead(authInfo, request, log, callback) { request, }; - return metadataValidateBucketAndObj(mdValParams, log, + return metadataValidateBucketAndObj(mdValParams, request.iamAuthzResults, log, (err, bucket, objMD) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); diff --git a/lib/api/websiteHead.js b/lib/api/websiteHead.js index 44f2f62a47..59c4cec023 100644 --- a/lib/api/websiteHead.js +++ b/lib/api/websiteHead.js @@ -104,7 +104,7 @@ function websiteHead(request, log, callback) { { error: err }); let returnErr = err; const bucketAuthorized = isBucketAuthorized(bucket, - 'bucketGet', constants.publicId, null, log, request); + 'bucketGet', constants.publicId, null, request.iamAuthzResults, log, request); // if index object does not exist and bucket is private AWS // returns 403 - AccessDenied error. if (err.is.NoSuchKey && !bucketAuthorized) { @@ -114,7 +114,7 @@ function websiteHead(request, log, callback) { reqObjectKey, corsHeaders, log, callback); } if (!isObjAuthorized(bucket, objMD, 'objectGet', - constants.publicId, null, log, request)) { + constants.publicId, null, request.iamAuthzResults, log, request)) { const err = errors.AccessDenied; log.trace('request not authorized', { error: err }); return _errorActions(err, routingRules, reqObjectKey, diff --git a/lib/routes/routeBackbeat.js b/lib/routes/routeBackbeat.js index 990a5d14c9..c8d74653bb 100644 --- a/lib/routes/routeBackbeat.js +++ b/lib/routes/routeBackbeat.js @@ -1149,6 +1149,8 @@ function routeBackbeat(clientIP, request, response, log) { // Attach the apiMethod method to the request, so it can used by monitoring in the server // eslint-disable-next-line no-param-reassign request.apiMethod = 'routeBackbeat'; + // eslint-disable-next-line no-param-reassign + request.iamAuthzResults = false; log.debug('routing request', { method: 'routeBackbeat', @@ -1273,7 +1275,7 @@ function routeBackbeat(clientIP, request, response, log) { requestType: 'ReplicateObject', request, }; - return metadataValidateBucketAndObj(mdValParams, log, next); + return metadataValidateBucketAndObj(mdValParams, request.iamAuthzResults, log, next); }, (bucketInfo, objMd, next) => { if (useMultipleBackend) { From 111ecf875b9f756552efd3003893aa32b846996b Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Wed, 13 Sep 2023 18:14:39 +0200 Subject: [PATCH 2/8] CLDSRV-431: updatye other API impDeny test logic --- tests/multipleBackend/multipartUpload.js | 8 ++++ tests/unit/api/bucketACLauth.js | 42 ++++++++--------- tests/unit/api/bucketHead.js | 1 + tests/unit/api/deleteMarker.js | 4 ++ tests/unit/api/deletedFlagBucket.js | 2 + tests/unit/api/listMultipartUploads.js | 9 ++++ tests/unit/api/listParts.js | 6 +++ tests/unit/api/multipartDelete.js | 4 ++ tests/unit/api/multipartUpload.js | 25 ++++++++++ tests/unit/api/objectACLauth.js | 60 ++++++++++++------------ tests/unit/api/objectHead.js | 12 +++++ tests/unit/api/objectReplicationMD.js | 2 + tests/unit/api/transientBucket.js | 2 + tests/unit/utils/bucketEncryption.js | 1 + tests/unit/utils/lifecycleHelpers.js | 1 + tests/unit/utils/mpuUtils.js | 3 ++ 16 files changed, 131 insertions(+), 51 deletions(-) diff --git a/tests/multipleBackend/multipartUpload.js b/tests/multipleBackend/multipartUpload.js index 3c2d512a0f..19b13bc9b1 100644 --- a/tests/multipleBackend/multipartUpload.js +++ b/tests/multipleBackend/multipartUpload.js @@ -54,6 +54,7 @@ const bucketPutRequest = { url: '/', post: '', parsedHost: 'localhost', + iamAuthzResults: false, }; const awsETag = 'be747eb4b75517bf6b3cf7c5fbb62f3a'; @@ -73,6 +74,7 @@ const completeBody = '' + const basicParams = { bucketName, namespace, + iamAuthzResults: false, }; function getObjectGetRequest(objectKey) { @@ -270,6 +272,7 @@ function mpuSetup(location, key, cb) { 'x-amz-meta-scal-location-constraint': location }, url: `/${key}?uploads`, parsedHost: 'localhost', + iamAuthzResults: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, (err, result) => { @@ -342,6 +345,7 @@ describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { 'x-amz-meta-scal-location-constraint': `${awsLocation}` }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', + iamAuthzResults: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -365,6 +369,7 @@ describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { `${awsLocationMismatch}` }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', + iamAuthzResults: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -389,6 +394,7 @@ describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', + iamAuthzResults: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -612,6 +618,7 @@ describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { 'x-amz-meta-scal-location-constraint': awsLocation }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', + iamAuthzResults: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, err => { @@ -712,6 +719,7 @@ describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: {}, + iamAuthzResults: false, }; listMultipartUploads(authInfo, listMpuParams, log, (err, mpuListXml) => { diff --git a/tests/unit/api/bucketACLauth.js b/tests/unit/api/bucketACLauth.js index 3da73f7637..a9e6a8c7f5 100644 --- a/tests/unit/api/bucketACLauth.js +++ b/tests/unit/api/bucketACLauth.js @@ -107,7 +107,7 @@ describe.skip('bucket authorization for bucketGet, bucketHead, ' + } bucket.setCannedAcl(value.canned); const results = requestTypes.map(type => - isBucketAuthorized(bucket, type, value.id, value.auth, log)); + isBucketAuthorized(bucket, type, value.id, value.auth, false, log)); assert.deepStrictEqual(results, value.response); done(); }); @@ -129,13 +129,13 @@ describe('bucket authorization for bucketGetACL', () => { it('should allow access to bucket owner', () => { const result = isBucketAuthorized(bucket, 'bucketGetACL', - ownerCanonicalId, authInfo); + ownerCanonicalId, authInfo, false); assert.strictEqual(result, true); }); it('should allow access to user in bucket owner account', () => { const result = isBucketAuthorized(bucket, 'bucketGetACL', - ownerCanonicalId, userAuthInfo); + ownerCanonicalId, userAuthInfo, false); assert.strictEqual(result, true); }); @@ -158,7 +158,7 @@ describe('bucket authorization for bucketGetACL', () => { orders.forEach(value => { it(`should allow access to ${value.it}`, done => { const noAuthResult = isBucketAuthorized(bucket, 'bucketGetACL', - value.id); + value.id, null, false); assert.strictEqual(noAuthResult, false); if (value.aclParam) { bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]); @@ -166,7 +166,7 @@ describe('bucket authorization for bucketGetACL', () => { bucket.setCannedAcl(value.canned); } const authorizedResult = isBucketAuthorized(bucket, 'bucketGetACL', - value.id, value.auth); + value.id, value.auth, false); assert.strictEqual(authorizedResult, true); done(); }); @@ -188,13 +188,13 @@ describe('bucket authorization for bucketPutACL', () => { it('should allow access to bucket owner', () => { const result = isBucketAuthorized(bucket, 'bucketPutACL', - ownerCanonicalId, authInfo); + ownerCanonicalId, authInfo, false); assert.strictEqual(result, true); }); it('should allow access to user in bucket owner account', () => { const result = isBucketAuthorized(bucket, 'bucketPutACL', - ownerCanonicalId, userAuthInfo); + ownerCanonicalId, userAuthInfo, false); assert.strictEqual(result, true); }); @@ -203,11 +203,11 @@ describe('bucket authorization for bucketPutACL', () => { it('should allow access to account if ' + `account was granted ${value} right`, done => { const noAuthResult = isBucketAuthorized(bucket, 'bucketPutACL', - accountToVet, altAcctAuthInfo); + accountToVet, altAcctAuthInfo, false); assert.strictEqual(noAuthResult, false); bucket.setSpecificAcl(accountToVet, value); const authorizedResult = isBucketAuthorized(bucket, 'bucketPutACL', - accountToVet, altAcctAuthInfo); + accountToVet, altAcctAuthInfo, false); assert.strictEqual(authorizedResult, true); done(); }); @@ -229,13 +229,13 @@ describe('bucket authorization for bucketOwnerAction', () => { it('should allow access to bucket owner', () => { const result = isBucketAuthorized(bucket, 'bucketDeleteCors', - ownerCanonicalId, authInfo); + ownerCanonicalId, authInfo, false); assert.strictEqual(result, true); }); it('should allow access to user in bucket owner account', () => { const result = isBucketAuthorized(bucket, 'bucketDeleteCors', - ownerCanonicalId, userAuthInfo); + ownerCanonicalId, userAuthInfo, false); assert.strictEqual(result, true); }); @@ -257,7 +257,7 @@ describe('bucket authorization for bucketOwnerAction', () => { } bucket.setCannedAcl(value.canned); const result = isBucketAuthorized(bucket, 'bucketDeleteCors', - value.id, value.auth); + value.id, value.auth, false); assert.strictEqual(result, false); done(); }); @@ -279,13 +279,13 @@ describe('bucket authorization for bucketDelete', () => { it('should allow access to bucket owner', () => { const result = isBucketAuthorized(bucket, 'bucketDelete', - ownerCanonicalId, authInfo); + ownerCanonicalId, authInfo, false); assert.strictEqual(result, true); }); it('should allow access to user in bucket owner account', () => { const result = isBucketAuthorized(bucket, 'bucketDelete', - ownerCanonicalId, userAuthInfo); + ownerCanonicalId, userAuthInfo, false); assert.strictEqual(result, true); }); @@ -306,7 +306,7 @@ describe('bucket authorization for bucketDelete', () => { bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]); } bucket.setCannedAcl(value.canned); - const result = isBucketAuthorized(bucket, 'bucketDelete', value.id, value.auth); + const result = isBucketAuthorized(bucket, 'bucketDelete', value.id, value.auth, false); assert.strictEqual(result, false); done(); }); @@ -330,13 +330,13 @@ describe('bucket authorization for objectDelete and objectPut', () => { it('should allow access to bucket owner', () => { const results = requestTypes.map(type => - isBucketAuthorized(bucket, type, ownerCanonicalId, authInfo)); + isBucketAuthorized(bucket, type, ownerCanonicalId, authInfo, false)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to user in bucket owner account', () => { const results = requestTypes.map(type => - isBucketAuthorized(bucket, type, ownerCanonicalId, userAuthInfo)); + isBucketAuthorized(bucket, type, ownerCanonicalId, userAuthInfo, false)); assert.deepStrictEqual(results, [true, true]); }); @@ -361,13 +361,13 @@ describe('bucket authorization for objectDelete and objectPut', () => { it(`should allow access to ${value.it}`, done => { bucket.setCannedAcl(value.canned); const noAuthResults = requestTypes.map(type => - isBucketAuthorized(bucket, type, value.id, value.auth)); + isBucketAuthorized(bucket, type, value.id, value.auth, false)); assert.deepStrictEqual(noAuthResults, value.response); if (value.aclParam) { bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]); } const authResults = requestTypes.map(type => - isBucketAuthorized(bucket, type, accountToVet, altAcctAuthInfo)); + isBucketAuthorized(bucket, type, accountToVet, altAcctAuthInfo, false)); assert.deepStrictEqual(authResults, [true, true]); done(); }); @@ -379,10 +379,10 @@ describe('bucket authorization for objectPutACL and objectGetACL', () => { 'are done at object level', done => { const requestTypes = ['objectPutACL', 'objectGetACL']; const results = requestTypes.map(type => - isBucketAuthorized(bucket, type, accountToVet, altAcctAuthInfo)); + isBucketAuthorized(bucket, type, accountToVet, altAcctAuthInfo, false)); assert.deepStrictEqual(results, [true, true]); const publicUserResults = requestTypes.map(type => - isBucketAuthorized(bucket, type, constants.publicId)); + isBucketAuthorized(bucket, type, constants.publicId, null, false)); assert.deepStrictEqual(publicUserResults, [true, true]); done(); }); diff --git a/tests/unit/api/bucketHead.js b/tests/unit/api/bucketHead.js index 1d71ec6763..a69a5483ae 100644 --- a/tests/unit/api/bucketHead.js +++ b/tests/unit/api/bucketHead.js @@ -14,6 +14,7 @@ const testRequest = { namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + iamAuthzResults: false, }; // TODO CLDSRV-431 remove skip describe.skip('bucketHead API', () => { diff --git a/tests/unit/api/deleteMarker.js b/tests/unit/api/deleteMarker.js index 56e92d158c..0edde69d53 100644 --- a/tests/unit/api/deleteMarker.js +++ b/tests/unit/api/deleteMarker.js @@ -28,6 +28,7 @@ const testPutBucketRequest = new DummyRequest({ namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + iamAuthzResults: false, }); const testDeleteRequest = new DummyRequest({ bucketName, @@ -35,6 +36,7 @@ const testDeleteRequest = new DummyRequest({ objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }); function _createBucketPutVersioningReq(status) { @@ -45,6 +47,7 @@ function _createBucketPutVersioningReq(status) { }, url: '/?versioning', query: { versioning: '' }, + iamAuthzResults: false, }; const xml = '' + @@ -62,6 +65,7 @@ function _createMultiObjectDeleteRequest(numObjects) { }, url: '/?delete', query: { delete: '' }, + iamAuthzResults: false, }; const xml = []; xml.push(''); diff --git a/tests/unit/api/deletedFlagBucket.js b/tests/unit/api/deletedFlagBucket.js index c14200fd6b..051b3bd734 100644 --- a/tests/unit/api/deletedFlagBucket.js +++ b/tests/unit/api/deletedFlagBucket.js @@ -58,11 +58,13 @@ const baseTestRequest = { post: '', headers: { host: `${bucketName}.s3.amazonaws.com` }, query: {}, + iamAuthzResults: false, }; const serviceGetRequest = { parsedHost: 's3.amazonaws.com', headers: { host: 's3.amazonaws.com' }, url: '/', + iamAuthzResults: false, }; const userBucketOwner = 'admin'; diff --git a/tests/unit/api/listMultipartUploads.js b/tests/unit/api/listMultipartUploads.js index c9cfbf2441..276b35344a 100644 --- a/tests/unit/api/listMultipartUploads.js +++ b/tests/unit/api/listMultipartUploads.js @@ -32,6 +32,7 @@ describe.skip('listMultipartUploads API', () => { namespace, headers: {}, url: `/${bucketName}`, + iamAuthzResults: false, }; const testInitiateMPURequest1 = { bucketName, @@ -39,6 +40,7 @@ describe.skip('listMultipartUploads API', () => { objectKey: objectName1, headers: {}, url: `/${bucketName}/${objectName1}?uploads`, + iamAuthzResults: false, }; const testInitiateMPURequest2 = { bucketName, @@ -46,6 +48,7 @@ describe.skip('listMultipartUploads API', () => { objectKey: objectName2, headers: {}, url: `/${bucketName}/${objectName2}?uploads`, + iamAuthzResults: false, }; const testInitiateMPURequest3 = { bucketName, @@ -53,6 +56,7 @@ describe.skip('listMultipartUploads API', () => { objectKey: objectName3, headers: {}, url: `/${bucketName}/${objectName3}?uploads`, + iamAuthzResults: false, }; it('should return the name of the common prefix ' + @@ -65,6 +69,7 @@ describe.skip('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads&delimiter=/&prefix=sub`, query: { delimiter, prefix }, + iamAuthzResults: false, }; async.waterfall([ @@ -94,6 +99,7 @@ describe.skip('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: {}, + iamAuthzResults: false, }; @@ -127,6 +133,7 @@ describe.skip('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'max-uploads': '1' }, + iamAuthzResults: false, }; async.waterfall([ @@ -163,6 +170,7 @@ describe.skip('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'encoding-type': 'url' }, + iamAuthzResults: false, }; async.waterfall([ @@ -195,6 +203,7 @@ describe.skip('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'key-marker': objectName1 }, + iamAuthzResults: false, }; async.waterfall([ diff --git a/tests/unit/api/listParts.js b/tests/unit/api/listParts.js index f286ecb114..cf37258ade 100644 --- a/tests/unit/api/listParts.js +++ b/tests/unit/api/listParts.js @@ -114,6 +114,7 @@ describe.skip('List Parts API', () => { url: `/${uploadKey}?uploadId=${uploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, + iamAuthzResults: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -160,6 +161,7 @@ describe.skip('List Parts API', () => { uploadId, 'encoding-type': 'url', }, + iamAuthzResults: false, }; const urlEncodedObjectKey = '%24makememulti'; @@ -185,6 +187,7 @@ describe.skip('List Parts API', () => { uploadId, 'max-parts': '4', }, + iamAuthzResults: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -226,6 +229,7 @@ describe.skip('List Parts API', () => { uploadId, 'max-parts': '6', }, + iamAuthzResults: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -267,6 +271,7 @@ describe.skip('List Parts API', () => { uploadId, 'part-number-marker': '2', }, + iamAuthzResults: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -312,6 +317,7 @@ describe.skip('List Parts API', () => { 'part-number-marker': '2', 'max-parts': '2', }, + iamAuthzResults: false, }; listParts(authInfo, listRequest, log, (err, xml) => { diff --git a/tests/unit/api/multipartDelete.js b/tests/unit/api/multipartDelete.js index 02c9877b07..2792cb9463 100644 --- a/tests/unit/api/multipartDelete.js +++ b/tests/unit/api/multipartDelete.js @@ -24,6 +24,7 @@ const bucketPutRequest = { namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + iamAuthzResults: false, }; const objectKey = 'testObject'; const initiateRequest = { @@ -32,6 +33,7 @@ const initiateRequest = { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, + iamAuthzResults: false, }; const eastLocation = 'us-east-1'; const westLocation = 'scality-internal-file'; @@ -68,6 +70,7 @@ function _createAndAbortMpu(usEastSetting, fakeUploadID, locationConstraint, partNumber: '1', uploadId, }, + iamAuthzResults: false, }, partBody); const testUploadId = fakeUploadID ? 'nonexistinguploadid' : uploadId; @@ -78,6 +81,7 @@ function _createAndAbortMpu(usEastSetting, fakeUploadID, locationConstraint, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploadId=${testUploadId}`, query: { uploadId: testUploadId }, + iamAuthzResults: false, }; next(null, partRequest, deleteMpuRequest); }, diff --git a/tests/unit/api/multipartUpload.js b/tests/unit/api/multipartUpload.js index 371f5cf140..2534952c53 100644 --- a/tests/unit/api/multipartUpload.js +++ b/tests/unit/api/multipartUpload.js @@ -49,6 +49,7 @@ const bucketPutRequest = { 'xmlns="http://s3.amazonaws.com/doc/2006-03-01/">' + 'scality-internal-mem' + '', + iamAuthzResults: false, }; const lockEnabledBucketRequest = Object.assign({}, bucketPutRequest); lockEnabledBucketRequest.bucketName = lockedBucket; @@ -62,6 +63,7 @@ const initiateRequest = { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, + iamAuthzResults: false, }; const retentionInitiateRequest = Object.assign({}, initiateRequest); retentionInitiateRequest.bucketName = lockedBucket; @@ -82,6 +84,7 @@ const getObjectLockInfoRequest = { namespace, objectKey, headers: { host: `${lockedBucket}.s3.amazonaws.com` }, + iamAuthzResults: false, }; const expectedRetentionConfig = { $: { xmlns: 'http://s3.amazonaws.com/doc/2006-03-01/' }, @@ -106,6 +109,7 @@ function _createPutPartRequest(uploadId, partNumber, partBody) { uploadId, }, calculatedHash, + iamAuthzResults: false, }, partBody); } @@ -128,6 +132,7 @@ function _createCompleteMpuRequest(uploadId, parts) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, + iamAuthzResults: false, }; } @@ -542,6 +547,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, + iamAuthzResults: false, }; const awsVerifiedETag = '"953e9e776f285afc0bfcf1ab4668299d-1"'; @@ -630,6 +636,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, + iamAuthzResults: false, }; const awsVerifiedETag = '"953e9e776f285afc0bfcf1ab4668299d-1"'; @@ -702,6 +709,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { @@ -757,6 +765,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { assert(err.is.MalformedXML); @@ -829,6 +838,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { @@ -887,6 +897,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { assert(err.is.InvalidPart); @@ -958,6 +969,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 3); completeMultipartUpload(authInfo, @@ -1040,6 +1052,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 3); completeMultipartUpload(authInfo, @@ -1122,6 +1135,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1153,6 +1167,7 @@ describe.skip('Multipart Upload API', () => { 'x-amz-acl': 'authenticated-read', }, url: `/${objectKey}?uploads`, + iamAuthzResults: false, }; async.waterfall([ @@ -1221,6 +1236,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1255,6 +1271,7 @@ describe.skip('Multipart Upload API', () => { 'x-amz-grant-read': `emailAddress="${granteeEmail}"`, }, url: `/${objectKey}?uploads`, + iamAuthzResults: false, }; async.waterfall([ @@ -1323,6 +1340,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1375,6 +1393,7 @@ describe.skip('Multipart Upload API', () => { url: `/${objectKey}?uploadId=${testUploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, + iamAuthzResults: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2); multipartDelete(authInfo, deleteRequest, log, err => { @@ -1425,6 +1444,7 @@ describe.skip('Multipart Upload API', () => { url: `/${objectKey}?uploadId=${testUploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: 'non-existent-upload-id' }, + iamAuthzResults: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2); multipartDelete(authInfo, deleteRequest, log, err => { @@ -1504,6 +1524,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1557,6 +1578,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, next); }, @@ -1628,6 +1650,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, next); }, @@ -1698,6 +1721,7 @@ describe.skip('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, + iamAuthzResults: false, }; completeMultipartUpload(authInfo, completeRequest, log, done); }, @@ -1790,6 +1814,7 @@ describe.skip('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, + iamAuthzResults: false, }; // show that second part data is there assert(ds[2]); diff --git a/tests/unit/api/objectACLauth.js b/tests/unit/api/objectACLauth.js index 8199aa4a51..818d9e5c49 100644 --- a/tests/unit/api/objectACLauth.js +++ b/tests/unit/api/objectACLauth.js @@ -47,35 +47,35 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { it('should allow access to object owner', () => { const results = requestTypes.map(type => isObjAuthorized(bucket, object, type, objectOwnerCanonicalId, - authInfo, log)); + authInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to user in object owner account', () => { const results = requestTypes.map(type => isObjAuthorized(bucket, object, type, objectOwnerCanonicalId, - userAuthInfo, log)); + userAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to bucket owner if same account as object owner', () => { const results = requestTypes.map(type => isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, - authInfo, log)); + authInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to anyone if canned public-read ACL', () => { object.acl.Canned = 'public-read'; const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to anyone if canned public-read-write ACL', () => { object.acl.Canned = 'public-read-write'; const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); @@ -83,7 +83,7 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { 'authenticated-read ACL', () => { object.acl.Canned = 'authenticated-read'; const publicResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, constants.publicId, null, log)); + isObjAuthorized(bucket, object, type, constants.publicId, null, false, log)); assert.deepStrictEqual(publicResults, [false, false]); }); @@ -91,7 +91,7 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { 'authenticated-read ACL', () => { object.acl.Canned = 'authenticated-read'; const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); @@ -109,12 +109,12 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { }; const noAuthResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - log)); + false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); altAcctObj.acl.Canned = 'bucket-owner-read'; const authResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - log)); + false, log)); assert.deepStrictEqual(authResults, [true, true]); }); @@ -132,46 +132,46 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { }; const noAuthResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - log)); + false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); altAcctObj.acl.Canned = 'bucket-owner-full-control'; const authResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - log)); + false, log)); assert.deepStrictEqual(authResults, [true, true]); }); it('should allow access to account if ' + 'account was granted FULL_CONTROL', () => { const noAuthResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); object.acl.FULL_CONTROL = [accountToVet]; const authResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(authResults, [true, true]); }); it('should allow access to account if ' + 'account was granted READ right', () => { const noAuthResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); object.acl.READ = [accountToVet]; const authResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(authResults, [true, true]); }); it('should not allow access to public user if private canned ACL', () => { const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [false, false]); }); it('should not allow access to just any user if private canned ACL', () => { const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [false, false]); }); }); @@ -181,11 +181,11 @@ describe.skip('object authorization for objectPut and objectDelete', () => { 'are done at bucket level', () => { const requestTypes = ['objectPut', 'objectDelete']; const results = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); const publicUserResults = requestTypes.map(type => isObjAuthorized(bucket, object, type, constants.publicId, null, - log)); + false, log)); assert.deepStrictEqual(publicUserResults, [true, true]); }); }); @@ -208,14 +208,14 @@ describe.skip('object authorization for objectPutACL and objectGetACL', () => { it('should allow access to object owner', () => { const results = requestTypes.map(type => isObjAuthorized(bucket, object, type, objectOwnerCanonicalId, - authInfo, log)); + authInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); it('should allow access to user in object owner account', () => { const results = requestTypes.map(type => isObjAuthorized(bucket, object, type, objectOwnerCanonicalId, - userAuthInfo, log)); + userAuthInfo, false, log)); assert.deepStrictEqual(results, [true, true]); }); @@ -233,45 +233,45 @@ describe.skip('object authorization for objectPutACL and objectGetACL', () => { }; const noAuthResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - log)); + false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); altAcctObj.acl.Canned = 'bucket-owner-full-control'; const authorizedResults = requestTypes.map(type => isObjAuthorized(bucket, altAcctObj, type, bucketOwnerCanonicalId, authInfo, - null, log)); + false, log)); assert.deepStrictEqual(authorizedResults, [true, true]); }); it('should allow access to account if ' + 'account was granted FULL_CONTROL right', () => { const noAuthResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(noAuthResults, [false, false]); object.acl.FULL_CONTROL = [accountToVet]; const authorizedResults = requestTypes.map(type => - isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, log)); + isObjAuthorized(bucket, object, type, accountToVet, altAcctAuthInfo, false, log)); assert.deepStrictEqual(authorizedResults, [true, true]); }); it('should allow objectPutACL access to account if ' + 'account was granted WRITE_ACP right', () => { const noAuthResult = isObjAuthorized(bucket, object, 'objectPutACL', - accountToVet, altAcctAuthInfo, log); + accountToVet, altAcctAuthInfo, false, log); assert.strictEqual(noAuthResult, false); object.acl.WRITE_ACP = [accountToVet]; const authorizedResult = isObjAuthorized(bucket, object, 'objectPutACL', - accountToVet, altAcctAuthInfo, log); + accountToVet, altAcctAuthInfo, false, log); assert.strictEqual(authorizedResult, true); }); it('should allow objectGetACL access to account if ' + 'account was granted READ_ACP right', () => { const noAuthResult = isObjAuthorized(bucket, object, 'objectGetACL', - accountToVet, altAcctAuthInfo, log); + accountToVet, altAcctAuthInfo, false, log); assert.strictEqual(noAuthResult, false); object.acl.READ_ACP = [accountToVet]; const authorizedResult = isObjAuthorized(bucket, object, 'objectGetACL', - accountToVet, altAcctAuthInfo, log); + accountToVet, altAcctAuthInfo, false, log); assert.strictEqual(authorizedResult, true); }); }); @@ -456,7 +456,7 @@ describe.skip('without object metadata', () => { bucket.setCannedAcl(value.canned); const results = requestTypes.map(type => - isObjAuthorized(bucket, null, type, value.id, authInfoUser, log)); + isObjAuthorized(bucket, null, type, value.id, authInfoUser, false, log)); assert.deepStrictEqual(results, value.response); done(); }); diff --git a/tests/unit/api/objectHead.js b/tests/unit/api/objectHead.js index f715839e89..df4ff97b31 100644 --- a/tests/unit/api/objectHead.js +++ b/tests/unit/api/objectHead.js @@ -29,6 +29,7 @@ const testPutBucketRequest = { namespace, headers: {}, url: `/${bucketName}`, + iamAuthzResults: false, }; const userMetadataKey = 'x-amz-meta-test'; const userMetadataValue = 'some metadata'; @@ -57,6 +58,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: { 'if-modified-since': laterDate }, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -80,6 +82,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: { 'if-unmodified-since': earlierDate }, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -105,6 +108,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: { 'if-match': incorrectMD5 }, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -129,6 +133,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: { 'if-none-match': correctMD5 }, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -151,6 +156,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: { range: 'bytes=1-9' }, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -175,6 +181,7 @@ describe.skip('objectHead API', () => { query: { partNumber: '1', }, + iamAuthzResults: false, }; const customizedInvalidRequestError = errors.InvalidRequest .customizeDescription('Cannot specify both Range header and ' + @@ -202,6 +209,7 @@ describe.skip('objectHead API', () => { query: { partNumber: 'nan', }, + iamAuthzResults: false, }; const customizedInvalidArgumentError = errors.InvalidArgument .customizeDescription('Part number must be a number.'); @@ -226,6 +234,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -246,6 +255,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -269,6 +279,7 @@ describe.skip('objectHead API', () => { namespace, headers: { 'x-amz-bucket-object-lock-enabled': 'true' }, url: `/${bucketName}`, + iamAuthzResults: false, }; const testPutObjectRequestLock = new DummyRequest({ bucketName, @@ -288,6 +299,7 @@ describe.skip('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, + iamAuthzResults: false, }; bucketPut(authInfo, testPutBucketRequestLock, log, () => { diff --git a/tests/unit/api/objectReplicationMD.js b/tests/unit/api/objectReplicationMD.js index 5cd46f3037..4eced54993 100644 --- a/tests/unit/api/objectReplicationMD.js +++ b/tests/unit/api/objectReplicationMD.js @@ -48,6 +48,7 @@ const objectACLReq = { }, url: `/${bucketName}/${keyA}?acl`, query: { acl: '' }, + iamAuthzResults: false, }; // Get an object request with the given key. @@ -185,6 +186,7 @@ function putMPU(key, body, cb) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: postBody, + iamAuthzResults: false, }; return completeMultipartUpload(authInfo, req, log, cb); } diff --git a/tests/unit/api/transientBucket.js b/tests/unit/api/transientBucket.js index da1d662cb0..ff67c05707 100644 --- a/tests/unit/api/transientBucket.js +++ b/tests/unit/api/transientBucket.js @@ -53,11 +53,13 @@ const baseTestRequest = { url: '/', post: '', headers: { host: `${bucketName}.s3.amazonaws.com` }, + iamAuthzResults: false, }; const serviceGetRequest = { parsedHost: 's3.amazonaws.com', headers: { host: 's3.amazonaws.com' }, url: '/', + iamAuthzResults: false, }; const userBucketOwner = 'admin'; diff --git a/tests/unit/utils/bucketEncryption.js b/tests/unit/utils/bucketEncryption.js index 7577689e42..ac36465d75 100644 --- a/tests/unit/utils/bucketEncryption.js +++ b/tests/unit/utils/bucketEncryption.js @@ -29,6 +29,7 @@ function templateRequest(bucketName, { post }) { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post, + iamAuthzResults: false, }; } diff --git a/tests/unit/utils/lifecycleHelpers.js b/tests/unit/utils/lifecycleHelpers.js index 0cbb88d660..e6a86d1aaf 100644 --- a/tests/unit/utils/lifecycleHelpers.js +++ b/tests/unit/utils/lifecycleHelpers.js @@ -4,6 +4,7 @@ function getLifecycleRequest(bucketName, xml) { headers: { host: `${bucketName}.s3.amazonaws.com`, }, + iamAuthzResults: false, }; if (xml) { request.post = xml; diff --git a/tests/unit/utils/mpuUtils.js b/tests/unit/utils/mpuUtils.js index 7fc2193eec..1d32b496fb 100644 --- a/tests/unit/utils/mpuUtils.js +++ b/tests/unit/utils/mpuUtils.js @@ -28,6 +28,7 @@ function createinitiateMPURequest(namespace, bucketName, objectKey) { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, + iamAuthzResults: false, }; return request; @@ -45,6 +46,7 @@ function createPutPartRequest(namespace, bucketName, objectKey, partNumber, test uploadId: testUploadId, }, calculatedHash, + iamAuthzResults: false, }, partBody); return request; @@ -68,6 +70,7 @@ function createCompleteRequest(namespace, bucketName, objectKey, testUploadId) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, + iamAuthzResults: false, }; return request; From 8725888bbd758060da50cb2ce38e61e4328ad9c9 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Wed, 13 Sep 2023 18:21:09 +0200 Subject: [PATCH 3/8] fixup: remove skips --- tests/functional/kmip/serverside_encryption.js | 6 ++---- tests/multipleBackend/multipartUpload.js | 3 +-- tests/multipleBackend/objectCopy.js | 3 +-- tests/multipleBackend/objectPutCopyPart.js | 7 ++----- tests/multipleBackend/objectPutPart.js | 7 ++----- tests/unit/api/bucketACLauth.js | 3 +-- tests/unit/api/bucketDelete.js | 9 +++------ tests/unit/api/bucketHead.js | 3 +-- tests/unit/api/deleteMarker.js | 3 +-- tests/unit/api/deletedFlagBucket.js | 3 +-- tests/unit/api/listMultipartUploads.js | 4 ++-- tests/unit/api/listParts.js | 4 ++-- tests/unit/api/multiObjectDelete.js | 4 ++-- tests/unit/api/multipartDelete.js | 4 ++-- tests/unit/api/multipartUpload.js | 11 +++++------ tests/unit/api/objectACLauth.js | 16 ++++++++-------- tests/unit/api/objectCopy.js | 6 ++---- tests/unit/api/objectCopyPart.js | 4 ++-- tests/unit/api/objectDelete.js | 3 +-- tests/unit/api/objectHead.js | 4 ++-- tests/unit/api/objectPut.js | 3 +-- tests/unit/api/objectReplicationMD.js | 7 +++---- tests/unit/api/serviceGet.js | 4 ++-- tests/utapi/awsNodeSdk.js | 3 +-- 24 files changed, 50 insertions(+), 74 deletions(-) diff --git a/tests/functional/kmip/serverside_encryption.js b/tests/functional/kmip/serverside_encryption.js index 2ef5a75736..c92587aca4 100644 --- a/tests/functional/kmip/serverside_encryption.js +++ b/tests/functional/kmip/serverside_encryption.js @@ -162,8 +162,7 @@ describe('KMIP backed server-side encryption', () => { }); }); - // TODO CLDSRV-431 remove skip - it.skip('should allow object copy with SSE header in encrypted bucket', done => { + it('should allow object copy with SSE header in encrypted bucket', done => { async.waterfall([ next => _createBucket(bucketName, false, err => next(err)), next => _putObject(bucketName, objectName, false, err => next(err)), @@ -176,8 +175,7 @@ describe('KMIP backed server-side encryption', () => { done(); }); }); - // TODO CLDSRV-431 remove skip - it.skip('should allow creating mpu with SSE header ' + + it('should allow creating mpu with SSE header ' + 'in encrypted bucket', done => { async.waterfall([ next => _createBucket(bucketName, true, err => next(err)), diff --git a/tests/multipleBackend/multipartUpload.js b/tests/multipleBackend/multipartUpload.js index 19b13bc9b1..29d1948185 100644 --- a/tests/multipleBackend/multipartUpload.js +++ b/tests/multipleBackend/multipartUpload.js @@ -320,8 +320,7 @@ function abortMultipleMpus(backendsInfo, callback) { callback(); }); } -// TODO CLDSRV-431 remove skip -describe.skip('Multipart Upload API with AWS Backend', function mpuTestSuite() { +describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { this.timeout(60000); beforeEach(done => { diff --git a/tests/multipleBackend/objectCopy.js b/tests/multipleBackend/objectCopy.js index 9397c81afc..aaa8552444 100644 --- a/tests/multipleBackend/objectCopy.js +++ b/tests/multipleBackend/objectCopy.js @@ -71,8 +71,7 @@ function copySetup(params, cb) { callback), ], err => cb(err)); } -// TODO CLDSRV-431 remove skip -describe.skip('ObjectCopy API with multiple backends', () => { +describe('ObjectCopy API with multiple backends', () => { before(() => { cleanup(); }); diff --git a/tests/multipleBackend/objectPutCopyPart.js b/tests/multipleBackend/objectPutCopyPart.js index be28356e84..b5778887e7 100644 --- a/tests/multipleBackend/objectPutCopyPart.js +++ b/tests/multipleBackend/objectPutCopyPart.js @@ -33,8 +33,7 @@ const awsLocation2 = 'awsbackend2'; const awsLocationMismatch = 'awsbackendmismatch'; const partETag = 'be747eb4b75517bf6b3cf7c5fbb62f3a'; -// TODO CLDSRV-431 reenable -// const describeSkipIfE2E = process.env.S3_END_TO_END ? describe.skip : describe; +const describeSkipIfE2E = process.env.S3_END_TO_END ? describe.skip : describe; function getSourceAndDestKeys() { const timestamp = Date.now(); @@ -175,9 +174,7 @@ function assertPartList(partList, uploadId) { assert.strictEqual(partList.Parts[0].Size, 11); } -// TODO CLDSRV-431 remove skip -// describeSkipIfE2E('ObjectCopyPutPart API with multiple backends', -describe.skip('ObjectCopyPutPart API with multiple backends', +describeSkipIfE2E('ObjectCopyPutPart API with multiple backends', function testSuite() { this.timeout(60000); diff --git a/tests/multipleBackend/objectPutPart.js b/tests/multipleBackend/objectPutPart.js index 63632f301d..43359def22 100644 --- a/tests/multipleBackend/objectPutPart.js +++ b/tests/multipleBackend/objectPutPart.js @@ -37,8 +37,7 @@ const md5Hash2 = crypto.createHash('md5'); const calculatedHash1 = md5Hash1.update(body1).digest('hex'); const calculatedHash2 = md5Hash2.update(body2).digest('hex'); -// TODO CLDSRV-431 reenable -// const describeSkipIfE2E = process.env.S3_END_TO_END ? describe.skip : describe; +const describeSkipIfE2E = process.env.S3_END_TO_END ? describe.skip : describe; function _getOverviewKey(objectKey, uploadId) { return `overview${splitter}${objectKey}${splitter}${uploadId}`; @@ -169,9 +168,7 @@ function listAndAbort(uploadId, calculatedHash2, objectName, done) { }); }); } -// TODO CLDSRV-431 remove skip -// describeSkipIfE2E('objectPutPart API with multiple backends', -describe.skip('objectPutPart API with multiple backends', +describeSkipIfE2E('objectPutPart API with multiple backends', function testSuite() { this.timeout(5000); diff --git a/tests/unit/api/bucketACLauth.js b/tests/unit/api/bucketACLauth.js index a9e6a8c7f5..a4aba0b125 100644 --- a/tests/unit/api/bucketACLauth.js +++ b/tests/unit/api/bucketACLauth.js @@ -18,8 +18,7 @@ const bucket = new BucketInfo('niftyBucket', ownerCanonicalId, authInfo.getAccountDisplayName(), creationDate); const log = new DummyRequestLogger(); -// TODO CLDSRV-431 remove skip -describe.skip('bucket authorization for bucketGet, bucketHead, ' + +describe('bucket authorization for bucketGet, bucketHead, ' + 'objectGet, and objectHead', () => { // Reset the bucket ACLs afterEach(() => { diff --git a/tests/unit/api/bucketDelete.js b/tests/unit/api/bucketDelete.js index c0f49df2bf..45bde5f2f8 100644 --- a/tests/unit/api/bucketDelete.js +++ b/tests/unit/api/bucketDelete.js @@ -129,8 +129,7 @@ describe('bucketDelete API', () => { }); }); - // TODO CLDSRV-431 remove skip - it.skip('should not return an error if the bucket has an initiated mpu', + it('should not return an error if the bucket has an initiated mpu', done => { bucketPut(authInfo, testRequest, log, err => { assert.strictEqual(err, null); @@ -160,13 +159,11 @@ describe('bucketDelete API', () => { }); }); - // TODO CLDSRV-431 remove skip - it.skip('should delete a bucket even if the bucket has ongoing mpu', + it('should delete a bucket even if the bucket has ongoing mpu', done => createMPU(testRequest, initiateRequest, false, done)); - // TODO CLDSRV-431 remove skip // if only part object (and no overview objects) is in mpu shadow bucket - it.skip('should delete a bucket even if the bucket has an orphan part', + it('should delete a bucket even if the bucket has an orphan part', done => createMPU(testRequest, initiateRequest, true, done)); diff --git a/tests/unit/api/bucketHead.js b/tests/unit/api/bucketHead.js index a69a5483ae..21c7a1beb5 100644 --- a/tests/unit/api/bucketHead.js +++ b/tests/unit/api/bucketHead.js @@ -16,8 +16,7 @@ const testRequest = { url: '/', iamAuthzResults: false, }; -// TODO CLDSRV-431 remove skip -describe.skip('bucketHead API', () => { +describe('bucketHead API', () => { beforeEach(() => { cleanup(); }); diff --git a/tests/unit/api/deleteMarker.js b/tests/unit/api/deleteMarker.js index 0edde69d53..1dc2ae1895 100644 --- a/tests/unit/api/deleteMarker.js +++ b/tests/unit/api/deleteMarker.js @@ -99,8 +99,7 @@ const undefHeadersExpected = [ 'expires', ]; -// TODO CLDSRV-431 remove skip -describe.skip('delete marker creation', () => { +describe('delete marker creation', () => { beforeEach(done => { cleanup(); bucketPut(authInfo, testPutBucketRequest, log, err => { diff --git a/tests/unit/api/deletedFlagBucket.js b/tests/unit/api/deletedFlagBucket.js index 051b3bd734..889376a0f9 100644 --- a/tests/unit/api/deletedFlagBucket.js +++ b/tests/unit/api/deletedFlagBucket.js @@ -105,8 +105,7 @@ function confirmDeleted(done) { }); } -// TODO CLDSRV-431 remove skip -describe.skip('deleted flag bucket handling', () => { +describe('deleted flag bucket handling', () => { beforeEach(done => { cleanup(); const bucketMD = new BucketInfo(bucketName, canonicalID, diff --git a/tests/unit/api/listMultipartUploads.js b/tests/unit/api/listMultipartUploads.js index 276b35344a..988323cb62 100644 --- a/tests/unit/api/listMultipartUploads.js +++ b/tests/unit/api/listMultipartUploads.js @@ -15,8 +15,8 @@ const canonicalID = 'accessKey1'; const authInfo = makeAuthInfo(canonicalID); const namespace = 'default'; const bucketName = 'bucketname'; -// TODO CLDSRV-431 remove skip -describe.skip('listMultipartUploads API', () => { + +describe('listMultipartUploads API', () => { beforeEach(() => { cleanup(); }); diff --git a/tests/unit/api/listParts.js b/tests/unit/api/listParts.js index cf37258ade..96f3ccd8e5 100644 --- a/tests/unit/api/listParts.js +++ b/tests/unit/api/listParts.js @@ -31,8 +31,8 @@ const partTwoKey = '4db92ccc-d89d-49d3-9fa6-e9c2c1eb31b0' + const partThreeKey = `4db92ccc-d89d-49d3-9fa6-e9c2c1eb31b0${splitter}00003`; const partFourKey = `4db92ccc-d89d-49d3-9fa6-e9c2c1eb31b0${splitter}00004`; const partFiveKey = `4db92ccc-d89d-49d3-9fa6-e9c2c1eb31b0${splitter}00005`; -// TODO CLDSRV-431 remove skip -describe.skip('List Parts API', () => { + +describe('List Parts API', () => { beforeEach(done => { cleanup(); const creationDate = new Date().toJSON(); diff --git a/tests/unit/api/multiObjectDelete.js b/tests/unit/api/multiObjectDelete.js index d074219c96..31cd1f1128 100644 --- a/tests/unit/api/multiObjectDelete.js +++ b/tests/unit/api/multiObjectDelete.js @@ -25,8 +25,8 @@ const testBucketPutRequest = new DummyRequest({ headers: {}, url: `/${bucketName}`, }); -// TODO CLDSRV-431 remove skip -describe.skip('getObjMetadataAndDelete function for multiObjectDelete', () => { + +describe('getObjMetadataAndDelete function for multiObjectDelete', () => { let testPutObjectRequest1; let testPutObjectRequest2; const request = new DummyRequest({ diff --git a/tests/unit/api/multipartDelete.js b/tests/unit/api/multipartDelete.js index 2792cb9463..b020d9e290 100644 --- a/tests/unit/api/multipartDelete.js +++ b/tests/unit/api/multipartDelete.js @@ -96,8 +96,8 @@ function _createAndAbortMpu(usEastSetting, fakeUploadID, locationConstraint, multipartDelete(authInfo, deleteMpuRequest, log, next), ], err => callback(err, uploadId)); } -// TODO CLDSRV-431 remove skip -describe.skip('Multipart Delete API', () => { + +describe('Multipart Delete API', () => { beforeEach(() => { cleanup(); }); diff --git a/tests/unit/api/multipartUpload.js b/tests/unit/api/multipartUpload.js index 2534952c53..616fb68b3a 100644 --- a/tests/unit/api/multipartUpload.js +++ b/tests/unit/api/multipartUpload.js @@ -136,8 +136,7 @@ function _createCompleteMpuRequest(uploadId, parts) { }; } -// TODO CLDSRV-431 remove skip -describe.skip('Multipart Upload API', () => { +describe('Multipart Upload API', () => { beforeEach(() => { cleanup(); }); @@ -1894,8 +1893,8 @@ describe.skip('Multipart Upload API', () => { }); }); }); -// TODO CLDSRV-431 remove skip -describe.skip('complete mpu with versioning', () => { + +describe('complete mpu with versioning', () => { const objData = ['foo0', 'foo1', 'foo2'].map(str => Buffer.from(str, 'utf8')); @@ -2125,8 +2124,8 @@ describe.skip('complete mpu with versioning', () => { }); }); }); -// TODO CLDSRV-431 remove skip -describe.skip('multipart upload with object lock', () => { + +describe('multipart upload with object lock', () => { before(done => { cleanup(); bucketPut(authInfo, lockEnabledBucketRequest, log, done); diff --git a/tests/unit/api/objectACLauth.js b/tests/unit/api/objectACLauth.js index 818d9e5c49..2248ec91b5 100644 --- a/tests/unit/api/objectACLauth.js +++ b/tests/unit/api/objectACLauth.js @@ -29,8 +29,8 @@ const object = { }, }; const log = new DummyRequestLogger(); -// TODO CLDSRV-431 remove skip -describe.skip('object acl authorization for objectGet and objectHead', () => { + +describe('object acl authorization for objectGet and objectHead', () => { // Reset the object ACLs afterEach(() => { object.acl = { @@ -175,8 +175,8 @@ describe.skip('object acl authorization for objectGet and objectHead', () => { assert.deepStrictEqual(results, [false, false]); }); }); -// TODO CLDSRV-431 remove skip -describe.skip('object authorization for objectPut and objectDelete', () => { + +describe('object authorization for objectPut and objectDelete', () => { it('should allow access to anyone since checks ' + 'are done at bucket level', () => { const requestTypes = ['objectPut', 'objectDelete']; @@ -189,8 +189,8 @@ describe.skip('object authorization for objectPut and objectDelete', () => { assert.deepStrictEqual(publicUserResults, [true, true]); }); }); -// TODO CLDSRV-431 remove skip -describe.skip('object authorization for objectPutACL and objectGetACL', () => { + +describe('object authorization for objectPutACL and objectGetACL', () => { // Reset the object ACLs afterEach(() => { object.acl = { @@ -275,8 +275,8 @@ describe.skip('object authorization for objectPutACL and objectGetACL', () => { assert.strictEqual(authorizedResult, true); }); }); -// TODO CLDSRV-431 remove skip -describe.skip('without object metadata', () => { + +describe('without object metadata', () => { afterEach(() => { bucket.setFullAcl({ Canned: 'private', diff --git a/tests/unit/api/objectCopy.js b/tests/unit/api/objectCopy.js index f6e2112651..d0772e49c5 100644 --- a/tests/unit/api/objectCopy.js +++ b/tests/unit/api/objectCopy.js @@ -53,8 +53,7 @@ const suspendVersioningRequest = versioningTestUtils const objData = ['foo0', 'foo1', 'foo2'].map(str => Buffer.from(str, 'utf8')); -// TODO CLDSRV-431 remove skip -describe.skip('objectCopy with versioning', () => { +describe('objectCopy with versioning', () => { const testPutObjectRequests = objData.slice(0, 2).map(data => versioningTestUtils.createPutObjectRequest(destBucketName, objectKey, data)); @@ -110,8 +109,7 @@ describe.skip('objectCopy with versioning', () => { }); }); -// TODO CLDSRV-431 remove skip -describe.skip('non-versioned objectCopy', () => { +describe('non-versioned objectCopy', () => { const testPutObjectRequest = versioningTestUtils .createPutObjectRequest(sourceBucketName, objectKey, objData[0]); diff --git a/tests/unit/api/objectCopyPart.js b/tests/unit/api/objectCopyPart.js index 896b7e7ed5..51bdaf0a08 100644 --- a/tests/unit/api/objectCopyPart.js +++ b/tests/unit/api/objectCopyPart.js @@ -58,8 +58,8 @@ function _createObjectCopyPartRequest(destBucketName, uploadId, headers) { const putDestBucketRequest = _createBucketPutRequest(destBucketName); const putSourceBucketRequest = _createBucketPutRequest(sourceBucketName); const initiateRequest = _createInitiateRequest(destBucketName); -// TODO CLDSRV-431 remove skip -describe.skip('objectCopyPart', () => { + +describe('objectCopyPart', () => { let uploadId; const objData = Buffer.from('foo', 'utf8'); const testPutObjectRequest = diff --git a/tests/unit/api/objectDelete.js b/tests/unit/api/objectDelete.js index 576793b3f4..4010717502 100644 --- a/tests/unit/api/objectDelete.js +++ b/tests/unit/api/objectDelete.js @@ -127,8 +127,7 @@ describe('objectDelete API', () => { }); }); - // TODO CLDSRV-431 remove skip - skipped due to MPU call - it.skip('should delete a multipart upload and send `uploadId` as `replayId` to deleteObject', done => { + it('should delete a multipart upload and send `uploadId` as `replayId` to deleteObject', done => { bucketPut(authInfo, testBucketPutRequest, log, () => { mpuUtils.createMPU(namespace, bucketName, objectKey, log, (err, testUploadId) => { diff --git a/tests/unit/api/objectHead.js b/tests/unit/api/objectHead.js index df4ff97b31..ab935a4c1b 100644 --- a/tests/unit/api/objectHead.js +++ b/tests/unit/api/objectHead.js @@ -35,8 +35,8 @@ const userMetadataKey = 'x-amz-meta-test'; const userMetadataValue = 'some metadata'; let testPutObjectRequest; -// TODO CLDSRV-431 remove skip -describe.skip('objectHead API', () => { + +describe('objectHead API', () => { beforeEach(() => { cleanup(); testPutObjectRequest = new DummyRequest({ diff --git a/tests/unit/api/objectPut.js b/tests/unit/api/objectPut.js index f6ddb13411..c534871b25 100644 --- a/tests/unit/api/objectPut.js +++ b/tests/unit/api/objectPut.js @@ -519,8 +519,7 @@ describe('objectPut API', () => { }); }); - // TODO CLDSRV-431 remove skip - it.skip('should not leave orphans in data when overwriting an multipart upload object', done => { + it('should not leave orphans in data when overwriting an multipart upload object', done => { bucketPut(authInfo, testPutBucketRequest, log, () => { mpuUtils.createMPU(namespace, bucketName, objectName, log, (err, testUploadId) => { diff --git a/tests/unit/api/objectReplicationMD.js b/tests/unit/api/objectReplicationMD.js index 4eced54993..0b3bc12d9a 100644 --- a/tests/unit/api/objectReplicationMD.js +++ b/tests/unit/api/objectReplicationMD.js @@ -209,8 +209,8 @@ function copyObject(sourceObjectKey, copyObjectKey, hasContent, cb) { log, cb); }); } -// TODO CLDSRV-431 remove skip -describe.skip('Replication object MD without bucket replication config', () => { + +describe('Replication object MD without bucket replication config', () => { beforeEach(() => { cleanup(); createBucket(); @@ -277,10 +277,9 @@ describe.skip('Replication object MD without bucket replication config', () => { })); }); }); -// TODO CLDSRV-431 remove skip [true, false].forEach(hasStorageClass => { - describe.skip('Replication object MD with bucket replication config ' + + describe('Replication object MD with bucket replication config ' + `${hasStorageClass ? 'with' : 'without'} storage class`, () => { const replicationMD = { status: 'PENDING', diff --git a/tests/unit/api/serviceGet.js b/tests/unit/api/serviceGet.js index 2eb3ab1eda..1b024e322d 100644 --- a/tests/unit/api/serviceGet.js +++ b/tests/unit/api/serviceGet.js @@ -14,8 +14,8 @@ const namespace = 'default'; const bucketName1 = 'bucketname1'; const bucketName2 = 'bucketname2'; const bucketName3 = 'bucketname3'; -// TODO CLDSRV-431 remove skip -describe.skip('serviceGet API', () => { + +describe('serviceGet API', () => { beforeEach(() => { cleanup(); }); diff --git a/tests/utapi/awsNodeSdk.js b/tests/utapi/awsNodeSdk.js index b273be8037..debaa17a78 100644 --- a/tests/utapi/awsNodeSdk.js +++ b/tests/utapi/awsNodeSdk.js @@ -191,8 +191,7 @@ function getObject(bucket, key, cb) { }); } -// TODO CLDSRV-431 remove skip -describe.skip('utapi v2 metrics incoming and outgoing bytes', function t() { +describe('utapi v2 metrics incoming and outgoing bytes', function t() { this.timeout(30000); const utapi = new MockUtapi(); From b4a986ec3aeea1bb759c9324113388f9536e8dd5 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Wed, 13 Sep 2023 18:22:14 +0200 Subject: [PATCH 4/8] fixup: remove skips --- .github/workflows/tests.yaml | 12 +++++------- tests/unit/api/transientBucket.js | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 1584472dc0..9e9dc091f4 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -227,13 +227,11 @@ jobs: - name: Setup CI services run: docker-compose up -d working-directory: .github/docker - # TODO CLDSRV-431 re-enable file backend tests - # Note: Disabled here to save time as due to API logic changes only 28 passing, 474 pending and 696 failing - # - name: Run file ft tests - # run: |- - # set -o pipefail; - # bash wait_for_local_port.bash 8000 40 - # yarn run ft_test | tee /tmp/artifacts/${{ github.job }}/tests.log + - name: Run file ft tests + run: |- + set -o pipefail; + bash wait_for_local_port.bash 8000 40 + yarn run ft_test | tee /tmp/artifacts/${{ github.job }}/tests.log - name: Upload logs to artifacts uses: scality/action-artifacts@v3 with: diff --git a/tests/unit/api/transientBucket.js b/tests/unit/api/transientBucket.js index ff67c05707..b119a71ee5 100644 --- a/tests/unit/api/transientBucket.js +++ b/tests/unit/api/transientBucket.js @@ -67,8 +67,8 @@ const creationDate = new Date().toJSON(); const usersBucket = new BucketInfo(usersBucketName, userBucketOwner, userBucketOwner, creationDate); const locationConstraint = 'us-east-1'; -// TODO CLDSRV-431 remove skip -describe.skip('transient bucket handling', () => { + +describe('transient bucket handling', () => { beforeEach(done => { cleanup(); const bucketMD = new BucketInfo(bucketName, canonicalID, From 9bf9130ca9a0568e4959b0e2c54a12200a784392 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Mon, 18 Sep 2023 15:14:31 +0200 Subject: [PATCH 5/8] change variable name --- .../apiUtils/object/abortMultipartUpload.js | 8 +-- lib/api/bucketHead.js | 2 +- lib/api/completeMultipartUpload.js | 8 +-- lib/api/initiateMultipartUpload.js | 2 +- lib/api/listMultipartUploads.js | 2 +- lib/api/listParts.js | 8 +-- lib/api/objectCopy.js | 10 ++-- lib/api/objectHead.js | 2 +- lib/api/websiteHead.js | 4 +- lib/routes/routeBackbeat.js | 4 +- tests/multipleBackend/multipartUpload.js | 16 +++--- tests/multipleBackend/objectPutCopyPart.js | 4 +- tests/multipleBackend/objectPutPart.js | 2 +- tests/unit/api/apiUtils/tagConditionKeys.js | 2 +- tests/unit/api/bucketHead.js | 2 +- tests/unit/api/deleteMarker.js | 8 +-- tests/unit/api/deletedFlagBucket.js | 4 +- tests/unit/api/listMultipartUploads.js | 18 +++---- tests/unit/api/listParts.js | 12 ++--- tests/unit/api/multipartDelete.js | 8 +-- tests/unit/api/multipartUpload.js | 50 +++++++++---------- tests/unit/api/objectHead.js | 24 ++++----- tests/unit/api/objectReplicationMD.js | 4 +- tests/unit/api/transientBucket.js | 4 +- tests/unit/utils/bucketEncryption.js | 2 +- tests/unit/utils/lifecycleHelpers.js | 2 +- tests/unit/utils/mpuUtils.js | 6 +-- 27 files changed, 109 insertions(+), 109 deletions(-) diff --git a/lib/api/apiUtils/object/abortMultipartUpload.js b/lib/api/apiUtils/object/abortMultipartUpload.js index 24688ddb10..2a9c999c96 100644 --- a/lib/api/apiUtils/object/abortMultipartUpload.js +++ b/lib/api/apiUtils/object/abortMultipartUpload.js @@ -22,7 +22,7 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, // but the requestType is the more general 'objectDelete' const metadataValParams = Object.assign({}, metadataValMPUparams); metadataValParams.requestType = 'objectPut'; - const authzIdentityResult = request ? request.iamAuthzResults : true; + const authzIdentityResult = request ? request.actionImplicitDenies : true; async.waterfall([ function checkDestBucketVal(next) { @@ -57,15 +57,15 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket, next) { const location = mpuOverviewObj.controllingLocationConstraint; - const originalIdentityAuthzResults = request.iamAuthzResults; + const originalIdentityImpDenies = request.actionImplicitDenies; // eslint-disable-next-line no-param-reassign // eslint-disable-next-line no-param-reassign - delete request.iamAuthzResults; + delete request.actionImplicitDenies; return data.abortMPU(objectKey, uploadId, location, bucketName, request, destBucket, locationConstraintCheck, log, (err, skipDataDelete) => { // eslint-disable-next-line no-param-reassign - request.iamAuthzResults = originalIdentityAuthzResults; + request.actionImplicitDenies = originalIdentityImpDenies; if (err) { return next(err, destBucket); } diff --git a/lib/api/bucketHead.js b/lib/api/bucketHead.js index 815689b527..df33bee472 100644 --- a/lib/api/bucketHead.js +++ b/lib/api/bucketHead.js @@ -21,7 +21,7 @@ function bucketHead(authInfo, request, log, callback) { requestType: 'bucketHead', request, }; - metadataValidateBucket(metadataValParams, request.iamAuthzResults, log, (err, bucket) => { + metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); if (err) { diff --git a/lib/api/completeMultipartUpload.js b/lib/api/completeMultipartUpload.js index fa0bcdfbcd..841441e156 100644 --- a/lib/api/completeMultipartUpload.js +++ b/lib/api/completeMultipartUpload.js @@ -120,7 +120,7 @@ function completeMultipartUpload(authInfo, request, log, callback) { // at the destinationBucket level are same as objectPut requestType: 'objectPut', }; - metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, next); + metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, next); }, function validateMultipart(destBucket, objMD, next) { if (objMD) { @@ -190,14 +190,14 @@ function completeMultipartUpload(authInfo, request, log, callback) { const mdInfo = { storedParts, mpuOverviewKey, splitter }; const mpuInfo = { objectKey, uploadId, jsonList, bucketName, destBucket }; - const originalIdentityAuthzResults = request.iamAuthzResults; + const originalIdentityImpDenies = request.actionImplicitDenies; // eslint-disable-next-line no-param-reassign - delete request.iamAuthzResults; + delete request.actionImplicitDenies; return data.completeMPU(request, mpuInfo, mdInfo, location, null, null, null, locationConstraintCheck, log, (err, completeObjData) => { // eslint-disable-next-line no-param-reassign - request.iamAuthzResults = originalIdentityAuthzResults; + request.actionImplicitDenies = originalIdentityImpDenies; if (err) { return next(err, destBucket); } diff --git a/lib/api/initiateMultipartUpload.js b/lib/api/initiateMultipartUpload.js index 2947f9dc16..50ed82d531 100644 --- a/lib/api/initiateMultipartUpload.js +++ b/lib/api/initiateMultipartUpload.js @@ -253,7 +253,7 @@ function initiateMultipartUpload(authInfo, request, log, callback) { } async.waterfall([ - next => metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, + next => metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (error, destinationBucket) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destinationBucket); diff --git a/lib/api/listMultipartUploads.js b/lib/api/listMultipartUploads.js index 471d2b72d0..5f50d17650 100644 --- a/lib/api/listMultipartUploads.js +++ b/lib/api/listMultipartUploads.js @@ -104,7 +104,7 @@ function listMultipartUploads(authInfo, request, log, callback) { function waterfall1(next) { // Check final destination bucket for authorization rather // than multipart upload bucket - metadataValidateBucket(metadataValParams, request.iamAuthzResults, log, + metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => next(err, bucket)); }, function getMPUBucket(bucket, next) { diff --git a/lib/api/listParts.js b/lib/api/listParts.js index 3a66808b8a..0a5e227482 100644 --- a/lib/api/listParts.js +++ b/lib/api/listParts.js @@ -112,7 +112,7 @@ function listParts(authInfo, request, log, callback) { async.waterfall([ function checkDestBucketVal(next) { - metadataValidateBucketAndObj(metadataValParams, request.iamAuthzResults, log, + metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, destinationBucket) => { if (err) { return next(err, destinationBucket, null); @@ -150,13 +150,13 @@ function listParts(authInfo, request, log, callback) { mpuOverviewObj, destBucket, }; - const originalIdentityAuthzResults = request.iamAuthzResults; + const originalIdentityImpDenies = request.actionImplicitDenies; // eslint-disable-next-line no-param-reassign - delete request.iamAuthzResults; + delete request.actionImplicitDenies; return data.listParts(mpuInfo, request, locationConstraintCheck, log, (err, backendPartList) => { // eslint-disable-next-line no-param-reassign - request.iamAuthzResults = originalIdentityAuthzResults; + request.actionImplicitDenies = originalIdentityImpDenies; if (err) { return next(err, destBucket); } diff --git a/lib/api/objectCopy.js b/lib/api/objectCopy.js index 56fef30863..eeb79ba7c3 100644 --- a/lib/api/objectCopy.js +++ b/lib/api/objectCopy.js @@ -245,7 +245,7 @@ function objectCopy(authInfo, request, sourceBucket, } return async.waterfall([ function checkDestAuth(next) { - return metadataValidateBucketAndObj(valPutParams, request.iamAuthzResults, log, + return metadataValidateBucketAndObj(valPutParams, request.actionImplicitDenies, log, (err, destBucketMD, destObjMD) => { if (err) { log.debug('error validating put part of request', @@ -263,7 +263,7 @@ function objectCopy(authInfo, request, sourceBucket, }); }, function checkSourceAuthorization(destBucketMD, destObjMD, next) { - return metadataValidateBucketAndObj(valGetParams, request.iamAuthzResults, log, + return metadataValidateBucketAndObj(valGetParams, request.actionImplicitDenies, log, (err, sourceBucketMD, sourceObjMD) => { if (err) { log.debug('error validating get part of request', @@ -408,15 +408,15 @@ function objectCopy(authInfo, request, sourceBucket, return next(null, storeMetadataParams, dataLocator, destObjMD, serverSideEncryption, destBucketMD); } - const originalIdentityAuthzResults = request.iamAuthzResults; + const originalIdentityImpDenies = request.actionImplicitDenies; // eslint-disable-next-line no-param-reassign - delete request.iamAuthzResults; + delete request.actionImplicitDenies; return data.copyObject(request, sourceLocationConstraintName, storeMetadataParams, dataLocator, dataStoreContext, backendInfoDest, sourceBucketMD, destBucketMD, serverSideEncryption, log, (err, results) => { // eslint-disable-next-line no-param-reassign - request.iamAuthzResults = originalIdentityAuthzResults; + request.actionImplicitDenies = originalIdentityImpDenies; if (err) { return next(err, destBucketMD); } diff --git a/lib/api/objectHead.js b/lib/api/objectHead.js index 362b828e8d..38977bad88 100644 --- a/lib/api/objectHead.js +++ b/lib/api/objectHead.js @@ -48,7 +48,7 @@ function objectHead(authInfo, request, log, callback) { request, }; - return metadataValidateBucketAndObj(mdValParams, request.iamAuthzResults, log, + return metadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log, (err, bucket, objMD) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); diff --git a/lib/api/websiteHead.js b/lib/api/websiteHead.js index 59c4cec023..493ce8dad6 100644 --- a/lib/api/websiteHead.js +++ b/lib/api/websiteHead.js @@ -104,7 +104,7 @@ function websiteHead(request, log, callback) { { error: err }); let returnErr = err; const bucketAuthorized = isBucketAuthorized(bucket, - 'bucketGet', constants.publicId, null, request.iamAuthzResults, log, request); + 'bucketGet', constants.publicId, null, request.actionImplicitDenies, log, request); // if index object does not exist and bucket is private AWS // returns 403 - AccessDenied error. if (err.is.NoSuchKey && !bucketAuthorized) { @@ -114,7 +114,7 @@ function websiteHead(request, log, callback) { reqObjectKey, corsHeaders, log, callback); } if (!isObjAuthorized(bucket, objMD, 'objectGet', - constants.publicId, null, request.iamAuthzResults, log, request)) { + constants.publicId, null, request.actionImplicitDenies, log, request)) { const err = errors.AccessDenied; log.trace('request not authorized', { error: err }); return _errorActions(err, routingRules, reqObjectKey, diff --git a/lib/routes/routeBackbeat.js b/lib/routes/routeBackbeat.js index c8d74653bb..60016ee9a2 100644 --- a/lib/routes/routeBackbeat.js +++ b/lib/routes/routeBackbeat.js @@ -1150,7 +1150,7 @@ function routeBackbeat(clientIP, request, response, log) { // eslint-disable-next-line no-param-reassign request.apiMethod = 'routeBackbeat'; // eslint-disable-next-line no-param-reassign - request.iamAuthzResults = false; + request.actionImplicitDenies = false; log.debug('routing request', { method: 'routeBackbeat', @@ -1275,7 +1275,7 @@ function routeBackbeat(clientIP, request, response, log) { requestType: 'ReplicateObject', request, }; - return metadataValidateBucketAndObj(mdValParams, request.iamAuthzResults, log, next); + return metadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log, next); }, (bucketInfo, objMd, next) => { if (useMultipleBackend) { diff --git a/tests/multipleBackend/multipartUpload.js b/tests/multipleBackend/multipartUpload.js index 29d1948185..37038b6a3a 100644 --- a/tests/multipleBackend/multipartUpload.js +++ b/tests/multipleBackend/multipartUpload.js @@ -54,7 +54,7 @@ const bucketPutRequest = { url: '/', post: '', parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; const awsETag = 'be747eb4b75517bf6b3cf7c5fbb62f3a'; @@ -74,7 +74,7 @@ const completeBody = '' + const basicParams = { bucketName, namespace, - iamAuthzResults: false, + actionImplicitDenies: false, }; function getObjectGetRequest(objectKey) { @@ -272,7 +272,7 @@ function mpuSetup(location, key, cb) { 'x-amz-meta-scal-location-constraint': location }, url: `/${key}?uploads`, parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, (err, result) => { @@ -344,7 +344,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { 'x-amz-meta-scal-location-constraint': `${awsLocation}` }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -368,7 +368,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { `${awsLocationMismatch}` }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -393,7 +393,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, @@ -617,7 +617,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { 'x-amz-meta-scal-location-constraint': awsLocation }, url: `/${objectKey}?uploads`, parsedHost: 'localhost', - iamAuthzResults: false, + actionImplicitDenies: false, }; initiateMultipartUpload(authInfo, initiateRequest, log, err => { @@ -718,7 +718,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: {}, - iamAuthzResults: false, + actionImplicitDenies: false, }; listMultipartUploads(authInfo, listMpuParams, log, (err, mpuListXml) => { diff --git a/tests/multipleBackend/objectPutCopyPart.js b/tests/multipleBackend/objectPutCopyPart.js index b5778887e7..b46a8b35dc 100644 --- a/tests/multipleBackend/objectPutCopyPart.js +++ b/tests/multipleBackend/objectPutCopyPart.js @@ -78,7 +78,7 @@ function copyPutPart(bucketLoc, mpuLoc, srcObjLoc, requestHost, cb, objectKey: destObjName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${destObjName}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; if (mpuLoc) { initiateReq.headers = { @@ -95,7 +95,7 @@ function copyPutPart(bucketLoc, mpuLoc, srcObjLoc, requestHost, cb, objectKey: sourceObjName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; if (srcObjLoc) { sourceObjPutParams.headers = { diff --git a/tests/multipleBackend/objectPutPart.js b/tests/multipleBackend/objectPutPart.js index 43359def22..c8b330a3ab 100644 --- a/tests/multipleBackend/objectPutPart.js +++ b/tests/multipleBackend/objectPutPart.js @@ -67,7 +67,7 @@ function putPart(bucketLoc, mpuLoc, requestHost, cb, objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectName}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; if (mpuLoc) { initiateReq.headers = { diff --git a/tests/unit/api/apiUtils/tagConditionKeys.js b/tests/unit/api/apiUtils/tagConditionKeys.js index 1aceb7015f..49c957f3b5 100644 --- a/tests/unit/api/apiUtils/tagConditionKeys.js +++ b/tests/unit/api/apiUtils/tagConditionKeys.js @@ -24,7 +24,7 @@ const bucketPutReq = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; const taggingUtil = new TaggingConfigTester(); diff --git a/tests/unit/api/bucketHead.js b/tests/unit/api/bucketHead.js index 21c7a1beb5..0f203fcd19 100644 --- a/tests/unit/api/bucketHead.js +++ b/tests/unit/api/bucketHead.js @@ -14,7 +14,7 @@ const testRequest = { namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; describe('bucketHead API', () => { beforeEach(() => { diff --git a/tests/unit/api/deleteMarker.js b/tests/unit/api/deleteMarker.js index 1dc2ae1895..357475f25f 100644 --- a/tests/unit/api/deleteMarker.js +++ b/tests/unit/api/deleteMarker.js @@ -28,7 +28,7 @@ const testPutBucketRequest = new DummyRequest({ namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }); const testDeleteRequest = new DummyRequest({ bucketName, @@ -36,7 +36,7 @@ const testDeleteRequest = new DummyRequest({ objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }); function _createBucketPutVersioningReq(status) { @@ -47,7 +47,7 @@ function _createBucketPutVersioningReq(status) { }, url: '/?versioning', query: { versioning: '' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const xml = '' + @@ -65,7 +65,7 @@ function _createMultiObjectDeleteRequest(numObjects) { }, url: '/?delete', query: { delete: '' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const xml = []; xml.push(''); diff --git a/tests/unit/api/deletedFlagBucket.js b/tests/unit/api/deletedFlagBucket.js index 889376a0f9..d46af2e677 100644 --- a/tests/unit/api/deletedFlagBucket.js +++ b/tests/unit/api/deletedFlagBucket.js @@ -58,13 +58,13 @@ const baseTestRequest = { post: '', headers: { host: `${bucketName}.s3.amazonaws.com` }, query: {}, - iamAuthzResults: false, + actionImplicitDenies: false, }; const serviceGetRequest = { parsedHost: 's3.amazonaws.com', headers: { host: 's3.amazonaws.com' }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; const userBucketOwner = 'admin'; diff --git a/tests/unit/api/listMultipartUploads.js b/tests/unit/api/listMultipartUploads.js index 988323cb62..e24fc13f40 100644 --- a/tests/unit/api/listMultipartUploads.js +++ b/tests/unit/api/listMultipartUploads.js @@ -32,7 +32,7 @@ describe('listMultipartUploads API', () => { namespace, headers: {}, url: `/${bucketName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const testInitiateMPURequest1 = { bucketName, @@ -40,7 +40,7 @@ describe('listMultipartUploads API', () => { objectKey: objectName1, headers: {}, url: `/${bucketName}/${objectName1}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const testInitiateMPURequest2 = { bucketName, @@ -48,7 +48,7 @@ describe('listMultipartUploads API', () => { objectKey: objectName2, headers: {}, url: `/${bucketName}/${objectName2}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const testInitiateMPURequest3 = { bucketName, @@ -56,7 +56,7 @@ describe('listMultipartUploads API', () => { objectKey: objectName3, headers: {}, url: `/${bucketName}/${objectName3}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; it('should return the name of the common prefix ' + @@ -69,7 +69,7 @@ describe('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads&delimiter=/&prefix=sub`, query: { delimiter, prefix }, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ @@ -99,7 +99,7 @@ describe('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: {}, - iamAuthzResults: false, + actionImplicitDenies: false, }; @@ -133,7 +133,7 @@ describe('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'max-uploads': '1' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ @@ -170,7 +170,7 @@ describe('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'encoding-type': 'url' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ @@ -203,7 +203,7 @@ describe('listMultipartUploads API', () => { headers: { host: '/' }, url: `/${bucketName}?uploads`, query: { 'key-marker': objectName1 }, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ diff --git a/tests/unit/api/listParts.js b/tests/unit/api/listParts.js index 96f3ccd8e5..48a9668e11 100644 --- a/tests/unit/api/listParts.js +++ b/tests/unit/api/listParts.js @@ -114,7 +114,7 @@ describe('List Parts API', () => { url: `/${uploadKey}?uploadId=${uploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, - iamAuthzResults: false, + actionImplicitDenies: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -161,7 +161,7 @@ describe('List Parts API', () => { uploadId, 'encoding-type': 'url', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const urlEncodedObjectKey = '%24makememulti'; @@ -187,7 +187,7 @@ describe('List Parts API', () => { uploadId, 'max-parts': '4', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -229,7 +229,7 @@ describe('List Parts API', () => { uploadId, 'max-parts': '6', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -271,7 +271,7 @@ describe('List Parts API', () => { uploadId, 'part-number-marker': '2', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; listParts(authInfo, listRequest, log, (err, xml) => { @@ -317,7 +317,7 @@ describe('List Parts API', () => { 'part-number-marker': '2', 'max-parts': '2', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; listParts(authInfo, listRequest, log, (err, xml) => { diff --git a/tests/unit/api/multipartDelete.js b/tests/unit/api/multipartDelete.js index b020d9e290..33bcfed19c 100644 --- a/tests/unit/api/multipartDelete.js +++ b/tests/unit/api/multipartDelete.js @@ -24,7 +24,7 @@ const bucketPutRequest = { namespace, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; const objectKey = 'testObject'; const initiateRequest = { @@ -33,7 +33,7 @@ const initiateRequest = { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const eastLocation = 'us-east-1'; const westLocation = 'scality-internal-file'; @@ -70,7 +70,7 @@ function _createAndAbortMpu(usEastSetting, fakeUploadID, locationConstraint, partNumber: '1', uploadId, }, - iamAuthzResults: false, + actionImplicitDenies: false, }, partBody); const testUploadId = fakeUploadID ? 'nonexistinguploadid' : uploadId; @@ -81,7 +81,7 @@ function _createAndAbortMpu(usEastSetting, fakeUploadID, locationConstraint, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploadId=${testUploadId}`, query: { uploadId: testUploadId }, - iamAuthzResults: false, + actionImplicitDenies: false, }; next(null, partRequest, deleteMpuRequest); }, diff --git a/tests/unit/api/multipartUpload.js b/tests/unit/api/multipartUpload.js index 616fb68b3a..089667a3c6 100644 --- a/tests/unit/api/multipartUpload.js +++ b/tests/unit/api/multipartUpload.js @@ -49,7 +49,7 @@ const bucketPutRequest = { 'xmlns="http://s3.amazonaws.com/doc/2006-03-01/">' + 'scality-internal-mem' + '', - iamAuthzResults: false, + actionImplicitDenies: false, }; const lockEnabledBucketRequest = Object.assign({}, bucketPutRequest); lockEnabledBucketRequest.bucketName = lockedBucket; @@ -63,7 +63,7 @@ const initiateRequest = { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const retentionInitiateRequest = Object.assign({}, initiateRequest); retentionInitiateRequest.bucketName = lockedBucket; @@ -84,7 +84,7 @@ const getObjectLockInfoRequest = { namespace, objectKey, headers: { host: `${lockedBucket}.s3.amazonaws.com` }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const expectedRetentionConfig = { $: { xmlns: 'http://s3.amazonaws.com/doc/2006-03-01/' }, @@ -109,7 +109,7 @@ function _createPutPartRequest(uploadId, partNumber, partBody) { uploadId, }, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }, partBody); } @@ -132,7 +132,7 @@ function _createCompleteMpuRequest(uploadId, parts) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; } @@ -546,7 +546,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; const awsVerifiedETag = '"953e9e776f285afc0bfcf1ab4668299d-1"'; @@ -635,7 +635,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; const awsVerifiedETag = '"953e9e776f285afc0bfcf1ab4668299d-1"'; @@ -708,7 +708,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { @@ -764,7 +764,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { assert(err.is.MalformedXML); @@ -837,7 +837,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { @@ -896,7 +896,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, err => { assert(err.is.InvalidPart); @@ -968,7 +968,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 3); completeMultipartUpload(authInfo, @@ -1051,7 +1051,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 3); completeMultipartUpload(authInfo, @@ -1134,7 +1134,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1166,7 +1166,7 @@ describe('Multipart Upload API', () => { 'x-amz-acl': 'authenticated-read', }, url: `/${objectKey}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ @@ -1235,7 +1235,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1270,7 +1270,7 @@ describe('Multipart Upload API', () => { 'x-amz-grant-read': `emailAddress="${granteeEmail}"`, }, url: `/${objectKey}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; async.waterfall([ @@ -1339,7 +1339,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1392,7 +1392,7 @@ describe('Multipart Upload API', () => { url: `/${objectKey}?uploadId=${testUploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, - iamAuthzResults: false, + actionImplicitDenies: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2); multipartDelete(authInfo, deleteRequest, log, err => { @@ -1443,7 +1443,7 @@ describe('Multipart Upload API', () => { url: `/${objectKey}?uploadId=${testUploadId}`, headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: 'non-existent-upload-id' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2); multipartDelete(authInfo, deleteRequest, log, err => { @@ -1523,7 +1523,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, (err, result) => { @@ -1577,7 +1577,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, next); }, @@ -1649,7 +1649,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, next); }, @@ -1720,7 +1720,7 @@ describe('Multipart Upload API', () => { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; completeMultipartUpload(authInfo, completeRequest, log, done); }, @@ -1813,7 +1813,7 @@ describe('Multipart Upload API', () => { query: { uploadId: testUploadId }, post: completeBody, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }; // show that second part data is there assert(ds[2]); diff --git a/tests/unit/api/objectHead.js b/tests/unit/api/objectHead.js index ab935a4c1b..a917835a2a 100644 --- a/tests/unit/api/objectHead.js +++ b/tests/unit/api/objectHead.js @@ -29,7 +29,7 @@ const testPutBucketRequest = { namespace, headers: {}, url: `/${bucketName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const userMetadataKey = 'x-amz-meta-test'; const userMetadataValue = 'some metadata'; @@ -58,7 +58,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: { 'if-modified-since': laterDate }, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -82,7 +82,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: { 'if-unmodified-since': earlierDate }, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -108,7 +108,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: { 'if-match': incorrectMD5 }, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -133,7 +133,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: { 'if-none-match': correctMD5 }, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -156,7 +156,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: { range: 'bytes=1-9' }, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -181,7 +181,7 @@ describe('objectHead API', () => { query: { partNumber: '1', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const customizedInvalidRequestError = errors.InvalidRequest .customizeDescription('Cannot specify both Range header and ' + @@ -209,7 +209,7 @@ describe('objectHead API', () => { query: { partNumber: 'nan', }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const customizedInvalidArgumentError = errors.InvalidArgument .customizeDescription('Part number must be a number.'); @@ -234,7 +234,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -255,7 +255,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -279,7 +279,7 @@ describe('objectHead API', () => { namespace, headers: { 'x-amz-bucket-object-lock-enabled': 'true' }, url: `/${bucketName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; const testPutObjectRequestLock = new DummyRequest({ bucketName, @@ -299,7 +299,7 @@ describe('objectHead API', () => { objectKey: objectName, headers: {}, url: `/${bucketName}/${objectName}`, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequestLock, log, () => { diff --git a/tests/unit/api/objectReplicationMD.js b/tests/unit/api/objectReplicationMD.js index 0b3bc12d9a..fa388f4875 100644 --- a/tests/unit/api/objectReplicationMD.js +++ b/tests/unit/api/objectReplicationMD.js @@ -48,7 +48,7 @@ const objectACLReq = { }, url: `/${bucketName}/${keyA}?acl`, query: { acl: '' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; // Get an object request with the given key. @@ -186,7 +186,7 @@ function putMPU(key, body, cb) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId }, post: postBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; return completeMultipartUpload(authInfo, req, log, cb); } diff --git a/tests/unit/api/transientBucket.js b/tests/unit/api/transientBucket.js index b119a71ee5..134654e1ec 100644 --- a/tests/unit/api/transientBucket.js +++ b/tests/unit/api/transientBucket.js @@ -53,13 +53,13 @@ const baseTestRequest = { url: '/', post: '', headers: { host: `${bucketName}.s3.amazonaws.com` }, - iamAuthzResults: false, + actionImplicitDenies: false, }; const serviceGetRequest = { parsedHost: 's3.amazonaws.com', headers: { host: 's3.amazonaws.com' }, url: '/', - iamAuthzResults: false, + actionImplicitDenies: false, }; const userBucketOwner = 'admin'; diff --git a/tests/unit/utils/bucketEncryption.js b/tests/unit/utils/bucketEncryption.js index ac36465d75..63cb33df9a 100644 --- a/tests/unit/utils/bucketEncryption.js +++ b/tests/unit/utils/bucketEncryption.js @@ -29,7 +29,7 @@ function templateRequest(bucketName, { post }) { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post, - iamAuthzResults: false, + actionImplicitDenies: false, }; } diff --git a/tests/unit/utils/lifecycleHelpers.js b/tests/unit/utils/lifecycleHelpers.js index e6a86d1aaf..b9399b00ac 100644 --- a/tests/unit/utils/lifecycleHelpers.js +++ b/tests/unit/utils/lifecycleHelpers.js @@ -4,7 +4,7 @@ function getLifecycleRequest(bucketName, xml) { headers: { host: `${bucketName}.s3.amazonaws.com`, }, - iamAuthzResults: false, + actionImplicitDenies: false, }; if (xml) { request.post = xml; diff --git a/tests/unit/utils/mpuUtils.js b/tests/unit/utils/mpuUtils.js index 1d32b496fb..7100a0ccb0 100644 --- a/tests/unit/utils/mpuUtils.js +++ b/tests/unit/utils/mpuUtils.js @@ -28,7 +28,7 @@ function createinitiateMPURequest(namespace, bucketName, objectKey) { objectKey, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectKey}?uploads`, - iamAuthzResults: false, + actionImplicitDenies: false, }; return request; @@ -46,7 +46,7 @@ function createPutPartRequest(namespace, bucketName, objectKey, partNumber, test uploadId: testUploadId, }, calculatedHash, - iamAuthzResults: false, + actionImplicitDenies: false, }, partBody); return request; @@ -70,7 +70,7 @@ function createCompleteRequest(namespace, bucketName, objectKey, testUploadId) { headers: { host: `${bucketName}.s3.amazonaws.com` }, query: { uploadId: testUploadId }, post: completeBody, - iamAuthzResults: false, + actionImplicitDenies: false, }; return request; From c82ce08fdb709c666cdc367155d385027ca39188 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Tue, 19 Sep 2023 11:00:34 +0200 Subject: [PATCH 6/8] permission fix: no allow on user from account by default --- lib/api/apiUtils/authorization/permissionChecks.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index fc99363fca..73a4d6167e 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -273,9 +273,10 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l let permission = 'defaultDeny'; // if requester is user within bucket owner account, actions should be // allowed unless explicitly denied (assumes allowed by IAM policy) - if (bucketOwner === canonicalID) { - permission = 'allow'; - } + // Update: manual testing on the 18/9/2023 found this not to be the case + // if (bucketOwner === canonicalID) { + // permission = 'allow'; + // } let copiedStatement = JSON.parse(JSON.stringify(policy.Statement)); while (copiedStatement.length > 0) { const s = copiedStatement[0]; From 3773bcf61c0d464a3bb7492abfc55f3ba6ff90a6 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Thu, 21 Sep 2023 11:41:30 +0200 Subject: [PATCH 7/8] add evaluateBucketPolicyWithIAM omissions --- lib/api/apiUtils/object/objectLockHelpers.js | 29 ++++- lib/api/multiObjectDelete.js | 114 ++++++++++--------- 2 files changed, 88 insertions(+), 55 deletions(-) diff --git a/lib/api/apiUtils/object/objectLockHelpers.js b/lib/api/apiUtils/object/objectLockHelpers.js index d9d69bedc5..2d81c1dc91 100644 --- a/lib/api/apiUtils/object/objectLockHelpers.js +++ b/lib/api/apiUtils/object/objectLockHelpers.js @@ -3,6 +3,7 @@ const moment = require('moment'); const { config } = require('../../../Config'); const vault = require('../../../auth/vault'); +const { evaluateBucketPolicyWithIAM } = require('../authorization/permissionChecks'); /** * Calculates retain until date for the locked object version @@ -302,7 +303,11 @@ function checkUserGovernanceBypass(request, authInfo, bucketMD, objectKey, log, if (err) { return cb(err); } - if (authorizationResults[0].isAllowed !== true) { + + // Deny immediately if there is any explicit deny + const explicitDenyExists = authorizationResults.some( + authzResult => authzResult.isAllowed === false && authzResult.isImplicit === false); + if (explicitDenyExists) { log.trace('authorization check failed for user', { 'method': 'checkUserPolicyGovernanceBypass', @@ -310,7 +315,25 @@ function checkUserGovernanceBypass(request, authInfo, bucketMD, objectKey, log, }); return cb(errors.AccessDenied); } - return cb(null); + + // Convert authorization results into an easier to handle format + const iamAuthzResults = authorizationResults.reduce((acc, curr, idx) => { + // eslint-disable-next-line no-param-reassign + acc[requestContextParams[idx].apiMethod] = curr.isImplicit; + return acc; + }, {}); + + // Evaluate against the bucket policies + const areAllActionsAllowed = evaluateBucketPolicyWithIAM( + bucketMD, + Object.keys(iamAuthzResults), + authInfo.getCanonicalID(), + authInfo, + iamAuthzResults, + log, + request); + + return cb(areAllActionsAllowed ? null : errors.AccessDenied); }); } @@ -322,4 +345,4 @@ module.exports = { hasGovernanceBypassHeader, checkUserGovernanceBypass, ObjectLockInfo, -}; +}; \ No newline at end of file diff --git a/lib/api/multiObjectDelete.js b/lib/api/multiObjectDelete.js index 3715e3fb80..6f540ce40f 100644 --- a/lib/api/multiObjectDelete.js +++ b/lib/api/multiObjectDelete.js @@ -11,11 +11,12 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); const services = require('../services'); const vault = require('../auth/vault'); -const { isBucketAuthorized } = +const { isBucketAuthorized, evaluateBucketPolicyWithIAM } = require('./apiUtils/authorization/permissionChecks'); const { preprocessingVersioningDelete } = require('./apiUtils/object/versioning'); const createAndStoreObject = require('./apiUtils/object/createAndStoreObject'); +const monitoring = require('../utilities/metrics'); const { metadataGetObject } = require('../metadata/metadataUtils'); const { config } = require('../Config'); const { hasGovernanceBypassHeader, checkUserGovernanceBypass, ObjectLockInfo } @@ -385,15 +386,37 @@ function multiObjectDelete(authInfo, request, log, callback) { return next(null, quietSetting, objects); }); }, - function checkPolicies(quietSetting, objects, next) { + function checkBucketMetadata(quietSetting, objects, next) { + const errorResults = []; + return metadata.getBucket(bucketName, log, (err, bucketMD) => { + if (err) { + log.trace('error retrieving bucket metadata', { error: err }); + return next(err); + } + if (bucketShield(bucketMD, 'objectDelete')) { + return next(errors.NoSuchBucket); + } + if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo, + request.actionImplicitDenies, authInfo)) { + log.trace("access denied due to bucket acl's"); + objects.forEach(entry => { + errorResults.push({ + entry, + error: errors.AccessDenied, + }); + }); + return next(null, quietSetting, errorResults, [], bucketMD); + } + return next(null, quietSetting, errorResults, objects, bucketMD); + }); + }, + function checkPolicies(quietSetting, errorResults, objects, bucketMD, next) { // track keys that are still on track to be deleted const inPlay = []; - const errorResults = []; + // if request from account, no need to check policies - // all objects are inPlay so send array of object keys - // as inPlay argument if (!authInfo.isRequesterAnIAMUser()) { - return next(null, quietSetting, errorResults, objects); + return next(null, quietSetting, errorResults, objects, bucketMD); } // TODO: once arsenal's extractParams is separated from doAuth @@ -429,7 +452,6 @@ function multiObjectDelete(authInfo, request, log, callback) { }; return vault.checkPolicies(requestContextParams, authInfo.getArn(), log, (err, authorizationResults) => { - // there were no policies so received a blanket AccessDenied if (err && err.is.AccessDenied) { objects.forEach(entry => { errorResults.push({ @@ -437,7 +459,7 @@ function multiObjectDelete(authInfo, request, log, callback) { error: errors.AccessDenied }); }); // send empty array for inPlay - return next(null, quietSetting, errorResults, []); + return next(null, quietSetting, errorResults, [], bucketMD); } if (err) { log.trace('error checking policies', { @@ -455,6 +477,16 @@ function multiObjectDelete(authInfo, request, log, callback) { }); return next(errors.InternalError); } + + // Convert authorization results into an easier to handle format + const iamAuthzResults = authorizationResults.reduce((acc, curr, idx) => { + const apiMethod = requestContextParams[idx].apiMethod; + // eslint-disable-next-line no-param-reassign + acc[apiMethod] = curr.isImplicit; + return acc; + }, {}); + + for (let i = 0; i < authorizationResults.length; i++) { const result = authorizationResults[i]; // result is { isAllowed: true, @@ -470,7 +502,27 @@ function multiObjectDelete(authInfo, request, log, callback) { key: result.arn.slice(slashIndex + 1), versionId: result.versionId, }; - if (result.isAllowed) { + + // Deny immediately if there is an explicit deny + if (!result.isImplicit && !result.isAllowed) { + errorResults.push({ + entry, + error: errors.AccessDenied, + }); + continue; + } + + // Evaluate against the bucket policies + const areAllActionsAllowed = evaluateBucketPolicyWithIAM( + bucketMD, + Object.keys(iamAuthzResults), + canonicalID, + authInfo, + iamAuthzResults, + log, + request); + + if (areAllActionsAllowed) { inPlay.push(entry); } else { errorResults.push({ @@ -479,51 +531,9 @@ function multiObjectDelete(authInfo, request, log, callback) { }); } } - return next(null, quietSetting, errorResults, inPlay); + return next(null, quietSetting, errorResults, inPlay, bucketMD); }); }, - function checkBucketMetadata(quietSetting, errorResults, inPlay, next) { - // if no objects in play, no need to check ACLs / get metadata, - // just move on if there is no Origin header - if (inPlay.length === 0 && !request.headers.origin) { - return next(null, quietSetting, errorResults, inPlay, - undefined); - } - return metadata.getBucket(bucketName, log, (err, bucketMD) => { - if (err) { - log.trace('error retrieving bucket metadata', - { error: err }); - return next(err); - } - // check whether bucket has transient or deleted flag - if (bucketShield(bucketMD, 'objectDelete')) { - return next(errors.NoSuchBucket); - } - // if no objects in play, no need to check ACLs - if (inPlay.length === 0) { - return next(null, quietSetting, errorResults, inPlay, - bucketMD); - } - if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo, - request.actionImplicitDenies, log, request)) { - log.trace("access denied due to bucket acl's"); - // if access denied at the bucket level, no access for - // any of the objects so all results will be error results - inPlay.forEach(entry => { - errorResults.push({ - entry, - error: errors.AccessDenied, - }); - }); - // by sending an empty array as the inPlay array - // async.forEachLimit below will not actually - // make any calls to metadata or data but will continue on - // to the next step to build xml - return next(null, quietSetting, errorResults, [], bucketMD); - } - return next(null, quietSetting, errorResults, inPlay, bucketMD); - }); - }, function getObjMetadataAndDeleteStep(quietSetting, errorResults, inPlay, bucket, next) { return getObjMetadataAndDelete(authInfo, canonicalID, request, From f817122415c307c13756670d2fa664441b7d54d2 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Mon, 25 Sep 2023 11:40:17 +0200 Subject: [PATCH 8/8] REVISIT THIS COMMIT The code in this commit causes an error in the 'implicit deny iam policy with "Allow" bucket policy for User ARN' test for PutObject, giving an AccessDenied because the implicitDent is true. It seems worth re-evaluating the ACL expectations at this point. --- lib/api/apiUtils/authorization/permissionChecks.js | 8 ++++---- lib/api/multiObjectDelete.js | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 73a4d6167e..81924b69bf 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -382,10 +382,10 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, if (!objectMD) { // User is already authorized on the bucket for FULL_CONTROL or WRITE or // bucket has canned ACL public-read-write - if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') { - results[_requestType] = actionImplicitDenies[_requestType] === false; - return; - } + // if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') { + // results[_requestType] = actionImplicitDenies[_requestType] === false; + // return; + // } // check bucket has read access // 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, diff --git a/lib/api/multiObjectDelete.js b/lib/api/multiObjectDelete.js index 6f540ce40f..e6b02a33fa 100644 --- a/lib/api/multiObjectDelete.js +++ b/lib/api/multiObjectDelete.js @@ -16,7 +16,6 @@ const { isBucketAuthorized, evaluateBucketPolicyWithIAM } = const { preprocessingVersioningDelete } = require('./apiUtils/object/versioning'); const createAndStoreObject = require('./apiUtils/object/createAndStoreObject'); -const monitoring = require('../utilities/metrics'); const { metadataGetObject } = require('../metadata/metadataUtils'); const { config } = require('../Config'); const { hasGovernanceBypassHeader, checkUserGovernanceBypass, ObjectLockInfo }