Merge pull request #8359 from romayalon/romy-check-access-configuration

NC | Change Check Access Behavior

Author: romayalon

config.js

@@ -890,6 +890,8 @@ config.NC_MASTER_KEYS_MANAGER_REFRESH_THRESHOLD = -1; // currently we want to di
 config.MASTER_KEYS_EXEC_MAX_RETRIES = 3;
 
 config.NC_DISABLE_ACCESS_CHECK = false;
+config.NC_DISABLE_HEALTH_ACCESS_CHECK = false;
+config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK = true;
 config.NC_DISABLE_SCHEMA_CHECK = false;
 
 ////////// GPFS //////////

docs/NooBaaNonContainerized/ConfigFileCustomizations.md

@@ -356,14 +356,16 @@ Warning: After setting this configuration, NooBaa will skip schema validations a
     ```
 
 
-### 25. Disable Read/Write accessibility check -
+### 25. Disable Read accessibility check -
 * <u>Key</u>: `NC_DISABLE_ACCESS_CHECK`  
 * <u>Type</u>: Boolean  
 * <u>Default</u>: false  
-* <u>Description</u>: This flag will disable Read/Write accessibility validations in the following flows -  
-    1. Bucket creation/update - NooBaa will not validate that the bucket owner has read/write permissions to the bucket's path.  
-    2. Account creation/update - NooBaa will not validate that the account owner has read/write permissions to the account's new_buckets_path.  
-    Warning - setting this configuration to true might result with unexpected behavior.  
+* <u>Description</u>: Setting this flag to true will disable Read accessibility validations in the following flows -  
+    1. Bucket creation/update - NooBaa will not validate that the bucket owner has read permissions to the bucket's path.  
+    2. Account creation/update - NooBaa will not validate that the account owner has read permissions to the account's new_buckets_path.  
+    3. Health buckets and accounts accessibility check.
+    Warning - setting this configuration to true might result with unexpected behavior. 
+
 * <u>Steps</u>:  
     ```
     1. Open /path/to/config_dir/config.json file.
