diff --git a/src/test/unit_tests/test_s3_bucket_policy.js b/src/test/unit_tests/test_s3_bucket_policy.js index 5def1a6118..59866c3fbc 100644 --- a/src/test/unit_tests/test_s3_bucket_policy.js +++ b/src/test/unit_tests/test_s3_bucket_policy.js @@ -337,99 +337,100 @@ mocha.describe('s3_bucket_policy', function() { })); }); - mocha.it('should not allow principal get object bucket policy with 2 statements: ' + - '(1) DENY principal by account ID (2) ALLOW account name as *', async function() { - if (!is_nc_coretest) this.skip(); // eslint-disable-line no-invalid-this - const policy = { - Statement: [{ - Sid: `Allow all s3 actions on bucket ${BKT_D} to all principals`, - Effect: 'Allow', - Principal: { AWS: ["*"] }, - Action: ['s3:*'], - Resource: [`arn:aws:s3:::${BKT_D}`, `arn:aws:s3:::${BKT_D}/*`] - }, - { - Sid: `Do not allow user ${user_a_account_details._id} get any object`, - Effect: 'Deny', - Principal: { AWS: [user_a_account_details._id] }, - Action: ['s3:*'], - Resource: [`arn:aws:s3:::${BKT_D}/*`] - } - ] + mocha.describe('s3_bucket_policy with more complex policies (conflict statements)', function() { + const allow_all_principal_all_s3_action_statement = { + Sid: `Allow all s3 actions on bucket ${BKT_D} to all principals`, + Effect: 'Allow', + Principal: { AWS: "*" }, + Action: ['s3:*'], + Resource: [`arn:aws:s3:::${BKT_D}`, `arn:aws:s3:::${BKT_D}/*`] }; - await s3_owner.putBucketPolicy({ - Bucket: BKT_D, - Policy: JSON.stringify(policy) - }); - // prepare - put the object to get - const key2 = 'file2.txt'; - const res_put_object = await s3_owner.putObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 - }); - assert.equal(res_put_object.$metadata.httpStatusCode, 200); - // should fail - user a has a DENY statement - await assert_throws_async(s3_a.getObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 - })); - // should fail - user b does not have a DENY statement (uses the general ALLOW statement) - const res_get_object = await s3_b.getObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 + mocha.it('should not allow principal get object bucket policy with 2 statements: ' + + '(1) DENY principal by account ID (2) ALLOW account name as *', async function() { + // in NC we allow principal to be also + if (!is_nc_coretest) this.skip(); // eslint-disable-line no-invalid-this + const policy = { + Statement: [ + allow_all_principal_all_s3_action_statement, + { + Sid: `Do not allow user ${user_a_account_details._id} get any object`, + Effect: 'Deny', + Principal: { AWS: [user_a_account_details._id] }, + Action: ['s3:*'], + Resource: [`arn:aws:s3:::${BKT_D}/*`] + } + ] + }; + await s3_owner.putBucketPolicy({ + Bucket: BKT_D, + Policy: JSON.stringify(policy) + }); + // prepare - put the object to get + const key2 = 'file2.txt'; + const res_put_object = await s3_owner.putObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + }); + assert.equal(res_put_object.$metadata.httpStatusCode, 200); + // should fail - user a has a DENY statement + await assert_throws_async(s3_a.getObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + })); + // should fail - user b does not have a DENY statement (uses the general ALLOW statement) + const res_get_object = await s3_b.getObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + }); + assert.equal(res_get_object.$metadata.httpStatusCode, 200); }); - assert.equal(res_get_object.$metadata.httpStatusCode, 200); - }); - mocha.it('should not allow principal get object bucket policy with 2 statements: ' + - '(1) DENY principal by account name (2) ALLOW account name as *', async function() { - const policy = { - Statement: [{ - Sid: `Allow all s3 actions on bucket ${BKT_D} to all principals`, - Effect: 'Allow', - Principal: { AWS: ["*"] }, - Action: ['s3:*'], - Resource: [`arn:aws:s3:::${BKT_D}`, `arn:aws:s3:::${BKT_D}/*`] - }, - { - Sid: `Do not allow user ${user_a_account_details.name} get any object`, - Effect: 'Deny', - Principal: { AWS: [user_a_account_details.name] }, - Action: ['s3:*'], - Resource: [`arn:aws:s3:::${BKT_D}/*`] - } - ] - }; - await s3_owner.putBucketPolicy({ - Bucket: BKT_D, - Policy: JSON.stringify(policy) - }); - // prepare - put the object to get - const key2 = 'file2.txt'; - const res_put_object = await s3_owner.putObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 - }); - assert.equal(res_put_object.$metadata.httpStatusCode, 200); - // should fail - user a has a DENY statement - await assert_throws_async(s3_a.getObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 - })); - // should fail - user b does not have a DENY statement (uses the general ALLOW statement) - const res_get_object = await s3_b.getObject({ - Body: BODY, - Bucket: BKT_D, - Key: key2 + mocha.it('should not allow principal get object bucket policy with 2 statements: ' + + '(1) DENY principal by account name (2) ALLOW account name as *', async function() { + const policy = { + Statement: [ + allow_all_principal_all_s3_action_statement, + { + Sid: `Do not allow user ${user_a_account_details.name} get any object`, + Effect: 'Deny', + Principal: { AWS: [user_a_account_details.name] }, + Action: ['s3:*'], + Resource: [`arn:aws:s3:::${BKT_D}/*`] + } + ] + }; + await s3_owner.putBucketPolicy({ + Bucket: BKT_D, + Policy: JSON.stringify(policy) + }); + // prepare - put the object to get + const key2 = 'file2.txt'; + const res_put_object = await s3_owner.putObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + }); + assert.equal(res_put_object.$metadata.httpStatusCode, 200); + // should fail - user a has a DENY statement + await assert_throws_async(s3_a.getObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + })); + // should fail - user b does not have a DENY statement (uses the general ALLOW statement) + const res_get_object = await s3_b.getObject({ + Body: BODY, + Bucket: BKT_D, + Key: key2 + }); + assert.equal(res_get_object.$metadata.httpStatusCode, 200); }); - assert.equal(res_get_object.$metadata.httpStatusCode, 200); }); + mocha.it('should be able to set bucket policy when none set', async function() { const self = this; // eslint-disable-line no-invalid-this self.timeout(15000);