Skip to content

Improvement/cldsrv 429 get api imp deny #5336

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 5 commits into
base: improvement/CLDSRV-430-del-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
2 changes: 1 addition & 1 deletion lib/api/bucketGet.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ function bucketGet(authInfo, request, log, callback) {
listParams.marker = params.marker;
}

metadataValidateBucket(metadataValParams, log, (err, bucket) => {
metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
if (err) {
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketGetACL.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function bucketGetACL(authInfo, request, log, callback) {
},
};

metadataValidateBucket(metadataValParams, log, (err, bucket) => {
metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
if (err) {
Expand Down
3 changes: 2 additions & 1 deletion lib/api/bucketGetCors.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ function bucketGetCors(authInfo, request, log, callback) {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);

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: 'bucketGetCors',
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketGetEncryption.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function bucketGetEncryption(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) => {
// If sseInfo is present but the `mandatory` flag is not set
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketGetLifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function bucketGetLifecycle(authInfo, request, log, callback) {
requestType: 'bucketGetLifecycle',
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/bucketGetLocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ function bucketGetLocation(authInfo, request, log, callback) {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);

if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo,
request.actionImplicitDenies, log, request)) {
log.debug('access denied for account on bucket', {
requestType,
method: 'bucketGetLocation',
Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketGetNotification.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function bucketGetNotification(authInfo, request, log, callback) {
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/bucketGetObjectLock.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function bucketGetObjectLock(authInfo, request, log, callback) {
requestType: 'bucketGetObjectLock',
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/bucketGetPolicy.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function bucketGetPolicy(authInfo, request, log, callback) {
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/bucketGetReplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function bucketGetReplication(authInfo, request, log, callback) {
requestType: 'bucketGetReplication',
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/bucketGetVersioning.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function bucketGetVersioning(authInfo, request, log, callback) {
request,
};

metadataValidateBucket(metadataValParams, log, (err, bucket) => {
metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
if (err) {
Expand Down
3 changes: 2 additions & 1 deletion lib/api/bucketGetWebsite.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ function bucketGetWebsite(authInfo, request, log, callback) {

const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
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: 'bucketGetWebsite',
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectGet.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function objectGet(authInfo, request, returnTagCount, log, callback) {
request,
};

return metadataValidateBucketAndObj(mdValParams, log,
return metadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log,
(err, bucket, objMD) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectGetACL.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function objectGetACL(authInfo, request, log, callback) {

return async.waterfall([
function validateBucketAndObj(next) {
return metadataValidateBucketAndObj(metadataValParams, log,
return metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
(err, bucket, objectMD) => {
if (err) {
log.trace('request authorization failed',
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectGetLegalHold.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function objectGetLegalHold(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
2 changes: 1 addition & 1 deletion lib/api/objectGetRetention.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function objectGetRetention(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
2 changes: 1 addition & 1 deletion lib/api/objectGetTagging.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function objectGetTagging(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
15 changes: 8 additions & 7 deletions lib/api/websiteGet.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ const { pushMetric } = require('../utapi/utilities');
* @param {string} objectKey - object key from request (or as translated in
* websiteGet)
* @param {object} corsHeaders - CORS-related response headers
* @param {object} request - normalized request object
* @param {object} log - Werelogs instance
* @param {function} callback - callback to function in route
* @return {undefined}
*/
function _errorActions(err, errorDocument, routingRules,
bucket, objectKey, corsHeaders, log, callback) {
bucket, objectKey, corsHeaders, request, log, callback) {
const bucketName = bucket.getName();
const errRoutingRule = findRoutingRule(routingRules,
objectKey, err.code);
Expand All @@ -47,7 +48,7 @@ function _errorActions(err, errorDocument, routingRules,
// return the default error message if the object is private
// rather than sending a stored error file
if (!isObjAuthorized(bucket, errObjMD, 'objectGet',
constants.publicId, null, log)) {
constants.publicId, null, request.actionImplicitDenies, log)) {
log.trace('errorObj not authorized', { error: err });
return callback(err, true, null, corsHeaders);
}
Expand Down Expand Up @@ -144,24 +145,24 @@ function websiteGet(request, log, callback) {
{ error: err });
let returnErr = err;
const bucketAuthorized = isBucketAuthorized(bucket,
'bucketGet', constants.publicId, null, log, request);
'bucketGet', constants.publicId, null, request.actionImplicitDenies, log, request);
// if index object does not exist and bucket is private AWS
// returns 403 - AccessDenied error.
if (err.is.NoSuchKey && !bucketAuthorized) {
returnErr = errors.AccessDenied;
}
return _errorActions(returnErr,
websiteConfig.getErrorDocument(), routingRules,
bucket, reqObjectKey, corsHeaders, log,
bucket, reqObjectKey, corsHeaders, request, log,
callback);
}
if (!isObjAuthorized(bucket, objMD, 'objectGet',
constants.publicId, null, log, request)) {
constants.publicId, null, request.actionImplicitDenies, log, request)) {
const err = errors.AccessDenied;
log.trace('request not authorized', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket,
reqObjectKey, corsHeaders, log, callback);
reqObjectKey, corsHeaders, request, log, callback);
}

const headerValResult = validateHeaders(request.headers,
Expand All @@ -171,7 +172,7 @@ function websiteGet(request, log, callback) {
log.trace('header validation error', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket, reqObjectKey,
corsHeaders, log, callback);
corsHeaders, request, log, callback);
}
// check if object to serve has website redirect header
// Note: AWS prioritizes website configuration rules over
Expand Down
3 changes: 1 addition & 2 deletions tests/functional/aws-node-sdk/test/bucket/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,7 @@ const tests = [
},
];

// TODO CLDSRV-928 remove skip
describe.skip('GET Bucket - AWS.S3.listObjects', () => {
describe('GET Bucket - AWS.S3.listObjects', () => {
describe('When user is unauthorized', () => {
let bucketUtil;
let bucketName;
Expand Down
7 changes: 3 additions & 4 deletions tests/unit/api/bucketGet.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const baseGetRequest = {
bucketName,
namespace,
headers: { host: '/' },
actionImplicitDenies: false,
};
const baseUrl = `/${bucketName}`;

Expand Down Expand Up @@ -173,8 +174,7 @@ const tests = [
},
},
];
// TODO CLDSRV-429 remove skip
describe.skip('bucketGet API', () => {
describe('bucketGet API', () => {
beforeEach(() => {
cleanup();
});
Expand Down Expand Up @@ -290,8 +290,7 @@ const testsForV2 = [...tests,
},
];

// TODO CLDSRV-429 remove skip
describe.skip('bucketGet API V2', () => {
describe('bucketGet API V2', () => {
beforeEach(() => {
cleanup();
});
Expand Down
13 changes: 11 additions & 2 deletions tests/unit/api/bucketGetACL.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ const authInfo = makeAuthInfo(accessKey);
const canonicalID = authInfo.getCanonicalID();
const namespace = 'default';
const bucketName = 'bucketname';
// TODO CLDSRV-429 remove skip
describe.skip('bucketGetACL API', () => {
describe('bucketGetACL API', () => {
beforeEach(() => {
cleanup();
});
Expand All @@ -25,13 +24,15 @@ describe.skip('bucketGetACL API', () => {
namespace,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
const testGetACLRequest = {
bucketName,
namespace,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

it('should get a canned private ACL', done => {
Expand All @@ -44,6 +45,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -76,6 +78,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -119,6 +122,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -156,6 +160,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -194,6 +199,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -248,6 +254,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};
const canonicalIDforSample1 =
'79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be';
Expand Down Expand Up @@ -338,6 +345,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down Expand Up @@ -377,6 +385,7 @@ describe.skip('bucketGetACL API', () => {
},
url: '/?acl',
query: { acl: '' },
actionImplicitDenies: false,
};

async.waterfall([
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/api/bucketGetCors.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const testBucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};

function _makeCorsRequest(xml) {
Expand All @@ -26,6 +27,7 @@ function _makeCorsRequest(xml) {
},
url: '/?cors',
query: { cors: '' },
actionImplicitDenies: false,
};

if (xml) {
Expand Down Expand Up @@ -55,8 +57,7 @@ function _comparePutGetXml(sampleXml, done) {
});
});
}
// TODO CLDSRV-429 remove skip
describe.skip('getBucketCors API', () => {
describe('getBucketCors API', () => {
beforeEach(done => {
cleanup();
bucketPut(authInfo, testBucketPutRequest, log, done);
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/api/bucketGetLifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ const testBucketPutRequest = {
bucketName,
headers: { host: `${bucketName}.s3.amazonaws.com` },
url: '/',
actionImplicitDenies: false,
};
// TODO CLDSRV-429 remove skip
describe.skip('getBucketLifecycle API', () => {
describe('getBucketLifecycle API', () => {
before(() => cleanup());
beforeEach(done => bucketPut(authInfo, testBucketPutRequest, log, done));
afterEach(() => cleanup());
Expand Down
Loading