Merge remote-tracking branch 'origin/bugfix/S3C-2663/withBucketPolicy' into w/8.2/bugfix/S3C-2663/withBucketPolicy

lib/api/apiUtils/authorization/permissionChecks.js

@@ -328,10 +328,87 @@ function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log) {
     return (aclPermission || (bucketPolicyPermission === 'allow'));
 }
 
+function _isPermissionGranted(permissionList, canonicalID) {
+    if (permissionList.indexOf(publicId) > -1) {
+        return true;
+    }
+
+    if (permissionList.indexOf(allAuthedUsersId) > -1 &&
+        canonicalID !== publicId) {
+        return true;
+    }
+
+    if (permissionList.indexOf(canonicalID) > -1) {
+        return true;
+    }
+
+    return false;
+}
+
+function hasBucketReadAccess(bucket, requestType, canonicalID, authInfo, log) {
+    const bucketAcl = bucket.getAcl();
+    const bucketOwner = bucket.getOwner();
+    const bucketPolicy = bucket.getBucketPolicy();
+    let arn = null;
+    let requesterIsNotUser = true;
+
+    // User is already authorized on the bucket for FULL_CONTROL or WRITE or
+    // bucket has canned ACL public-read-write
+    if (requestType === 'objectPut' || requestType === 'objectDelete') {
+        return true;
+    }
+
+    if (authInfo) {
+        requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
+        arn = authInfo.getArn();
+    }
+
+    // bucket policies over acls if any is applicable
+    if (bucketPolicy) {
+        // bucketGet covers listObjects and listMultipartUploads, bucket read
+        // actions
+        const bucketListPermission = checkBucketPolicy(
+            bucketPolicy,
+            'bucketGet',
+            canonicalID,
+            arn,
+            bucketOwner,
+            log
+        );
+
+        if (bucketListPermission === 'explicitDeny') {
+            return false;
+        }
+
+        if (bucketListPermission === 'allow') {
+            return true;
+        }
+        // defaultDeny, fallback onto acls
+    }
+
+    if ((canonicalID === bucketOwner && requesterIsNotUser) ||
+        _isPermissionGranted(bucketAcl.FULL_CONTROL, canonicalID) ||
+        _isPermissionGranted(bucketAcl.READ, canonicalID)) {
+        return true;
+    }
+
+    if (bucketAcl.Canned === 'public-read' ||
+        bucketAcl.Canned === 'public-read-write' ||
+        (bucketAcl.Canned === 'authenticated-read' &&
+         canonicalID !== publicId)) {
+        return true;
+    }
+
+    return false;
+}
+
 function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log) {
     const bucketOwner = bucket.getOwner();
     if (!objectMD) {
-        return false;
+        // if read access is granted, return true to have the api handler
+        // respond accordingly to the missing object metadata
+        // if read access is not granted, return false for AccessDenied
+        return hasBucketReadAccess(bucket, requestType, canonicalID, authInfo, log);
     }
     let requesterIsNotUser = true;
     let arn = null;

lib/metadata/metadataUtils.js

@@ -150,11 +150,7 @@ function metadataValidateBucketAndObj(params, log, callback) {
             return next(null, bucket, objMD);
         },
         function checkObjectAuth(bucket, objMD, next) {
-            if (!objMD) {
-                return next(null, bucket);
-            }
-            if (!isObjAuthorized(bucket, objMD, requestType, canonicalID,
-            authInfo, log)) {
+            if (!isObjAuthorized(bucket, objMD, requestType, canonicalID, authInfo, log)) {
                 log.debug('access denied for user on object', { requestType });
                 return next(errors.AccessDenied, bucket);
             }

tests/unit/api/objectACLauth.js

@@ -275,3 +275,188 @@ describe('object authorization for objectPutACL and objectGetACL', () => {
         assert.strictEqual(authorizedResult, true);
     });
 });
+
+describe('without object metadata', () => {
+    afterEach(() => {
+        bucket.setFullAcl({
+            Canned: 'private',
+            FULL_CONTROL: [],
+            WRITE: [],
+            WRITE_ACP: [],
+            READ: [],
+            READ_ACP: [],
+        });
+        bucket.setBucketPolicy(null);
+    });
+
+    const requestTypes = [
+        'objectGet',
+        'objectHead',
+        'objectPutACL',
+        'objectGetACL',
+    ];
+
+    const allowedAccess = [true, true, true, true];
+    const deniedAccess = [false, false, false, false];
+
+    const tests = [
+        {
+            it: 'should allow bucket owner',
+            canned: 'private', id: bucketOwnerCanonicalId,
+            aclParam: null,
+            response: allowedAccess,
+        },
+        {
+            it: 'should not allow public if canned private',
+            canned: 'private', id: constants.publicId,
+            aclParam: null,
+            response: deniedAccess,
+        },
+        {
+            it: 'should not other accounts if canned private',
+            canned: 'private', id: accountToVet,
+            aclParam: null,
+            response: deniedAccess,
+        },
+        {
+            it: 'should allow public if bucket is canned public-read',
+            canned: 'public-read', id: constants.publicId,
+            aclParam: null,
+            response: allowedAccess,
+        },
+        {
+            it: 'should allow public if bucket is canned public-read-write',
+            canned: 'public-read-write', id: constants.publicId,
+            aclParam: null,
+            response: allowedAccess,
+        },
+        {
+            it: 'should not allow public if bucket is canned ' +
+            'authenticated-read',
+            canned: 'authenticated-read', id: constants.publicId,
+            aclParam: null,
+            response: deniedAccess,
+        },
+        {
+            it: 'should allow authenticated users if bucket is canned ' +
+            'authenticated-read',
+            canned: 'authenticated-read', id: accountToVet,
+            aclParam: null,
+            response: allowedAccess,
+        },
+        {
+            it: 'should allow account if granted bucket READ',
+            canned: '', id: accountToVet,
+            aclParam: ['READ', accountToVet],
+            response: allowedAccess,
+        },
+        {
+            it: 'should allow account if granted bucket FULL_CONTROL',
+            canned: '', id: accountToVet,
+            aclParam: ['FULL_CONTROL', accountToVet],
+            response: allowedAccess,
+        },
+        {
+            it: 'should allow public if granted bucket read action in policy',
+            canned: 'private', id: constants.publicId,
+            aclParam: null,
+            policy: {
+                Version: '2012-10-17',
+                Statement: [
+                    {
+                        Effect: 'Allow',
+                        Resource: 'arn:aws:s3:::niftybucket',
+                        Principal: '*',
+                        Action: ['s3:ListBucket'],
+                    },
+                ],
+            },
+            response: allowedAccess,
+        },
+        {
+            it: 'should not allow public if denied bucket read action in policy',
+            canned: 'public-read', id: constants.publicId,
+            aclParam: null,
+            policy: {
+                Version: '2012-10-17',
+                Statement: [
+                    {
+                        Effect: 'Deny',
+                        Resource: 'arn:aws:s3:::niftybucket',
+                        Principal: '*',
+                        Action: ['s3:ListBucket'],
+                    },
+                ],
+            },
+            response: deniedAccess,
+        },
+        {
+            it: 'should allow account if granted bucket read action in policy',
+            canned: 'private', id: accountToVet,
+            aclParam: null,
+            policy: {
+                Version: '2012-10-17',
+                Statement: [
+                    {
+                        Effect: 'Allow',
+                        Resource: 'arn:aws:s3:::niftybucket',
+                        Principal: { AWS: [altAcctAuthInfo.getArn()] },
+                        Action: ['s3:ListBucket'],
+                    },
+                ],
+            },
+            response: allowedAccess,
+            authInfo: altAcctAuthInfo,
+        },
+        {
+            it: 'should not allow account if denied bucket read action in policy',
+            canned: 'public-read', id: accountToVet,
+            aclParam: null,
+            policy: {
+                Version: '2012-10-17',
+                Statement: [
+                    {
+                        Effect: 'Deny',
+                        Resource: 'arn:aws:s3:::niftybucket',
+                        Principal: { CanonicalUser: [altAcctAuthInfo.getCanonicalID()] },
+                        Action: ['s3:ListBucket'],
+                    },
+                ],
+            },
+            response: deniedAccess,
+            authInfo: altAcctAuthInfo,
+        },
+    ];
+
+    tests.forEach(value => {
+        it(value.it, done => {
+            const authInfoUser = value.authInfo ? value.authInfo : authInfo;
+
+            if (value.aclParam) {
+                bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]);
+            }
+
+            if (value.policy) {
+                bucket.setBucketPolicy(value.policy);
+            }
+
+            bucket.setCannedAcl(value.canned);
+            const results = requestTypes.map(type =>
+                isObjAuthorized(bucket, null, type, value.id, authInfoUser, log));
+            assert.deepStrictEqual(results, value.response);
+            done();
+        });
+    });
+
+
+    it('should allow access to anyone since checks ' +
+        'are done at bucket level', () => {
+        const requestTypes = ['objectPut', 'objectDelete'];
+        const results = requestTypes.map(type =>
+            isObjAuthorized(bucket, null, type, accountToVet, authInfo, log));
+        assert.deepStrictEqual(results, [true, true]);
+        const publicUserResults = requestTypes.map(type =>
+            isObjAuthorized(bucket, null, type, constants.publicId, authInfo, log));
+        assert.deepStrictEqual(publicUserResults, [true, true]);
+    });
+});

