From 5269af10ba0f7e2fab89d993270dc6aca92e04ce Mon Sep 17 00:00:00 2001
From: shirady <57721533+shirady@users.noreply.github.com>
Date: Mon, 13 Jan 2025 13:09:19 +0200
Subject: [PATCH] add tests

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
---
 src/test/unit_tests/test_s3_bucket_policy.js | 98 +++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/src/test/unit_tests/test_s3_bucket_policy.js b/src/test/unit_tests/test_s3_bucket_policy.js
index 65123d97f4..0b56fbe344 100644
--- a/src/test/unit_tests/test_s3_bucket_policy.js
+++ b/src/test/unit_tests/test_s3_bucket_policy.js
@@ -33,6 +33,7 @@ async function assert_throws_async(promise, expected_message = 'Access Denied')
 const BKT = 'test2-bucket-policy-ops';
 const BKT_B = 'test2-bucket-policy-ops-1';
 const BKT_C = 'test2-bucket-policy-ops-2';
+const BKT_D = 'test2-bucket-policy-ops-3';
 const KEY = 'file1.txt';
 const user_a = 'alice';
 const user_b = 'bob';
@@ -134,6 +135,7 @@ async function setup() {
     s3_owner = new S3(s3_creds);
     await s3_owner.createBucket({ Bucket: BKT });
     await s3_owner.createBucket({ Bucket: BKT_C });
+    await s3_owner.createBucket({ Bucket: BKT_D });
     s3_anon = new S3({
         ...s3_creds,
         credentials: {
@@ -147,7 +149,7 @@ async function setup() {
     });
 }
 
-/*eslint max-lines-per-function: ["error", 1600]*/
+/*eslint max-lines-per-function: ["error", 2000]*/
 mocha.describe('s3_bucket_policy', function() {
     mocha.before(setup);
     mocha.it('should fail setting bucket policy when user doesn\'t exist', async function() {
@@ -335,6 +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}/*`]
+                }
+            ]
+        };
+        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);
+    });
+
+    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() {
+        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.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);
+    });
+
     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);