Compare commits

...

2 Commits

Author SHA1 Message Date
Will Toozs 8cebedf178
CLDSRV-426: update ACL permission checks for implicitDeny logic
CLDSRV-426: add tests for ACL permission check updates

CLDSRV-426: add tests for ACL permission check updates
2023-11-07 11:38:05 +01:00
Will Toozs 849c926875
CLDSRV-424: api call updated with implicit deny logic 2023-11-07 11:34:12 +01:00
2 changed files with 306 additions and 12 deletions

View File

@ -8,13 +8,32 @@ const { allAuthedUsersId, bucketOwnerActions, logId, publicId,
const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ? const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ?
process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : []; process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];
function checkBucketAcls(bucket, requestType, canonicalID) { function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) {
// Same logic applies on the Versioned APIs, so let's simplify it.
const requestTypeParsed = requestType.endsWith('Version') ?
requestType.slice(0, -7) : requestType;
if (bucket.getOwner() === canonicalID) { if (bucket.getOwner() === canonicalID) {
return true; return true;
} }
// Backward compatibility
const arrayOfAllowed = [
'objectPutTagging',
'objectPutLegalHold',
'objectPutRetention',
];
if (mainApiCall === 'objectGet') {
if (requestTypeParsed === 'objectGetTagging') {
return true;
}
}
if (mainApiCall === 'objectPut') {
if (arrayOfAllowed.includes(requestTypeParsed)) {
return true;
}
}
const bucketAcl = bucket.getAcl(); const bucketAcl = bucket.getAcl();
if (requestType === 'bucketGet' || requestType === 'bucketHead') { if (requestTypeParsed === 'bucketGet' || requestTypeParsed === 'bucketHead') {
if (bucketAcl.Canned === 'public-read' if (bucketAcl.Canned === 'public-read'
|| bucketAcl.Canned === 'public-read-write' || bucketAcl.Canned === 'public-read-write'
|| (bucketAcl.Canned === 'authenticated-read' || (bucketAcl.Canned === 'authenticated-read'
@ -32,7 +51,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
return true; return true;
} }
} }
if (requestType === 'bucketGetACL') { if (requestTypeParsed === 'bucketGetACL') {
if ((bucketAcl.Canned === 'log-delivery-write' if ((bucketAcl.Canned === 'log-delivery-write'
&& canonicalID === logId) && canonicalID === logId)
|| bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1 || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
@ -48,7 +67,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
} }
} }
if (requestType === 'bucketPutACL') { if (requestTypeParsed === 'bucketPutACL') {
if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1 if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
|| bucketAcl.WRITE_ACP.indexOf(canonicalID) > -1) { || bucketAcl.WRITE_ACP.indexOf(canonicalID) > -1) {
return true; return true;
@ -62,11 +81,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
} }
} }
if (requestType === 'bucketDelete' && bucket.getOwner() === canonicalID) { if (requestTypeParsed === 'objectDelete' || requestTypeParsed === 'objectPut') {
return true;
}
if (requestType === 'objectDelete' || requestType === 'objectPut') {
if (bucketAcl.Canned === 'public-read-write' if (bucketAcl.Canned === 'public-read-write'
|| bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1 || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
|| bucketAcl.WRITE.indexOf(canonicalID) > -1) { || bucketAcl.WRITE.indexOf(canonicalID) > -1) {
@ -86,11 +101,12 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
// objectPutACL, objectGetACL, objectHead or objectGet, the bucket // objectPutACL, objectGetACL, objectHead or objectGet, the bucket
// authorization check should just return true so can move on to check // authorization check should just return true so can move on to check
// rights at the object level. // rights at the object level.
return (requestType === 'objectPutACL' || requestType === 'objectGetACL' || return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL' ||
requestType === 'objectGet' || requestType === 'objectHead'); requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
} }
function checkObjectAcls(bucket, objectMD, requestType, canonicalID) { function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser,
isUserUnauthenticated, mainApiCall) {
const bucketOwner = bucket.getOwner(); const bucketOwner = bucket.getOwner();
// acls don't distinguish between users and accounts, so both should be allowed // acls don't distinguish between users and accounts, so both should be allowed
if (bucketOwnerActions.includes(requestType) if (bucketOwnerActions.includes(requestType)
@ -100,6 +116,15 @@ function checkObjectAcls(bucket, objectMD, requestType, canonicalID) {
if (objectMD['owner-id'] === canonicalID) { if (objectMD['owner-id'] === canonicalID) {
return true; return true;
} }
// Backward compatibility
if (mainApiCall === 'objectGet') {
if ((isUserUnauthenticated || (requesterIsNotUser && bucketOwner === objectMD['owner-id']))
&& requestType === 'objectGetTagging') {
return true;
}
}
if (!objectMD.acl) { if (!objectMD.acl) {
return false; return false;
} }

View File

@ -0,0 +1,269 @@
const assert = require('assert');
const { checkBucketAcls, checkObjectAcls } = require('../../../lib/api/apiUtils/authorization/permissionChecks');
const constants = require('../../../constants');
const { bucketOwnerActions } = constants;
describe('checkBucketAcls', () => {
const mockBucket = {
getOwner: () => 'ownerId',
getAcl: () => ({
Canned: '',
FULL_CONTROL: [],
READ: [],
READ_ACP: [],
WRITE: [],
WRITE_ACP: [],
}),
};
const testScenarios = [
{
description: 'should return true if bucket owner matches canonicalID',
input: {
bucketAcl: {}, requestType: 'anyType', canonicalID: 'ownerId', mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for objectGetTagging when mainApiCall is objectGet',
input: {
bucketAcl: {}, requestType: 'objectGetTagging', canonicalID: 'anyId', mainApiCall: 'objectGet',
},
expected: true,
},
{
description: 'should return true for objectPutTagging when mainApiCall is objectPut',
input: {
bucketAcl: {}, requestType: 'objectPutTagging', canonicalID: 'anyId', mainApiCall: 'objectPut',
},
expected: true,
},
{
description: 'should return true for objectPutLegalHold when mainApiCall is objectPut',
input: {
bucketAcl: {}, requestType: 'objectPutLegalHold', canonicalID: 'anyId', mainApiCall: 'objectPut',
},
expected: true,
},
{
description: 'should return true for objectPutRetention when mainApiCall is objectPut',
input: {
bucketAcl: {}, requestType: 'objectPutRetention', canonicalID: 'anyId', mainApiCall: 'objectPut',
},
expected: true,
},
{
description: 'should return true for bucketGet if canned acl is public-read-write',
input: {
bucketAcl: { Canned: 'public-read-write' },
requestType: 'bucketGet',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for bucketGet if canned acl is authenticated-read and id is not publicId',
input: {
bucketAcl: { Canned: 'authenticated-read' },
requestType: 'bucketGet',
canonicalID: 'anyIdNotPublic',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for bucketHead if canned acl is public-read',
input: {
bucketAcl: { Canned: 'public-read' },
requestType: 'bucketHead',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for bucketGet if canonicalID has FULL_CONTROL access',
input: {
bucketAcl: { FULL_CONTROL: ['anyId'], READ: [] },
requestType: 'bucketGet',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for bucketGetACL if canonicalID has FULL_CONTROL',
input: {
bucketAcl: { FULL_CONTROL: ['anyId'], READ_ACP: [] },
requestType: 'bucketGetACL',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for objectDelete if bucketAcl.Canned is public-read-write',
input: {
bucketAcl: { Canned: 'public-read-write' },
requestType: 'objectDelete',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for requestType ending with "Version"',
input: {
bucketAcl: {},
requestType: 'objectGetVersion',
canonicalID: 'anyId',
mainApiCall: 'objectGet',
},
expected: true,
},
{
description: 'should return true for objectPutACL',
input: {
bucketAcl: {},
requestType: 'objectPutACL',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return true for objectGetACL',
input: {
bucketAcl: {},
requestType: 'objectGetACL',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: true,
},
{
description: 'should return false for unmatched scenarios',
input: {
bucketAcl: {},
requestType: 'unmatchedRequest',
canonicalID: 'anyId',
mainApiCall: 'anyApiCall',
},
expected: false,
},
];
testScenarios.forEach(scenario => {
it(scenario.description, () => {
// Mock the bucket based on the test scenario's input
mockBucket.getAcl = () => scenario.input.bucketAcl;
const result = checkBucketAcls(mockBucket,
scenario.input.requestType, scenario.input.canonicalID, scenario.input.mainApiCall);
assert.strictEqual(result, scenario.expected);
});
});
});
describe('checkObjectAcls', () => {
const mockBucket = {
getOwner: () => 'bucketOwnerId',
getName: () => 'bucketName',
getAcl: () => ({ Canned: '' }),
};
const mockObjectMD = {
'owner-id': 'objectOwnerId',
'acl': {
Canned: '',
FULL_CONTROL: [],
READ: [],
READ_ACP: [],
WRITE: [],
WRITE_ACP: [],
},
};
it('should return true if request type is in bucketOwnerActions and bucket owner matches canonicalID', () => {
assert.strictEqual(checkObjectAcls(mockBucket, mockObjectMD, bucketOwnerActions[0],
'bucketOwnerId', false, false, 'anyApiCall'), true);
});
it('should return true if objectMD owner matches canonicalID', () => {
assert.strictEqual(checkObjectAcls(mockBucket, mockObjectMD, 'anyType',
'objectOwnerId', false, false, 'anyApiCall'), true);
});
it('should return true for objectGetTagging when mainApiCall is objectGet and conditions met', () => {
assert.strictEqual(checkObjectAcls(mockBucket, mockObjectMD, 'objectGetTagging',
'anyIdNotPublic', true, true, 'objectGet'), true);
});
it('should return false if no acl provided in objectMD', () => {
const objMDWithoutAcl = Object.assign({}, mockObjectMD);
delete objMDWithoutAcl.acl;
assert.strictEqual(checkObjectAcls(mockBucket, objMDWithoutAcl, 'anyType',
'anyId', false, false, 'anyApiCall'), false);
});
const tests = [
{
acl: 'public-read', reqType: 'objectGet', id: 'anyIdNotPublic', expected: true,
},
{
acl: 'public-read-write', reqType: 'objectGet', id: 'anyIdNotPublic', expected: true,
},
{
acl: 'authenticated-read', reqType: 'objectGet', id: 'anyIdNotPublic', expected: true,
},
{
acl: 'bucket-owner-read', reqType: 'objectGet', id: 'bucketOwnerId', expected: true,
},
{
acl: 'bucket-owner-full-control', reqType: 'objectGet', id: 'bucketOwnerId', expected: true,
},
{
aclList: ['someId', 'anyIdNotPublic'],
aclField: 'FULL_CONTROL',
reqType: 'objectGet',
id: 'anyIdNotPublic',
expected: true,
},
{
aclList: ['someId', 'anyIdNotPublic'],
aclField: 'READ',
reqType: 'objectGet',
id: 'anyIdNotPublic',
expected: true,
},
{ reqType: 'objectPut', id: 'anyId', expected: true },
{ reqType: 'objectDelete', id: 'anyId', expected: true },
{
aclList: ['anyId'], aclField: 'FULL_CONTROL', reqType: 'objectPutACL', id: 'anyId', expected: true,
},
{
aclList: ['anyId'], aclField: 'FULL_CONTROL', reqType: 'objectGetACL', id: 'anyId', expected: true,
},
{
acl: '', reqType: 'objectGet', id: 'randomId', expected: false,
},
];
tests.forEach(test => {
it(`should return ${test.expected} for ${test.reqType} with ACL as ${test.acl
|| (`${test.aclField}:${JSON.stringify(test.aclList)}`)}`, () => {
if (test.acl) {
mockObjectMD.acl.Canned = test.acl;
} else if (test.aclList && test.aclField) {
mockObjectMD.acl[test.aclField] = test.aclList;
}
assert.strictEqual(
checkObjectAcls(mockBucket, mockObjectMD, test.reqType, test.id, false, false, 'anyApiCall'),
test.expected,
);
});
});
});