Skip to content

Improvement/cldsrv 430 del api imp deny #5335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: improvement/CLDSRV-428-put-api-impDeny
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions lib/api/apiUtils/bucket/bucketDeletion.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function _deleteMPUbucket(destinationBucketName, log, cb) {
});
}

function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, request, log, cb) {
async.mapLimit(mpus, 1, (mpu, next) => {
const splitterChar = mpu.key.includes(oldSplitter) ?
oldSplitter : splitter;
Expand All @@ -40,7 +40,7 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
byteLength: partSizeSum,
});
next(err);
});
}, request);
}, cb);
}
/**
Expand All @@ -49,11 +49,13 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
* @param {object} bucketMD - bucket attributes/metadata
* @param {string} bucketName - bucket in which objectMetadata is stored
* @param {string} canonicalID - account canonicalID of requester
* @param {object} request - request object given by router
* including normalized headers
* @param {object} log - Werelogs logger
* @param {function} cb - callback from async.waterfall in bucketDelete
* @return {undefined}
*/
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log, cb) {
log.trace('deleting bucket from metadata');
assert.strictEqual(typeof bucketName, 'string');
assert.strictEqual(typeof canonicalID, 'string');
Expand Down Expand Up @@ -100,7 +102,7 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
}
if (objectsListRes.Contents.length) {
return _deleteOngoingMPUs(authInfo, bucketName,
bucketMD, objectsListRes.Contents, log, err => {
bucketMD, objectsListRes.Contents, request, log, err => {
if (err) {
return next(err);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/api/bucketDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function bucketDelete(authInfo, request, log, cb) {
request,
};

return metadataValidateBucket(metadataValParams, log,
return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucketMD) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucketMD);
Expand All @@ -43,7 +43,7 @@ function bucketDelete(authInfo, request, log, cb) {
log.trace('passed checks',
{ method: 'metadataValidateBucket' });
return deleteBucket(authInfo, bucketMD, bucketName,
authInfo.getCanonicalID(), log, err => {
authInfo.getCanonicalID(), request, log, err => {
if (err) {
return cb(err, corsHeaders);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/api/bucketDeleteCors.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ function bucketDeleteCors(authInfo, request, log, callback) {
}
log.trace('found bucket in metadata');

if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo,
request.actionImplicitDenies, log, request)) {
log.debug('access denied for user on bucket', {
requestType,
method: 'bucketDeleteCors',
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketDeleteEncryption.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function bucketDeleteEncryption(authInfo, request, log, callback) {
};

return async.waterfall([
next => metadataValidateBucket(metadataValParams, log, next),
next => metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, next),
(bucket, next) => checkExpectedBucketOwner(request.headers, bucket, log, err => next(err, bucket)),
(bucket, next) => {
const sseConfig = bucket.getServerSideEncryption();
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketDeleteLifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function bucketDeleteLifecycle(authInfo, request, log, callback) {
requestType: 'bucketDeleteLifecycle',
request,
};
return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);
if (err) {
log.debug('error processing request', {
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketDeletePolicy.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function bucketDeletePolicy(authInfo, request, log, callback) {
requestType: 'bucketDeletePolicy',
request,
};
return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);
if (err) {
log.debug('error processing request', {
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketDeleteReplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function bucketDeleteReplication(authInfo, request, log, callback) {
requestType: 'bucketDeleteReplication',
request,
};
return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);
if (err) {
log.debug('error processing request', {
Expand Down
3 changes: 2 additions & 1 deletion lib/api/bucketDeleteWebsite.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ function bucketDeleteWebsite(authInfo, request, log, callback) {
}
log.trace('found bucket in metadata');

if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo,
request.actionImplicitDenies, log, request)) {
log.debug('access denied for user on bucket', {
requestType,
method: 'bucketDeleteWebsite',
Expand Down
3 changes: 2 additions & 1 deletion lib/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,8 @@ function multiObjectDelete(authInfo, request, log, callback) {
return next(null, quietSetting, errorResults, inPlay,
bucketMD);
}
if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucketMD, 'objectDelete', canonicalID, authInfo,
request.actionImplicitDenies, log, request)) {
log.trace("access denied due to bucket acl's");
// if access denied at the bucket level, no access for
// any of the objects so all results will be error results
Expand Down
4 changes: 2 additions & 2 deletions lib/api/objectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ function objectDelete(authInfo, request, log, cb) {
const canonicalID = authInfo.getCanonicalID();
return async.waterfall([
function validateBucketAndObj(next) {
return metadataValidateBucketAndObj(valParams, log,
(err, bucketMD, objMD) => {
return metadataValidateBucketAndObj(valParams, request.actionImplicitDenies, log,
(err, bucketMD, objMD) => {
if (err) {
return next(err, bucketMD);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectDeleteTagging.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function objectDeleteTagging(authInfo, request, log, callback) {
};

return async.waterfall([
next => metadataValidateBucketAndObj(metadataValParams, log,
next => metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
(err, bucket, objectMD) => {
if (err) {
log.trace('request authorization failed',
Expand Down
14 changes: 9 additions & 5 deletions tests/unit/api/bucketDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ function createMPU(testRequest, initiateRequest, deleteOverviewMPUObj, cb) {
});
});
}
// TODO CLDSRV-430 remove skip
describe.skip('bucketDelete API', () => {
describe('bucketDelete API', () => {
beforeEach(() => {
cleanup();
});
Expand All @@ -88,6 +87,7 @@ describe.skip('bucketDelete API', () => {
namespace,
headers: {},
url: `/${bucketName}`,
actionImplicitDenies: false,
};

const initiateRequest = {
Expand All @@ -96,6 +96,7 @@ describe.skip('bucketDelete API', () => {
objectKey: objectName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: `/${objectName}?uploads`,
actionImplicitDenies: false,
};

it('should return an error if the bucket is not empty', done => {
Expand Down Expand Up @@ -128,7 +129,8 @@ describe.skip('bucketDelete API', () => {
});
});

it('should not return an error if the bucket has an initiated mpu',
// TODO CLDSRV-431 remove skip
it.skip('should not return an error if the bucket has an initiated mpu',
done => {
bucketPut(authInfo, testRequest, log, err => {
assert.strictEqual(err, null);
Expand Down Expand Up @@ -158,11 +160,13 @@ describe.skip('bucketDelete API', () => {
});
});

it('should delete a bucket even if the bucket has ongoing mpu',
// TODO CLDSRV-431 remove skip
it.skip('should delete a bucket even if the bucket has ongoing mpu',
done => createMPU(testRequest, initiateRequest, false, done));

// TODO CLDSRV-431 remove skip
// if only part object (and no overview objects) is in mpu shadow bucket
it('should delete a bucket even if the bucket has an orphan part',
it.skip('should delete a bucket even if the bucket has an orphan part',
done => createMPU(testRequest, initiateRequest, true, done));


Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/bucketDeleteCors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ const testBucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
const testBucketPutCorsRequest =
corsUtil.createBucketCorsRequest('PUT', bucketName);
const testBucketDeleteCorsRequest =
corsUtil.createBucketCorsRequest('DELETE', bucketName);
// TODO CLDSRV-430 remove skip
describe.skip('deleteBucketCors API', () => {
describe('deleteBucketCors API', () => {
beforeEach(done => {
cleanup();
bucketPut(authInfo, testBucketPutRequest, log, () => {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/bucketDeleteEncryption.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const bucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
// TODO CLDSRV-430 remove skip
describe.skip('bucketDeleteEncryption API', () => {
describe('bucketDeleteEncryption API', () => {
before(() => cleanup());

beforeEach(done => bucketPut(authInfo, bucketPutRequest, log, done));
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/bucketDeleteLifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ function _makeRequest(includeXml) {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
if (includeXml) {
request.post = '<LifecycleConfiguration ' +
Expand All @@ -30,8 +31,7 @@ function _makeRequest(includeXml) {
}
return request;
}
// TODO CLDSRV-430 remove skip
describe.skip('deleteBucketLifecycle API', () => {
describe('deleteBucketLifecycle API', () => {
before(() => cleanup());
beforeEach(done => bucketPut(authInfo, _makeRequest(), log, done));
afterEach(() => cleanup());
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/bucketDeletePolicy.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ function _makeRequest(includePolicy) {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
if (includePolicy) {
const examplePolicy = {
Expand All @@ -36,8 +37,7 @@ function _makeRequest(includePolicy) {
}
return request;
}
// TODO CLDSRV-430 remove skip
describe.skip('deleteBucketPolicy API', () => {
describe('deleteBucketPolicy API', () => {
before(() => cleanup());
beforeEach(done => bucketPut(authInfo, _makeRequest(), log, done));
afterEach(() => cleanup());
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/api/bucketDeleteWebsite.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const testBucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
const testBucketDeleteWebsiteRequest = {
bucketName,
Expand All @@ -28,11 +29,11 @@ const testBucketDeleteWebsiteRequest = {
},
url: '/?website',
query: { website: '' },
actionImplicitDenies: false,
};
const testBucketPutWebsiteRequest = Object.assign({ post: config.getXml() },
testBucketDeleteWebsiteRequest);
// TODO CLDSRV-430 remove skip
describe.skip('deleteBucketWebsite API', () => {
describe('deleteBucketWebsite API', () => {
beforeEach(done => {
cleanup();
bucketPut(authInfo, testBucketPutRequest, log, () => {
Expand Down
12 changes: 7 additions & 5 deletions tests/unit/api/objectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ function testAuth(bucketOwner, authUser, bucketPutReq, objPutReq, objDelReq,
});
});
}
// TODO CLDSRV-430 remove skip
describe.skip('objectDelete API', () => {
describe('objectDelete API', () => {
let testPutObjectRequest;

before(() => {
Expand Down Expand Up @@ -85,7 +84,8 @@ describe.skip('objectDelete API', () => {
url: `/${bucketName}/${objectKey}`,
});

it('should delete an object', done => {
// TODO CLDSRV-429 remove skip - skipped due to get at the end
it.skip('should delete an object', done => {
bucketPut(authInfo, testBucketPutRequest, log, () => {
objectPut(authInfo, testPutObjectRequest,
undefined, log, () => {
Expand All @@ -102,7 +102,8 @@ describe.skip('objectDelete API', () => {
});
});

it('should delete a 0 bytes object', done => {
// TODO CLDSRV-429 remove skip - skipped due to get at the end
it.skip('should delete a 0 bytes object', done => {
const testPutObjectRequest = new DummyRequest({
bucketName,
namespace,
Expand All @@ -128,7 +129,8 @@ describe.skip('objectDelete API', () => {
});
});

it('should delete a multipart upload and send `uploadId` as `replayId` to deleteObject', done => {
// TODO CLDSRV-431 remove skip - skipped due to MPU call
it.skip('should delete a multipart upload and send `uploadId` as `replayId` to deleteObject', done => {
bucketPut(authInfo, testBucketPutRequest, log, () => {
mpuUtils.createMPU(namespace, bucketName, objectKey, log,
(err, testUploadId) => {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/objectDeleteTagging.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const testBucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};

const testPutObjectRequest = new DummyRequest({
Expand All @@ -31,8 +32,7 @@ const testPutObjectRequest = new DummyRequest({
headers: {},
url: `/${bucketName}/${objectName}`,
}, postBody);
// TODO CLDSRV-430 remove skip
describe.skip('deleteObjectTagging API', () => {
describe('deleteObjectTagging API', () => {
beforeEach(done => {
cleanup();
bucketPut(authInfo, testBucketPutRequest, log, err => {
Expand Down