yarn.lock

@@ -695,17 +695,16 @@ arraybuffer.slice@~0.0.7:
   dependencies:
     "@hapi/joi" "^15.1.0"
     JSONStream "^1.0.0"
-    agentkeepalive "^4.1.3"
     ajv "6.12.2"
     async "~2.6.1"
     aws-sdk "2.80.0"
-    azure-storage "~2.1.0"
+    azure-storage "^2.1.0"
     backo "^1.1.0"
     bson "4.0.0"
     debug "~4.1.0"
     diskusage "^1.1.1"
     fcntl "github:scality/node-fcntl"
-    hdclient scality/hdclient#489db2106570b9ea41bdacdf56131324d0db6a58
+    hdclient scality/hdclient#5145e04e5ed33e85106765b1caa90cd245ef482b
     https-proxy-agent "^2.2.0"
     ioredis "4.9.5"
     ipaddr.js "1.9.1"
@@ -1298,9 +1297,9 @@ bucketclient@scality/bucketclient:
   version "8.1.0"
   resolved "https://codeload.github.com/scality/bucketclient/tar.gz/07d5a3813878b2746ed00e41c177bc2a0460e243"
   dependencies:
-    agentkeepalive "^4.1.3"
-    arsenal scality/Arsenal#8ed8478
+    arsenal scality/Arsenal#c57cde8
     werelogs scality/werelogs#351a2a3
+    yarn "^1.17.3"
 
 bucketclient@scality/bucketclient#949f11a:
   version "8.1.0"