NC | bucket policy status fix

Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>

src/sdk/bucketspace_fs.js

@@ -615,16 +615,14 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
         }
     }
 
-    async get_bucket_policy(params) {
+    async get_bucket_policy(params, object_sdk) {
         try {
             const { name } = params;
             dbg.log0('BucketSpaceFS.get_bucket_policy: Bucket name', name);
-            const bucket_path = this._get_bucket_config_path(name);
-            const { data } = await nb_native().fs.readFile(this.fs_context, bucket_path);
-            const bucket = JSON.parse(data.toString());
-            dbg.log0('BucketSpaceFS.get_bucket_policy: policy', bucket.s3_policy);
+            const bucket_policy_info = await object_sdk.read_bucket_sdk_policy_info(name);
+            dbg.log0('BucketSpaceFS.get_bucket_policy: policy', bucket_policy_info);
             return {
-                policy: bucket.s3_policy
+                policy: bucket_policy_info.s3_policy
             };
         } catch (err) {
             throw this._translate_bucket_error_codes(err);

src/sdk/nb.d.ts

@@ -844,7 +844,7 @@ interface BucketSpace {
 
     put_bucket_policy(params: object): Promise<any>;
     delete_bucket_policy(params: object): Promise<any>;
-    get_bucket_policy(params: object): Promise<any>;
+    get_bucket_policy(params: object, object_sdk: ObjectSDK): Promise<any>;
 
     get_object_lock_configuration(params: object, object_sdk: ObjectSDK): Promise<any>;
     put_object_lock_configuration(params: object, object_sdk: ObjectSDK): Promise<any>;

src/sdk/object_sdk.js

@@ -1033,7 +1033,7 @@ class ObjectSDK {
     async get_bucket_policy(params) {
         const bs = this._get_bucketspace();
         bucket_namespace_cache.invalidate_key(params.name);
-        return bs.get_bucket_policy(params);
+        return bs.get_bucket_policy(params, this);
     }
 
     should_run_triggers({ active_triggers, operation, obj }) {

src/test/unit_tests/nc_index.js

@@ -16,10 +16,10 @@ require('./test_nsfs_access');
 require('./test_bucketspace');
 require('./test_bucketspace_fs');
 require('./test_nsfs_glacier_backend');
+require('./test_s3_bucket_policy');
 
 // TODO: uncomment when supported
 //require('./test_s3_ops');
-//require('./test_s3_bucket_policy');
 //require('./test_s3_list_objects');
 //require('./test_s3_encryption');
 // require('./test_s3select');

src/test/unit_tests/test_bucketspace_fs.js

@@ -154,6 +154,10 @@ function make_dummy_object_sdk() {
         },
         read_bucket_sdk_config_info(name) {
             return bucketspace_fs.read_bucket_sdk_info({ name });
+        },
+        async read_bucket_sdk_policy_info(name) {
+            const bucket_info = await bucketspace_fs.read_bucket_sdk_info({ name });
+            return { s3_policy: bucket_info.s3_policy };
         }
     };
 }
@@ -453,20 +457,19 @@ mocha.describe('bucketspace_fs', function() {
                         Resource: ['arn:aws:s3:::*']
                         }
                     ]
-                };
-            const param = {name: test_bucket, policy: policy};
+            };
+            const param = { name: test_bucket, policy: policy };
             await bucketspace_fs.put_bucket_policy(param);
-            const policy_res = await bucketspace_fs.get_bucket_policy(param);
-            principle_unwrap(policy);
+            const policy_res = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.deepEqual(policy_res.policy, policy);
             const info_res = await bucketspace_fs.read_bucket_sdk_info(param);
-            principle_unwrap(info_res.s3_policy);
             assert.deepEqual(info_res.s3_policy, policy);
         });
+
         mocha.it('delete_bucket_policy ', async function() {
             const param = {name: test_bucket};
             await bucketspace_fs.delete_bucket_policy(param);
-            const delete_res = await bucketspace_fs.get_bucket_policy(param);
+            const delete_res = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.ok(delete_res.policy === undefined);
         });
 
@@ -482,13 +485,11 @@ mocha.describe('bucketspace_fs', function() {
                         }
                     ]
                 };
-            const param = {name: test_bucket, policy: policy};
+            const param = { name: test_bucket, policy: policy };
             await bucketspace_fs.put_bucket_policy(param);
-            const bucket_policy = await bucketspace_fs.get_bucket_policy(param);
-            principle_unwrap(policy);
+            const bucket_policy = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.deepEqual(bucket_policy.policy, policy);
             const info_res = await bucketspace_fs.read_bucket_sdk_info(param);
-            principle_unwrap(info_res.s3_policy);
             assert.deepEqual(info_res.s3_policy, policy);
         });
 
