From 2596f3fda8265b981e0241dcf3a66a87379efb79 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 24 Nov 2023 01:35:49 +0100 Subject: [PATCH] CLDSRV-428: put apis updated for implicit deny In this commit put apis have been updated to check for implicit deny returned by vault and added as a parameter in the request Object. Tests have also been added for the metadataUtils validateBucket function. MetadataUtils functions have been updated to check for implicit deny. The goal is to implement the same authorization logic as AWS, where an implicit deny from IAM and an Allow from the Bucket Policy should allow the request for example. For the delete on the objectPutCopyPart and objectPutPart as we need to deferentiate between the vault request and the external backend once a delete is applied to the request directly as it's unique per API call this value is then added to the request object. here's the link to the design doc for more details: https://github.com/scality/citadel/blob/development/1.0/docs/design/bucket-policies.md?plain=1#L263 --- constants.js | 2 + lib/api/bucketPutACL.js | 31 ++-- lib/api/bucketPutCors.js | 5 +- lib/api/bucketPutEncryption.js | 8 +- lib/api/bucketPutLifecycle.js | 11 +- lib/api/bucketPutNotification.js | 8 +- lib/api/bucketPutObjectLock.js | 8 +- lib/api/bucketPutPolicy.js | 6 +- lib/api/bucketPutReplication.js | 6 +- lib/api/bucketPutVersioning.js | 6 +- lib/api/bucketPutWebsite.js | 5 +- lib/api/objectPut.js | 6 +- lib/api/objectPutACL.js | 9 +- lib/api/objectPutCopyPart.js | 13 +- lib/api/objectPutLegalHold.js | 6 +- lib/api/objectPutPart.js | 12 +- lib/api/objectPutRetention.js | 6 +- lib/api/objectPutTagging.js | 9 +- lib/metadata/metadataUtils.js | 193 +++++++++++++++----- tests/multipleBackend/objectPutCopyPart.js | 2 + tests/multipleBackend/objectPutPart.js | 1 + tests/unit/DummyRequest.js | 2 +- tests/unit/api/apiUtils/tagConditionKeys.js | 1 + tests/unit/api/bucketPutACL.js | 21 +++ tests/unit/api/bucketPutCors.js | 1 + tests/unit/api/bucketPutEncryption.js | 1 + tests/unit/api/bucketPutLifecycle.js | 1 + tests/unit/api/bucketPutNotification.js | 2 + tests/unit/api/bucketPutObjectLock.js | 2 + tests/unit/api/bucketPutPolicy.js | 2 + tests/unit/api/bucketPutWebsite.js | 2 + tests/unit/api/objectPutACL.js | 14 ++ tests/unit/api/objectPutLegalHold.js | 2 + tests/unit/api/objectPutRetention.js | 7 + tests/unit/api/objectPutTagging.js | 4 +- tests/unit/helpers.js | 3 + tests/unit/metadata/metadataUtils.spec.js | 55 ++++++ 37 files changed, 357 insertions(+), 116 deletions(-) create mode 100644 tests/unit/metadata/metadataUtils.spec.js diff --git a/constants.js b/constants.js index ded42c977f..55e6c4ebbd 100644 --- a/constants.js +++ b/constants.js @@ -153,6 +153,8 @@ const constants = { 'objectDeleteTagging', 'objectGetTagging', 'objectPutTagging', + 'objectPutLegalHold', + 'objectPutRetention', ], // response header to be sent when there are invalid // user metadata in the object's metadata diff --git a/lib/api/bucketPutACL.js b/lib/api/bucketPutACL.js index 0ca8d681d5..348a7172d9 100644 --- a/lib/api/bucketPutACL.js +++ b/lib/api/bucketPutACL.js @@ -6,7 +6,7 @@ const aclUtils = require('../utilities/aclUtils'); const { cleanUpBucket } = require('./apiUtils/bucket/bucketCreation'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const constants = require('../../constants'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const vault = require('../auth/vault'); const { pushMetric } = require('../utapi/utilities'); @@ -43,7 +43,7 @@ const { pushMetric } = require('../utapi/utilities'); function bucketPutACL(authInfo, request, log, callback) { log.debug('processing request', { method: 'bucketPutACL' }); - const bucketName = request.bucketName; + const { bucketName } = request; const canonicalID = authInfo.getCanonicalID(); const newCannedACL = request.headers['x-amz-acl']; const possibleCannedACL = [ @@ -53,17 +53,6 @@ function bucketPutACL(authInfo, request, log, callback) { 'authenticated-read', 'log-delivery-write', ]; - if (newCannedACL && possibleCannedACL.indexOf(newCannedACL) === -1) { - log.trace('invalid canned acl argument', { - acl: newCannedACL, - method: 'bucketPutACL', - }); - return callback(errors.InvalidArgument); - } - if (!aclUtils.checkGrantHeaderValidity(request.headers)) { - log.trace('invalid acl header'); - return callback(errors.InvalidArgument); - } const possibleGroups = [constants.allAuthedUsersId, constants.publicId, constants.logId, @@ -71,7 +60,7 @@ function bucketPutACL(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutACL', + requestType: request.apiMethods || 'bucketPutACL', request, }; const possibleGrants = ['FULL_CONTROL', 'WRITE', @@ -102,7 +91,7 @@ function bucketPutACL(authInfo, request, log, callback) { return async.waterfall([ function waterfall1(next) { - metadataValidateBucket(metadataValParams, log, + standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { if (err) { log.trace('request authorization failed', { @@ -111,6 +100,18 @@ function bucketPutACL(authInfo, request, log, callback) { }); return next(err, bucket); } + // if the API call is allowed, ensure that the parameters are valid + if (newCannedACL && possibleCannedACL.indexOf(newCannedACL) === -1) { + log.trace('invalid canned acl argument', { + acl: newCannedACL, + method: 'bucketPutACL', + }); + return next(errors.InvalidArgument); + } + if (!aclUtils.checkGrantHeaderValidity(request.headers)) { + log.trace('invalid acl header'); + return next(errors.InvalidArgument); + } return next(null, bucket); }); }, diff --git a/lib/api/bucketPutCors.js b/lib/api/bucketPutCors.js index 23876b2ce1..321b82e19b 100644 --- a/lib/api/bucketPutCors.js +++ b/lib/api/bucketPutCors.js @@ -22,7 +22,7 @@ const requestType = 'bucketPutCors'; */ function bucketPutCors(authInfo, request, log, callback) { log.debug('processing request', { method: 'bucketPutCors' }); - const bucketName = request.bucketName; + const { bucketName } = request; const canonicalID = authInfo.getCanonicalID(); if (!request.post) { @@ -66,7 +66,8 @@ function bucketPutCors(authInfo, request, log, callback) { }); }, function validateBucketAuthorization(bucket, rules, corsHeaders, next) { - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) { + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, + log, request, request.actionImplicitDenies)) { log.debug('access denied for account on bucket', { requestType, }); diff --git a/lib/api/bucketPutEncryption.js b/lib/api/bucketPutEncryption.js index 148c1ec3a3..5a407f2f63 100644 --- a/lib/api/bucketPutEncryption.js +++ b/lib/api/bucketPutEncryption.js @@ -3,7 +3,7 @@ const async = require('async'); const { parseEncryptionXml } = require('./apiUtils/bucket/bucketEncryption'); const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const kms = require('../kms/wrapper'); const { pushMetric } = require('../utapi/utilities'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); @@ -18,17 +18,17 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); */ function bucketPutEncryption(authInfo, request, log, callback) { - const bucketName = request.bucketName; + const { bucketName } = request; const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutEncryption', + requestType: request.apiMethods || 'bucketPutEncryption', request, }; return async.waterfall([ - next => metadataValidateBucket(metadataValParams, log, next), + next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, next), (bucket, next) => checkExpectedBucketOwner(request.headers, bucket, log, err => next(err, bucket)), (bucket, next) => { log.trace('parsing encryption config', { method: 'bucketPutEncryption' }); diff --git a/lib/api/bucketPutLifecycle.js b/lib/api/bucketPutLifecycle.js index f8b4cd0863..42716eff04 100644 --- a/lib/api/bucketPutLifecycle.js +++ b/lib/api/bucketPutLifecycle.js @@ -1,12 +1,11 @@ const { waterfall } = require('async'); const uuid = require('uuid/v4'); -const LifecycleConfiguration = - require('arsenal').models.LifecycleConfiguration; +const { LifecycleConfiguration } = require('arsenal').models; const parseXML = require('../utilities/parseXML'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); /** @@ -21,11 +20,11 @@ const { pushMetric } = require('../utapi/utilities'); function bucketPutLifecycle(authInfo, request, log, callback) { log.debug('processing request', { method: 'bucketPutLifecycle' }); - const bucketName = request.bucketName; + const { bucketName } = request; const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutLifecycle', + requestType: request.apiMethods || 'bucketPutLifecycle', request, }; return waterfall([ @@ -42,7 +41,7 @@ function bucketPutLifecycle(authInfo, request, log, callback) { return next(null, configObj); }); }, - (lcConfig, next) => metadataValidateBucket(metadataValParams, log, + (lcConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { if (err) { return next(err, bucket); diff --git a/lib/api/bucketPutNotification.js b/lib/api/bucketPutNotification.js index 3418ca5e2b..ab82b5109a 100644 --- a/lib/api/bucketPutNotification.js +++ b/lib/api/bucketPutNotification.js @@ -4,7 +4,7 @@ const parseXML = require('../utilities/parseXML'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const getNotificationConfiguration = require('./apiUtils/bucket/getNotificationConfiguration'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); /** @@ -19,11 +19,11 @@ const { pushMetric } = require('../utapi/utilities'); function bucketPutNotification(authInfo, request, log, callback) { log.debug('processing request', { method: 'bucketPutNotification' }); - const bucketName = request.bucketName; + const { bucketName } = request; const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutNotification', + requestType: request.apiMethods || 'bucketPutNotification', request, }; @@ -34,7 +34,7 @@ function bucketPutNotification(authInfo, request, log, callback) { const notifConfig = notificationConfig.error ? undefined : notificationConfig; process.nextTick(() => next(notificationConfig.error, notifConfig)); }, - (notifConfig, next) => metadataValidateBucket(metadataValParams, log, + (notifConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => next(err, bucket, notifConfig)), (bucket, notifConfig, next) => { bucket.setNotificationConfiguration(notifConfig); diff --git a/lib/api/bucketPutObjectLock.js b/lib/api/bucketPutObjectLock.js index ba240516f1..1b3a452640 100644 --- a/lib/api/bucketPutObjectLock.js +++ b/lib/api/bucketPutObjectLock.js @@ -1,13 +1,13 @@ const { waterfall } = require('async'); const arsenal = require('arsenal'); -const errors = arsenal.errors; -const ObjectLockConfiguration = arsenal.models.ObjectLockConfiguration; +const { errors } = arsenal; +const { models: { ObjectLockConfiguration } } = arsenal; const parseXML = require('../utilities/parseXML'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); /** @@ -41,7 +41,7 @@ function bucketPutObjectLock(authInfo, request, log, callback) { return next(configObj.error || null, configObj); }); }, - (objectLockConfig, next) => metadataValidateBucket(metadataValParams, + (objectLockConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { if (err) { return next(err, bucket); diff --git a/lib/api/bucketPutPolicy.js b/lib/api/bucketPutPolicy.js index 328c98a306..024a69209f 100644 --- a/lib/api/bucketPutPolicy.js +++ b/lib/api/bucketPutPolicy.js @@ -3,7 +3,7 @@ const { errors, models } = require('arsenal'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { validatePolicyResource } = require('./apiUtils/authorization/permissionChecks'); const { BucketPolicy } = models; @@ -37,7 +37,7 @@ function bucketPutPolicy(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutPolicy', + requestType: request.apiMethods || 'bucketPutPolicy', request, }; @@ -70,7 +70,7 @@ function bucketPutPolicy(authInfo, request, log, callback) { return next(null, bucketPolicy); }); }, - (bucketPolicy, next) => metadataValidateBucket(metadataValParams, log, + (bucketPolicy, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { if (err) { return next(err, bucket); diff --git a/lib/api/bucketPutReplication.js b/lib/api/bucketPutReplication.js index 2937a9cb72..68de6be95a 100644 --- a/lib/api/bucketPutReplication.js +++ b/lib/api/bucketPutReplication.js @@ -2,7 +2,7 @@ const { waterfall } = require('async'); const { errors } = require('arsenal'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const { getReplicationConfiguration } = require('./apiUtils/bucket/getReplicationConfiguration'); @@ -27,7 +27,7 @@ function bucketPutReplication(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutReplication', + requestType: request.apiMethods || 'bucketPutReplication', request, }; return waterfall([ @@ -36,7 +36,7 @@ function bucketPutReplication(authInfo, request, log, callback) { // Check bucket user privileges and ensure versioning is 'Enabled'. (config, next) => // TODO: Validate that destination bucket exists and has versioning. - metadataValidateBucket(metadataValParams, log, (err, bucket) => { + standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { if (err) { return next(err); } diff --git a/lib/api/bucketPutVersioning.js b/lib/api/bucketPutVersioning.js index ced5261d67..f110df21b0 100644 --- a/lib/api/bucketPutVersioning.js +++ b/lib/api/bucketPutVersioning.js @@ -4,7 +4,7 @@ const { errors } = require('arsenal'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucket } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const versioningNotImplBackends = require('../../constants').versioningNotImplBackends; @@ -87,13 +87,13 @@ function bucketPutVersioning(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutVersioning', + requestType: request.apiMethods || 'bucketPutVersioning', request, }; return waterfall([ next => _parseXML(request, log, next), - next => metadataValidateBucket(metadataValParams, log, + next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => next(err, bucket)), // ignore extra null object, (bucket, next) => parseString(request.post, (err, result) => { // just for linting; there should not be any parsing error here diff --git a/lib/api/bucketPutWebsite.js b/lib/api/bucketPutWebsite.js index ea15c4e433..027c946688 100644 --- a/lib/api/bucketPutWebsite.js +++ b/lib/api/bucketPutWebsite.js @@ -21,7 +21,7 @@ const requestType = 'bucketPutWebsite'; */ function bucketPutWebsite(authInfo, request, log, callback) { log.debug('processing request', { method: 'bucketPutWebsite' }); - const bucketName = request.bucketName; + const { bucketName } = request; const canonicalID = authInfo.getCanonicalID(); if (!request.post) { @@ -46,7 +46,8 @@ function bucketPutWebsite(authInfo, request, log, callback) { }); }, function validateBucketAuthorization(bucket, config, next) { - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) { + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, + log, request, request.actionImplicitDenies)) { log.debug('access denied for user on bucket', { requestType, method: 'bucketPutWebsite', diff --git a/lib/api/objectPut.js b/lib/api/objectPut.js index 35dd404fe0..204fbe6968 100644 --- a/lib/api/objectPut.js +++ b/lib/api/objectPut.js @@ -7,7 +7,7 @@ const { getObjectSSEConfiguration } = require('./apiUtils/bucket/bucketEncryptio const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const createAndStoreObject = require('./apiUtils/object/createAndStoreObject'); const { checkQueryVersionId } = require('./apiUtils/object/versioning'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const { validateHeaders } = require('./apiUtils/object/objectLockHelpers'); const { hasNonPrintables } = require('../utilities/stringChecks'); @@ -58,7 +58,7 @@ function objectPut(authInfo, request, streamingV4Params, log, callback) { } const invalidSSEError = errors.InvalidArgument.customizeDescription( 'The encryption method specified is not supported'); - const requestType = 'objectPut'; + const requestType = request.apiMethods || 'objectPut'; const valParams = { authInfo, bucketName, objectKey, requestType, request }; const canonicalID = authInfo.getCanonicalID(); @@ -75,7 +75,7 @@ function objectPut(authInfo, request, streamingV4Params, log, callback) { log.trace('owner canonicalID to send to data', { canonicalID }); - return metadataValidateBucketAndObj(valParams, log, + return standardMetadataValidateBucketAndObj(valParams, request.actionImplicitDenies, log, (err, bucket, objMD) => { const responseHeaders = collectCorsHeaders(headers.origin, method, bucket); diff --git a/lib/api/objectPutACL.js b/lib/api/objectPutACL.js index 0b673a4e1b..68cb6760d2 100644 --- a/lib/api/objectPutACL.js +++ b/lib/api/objectPutACL.js @@ -9,7 +9,7 @@ const constants = require('../../constants'); const vault = require('../auth/vault'); const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/versioning'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); /* Format of xml request: @@ -43,8 +43,7 @@ const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); */ function objectPutACL(authInfo, request, log, cb) { log.debug('processing request', { method: 'objectPutACL' }); - const bucketName = request.bucketName; - const objectKey = request.objectKey; + const { bucketName, objectKey } = request; const newCannedACL = request.headers['x-amz-acl']; const possibleCannedACL = [ 'private', @@ -82,7 +81,7 @@ function objectPutACL(authInfo, request, log, cb) { authInfo, bucketName, objectKey, - requestType: 'objectPutACL', + requestType: request.apiMethods || 'objectPutACL', versionId: reqVersionId, }; @@ -107,7 +106,7 @@ function objectPutACL(authInfo, request, log, cb) { return async.waterfall([ function validateBucketAndObj(next) { - return metadataValidateBucketAndObj(metadataValParams, log, + return standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, bucket, objectMD) => { if (err) { return next(err); diff --git a/lib/api/objectPutCopyPart.js b/lib/api/objectPutCopyPart.js index 8fb224204f..0b09d26314 100644 --- a/lib/api/objectPutCopyPart.js +++ b/lib/api/objectPutCopyPart.js @@ -12,7 +12,7 @@ const { pushMetric } = require('../utapi/utilities'); const logger = require('../utilities/logger'); const services = require('../services'); const setUpCopyLocator = require('./apiUtils/object/setUpCopyLocator'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const versionIdUtils = versioning.VersionID; const { config } = require('../Config'); @@ -58,7 +58,7 @@ 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 uploadId = request.query.uploadId; + const { query: { uploadId } } = request; const valPutParams = { authInfo, @@ -89,7 +89,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket, return async.waterfall([ function checkDestAuth(next) { - return metadataValidateBucketAndObj(valPutParams, log, + return standardMetadataValidateBucketAndObj(valPutParams, request.actionImplicitDenies, log, (err, destBucketMD) => { if (err) { log.debug('error validating authorization for ' + @@ -108,7 +108,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket, }); }, function checkSourceAuthorization(destBucketMD, next) { - return metadataValidateBucketAndObj(valGetParams, log, + return standardMetadataValidateBucketAndObj(valGetParams, request.actionImplicitDenies, log, (err, sourceBucketMD, sourceObjMD) => { if (err) { log.debug('error validating get part of request', @@ -249,6 +249,9 @@ function objectPutCopyPart(authInfo, request, sourceBucket, splitter, next, ) { + const originalIdentityAuthzResults = request.actionImplicitDenies; + // eslint-disable-next-line no-param-reassign + delete request.actionImplicitDenies; data.uploadPartCopy( request, log, @@ -259,6 +262,8 @@ function objectPutCopyPart(authInfo, request, sourceBucket, dataStoreContext, locationConstraintCheck, (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, diff --git a/lib/api/objectPutLegalHold.js b/lib/api/objectPutLegalHold.js index b040fedff5..b37f3b28cd 100644 --- a/lib/api/objectPutLegalHold.js +++ b/lib/api/objectPutLegalHold.js @@ -6,7 +6,7 @@ const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/versioning'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const metadata = require('../metadata/wrapper'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const { parseLegalHoldXml } = s3middleware.objectLegalHold; @@ -40,13 +40,13 @@ function objectPutLegalHold(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectPutLegalHold', + requestType: request.apiMethods || 'objectPutLegalHold', versionId, request, }; return async.waterfall([ - next => metadataValidateBucketAndObj(metadataValParams, log, + next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, bucket, objectMD) => { if (err) { log.trace('request authorization failed', diff --git a/lib/api/objectPutPart.js b/lib/api/objectPutPart.js index f981e5f809..cd56fe2fde 100644 --- a/lib/api/objectPutPart.js +++ b/lib/api/objectPutPart.js @@ -91,9 +91,10 @@ function objectPutPart(authInfo, request, streamingV4Params, log, }); // Note that keys in the query object retain their case, so // `request.query.uploadId` must be called with that exact capitalization. - const uploadId = request.query.uploadId; + const { query: { uploadId } } = request; const mpuBucketName = `${constants.mpuBucketPrefix}${bucketName}`; - const objectKey = request.objectKey; + const { objectKey } = request; + const originalIdentityAuthzResults = request.actionImplicitDenies; return async.waterfall([ // Get the destination bucket. @@ -116,7 +117,8 @@ function objectPutPart(authInfo, request, streamingV4Params, log, // For validating the request at the destinationBucket level the // `requestType` is the general 'objectPut'. const requestType = 'objectPut'; - if (!isBucketAuthorized(destinationBucket, requestType, canonicalID, authInfo, log, request)) { + if (!isBucketAuthorized(destinationBucket, request.apiMethods || requestType, canonicalID, authInfo, + log, request, request.actionImplicitDenies)) { log.debug('access denied for user on bucket', { requestType }); return next(errors.AccessDenied, destinationBucket); } @@ -203,6 +205,8 @@ function objectPutPart(authInfo, request, streamingV4Params, log, partNumber, bucketName, }; + // eslint-disable-next-line no-param-reassign + delete request.actionImplicitDenies; writeContinue(request, request._response); return data.putPart(request, mpuInfo, streamingV4Params, objectLocationConstraint, locationConstraintCheck, log, @@ -385,6 +389,8 @@ function objectPutPart(authInfo, request, streamingV4Params, log, ], (err, destinationBucket, hexDigest, prevObjectSize) => { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destinationBucket); + // eslint-disable-next-line no-param-reassign + request.actionImplicitDenies = originalIdentityAuthzResults; if (err) { if (err === skipError) { return cb(null, hexDigest, corsHeaders); diff --git a/lib/api/objectPutRetention.js b/lib/api/objectPutRetention.js index 578af167db..dc551bc7bd 100644 --- a/lib/api/objectPutRetention.js +++ b/lib/api/objectPutRetention.js @@ -5,7 +5,7 @@ const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/versioning'); const { ObjectLockInfo, checkUserGovernanceBypass, hasGovernanceBypassHeader } = require('./apiUtils/object/objectLockHelpers'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); @@ -41,13 +41,13 @@ function objectPutRetention(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectPutRetention', + requestType: request.apiMethods || 'objectPutRetention', versionId: reqVersionId, request, }; return async.waterfall([ - next => metadataValidateBucketAndObj(metadataValParams, log, + next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, bucket, objectMD) => { if (err) { log.trace('request authorization failed', diff --git a/lib/api/objectPutTagging.js b/lib/api/objectPutTagging.js index e20f9e6c68..8514b8340b 100644 --- a/lib/api/objectPutTagging.js +++ b/lib/api/objectPutTagging.js @@ -4,7 +4,7 @@ const { errors, s3middleware } = require('arsenal'); const { decodeVersionId, getVersionIdResHeader } = require('./apiUtils/object/versioning'); -const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); +const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); @@ -24,8 +24,7 @@ const REPLICATION_ACTION = 'PUT_TAGGING'; function objectPutTagging(authInfo, request, log, callback) { log.debug('processing request', { method: 'objectPutTagging' }); - const bucketName = request.bucketName; - const objectKey = request.objectKey; + const { bucketName, objectKey } = request; const decodedVidResult = decodeVersionId(request.query); if (decodedVidResult instanceof Error) { @@ -41,13 +40,13 @@ function objectPutTagging(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectPutTagging', + requestType: request.apiMethods || 'objectPutTagging', versionId: reqVersionId, request, }; return async.waterfall([ - next => metadataValidateBucketAndObj(metadataValParams, log, + next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, bucket, objectMD) => { if (err) { log.trace('request authorization failed', diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index b03c2ffc14..7a33fb722f 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -42,7 +42,6 @@ function getNullVersion(objMD, bucketName, objectKey, log, cb) { * NOTE: If the value of `versionId` param is 'null', this function returns the * master version objMD. The null version object md must be retrieved in a * separate step using the master object md: see getNullVersion(). - * @param {string} requestType - type of request * @param {string} bucketName - name of bucket * @param {string} objectKey - name of object key * @param {string} [versionId] - version of object to retrieve @@ -50,7 +49,7 @@ function getNullVersion(objMD, bucketName, objectKey, log, cb) { * @param {function} cb - callback * @return {undefined} - and call callback with err, bucket md and object md */ -function metadataGetBucketAndObject(requestType, bucketName, objectKey, +function metadataGetBucketAndObject(bucketName, objectKey, versionId, log, cb) { const options = { // if attempting to get 'null' version, must retrieve null version id @@ -73,13 +72,6 @@ function metadataGetBucketAndObject(requestType, bucketName, objectKey, }); return cb(errors.NoSuchBucket); } - if (bucketShield(bucket, requestType)) { - log.debug('bucket is shielded from request', { - requestType, - method: 'metadataGetBucketAndObject', - }); - return cb(errors.NoSuchBucket); - } log.trace('found bucket in metadata'); return cb(null, bucket, obj); }); @@ -118,6 +110,53 @@ function metadataGetObject(bucketName, objectKey, versionId, log, cb) { }); } +/** + * Validate that a bucket is accessible and authorized to the user, + * return a specific error code otherwise + * + * @param {BucketInfo} bucket - bucket info + * @param {object} params - function parameters + * @param {AuthInfo} params.authInfo - AuthInfo class instance, requester's info + * @param {string} params.requestType - type of request + * @param {string} [params.preciseRequestType] - precise type of request + * @param {object} params.request - http request object + * @param {RequestLogger} log - request logger + * @param {object} actionImplicitDenies - identity authorization results + * @return {ArsenalError|null} returns a validation error, or null if validation OK + * The following errors may be returned: + * - NoSuchBucket: bucket is shielded + * - MethodNotAllowed: requester is not bucket owner and asking for a + * bucket policy operation + * - AccessDenied: bucket is not authorized + */ +function validateBucket(bucket, params, log, actionImplicitDenies = {}) { + const { authInfo, preciseRequestType, request } = params; + let requestType = params.requestType; + if (bucketShield(bucket, requestType)) { + log.debug('bucket is shielded from request', { + requestType, + method: 'validateBucket', + }); + return errors.NoSuchBucket; + } + // if requester is not bucket owner, bucket policy actions should be denied with + // MethodNotAllowed error + const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy']; + const canonicalID = authInfo.getCanonicalID(); + if (!Array.isArray(requestType)) { + requestType = [requestType]; + } + if (bucket.getOwner() !== canonicalID && requestType.some(type => onlyOwnerAllowed.includes(type))) { + return errors.MethodNotAllowed; + } + if (!isBucketAuthorized(bucket, (preciseRequestType || requestType), canonicalID, + authInfo, log, request, actionImplicitDenies)) { + log.debug('access denied for user on bucket', { requestType }); + return errors.AccessDenied; + } + return null; +} + /** metadataValidateBucketAndObj - retrieve bucket and object md from metadata * and check if user is authorized to access them. * @param {object} params - function parameters @@ -132,36 +171,94 @@ function metadataGetObject(bucketName, objectKey, versionId, log, cb) { * @return {undefined} - and call callback with params err, bucket md */ function metadataValidateBucketAndObj(params, log, callback) { - const { authInfo, bucketName, objectKey, versionId, requestType, preciseRequestType, request } = params; - const canonicalID = authInfo.getCanonicalID(); + const { authInfo, bucketName, objectKey, versionId, request } = params; + let requestType = params.requestType; + if (!Array.isArray(requestType)) { + requestType = [requestType]; + } async.waterfall([ - function getBucketAndObjectMD(next) { - return metadataGetBucketAndObject(requestType, bucketName, - objectKey, versionId, log, next); - }, - function checkBucketAuth(bucket, objMD, next) { - // if requester is not bucket owner, bucket policy actions should be denied with - // MethodNotAllowed error - const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy']; - if (bucket.getOwner() !== canonicalID && onlyOwnerAllowed.includes(requestType)) { - return next(errors.MethodNotAllowed, bucket); + next => metadataGetBucketAndObject(bucketName, + objectKey, versionId, log, (err, bucket, objMD) => { + if (err) { + return next(err); + } + return next(null, bucket, objMD); + }), + (bucket, objMD, next) => { + const validationError = validateBucket(bucket, params, log); + if (validationError) { + return next(validationError, bucket); } - if (!isBucketAuthorized(bucket, (preciseRequestType || requestType), canonicalID, - authInfo, log, request)) { - log.debug('access denied for user on bucket', { requestType }); + if (objMD && versionId === 'null') { + return getNullVersion(objMD, bucketName, objectKey, log, + (err, nullVer) => next(err, bucket, nullVer)); + } + return next(null, bucket, objMD); + }, + (bucket, objMD, next) => { + const canonicalID = authInfo.getCanonicalID(); + if (!isObjAuthorized(bucket, objMD, requestType, canonicalID, authInfo, + log, request)) { + log.debug('access denied for user on object', { requestType }); return next(errors.AccessDenied, bucket); } return next(null, bucket, objMD); }, - function handleNullVersionGet(bucket, objMD, next) { + ], (err, bucket, objMD) => { + if (err) { + // still return bucket for cors headers + return callback(err, bucket); + } + return callback(null, bucket, objMD); + }); +} + +/** standardMetadataValidateBucketAndObj - retrieve bucket and object md from metadata + * and check if user is authorized to access them. + * @param {object} params - function parameters + * @param {AuthInfo} params.authInfo - AuthInfo class instance, requester's info + * @param {string} params.bucketName - name of bucket + * @param {string} params.objectKey - name of object + * @param {string} [params.versionId] - version id if getting specific version + * @param {string} params.requestType - type of request + * @param {object} params.request - http request object + * @param {boolean} actionImplicitDenies - identity authorization results + * @param {RequestLogger} log - request logger + * @param {function} callback - callback + * @return {undefined} - and call callback with params err, bucket md + */ +function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, callback) { + const { authInfo, bucketName, objectKey, versionId, request } = params; + let requestType = params.requestType; + if (!Array.isArray(requestType)) { + requestType = [requestType]; + } + return async.waterfall([ + next => metadataGetBucketAndObject(bucketName, + objectKey, versionId, log, (err, bucket, objMD) => { + if (err) { + if (actionImplicitDenies && Object.values(actionImplicitDenies).some(v => v === true)) { + return next(errors.AccessDenied); + } + return next(err); + } + return next(null, bucket, objMD); + }), + (bucket, objMD, next) => { + const validationError = validateBucket(bucket, params, log, actionImplicitDenies); + if (validationError) { + return next(validationError, bucket); + } if (objMD && versionId === 'null') { return getNullVersion(objMD, bucketName, objectKey, log, (err, nullVer) => next(err, bucket, nullVer)); } return next(null, bucket, objMD); }, - function checkObjectAuth(bucket, objMD, next) { - if (!isObjAuthorized(bucket, objMD, requestType, canonicalID, authInfo, log, request)) { + (bucket, objMD, next) => { + const canonicalID = authInfo.getCanonicalID(); + if (!isObjAuthorized(bucket, objMD, requestType, canonicalID, authInfo, + log, request, actionImplicitDenies)) { log.debug('access denied for user on object', { requestType }); return next(errors.AccessDenied, bucket); } @@ -169,7 +266,6 @@ function metadataValidateBucketAndObj(params, log, callback) { }, ], (err, bucket, objMD) => { if (err) { - // still return bucket for cors headers return callback(err, bucket); } return callback(null, bucket, objMD); @@ -214,29 +310,44 @@ function metadataGetBucket(requestType, bucketName, log, cb) { * @return {undefined} - and call callback with params err, bucket md */ function metadataValidateBucket(params, log, callback) { - const { authInfo, bucketName, requestType, preciseRequestType, request } = params; - const canonicalID = authInfo.getCanonicalID(); + const { bucketName, requestType } = params; return metadataGetBucket(requestType, bucketName, log, (err, bucket) => { if (err) { return callback(err); } - // if requester is not bucket owner, bucket policy actions should be denied with - // MethodNotAllowed error - const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy']; - if (bucket.getOwner() !== canonicalID && onlyOwnerAllowed.includes(requestType)) { - return callback(errors.MethodNotAllowed, bucket); - } - // still return bucket for cors headers - if (!isBucketAuthorized(bucket, (preciseRequestType || requestType), canonicalID, authInfo, log, request)) { - log.debug('access denied for user on bucket', { requestType }); - return callback(errors.AccessDenied, bucket); + const validationError = validateBucket(bucket, params, log); + return callback(validationError, bucket); + }); +} + +/** standardMetadataValidateBucket - retrieve bucket from metadata and check if user + * is authorized to access it + * @param {object} params - function parameters + * @param {AuthInfo} params.authInfo - AuthInfo class instance, requester's info + * @param {string} params.bucketName - name of bucket + * @param {string} params.requestType - type of request + * @param {string} params.request - http request object + * @param {boolean} actionImplicitDenies - identity authorization results + * @param {RequestLogger} log - request logger + * @param {function} callback - callback + * @return {undefined} - and call callback with params err, bucket md + */ +function standardMetadataValidateBucket(params, actionImplicitDenies, log, callback) { + const { bucketName, requestType } = params; + return metadataGetBucket(requestType, bucketName, log, (err, bucket) => { + if (err) { + return callback(err); } - return callback(null, bucket); + const validationError = validateBucket(bucket, params, log, actionImplicitDenies); + return callback(validationError, bucket); }); } module.exports = { metadataGetObject, + validateBucket, metadataValidateBucketAndObj, + standardMetadataValidateBucketAndObj, metadataValidateBucket, + standardMetadataValidateBucket, }; diff --git a/tests/multipleBackend/objectPutCopyPart.js b/tests/multipleBackend/objectPutCopyPart.js index 1a7a3df05d..e869b4d1e3 100644 --- a/tests/multipleBackend/objectPutCopyPart.js +++ b/tests/multipleBackend/objectPutCopyPart.js @@ -80,6 +80,7 @@ errorPutCopyPart) { objectKey: destObjName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${destObjName}?uploads`, + actionImplicitDenies: false, }; if (mpuLoc) { initiateReq.headers = { 'host': `${bucketName}.s3.amazonaws.com`, @@ -94,6 +95,7 @@ errorPutCopyPart) { objectKey: sourceObjName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; if (srcObjLoc) { sourceObjPutParams.headers = { 'host': `${bucketName}.s3.amazonaws.com`, diff --git a/tests/multipleBackend/objectPutPart.js b/tests/multipleBackend/objectPutPart.js index 33f7ce5ad7..838eacf531 100644 --- a/tests/multipleBackend/objectPutPart.js +++ b/tests/multipleBackend/objectPutPart.js @@ -70,6 +70,7 @@ errorDescription) { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: `/${objectName}?uploads`, + actionImplicitDenies: false, }; if (mpuLoc) { initiateReq.headers = { 'host': `${bucketName}.s3.amazonaws.com`, diff --git a/tests/unit/DummyRequest.js b/tests/unit/DummyRequest.js index e61c37feda..28b21337eb 100644 --- a/tests/unit/DummyRequest.js +++ b/tests/unit/DummyRequest.js @@ -16,7 +16,7 @@ class DummyRequest extends http.IncomingMessage { this.parsedContentLength = 0; } } - + this.actionImplicitDenies = false; if (Array.isArray(msg)) { msg.forEach(part => { this.push(part); diff --git a/tests/unit/api/apiUtils/tagConditionKeys.js b/tests/unit/api/apiUtils/tagConditionKeys.js index 4a653f78b6..49c957f3b5 100644 --- a/tests/unit/api/apiUtils/tagConditionKeys.js +++ b/tests/unit/api/apiUtils/tagConditionKeys.js @@ -24,6 +24,7 @@ const bucketPutReq = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const taggingUtil = new TaggingConfigTester(); diff --git a/tests/unit/api/bucketPutACL.js b/tests/unit/api/bucketPutACL.js index f6a11b5b14..6b3cad617a 100644 --- a/tests/unit/api/bucketPutACL.js +++ b/tests/unit/api/bucketPutACL.js @@ -72,6 +72,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -90,6 +91,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { assert.strictEqual(err, undefined); @@ -111,6 +113,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; const testACLRequest2 = { bucketName, @@ -121,6 +124,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { assert.strictEqual(err, undefined); @@ -149,6 +153,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; const testACLRequest2 = { bucketName, @@ -159,6 +164,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -197,6 +203,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { assert.strictEqual(err, undefined); @@ -238,6 +245,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { assert.strictEqual(err, undefined); @@ -271,6 +279,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; return bucketPutACL(authInfo, testACLRequest, log, err => { assert.deepStrictEqual(err, errors.InvalidArgument); @@ -292,6 +301,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -351,6 +361,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -386,6 +397,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -418,6 +430,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -461,6 +474,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -493,6 +507,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; return bucketPutACL(authInfo, testACLRequest, log, err => { @@ -525,6 +540,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { assert.deepStrictEqual(err, errors.UnresolvableGrantByEmailAddress); @@ -560,6 +576,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -605,6 +622,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -640,6 +658,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -674,6 +693,7 @@ describe('putBucketACL API', () => { '', url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { @@ -695,6 +715,7 @@ describe('putBucketACL API', () => { }, url: '/?acl', query: { acl: '' }, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { diff --git a/tests/unit/api/bucketPutCors.js b/tests/unit/api/bucketPutCors.js index 5170edce8e..69f6bda7e6 100644 --- a/tests/unit/api/bucketPutCors.js +++ b/tests/unit/api/bucketPutCors.js @@ -19,6 +19,7 @@ const testBucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; function _testPutBucketCors(authInfo, request, log, errCode, cb) { diff --git a/tests/unit/api/bucketPutEncryption.js b/tests/unit/api/bucketPutEncryption.js index bc83ef5fe3..592b25008e 100644 --- a/tests/unit/api/bucketPutEncryption.js +++ b/tests/unit/api/bucketPutEncryption.js @@ -14,6 +14,7 @@ const bucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; describe('bucketPutEncryption API', () => { diff --git a/tests/unit/api/bucketPutLifecycle.js b/tests/unit/api/bucketPutLifecycle.js index 5df87bb21f..b3cd0071ec 100644 --- a/tests/unit/api/bucketPutLifecycle.js +++ b/tests/unit/api/bucketPutLifecycle.js @@ -17,6 +17,7 @@ const testBucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const expectedLifecycleConfig = { diff --git a/tests/unit/api/bucketPutNotification.js b/tests/unit/api/bucketPutNotification.js index e7ecceef3e..42456fba5b 100644 --- a/tests/unit/api/bucketPutNotification.js +++ b/tests/unit/api/bucketPutNotification.js @@ -15,6 +15,7 @@ const bucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const expectedNotifConfig = { @@ -52,6 +53,7 @@ function getNotifRequest(empty) { host: `${bucketName}.s3.amazonaws.com`, }, post: notifXml, + actionImplicitDenies: false, }; return putNotifConfigRequest; } diff --git a/tests/unit/api/bucketPutObjectLock.js b/tests/unit/api/bucketPutObjectLock.js index b70a42400b..e048bb40f2 100644 --- a/tests/unit/api/bucketPutObjectLock.js +++ b/tests/unit/api/bucketPutObjectLock.js @@ -15,6 +15,7 @@ const bucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const objectLockXml = ' { headers: { 'x-amz-acl': 'invalid-option' }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -79,6 +80,7 @@ describe('putObjectACL API', () => { headers: { 'x-amz-acl': 'public-read-write' }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -108,6 +110,7 @@ describe('putObjectACL API', () => { headers: { 'x-amz-acl': 'public-read' }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; const testObjACLRequest2 = { @@ -117,6 +120,7 @@ describe('putObjectACL API', () => { headers: { 'x-amz-acl': 'authenticated-read' }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -162,6 +166,7 @@ describe('putObjectACL API', () => { }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { objectPut(authInfo, testPutObjectRequest, undefined, log, @@ -204,6 +209,7 @@ describe('putObjectACL API', () => { }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -234,6 +240,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(acp.getXml(), 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -271,6 +278,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(acp.getXml(), 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -299,6 +307,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(acp.getXml(), 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -337,6 +346,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(acp.getXml(), 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; @@ -366,6 +376,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(modifiedXml, 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -394,6 +405,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(modifiedXml, 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; @@ -422,6 +434,7 @@ describe('putObjectACL API', () => { url: `/${bucketName}/${objectName}?acl`, post: [Buffer.from(acp.getXml(), 'utf8')], query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { @@ -450,6 +463,7 @@ describe('putObjectACL API', () => { }, url: `/${bucketName}/${objectName}?acl`, query: { acl: '' }, + actionImplicitDenies: false, }; bucketPut(authInfo, testPutBucketRequest, log, () => { diff --git a/tests/unit/api/objectPutLegalHold.js b/tests/unit/api/objectPutLegalHold.js index 2ab27b8770..839d7d7edf 100644 --- a/tests/unit/api/objectPutLegalHold.js +++ b/tests/unit/api/objectPutLegalHold.js @@ -19,6 +19,7 @@ const putBucketRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const putObjectRequest = new DummyRequest({ @@ -39,6 +40,7 @@ const putLegalHoldReq = status => ({ objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectLegalHoldXml(status), + actionImplicitDenies: false, }); describe('putObjectLegalHold API', () => { diff --git a/tests/unit/api/objectPutRetention.js b/tests/unit/api/objectPutRetention.js index d7c187109e..9a24d7a340 100644 --- a/tests/unit/api/objectPutRetention.js +++ b/tests/unit/api/objectPutRetention.js @@ -23,6 +23,7 @@ const bucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const putObjectRequest = new DummyRequest({ @@ -68,6 +69,7 @@ const putObjRetRequestGovernance = { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectRetentionXmlGovernance, + actionImplicitDenies: false, }; const putObjRetRequestGovernanceWithHeader = { @@ -78,6 +80,7 @@ const putObjRetRequestGovernanceWithHeader = { 'x-amz-bypass-governance-retention': 'true', }, post: objectRetentionXmlGovernance, + actionImplicitDenies: false, }; const putObjRetRequestCompliance = { @@ -85,6 +88,7 @@ const putObjRetRequestCompliance = { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectRetentionXmlCompliance, + actionImplicitDenies: false, }; const putObjRetRequestComplianceShorter = { @@ -92,6 +96,7 @@ const putObjRetRequestComplianceShorter = { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectRetentionXmlComplianceShorter, + actionImplicitDenies: false, }; const putObjRetRequestGovernanceLonger = { @@ -99,6 +104,7 @@ const putObjRetRequestGovernanceLonger = { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectRetentionXmlGovernanceLonger, + actionImplicitDenies: false, }; const putObjRetRequestGovernanceShorter = { @@ -106,6 +112,7 @@ const putObjRetRequestGovernanceShorter = { objectKey: objectName, headers: { host: `${bucketName}.s3.amazonaws.com` }, post: objectRetentionXmlGovernanceShorter, + actionImplicitDenies: false, }; describe('putObjectRetention API', () => { diff --git a/tests/unit/api/objectPutTagging.js b/tests/unit/api/objectPutTagging.js index 72a0dbbb6b..faa6d9a188 100644 --- a/tests/unit/api/objectPutTagging.js +++ b/tests/unit/api/objectPutTagging.js @@ -25,6 +25,7 @@ const testBucketPutRequest = { bucketName, headers: { host: `${bucketName}.s3.amazonaws.com` }, url: '/', + actionImplicitDenies: false, }; const testPutObjectRequest = new DummyRequest({ @@ -172,8 +173,7 @@ describe('PUT object tagging :: helper validation functions ', () => { taggingTests.forEach(taggingTest => { it(taggingTest.it, done => { - const key = taggingTest.tag.key; - const value = taggingTest.tag.value; + const { tag: { key, value } } = taggingTest; const xml = _generateSampleXml(key, value); parseTagXml(xml, log, (err, result) => { if (taggingTest.error) { diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index e81c889dc2..f8ed6bac42 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -340,6 +340,7 @@ class CorsConfigTester { }, url: '/?cors', query: { cors: '' }, + actionImplicitDenies: false, }; if (method === 'PUT') { request.post = body || this.constructXml(); @@ -381,6 +382,7 @@ const versioningTestUtils = { }, url: '/?versioning', query: { versioning: '' }, + actionImplicitDenies: false, }; const xml = '' + @@ -431,6 +433,7 @@ class TaggingConfigTester { objectKey: objectName, url: '/?tagging', query: { tagging: '' }, + actionImplicitDenies: false, }; if (method === 'PUT') { request.post = body || this.constructXml(); diff --git a/tests/unit/metadata/metadataUtils.spec.js b/tests/unit/metadata/metadataUtils.spec.js new file mode 100644 index 0000000000..d896712e0a --- /dev/null +++ b/tests/unit/metadata/metadataUtils.spec.js @@ -0,0 +1,55 @@ +const assert = require('assert'); + +const { models } = require('arsenal'); +const { BucketInfo } = models; +const { DummyRequestLogger, makeAuthInfo } = require('../helpers'); + +const creationDate = new Date().toJSON(); +const authInfo = makeAuthInfo('accessKey'); +const otherAuthInfo = makeAuthInfo('otherAccessKey'); +const ownerCanonicalId = authInfo.getCanonicalID(); + +const bucket = new BucketInfo('niftyBucket', ownerCanonicalId, + authInfo.getAccountDisplayName(), creationDate); +const log = new DummyRequestLogger(); + +const { validateBucket } = require('../../../lib/metadata/metadataUtils'); + +describe('validateBucket', () => { + it('action bucketPutPolicy by bucket owner', () => { + const validationResult = validateBucket(bucket, { + authInfo, + requestType: 'bucketPutPolicy', + request: null, + }, log, false); + assert.ifError(validationResult); + }); + it('action bucketPutPolicy by other than bucket owner', () => { + const validationResult = validateBucket(bucket, { + authInfo: otherAuthInfo, + requestType: 'bucketPutPolicy', + request: null, + }, log, false); + assert(validationResult); + assert(validationResult.is.MethodNotAllowed); + }); + + it('action bucketGet by bucket owner', () => { + const validationResult = validateBucket(bucket, { + authInfo, + requestType: 'bucketGet', + request: null, + }, log, false); + assert.ifError(validationResult); + }); + + it('action bucketGet by other than bucket owner', () => { + const validationResult = validateBucket(bucket, { + authInfo: otherAuthInfo, + requestType: 'bucketGet', + request: null, + }, log, false); + assert(validationResult); + assert(validationResult.is.AccessDenied); + }); +});