From 0e323fbefe60f0f52ee1e894b3a81a72e7b4e203 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Mon, 28 Aug 2023 11:38:50 +0200 Subject: [PATCH 1/3] =?UTF-8?q?CLDSRV-427:=20update=20bucket/object=20perm?= =?UTF-8?q?=20checks=20to=20account=20for=20implicit=20=E2=80=A6=20?= =?UTF-8?q?=E2=80=A6denies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit c01898f1a002508e9c29662a2a5b07a4b0a26dc8) (cherry picked from commit 7aa326cba9f13975f8cdba885a2ea75be5503414) --- .../authorization/permissionChecks.js | 179 +++++++++++++++--- 1 file changed, 157 insertions(+), 22 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 2e3733e108..38e417cbca 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -298,7 +298,26 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l return permission; } -function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request) { +function isBucketAuthorizedNew(bucket, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { + if (!Array.isArray(requestTypes)) { + // eslint-disable-next-line no-param-reassign + requestTypes = [requestTypes]; + } + if (iamAuthzResults === false) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults = {}; + } + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + requestTypes.forEach(requestType => { + if (iamAuthzResults[requestType] === undefined) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults[requestType] = false; + } + }); + const mainApiCall = requestTypes[0]; + const results = {}; + requestTypes.forEach(_requestType => { // Check to see if user is authorized to perform a // particular action on bucket based on ACLs. // TODO: Add IAM checks @@ -310,63 +329,178 @@ function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, req } // if the bucket owner is an account, users should not have default access if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) { - return true; + results[_requestType] = iamAuthzResults[_requestType] === false; + return; } - const aclPermission = checkBucketAcls(bucket, requestType, canonicalID); + const aclPermission = checkBucketAcls(bucket, _requestType, canonicalID, mainApiCall); const bucketPolicy = bucket.getBucketPolicy(); if (!bucketPolicy) { - return aclPermission; + results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + return; } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, + const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, canonicalID, arn, bucket.getOwner(), log, request); if (bucketPolicyPermission === 'explicitDeny') { - return false; - } - return (aclPermission || (bucketPolicyPermission === 'allow')); + results[_requestType] = false; + return; + } + // If the bucket policy returns an allow, we accept the request, as the + // IAM response here is either Allow or implicit deny. + if (bucketPolicyPermission === 'allow') { + results[_requestType] = true; + return; } + results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + }); -function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log, request) { + // final result is true if all the results are true + return Object.keys(results).every(key => results[key] === true); +} + +function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request) { + const iamAuthzResults = {}; + const result = isBucketAuthorizedNew(bucket, [requestType], canonicalID, authInfo, iamAuthzResults, log, request); + + return result; +} + +function isObjAuthorizedNew(bucket, objectMD, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { + if (!Array.isArray(requestTypes)) { + // eslint-disable-next-line no-param-reassign + requestTypes = [requestTypes]; + } + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + if (iamAuthzResults === false) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults = {}; + } + requestTypes.forEach(requestType => { + if (iamAuthzResults[requestType] === undefined) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults[requestType] = false; + } + }); + const results = {}; + const mainApiCall = requestTypes[0]; + requestTypes.forEach(_requestType => { + const parsedMethodName = _requestType.endsWith('Version') ? + _requestType.slice(0, -7) : _requestType; const bucketOwner = bucket.getOwner(); if (!objectMD) { // User is already authorized on the bucket for FULL_CONTROL or WRITE or // bucket has canned ACL public-read-write - if (requestType === 'objectPut' || requestType === 'objectDelete') { - return true; + if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') { + results[_requestType] = iamAuthzResults[_requestType] === false; + return; } // check bucket has read access // 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions - return isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request); + results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, false, log, request); + return; } let requesterIsNotUser = true; let arn = null; + let isUserUnauthenticated = false; if (authInfo) { requesterIsNotUser = !authInfo.isRequesterAnIAMUser(); arn = authInfo.getArn(); + isUserUnauthenticated = arn === undefined; } if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) { - return true; + results[_requestType] = iamAuthzResults[_requestType] === false; + return; } // account is authorized if: // - requesttype is included in bucketOwnerActions and // - account is the bucket owner // - requester is account, not user - if (bucketOwnerActions.includes(requestType) + if (bucketOwnerActions.includes(parsedMethodName) && (bucketOwner === canonicalID) && requesterIsNotUser) { - return true; + results[_requestType] = iamAuthzResults[_requestType] === false; + return; + } + const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName, + canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall); + const bucketPolicy = bucket.getBucketPolicy(); + if (!bucketPolicy) { + results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + return; + } + const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, + canonicalID, arn, bucket.getOwner(), log, request); + if (bucketPolicyPermission === 'explicitDeny') { + results[_requestType] = false; + return; + } + // If the bucket policy returns an allow, we accept the request, as the + // IAM response here is either Allow or implicit deny. + if (bucketPolicyPermission === 'allow') { + results[_requestType] = true; + return; + } + results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + }); + + // final result is true if all the results are true + return Object.keys(results).every(key => results[key] === true); +} + +function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log, request) { + const iamAuthzResults = {}; + const result = isObjAuthorizedNew(bucket, objectMD, + [requestType], canonicalID, authInfo, iamAuthzResults, log, request); + + return result; +} + +function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { + if (!Array.isArray(requestTypes)) { + // eslint-disable-next-line no-param-reassign + requestTypes = [requestTypes]; + } + if (iamAuthzResults === false) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults = {}; + } + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + requestTypes.forEach(requestType => { + if (iamAuthzResults[requestType] === undefined) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults[requestType] = false; + } + }); + + const results = {}; + requestTypes.forEach(_requestType => { + let arn = null; + if (authInfo) { + arn = authInfo.getArn(); } - const aclPermission = checkObjectAcls(bucket, objectMD, requestType, - canonicalID); const bucketPolicy = bucket.getBucketPolicy(); if (!bucketPolicy) { - return aclPermission; + results[_requestType] = iamAuthzResults[_requestType] === false; + return; } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, + const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, canonicalID, arn, bucket.getOwner(), log, request); if (bucketPolicyPermission === 'explicitDeny') { - return false; - } - return (aclPermission || (bucketPolicyPermission === 'allow')); + results[_requestType] = false; + return; + } + // If the bucket policy returns an allow, we accept the request, as the + // IAM response here is either Allow or implicit deny. + if (bucketPolicyPermission === 'allow') { + results[_requestType] = true; + return; + } + results[_requestType] = iamAuthzResults[_requestType] === false; + }); + + // final result is true if all the results are true + return Object.keys(results).every(key => results[key] === true); } function _checkResource(resource, bucketArn) { @@ -425,4 +559,5 @@ module.exports = { checkObjectAcls, validatePolicyResource, isLifecycleSession, + evaluateBucketPolicyWithIAM, }; From fa2f877825c5af927c138333d4d87198ce32267b Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 3 Nov 2023 10:15:23 +0100 Subject: [PATCH 2/3] CLDSRV-427: linting fixups and retrocompatibility changes (cherry picked from commit a7396a721c13f911f5e92625df75f2295d306bf9) --- .../authorization/permissionChecks.js | 159 ++++++++---------- 1 file changed, 72 insertions(+), 87 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 38e417cbca..019732e02e 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -298,87 +298,79 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l return permission; } -function isBucketAuthorizedNew(bucket, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { +function isBucketAuthorized(bucket, requestTypes, canonicalID, authInfo, log, request, actionImplicitDenies) { if (!Array.isArray(requestTypes)) { // eslint-disable-next-line no-param-reassign requestTypes = [requestTypes]; } - if (iamAuthzResults === false) { + if (!actionImplicitDenies) { // eslint-disable-next-line no-param-reassign - iamAuthzResults = {}; + actionImplicitDenies = {}; } // By default, all missing actions are defined as allowed from IAM, to be // backward compatible requestTypes.forEach(requestType => { - if (iamAuthzResults[requestType] === undefined) { + if (actionImplicitDenies[requestType] === undefined) { // eslint-disable-next-line no-param-reassign - iamAuthzResults[requestType] = false; + actionImplicitDenies[requestType] = false; } }); const mainApiCall = requestTypes[0]; const results = {}; requestTypes.forEach(_requestType => { - // Check to see if user is authorized to perform a - // particular action on bucket based on ACLs. - // TODO: Add IAM checks - let requesterIsNotUser = true; - let arn = null; - if (authInfo) { - requesterIsNotUser = !authInfo.isRequesterAnIAMUser(); - arn = authInfo.getArn(); - } - // if the bucket owner is an account, users should not have default access - if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) { - results[_requestType] = iamAuthzResults[_requestType] === false; + // Check to see if user is authorized to perform a + // particular action on bucket based on ACLs. + // TODO: Add IAM checks + let requesterIsNotUser = true; + let arn = null; + if (authInfo) { + requesterIsNotUser = !authInfo.isRequesterAnIAMUser(); + arn = authInfo.getArn(); + } + // if the bucket owner is an account, users should not have default access + if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) { + results[_requestType] = actionImplicitDenies[_requestType] === false; return; - } + } const aclPermission = checkBucketAcls(bucket, _requestType, canonicalID, mainApiCall); - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + const bucketPolicy = bucket.getBucketPolicy(); + if (!bucketPolicy) { + results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; return; - } + } const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, - canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { + canonicalID, arn, bucket.getOwner(), log, request); + if (bucketPolicyPermission === 'explicitDeny') { results[_requestType] = false; return; - } + } // If the bucket policy returns an allow, we accept the request, as the // IAM response here is either Allow or implicit deny. if (bucketPolicyPermission === 'allow') { results[_requestType] = true; return; -} - results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + } + results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; }); // final result is true if all the results are true return Object.keys(results).every(key => results[key] === true); } - -function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request) { - const iamAuthzResults = {}; - const result = isBucketAuthorizedNew(bucket, [requestType], canonicalID, authInfo, iamAuthzResults, log, request); - - return result; -} - -function isObjAuthorizedNew(bucket, objectMD, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { +function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, log, request, actionImplicitDenies) { if (!Array.isArray(requestTypes)) { // eslint-disable-next-line no-param-reassign requestTypes = [requestTypes]; } // By default, all missing actions are defined as allowed from IAM, to be // backward compatible - if (iamAuthzResults === false) { + if (!actionImplicitDenies) { // eslint-disable-next-line no-param-reassign - iamAuthzResults = {}; + actionImplicitDenies = {}; } requestTypes.forEach(requestType => { - if (iamAuthzResults[requestType] === undefined) { + if (actionImplicitDenies[requestType] === undefined) { // eslint-disable-next-line no-param-reassign - iamAuthzResults[requestType] = false; + actionImplicitDenies[requestType] = false; } }); const results = {}; @@ -386,46 +378,46 @@ function isObjAuthorizedNew(bucket, objectMD, requestTypes, canonicalID, authInf requestTypes.forEach(_requestType => { const parsedMethodName = _requestType.endsWith('Version') ? _requestType.slice(0, -7) : _requestType; - const bucketOwner = bucket.getOwner(); - if (!objectMD) { - // User is already authorized on the bucket for FULL_CONTROL or WRITE or - // bucket has canned ACL public-read-write + const bucketOwner = bucket.getOwner(); + if (!objectMD) { + // User is already authorized on the bucket for FULL_CONTROL or WRITE or + // bucket has canned ACL public-read-write if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') { - results[_requestType] = iamAuthzResults[_requestType] === false; + results[_requestType] = actionImplicitDenies[_requestType] === false; return; - } - // check bucket has read access - // 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions - results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, false, log, request); + } + // check bucket has read access + // 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions + results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request, false); return; - } - let requesterIsNotUser = true; - let arn = null; + } + let requesterIsNotUser = true; + let arn = null; let isUserUnauthenticated = false; - if (authInfo) { - requesterIsNotUser = !authInfo.isRequesterAnIAMUser(); - arn = authInfo.getArn(); + if (authInfo) { + requesterIsNotUser = !authInfo.isRequesterAnIAMUser(); + arn = authInfo.getArn(); isUserUnauthenticated = arn === undefined; - } - if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) { - results[_requestType] = iamAuthzResults[_requestType] === false; + } + if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) { + results[_requestType] = actionImplicitDenies[_requestType] === false; return; - } - // account is authorized if: - // - requesttype is included in bucketOwnerActions and - // - account is the bucket owner - // - requester is account, not user + } + // account is authorized if: + // - requesttype is included in bucketOwnerActions and + // - account is the bucket owner + // - requester is account, not user if (bucketOwnerActions.includes(parsedMethodName) && (bucketOwner === canonicalID) && requesterIsNotUser) { - results[_requestType] = iamAuthzResults[_requestType] === false; + results[_requestType] = actionImplicitDenies[_requestType] === false; return; } const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName, canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall); const bucketPolicy = bucket.getBucketPolicy(); if (!bucketPolicy) { - results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; return; } const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, @@ -440,36 +432,28 @@ function isObjAuthorizedNew(bucket, objectMD, requestTypes, canonicalID, authInf results[_requestType] = true; return; } - results[_requestType] = iamAuthzResults[_requestType] === false && aclPermission; + results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; }); // final result is true if all the results are true return Object.keys(results).every(key => results[key] === true); } -function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log, request) { - const iamAuthzResults = {}; - const result = isObjAuthorizedNew(bucket, objectMD, - [requestType], canonicalID, authInfo, iamAuthzResults, log, request); - - return result; -} - -function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, iamAuthzResults, log, request) { +function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, actionImplicitDenies, log, request) { if (!Array.isArray(requestTypes)) { // eslint-disable-next-line no-param-reassign requestTypes = [requestTypes]; } - if (iamAuthzResults === false) { + if (actionImplicitDenies === false) { // eslint-disable-next-line no-param-reassign - iamAuthzResults = {}; + actionImplicitDenies = {}; } // By default, all missing actions are defined as allowed from IAM, to be // backward compatible requestTypes.forEach(requestType => { - if (iamAuthzResults[requestType] === undefined) { + if (actionImplicitDenies[requestType] === undefined) { // eslint-disable-next-line no-param-reassign - iamAuthzResults[requestType] = false; + actionImplicitDenies[requestType] = false; } }); @@ -478,25 +462,25 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo let arn = null; if (authInfo) { arn = authInfo.getArn(); - } - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = iamAuthzResults[_requestType] === false; + } + const bucketPolicy = bucket.getBucketPolicy(); + if (!bucketPolicy) { + results[_requestType] = actionImplicitDenies[_requestType] === false; return; - } + } const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { + if (bucketPolicyPermission === 'explicitDeny') { results[_requestType] = false; return; - } + } // If the bucket policy returns an allow, we accept the request, as the // IAM response here is either Allow or implicit deny. if (bucketPolicyPermission === 'allow') { results[_requestType] = true; return; } - results[_requestType] = iamAuthzResults[_requestType] === false; + results[_requestType] = actionImplicitDenies[_requestType] === false; }); // final result is true if all the results are true @@ -561,3 +545,4 @@ module.exports = { isLifecycleSession, evaluateBucketPolicyWithIAM, }; + From 6caa5cc26aafe5a2abe235e89988cba0172fd981 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 10 Nov 2023 11:17:35 +0100 Subject: [PATCH 3/3] CLDSRV-427: Improving functions using helper function - In this commit , I added a helper (processBucketPolicy) function for the bycket policies checks that are shared between the isbucketAuthorized, isObjAuthorized and evaluateBucketPolicyWithIAM for a better code readability and to avoid long functions. (cherry picked from commit 33d7c99e0c4df43b97e5416cfc5dcefd4b00629e) --- .../authorization/permissionChecks.js | 228 +++++++----------- 1 file changed, 82 insertions(+), 146 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 019732e02e..76ad9f0e70 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -1,12 +1,14 @@ const { evaluators, actionMaps, RequestContext } = require('arsenal').policies; const constants = require('../../../../constants'); -const { allAuthedUsersId, bucketOwnerActions, logId, publicId, - assumedRoleArnResourceType, backbeatLifecycleSessionName, arrayOfAllowed } = constants; +const { + allAuthedUsersId, bucketOwnerActions, logId, publicId, + assumedRoleArnResourceType, backbeatLifecycleSessionName, arrayOfAllowed, +} = constants; // whitelist buckets to allow public read on objects -const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ? - process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : []; +const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS + ? process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : []; /** * Checks the access control for a given bucket based on the request type and user's canonical ID. @@ -106,8 +108,8 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) { // objectPutACL, objectGetACL, objectHead or objectGet, the bucket // authorization check should just return true so can move on to check // rights at the object level. - return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL' || - requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead'); + return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL' + || requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead'); } function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser, @@ -199,9 +201,9 @@ function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIs // allow public reads on buckets that are whitelisted for anonymous reads // TODO: remove this after bucket policies are implemented const bucketAcl = bucket.getAcl(); - const allowPublicReads = publicReadBuckets.includes(bucket.getName()) && - bucketAcl.Canned === 'public-read' && - (requestType === 'objectGet' || requestType === 'objectHead'); + const allowPublicReads = publicReadBuckets.includes(bucket.getName()) + && bucketAcl.Canned === 'public-read' + && (requestType === 'objectGet' || requestType === 'objectHead'); if (allowPublicReads) { return true; } @@ -298,26 +300,37 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l return permission; } -function isBucketAuthorized(bucket, requestTypes, canonicalID, authInfo, log, request, actionImplicitDenies) { - if (!Array.isArray(requestTypes)) { - // eslint-disable-next-line no-param-reassign - requestTypes = [requestTypes]; - } - if (!actionImplicitDenies) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies = {}; - } - // By default, all missing actions are defined as allowed from IAM, to be - // backward compatible - requestTypes.forEach(requestType => { - if (actionImplicitDenies[requestType] === undefined) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies[requestType] = false; +function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner, log, + request, aclPermission, results, actionImplicitDenies) { + const bucketPolicy = bucket.getBucketPolicy(); + let processedResult = results[requestType]; + if (!bucketPolicy) { + processedResult = actionImplicitDenies[requestType] === false && aclPermission; + } else { + const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn, + bucketOwner, log, request); + + if (bucketPolicyPermission === 'explicitDeny') { + processedResult = false; + } else if (bucketPolicyPermission === 'allow') { + processedResult = true; + } else { + processedResult = actionImplicitDenies[requestType] === false && aclPermission; } - }); + } + return processedResult; +} + +function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, log, request, + actionImplicitDeniesInput = {}) { + const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput]; + const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput; const mainApiCall = requestTypes[0]; const results = {}; - requestTypes.forEach(_requestType => { + return requestTypes.every(_requestType => { + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false; // Check to see if user is authorized to perform a // particular action on bucket based on ACLs. // TODO: Add IAM checks @@ -330,66 +343,57 @@ function isBucketAuthorized(bucket, requestTypes, canonicalID, authInfo, log, re // if the bucket owner is an account, users should not have default access if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) { results[_requestType] = actionImplicitDenies[_requestType] === false; - return; + return results[_requestType]; } const aclPermission = checkBucketAcls(bucket, _requestType, canonicalID, mainApiCall); - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; - return; - } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, - canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { - results[_requestType] = false; - return; - } - // If the bucket policy returns an allow, we accept the request, as the - // IAM response here is either Allow or implicit deny. - if (bucketPolicyPermission === 'allow') { - results[_requestType] = true; - return; - } - results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; + return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log, + request, aclPermission, results, actionImplicitDenies); }); - - // final result is true if all the results are true - return Object.keys(results).every(key => results[key] === true); } -function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, log, request, actionImplicitDenies) { - if (!Array.isArray(requestTypes)) { - // eslint-disable-next-line no-param-reassign - requestTypes = [requestTypes]; - } - // By default, all missing actions are defined as allowed from IAM, to be - // backward compatible - if (!actionImplicitDenies) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies = {}; - } - requestTypes.forEach(requestType => { - if (actionImplicitDenies[requestType] === undefined) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies[requestType] = false; + +function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, authInfo, actionImplicitDeniesInput = {}, + log, request) { + const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput]; + const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput; + const results = {}; + return requestTypes.every(_requestType => { + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false; + let arn = null; + if (authInfo) { + arn = authInfo.getArn(); } + return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log, + request, true, results, actionImplicitDenies); }); +} + +function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authInfo, log, request, + actionImplicitDeniesInput = {}) { + const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput]; + const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput; const results = {}; const mainApiCall = requestTypes[0]; - requestTypes.forEach(_requestType => { - const parsedMethodName = _requestType.endsWith('Version') ? - _requestType.slice(0, -7) : _requestType; + return requestTypes.every(_requestType => { + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false; + const parsedMethodName = _requestType.endsWith('Version') + ? _requestType.slice(0, -7) : _requestType; const bucketOwner = bucket.getOwner(); if (!objectMD) { - // User is already authorized on the bucket for FULL_CONTROL or WRITE or - // bucket has canned ACL public-read-write + // User is already authorized on the bucket for FULL_CONTROL or WRITE or + // bucket has canned ACL public-read-write if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') { results[_requestType] = actionImplicitDenies[_requestType] === false; - return; + return results[_requestType]; } // check bucket has read access // 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions - results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request, false); - return; + results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request, + actionImplicitDenies); + return results[_requestType]; } let requesterIsNotUser = true; let arn = null; @@ -401,7 +405,7 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, } if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) { results[_requestType] = actionImplicitDenies[_requestType] === false; - return; + return results[_requestType]; } // account is authorized if: // - requesttype is included in bucketOwnerActions and @@ -411,80 +415,13 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, && (bucketOwner === canonicalID) && requesterIsNotUser) { results[_requestType] = actionImplicitDenies[_requestType] === false; - return; + return results[_requestType]; } const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName, canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall); - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; - return; - } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, - canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { - results[_requestType] = false; - return; - } - // If the bucket policy returns an allow, we accept the request, as the - // IAM response here is either Allow or implicit deny. - if (bucketPolicyPermission === 'allow') { - results[_requestType] = true; - return; - } - results[_requestType] = actionImplicitDenies[_requestType] === false && aclPermission; + return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner, + log, request, aclPermission, results, actionImplicitDenies); }); - - // final result is true if all the results are true - return Object.keys(results).every(key => results[key] === true); -} - -function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, actionImplicitDenies, log, request) { - if (!Array.isArray(requestTypes)) { - // eslint-disable-next-line no-param-reassign - requestTypes = [requestTypes]; - } - if (actionImplicitDenies === false) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies = {}; - } - // By default, all missing actions are defined as allowed from IAM, to be - // backward compatible - requestTypes.forEach(requestType => { - if (actionImplicitDenies[requestType] === undefined) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies[requestType] = false; - } - }); - - const results = {}; - requestTypes.forEach(_requestType => { - let arn = null; - if (authInfo) { - arn = authInfo.getArn(); - } - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = actionImplicitDenies[_requestType] === false; - return; - } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, - canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { - results[_requestType] = false; - return; - } - // If the bucket policy returns an allow, we accept the request, as the - // IAM response here is either Allow or implicit deny. - if (bucketPolicyPermission === 'allow') { - results[_requestType] = true; - return; - } - results[_requestType] = actionImplicitDenies[_requestType] === false; - }); - - // final result is true if all the results are true - return Object.keys(results).every(key => results[key] === true); } function _checkResource(resource, bucketArn) { @@ -531,9 +468,9 @@ function isLifecycleSession(arn) { const resourceType = resourceNames[0]; const sessionName = resourceNames[resourceNames.length - 1]; - return (service === 'sts' && - resourceType === assumedRoleArnResourceType && - sessionName === backbeatLifecycleSessionName); + return (service === 'sts' + && resourceType === assumedRoleArnResourceType + && sessionName === backbeatLifecycleSessionName); } module.exports = { @@ -545,4 +482,3 @@ module.exports = { isLifecycleSession, evaluateBucketPolicyWithIAM, }; -