@@ -528,11 +529,9 @@ mocha.describe('bucketspace_fs', function() {
                 };
             const param = {name: test_bucket, policy: policy};
             await bucketspace_fs.put_bucket_policy(param);
-            const bucket_policy = await bucketspace_fs.get_bucket_policy(param);
-            principle_unwrap(policy);
+            const bucket_policy = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.deepEqual(bucket_policy.policy, policy);
             const info_res = await bucketspace_fs.read_bucket_sdk_info(param);
-            principle_unwrap(info_res.s3_policy);
             assert.deepEqual(info_res.s3_policy, policy);
         });
 
@@ -550,18 +549,16 @@ mocha.describe('bucketspace_fs', function() {
                 };
             const param = {name: test_bucket, policy: policy};
             await bucketspace_fs.put_bucket_policy(param);
-            const bucket_policy = await bucketspace_fs.get_bucket_policy(param);
-            principle_unwrap(policy);
+            const bucket_policy = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.deepEqual(bucket_policy.policy, policy);
             const info_res = await bucketspace_fs.read_bucket_sdk_info(param);
-            principle_unwrap(info_res.s3_policy);
             assert.deepEqual(info_res.s3_policy, policy);
         });
 
         mocha.it('delete_bucket_policy ', async function() {
-            const param = {name: test_bucket};
+            const param = { name: test_bucket };
             await bucketspace_fs.delete_bucket_policy(param);
-            const bucket_policy = await bucketspace_fs.get_bucket_policy(param);
+            const bucket_policy = await bucketspace_fs.get_bucket_policy(param, dummy_object_sdk);
             assert.ok(bucket_policy.policy === undefined);
         });
     });
@@ -585,17 +582,3 @@ function get_access_key_symlink_path(config_type_path, file_name) {
     return path.join(config_root, config_type_path, file_name + '.symlink');
 }
 
-function principle_unwrap(policy) {
-    for (const [s_index, statement] of policy.Statement.entries()) {
-        const statement_principal = statement.Principal || statement.NotPrincipal;
-        if (statement_principal.AWS) {
-            const sensitive_arr = _.flatten([statement_principal.AWS]).map(principal => principal.unwrap());
-            if (statement.Principal) policy.Statement[s_index].Principal.AWS = sensitive_arr;
-            if (statement.NotPrincipal) policy.Statement[s_index].NotPrincipal.AWS = sensitive_arr;
-        } else {
-            const sensitive_principal = statement_principal.unwrap();
-            if (statement.Principal) policy.Statement[s_index].Principal = sensitive_principal;
-            if (statement.NotPrincipal) policy.Statement[s_index].NotPrincipal = sensitive_principal;
-        }
-    }
-}

src/test/unit_tests/test_s3_bucket_policy.js

@@ -3,10 +3,14 @@
 'use strict';
 
 // setup coretest first to prepare the env
-const coretest = require('./coretest');
+const { get_coretest_path, TMP_PATH } = require('../system_tests/test_utils');
+const coretest_path = get_coretest_path();
+const coretest = require(coretest_path);
 const { rpc_client, EMAIL, POOL_LIST, anon_rpc_client } = coretest;
 const MDStore = require('../../server/object_services/md_store').MDStore;
-coretest.setup({ pools_to_create: [POOL_LIST[1]] });
+coretest.setup({ pools_to_create: process.env.NC_CORETEST ? undefined : [POOL_LIST[1]] });
+const path = require('path');
+const fs_utils = require('../../util/fs_utils');
 
 const { S3 } = require('@aws-sdk/client-s3');
 const { NodeHttpHandler } = require("@smithy/node-http-handler");
@@ -62,11 +66,30 @@ async function setup() {
             httpAgent: new http.Agent({ keepAlive: false })
         }),
     };
+    const nsr = 's3_bucket_policy_nsr';
+    const tmp_fs_root = path.join(TMP_PATH, 'test_s3_bucket_policy');
+
+    if (process.env.NC_CORETEST) {
+        await fs_utils.create_fresh_path(tmp_fs_root, 0o777);
+        await rpc_client.pool.create_namespace_resource({
+            name: nsr,
+            nsfs_config: {
+                fs_root_path: tmp_fs_root,
+            }
+        });
+    }
     const account = {
         has_login: false,
         s3_access: true,
-        default_resource: POOL_LIST[1].name
+        default_resource: process.env.NC_CORETEST ? nsr : POOL_LIST[1].name
     };
+    if (process.env.NC_CORETEST) {
+        account.nsfs_account_config = {
+            uid: process.getuid(),
+            gid: process.getgid(),
+            new_buckets_path: '/'
+        };
+    }
     const admin_keys = (await rpc_client.account.read_account({
         email: EMAIL,
     })).access_keys;
