Skip to content

Commit ecb74a2

Browse files
benzekrimahabenzekrimaha
committed
Merge remote-tracking branch 'origin/improvement/CLDSRV-431-misc-api-implicitDeny' into w/7.70/improvement/CLDSRV-431-misc-api-implicitDeny
2 parents 52994c0 + cdcdf8e commit ecb74a2

31 files changed

+162
-38
lines changed

‎lib/api/apiUtils/authorization/permissionChecks.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,11 @@ function _checkPrincipals(canonicalID, arn, principal) {
274274
return false;
275275
}
276276

277-
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request) {
277+
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
278278
let permission = 'defaultDeny';
279279
// if requester is user within bucket owner account, actions should be
280280
// allowed unless explicitly denied (assumes allowed by IAM policy)
281-
if (bucketOwner === canonicalID) {
281+
if (bucketOwner === canonicalID && actionImplicitDenies[requestType] === false) {
282282
permission = 'allow';
283283
}
284284
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
@@ -308,7 +308,7 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
308308
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
309309
} else {
310310
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
311-
bucketOwner, log, request);
311+
bucketOwner, log, request, actionImplicitDenies);
312312

313313
if (bucketPolicyPermission === 'explicitDeny') {
314314
processedResult = false;

‎lib/api/apiUtils/bucket/bucketDeletion.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function _deleteMPUbucket(destinationBucketName, log, cb) {
2424
});
2525
}
2626