@@ -372,6 +374,38 @@ Warning: After setting this configuration, NooBaa will skip schema validations a
     "NC_DISABLE_ACCESS_CHECK": true
     ```
 
+### 26. Disable Read accessibility check on the Health CLI -
+* <u>Key</u>: `NC_DISABLE_HEALTH_ACCESS_CHECK`  
+* <u>Type</u>: Boolean  
+* <u>Default</u>: false  
+* <u>Description</u>: This flag will disable Read accessibility validations in Health check of buckets and accounts.
+
+* <u>Steps</u>:  
+    ```
+    1. Open /path/to/config_dir/config.json file.
+    2. Set the config key -
+    Example:
+    "NC_DISABLE_HEALTH_ACCESS_CHECK": true
+    ```
+
+### 27. Disable Read/Write POSIX mode bits check -
+* <u>Key</u>: `NC_DISABLE_POSIX_MODE_ACCESS_CHECK`  
+* <u>Type</u>: Boolean  
+* <u>Default</u>: true  
+* <u>Description</u>: Setting this flag to false will enable Read/Write mode bits accessibility validations by in the following flows -  
+    1. Bucket creation/update - NooBaa will validate that the bucket owner has read/write permissions to the bucket's path.  
+    2. Account creation/update - NooBaa will validate that the account owner has read/write permissions to the account's new_buckets_path.  
+    3. Health buckets and accounts accessibility check.
+    Warning - This configuration is disabled by default because it's not supporting ACLs, setting this configuration to false won't support a check of the ACLs and be based only on mode bits check.
+
+* <u>Steps</u>:  
+    ```
+    1. Open /path/to/config_dir/config.json file.
+    2. Set the config key -
+    Example:
+    "NC_DISABLE_POSIX_MODE_ACCESS_CHECK": false
+    ```
+
 
 ### 26. Set Endpoint process title -
 * <u>Key</u>: `ENDPOINT_PROCESS_TITLE`  

src/manage_nsfs/health.js

@@ -474,12 +474,14 @@ async function is_new_buckets_path_valid(config_file_path, config_data, new_buck
     }
 
     try {
-        await nb_native().fs.stat(account_fs_context, new_buckets_path);
-        const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, new_buckets_path);
-        if (!accessible) {
-            const new_err = new Error('ACCESS DENIED');
-            new_err.code = 'EACCES';
-            throw new_err;
+        if (!_should_skip_health_access_check()) {
+            await nb_native().fs.stat(account_fs_context, new_buckets_path);
+            const accessible = await native_fs_utils.is_dir_accessible(account_fs_context, new_buckets_path);
+            if (!accessible) {
+                const new_err = new Error('ACCESS DENIED');
+                new_err.code = 'EACCES';
+                throw new_err;
+            }
         }
         res_obj = get_valid_object(config_data.name, undefined, new_buckets_path);
     } catch (err) {
@@ -505,7 +507,9 @@ async function is_new_buckets_path_valid(config_file_path, config_data, new_buck
 async function is_bucket_storage_path_exists(fs_context, config_data, storage_path) {
     let res_obj;
     try {
-        await nb_native().fs.stat(fs_context, storage_path);
+        if (!_should_skip_health_access_check()) {
+            await nb_native().fs.stat(fs_context, storage_path);
+        }
         res_obj = get_valid_object(config_data.name, undefined, storage_path);
     } catch (err) {
         let err_code;
@@ -556,5 +560,13 @@ function get_invalid_object(name, config_path, storage_path, err_code) {
     };
 }
 
+/**
+ * _should_skip_access_check returns true if the health CLI should skip access check
+ * @returns {Boolean}
+ */
+function _should_skip_health_access_check() {
+    return config.NC_DISABLE_HEALTH_ACCESS_CHECK || config.NC_DISABLE_ACCESS_CHECK;
+}
+
 exports.get_health_status = get_health_status;
 exports.NSFSHealth = NSFSHealth;

src/manage_nsfs/manage_nsfs_validations.js

@@ -359,17 +359,19 @@ async function validate_bucket_args(config_fs, data, action) {
         await check_new_name_exists(TYPES.BUCKET, config_fs, action, data);
         // in case we have the fs_backend it changes the fs_context that we use for the path
         const fs_context_fs_backend = native_fs_utils.get_process_fs_context(data.fs_backend);
-        const exists = await native_fs_utils.is_path_exists(fs_context_fs_backend, data.path);
-        if (!exists) {
-            throw_cli_error(ManageCLIError.InvalidStoragePath, data.path);
+        if (!config.NC_DISABLE_ACCESS_CHECK) {
+            const exists = await native_fs_utils.is_path_exists(fs_context_fs_backend, data.path);
+            if (!exists) {
+                throw_cli_error(ManageCLIError.InvalidStoragePath, data.path);
+            }
         }
 
         // bucket owner account validations 
         const owner_account_data = await get_bucket_owner_account(config_fs, undefined, data.owner_account);
         const account_fs_context = await native_fs_utils.get_fs_context(owner_account_data.nsfs_account_config,
             owner_account_data.nsfs_account_config.fs_backend);
         if (!config.NC_DISABLE_ACCESS_CHECK) {
-            const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, data.path);
+            const accessible = await native_fs_utils.is_dir_accessible(account_fs_context, data.path);
             if (!accessible) {
                 throw_cli_error(ManageCLIError.InaccessibleStoragePath, data.path);
             }
@@ -454,13 +456,15 @@ async function validate_account_args(config_fs, data, action, is_flag_iam_operat
         }
         // in case we have the fs_backend it changes the fs_context that we use for the new_buckets_path
         const fs_context_fs_backend = native_fs_utils.get_process_fs_context(data.fs_backend);
-        const exists = await native_fs_utils.is_path_exists(fs_context_fs_backend, data.nsfs_account_config.new_buckets_path);
-        if (!exists) {
-            throw_cli_error(ManageCLIError.InvalidAccountNewBucketsPath, data.nsfs_account_config.new_buckets_path);
+        if (!config.NC_DISABLE_ACCESS_CHECK) {
+            const exists = await native_fs_utils.is_path_exists(fs_context_fs_backend, data.nsfs_account_config.new_buckets_path);
+            if (!exists) {
+                throw_cli_error(ManageCLIError.InvalidAccountNewBucketsPath, data.nsfs_account_config.new_buckets_path);
+            }
         }
         if (!config.NC_DISABLE_ACCESS_CHECK) {
             const account_fs_context = await native_fs_utils.get_fs_context(data.nsfs_account_config, data.fs_backend);
-            const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, data.nsfs_account_config.new_buckets_path);
+            const accessible = await native_fs_utils.is_dir_accessible(account_fs_context, data.nsfs_account_config.new_buckets_path);
             if (!accessible) {
                 throw_cli_error(ManageCLIError.InaccessibleAccountNewBucketsPath, data.nsfs_account_config.new_buckets_path);
             }

src/sdk/config_fs.js

@@ -161,7 +161,7 @@ class ConfigFS {
     }
 
     /**
-     * create_config_json_file created the config.json file with the configuration data
+     * create_config_json_file creates the config.json file with the configuration data
      * @param {object} data
      * @returns {Promise<void>} 
      */
@@ -178,6 +178,13 @@ class ConfigFS {
         await native_fs_utils.update_config_file(this.fs_context, this.config_root, this.config_json_path, data);
     }
 
+    /**
+     * delete_config_json_file deletes the config.json file
+     * @returns {Promise<void>} 
+     */
+    async delete_config_json_file() {
+        await native_fs_utils.delete_config_file(this.fs_context, this.config_root, this.config_json_path);
+    }
 
     /**
      * get_config_data reads a config file and returns its content 

src/server/system_services/schemas/nsfs_config_schema.js

@@ -124,7 +124,15 @@ const nsfs_node_config_schema = {
         },
         NC_DISABLE_ACCESS_CHECK: {
             type: 'boolean',
-            doc: 'indicate whether read/write access will be validated on bucket/account creation/update.'
+            doc: 'indicate whether read access will be validated on bucket/account creation/update and on the health check.'
+        },
+        NC_DISABLE_HEALTH_ACCESS_CHECK: {
+            type: 'boolean',
+            doc: 'indicate whether read access will be validated on bucket/account health check.'
+        },
+        NC_DISABLE_POSIX_MODE_ACCESS_CHECK: {
+            type: 'boolean',
+            doc: 'indicate whether posix mode read/write access will be validated on bucket/account creation/update and health check.'
         },
         ENDPOINT_PROCESS_TITLE: {
             type: 'string',

src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js

@@ -1773,7 +1773,9 @@ describe('manage nsfs cli account flow', () => {
 describe('cli account flow distinguished_name - permissions', function() {
     const type = TYPES.ACCOUNT;
     const config_root = path.join(tmp_fs_path, 'config_root_manage_dn');
+    const config_fs = new ConfigFS(config_root);
     const new_buckets_path = path.join(tmp_fs_path, 'new_buckets_path_user_dn_test/');
+
     const accounts = {
         root: {
             cli_options: {
@@ -1903,56 +1905,77 @@ describe('cli account flow distinguished_name - permissions', function() {
     }, timeout);
 
     it('cli account update - should fail - no permissions to new_buckets_path', async function() {
-        const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_perm_to_owner/`;
+        const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_perm_to_owner1/`;
         await fs_utils.create_fresh_path(no_permissions_new_buckets_path);
         await fs_utils.file_must_exist(no_permissions_new_buckets_path);
-        await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o077);
+        await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o000);
         const action = ACTIONS.UPDATE;
         const update_options = {
             config_root,
-            name: accounts.root.cli_options.name,
+            ...accounts.accessible_user.cli_options,
             new_buckets_path: no_permissions_new_buckets_path,
         };
         const res = await exec_manage_cli(type, action, update_options);
         expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleAccountNewBucketsPath.code);
     }, timeout);
 
-    it('cli account update - should fail - no write permissions of new_buckets_path', async function() {
+    it('cli account update - should fail - posix mode write permissions of new_buckets_path', async function() {
         const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_r_perm_to_owner/`;
         await fs_utils.create_fresh_path(no_permissions_new_buckets_path);
         await fs_utils.file_must_exist(no_permissions_new_buckets_path);