@@ -408,11 +431,12 @@ mocha.describe('s3_bucket_policy', function() {
             Bucket: BKT,
             Policy: JSON.stringify(policy)
         });
-        await s3_a.putObject({
+        const first_res = await s3_a.putObject({
             Body: 'Some data for the file... bla bla bla... version I',
             Bucket: BKT,
             Key: new_key
         });
+        const first_version_etag = first_res.ETag.replaceAll('"', '');
         await s3_b.putBucketVersioning({
             Bucket: BKT,
             VersioningConfiguration: {
@@ -433,16 +457,18 @@ mocha.describe('s3_bucket_policy', function() {
         await s3_a.deleteObject({ // delete the file versions
             Bucket: BKT,
             Key: new_key,
-            VersionId: 'nbver-' + seq
+            VersionId: process.env.NC_CORETEST ? res.VersionId : 'nbver-' + seq
         });
         await s3_a.deleteObject({
             Bucket: BKT,
             Key: new_key,
-            VersionId: 'nbver-' + (seq - 1)
+            VersionId: process.env.NC_CORETEST ? first_version_etag : 'nbver-' + (seq - 1)
         });
     });
 
     mocha.it('should deny bucket owner access', async function() {
+        // test fails on NC_CORETEST because system_owner === bucket_owner, until this behavior changes skipping the test
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const policy = {
             Version: '2012-10-17',
             Statement: [{
@@ -494,6 +520,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('anonymous user should be able to list bucket objects', async function() {
+        // anonymous not implemented on NC yet - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         await s3_owner.putBucketPolicy({
             Bucket: BKT,
             Policy: JSON.stringify(anon_access_policy)
@@ -503,6 +531,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('anonymous user should not be able to list bucket objects when there is no policy', async function() {
+        // anonymous not implemented on NC yet - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         await s3_owner.deleteBucketPolicy({
             Bucket: BKT,
         });
@@ -511,6 +541,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('anonymous user should not be able to list bucket objects when policy doesn\'t allow', async function() {
+        // anonymous not implemented on NC yet - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const anon_deny_policy = {
             Version: '2012-10-17',
             Statement: [
@@ -532,6 +564,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('[RPC TEST]: anonymous user should not be able to read_object_md when not explicitly allowed', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         // Ensure that the bucket has no policy
         await s3_owner.deleteBucketPolicy({
             Bucket: BKT,
@@ -544,6 +578,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('[RPC TEST]: anonymous user should be able to read_object_md when explicitly allowed', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const anon_read_policy = {
             Version: '2012-10-17',
             Statement: [
@@ -567,6 +603,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('[RPC TEST]: anonymous user should be able to read_object_md when explicitly allowed to access only specific key', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const anon_read_policy_2 = {
             Version: '2012-10-17',
             Statement: [
@@ -591,6 +629,8 @@ mocha.describe('s3_bucket_policy', function() {
 
 
     mocha.it('[RPC TEST]: anonymous user should not be able to read_object_mapping when not explicitly allowed', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         // Ensure that the bucket has no policy
         await s3_owner.deleteBucketPolicy({
             Bucket: BKT,
@@ -604,6 +644,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('[RPC TEST]: anonymous user should be able to read_object_mapping when explicitly allowed', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const anon_read_policy = {
             Version: '2012-10-17',
             Statement: [
@@ -628,6 +670,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('[RPC TEST]: anonymous user should be able to read_object_mapping when explicitly allowed to access only specific key', async function() {
+        // MD ops not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const anon_read_policy_2 = {
             Version: '2012-10-17',
             Statement: [
@@ -653,6 +697,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('should be able to deny based on server side encryption', async function() {
+        // SSE not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const self = this; // eslint-disable-line no-invalid-this
         self.timeout(15000);
         const auth_put_policy = {
@@ -694,6 +740,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('should be able to deny unencrypted object uploads', async function() {
+        // SSE not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const self = this; // eslint-disable-line no-invalid-this
         self.timeout(15000);
         const auth_put_policy = {
@@ -734,6 +782,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('should be able to add StringLike and StringEqualsIgnoreCase condition statements, ', async function() {
+        // SSE not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const self = this; // eslint-disable-line no-invalid-this
         self.timeout(15000);
         const ignore_case_policy = {
@@ -784,6 +834,8 @@ mocha.describe('s3_bucket_policy', function() {
     });
 
     mocha.it('should be able to deny based on object tag', async function() {
+        // Tags not implemented on NC - skipping
+        if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this
         const self = this; // eslint-disable-line no-invalid-this
         self.timeout(15000);
         const allow_tag = {key: "key", value: "allow"};