Skip to content

Commit

Permalink
CLDSRV-426: update ACL permission checks for implicitDeny logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Will Toozs authored and benzekrimaha committed Nov 7, 2023
1 parent ae770d0 commit faca32f
Showing 1 changed file with 37 additions and 12 deletions.
49 changes: 37 additions & 12 deletions lib/api/apiUtils/authorization/permissionChecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,32 @@ const { allAuthedUsersId, bucketOwnerActions, logId, publicId,
const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ?
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) {
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();
if (requestType === 'bucketGet' || requestType === 'bucketHead') {
if (requestTypeParsed === 'bucketGet' || requestTypeParsed === 'bucketHead') {
if (bucketAcl.Canned === 'public-read'
|| bucketAcl.Canned === 'public-read-write'
|| (bucketAcl.Canned === 'authenticated-read'
Expand All @@ -32,7 +51,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
return true;
}
}
if (requestType === 'bucketGetACL') {
if (requestTypeParsed === 'bucketGetACL') {
if ((bucketAcl.Canned === 'log-delivery-write'
&& canonicalID === logId)
|| bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
Expand All @@ -48,7 +67,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
}
}

if (requestType === 'bucketPutACL') {
if (requestTypeParsed === 'bucketPutACL') {
if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
|| bucketAcl.WRITE_ACP.indexOf(canonicalID) > -1) {
return true;
Expand All @@ -62,11 +81,7 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
}
}

if (requestType === 'bucketDelete' && bucket.getOwner() === canonicalID) {
return true;
}

if (requestType === 'objectDelete' || requestType === 'objectPut') {
if (requestTypeParsed === 'objectDelete' || requestTypeParsed === 'objectPut') {
if (bucketAcl.Canned === 'public-read-write'
|| bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
|| bucketAcl.WRITE.indexOf(canonicalID) > -1) {
Expand All @@ -86,11 +101,12 @@ function checkBucketAcls(bucket, requestType, canonicalID) {
// 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 (requestType === 'objectPutACL' || requestType === 'objectGetACL' ||
requestType === 'objectGet' || requestType === 'objectHead');
return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL' ||
requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
}

function checkObjectAcls(bucket, objectMD, requestType, canonicalID) {
function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser,
isUserUnauthenticated, mainApiCall) {
const bucketOwner = bucket.getOwner();
// acls don't distinguish between users and accounts, so both should be allowed
if (bucketOwnerActions.includes(requestType)
Expand All @@ -100,6 +116,15 @@ function checkObjectAcls(bucket, objectMD, requestType, canonicalID) {
if (objectMD['owner-id'] === canonicalID) {
return true;
}

// Backward compatibility
if (mainApiCall === 'objectGet') {
if ((isUserUnauthenticated || (requesterIsNotUser && bucketOwner === objectMD['owner-id']))
&& requestType === 'objectGetTagging') {
return true;
}
}

if (!objectMD.acl) {
return false;
}
Expand Down

0 comments on commit faca32f

Please sign in to comment.