27-
function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
27+
function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, request, log, cb) {
2828
async.mapLimit(mpus, 1, (mpu, next) => {
2929
const splitterChar = mpu.key.includes(oldSplitter) ?
3030
oldSplitter : splitter;
@@ -40,7 +40,7 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
4040
byteLength: partSizeSum,
4141
});
4242
next(err);
43-
});
43+
}, request);
4444
}, cb);
4545
}
4646
/**
@@ -49,11 +49,13 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
4949
* @param {object} bucketMD - bucket attributes/metadata
5050
* @param {string} bucketName - bucket in which objectMetadata is stored
5151
* @param {string} canonicalID - account canonicalID of requester
52+
* @param {object} request - request object given by router
53+
* including normalized headers
5254
* @param {object} log - Werelogs logger
5355
* @param {function} cb - callback from async.waterfall in bucketDelete
5456
* @return {undefined}
5557
*/
56-
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
58+
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log, cb) {
5759
log.trace('deleting bucket from metadata');
5860
assert.strictEqual(typeof bucketName, 'string');
5961
assert.strictEqual(typeof canonicalID, 'string');
@@ -100,7 +102,7 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
100102
}
101103
if (objectsListRes.Contents.length) {
102104
return _deleteOngoingMPUs(authInfo, bucketName,
103-
bucketMD, objectsListRes.Contents, log, err => {
105+
bucketMD, objectsListRes.Contents, request, log, err => {
104106
if (err) {
105107
return next(err);
106108
}

‎lib/api/apiUtils/object/abortMultipartUpload.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const async = require('async');
33
const constants = require('../../../../constants');
44
const { data } = require('../../../data/wrapper');
55
const locationConstraintCheck = require('../object/locationConstraintCheck');
6-
const { metadataValidateBucketAndObj } =
6+
const { standardMetadataValidateBucketAndObj } =
77
require('../../../metadata/metadataUtils');
88
const services = require('../../../services');
99

@@ -22,10 +22,11 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
2222
// but the requestType is the more general 'objectDelete'
2323
const metadataValParams = Object.assign({}, metadataValMPUparams);
2424
metadataValParams.requestType = 'objectPut';
25+
const authzIdentityResult = request ? request.actionImplicitDenies : false;
2526

2627
async.waterfall([
2728
function checkDestBucketVal(next) {
28-
metadataValidateBucketAndObj(metadataValParams, log,
29+
standardMetadataValidateBucketAndObj(metadataValParams, authzIdentityResult, log,
2930
(err, destinationBucket) => {
3031
if (err) {
3132
return next(err, destinationBucket);
@@ -56,9 +57,14 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
5657
function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket,
5758
next) {
5859
const location = mpuOverviewObj.controllingLocationConstraint;
60+
const originalIdentityAuthzResults = request.actionImplicitDenies;
61+
// eslint-disable-next-line no-param-reassign
62+
delete request.actionImplicitDenies;
5963
return data.abortMPU(objectKey, uploadId, location, bucketName,
6064
request, destBucket, locationConstraintCheck, log,
6165
(err, skipDataDelete) => {
66+
// eslint-disable-next-line no-param-reassign
67+
request.actionImplicitDenies = originalIdentityAuthzResults;
6268
if (err) {
6369
return next(err, destBucket);
6470
}

‎lib/api/apiUtils/object/objectLockHelpers.js‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const moment = require('moment');
33

44
const { config } = require('../../../Config');
55
const vault = require('../../../auth/vault');
6+
const { evaluateBucketPolicyWithIAM } = require('../authorization/permissionChecks');
67

78
/**
89
* Calculates retain until date for the locked object version
@@ -302,15 +303,35 @@ function checkUserGovernanceBypass(request, authInfo, bucketMD, objectKey, log,
302303
if (err) {
303304
return cb(err);
304305
}
305-
if (authorizationResults[0].isAllowed !== true) {
306+
const explicitDenyExists = authorizationResults.some(
307+
authzResult => authzResult.isAllowed === false && authzResult.isImplicit === false);
308+
if (explicitDenyExists) {
306309
log.trace('authorization check failed for user',
307310
{
308311
'method': 'checkUserPolicyGovernanceBypass',
309312
's3:BypassGovernanceRetention': false,
310313
});
311314
return cb(errors.AccessDenied);
312315
}
313-
return cb(null);
316+
// Convert authorization results into an easier to handle format
317+
const actionImplicitDenies = authorizationResults.reduce((acc, curr, idx) => {
318+
const apiMethod = authorizationResults[idx].action;
319+
// eslint-disable-next-line no-param-reassign
320+
acc[apiMethod] = curr.isImplicit;
321+
return acc;
322+
}, {});
323+
324+
// Evaluate against the bucket policies
325+
const areAllActionsAllowed = evaluateBucketPolicyWithIAM(
326+
bucketMD,
327+
Object.keys(actionImplicitDenies),
328+
authInfo.getCanonicalID(),
329+
authInfo,
330+
actionImplicitDenies,
331+
log,
332+
request);
333+
334+
return cb(areAllActionsAllowed === true ? null : errors.AccessDenied);
314335
});
315336
}
316337

‎lib/api/bucketDelete.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ function bucketDelete(authInfo, request, log, cb) {
4848
log.trace('passed checks',
4949
{ method: 'metadataValidateBucket' });
5050
return deleteBucket(authInfo, bucketMD, bucketName,
51-
authInfo.getCanonicalID(), log, err => {
51+
authInfo.getCanonicalID(), request, log, err => {
5252
if (err) {
5353
monitoring.promMetrics(
5454
'DELETE', bucketName, err.code, 'deleteBucket');

‎lib/api/bucketHead.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
2-
const { metadataValidateBucket } = require('../metadata/metadataUtils');
2+
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
33

44
const { pushMetric } = require('../utapi/utilities');
55
const monitoring = require('../utilities/metrics');
@@ -22,7 +22,7 @@ function bucketHead(authInfo, request, log, callback) {
2222
requestType: 'bucketHead',
2323
request,
2424
};
25-
metadataValidateBucket(metadataValParams, log, (err, bucket) => {
25+
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
2626
const corsHeaders = collectCorsHeaders(request.headers.origin,
2727
request.method, bucket);
2828
if (err) {

‎lib/api/completeMultipartUpload.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const constants = require('../../constants');
1515
const { versioningPreprocessing, checkQueryVersionId }
1616
= require('./apiUtils/object/versioning');
1717
const services = require('../services');
18-
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
18+
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
1919
const { skipMpuPartProcessing } = require('arsenal').storage.data.external.backendUtils;
2020
const locationConstraintCheck
2121
= require('./apiUtils/object/locationConstraintCheck');
@@ -120,7 +120,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
120120
// at the destinationBucket level are same as objectPut
121121
requestType: 'objectPut',
122122
};
123-
metadataValidateBucketAndObj(metadataValParams, log, next);
123+
standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, next);
124124
},
125125
function validateMultipart(destBucket, objMD, next) {
126126
if (objMD) {
@@ -190,9 +190,14 @@ function completeMultipartUpload(authInfo, request, log, callback) {
190190
const mdInfo = { storedParts, mpuOverviewKey, splitter };
191191
const mpuInfo =
192192
{ objectKey, uploadId, jsonList, bucketName, destBucket };
193+
const originalIdentityImpDenies = request.actionImplicitDenies;
194+
// eslint-disable-next-line no-param-reassign
195+
delete request.actionImplicitDenies;
193196
return data.completeMPU(request, mpuInfo, mdInfo, location,
194197
null, null, null, locationConstraintCheck, log,
195198
(err, completeObjData) => {
199+
// eslint-disable-next-line no-param-reassign
200+
request.actionImplicitDenies = originalIdentityImpDenies;
196201
if (err) {
197202
return next(err, destBucket);
198203
}

‎lib/api/initiateMultipartUpload.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const { hasNonPrintables } = require('../utilities/stringChecks');
99
const { cleanUpBucket } = require('./apiUtils/bucket/bucketCreation');
1010
const constants = require('../../constants');
1111
const services = require('../services');
12-
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
12+
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
1313
const locationConstraintCheck
1414
= require('./apiUtils/object/locationConstraintCheck');
1515
const validateWebsiteHeader = require('./apiUtils/object/websiteServing')
@@ -262,7 +262,7 @@ function initiateMultipartUpload(authInfo, request, log, callback) {
262262
}
263263

264264
async.waterfall([
265-
next => metadataValidateBucketAndObj(metadataValParams, log,
265+
next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
266266
(error, destinationBucket) => {
267267
const corsHeaders = collectCorsHeaders(request.headers.origin,
268268
request.method, destinationBucket);

‎lib/api/listMultipartUploads.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const convertToXml = s3middleware.convertToXml;
66
const constants = require('../../constants');
77
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
88
const services = require('../services');
9-
const { metadataValidateBucket } = require('../metadata/metadataUtils');
9+
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
1010
const { pushMetric } = require('../utapi/utilities');
1111
const monitoring = require('../utilities/metrics');
1212

@@ -105,7 +105,7 @@ function listMultipartUploads(authInfo, request, log, callback) {
105105
function waterfall1(next) {
106106
// Check final destination bucket for authorization rather
107107
// than multipart upload bucket
108-
metadataValidateBucket(metadataValParams, log,
108+
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
109109
(err, bucket) => next(err, bucket));
110110
},
111111
function getMPUBucket(bucket, next) {

‎lib/api/listParts.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
99
const locationConstraintCheck =
1010
require('./apiUtils/object/locationConstraintCheck');
1111
const services = require('../services');
12-
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
12+
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
1313
const escapeForXml = s3middleware.escapeForXml;
1414
const { pushMetric } = require('../utapi/utilities');
1515
const monitoring = require('../utilities/metrics');
@@ -115,7 +115,7 @@ function listParts(authInfo, request, log, callback) {
115115

116116
async.waterfall([
117117
function checkDestBucketVal(next) {
118-
metadataValidateBucketAndObj(metadataValParams, log,
118+
standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
119119
(err, destinationBucket) => {
120120
if (err) {
121121
return next(err, destinationBucket, null);
@@ -153,8 +153,13 @@ function listParts(authInfo, request, log, callback) {
153153
mpuOverviewObj,
154154
destBucket,
155155
};
156+
const originalIdentityImpDenies = request.actionImplicitDenies;
157+
// eslint-disable-next-line no-param-reassign
158+
delete request.actionImplicitDenies;
156159
return data.listParts(mpuInfo, request, locationConstraintCheck,
157160
log, (err, backendPartList) => {
161+
// eslint-disable-next-line no-param-reassign
162+
request.actionImplicitDenies = originalIdentityImpDenies;
158163
if (err) {
159164
return next(err, destBucket);
160165
}

0 commit comments

Comments
 (0)