From 760ab8947810fca7d1f1e252cffdfa6478197238 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 29 Jul 2026 18:27:54 +0200 Subject: [PATCH 1/7] Send signatureVersion and authType unswapped to Vault checkPolicies multiObjectDelete assigned authType to signatureVersion and vice versa when building the checkPolicies request context, so Vault validated and evaluated each field against the wrong value. Issue: CLDSRV-963 --- lib/api/multiObjectDelete.js | 4 +- tests/unit/api/multiObjectDelete.js | 59 ++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/lib/api/multiObjectDelete.js b/lib/api/multiObjectDelete.js index e62e6b756e..4db9b4f4d7 100644 --- a/lib/api/multiObjectDelete.js +++ b/lib/api/multiObjectDelete.js @@ -549,8 +549,8 @@ function multiObjectDelete(authInfo, request, log, callback) { awsService: 's3', locationConstraint: null, requesterInfo: authInfo, - signatureVersion: authParams.params.data.authType, - authType: authParams.params.data.signatureVersion, + signatureVersion: authParams.params.data.signatureVersion, + authType: authParams.params.data.authType, signatureAge: authParams.params.data.signatureAge, }, parameterize: { diff --git a/tests/unit/api/multiObjectDelete.js b/tests/unit/api/multiObjectDelete.js index b3d71d03ec..d1fcd3ba7f 100644 --- a/tests/unit/api/multiObjectDelete.js +++ b/tests/unit/api/multiObjectDelete.js @@ -1,6 +1,6 @@ const crypto = require('crypto'); const assert = require('assert'); -const { errors, storage } = require('arsenal'); +const { auth, errors, storage } = require('arsenal'); const { decodeObjectVersion, getObjMetadataAndDelete, initializeMultiObjectDeleteWithBatchingSupport } = require('../../../lib/api/multiObjectDelete'); @@ -28,6 +28,7 @@ const objectKey1 = 'objectName1'; const objectKey2 = 'objectName2'; const metadataUtils = require('../../../lib/metadata/metadataUtils'); const services = require('../../../lib/services'); +const vault = require('../../../lib/auth/vault'); const { BucketInfo } = require('arsenal/build/lib/models'); const testBucketPutRequest = new DummyRequest({ bucketName, @@ -456,3 +457,59 @@ describe('multiObjectDelete function', () => { }); }); }); + +describe('multiObjectDelete checkPolicies request context', () => { + afterEach(() => { + sinon.restore(); + }); + + it('should forward auth params unswapped to vault, including a zero signatureAge', done => { + const post = 'objectname'; + const request = new DummyRequest({ + bucketName: 'bucketname', + objectKey: 'objectname', + parsedHost: 'localhost', + headers: { + 'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64'), + }, + post, + socket: { + remoteAddress: '127.0.0.1', + }, + url: '/bucketname', + }); + // IAM user so that the request goes through the checkPolicies path + const userAuthInfo = makeAuthInfo('accessKey1', 'testuser'); + + sinon.stub(metadataWrapper, 'getBucket').callsFake((bucketName, log, cb) => + cb(null, new BucketInfo( + 'bucketname', + userAuthInfo.getCanonicalID(), + 'accountA', + new Date().toISOString(), + 15, + ), undefined)); + sinon.stub(auth.server, 'extractParams').returns({ + params: { + version: 4, + data: { + signatureVersion: 'AWS4-HMAC-SHA256', + authType: 'REST-HEADER', + signatureAge: 0, + }, + }, + }); + const checkPoliciesStub = sinon.stub(vault, 'checkPolicies') + .callsFake((requestContextParams, arn, log, cb) => cb(errors.AccessDenied)); + + multiObjectDelete.multiObjectDelete(userAuthInfo, request, log, err => { + assert.strictEqual(err, null); + sinon.assert.calledOnce(checkPoliciesStub); + const { constantParams } = checkPoliciesStub.getCall(0).args[0]; + assert.strictEqual(constantParams.signatureVersion, 'AWS4-HMAC-SHA256'); + assert.strictEqual(constantParams.authType, 'REST-HEADER'); + assert.strictEqual(constantParams.signatureAge, 0); + done(); + }); + }); +}); From ea42afc5561901723cf52ab04fc45f6dc27e5fd5 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 29 Jul 2026 18:28:11 +0200 Subject: [PATCH 2/7] Bump project version Issue: CLDSRV-963 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 33561b70b2..4372f1dcfc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.1.14", + "version": "9.1.15", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": { From dbea94184c353120a549dada5ab1e5429c6fc567 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Tue, 4 Aug 2026 20:52:21 +0200 Subject: [PATCH 3/7] Format multiObjectDelete files with prettier Issue: CLDSRV-963 --- lib/api/multiObjectDelete.js | 1022 +++++++++++++++------------ tests/unit/api/multiObjectDelete.js | 351 +++++---- 2 files changed, 766 insertions(+), 607 deletions(-) diff --git a/lib/api/multiObjectDelete.js b/lib/api/multiObjectDelete.js index ca743c0f9e..08d65c5cfa 100644 --- a/lib/api/multiObjectDelete.js +++ b/lib/api/multiObjectDelete.js @@ -9,18 +9,19 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); const services = require('../services'); const vault = require('../auth/vault'); -const { isBucketAuthorized, evaluateBucketPolicyWithIAM } = - require('./apiUtils/authorization/permissionChecks'); -const { preprocessingVersioningDelete } - = require('./apiUtils/object/versioning'); +const { isBucketAuthorized, evaluateBucketPolicyWithIAM } = require('./apiUtils/authorization/permissionChecks'); +const { preprocessingVersioningDelete } = require('./apiUtils/object/versioning'); const createAndStoreObject = require('./apiUtils/object/createAndStoreObject'); const monitoring = require('../utilities/monitoringHandler'); const metadataUtils = require('../metadata/metadataUtils'); const { config } = require('../Config'); const constants = require('../../constants'); const { isRequesterNonAccountUser } = require('./apiUtils/authorization/permissionChecks'); -const { hasGovernanceBypassHeader, checkUserGovernanceBypass, ObjectLockInfo } - = require('./apiUtils/object/objectLockHelpers'); +const { + hasGovernanceBypassHeader, + checkUserGovernanceBypass, + ObjectLockInfo, +} = require('./apiUtils/object/objectLockHelpers'); const requestUtils = policies.requestUtils; const { validObjectKeys } = require('../routes/routeVeeam'); const { deleteVeeamCapabilities } = require('../routes/veeam/delete'); @@ -48,8 +49,7 @@ const { initializeInternalLogRequestQueue, queueInternalLogRequest } = require(' */ - - /* +/* Format of xml response: @@ -65,36 +65,35 @@ const { initializeInternalLogRequestQueue, queueInternalLogRequest } = require(' */ /** -* formats xml for response -* @param {boolean} quietSetting - true if xml should just include error list -* and false if should include deleted list and error list -* @param {object []} errorResults - list of error result objects with each -* object containing -- entry: { key, versionId }, error: arsenal error -* @param {object []} deleted - list of object deleted, an object has the format -* object: { entry, isDeleteMarker, isDeletingDeleteMarker } -* object.entry : above -* object.newDeleteMarker: if deletion resulted in delete marker -* object.isDeletingDeleteMarker: if a delete marker was deleted -* @return {string} xml string -*/ + * formats xml for response + * @param {boolean} quietSetting - true if xml should just include error list + * and false if should include deleted list and error list + * @param {object []} errorResults - list of error result objects with each + * object containing -- entry: { key, versionId }, error: arsenal error + * @param {object []} deleted - list of object deleted, an object has the format + * object: { entry, isDeleteMarker, isDeletingDeleteMarker } + * object.entry : above + * object.newDeleteMarker: if deletion resulted in delete marker + * object.isDeletingDeleteMarker: if a delete marker was deleted + * @return {string} xml string + */ function _formatXML(quietSetting, errorResults, deleted) { let errorXML = []; errorResults.forEach(errorObj => { errorXML.push( - '', - '', escapeForXml(errorObj.entry.key), '', - '', escapeForXml(errorObj.error.message), ''); + '', + '', + escapeForXml(errorObj.entry.key), + '', + '', + escapeForXml(errorObj.error.message), + '', + ); if (errorObj.entry.versionId) { - const version = errorObj.entry.versionId === 'null' ? - 'null' : escapeForXml(errorObj.entry.versionId); + const version = errorObj.entry.versionId === 'null' ? 'null' : escapeForXml(errorObj.entry.versionId); errorXML.push('', version, ''); } - errorXML.push( - '', - escapeForXml(errorObj.error.description), - '', - '' - ); + errorXML.push('', escapeForXml(errorObj.error.description), '', ''); }); errorXML = errorXML.join(''); const xml = [ @@ -115,18 +114,9 @@ function _formatXML(quietSetting, errorResults, deleted) { const isDeleteMarker = version.isDeleteMarker; const deleteMarkerVersionId = version.deleteMarkerVersionId; // if deletion resulted in new delete marker or deleting a delete marker - deletedXML.push( - '', - '', - escapeForXml(version.entry.key), - '' - ); + deletedXML.push('', '', escapeForXml(version.entry.key), ''); if (version.entry.versionId) { - deletedXML.push( - '', - escapeForXml(version.entry.versionId), - '' - ); + deletedXML.push('', escapeForXml(version.entry.versionId), ''); } if (isDeleteMarker) { deletedXML.push( @@ -135,7 +125,7 @@ function _formatXML(quietSetting, errorResults, deleted) { '', '', deleteMarkerVersionId, - '' + '', ); } deletedXML.push(''); @@ -182,8 +172,7 @@ function _parseXml(xmlToParse, next) { function decodeObjectVersion(entry) { let decodedVersionId; if (entry.versionId) { - decodedVersionId = entry.versionId === 'null' ? - 'null' : versionIdUtils.decode(entry.versionId); + decodedVersionId = entry.versionId === 'null' ? 'null' : versionIdUtils.decode(entry.versionId); } if (decodedVersionId instanceof Error) { return [errors.NoSuchVersion]; @@ -231,25 +220,35 @@ function initializeMultiObjectDeleteWithBatchingSupport(bucketName, inPlay, log, } /** -* gets object metadata and deletes object -* @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info -* @param {string} canonicalID - canonicalId of requester -* @param {object} request - http request -* @param {string} bucketName - bucketName -* @param {BucketInfo} bucket - bucket -* @param {boolean} quietSetting - true if xml should just include error list -* and false if should include deleted list and error list -* @param {object []} errorResults - list of error result objects with each -* object containing -- key: objectName, error: arsenal error -* @param {string []} inPlay - list of object keys still in play -* @param {object} log - logger object -* @param {function} next - callback to next step in waterfall -* @return {undefined} -* @callback called with (err, quietSetting, errorResults, numOfObjects, -* successfullyDeleted, totalContentLengthDeleted) -*/ -function getObjMetadataAndDelete(authInfo, canonicalID, request, - bucketName, bucket, quietSetting, errorResults, inPlay, log, next) { + * gets object metadata and deletes object + * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info + * @param {string} canonicalID - canonicalId of requester + * @param {object} request - http request + * @param {string} bucketName - bucketName + * @param {BucketInfo} bucket - bucket + * @param {boolean} quietSetting - true if xml should just include error list + * and false if should include deleted list and error list + * @param {object []} errorResults - list of error result objects with each + * object containing -- key: objectName, error: arsenal error + * @param {string []} inPlay - list of object keys still in play + * @param {object} log - logger object + * @param {function} next - callback to next step in waterfall + * @return {undefined} + * @callback called with (err, quietSetting, errorResults, numOfObjects, + * successfullyDeleted, totalContentLengthDeleted) + */ +function getObjMetadataAndDelete( + authInfo, + canonicalID, + request, + bucketName, + bucket, + quietSetting, + errorResults, + inPlay, + log, + next, +) { const successfullyDeleted = []; let totalContentLengthDeleted = 0; let numOfObjectsRemoved = 0; @@ -260,211 +259,301 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, // Initialize the queue for internal log request logging before concurrent execution initializeInternalLogRequestQueue(request); - return async.waterfall([ - callback => initializeMultiObjectDeleteWithBatchingSupport(bucketName, inPlay, log, callback), - (cache, callback) => async.forEachLimit(inPlay, config.multiObjectDeleteConcurrency, (entry, moveOn) => { - async.waterfall([ - callback => callback(...decodeObjectVersion(entry, bucketName)), - // for obj deletes, no need to check acl's at object level - // (authority is at the bucket level for obj deletes) - (versionId, callback) => metadataUtils.metadataGetObject(bucketName, entry.key, - versionId, cache, log, (err, objMD) => callback(err, objMD, versionId)), - (objMD, versionId, callback) => { - if (!objMD) { - const verCfg = bucket.getVersioningConfiguration(); - // To adhere to AWS behavior, create a delete marker - // if trying to delete an object that does not exist - // when versioning has been configured - if (verCfg && !entry.versionId) { - log.debug('trying to delete specific version ' + - 'that does not exist'); - return callback(null, objMD, versionId); - } - // otherwise if particular key does not exist, AWS - // returns success for key so add to successfullyDeleted - // list and move on - successfullyDeleted.push({ entry }); - return callback(skipError); - } - if (versionId && objMD.location && - Array.isArray(objMD.location) && objMD.location[0]) { - // we need this information for data deletes to AWS - // eslint-disable-next-line no-param-reassign - objMD.location[0].deleteVersion = true; - } - return callback(null, objMD, versionId); - }, - (objMD, versionId, callback) => { - // AWS only returns an object lock error if a version id - // is specified, else continue to create a delete marker - if (!versionId || !bucket.isObjectLockEnabled()) { - return callback(null, null, objMD, versionId); - } - const hasGovernanceBypass = hasGovernanceBypassHeader(request.headers); - if (hasGovernanceBypass && isRequesterNonAccountUser(authInfo)) { - return checkUserGovernanceBypass(request, authInfo, bucket, entry.key, log, error => { - if (error && error.is.AccessDenied) { - log.debug('user does not have BypassGovernanceRetention and object is locked', - { error }); - return callback(objectLockedError); - } - if (error) { - return callback(error); - } - return callback(null, hasGovernanceBypass, objMD, versionId); - }); - } - return callback(null, hasGovernanceBypass, objMD, versionId); - }, - (hasGovernanceBypass, objMD, versionId, callback) => { - // AWS only returns an object lock error if a version id - // is specified, else continue to create a delete marker - if (!versionId || !bucket.isObjectLockEnabled()) { - return callback(null, objMD, versionId); - } - const objLockInfo = new ObjectLockInfo({ - mode: objMD.retentionMode, - date: objMD.retentionDate, - legalHold: objMD.legalHold || false, - }); + return async.waterfall( + [ + callback => initializeMultiObjectDeleteWithBatchingSupport(bucketName, inPlay, log, callback), + (cache, callback) => + async.forEachLimit( + inPlay, + config.multiObjectDeleteConcurrency, + (entry, moveOn) => { + async.waterfall( + [ + callback => callback(...decodeObjectVersion(entry, bucketName)), + // for obj deletes, no need to check acl's at object level + // (authority is at the bucket level for obj deletes) + (versionId, callback) => + metadataUtils.metadataGetObject( + bucketName, + entry.key, + versionId, + cache, + log, + (err, objMD) => callback(err, objMD, versionId), + ), + (objMD, versionId, callback) => { + if (!objMD) { + const verCfg = bucket.getVersioningConfiguration(); + // To adhere to AWS behavior, create a delete marker + // if trying to delete an object that does not exist + // when versioning has been configured + if (verCfg && !entry.versionId) { + log.debug('trying to delete specific version ' + 'that does not exist'); + return callback(null, objMD, versionId); + } + // otherwise if particular key does not exist, AWS + // returns success for key so add to successfullyDeleted + // list and move on + successfullyDeleted.push({ entry }); + return callback(skipError); + } + if ( + versionId && + objMD.location && + Array.isArray(objMD.location) && + objMD.location[0] + ) { + // we need this information for data deletes to AWS + // eslint-disable-next-line no-param-reassign + objMD.location[0].deleteVersion = true; + } + return callback(null, objMD, versionId); + }, + (objMD, versionId, callback) => { + // AWS only returns an object lock error if a version id + // is specified, else continue to create a delete marker + if (!versionId || !bucket.isObjectLockEnabled()) { + return callback(null, null, objMD, versionId); + } + const hasGovernanceBypass = hasGovernanceBypassHeader(request.headers); + if (hasGovernanceBypass && isRequesterNonAccountUser(authInfo)) { + return checkUserGovernanceBypass( + request, + authInfo, + bucket, + entry.key, + log, + error => { + if (error && error.is.AccessDenied) { + log.debug( + 'user does not have BypassGovernanceRetention ' + + 'and object is locked', + { error }, + ); + return callback(objectLockedError); + } + if (error) { + return callback(error); + } + return callback(null, hasGovernanceBypass, objMD, versionId); + }, + ); + } + return callback(null, hasGovernanceBypass, objMD, versionId); + }, + (hasGovernanceBypass, objMD, versionId, callback) => { + // AWS only returns an object lock error if a version id + // is specified, else continue to create a delete marker + if (!versionId || !bucket.isObjectLockEnabled()) { + return callback(null, objMD, versionId); + } + const objLockInfo = new ObjectLockInfo({ + mode: objMD.retentionMode, + date: objMD.retentionDate, + legalHold: objMD.legalHold || false, + }); - // If the object can not be deleted raise an error - if (!objLockInfo.canModifyObject(hasGovernanceBypass)) { - log.debug('trying to delete locked object'); - return callback(objectLockedError); - } + // If the object can not be deleted raise an error + if (!objLockInfo.canModifyObject(hasGovernanceBypass)) { + log.debug('trying to delete locked object'); + return callback(objectLockedError); + } - return callback(null, objMD, versionId); - }, - (objMD, versionId, callback) => { - const bytes = processBytesToWrite('objectDelete', bucket, versionId, 0, objMD); - return validateQuotas(request, bucket, request.accountQuotas, ['objectDelete'], - 'objectDelete', bytes, false, log, err => callback(err, objMD, versionId)); - }, - (objMD, versionId, callback) => { - const options = preprocessingVersioningDelete( - bucketName, bucket, objMD, versionId, config.nullVersionCompatMode); - const deleteInfo = {}; - if (options && options.deleteData) { - options.overheadField = overheadField; - deleteInfo.deleted = true; - if (!_deleteRequiresOplogUpdate(objMD, bucket)) { - options.doesNotNeedOpogUpdate = true; - } - if (objMD.uploadId) { - options.replayId = objMD.uploadId; - } - return services.deleteObject(bucketName, objMD, - entry.key, options, config.multiObjectDeleteEnableOptimizations, log, - 's3:ObjectRemoved:Delete', (err, toDelete) => { - if (err) { - return callback(err); + return callback(null, objMD, versionId); + }, + (objMD, versionId, callback) => { + const bytes = processBytesToWrite('objectDelete', bucket, versionId, 0, objMD); + return validateQuotas( + request, + bucket, + request.accountQuotas, + ['objectDelete'], + 'objectDelete', + bytes, + false, + log, + err => callback(err, objMD, versionId), + ); + }, + (objMD, versionId, callback) => { + const options = preprocessingVersioningDelete( + bucketName, + bucket, + objMD, + versionId, + config.nullVersionCompatMode, + ); + const deleteInfo = {}; + if (options && options.deleteData) { + options.overheadField = overheadField; + deleteInfo.deleted = true; + if (!_deleteRequiresOplogUpdate(objMD, bucket)) { + options.doesNotNeedOpogUpdate = true; + } + if (objMD.uploadId) { + options.replayId = objMD.uploadId; + } + return services.deleteObject( + bucketName, + objMD, + entry.key, + options, + config.multiObjectDeleteEnableOptimizations, + log, + 's3:ObjectRemoved:Delete', + (err, toDelete) => { + if (err) { + return callback(err); + } + if (toDelete) { + deleteFromStorage = deleteFromStorage.concat(toDelete); + } + return callback(null, objMD, deleteInfo); + }, + ); + } + deleteInfo.newDeleteMarker = true; + // This call will create a delete-marker + return createAndStoreObject( + bucketName, + bucket, + entry.key, + objMD, + authInfo, + canonicalID, + null, + request, + deleteInfo.newDeleteMarker, + null, + overheadField, + log, + 's3:ObjectRemoved:DeleteMarkerCreated', + (err, result) => callback(err, objMD, deleteInfo, result.versionId), + ); + }, + ], + (err, objMD, deleteInfo, versionId) => { + if (err === skipError) { + // Object doesn't exist - log without object size (AWS behavior) + queueInternalLogRequest(request, { + objectKey: entry.key, + objectSize: null, + error: null, + }); + return moveOn(); + } else if (err === objectLockedError) { + errorResults.push({ entry, error: errors.AccessDenied, objectLocked: true }); + // Log locked object with size if available + const objectSize = + objMD && objMD['content-length'] ? objMD['content-length'] : null; + queueInternalLogRequest(request, { + objectKey: entry.key, + objectSize, + error: errors.AccessDenied, + }); + return moveOn(); + } else if (err) { + log.error('error deleting object', { error: err, entry }); + errorResults.push({ entry, error: err }); + // Log error case with size if available + const objectSize = + objMD && objMD['content-length'] ? objMD['content-length'] : null; + queueInternalLogRequest(request, { objectKey: entry.key, objectSize, error: err }); + return moveOn(); } - if (toDelete) { - deleteFromStorage = deleteFromStorage.concat(toDelete); + if (deleteInfo.deleted && objMD['content-length']) { + numOfObjectsRemoved++; + totalContentLengthDeleted += objMD['content-length']; } - return callback(null, objMD, deleteInfo); - }); - } - deleteInfo.newDeleteMarker = true; - // This call will create a delete-marker - return createAndStoreObject(bucketName, bucket, entry.key, - objMD, authInfo, canonicalID, null, request, - deleteInfo.newDeleteMarker, null, overheadField, log, - 's3:ObjectRemoved:DeleteMarkerCreated', (err, result) => - callback(err, objMD, deleteInfo, result.versionId)); - }, - ], (err, objMD, deleteInfo, versionId) => { - if (err === skipError) { - // Object doesn't exist - log without object size (AWS behavior) - queueInternalLogRequest(request, { objectKey: entry.key, objectSize: null, error: null }); - return moveOn(); - } else if (err === objectLockedError) { - errorResults.push({ entry, error: errors.AccessDenied, objectLocked: true }); - // Log locked object with size if available - const objectSize = objMD && objMD['content-length'] ? objMD['content-length'] : null; - queueInternalLogRequest(request, { objectKey: entry.key, objectSize, error: errors.AccessDenied }); - return moveOn(); - } else if (err) { - log.error('error deleting object', { error: err, entry }); - errorResults.push({ entry, error: err }); - // Log error case with size if available - const objectSize = objMD && objMD['content-length'] ? objMD['content-length'] : null; - queueInternalLogRequest(request, { objectKey: entry.key, objectSize, error: err }); - return moveOn(); - } - if (deleteInfo.deleted && objMD['content-length']) { - numOfObjectsRemoved++; - totalContentLengthDeleted += objMD['content-length']; - } - let isDeleteMarker; - let deleteMarkerVersionId; - // - If trying to delete an object that does not exist (if a new - // delete marker was created) - // - Or if an object exists but no version was specified - // return DeleteMarkerVersionId equals the versionID of the marker - // you just generated and DeleteMarker tag equals true - if (deleteInfo.newDeleteMarker) { - isDeleteMarker = true; - deleteMarkerVersionId = versionIdUtils.encode(versionId); - // In this case we are putting a new object (i.e., the delete - // marker), so we decrement the numOfObjectsRemoved value. - numOfObjectsRemoved--; - // If trying to delete a delete marker, DeleteMarkerVersionId equals - // deleteMarker's versionID and DeleteMarker equals true - } else if (objMD && objMD.isDeleteMarker) { - isDeleteMarker = true; - deleteMarkerVersionId = entry.versionId; - } - successfullyDeleted.push({ - entry, isDeleteMarker, - deleteMarkerVersionId, - }); - // Queue successful deletion with object size - const objectSize = objMD && objMD['content-length'] ? objMD['content-length'] : null; - queueInternalLogRequest(request, { objectKey: entry.key, objectSize, error: null }); - return moveOn(); - }); - }, - // end of forEach func - err => { - // Batch delete all objects - const onDone = () => callback(err, quietSetting, errorResults, numOfObjectsRemoved, - successfullyDeleted, totalContentLengthDeleted, bucket); - - if (err && deleteFromStorage.length === 0) { - log.trace('no objects to delete from data backend'); - return onDone(); - } - // If error but we have objects in the list, delete them to ensure - // consistent state. - log.trace('deleting objects from data backend'); + let isDeleteMarker; + let deleteMarkerVersionId; + // - If trying to delete an object that does not exist (if a new + // delete marker was created) + // - Or if an object exists but no version was specified + // return DeleteMarkerVersionId equals the versionID of the marker + // you just generated and DeleteMarker tag equals true + if (deleteInfo.newDeleteMarker) { + isDeleteMarker = true; + deleteMarkerVersionId = versionIdUtils.encode(versionId); + // In this case we are putting a new object (i.e., the delete + // marker), so we decrement the numOfObjectsRemoved value. + numOfObjectsRemoved--; + // If trying to delete a delete marker, DeleteMarkerVersionId equals + // deleteMarker's versionID and DeleteMarker equals true + } else if (objMD && objMD.isDeleteMarker) { + isDeleteMarker = true; + deleteMarkerVersionId = entry.versionId; + } + successfullyDeleted.push({ + entry, + isDeleteMarker, + deleteMarkerVersionId, + }); + // Queue successful deletion with object size + const objectSize = objMD && objMD['content-length'] ? objMD['content-length'] : null; + queueInternalLogRequest(request, { objectKey: entry.key, objectSize, error: null }); + return moveOn(); + }, + ); + }, + // end of forEach func + err => { + // Batch delete all objects + const onDone = () => + callback( + err, + quietSetting, + errorResults, + numOfObjectsRemoved, + successfullyDeleted, + totalContentLengthDeleted, + bucket, + ); - // Split the array into chunks - const chunks = []; - while (deleteFromStorage.length > 0) { - chunks.push(deleteFromStorage.splice(0, config.multiObjectDeleteConcurrency)); - } + if (err && deleteFromStorage.length === 0) { + log.trace('no objects to delete from data backend'); + return onDone(); + } + // If error but we have objects in the list, delete them to ensure + // consistent state. + log.trace('deleting objects from data backend'); - return async.each(chunks, (chunk, done) => data.batchDelete(chunk, null, null, - logger.newRequestLoggerFromSerializedUids(log.getSerializedUids()), done), - err => { - if (err) { - log.error('error deleting objects from data backend', { error: err }); - return onDone(err); + // Split the array into chunks + const chunks = []; + while (deleteFromStorage.length > 0) { + chunks.push(deleteFromStorage.splice(0, config.multiObjectDeleteConcurrency)); } - return onDone(); - }); - }), - ], (err, ...results) => { - // if general error from metadata return error - if (err) { - monitoring.promMetrics('DELETE', bucketName, err.code, - 'multiObjectDelete'); - return next(err); - } - return next(null, ...results); - }); + + return async.each( + chunks, + (chunk, done) => + data.batchDelete( + chunk, + null, + null, + logger.newRequestLoggerFromSerializedUids(log.getSerializedUids()), + done, + ), + err => { + if (err) { + log.error('error deleting objects from data backend', { error: err }); + return onDone(err); + } + return onDone(); + }, + ); + }, + ), + ], + (err, ...results) => { + // if general error from metadata return error + if (err) { + monitoring.promMetrics('DELETE', bucketName, err.code, 'multiObjectDelete'); + return next(err); + } + return next(null, ...results); + }, + ); } /** @@ -484,8 +573,7 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, function multiObjectDelete(authInfo, request, log, callback) { log.debug('processing request', { method: 'multiObjectDelete' }); if (!request.post) { - monitoring.promMetrics('DELETE', request.bucketName, 400, - 'multiObjectDelete'); + monitoring.promMetrics('DELETE', request.bucketName, 400, 'multiObjectDelete'); return callback(errors.MissingRequestBodyError); } @@ -495,217 +583,241 @@ function multiObjectDelete(authInfo, request, log, callback) { const ip = requestUtils.getClientIp(request, config); const isSecure = requestUtils.getHttpProtocolSecurity(request, config); - return async.waterfall([ - function parseXML(next) { - return _parseXml(request.post, - (err, quietSetting, objects) => { + return async.waterfall( + [ + function parseXML(next) { + return _parseXml(request.post, (err, quietSetting, objects) => { if (err || objects.length < 1 || objects.length > constants.maxMultiObjectDeleteLen) { return next(errors.MalformedXML); } return next(null, quietSetting, objects); }); - }, - function checkBucketMetadata(quietSetting, objects, next) { - const errorResults = []; - return metadata.getBucket(bucketName, log, (err, bucketMD, raftSessionId) => { - 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); - } - metadataUtils.storeServerAccessLogInfo(request, bucketMD, raftSessionId); - // The implicit deny flag is ignored in the DeleteObjects API, as authorization only - // affects the objects. - if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo, 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 - objects.forEach(entry => { - errorResults.push({ - entry, - error: errors.AccessDenied, - }); - }); - // by sending an empty array as the objects 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, objects, bucketMD); - }); - }, - function checkPolicies(quietSetting, errorResults, objects, bucketMD, next) { - // track keys that are still on track to be deleted - const inPlay = []; - // if request from account, no need to check policies - // all objects are inPlay so send array of object keys - // as inPlay argument - if (!isRequesterNonAccountUser(authInfo)) { - return next(null, quietSetting, errorResults, objects, bucketMD); - } - - // TODO: once arsenal's extractParams is separated from doAuth - // function, refactor so only extract once and send - // params on to this api - const authParams = auth.server.extractParams(request, log, - 's3', request.query); - const requestContextParams = { - constantParams: { - headers: request.headers, - query: request.query, - generalResource: request.bucketName, - requesterIp: ip, - sslEnabled: isSecure, - apiMethod: 'objectDelete', - awsService: 's3', - locationConstraint: null, - requesterInfo: authInfo, - signatureVersion: authParams.params.data.signatureVersion, - authType: authParams.params.data.authType, - signatureAge: authParams.params.data.signatureAge, - }, - parameterize: { - // eslint-disable-next-line - specificResource: objects.map(entry => { - return { - key: entry.key, - versionId: entry.versionId, - }; - }), - }, - }; - return vault.checkPolicies(requestContextParams, authInfo.getArn(), - log, (err, authorizationResults) => { - // there were no policies so received a blanket AccessDenied - if (err?.is?.AccessDenied) { - objects.forEach(entry => { - errorResults.push({ - entry, - error: errors.AccessDenied }); - }); - // send empty array for inPlay - return next(null, quietSetting, errorResults, [], bucketMD); - } + }, + function checkBucketMetadata(quietSetting, objects, next) { + const errorResults = []; + return metadata.getBucket(bucketName, log, (err, bucketMD, raftSessionId) => { if (err) { - log.trace('error checking policies', { - error: err, - method: 'multiObjectDelete.checkPolicies', - }); + log.trace('error retrieving bucket metadata', { error: err }); return next(err); } - if (objects.length !== authorizationResults.length) { - log.error('vault did not return correct number of ' + - 'authorization results', { - authorizationResultsLength: - authorizationResults.length, - objectsLength: objects.length, - }); - return next(errors.InternalError); + // check whether bucket has transient or deleted flag + if (bucketShield(bucketMD, 'objectDelete')) { + return next(errors.NoSuchBucket); } - // Convert authorization results into an easier to handle format - const actionImplicitDenies = authorizationResults.reduce((acc, curr, idx) => { - const apiMethod = authorizationResults[idx].action; - // 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, - // arn: arn:aws:s3:::bucket/object, - // versionId: sampleversionId } unless not allowed - // in which case no isAllowed key will be present - const slashIndex = result.arn.indexOf('/'); - if (slashIndex === -1) { - log.error('wrong arn format from vault'); - return next(errors.InternalError); - } - const entry = { - key: result.arn.slice(slashIndex + 1), - versionId: result.versionId, - }; - // Deny immediately if there is an explicit deny - if (!result.isImplicit && !result.isAllowed) { + metadataUtils.storeServerAccessLogInfo(request, bucketMD, raftSessionId); + // The implicit deny flag is ignored in the DeleteObjects API, as authorization only + // affects the objects. + if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo, 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 + objects.forEach(entry => { errorResults.push({ entry, error: errors.AccessDenied, }); - continue; + }); + // by sending an empty array as the objects 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, objects, bucketMD); + }); + }, + function checkPolicies(quietSetting, errorResults, objects, bucketMD, next) { + // track keys that are still on track to be deleted + const inPlay = []; + // if request from account, no need to check policies + // all objects are inPlay so send array of object keys + // as inPlay argument + if (!isRequesterNonAccountUser(authInfo)) { + return next(null, quietSetting, errorResults, objects, bucketMD); + } + + // TODO: once arsenal's extractParams is separated from doAuth + // function, refactor so only extract once and send + // params on to this api + const authParams = auth.server.extractParams(request, log, 's3', request.query); + const requestContextParams = { + constantParams: { + headers: request.headers, + query: request.query, + generalResource: request.bucketName, + requesterIp: ip, + sslEnabled: isSecure, + apiMethod: 'objectDelete', + awsService: 's3', + locationConstraint: null, + requesterInfo: authInfo, + signatureVersion: authParams.params.data.signatureVersion, + authType: authParams.params.data.authType, + signatureAge: authParams.params.data.signatureAge, + }, + parameterize: { + // eslint-disable-next-line + specificResource: objects.map(entry => { + return { + key: entry.key, + versionId: entry.versionId, + }; + }), + }, + }; + return vault.checkPolicies( + requestContextParams, + authInfo.getArn(), + log, + (err, authorizationResults) => { + // there were no policies so received a blanket AccessDenied + if (err?.is?.AccessDenied) { + objects.forEach(entry => { + errorResults.push({ + entry, + error: errors.AccessDenied, + }); + }); + // send empty array for inPlay + return next(null, quietSetting, errorResults, [], bucketMD); } + if (err) { + log.trace('error checking policies', { + error: err, + method: 'multiObjectDelete.checkPolicies', + }); + return next(err); + } + if (objects.length !== authorizationResults.length) { + log.error('vault did not return correct number of ' + 'authorization results', { + authorizationResultsLength: authorizationResults.length, + objectsLength: objects.length, + }); + return next(errors.InternalError); + } + // Convert authorization results into an easier to handle format + const actionImplicitDenies = authorizationResults.reduce((acc, curr, idx) => { + const apiMethod = authorizationResults[idx].action; + // 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, + // arn: arn:aws:s3:::bucket/object, + // versionId: sampleversionId } unless not allowed + // in which case no isAllowed key will be present + const slashIndex = result.arn.indexOf('/'); + if (slashIndex === -1) { + log.error('wrong arn format from vault'); + return next(errors.InternalError); + } + const entry = { + key: result.arn.slice(slashIndex + 1), + versionId: result.versionId, + }; + // 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(actionImplicitDenies), - canonicalID, - authInfo, - actionImplicitDenies, - log, - request); + // Evaluate against the bucket policies + const areAllActionsAllowed = evaluateBucketPolicyWithIAM( + bucketMD, + Object.keys(actionImplicitDenies), + canonicalID, + authInfo, + actionImplicitDenies, + log, + request, + ); - if (areAllActionsAllowed) { - if (validObjectKeys.includes(entry.key)) { - inPlayInternal.push(entry.key); + if (areAllActionsAllowed) { + if (validObjectKeys.includes(entry.key)) { + inPlayInternal.push(entry.key); + } else { + inPlay.push(entry); + } } else { - inPlay.push(entry); + errorResults.push({ + entry, + error: errors.AccessDenied, + }); } - } else { - errorResults.push({ - entry, - error: errors.AccessDenied, - }); } - } - return next(null, quietSetting, errorResults, inPlay, bucketMD); - }); - }, - function handleInternalFiles(quietSetting, errorResults, inPlay, bucketMD, next) { - return async.each(inPlayInternal, - (localInPlay, next) => deleteVeeamCapabilities(bucketName, localInPlay, bucketMD, log, next), - err => next(err, quietSetting, errorResults, inPlay, bucketMD)); - }, - function getObjMetadataAndDeleteStep(quietSetting, errorResults, inPlay, - bucket, next) { - return getObjMetadataAndDelete(authInfo, canonicalID, request, - bucketName, bucket, quietSetting, errorResults, inPlay, - log, next); + return next(null, quietSetting, errorResults, inPlay, bucketMD); + }, + ); + }, + function handleInternalFiles(quietSetting, errorResults, inPlay, bucketMD, next) { + return async.each( + inPlayInternal, + (localInPlay, next) => deleteVeeamCapabilities(bucketName, localInPlay, bucketMD, log, next), + err => next(err, quietSetting, errorResults, inPlay, bucketMD), + ); + }, + function getObjMetadataAndDeleteStep(quietSetting, errorResults, inPlay, bucket, next) { + return getObjMetadataAndDelete( + authInfo, + canonicalID, + request, + bucketName, + bucket, + quietSetting, + errorResults, + inPlay, + log, + next, + ); + }, + ], + ( + err, + quietSetting, + errorResults, + numOfObjectsRemoved, + successfullyDeleted, + totalContentLengthDeleted, + bucket, + ) => { + const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); + if (err) { + monitoring.promMetrics('DELETE', bucketName, err.code, 'multiObjectDelete'); + return callback(err, null, corsHeaders); + } + const xml = _formatXML(quietSetting, errorResults, successfullyDeleted); + const deletedKeys = successfullyDeleted.map(item => item.key); + const removedDeleteMarkers = successfullyDeleted.filter( + item => item.isDeleteMarker && item.entry && item.entry.versionId, + ).length; + pushMetric('multiObjectDelete', log, { + authInfo, + canonicalID: bucket ? bucket.getOwner() : '', + bucket: bucketName, + keys: deletedKeys, + byteLength: Number.parseInt(totalContentLengthDeleted, 10), + numberOfObjects: numOfObjectsRemoved, + removedDeleteMarkers, + isDelete: true, + }); + monitoring.promMetrics( + 'DELETE', + bucketName, + '200', + 'multiObjectDelete', + Number.parseInt(totalContentLengthDeleted, 10), + null, + null, + numOfObjectsRemoved, + ); + return callback(null, xml, corsHeaders); }, - ], (err, quietSetting, errorResults, numOfObjectsRemoved, - successfullyDeleted, totalContentLengthDeleted, bucket) => { - const corsHeaders = collectCorsHeaders(request.headers.origin, - request.method, bucket); - if (err) { - monitoring.promMetrics('DELETE', bucketName, err.code, - 'multiObjectDelete'); - return callback(err, null, corsHeaders); - } - const xml = _formatXML(quietSetting, errorResults, - successfullyDeleted); - const deletedKeys = successfullyDeleted.map(item => item.key); - const removedDeleteMarkers = successfullyDeleted - .filter(item => item.isDeleteMarker && item.entry && item.entry.versionId) - .length; - pushMetric('multiObjectDelete', log, { - authInfo, - canonicalID: bucket ? bucket.getOwner() : '', - bucket: bucketName, - keys: deletedKeys, - byteLength: Number.parseInt(totalContentLengthDeleted, 10), - numberOfObjects: numOfObjectsRemoved, - removedDeleteMarkers, - isDelete: true, - }); - monitoring.promMetrics('DELETE', bucketName, '200', - 'multiObjectDelete', - Number.parseInt(totalContentLengthDeleted, 10), null, null, - numOfObjectsRemoved); - return callback(null, xml, corsHeaders); - }); + ); } module.exports = { diff --git a/tests/unit/api/multiObjectDelete.js b/tests/unit/api/multiObjectDelete.js index d1fcd3ba7f..5cc433d9e7 100644 --- a/tests/unit/api/multiObjectDelete.js +++ b/tests/unit/api/multiObjectDelete.js @@ -2,8 +2,11 @@ const crypto = require('crypto'); const assert = require('assert'); const { auth, errors, storage } = require('arsenal'); -const { decodeObjectVersion, getObjMetadataAndDelete, initializeMultiObjectDeleteWithBatchingSupport } - = require('../../../lib/api/multiObjectDelete'); +const { + decodeObjectVersion, + getObjMetadataAndDelete, + initializeMultiObjectDeleteWithBatchingSupport, +} = require('../../../lib/api/multiObjectDelete'); const multiObjectDelete = require('../../../lib/api/multiObjectDelete'); const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers'); const DummyRequest = require('../DummyRequest'); @@ -40,10 +43,13 @@ const testBucketPutRequest = new DummyRequest({ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { let testPutObjectRequest1; let testPutObjectRequest2; - const request = new DummyRequest({ - headers: {}, - parsedContentLength: contentLength, - }, postBody); + const request = new DummyRequest( + { + headers: {}, + parsedContentLength: contentLength, + }, + postBody, + ); const bucket = { isVersioningEnabled: () => false, getVersioningConfiguration: () => null, @@ -53,34 +59,34 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { beforeEach(done => { cleanup(); sinon.spy(metadataswitch, 'deleteObjectMD'); - testPutObjectRequest1 = new DummyRequest({ - bucketName, - namespace, - objectKey: objectKey1, - headers: {}, - url: `/${bucketName}/${objectKey1}`, - }, postBody); - testPutObjectRequest2 = new DummyRequest({ - bucketName, - namespace, - objectKey: objectKey2, - headers: {}, - url: `/${bucketName}/${objectKey2}`, - }, postBody); + testPutObjectRequest1 = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectKey1, + headers: {}, + url: `/${bucketName}/${objectKey1}`, + }, + postBody, + ); + testPutObjectRequest2 = new DummyRequest( + { + bucketName, + namespace, + objectKey: objectKey2, + headers: {}, + url: `/${bucketName}/${objectKey2}`, + }, + postBody, + ); bucketPut(authInfo, testBucketPutRequest, log, () => { - objectPut(authInfo, testPutObjectRequest1, - undefined, log, () => { - objectPut(authInfo, testPutObjectRequest2, - undefined, log, () => { - assert.strictEqual(metadata.keyMaps - .get(bucketName) - .has(objectKey1), true); - assert.strictEqual(metadata.keyMaps - .get(bucketName) - .has(objectKey2), true); - done(); - }); + objectPut(authInfo, testPutObjectRequest1, undefined, log, () => { + objectPut(authInfo, testPutObjectRequest2, undefined, log, () => { + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), true); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), true); + done(); }); + }); }); }); @@ -88,60 +94,76 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { sinon.restore(); }); - it('should successfully get object metadata and then ' + - 'delete metadata and data', done => { - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, bucket, - true, [], [{ key: objectKey1 }, { key: objectKey2 }], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { + it('should successfully get object metadata and then ' + 'delete metadata and data', done => { + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + [], + [{ key: objectKey1 }, { key: objectKey2 }], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { assert.ifError(err); assert.strictEqual(quietSetting, true); assert.deepStrictEqual(errorResults, []); assert.strictEqual(numOfObjects, 2); assert.strictEqual(totalContentLengthDeleted, contentLength); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey1), false); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey2), false); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), false); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), false); // call to delete data is async so wait 20 ms to check // that data deleted setTimeout(() => { // eslint-disable-next-line - assert.deepStrictEqual(ds, [ , , , ]); + assert.deepStrictEqual(ds, [, , ,]); done(); }, 20); - }); + }, + ); }); it('should return success results if no such key', done => { - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, bucket, - true, [], [{ key: 'madeup1' }, { key: 'madeup2' }], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + [], + [{ key: 'madeup1' }, { key: 'madeup2' }], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { assert.ifError(err); assert.strictEqual(quietSetting, true); assert.deepStrictEqual(errorResults, []); assert.strictEqual(numOfObjects, 0); - assert.strictEqual(totalContentLengthDeleted, - 0); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey1), true); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey2), true); + assert.strictEqual(totalContentLengthDeleted, 0); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), true); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), true); done(); - }); + }, + ); }); - it('should return error results if err from metadata getting object' + - 'is error other than NoSuchKey', done => { + it('should return error results if err from metadata getting object' + 'is error other than NoSuchKey', done => { // we fake an error by calling on an imaginary bucket // even though the getObjMetadataAndDelete function would // never be called if there was no bucket (would error out earlier // in API) - getObjMetadataAndDelete(authInfo, 'foo', request, 'madeupbucket', - bucket, true, [], [{ key: objectKey1 }, { key: objectKey2 }], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + 'madeupbucket', + bucket, + true, + [], + [{ key: objectKey1 }, { key: objectKey2 }], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { assert.ifError(err); assert.strictEqual(quietSetting, true); assert.deepStrictEqual(errorResults, [ @@ -154,31 +176,35 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { error: errors.NoSuchBucket, }, ]); - assert.strictEqual(totalContentLengthDeleted, - 0); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey1), true); - assert.strictEqual(metadata.keyMaps.get(bucketName) - .has(objectKey2), true); + assert.strictEqual(totalContentLengthDeleted, 0); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), true); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), true); done(); - }); + }, + ); }); - it('should return no error or success results if no objects in play', - done => { - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, - bucket, true, [], [], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { - assert.ifError(err); - assert.strictEqual(quietSetting, true); - assert.deepStrictEqual(errorResults, []); - assert.strictEqual(numOfObjects, 0); - assert.strictEqual(totalContentLengthDeleted, - 0); - done(); - }); - }); + it('should return no error or success results if no objects in play', done => { + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + [], + [], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { + assert.ifError(err); + assert.strictEqual(quietSetting, true); + assert.deepStrictEqual(errorResults, []); + assert.strictEqual(numOfObjects, 0); + assert.strictEqual(totalContentLengthDeleted, 0); + done(); + }, + ); + }); it('should pass along error results', done => { const errorResultsSample = [ @@ -191,18 +217,25 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { error: errors.AccessDenied, }, ]; - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, bucket, - true, errorResultsSample, - [{ key: objectKey1 }, { key: objectKey2 }], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + errorResultsSample, + [{ key: objectKey1 }, { key: objectKey2 }], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { assert.ifError(err); assert.strictEqual(quietSetting, true); assert.deepStrictEqual(errorResults, errorResultsSample); assert.strictEqual(numOfObjects, 2); assert.strictEqual(totalContentLengthDeleted, contentLength); done(); - }); + }, + ); }); it('should properly batch delete data even if there are errors in other objects', done => { @@ -210,35 +243,51 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { deleteObjectStub.onCall(0).callsArgWith(7, errors.InternalError); deleteObjectStub.onCall(1).callsArgWith(7, null); - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, bucket, - true, [], [{ key: objectKey1 }, { key: objectKey2 }], log, - (err, quietSetting, errorResults, numOfObjects, - successfullyDeleted, totalContentLengthDeleted) => { - assert.ifError(err); - assert.strictEqual(quietSetting, true); - assert.deepStrictEqual(errorResults, [ - { - entry: { - key: objectKey1, + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + [], + [{ key: objectKey1 }, { key: objectKey2 }], + log, + (err, quietSetting, errorResults, numOfObjects, successfullyDeleted, totalContentLengthDeleted) => { + assert.ifError(err); + assert.strictEqual(quietSetting, true); + assert.deepStrictEqual(errorResults, [ + { + entry: { + key: objectKey1, + }, + error: errors.InternalError, }, - error: errors.InternalError, - }, - ]); - assert.strictEqual(numOfObjects, 1); - assert.strictEqual(totalContentLengthDeleted, contentLength / 2); - // Expect still in memory as we stubbed the function - assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), true); - assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), true); - // ensure object 2 only is in the list of successful deletions - assert.strictEqual(successfullyDeleted.length, 1); - assert.deepStrictEqual(successfullyDeleted[0].entry.key, objectKey2); - return done(); - }); + ]); + assert.strictEqual(numOfObjects, 1); + assert.strictEqual(totalContentLengthDeleted, contentLength / 2); + // Expect still in memory as we stubbed the function + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey1), true); + assert.strictEqual(metadata.keyMaps.get(bucketName).has(objectKey2), true); + // ensure object 2 only is in the list of successful deletions + assert.strictEqual(successfullyDeleted.length, 1); + assert.deepStrictEqual(successfullyDeleted[0].entry.key, objectKey2); + return done(); + }, + ); }); it('should pass overheadField to metadata', done => { - getObjMetadataAndDelete(authInfo, 'foo', request, bucketName, bucket, - true, [], [{ key: objectKey1 }, { key: objectKey2 }], log, + getObjMetadataAndDelete( + authInfo, + 'foo', + request, + bucketName, + bucket, + true, + [], + [{ key: objectKey1 }, { key: objectKey2 }], + log, (err, quietSetting, errorResults, numOfObjects) => { assert.ifError(err); assert.strictEqual(numOfObjects, 2); @@ -248,7 +297,7 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { objectKey1, sinon.match({ overheadField: sinon.match.array }), sinon.match.any, - sinon.match.any + sinon.match.any, ); sinon.assert.calledWith( metadataswitch.deleteObjectMD, @@ -256,10 +305,11 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => { objectKey2, sinon.match({ overheadField: sinon.match.array }), sinon.match.any, - sinon.match.any + sinon.match.any, ); done(); - }); + }, + ); }); }); @@ -312,8 +362,9 @@ describe('initializeMultiObjectDeleteWithBatchingSupport', () => { }); it('should not return an error if the metadataGetObjects function fails', done => { - const metadataGetObjectsStub = - sinon.stub(metadataUtils, 'metadataGetObjects').yields(new Error('metadata error'), null); + const metadataGetObjectsStub = sinon + .stub(metadataUtils, 'metadataGetObjects') + .yields(new Error('metadata error'), null); const objectVersion = 'someVersionId'; sinon.stub(multiObjectDelete, 'decodeObjectVersion').returns([null, objectVersion]); @@ -394,22 +445,16 @@ describe('multiObjectDelete function', () => { }); const authInfo = makeAuthInfo('123456'); - sinon.stub(metadataWrapper, 'getBucket').callsFake((bucketName, log, cb) => - cb(null, new BucketInfo( - 'bucketname', - '123456', - 'accountA', - new Date().toISOString(), - 15, - ), undefined)); + sinon + .stub(metadataWrapper, 'getBucket') + .callsFake((bucketName, log, cb) => + cb(null, new BucketInfo('bucketname', '123456', 'accountA', new Date().toISOString(), 15), undefined), + ); multiObjectDelete.multiObjectDelete(authInfo, request, log, (err, res) => { // Expected result is an access denied on the object, and no error, as the API was authorized assert.strictEqual(err, null); - assert.strictEqual( - res.includes('objectnameAccessDenied'), - true - ); + assert.strictEqual(res.includes('objectnameAccessDenied'), true); done(); }); }); @@ -437,22 +482,16 @@ describe('multiObjectDelete function', () => { }); const authInfo = makeAuthInfo('123456'); - sinon.stub(metadataWrapper, 'getBucket').callsFake((bucketName, log, cb) => - cb(null, new BucketInfo( - 'bucketname', - '123456', - 'accountA', - new Date().toISOString(), - 15, - ), undefined)); + sinon + .stub(metadataWrapper, 'getBucket') + .callsFake((bucketName, log, cb) => + cb(null, new BucketInfo('bucketname', '123456', 'accountA', new Date().toISOString(), 15), undefined), + ); multiObjectDelete.multiObjectDelete(authInfo, request, log, (err, res) => { // Expected result is an access denied on the object, and no error, as the API was authorized assert.strictEqual(err, null); - assert.strictEqual( - res.includes('objectnameAccessDenied'), - true - ); + assert.strictEqual(res.includes('objectnameAccessDenied'), true); done(); }); }); @@ -481,14 +520,21 @@ describe('multiObjectDelete checkPolicies request context', () => { // IAM user so that the request goes through the checkPolicies path const userAuthInfo = makeAuthInfo('accessKey1', 'testuser'); - sinon.stub(metadataWrapper, 'getBucket').callsFake((bucketName, log, cb) => - cb(null, new BucketInfo( - 'bucketname', - userAuthInfo.getCanonicalID(), - 'accountA', - new Date().toISOString(), - 15, - ), undefined)); + sinon + .stub(metadataWrapper, 'getBucket') + .callsFake((bucketName, log, cb) => + cb( + null, + new BucketInfo( + 'bucketname', + userAuthInfo.getCanonicalID(), + 'accountA', + new Date().toISOString(), + 15, + ), + undefined, + ), + ); sinon.stub(auth.server, 'extractParams').returns({ params: { version: 4, @@ -499,7 +545,8 @@ describe('multiObjectDelete checkPolicies request context', () => { }, }, }); - const checkPoliciesStub = sinon.stub(vault, 'checkPolicies') + const checkPoliciesStub = sinon + .stub(vault, 'checkPolicies') .callsFake((requestContextParams, arn, log, cb) => cb(errors.AccessDenied)); multiObjectDelete.multiObjectDelete(userAuthInfo, request, log, err => { From 9b02cb46a488c3982ff18e0f5ba26113f721bb90 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 5 Aug 2026 10:52:12 +0200 Subject: [PATCH 4/7] Send signatureVersion and authType unswapped in bucketPut checkPolicies Issue: CLDSRV-963 --- lib/api/bucketPut.js | 4 ++-- tests/unit/api/bucketPut.js | 42 ++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/api/bucketPut.js b/lib/api/bucketPut.js index 5f3409da04..5bc7019bcc 100644 --- a/lib/api/bucketPut.js +++ b/lib/api/bucketPut.js @@ -126,8 +126,8 @@ function _buildConstantParams({ sslEnabled: isSecure, awsService: 's3', requesterInfo: authInfo, - signatureVersion: authParams.params.data.authType, - authType: authParams.params.data.signatureVersion, + signatureVersion: authParams.params.data.signatureVersion, + authType: authParams.params.data.authType, signatureAge: authParams.params.data.signatureAge, apiMethod, locationConstraint, diff --git a/tests/unit/api/bucketPut.js b/tests/unit/api/bucketPut.js index 529f2947cb..2ed309251c 100644 --- a/tests/unit/api/bucketPut.js +++ b/tests/unit/api/bucketPut.js @@ -1,5 +1,5 @@ const assert = require('assert'); -const { errors } = require('arsenal'); +const { auth, errors } = require('arsenal'); const sinon = require('sinon'); const inMemory = require('../../../lib/kms/in_memory/backend').backend; const vault = require('../../../lib/auth/vault'); @@ -934,3 +934,43 @@ describe('bucketPut API with SSE Configurations', () => { }); }); }); + +describe('bucketPut checkPolicies request context', () => { + afterEach(() => { + sinon.restore(); + cleanup(); + }); + + it('should forward auth params unswapped to vault, including a zero signatureAge', done => { + // IAM user so that the request goes through the checkPolicies path + const userAuthInfo = makeAuthInfo(accessKey, 'testuser'); + sinon.stub(auth.server, 'extractParams').returns({ + params: { + version: 4, + data: { + signatureVersion: 'AWS4-HMAC-SHA256', + authType: 'REST-HEADER', + signatureAge: 0, + }, + }, + }); + const checkPoliciesStub = sinon.stub(vault, 'checkPolicies') + .callsFake((requestContextParams, arn, log, cb) => cb(errors.AccessDenied)); + const request = { + ...testRequest, + socket: { + remoteAddress: '127.0.0.1', + }, + }; + + bucketPut(userAuthInfo, request, log, err => { + assert(err && err.AccessDenied); + sinon.assert.calledOnce(checkPoliciesStub); + const { constantParams } = checkPoliciesStub.getCall(0).args[0][0]; + assert.strictEqual(constantParams.signatureVersion, 'AWS4-HMAC-SHA256'); + assert.strictEqual(constantParams.authType, 'REST-HEADER'); + assert.strictEqual(constantParams.signatureAge, 0); + done(); + }); + }); +}); From af2e3b113e36bc495ed8c1b1ccfce4eaf12d2c54 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 5 Aug 2026 10:56:13 +0200 Subject: [PATCH 5/7] Format bucketPut files with prettier Issue: CLDSRV-963 --- lib/api/bucketPut.js | 188 +++++++++++++++++------------------- tests/unit/api/bucketPut.js | 3 +- 2 files changed, 92 insertions(+), 99 deletions(-) diff --git a/lib/api/bucketPut.js b/lib/api/bucketPut.js index 5bc7019bcc..358d3ab55f 100644 --- a/lib/api/bucketPut.js +++ b/lib/api/bucketPut.js @@ -45,22 +45,19 @@ function checkLocationConstraint(request, locationConstraint, log) { } else if (parsedHost && restEndpoints[parsedHost]) { locationConstraintChecked = restEndpoints[parsedHost]; } else { - log.trace('no location constraint provided on bucket put;' + - 'setting us-east-1'); + log.trace('no location constraint provided on bucket put;' + 'setting us-east-1'); locationConstraintChecked = 'us-east-1'; } if (!locationConstraints[locationConstraintChecked]) { - const errMsg = 'value of the location you are attempting to set - ' + + const errMsg = + 'value of the location you are attempting to set - ' + `${locationConstraintChecked} - is not listed in the ` + 'locationConstraint config'; - log.trace(`locationConstraint is invalid - ${errMsg}`, - { locationConstraint: locationConstraintChecked }); - return { error: errorInstances.InvalidLocationConstraint. - customizeDescription(errMsg) }; + log.trace(`locationConstraint is invalid - ${errMsg}`, { locationConstraint: locationConstraintChecked }); + return { error: errorInstances.InvalidLocationConstraint.customizeDescription(errMsg) }; } - if (locationConstraints[locationConstraintChecked].isCold || - locationConstraints[locationConstraintChecked].isCRR) { + if (locationConstraints[locationConstraintChecked].isCold || locationConstraints[locationConstraintChecked].isCRR) { return { error: errors.InvalidLocationConstraint }; } if (locationConstraintAddon) { @@ -84,18 +81,18 @@ function checkLocationConstraint(request, locationConstraint, log) { function _parseXML(request, log, cb) { if (request.post) { return parseString(request.post, (err, result) => { - if (err || !result.CreateBucketConfiguration - || !result.CreateBucketConfiguration.LocationConstraint - || !result.CreateBucketConfiguration.LocationConstraint[0]) { + if ( + err || + !result.CreateBucketConfiguration || + !result.CreateBucketConfiguration.LocationConstraint || + !result.CreateBucketConfiguration.LocationConstraint[0] + ) { log.debug('request xml is malformed'); return cb(errors.MalformedXML); } - const locationConstraint = result.CreateBucketConfiguration - .LocationConstraint[0]; - log.trace('location constraint', - { locationConstraint }); - const locationCheck = checkLocationConstraint(request, - locationConstraint, log); + const locationConstraint = result.CreateBucketConfiguration.LocationConstraint[0]; + log.trace('location constraint', { locationConstraint }); + const locationCheck = checkLocationConstraint(request, locationConstraint, log); if (locationCheck.error) { return cb(locationCheck.error); } @@ -103,8 +100,7 @@ function _parseXML(request, log, cb) { }); } return process.nextTick(() => { - const locationCheck = checkLocationConstraint(request, - undefined, log); + const locationCheck = checkLocationConstraint(request, undefined, log); if (locationCheck.error) { return cb(locationCheck.error); } @@ -113,7 +109,15 @@ function _parseXML(request, log, cb) { } function _buildConstantParams({ - request, bucketName, authInfo, authParams, ip, isSecure, locationConstraint, apiMethod }) { + request, + bucketName, + authInfo, + authParams, + ip, + isSecure, + locationConstraint, + apiMethod, +}) { return { constantParams: { headers: request.headers, @@ -140,16 +144,15 @@ function _handleAuthResults(locationConstraint, log, cb) { if (err) { return cb(err); } - if (!authorizationResults.every(res => { - if (Array.isArray(res)) { - return res.every(subRes => subRes.isAllowed); - } - return res.isAllowed; - })) { - log.trace( - 'authorization check failed for user', - { locationConstraint }, - ); + if ( + !authorizationResults.every(res => { + if (Array.isArray(res)) { + return res.every(subRes => subRes.isAllowed); + } + return res.isAllowed; + }) + ) { + log.trace('authorization check failed for user', { locationConstraint }); return cb(errors.AccessDenied); } return cb(null, locationConstraint); @@ -177,30 +180,15 @@ function authBucketPut(authParams, bucketName, locationConstraint, request, auth authInfo, locationConstraint, }; - const requestConstantParams = [Object.assign( - baseParams, - { apiMethod: 'bucketPut' }, - )]; + const requestConstantParams = [Object.assign(baseParams, { apiMethod: 'bucketPut' })]; if (_isObjectLockEnabled(request.headers)) { - requestConstantParams.push(Object.assign( - {}, - baseParams, - { apiMethod: 'bucketPutObjectLock' }, - )); - requestConstantParams.push(Object.assign( - {}, - baseParams, - { apiMethod: 'bucketPutVersioning' }, - )); + requestConstantParams.push(Object.assign({}, baseParams, { apiMethod: 'bucketPutObjectLock' })); + requestConstantParams.push(Object.assign({}, baseParams, { apiMethod: 'bucketPutVersioning' })); } if (_isAclProvided(request.headers)) { - requestConstantParams.push(Object.assign( - {}, - baseParams, - { apiMethod: 'bucketPutACL' }, - )); + requestConstantParams.push(Object.assign({}, baseParams, { apiMethod: 'bucketPutACL' })); } return requestConstantParams; @@ -219,66 +207,70 @@ function bucketPut(authInfo, request, log, callback) { if (authInfo.isRequesterPublicUser()) { log.debug('operation not available for public user'); - monitoring.promMetrics( - 'PUT', request.bucketName, 403, 'createBucket'); + monitoring.promMetrics('PUT', request.bucketName, 403, 'createBucket'); return callback(errors.AccessDenied); } if (!aclUtils.checkGrantHeaderValidity(request.headers)) { log.trace('invalid acl header'); - monitoring.promMetrics( - 'PUT', request.bucketName, 400, 'createBucket'); + monitoring.promMetrics('PUT', request.bucketName, 400, 'createBucket'); return callback(errors.InvalidArgument); } const { bucketName } = request; - - if (request.bucketName === 'METADATA' - // Note: for this to work with Vault, would need way to set - // canonical ID to http://acs.zenko.io/accounts/service/clueso - && !authInfo.isRequesterThisServiceAccount('clueso')) { - monitoring.promMetrics( - 'PUT', bucketName, 403, 'createBucket'); - return callback(errorInstances.AccessDenied - .customizeDescription('The bucket METADATA is used ' + - 'for internal purposes')); + if ( + request.bucketName === 'METADATA' && + // Note: for this to work with Vault, would need way to set + // canonical ID to http://acs.zenko.io/accounts/service/clueso + !authInfo.isRequesterThisServiceAccount('clueso') + ) { + monitoring.promMetrics('PUT', bucketName, 403, 'createBucket'); + return callback( + errorInstances.AccessDenied.customizeDescription('The bucket METADATA is used ' + 'for internal purposes'), + ); } - return waterfall([ - next => _parseXML(request, log, next), - (locationConstraint, next) => { - if (!isRequesterNonAccountUser(authInfo)) { - return next(null, locationConstraint); - } + return waterfall( + [ + next => _parseXML(request, log, next), + (locationConstraint, next) => { + if (!isRequesterNonAccountUser(authInfo)) { + return next(null, locationConstraint); + } - const authParams = auth.server.extractParams(request, log, 's3', request.query); - const requestConstantParams = authBucketPut( - authParams, bucketName, locationConstraint, request, authInfo - ); + const authParams = auth.server.extractParams(request, log, 's3', request.query); + const requestConstantParams = authBucketPut( + authParams, + bucketName, + locationConstraint, + request, + authInfo, + ); - return vault.checkPolicies( - requestConstantParams.map(_buildConstantParams), - authInfo.getArn(), - log, - _handleAuthResults(locationConstraint, log, next), - ); - }, - (locationConstraint, next) => createBucket(authInfo, bucketName, - request.headers, locationConstraint, log, (err, previousBucket) => { - // if bucket already existed, gather any relevant cors - // headers - const corsHeaders = collectCorsHeaders( - request.headers.origin, request.method, previousBucket); - if (err) { - return next(err, corsHeaders); - } - pushMetric('createBucket', log, { - authInfo, - bucket: bucketName, - }); - monitoring.promMetrics('PUT', bucketName, '200', 'createBucket'); - return next(null, corsHeaders); - }), - ], callback); + return vault.checkPolicies( + requestConstantParams.map(_buildConstantParams), + authInfo.getArn(), + log, + _handleAuthResults(locationConstraint, log, next), + ); + }, + (locationConstraint, next) => + createBucket(authInfo, bucketName, request.headers, locationConstraint, log, (err, previousBucket) => { + // if bucket already existed, gather any relevant cors + // headers + const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, previousBucket); + if (err) { + return next(err, corsHeaders); + } + pushMetric('createBucket', log, { + authInfo, + bucket: bucketName, + }); + monitoring.promMetrics('PUT', bucketName, '200', 'createBucket'); + return next(null, corsHeaders); + }), + ], + callback, + ); } module.exports = { diff --git a/tests/unit/api/bucketPut.js b/tests/unit/api/bucketPut.js index d7f7242799..f575d78521 100644 --- a/tests/unit/api/bucketPut.js +++ b/tests/unit/api/bucketPut.js @@ -1030,7 +1030,8 @@ describe('bucketPut checkPolicies request context', () => { }, }, }); - const checkPoliciesStub = sinon.stub(vault, 'checkPolicies') + const checkPoliciesStub = sinon + .stub(vault, 'checkPolicies') .callsFake((requestContextParams, arn, log, cb) => cb(errors.AccessDenied)); const request = { ...testRequest, From 5564b2c9863b793c8a2945d69ab916f2b65f4533 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 5 Aug 2026 12:29:48 +0200 Subject: [PATCH 6/7] Assert the exact error in the bucketPut checkPolicies test Issue: CLDSRV-963 --- tests/unit/api/bucketPut.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/api/bucketPut.js b/tests/unit/api/bucketPut.js index 2ed309251c..3b0ba3a51c 100644 --- a/tests/unit/api/bucketPut.js +++ b/tests/unit/api/bucketPut.js @@ -964,7 +964,7 @@ describe('bucketPut checkPolicies request context', () => { }; bucketPut(userAuthInfo, request, log, err => { - assert(err && err.AccessDenied); + assert.strictEqual(err.is.AccessDenied, true); sinon.assert.calledOnce(checkPoliciesStub); const { constantParams } = checkPoliciesStub.getCall(0).args[0][0]; assert.strictEqual(constantParams.signatureVersion, 'AWS4-HMAC-SHA256'); From 0f2f742233b973d03ed0bb81f456427c5109282f Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Wed, 5 Aug 2026 17:10:59 +0200 Subject: [PATCH 7/7] Report the received error in the bucketPut checkPolicies test assertion Issue: CLDSRV-963 --- tests/unit/api/bucketPut.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/api/bucketPut.js b/tests/unit/api/bucketPut.js index 3b0ba3a51c..d9e3344905 100644 --- a/tests/unit/api/bucketPut.js +++ b/tests/unit/api/bucketPut.js @@ -964,7 +964,7 @@ describe('bucketPut checkPolicies request context', () => { }; bucketPut(userAuthInfo, request, log, err => { - assert.strictEqual(err.is.AccessDenied, true); + assert.strictEqual(err.is.AccessDenied, true, `expected AccessDenied, got ${err && err.message}`); sinon.assert.calledOnce(checkPoliciesStub); const { constantParams } = checkPoliciesStub.getCall(0).args[0][0]; assert.strictEqual(constantParams.signatureVersion, 'AWS4-HMAC-SHA256');