-        await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o477);
+        await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o477);
         const action = ACTIONS.UPDATE;
         const update_options = {
             config_root,
-            name: accounts.root.cli_options.name,
+            ...accounts.accessible_user.cli_options,
             new_buckets_path: no_permissions_new_buckets_path,
         };
+        await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false }));
         const res = await exec_manage_cli(type, action, update_options);
+        await config_fs.delete_config_json_file();
         expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleAccountNewBucketsPath.code);
     }, timeout);
 
     it('cli account update - should fail - no read permissions of new_buckets_path', async function() {
         const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_w_perm_to_owner/`;
         await fs_utils.create_fresh_path(no_permissions_new_buckets_path);
         await fs_utils.file_must_exist(no_permissions_new_buckets_path);
-        await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o277);
+        await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o000);
         const action = ACTIONS.UPDATE;
         const update_options = {
             config_root,
-            name: accounts.root.cli_options.name,
+            ...accounts.accessible_user.cli_options,
             new_buckets_path: no_permissions_new_buckets_path,
         };
         const res = await exec_manage_cli(type, action, update_options);
         expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleAccountNewBucketsPath.code);
     });
 
+    it('cli account update - should succeed - no read permissions of new_buckets_path - NC_DISABLE_ACCESS_CHECK: true', async function() {
+        const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_w_perm_to_owner/`;
+        await fs_utils.create_fresh_path(no_permissions_new_buckets_path);
+        await fs_utils.file_must_exist(no_permissions_new_buckets_path);
+
+        await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o000);
+        const action = ACTIONS.UPDATE;
+        const update_options = {
+            config_root,
+            ...accounts.accessible_user.cli_options,
+            new_buckets_path: no_permissions_new_buckets_path,
+        };
+        await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
+        const res = await exec_manage_cli(type, action, update_options);
+        await config_fs.delete_config_json_file();
+        assert_account(JSON.parse(res).response.reply, update_options, false);
+    });
+
     it('cli account create - account cant access new_bucket_path - NC_DISABLE_ACCESS_CHECK = true', async function() {
         let action = ACTIONS.ADD;
         config.NC_DISABLE_ACCESS_CHECK = true;
         set_nc_config_dir_in_config(config_root);
-        await fs.promises.writeFile(path.join(config_root, 'config.json'), JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
+        await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
         const res = await exec_manage_cli(type, action, accounts.inaccessible_user.cli_options);
+        await config_fs.delete_config_json_file();
         expect(JSON.parse(res).response.code).toEqual(ManageCLIResponse.AccountCreated.code);
         assert_account(JSON.parse(res).response.reply, { ...accounts.inaccessible_user.cli_options }, false);
         action = ACTIONS.DELETE;

src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js

@@ -113,13 +113,15 @@ describe('manage nsfs cli bucket flow', () => {
             expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code);
         });
 
-        it('should fail - cli create bucket - owner does not have write permission to path', async () => {
+        it('should fail - cli create bucket - owner does not have posix write permission to path', async () => {
             const action = ACTIONS.ADD;
             const bucket_options = { config_root, ...bucket_defaults};
             await fs_utils.create_fresh_path(bucket_options.path);
             await fs_utils.file_must_exist(bucket_options.path);
             await set_path_permissions_and_owner(bucket_options.path, account_defaults, 0o477);
+            await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false }));
             const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
+            await config_fs.delete_config_json_file();
             expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code);
         });
 
