diff --git a/lib/api/apiUtils/quotas/quotaUtils.js b/lib/api/apiUtils/quotas/quotaUtils.js index 6c967bec94..1b20de40d4 100644 --- a/lib/api/apiUtils/quotas/quotaUtils.js +++ b/lib/api/apiUtils/quotas/quotaUtils.js @@ -1,11 +1,7 @@ const async = require('async'); const { errors } = require('arsenal'); const monitoring = require('../../../utilities/monitoringHandler'); -const { - actionNeedQuotaCheckCopy, - actionNeedQuotaCheck, - actionWithDataDeletion, -} = require('arsenal').policies; +const { actionNeedQuotaCheck, actionWithDataDeletion } = require('arsenal').policies; const { config } = require('../../../Config'); const QuotaService = require('../../../utilization/instance'); @@ -44,7 +40,7 @@ function processBytesToWrite(apiMethod, bucket, versionId, contentLength, objMD, // but it also replaces the target, which decreases storage bytes -= getHotContentLength(destObjMD); } - } else if (!bucket.isVersioningEnabled() || bucket.isVersioningEnabled() && versionId) { + } else if (!bucket.isVersioningEnabled() || (bucket.isVersioningEnabled() && versionId)) { // object is being deleted (non versioned) or hard-deleted (versioned, as indicated by // the `versionId` field) bytes = -getHotContentLength(objMD); @@ -68,8 +64,7 @@ function processBytesToWrite(apiMethod, bucket, versionId, contentLength, objMD, * @returns {boolean} Returns true if the metric is stale, false otherwise. */ function isMetricStale(metric, resourceType, resourceName, action, inflight, log) { - if (metric.date && Date.now() - new Date(metric.date).getTime() > - QuotaService.maxStaleness) { + if (metric.date && Date.now() - new Date(metric.date).getTime() > QuotaService.maxStaleness) { log.warn('Stale metrics from the quota service, allowing the request', { resourceType, resourceName, @@ -110,70 +105,87 @@ function _evaluateQuotas( let bucketQuotaExceeded = false; let accountQuotaExceeded = false; const creationDate = new Date(bucket.getCreationDate()).getTime(); - return async.parallel({ - bucketQuota: parallelDone => { - if (bucketQuota > 0) { - return QuotaService.getUtilizationMetrics('bucket', - `${bucket.getName()}_${creationDate}`, null, { - action, - inflight, - }, (err, bucketMetrics) => { - if (err || inflight < 0) { - return parallelDone(err); - } - if (!isMetricStale(bucketMetrics, 'bucket', bucket.getName(), action, inflight, log) && - BigInt(bucketMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > bucketQuota) { - log.debug('Bucket quota exceeded', { - bucket: bucket.getName(), + return async.parallel( + { + bucketQuota: parallelDone => { + if (bucketQuota > 0) { + return QuotaService.getUtilizationMetrics( + 'bucket', + `${bucket.getName()}_${creationDate}`, + null, + { action, inflight, - quota: bucketQuota, - bytesTotal: bucketMetrics.bytesTotal, - }); - bucketQuotaExceeded = true; - } - return parallelDone(); - }); - } - return parallelDone(); - }, - accountQuota: parallelDone => { - if (accountQuota > 0 && account?.account) { - return QuotaService.getUtilizationMetrics('account', - account.account, null, { - action, - inflight, - }, (err, accountMetrics) => { - if (err || inflight < 0) { - return parallelDone(err); - } - // Metrics are served as BigInt strings - if (!isMetricStale(accountMetrics, 'account', account.account, action, inflight, log) && - BigInt(accountMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > accountQuota) { - log.debug('Account quota exceeded', { - accountId: account.account, + }, + (err, bucketMetrics) => { + if (err || inflight < 0) { + return parallelDone(err); + } + if ( + !isMetricStale(bucketMetrics, 'bucket', bucket.getName(), action, inflight, log) && + BigInt(bucketMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > bucketQuota + ) { + log.debug('Bucket quota exceeded', { + bucket: bucket.getName(), + action, + inflight, + quota: bucketQuota, + bytesTotal: bucketMetrics.bytesTotal, + }); + bucketQuotaExceeded = true; + } + return parallelDone(); + }, + ); + } + return parallelDone(); + }, + accountQuota: parallelDone => { + if (accountQuota > 0 && account?.account) { + return QuotaService.getUtilizationMetrics( + 'account', + account.account, + null, + { action, inflight, - quota: accountQuota, - bytesTotal: accountMetrics.bytesTotal, - }); - accountQuotaExceeded = true; - } - return parallelDone(); + }, + (err, accountMetrics) => { + if (err || inflight < 0) { + return parallelDone(err); + } + // Metrics are served as BigInt strings + if ( + !isMetricStale(accountMetrics, 'account', account.account, action, inflight, log) && + BigInt(accountMetrics.bytesTotal || 0) + BigInt(inflightForCheck || 0) > accountQuota + ) { + log.debug('Account quota exceeded', { + accountId: account.account, + action, + inflight, + quota: accountQuota, + bytesTotal: accountMetrics.bytesTotal, + }); + accountQuotaExceeded = true; + } + return parallelDone(); + }, + ); + } + return parallelDone(); + }, + }, + err => { + if (err) { + log.warn('Error evaluating quotas', { + error: err.name, + description: err.message, + isInflightDeletion: inflight < 0, }); } - return parallelDone(); + return callback(err, bucketQuotaExceeded, accountQuotaExceeded); }, - }, err => { - if (err) { - log.warn('Error evaluating quotas', { - error: err.name, - description: err.message, - isInflightDeletion: inflight < 0, - }); - } - return callback(err, bucketQuotaExceeded, accountQuotaExceeded); - }); + ); } /** @@ -186,11 +198,13 @@ function _evaluateQuotas( * @returns {undefined} - Returns nothing. */ function monitorQuotaEvaluationDuration(apiMethod, type, code, duration) { - monitoring.quotaEvaluationDuration.labels({ - action: apiMethod, - type, - code, - }).observe(duration / 1e9); + monitoring.quotaEvaluationDuration + .labels({ + action: apiMethod, + type, + code, + }) + .observe(duration / 1e9); } /** @@ -248,76 +262,107 @@ function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight, inflight = 0; } - return async.forEach(apiNames, (apiName, done) => { - // Object copy operations first check the target object, - // meaning the source object, containing the current bytes, - // is checked second. This logic handles these APIs calls by - // ensuring the bytes are positives (i.e., not an object - // replacement). - if (actionNeedQuotaCheckCopy(apiName, apiMethod)) { - // eslint-disable-next-line no-param-reassign - inflight = Math.abs(inflight); - } else if (!actionNeedQuotaCheck[apiName] && !actionWithDataDeletion[apiName]) { - return done(); - } - // When inflights are disabled, the sum of the current utilization metrics - // and the current bytes are compared with the quota. The current bytes - // are not sent to the utilization service. When inflights are enabled, - // the sum of the current utilization metrics only are compared with the - // quota. They include the current inflight bytes sent in the request. - let _inflights = shouldSendInflights ? inflight : undefined; - const inflightForCheck = shouldSendInflights ? 0 : inflight; - return _evaluateQuotas(bucketQuota, accountQuota, bucket, account, _inflights, - inflightForCheck, apiName, log, - (err, _bucketQuotaExceeded, _accountQuotaExceeded) => { - if (err) { - return done(err); - } + return async.forEach( + apiNames, + (apiName, done) => { + // Object copy operations first check the target object, + // meaning the source object, containing the current bytes, + // is checked second. This logic handles these APIs calls by + // ensuring the bytes are positives (i.e., not an object + // replacement). The source is checked with objectGetVersion + // instead of objectGet when the copy specifies its version. + const isCopySource = + (apiName === 'objectGet' || apiName === 'objectGetVersion') && + (apiMethod === 'objectCopy' || apiMethod === 'objectPutCopyPart'); + if (isCopySource) { + // eslint-disable-next-line no-param-reassign + inflight = Math.abs(inflight); + } else if (!actionNeedQuotaCheck[apiName] && !actionWithDataDeletion[apiName]) { + return done(); + } + // When inflights are disabled, the sum of the current utilization metrics + // and the current bytes are compared with the quota. The current bytes + // are not sent to the utilization service. When inflights are enabled, + // the sum of the current utilization metrics only are compared with the + // quota. They include the current inflight bytes sent in the request. + let _inflights = shouldSendInflights ? inflight : undefined; + const inflightForCheck = shouldSendInflights ? 0 : inflight; + return _evaluateQuotas( + bucketQuota, + accountQuota, + bucket, + account, + _inflights, + inflightForCheck, + apiName, + log, + (err, _bucketQuotaExceeded, _accountQuotaExceeded) => { + if (err) { + return done(err); + } - bucketQuotaExceeded = _bucketQuotaExceeded; - accountQuotaExceeded = _accountQuotaExceeded; + bucketQuotaExceeded = _bucketQuotaExceeded; + accountQuotaExceeded = _accountQuotaExceeded; - // Inflights are inverted: in case of cleanup, we just re-issue - // the same API call. - if (_inflights) { - _inflights = -_inflights; - } + // Inflights are inverted: in case of cleanup, we just re-issue + // the same API call. + if (_inflights) { + _inflights = -_inflights; + } - request.finalizerHooks.push((errorFromAPI, _done) => { - const code = (bucketQuotaExceeded || accountQuotaExceeded) ? 429 : 200; - const quotaCleanUpStartTime = process.hrtime.bigint(); - // Quotas are cleaned only in case of error in the API - async.waterfall([ - cb => { - if (errorFromAPI) { - return _evaluateQuotas(bucketQuota, accountQuota, bucket, account, _inflights, - null, apiName, log, cb); - } - return cb(); - }, - ], () => { - monitorQuotaEvaluationDuration(apiMethod, type, code, quotaEvaluationDuration + - Number(process.hrtime.bigint() - quotaCleanUpStartTime)); - return _done(); + request.finalizerHooks.push((errorFromAPI, _done) => { + const code = bucketQuotaExceeded || accountQuotaExceeded ? 429 : 200; + const quotaCleanUpStartTime = process.hrtime.bigint(); + // Quotas are cleaned only in case of error in the API + async.waterfall( + [ + cb => { + if (errorFromAPI) { + return _evaluateQuotas( + bucketQuota, + accountQuota, + bucket, + account, + _inflights, + null, + apiName, + log, + cb, + ); + } + return cb(); + }, + ], + () => { + monitorQuotaEvaluationDuration( + apiMethod, + type, + code, + quotaEvaluationDuration + Number(process.hrtime.bigint() - quotaCleanUpStartTime), + ); + return _done(); + }, + ); }); - }); - return done(); - }); - }, err => { - quotaEvaluationDuration = Number(process.hrtime.bigint() - requestStartTime); - if (err) { - log.warn('Error getting metrics from the quota service, allowing the request', { - error: err.name, - description: err.message, - }); - } - if (!actionWithDataDeletion[apiMethod] && - (bucketQuotaExceeded || accountQuotaExceeded)) { - return callback(errors.QuotaExceeded); - } - return callback(); - }); + return done(); + }, + ); + }, + err => { + quotaEvaluationDuration = Number(process.hrtime.bigint() - requestStartTime); + if (err) { + log.warn('Error getting metrics from the quota service, allowing the request', { + error: err.name, + description: err.message, + }); + } + if (!actionWithDataDeletion[apiMethod] && (bucketQuotaExceeded || accountQuotaExceeded)) { + return callback(errors.QuotaExceeded); + } + return callback(); + }, + ); } module.exports = { diff --git a/lib/api/objectCopy.js b/lib/api/objectCopy.js index 6073678bc3..8555ce6b48 100644 --- a/lib/api/objectCopy.js +++ b/lib/api/objectCopy.js @@ -232,7 +232,8 @@ function objectCopy(authInfo, request, sourceBucket, sourceObject, sourceVersion objectKey: sourceObject, versionId: sourceVersionId, getDeleteMarker: true, - requestType: 'objectGet', + // Same action as the one authorized by Vault for the source + requestType: sourceVersionId ? 'objectGetVersion' : 'objectGet', /** * Authorization will first check the target object, with an objectPut * action. But in this context, the source object metadata is still diff --git a/lib/api/objectPutCopyPart.js b/lib/api/objectPutCopyPart.js index 3f3999b212..23b92baa6b 100644 --- a/lib/api/objectPutCopyPart.js +++ b/lib/api/objectPutCopyPart.js @@ -5,8 +5,7 @@ const validateHeaders = s3middleware.validateConditionalHeaders; const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const constants = require('../../constants'); const { data } = require('../data/wrapper'); -const locationConstraintCheck = - require('./apiUtils/object/locationConstraintCheck'); +const locationConstraintCheck = require('./apiUtils/object/locationConstraintCheck'); const metadata = require('../metadata/wrapper'); const { pushMetric } = require('../utapi/utilities'); const services = require('../services'); @@ -35,8 +34,7 @@ const skipError = new Error('skip'); * @param {function} callback - final callback to call with the result * @return {undefined} */ -function objectPutCopyPart(authInfo, request, sourceBucket, - sourceObject, reqVersionId, log, callback) { +function objectPutCopyPart(authInfo, request, sourceBucket, sourceObject, reqVersionId, log, callback) { log.debug('processing request', { method: 'objectPutCopyPart' }); const destBucketName = request.bucketName; const destObjectKey = request.objectKey; @@ -47,7 +45,8 @@ function objectPutCopyPart(authInfo, request, sourceBucket, objectKey: sourceObject, versionId: reqVersionId, getDeleteMarker: true, - requestType: 'objectGet', + // Same action as the one authorized by Vault for the source + requestType: reqVersionId ? 'objectGetVersion' : 'objectGet', /** * Authorization will first check the target object, with an objectPut * action. But in this context, the source object metadata is still @@ -62,8 +61,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket, const partNumber = Number.parseInt(request.query.partNumber, 10); // AWS caps partNumbers at 10,000 if (partNumber > 10000 || !Number.isInteger(partNumber) || partNumber < 1) { - monitoring.promMetrics('PUT', destBucketName, 400, - 'putObjectCopyPart'); + monitoring.promMetrics('PUT', destBucketName, 400, 'putObjectCopyPart'); return callback(errors.InvalidArgument); } // We pad the partNumbers so that the parts will be sorted @@ -72,7 +70,9 @@ function objectPutCopyPart(authInfo, request, sourceBucket, // Note that keys in the query object retain their case, so // request.query.uploadId must be called with that exact // capitalization - const { query: { uploadId } } = request; + const { + query: { uploadId }, + } = request; const valPutParams = { authInfo, @@ -87,10 +87,13 @@ function objectPutCopyPart(authInfo, request, sourceBucket, // as validating for the destination bucket except additionally need // the uploadId and splitter. // Also, requestType is 'putPart or complete' - const valMPUParams = Object.assign({ - uploadId, - splitter: constants.splitter, - }, valPutParams); + const valMPUParams = Object.assign( + { + uploadId, + splitter: constants.splitter, + }, + valPutParams, + ); valMPUParams.requestType = 'putPart or complete'; const dataStoreContext = { @@ -103,131 +106,161 @@ function objectPutCopyPart(authInfo, request, sourceBucket, enableQuota: true, }; - return async.waterfall([ - function checkDestAuth(next) { - return standardMetadataValidateBucketAndObj(valPutParams, request.actionImplicitDenies, log, - (err, destBucketMD) => { - if (err) { - log.debug('error validating authorization for ' + - 'destination bucket', - { error: err }); - return next(err, destBucketMD); - } - const flag = destBucketMD.hasDeletedFlag() - || destBucketMD.hasTransientFlag(); - if (flag) { - log.trace('deleted flag or transient flag ' + - 'on destination bucket', { flag }); - return next(errors.NoSuchBucket); - } - return next(null, destBucketMD); - }); - }, - function checkSourceAuthorization(destBucketMD, next) { - return standardMetadataValidateBucketAndObj({ - ...valGetParams, - serverAccessLogOptions: { copySource: true }, - }, request.actionImplicitDenies, log, - (err, sourceBucketMD, sourceObjMD) => { - if (err) { - log.debug('error validating get part of request', - { error: err }); - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); - return next(err, destBucketMD); - } - if (!sourceObjMD) { - log.debug('no source object', { sourceObject }); - const err = reqVersionId ? errors.NoSuchVersion : - errors.NoSuchKey; - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); - return next(err, destBucketMD); - } - let sourceLocationConstraintName = - sourceObjMD.dataStoreName; - // for backwards compatibility before storing dataStoreName - // TODO: handle in objectMD class - if (!sourceLocationConstraintName && - sourceObjMD.location[0] && - sourceObjMD.location[0].dataStoreName) { - sourceLocationConstraintName = - sourceObjMD.location[0].dataStoreName; - } - // check if object data is in a cold storage - const coldErr = verifyColdObjectAvailable(sourceObjMD); - if (coldErr) { - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = coldErr); - return next(coldErr, null); - } - if (sourceObjMD.isDeleteMarker) { - log.debug('delete marker on source object', - { sourceObject }); - let err; - if (reqVersionId) { - err = errorInstances.InvalidRequest - .customizeDescription('The source of a copy ' + - 'request may not specifically refer to a delete' + - 'marker by version id.'); - } else { - // if user specifies a key in a versioned source bucket - // without specifying a version, and the object has a - // delete marker, return NoSuchKey - err = errors.NoSuchKey; + return async.waterfall( + [ + function checkDestAuth(next) { + return standardMetadataValidateBucketAndObj( + valPutParams, + request.actionImplicitDenies, + log, + (err, destBucketMD) => { + if (err) { + log.debug('error validating authorization for ' + 'destination bucket', { error: err }); + return next(err, destBucketMD); } - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); - return next(err, destBucketMD); - } - const headerValResult = - validateHeaders(request.headers, - sourceObjMD['last-modified'], - sourceObjMD['content-md5']); - if (headerValResult.error) { - request.sourceServerAccessLog + const flag = destBucketMD.hasDeletedFlag() || destBucketMD.hasTransientFlag(); + if (flag) { + log.trace('deleted flag or transient flag ' + 'on destination bucket', { flag }); + return next(errors.NoSuchBucket); + } + return next(null, destBucketMD); + }, + ); + }, + function checkSourceAuthorization(destBucketMD, next) { + return standardMetadataValidateBucketAndObj( + { + ...valGetParams, + serverAccessLogOptions: { copySource: true }, + }, + request.actionImplicitDenies, + log, + (err, sourceBucketMD, sourceObjMD) => { + if (err) { + log.debug('error validating get part of request', { error: err }); // eslint-disable-next-line no-param-reassign - && (request.sourceServerAccessLog.error = errors.PreconditionFailed); - return next(errors.PreconditionFailed, destBucketMD); - } - const copyLocator = setUpCopyLocator(sourceObjMD, - request.headers['x-amz-copy-source-range'], log); - if (copyLocator.error) { - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = copyLocator.error); - return next(copyLocator.error, destBucketMD); - } - let sourceVerId; - // If specific version requested, include copy source - // version id in response. Include in request by default - // if versioning is enabled or suspended. - if (sourceBucketMD.getVersioningConfiguration() || - reqVersionId) { - if (sourceObjMD.isNull || !sourceObjMD.versionId) { - sourceVerId = 'null'; - } else { - sourceVerId = - versionIdUtils.encode(sourceObjMD.versionId); + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); + return next(err, destBucketMD); } - } - return next(null, copyLocator.dataLocator, destBucketMD, - copyLocator.copyObjectSize, sourceVerId, - sourceLocationConstraintName, sourceObjMD); - }); - }, - function _validateQuotas(dataLocator, destBucketMD, - copyObjectSize, sourceVerId, - sourceLocationConstraintName, sourceObjMD, next) { - return validateQuotas(request, destBucketMD, request.accountQuotas, valPutParams.requestType, - request.apiMethod, sourceObjMD?.['content-length'] || 0, false, log, err => - next(err, dataLocator, destBucketMD, copyObjectSize, sourceVerId, sourceLocationConstraintName)); - }, - // get MPU shadow bucket to get splitter based on MD version - function getMpuShadowBucket(dataLocator, destBucketMD, - copyObjectSize, sourceVerId, - sourceLocationConstraintName, next) { - return metadata.getBucket(mpuBucketName, log, - (err, mpuBucket) => { + if (!sourceObjMD) { + log.debug('no source object', { sourceObject }); + const err = reqVersionId ? errors.NoSuchVersion : errors.NoSuchKey; + // eslint-disable-next-line no-param-reassign + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); + return next(err, destBucketMD); + } + let sourceLocationConstraintName = sourceObjMD.dataStoreName; + // for backwards compatibility before storing dataStoreName + // TODO: handle in objectMD class + if ( + !sourceLocationConstraintName && + sourceObjMD.location[0] && + sourceObjMD.location[0].dataStoreName + ) { + sourceLocationConstraintName = sourceObjMD.location[0].dataStoreName; + } + // check if object data is in a cold storage + const coldErr = verifyColdObjectAvailable(sourceObjMD); + if (coldErr) { + // eslint-disable-next-line no-param-reassign + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = coldErr); + return next(coldErr, null); + } + if (sourceObjMD.isDeleteMarker) { + log.debug('delete marker on source object', { sourceObject }); + let err; + if (reqVersionId) { + err = errorInstances.InvalidRequest.customizeDescription( + 'The source of a copy ' + + 'request may not specifically refer to a delete' + + 'marker by version id.', + ); + } else { + // if user specifies a key in a versioned source bucket + // without specifying a version, and the object has a + // delete marker, return NoSuchKey + err = errors.NoSuchKey; + } + // eslint-disable-next-line no-param-reassign + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err); + return next(err, destBucketMD); + } + const headerValResult = validateHeaders( + request.headers, + sourceObjMD['last-modified'], + sourceObjMD['content-md5'], + ); + if (headerValResult.error) { + request.sourceServerAccessLog && + // eslint-disable-next-line no-param-reassign + (request.sourceServerAccessLog.error = errors.PreconditionFailed); + return next(errors.PreconditionFailed, destBucketMD); + } + const copyLocator = setUpCopyLocator( + sourceObjMD, + request.headers['x-amz-copy-source-range'], + log, + ); + if (copyLocator.error) { + // eslint-disable-next-line no-param-reassign + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = copyLocator.error); + return next(copyLocator.error, destBucketMD); + } + let sourceVerId; + // If specific version requested, include copy source + // version id in response. Include in request by default + // if versioning is enabled or suspended. + if (sourceBucketMD.getVersioningConfiguration() || reqVersionId) { + if (sourceObjMD.isNull || !sourceObjMD.versionId) { + sourceVerId = 'null'; + } else { + sourceVerId = versionIdUtils.encode(sourceObjMD.versionId); + } + } + return next( + null, + copyLocator.dataLocator, + destBucketMD, + copyLocator.copyObjectSize, + sourceVerId, + sourceLocationConstraintName, + sourceObjMD, + ); + }, + ); + }, + function _validateQuotas( + dataLocator, + destBucketMD, + copyObjectSize, + sourceVerId, + sourceLocationConstraintName, + sourceObjMD, + next, + ) { + return validateQuotas( + request, + destBucketMD, + request.accountQuotas, + valPutParams.requestType, + request.apiMethod, + sourceObjMD?.['content-length'] || 0, + false, + log, + err => + next(err, dataLocator, destBucketMD, copyObjectSize, sourceVerId, sourceLocationConstraintName), + ); + }, + // get MPU shadow bucket to get splitter based on MD version + function getMpuShadowBucket( + dataLocator, + destBucketMD, + copyObjectSize, + sourceVerId, + sourceLocationConstraintName, + next, + ) { + return metadata.getBucket(mpuBucketName, log, (err, mpuBucket) => { // TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver if (err && err.NoSuchBucket) { return next(errors.NoSuchUpload); @@ -243,105 +276,140 @@ function objectPutCopyPart(authInfo, request, sourceBucket, if (mpuBucket.getMdBucketModelVersion() < 2) { splitter = constants.oldSplitter; } - return next(null, dataLocator, destBucketMD, - copyObjectSize, sourceVerId, splitter, - sourceLocationConstraintName); + return next( + null, + dataLocator, + destBucketMD, + copyObjectSize, + sourceVerId, + splitter, + sourceLocationConstraintName, + ); }); - }, - // Get MPU overview object to check authorization to put a part - // and to get any object location constraint info - function getMpuOverviewObject(dataLocator, destBucketMD, - copyObjectSize, sourceVerId, splitter, - sourceLocationConstraintName, next) { - const mpuOverviewKey = - `overview${splitter}${destObjectKey}${splitter}${uploadId}`; - return metadata.getObjectMD(mpuBucketName, mpuOverviewKey, - null, log, (err, res) => { - if (err) { - // TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver - if (err.NoSuchKey) { - return next(errors.NoSuchUpload); - } - log.error('error getting overview object from ' + - 'mpu bucket', { - error: err, - method: 'objectPutCopyPart::' + - 'metadata.getObjectMD', - }); - return next(err); - } - const initiatorID = res.initiator.ID; - const requesterID = authInfo.isRequesterAnIAMUser() ? - authInfo.getArn() : authInfo.getCanonicalID(); - if (initiatorID !== requesterID) { - return next(errors.AccessDenied); - } - const destObjLocationConstraint = - res.controllingLocationConstraint; - const sseAlgo = res['x-amz-server-side-encryption']; - const sse = sseAlgo ? { - algorithm: sseAlgo, - masterKeyId: res['x-amz-server-side-encryption-aws-kms-key-id'], - } : null; - return next(null, dataLocator, destBucketMD, - destObjLocationConstraint, copyObjectSize, - sourceVerId, sourceLocationConstraintName, sse, splitter); - }); - }, - function goGetData( - dataLocator, - destBucketMD, - destObjLocationConstraint, - copyObjectSize, - sourceVerId, - sourceLocationConstraintName, - sse, - splitter, - next, - ) { - const originalIdentityAuthzResults = request.actionImplicitDenies; - // eslint-disable-next-line no-param-reassign - delete request.actionImplicitDenies; - data.uploadPartCopy( - request, - log, + }, + // Get MPU overview object to check authorization to put a part + // and to get any object location constraint info + function getMpuOverviewObject( + dataLocator, destBucketMD, + copyObjectSize, + sourceVerId, + splitter, sourceLocationConstraintName, - destObjLocationConstraint, - dataLocator, - dataStoreContext, - locationConstraintCheck, - sse, - (error, eTag, lastModified, serverSideEncryption, locations) => { - // eslint-disable-next-line no-param-reassign - request.actionImplicitDenies = originalIdentityAuthzResults; - if (error) { - if (error.message === 'skip') { - return next(skipError, destBucketMD, eTag, - lastModified, sourceVerId, - serverSideEncryption); + next, + ) { + const mpuOverviewKey = `overview${splitter}${destObjectKey}${splitter}${uploadId}`; + return metadata.getObjectMD(mpuBucketName, mpuOverviewKey, null, log, (err, res) => { + if (err) { + // TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver + if (err.NoSuchKey) { + return next(errors.NoSuchUpload); } - // eslint-disable-next-line no-param-reassign - request.sourceServerAccessLog && (request.sourceServerAccessLog.error = error); - return next(error, destBucketMD); + log.error('error getting overview object from ' + 'mpu bucket', { + error: err, + method: 'objectPutCopyPart::' + 'metadata.getObjectMD', + }); + return next(err); } - return next(null, destBucketMD, locations, eTag, - copyObjectSize, sourceVerId, serverSideEncryption, - lastModified, splitter); + const initiatorID = res.initiator.ID; + const requesterID = authInfo.isRequesterAnIAMUser() ? authInfo.getArn() : authInfo.getCanonicalID(); + if (initiatorID !== requesterID) { + return next(errors.AccessDenied); + } + const destObjLocationConstraint = res.controllingLocationConstraint; + const sseAlgo = res['x-amz-server-side-encryption']; + const sse = sseAlgo + ? { + algorithm: sseAlgo, + masterKeyId: res['x-amz-server-side-encryption-aws-kms-key-id'], + } + : null; + return next( + null, + dataLocator, + destBucketMD, + destObjLocationConstraint, + copyObjectSize, + sourceVerId, + sourceLocationConstraintName, + sse, + splitter, + ); }); - }, - function getExistingPartInfo(destBucketMD, locations, totalHash, - copyObjectSize, sourceVerId, serverSideEncryption, lastModified, - splitter, next) { - const partKey = - `${uploadId}${constants.splitter}${paddedPartNumber}`; - metadata.getObjectMD(mpuBucketName, partKey, {}, log, - (err, result) => { + }, + function goGetData( + dataLocator, + destBucketMD, + destObjLocationConstraint, + copyObjectSize, + sourceVerId, + sourceLocationConstraintName, + sse, + splitter, + next, + ) { + const originalIdentityAuthzResults = request.actionImplicitDenies; + // eslint-disable-next-line no-param-reassign + delete request.actionImplicitDenies; + data.uploadPartCopy( + request, + log, + destBucketMD, + sourceLocationConstraintName, + destObjLocationConstraint, + dataLocator, + dataStoreContext, + locationConstraintCheck, + sse, + (error, eTag, lastModified, serverSideEncryption, locations) => { + // eslint-disable-next-line no-param-reassign + request.actionImplicitDenies = originalIdentityAuthzResults; + if (error) { + if (error.message === 'skip') { + return next( + skipError, + destBucketMD, + eTag, + lastModified, + sourceVerId, + serverSideEncryption, + ); + } + // eslint-disable-next-line no-param-reassign + request.sourceServerAccessLog && (request.sourceServerAccessLog.error = error); + return next(error, destBucketMD); + } + return next( + null, + destBucketMD, + locations, + eTag, + copyObjectSize, + sourceVerId, + serverSideEncryption, + lastModified, + splitter, + ); + }, + ); + }, + function getExistingPartInfo( + destBucketMD, + locations, + totalHash, + copyObjectSize, + sourceVerId, + serverSideEncryption, + lastModified, + splitter, + next, + ) { + const partKey = `${uploadId}${constants.splitter}${paddedPartNumber}`; + metadata.getObjectMD(mpuBucketName, partKey, {}, log, (err, result) => { // If there is nothing being overwritten just move on // TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver if (err && !err.NoSuchKey) { - log.debug('error getting current part (if any)', - { error: err }); + log.debug('error getting current part (if any)', { error: err }); return next(err); } let oldLocations; @@ -352,158 +420,242 @@ function objectPutCopyPart(authInfo, request, sourceBucket, // Pull locations to clean up any potential orphans // in data if object put is an overwrite of // already existing object with same key and part number - oldLocations = Array.isArray(oldLocations) ? - oldLocations : [oldLocations]; + oldLocations = Array.isArray(oldLocations) ? oldLocations : [oldLocations]; } - return next(null, destBucketMD, locations, totalHash, - prevObjectSize, copyObjectSize, sourceVerId, - serverSideEncryption, lastModified, oldLocations, splitter); + return next( + null, + destBucketMD, + locations, + totalHash, + prevObjectSize, + copyObjectSize, + sourceVerId, + serverSideEncryption, + lastModified, + oldLocations, + splitter, + ); }); - }, - function storeNewPartMetadata(destBucketMD, locations, totalHash, - prevObjectSize, copyObjectSize, sourceVerId, serverSideEncryption, - lastModified, oldLocations, splitter, next) { - const metaStoreParams = { - partNumber: paddedPartNumber, - contentMD5: totalHash, - size: copyObjectSize, - uploadId, - splitter: constants.splitter, + }, + function storeNewPartMetadata( + destBucketMD, + locations, + totalHash, + prevObjectSize, + copyObjectSize, + sourceVerId, + serverSideEncryption, lastModified, - overheadField: constants.overheadField, - ownerId: destBucketMD.getOwner(), - }; - return services.metadataStorePart(mpuBucketName, - locations, metaStoreParams, log, err => { + oldLocations, + splitter, + next, + ) { + const metaStoreParams = { + partNumber: paddedPartNumber, + contentMD5: totalHash, + size: copyObjectSize, + uploadId, + splitter: constants.splitter, + lastModified, + overheadField: constants.overheadField, + ownerId: destBucketMD.getOwner(), + }; + return services.metadataStorePart(mpuBucketName, locations, metaStoreParams, log, err => { if (err) { - log.debug('error storing new metadata', - { error: err, method: 'storeNewPartMetadata' }); + log.debug('error storing new metadata', { error: err, method: 'storeNewPartMetadata' }); return next(err); } - return next(null, locations, oldLocations, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize, splitter); + return next( + null, + locations, + oldLocations, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + splitter, + ); }); - }, - function checkCanDeleteOldLocations(partLocations, oldLocations, destBucketMD, - totalHash, lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize, splitter, next) { - if (!oldLocations) { - return next(null, oldLocations, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize); - } - return services.isCompleteMPUInProgress({ - bucketName: destBucketName, - objectKey: destObjectKey, - uploadId, + }, + function checkCanDeleteOldLocations( + partLocations, + oldLocations, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, splitter, - }, log, (err, completeInProgress) => { - if (err) { - return next(err, destBucketMD); + next, + ) { + if (!oldLocations) { + return next( + null, + oldLocations, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + ); } - let oldLocationsToDelete = oldLocations; - // Prevent deletion of old data if a completeMPU - // is already in progress because then there is no - // guarantee that the old location will not be the - // committed one. - if (completeInProgress) { - log.warn('not deleting old locations because CompleteMPU is in progress', { - method: 'objectPutCopyPart::checkCanDeleteOldLocations', + return services.isCompleteMPUInProgress( + { bucketName: destBucketName, objectKey: destObjectKey, uploadId, - partLocations, - oldLocations, - }); - oldLocationsToDelete = null; - } - return next(null, oldLocationsToDelete, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize); - }); - }, - function cleanupExistingData(oldLocationsToDelete, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize, next) { - // Clean up the old data now that new metadata (with new - // data locations) has been stored - if (oldLocationsToDelete) { - return data.batchDelete(oldLocationsToDelete, request.method, null, - log, err => { + splitter, + }, + log, + (err, completeInProgress) => { + if (err) { + return next(err, destBucketMD); + } + let oldLocationsToDelete = oldLocations; + // Prevent deletion of old data if a completeMPU + // is already in progress because then there is no + // guarantee that the old location will not be the + // committed one. + if (completeInProgress) { + log.warn('not deleting old locations because CompleteMPU is in progress', { + method: 'objectPutCopyPart::checkCanDeleteOldLocations', + bucketName: destBucketName, + objectKey: destObjectKey, + uploadId, + partLocations, + oldLocations, + }); + oldLocationsToDelete = null; + } + return next( + null, + oldLocationsToDelete, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + ); + }, + ); + }, + function cleanupExistingData( + oldLocationsToDelete, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + next, + ) { + // Clean up the old data now that new metadata (with new + // data locations) has been stored + if (oldLocationsToDelete) { + return data.batchDelete(oldLocationsToDelete, request.method, null, log, err => { if (err) { // if error, log the error and move on as it is not // relevant to the client as the client's // object already succeeded putting data, metadata - log.error('error deleting existing data', - { error: err }); + log.error('error deleting existing data', { error: err }); } - return next(null, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize); + return next( + null, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + ); }); - } - return next(null, destBucketMD, totalHash, - lastModified, sourceVerId, serverSideEncryption, - prevObjectSize, copyObjectSize); - }, - ], (err, destBucketMD, totalHash, lastModified, sourceVerId, - serverSideEncryption, prevObjectSize, copyObjectSize) => { - const corsHeaders = collectCorsHeaders(request.headers.origin, - request.method, destBucketMD); + } + return next( + null, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + ); + }, + ], + ( + err, + destBucketMD, + totalHash, + lastModified, + sourceVerId, + serverSideEncryption, + prevObjectSize, + copyObjectSize, + ) => { + const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destBucketMD); - // Store full object size for server access logs - if (request.serverAccessLog) { - // eslint-disable-next-line no-param-reassign - request.serverAccessLog.objectSize = copyObjectSize; - } + // Store full object size for server access logs + if (request.serverAccessLog) { + // eslint-disable-next-line no-param-reassign + request.serverAccessLog.objectSize = copyObjectSize; + } - // Initialize the queue for internal log request logging - initializeInternalLogRequestQueue(request); - // Queue the source-side access log (REST.COPY.PART_GET) - queueInternalLogRequest(request, { - operation: 'REST.COPY.PART_GET', - sourceBucket, - sourceObject, - objectSize: copyObjectSize || null, - }); + // Initialize the queue for internal log request logging + initializeInternalLogRequestQueue(request); + // Queue the source-side access log (REST.COPY.PART_GET) + queueInternalLogRequest(request, { + operation: 'REST.COPY.PART_GET', + sourceBucket, + sourceObject, + objectSize: copyObjectSize || null, + }); - if (err && err !== skipError) { - log.trace('error from copy part waterfall', - { error: err }); - monitoring.promMetrics('PUT', destBucketName, err.code, - 'putObjectCopyPart'); - return callback(err, null, corsHeaders); - } - const xml = [ - '', - '', - '', new Date(lastModified) - .toISOString(), '', - '"', totalHash, '"', - '', - ].join(''); + if (err && err !== skipError) { + log.trace('error from copy part waterfall', { error: err }); + monitoring.promMetrics('PUT', destBucketName, err.code, 'putObjectCopyPart'); + return callback(err, null, corsHeaders); + } + const xml = [ + '', + '', + '', + new Date(lastModified).toISOString(), + '', + '"', + totalHash, + '"', + '', + ].join(''); - const additionalHeaders = corsHeaders || {}; - if (serverSideEncryption) { - setSSEHeaders(additionalHeaders, - serverSideEncryption.algorithm, - serverSideEncryption.masterKeyId); - } - additionalHeaders['x-amz-copy-source-version-id'] = sourceVerId; - pushMetric('uploadPartCopy', log, { - authInfo, - canonicalID: destBucketMD.getOwner(), - bucket: destBucketName, - keys: [destObjectKey], - newByteLength: copyObjectSize, - oldByteLength: prevObjectSize, - location: destBucketMD.getLocationConstraint(), - }); - monitoring.promMetrics( - 'PUT', destBucketName, '200', 'putObjectCopyPart'); - return callback(null, xml, additionalHeaders); - }); + const additionalHeaders = corsHeaders || {}; + if (serverSideEncryption) { + setSSEHeaders(additionalHeaders, serverSideEncryption.algorithm, serverSideEncryption.masterKeyId); + } + additionalHeaders['x-amz-copy-source-version-id'] = sourceVerId; + pushMetric('uploadPartCopy', log, { + authInfo, + canonicalID: destBucketMD.getOwner(), + bucket: destBucketName, + keys: [destObjectKey], + newByteLength: copyObjectSize, + oldByteLength: prevObjectSize, + location: destBucketMD.getLocationConstraint(), + }); + monitoring.promMetrics('PUT', destBucketName, '200', 'putObjectCopyPart'); + return callback(null, xml, additionalHeaders); + }, + ); } module.exports = objectPutCopyPart; diff --git a/tests/functional/aws-node-sdk/lib/utility/iam-util.js b/tests/functional/aws-node-sdk/lib/utility/iam-util.js new file mode 100644 index 0000000000..d2c8094812 --- /dev/null +++ b/tests/functional/aws-node-sdk/lib/utility/iam-util.js @@ -0,0 +1,61 @@ +const { S3Client } = require('@aws-sdk/client-s3'); +const { + IAMClient, + CreateUserCommand, + CreateAccessKeyCommand, + DeleteAccessKeyCommand, + DeleteUserCommand, + CreatePolicyCommand, + AttachUserPolicyCommand, + DetachUserPolicyCommand, + DeletePolicyCommand, +} = require('@aws-sdk/client-iam'); +const { v4: uuid } = require('uuid'); + +const getConfig = require('../../test/support/config'); +const { config } = require('../../../../../lib/Config'); + +const vaultHost = config.vaultd?.host || 'localhost'; + +function makeIamClient(profile) { + const iamConfig = getConfig(profile, { region: 'us-east-1' }); + iamConfig.endpoint = `http://${vaultHost}:8600`; // define outside of getConfig for Integration + return new IAMClient(iamConfig); +} + +// Creates an IAM user with one managed policy, returns its S3 client and a +// function removing the user +async function createUserWithPolicy(iamClient, statements) { + const name = `test-user-${uuid()}`; + const policyResponse = await iamClient.send( + new CreatePolicyCommand({ + PolicyName: name, + PolicyDocument: JSON.stringify({ Version: '2012-10-17', Statement: statements }), + }), + ); + const policyArn = policyResponse.Policy.Arn; + await iamClient.send(new CreateUserCommand({ UserName: name })); + await iamClient.send(new AttachUserPolicyCommand({ UserName: name, PolicyArn: policyArn })); + const accessKeyResponse = await iamClient.send(new CreateAccessKeyCommand({ UserName: name })); + const { AccessKeyId, SecretAccessKey } = accessKeyResponse.AccessKey; + const s3Client = new S3Client( + getConfig('default', { + credentials: { + accessKeyId: AccessKeyId, + secretAccessKey: SecretAccessKey, + }, + }), + ); + async function deleteUser() { + await iamClient.send(new DeleteAccessKeyCommand({ UserName: name, AccessKeyId })); + await iamClient.send(new DetachUserPolicyCommand({ UserName: name, PolicyArn: policyArn })); + await iamClient.send(new DeletePolicyCommand({ PolicyArn: policyArn })); + await iamClient.send(new DeleteUserCommand({ UserName: name })); + } + return { s3Client, deleteUser }; +} + +module.exports = { + makeIamClient, + createUserWithPolicy, +}; diff --git a/tests/functional/aws-node-sdk/test/object/copySourceVersionAuthorization.js b/tests/functional/aws-node-sdk/test/object/copySourceVersionAuthorization.js new file mode 100644 index 0000000000..402fadcc21 --- /dev/null +++ b/tests/functional/aws-node-sdk/test/object/copySourceVersionAuthorization.js @@ -0,0 +1,226 @@ +const assert = require('assert'); +const { + PutObjectCommand, + GetObjectCommand, + CopyObjectCommand, + CreateMultipartUploadCommand, + UploadPartCopyCommand, + AbortMultipartUploadCommand, + PutBucketPolicyCommand, + DeleteBucketPolicyCommand, + PutBucketVersioningCommand, +} = require('@aws-sdk/client-s3'); +const { v4: uuid } = require('uuid'); + +const BucketUtility = require('../../lib/utility/bucket-util'); +const { makeIamClient, createUserWithPolicy } = require('../../lib/utility/iam-util'); +const { config } = require('../../../../../lib/Config'); + +// this test needs a real vault to create IAM users +const isVaultScality = config.backends.auth !== 'mem'; + +const describeWithVault = isVaultScality ? describe : describe.skip; + +describeWithVault('Copy of a specific source version', () => { + const secretKey = 'secret'; + + async function assertAccessDenied(promise) { + await assert.rejects(promise, { name: 'AccessDenied' }); + } + + describe('in the same account, without IAM permission to read the source', () => { + const ownBucketName = `copy-source-own-${uuid()}`; + const bucketUtil = new BucketUtility('default'); + const iamClient = makeIamClient('default'); + const mpuKey = 'mine/mpu-copy'; + let user; + let uploadId; + + before(async () => { + await bucketUtil.createOne(ownBucketName); + await bucketUtil.s3.send( + new PutObjectCommand({ + Bucket: ownBucketName, + Key: secretKey, + Body: 'secret content', + }), + ); + user = await createUserWithPolicy(iamClient, [ + { + Effect: 'Allow', + Action: ['s3:GetObject', 's3:PutObject'], + Resource: `arn:aws:s3:::${ownBucketName}/mine/*`, + }, + ]); + const mpuResponse = await user.s3Client.send( + new CreateMultipartUploadCommand({ + Bucket: ownBucketName, + Key: mpuKey, + }), + ); + uploadId = mpuResponse.UploadId; + }); + + after(async () => { + if (uploadId) { + await bucketUtil.s3.send( + new AbortMultipartUploadCommand({ + Bucket: ownBucketName, + Key: mpuKey, + UploadId: uploadId, + }), + ); + } + await bucketUtil.empty(ownBucketName); + await bucketUtil.deleteOne(ownBucketName); + if (user) { + await user.deleteUser(); + } + }); + + it('should deny CopyObject from the latest version', async () => { + await assertAccessDenied( + user.s3Client.send( + new CopyObjectCommand({ + Bucket: ownBucketName, + Key: 'mine/copy', + CopySource: `${ownBucketName}/${secretKey}`, + }), + ), + ); + }); + + it('should deny CopyObject from the null version', async () => { + await assertAccessDenied( + user.s3Client.send( + new CopyObjectCommand({ + Bucket: ownBucketName, + Key: 'mine/copy', + CopySource: `${ownBucketName}/${secretKey}?versionId=null`, + }), + ), + ); + }); + + it('should deny UploadPartCopy from the null version', async () => { + await assertAccessDenied( + user.s3Client.send( + new UploadPartCopyCommand({ + Bucket: ownBucketName, + Key: mpuKey, + UploadId: uploadId, + PartNumber: 1, + CopySource: `${ownBucketName}/${secretKey}?versionId=null`, + }), + ), + ); + }); + }); + + describe('with a bucket policy denying s3:GetObjectVersion', () => { + const policyBucketName = `copy-source-policy-${uuid()}`; + const bucketUtil = new BucketUtility('default'); + const iamClient = makeIamClient('default'); + const latestContent = 'latest content'; + let user; + let oldVersionId; + + before(async () => { + await bucketUtil.createOne(policyBucketName); + await bucketUtil.s3.send( + new PutBucketVersioningCommand({ + Bucket: policyBucketName, + VersioningConfiguration: { Status: 'Enabled' }, + }), + ); + const oldPut = await bucketUtil.s3.send( + new PutObjectCommand({ + Bucket: policyBucketName, + Key: secretKey, + Body: 'old content', + }), + ); + oldVersionId = oldPut.VersionId; + await bucketUtil.s3.send( + new PutObjectCommand({ + Bucket: policyBucketName, + Key: secretKey, + Body: latestContent, + }), + ); + await bucketUtil.s3.send( + new PutBucketPolicyCommand({ + Bucket: policyBucketName, + Policy: JSON.stringify({ + Version: '2012-10-17', + Statement: [ + { + Effect: 'Deny', + Principal: '*', + Action: 's3:GetObjectVersion', + Resource: `arn:aws:s3:::${policyBucketName}/*`, + }, + ], + }), + }), + ); + user = await createUserWithPolicy(iamClient, [ + { + Effect: 'Allow', + Action: ['s3:GetObject', 's3:GetObjectVersion', 's3:PutObject'], + Resource: `arn:aws:s3:::${policyBucketName}/*`, + }, + ]); + }); + + after(async () => { + await bucketUtil.s3.send(new DeleteBucketPolicyCommand({ Bucket: policyBucketName })); + await bucketUtil.empty(policyBucketName); + await bucketUtil.deleteOne(policyBucketName); + if (user) { + await user.deleteUser(); + } + }); + + it('should deny GetObject of an older version', async () => { + await assertAccessDenied( + user.s3Client.send( + new GetObjectCommand({ + Bucket: policyBucketName, + Key: secretKey, + VersionId: oldVersionId, + }), + ), + ); + }); + + it('should deny CopyObject from an older version', async () => { + await assertAccessDenied( + user.s3Client.send( + new CopyObjectCommand({ + Bucket: policyBucketName, + Key: 'leak', + CopySource: `${policyBucketName}/${secretKey}?versionId=${oldVersionId}`, + }), + ), + ); + }); + + it('should allow CopyObject from the latest version', async () => { + await user.s3Client.send( + new CopyObjectCommand({ + Bucket: policyBucketName, + Key: 'latest-copy', + CopySource: `${policyBucketName}/${secretKey}`, + }), + ); + const getResponse = await user.s3Client.send( + new GetObjectCommand({ + Bucket: policyBucketName, + Key: 'latest-copy', + }), + ); + assert.strictEqual(await getResponse.Body.transformToString(), latestContent); + }); + }); +}); diff --git a/tests/unit/api/apiUtils/quotas/quotaUtils.js b/tests/unit/api/apiUtils/quotas/quotaUtils.js index 67b4750c43..ae9095f2bb 100644 --- a/tests/unit/api/apiUtils/quotas/quotaUtils.js +++ b/tests/unit/api/apiUtils/quotas/quotaUtils.js @@ -75,15 +75,13 @@ describe('validateQuotas (buckets)', () => { validateQuotas(request, mockBucket, {}, ['objectPut', 'getObject'], 'objectPut', 1, false, mockLog, err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectPut', inflight: 1, - } - ), true); + }), + true, + ); done(); }); }); @@ -102,19 +100,30 @@ describe('validateQuotas (buckets)', () => { assert.strictEqual(err.is.QuotaExceeded, true); assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); assert.strictEqual(request.finalizerHooks.length, 1); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectPut', inflight: 1, - } - ), true); + }), + true, + ); done(); }); }); + ['objectCopy', 'objectPutCopyPart'].forEach(apiMethod => { + it(`should return errors.QuotaExceeded if quota is exceeded by a ${apiMethod} of a source version`, done => { + QuotaService._getLatestMetricsCallback.yields(null, { bytesTotal: BigInt(150) }); + + validateQuotas(request, mockBucket, {}, ['objectGetVersion'], apiMethod, 1, false, mockLog, err => { + assert(err, 'expected the quota to be evaluated'); + assert.strictEqual(err.is.QuotaExceeded, true); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); + done(); + }); + }); + }); + it('should not return QuotaExceeded if quotas are exceeded but operation is creating a delete marker', done => { const result1 = { bytesTotal: BigInt(150), @@ -128,15 +137,13 @@ describe('validateQuotas (buckets)', () => { validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDelete', 0, false, mockLog, err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectDelete', inflight: 0, - } - ), true); + }), + true, + ); done(); }); }); @@ -154,15 +161,13 @@ describe('validateQuotas (buckets)', () => { validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDelete', -50, false, mockLog, err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectDelete', inflight: -50, - } - ), true); + }), + true, + ); done(); }); }); @@ -180,15 +185,13 @@ describe('validateQuotas (buckets)', () => { validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDeleteVersion', -50, false, mockLog, err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectDelete', inflight: -50, - } - ), true); + }), + true, + ); done(); }); }); @@ -206,15 +209,13 @@ describe('validateQuotas (buckets)', () => { validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDelete', -5000, false, mockLog, err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectDelete', inflight: -5000, - } - ), true); + }), + true, + ); done(); }); }); @@ -229,21 +230,28 @@ describe('validateQuotas (buckets)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, {}, ['objectRestore', 'objectPut'], 'objectRestore', - true, false, mockLog, err => { + validateQuotas( + request, + mockBucket, + {}, + ['objectRestore', 'objectPut'], + 'objectRestore', + true, + false, + mockLog, + err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledTwice, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectRestore', inflight: true, - } - ), true); + }), + true, + ); done(); - }); + }, + ); }); it('should not include the inflights in the request if they are disabled', done => { @@ -257,21 +265,28 @@ describe('validateQuotas (buckets)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, {}, ['objectRestore', 'objectPut'], 'objectRestore', - true, false, mockLog, err => { + validateQuotas( + request, + mockBucket, + {}, + ['objectRestore', 'objectPut'], + 'objectRestore', + true, + false, + mockLog, + err => { assert.ifError(err); assert.strictEqual(QuotaService._getLatestMetricsCallback.calledTwice, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { action: 'objectRestore', inflight: undefined, - } - ), true); - done(); - }); + }), + true, + ); + done(); + }, + ); }); it('should evaluate the quotas and not update the inflights when isStorageReserved is true', done => { @@ -284,21 +299,18 @@ describe('validateQuotas (buckets)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, {}, ['objectPut'], 'objectPut', - true, true, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { - action: 'objectPut', - inflight: 0, - } - ), true); - done(); - }); + validateQuotas(request, mockBucket, {}, ['objectPut'], 'objectPut', true, true, mockLog, err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { + action: 'objectPut', + inflight: 0, + }), + true, + ); + done(); + }); }); it('should handle numbers above MAX_SAFE_INTEGER when quota is not exceeded', done => { @@ -308,23 +320,31 @@ describe('validateQuotas (buckets)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, { - ...mockBucket, - getQuota: () => 9007199254740993n, - }, {}, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { - action: 'objectPut', - inflight: 1, - }, - ), true); - done(); - }); + validateQuotas( + request, + { + ...mockBucket, + getQuota: () => 9007199254740993n, + }, + {}, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { + action: 'objectPut', + inflight: 1, + }), + true, + ); + done(); + }, + ); }); it('should handle numbers above MAX_SAFE_INTEGER when quota is exceeded', done => { @@ -334,15 +354,25 @@ describe('validateQuotas (buckets)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, { - ...mockBucket, - getQuota: () => 9007199254740991n, - }, {}, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.strictEqual(err.is.QuotaExceeded, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(request.finalizerHooks.length, 1); - done(); - }); + validateQuotas( + request, + { + ...mockBucket, + getQuota: () => 9007199254740991n, + }, + {}, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.strictEqual(err.is.QuotaExceeded, true); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual(request.finalizerHooks.length, 1); + done(); + }, + ); }); it('should handle numbers above MAX_SAFE_INTEGER with disabled inflights when quota is not exceeded', done => { @@ -353,23 +383,31 @@ describe('validateQuotas (buckets)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, { - ...mockBucket, - getQuota: () => 9007199254740993n, - }, {}, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { - action: 'objectPut', - inflight: undefined, - }, - ), true); - done(); - }); + validateQuotas( + request, + { + ...mockBucket, + getQuota: () => 9007199254740993n, + }, + {}, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { + action: 'objectPut', + inflight: undefined, + }), + true, + ); + done(); + }, + ); }); }); @@ -398,37 +436,67 @@ describe('validateQuotas (with accounts)', () => { }); it('should return null if quota is <= 0', done => { - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 0n, - }, [], '', false, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 0n, + }, + [], + '', + false, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); + done(); + }, + ); }); it('should not return null if bucket quota is <= 0 but account quota is > 0', done => { - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 1000n, - }, [], '', false, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 1000n, + }, + [], + '', + false, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); + done(); + }, + ); }); it('should return null if scuba is disabled', done => { QuotaService.enabled = false; - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, [], '', false, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + [], + '', + false, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.called, false); + done(); + }, + ); }); it('should return null if metrics retrieval fails', done => { @@ -436,23 +504,31 @@ describe('validateQuotas (with accounts)', () => { const error = new Error('Failed to get metrics'); QuotaService._getLatestMetricsCallback.yields(error); - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, ['objectPut', 'getObject'], 'objectPut', 1, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'bucket', - 'bucketName_1640995200000', - null, - { - action: 'objectPut', - inflight: 1, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + ['objectPut', 'getObject'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('bucket', 'bucketName_1640995200000', null, { + action: 'objectPut', + inflight: 1, + }), + true, + ); + done(); + }, + ); }); it('should return errors.QuotaExceeded if quota is exceeded', done => { @@ -465,24 +541,32 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 100n, - }, ['objectPut', 'getObject'], 'objectPut', 1, false, mockLog, err => { - assert.strictEqual(err.is.QuotaExceeded, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); - assert.strictEqual(request.finalizerHooks.length, 1); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectPut', - inflight: 1, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 100n, + }, + ['objectPut', 'getObject'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.strictEqual(err.is.QuotaExceeded, true); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); + assert.strictEqual(request.finalizerHooks.length, 1); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectPut', + inflight: 1, + }), + true, + ); + done(); + }, + ); }); it('should not return QuotaExceeded if the quotas are exceeded but operation is a delete', done => { @@ -495,23 +579,31 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 1000n, - }, ['objectDelete'], 'objectDelete', -50, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectDelete', - inflight: -50, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 1000n, + }, + ['objectDelete'], + 'objectDelete', + -50, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectDelete', + inflight: -50, + }), + true, + ); + done(); + }, + ); }); it('should decrease the inflights by deleting data, and go below 0 to unblock operations', done => { @@ -524,23 +616,31 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 1000n, - }, ['objectDelete'], 'objectDelete', -5000, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectDelete', - inflight: -5000, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 1000n, + }, + ['objectDelete'], + 'objectDelete', + -5000, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 1); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectDelete', + inflight: -5000, + }), + true, + ); + done(); + }, + ); }); it('should return null if quota is not exceeded', done => { @@ -553,23 +653,31 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, ['objectRestore', 'objectPut'], 'objectRestore', true, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 4); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectRestore', - inflight: true, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + ['objectRestore', 'objectPut'], + 'objectRestore', + true, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 4); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectRestore', + inflight: true, + }), + true, + ); + done(); + }, + ); }); it('should return quota exceeded if account and bucket quotas are different', done => { @@ -582,15 +690,25 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, ['objectPut', 'getObject'], 'objectPut', 1, false, mockLog, err => { - assert.strictEqual(err.is.QuotaExceeded, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 2); - assert.strictEqual(request.finalizerHooks.length, 1); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + ['objectPut', 'getObject'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.strictEqual(err.is.QuotaExceeded, true); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 2); + assert.strictEqual(request.finalizerHooks.length, 1); + done(); + }, + ); }); it('should update the request with one function per action to clear quota updates', done => { @@ -603,23 +721,31 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, ['objectRestore', 'objectPut'], 'objectRestore', true, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 4); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectRestore', - inflight: true, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + ['objectRestore', 'objectPut'], + 'objectRestore', + true, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.callCount, 4); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectRestore', + inflight: true, + }), + true, + ); + done(); + }, + ); }); it('should evaluate the quotas and not update the inflights when isStorageReserved is true', done => { @@ -632,23 +758,31 @@ describe('validateQuotas (with accounts)', () => { QuotaService._getLatestMetricsCallback.yields(null, result1); QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2); - validateQuotas(request, mockBucket, { - account: 'test_1', - quota: 1000n, - }, ['objectPut'], 'objectPut', true, true, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledTwice, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectPut', - inflight: 0, - } - ), true); - done(); - }); + validateQuotas( + request, + mockBucket, + { + account: 'test_1', + quota: 1000n, + }, + ['objectPut'], + 'objectPut', + true, + true, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledTwice, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectPut', + inflight: 0, + }), + true, + ); + done(); + }, + ); }); it('should handle account numbers above MAX_SAFE_INTEGER when quota is not exceeded', done => { @@ -658,23 +792,31 @@ describe('validateQuotas (with accounts)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 9007199254740993n, - }, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectPut', - inflight: 1, - }, - ), true); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 9007199254740993n, + }, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectPut', + inflight: 1, + }), + true, + ); + done(); + }, + ); }); it('should handle account numbers above MAX_SAFE_INTEGER when quota is exceeded', done => { @@ -684,15 +826,25 @@ describe('validateQuotas (with accounts)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 9007199254740991n, - }, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.strictEqual(err.is.QuotaExceeded, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(request.finalizerHooks.length, 1); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 9007199254740991n, + }, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.strictEqual(err.is.QuotaExceeded, true); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual(request.finalizerHooks.length, 1); + done(); + }, + ); }); it('should handle account numbers above MAX_SAFE_INTEGER with disabled inflights', done => { @@ -703,23 +855,31 @@ describe('validateQuotas (with accounts)', () => { }; QuotaService._getLatestMetricsCallback.yields(null, result1); - validateQuotas(request, mockBucketNoQuota, { - account: 'test_1', - quota: 9007199254740993n, - }, ['objectPut'], 'objectPut', 1, false, mockLog, err => { - assert.ifError(err); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); - assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith( - 'account', - 'test_1', - null, - { - action: 'objectPut', - inflight: undefined, - }, - ), true); - done(); - }); + validateQuotas( + request, + mockBucketNoQuota, + { + account: 'test_1', + quota: 9007199254740993n, + }, + ['objectPut'], + 'objectPut', + 1, + false, + mockLog, + err => { + assert.ifError(err); + assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true); + assert.strictEqual( + QuotaService._getLatestMetricsCallback.calledWith('account', 'test_1', null, { + action: 'objectPut', + inflight: undefined, + }), + true, + ); + done(); + }, + ); }); }); @@ -746,7 +906,7 @@ describe('processBytesToWrite', () => { ...hotObject, dataStoreName: 'glacier', archive: { - archiveInfo: '{archiveID,archiveVersion}' + archiveInfo: '{archiveID,archiveVersion}', }, }; const restoringObject = { diff --git a/tests/unit/api/objectCopy.js b/tests/unit/api/objectCopy.js index 526bef3fb8..d8c85eac6a 100644 --- a/tests/unit/api/objectCopy.js +++ b/tests/unit/api/objectCopy.js @@ -790,3 +790,124 @@ describe('objectCopy source size limit', () => { }); }); }); + +describe('objectCopy of a specific source version', () => { + const ownBucketName = 'ownversionbucket'; + const policyBucketName = 'policyversionbucket'; + const userAuthInfo = makeAuthInfo(canonicalID, 'user'); + const enableRequest = versioningTestUtils.createBucketPutVersioningReq(policyBucketName, 'Enabled'); + // The version IDs of the policy bucket source, as the client sees them + const versionIds = []; + + function _createPutPolicyRequest(bucketName, statement) { + return new DummyRequest({ + bucketName, + namespace, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: '/', + socket: {}, + post: JSON.stringify({ + Version: '2012-10-17', + Statement: [statement], + }), + }); + } + + function _createUserCopyRequest(bucketName, actionImplicitDenies) { + const request = new DummyRequest({ + bucketName, + namespace, + objectKey: 'copy', + headers: {}, + url: `/${bucketName}/copy`, + query: {}, + socket: {}, + }); + request.actionImplicitDenies = actionImplicitDenies; + return request; + } + + function _putObject(bucketName, body, callback) { + objectPut( + authInfo, + versioningTestUtils.createPutObjectRequest(bucketName, 'secret', body), + undefined, + log, + (err, resHeaders) => callback(err, resHeaders), + ); + } + + before(done => { + cleanup(); + async.series( + [ + callback => bucketPut(authInfo, _createBucketPutRequest(ownBucketName), log, callback), + callback => _putObject(ownBucketName, objData[0], callback), + callback => bucketPut(authInfo, _createBucketPutRequest(policyBucketName), log, callback), + callback => bucketPutVersioning(authInfo, enableRequest, log, callback), + callback => + _putObject(policyBucketName, objData[0], (err, resHeaders) => { + versionIds.push(resHeaders?.['x-amz-version-id']); + callback(err); + }), + callback => + _putObject(policyBucketName, objData[1], (err, resHeaders) => { + versionIds.push(resHeaders?.['x-amz-version-id']); + callback(err); + }), + callback => + bucketPutPolicy( + authInfo, + _createPutPolicyRequest(policyBucketName, { + Effect: 'Deny', + Principal: '*', + Action: 's3:GetObjectVersion', + Resource: `arn:aws:s3:::${policyBucketName}/*`, + }), + log, + callback, + ), + ], + done, + ); + }); + + after(() => { + cleanup(); + }); + + it('should deny a null version copy when IAM does not allow reading the source version', done => { + const request = _createUserCopyRequest(ownBucketName, { objectGetVersion: true, objectPut: false }); + objectCopy(userAuthInfo, request, ownBucketName, 'secret', 'null', log, err => { + assert(err, 'expected the copy to be denied'); + assert.strictEqual(err.is.AccessDenied, true); + done(); + }); + }); + + it('should allow a null version copy when IAM allows reading the source version', done => { + const request = _createUserCopyRequest(ownBucketName, { objectGetVersion: false, objectPut: false }); + objectCopy(userAuthInfo, request, ownBucketName, 'secret', 'null', log, err => { + assert.ifError(err); + done(); + }); + }); + + it('should deny copying a version when the bucket policy denies s3:GetObjectVersion', done => { + const request = _createUserCopyRequest(policyBucketName, { objectGetVersion: false, objectPut: false }); + const sourceVersionId = versioning.VersionID.decode(versionIds[0]); + objectCopy(userAuthInfo, request, policyBucketName, 'secret', sourceVersionId, log, err => { + assert(err, 'expected the copy to be denied'); + assert.strictEqual(err.is.AccessDenied, true); + done(); + }); + }); + + it('should allow copying the latest version when the bucket policy denies s3:GetObjectVersion', done => { + const request = _createUserCopyRequest(policyBucketName, { objectGet: false, objectPut: false }); + objectCopy(userAuthInfo, request, policyBucketName, 'secret', undefined, log, err => { + assert.ifError(err); + done(); + }); + }); +}); diff --git a/tests/unit/api/objectCopyPart.js b/tests/unit/api/objectCopyPart.js index d81920ccf1..fba1b2d3a5 100644 --- a/tests/unit/api/objectCopyPart.js +++ b/tests/unit/api/objectCopyPart.js @@ -252,3 +252,86 @@ describe('objectCopyPart through api.callApiMethod', () => { }); }); }); + +describe('objectCopyPart of a specific source version', () => { + const bucketName = 'ownversionbucket'; + const sourceKey = 'secret'; + const destKey = 'mpu-copy'; + const userAuthInfo = makeAuthInfo(canonicalID, 'user'); + const initiateUserRequest = new DummyRequest({ + bucketName, + namespace, + objectKey: destKey, + headers: { host: `${bucketName}.s3.amazonaws.com` }, + url: `/${destKey}?uploads`, + query: {}, + socket: {}, + }); + let uploadId; + + function _createUserCopyPartRequest(actionImplicitDenies) { + const request = new DummyRequest({ + bucketName, + namespace, + objectKey: destKey, + headers: {}, + url: `/${bucketName}/${destKey}?partNumber=1`, + query: { + partNumber: 1, + uploadId, + }, + socket: {}, + }); + request.actionImplicitDenies = actionImplicitDenies; + return request; + } + + before(done => { + cleanup(); + async.waterfall( + [ + callback => bucketPut(authInfo, _createBucketPutRequest(bucketName), log, err => callback(err)), + callback => + objectPut( + authInfo, + versioningTestUtils.createPutObjectRequest(bucketName, sourceKey, Buffer.from('foo', 'utf8')), + undefined, + log, + err => callback(err), + ), + callback => + initiateMultipartUpload(userAuthInfo, initiateUserRequest, log, (err, res) => callback(err, res)), + ], + (err, res) => { + if (err) { + return done(err); + } + return parseString(res, (err, json) => { + uploadId = json.InitiateMultipartUploadResult.UploadId[0]; + return done(err); + }); + }, + ); + }); + + after(() => { + cleanup(); + }); + + it('should deny a null version part copy when IAM does not allow reading the source version', done => { + const request = _createUserCopyPartRequest({ objectGetVersion: true, objectPut: false }); + objectPutCopyPart(userAuthInfo, request, bucketName, sourceKey, 'null', log, err => { + assert(err, 'expected the part copy to be denied'); + assert.strictEqual(err.is.AccessDenied, true); + done(); + }); + }); + + it('should allow a null version part copy when IAM allows reading the source version', done => { + const request = _createUserCopyPartRequest({ objectGetVersion: false, objectPut: false }); + objectPutCopyPart(userAuthInfo, request, bucketName, sourceKey, 'null', log, err => { + assert.ifError(err); + done(); + }); + }); +});