@@ -150,9 +152,10 @@ describe('manage nsfs cli bucket flow', () => {
             await fs_utils.create_fresh_path(bucket_options.path);
             await fs_utils.file_must_exist(bucket_options.path);
             set_nc_config_dir_in_config(config_root);
-            await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
             await set_path_permissions_and_owner(bucket_options.path, account_defaults, 0o000);
+            await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
             const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
+            await config_fs.delete_config_json_file();
             expect(JSON.parse(res).response.code).toEqual(ManageCLIResponse.BucketCreated.code);
         });
 
@@ -532,7 +535,9 @@ describe('manage nsfs cli bucket flow', () => {
             await fs_utils.create_fresh_path(bucket_defaults.path);
             await fs_utils.file_must_exist(bucket_defaults.path);
             await set_path_permissions_and_owner(bucket_defaults.path, account_defaults2, 0o477);
+            await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false }));
             const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
+            await config_fs.delete_config_json_file();
             expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code);
         });
 
@@ -546,6 +551,18 @@ describe('manage nsfs cli bucket flow', () => {
             expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code);
         });
 
+        it('cli update bucket owner - owner does not have read permission to path - NC_DISABLE_ACCESS_CHECK: true', async () => {
+            const action = ACTIONS.UPDATE;
+            const bucket_options = { config_root, name: bucket_defaults.name, owner: account_defaults2.name};
+            await fs_utils.create_fresh_path(bucket_defaults.path);
+            await fs_utils.file_must_exist(bucket_defaults.path);
+            await set_path_permissions_and_owner(bucket_defaults.path, account_defaults2, 0o000);
+            await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
+            const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
+            await config_fs.delete_config_json_file();
+            expect(JSON.parse(res).response.code).toEqual(ManageCLIResponse.BucketUpdated.code);
+        });
+
         it('cli update bucket owner - account can access path', async () => {
             const action = ACTIONS.UPDATE;
             const bucket_options = { config_root, name: bucket_defaults.name, owner: account_defaults2.name};