diff --git a/docs/NooBaaNonContainerized/NooBaaCLI.md b/docs/NooBaaNonContainerized/NooBaaCLI.md index 40557c3353..a9878c8333 100644 --- a/docs/NooBaaNonContainerized/NooBaaCLI.md +++ b/docs/NooBaaNonContainerized/NooBaaCLI.md @@ -84,6 +84,11 @@ noobaa-cli account add --name --uid --gid [--user] - Type: String - Description: Specifies the File system user representing the account. (user can be replaced by --uid and --gid option) +- `supplemental_groups` + - Type: Number[] + - Description: specifies additional groups(GID) this user can be a part of. allows access to reasorces assigned to + one of these groups + - `new_buckets_path` - Type: String - Description: Specifies a file system directory to be used for creating underlying directories that represent buckets created by an account using the S3 API. @@ -152,6 +157,11 @@ noobaa-cli account update --name [--new_name][--uid][--gid][--use - Type: Number - Description: Specifies the File system user representing the account. (user can be replaced by --uid and --gid option) +- `supplemental_groups` + - Type: Number[] + - Description: specifies additional groups(GID) this user can be a part of. allows access to reasorces assigned to + one of these groups + - `new_buckets_path` - Type: String - Description: Specifies a file system directory to be used for creating underlying directories that represent buckets created by an account using the S3 API. diff --git a/src/api/account_api.js b/src/api/account_api.js index 6e21f0a6e6..a91714abf4 100644 --- a/src/api/account_api.js +++ b/src/api/account_api.js @@ -312,7 +312,13 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + type: 'array', + items: { + type: 'number' + } + }, } }, }, diff --git a/src/api/common_api.js b/src/api/common_api.js index 671f520cca..4becb5c18a 100644 --- a/src/api/common_api.js +++ b/src/api/common_api.js @@ -1354,7 +1354,13 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + type: 'array', + items: { + type: 'number' + } + } } }, { type: 'object', diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js index 2f5766679c..ed2cddbb84 100644 --- a/src/cmd/manage_nsfs.js +++ b/src/cmd/manage_nsfs.js @@ -24,7 +24,7 @@ const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils'); const { TYPES, ACTIONS, LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants'); const { throw_cli_error, get_bucket_owner_account_by_name, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level, - is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils'); + is_name_update, is_access_key_update, parse_comma_delimited_string } = require('../manage_nsfs/manage_nsfs_cli_utils'); const manage_nsfs_validations = require('../manage_nsfs/manage_nsfs_validations'); const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance(); const notifications_util = require('../util/notifications_util'); @@ -358,6 +358,11 @@ async function fetch_account_data(action, user_input) { data.access_keys[0].secret_key = data.access_keys[0].secret_key === undefined ? undefined : new SensitiveString(String(data.access_keys[0].secret_key)); } + //since supplemental_groups is an array, new list will merge with the old one instead of replacing it in fetch_existing_account_data + //so we need to replace this value after merging the data + data.nsfs_account_config.supplemental_groups = _.isUndefined(user_input.supplemental_groups) ? + data.nsfs_account_config.supplemental_groups : parse_comma_delimited_string(user_input.supplemental_groups); + if (data.new_access_key) data.new_access_key = new SensitiveString(data.new_access_key); // fs_backend deletion specified with empty string '' (but it is not part of the schema) data.nsfs_account_config.fs_backend = data.nsfs_account_config.fs_backend || undefined; diff --git a/src/manage_nsfs/manage_nsfs_cli_errors.js b/src/manage_nsfs/manage_nsfs_cli_errors.js index 19ad998768..c6a4713155 100644 --- a/src/manage_nsfs/manage_nsfs_cli_errors.js +++ b/src/manage_nsfs/manage_nsfs_cli_errors.js @@ -346,6 +346,11 @@ ManageCLIError.InvalidGlacierOperation = Object.freeze({ message: 'only "migrate", "restore" and "expiry" subcommands are supported', http_code: 400, }); +ManageCLIError.InvalidSupplementalGroupsList = Object.freeze({ + code: 'InvalidSupplementalGroupsList', + message: 'supplemental groups must be a list of comma seperated gids (positive integers)', + http_code: 400, +}); //////////////////////// diff --git a/src/manage_nsfs/manage_nsfs_cli_utils.js b/src/manage_nsfs/manage_nsfs_cli_utils.js index 94ce986a90..3fd2259d4d 100644 --- a/src/manage_nsfs/manage_nsfs_cli_utils.js +++ b/src/manage_nsfs/manage_nsfs_cli_utils.js @@ -90,8 +90,18 @@ function get_boolean_or_string_value(value) { } } +function parse_comma_delimited_string(value) { + if (typeof value === 'object') { + return value; + } + if (typeof value === 'number') { + return [value]; + } + return value.split(',').map(val => Number(val)); +} + /** - * get_options_from_file will read a JSON file that include key-value of the options + * get_options_from_file will read a JSON file that include key-value of the options * (instead of flags) and return its content * @param {string} file_path */ @@ -156,6 +166,7 @@ function is_access_key_update(data) { exports.throw_cli_error = throw_cli_error; exports.write_stdout_response = write_stdout_response; exports.get_boolean_or_string_value = get_boolean_or_string_value; +exports.parse_comma_delimited_string = parse_comma_delimited_string; exports.get_bucket_owner_account_by_name = get_bucket_owner_account_by_name; exports.get_bucket_owner_account_by_id = get_bucket_owner_account_by_id; exports.get_options_from_file = get_options_from_file; diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js index 4e4ae5605a..5839deaba3 100644 --- a/src/manage_nsfs/manage_nsfs_constants.js +++ b/src/manage_nsfs/manage_nsfs_constants.js @@ -44,16 +44,16 @@ const FROM_FILE = 'from_file'; const ANONYMOUS = 'anonymous'; const VALID_OPTIONS_ACCOUNT = { - 'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', FROM_FILE, ...CLI_MUTUAL_OPTIONS]), - 'update': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', 'new_name', 'regenerate', ...CLI_MUTUAL_OPTIONS]), + 'add': new Set(['name', 'uid', 'gid', 'supplemental_groups', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', FROM_FILE, ...CLI_MUTUAL_OPTIONS]), + 'update': new Set(['name', 'uid', 'gid', 'supplemental_groups', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', 'new_name', 'regenerate', ...CLI_MUTUAL_OPTIONS]), 'delete': new Set(['name', ...CLI_MUTUAL_OPTIONS]), 'list': new Set(['wide', 'show_secrets', 'gid', 'uid', 'user', 'name', 'access_key', ...CLI_MUTUAL_OPTIONS]), 'status': new Set(['name', 'access_key', 'show_secrets', ...CLI_MUTUAL_OPTIONS]), }; const VALID_OPTIONS_ANONYMOUS_ACCOUNT = { - 'add': new Set(['uid', 'gid', 'user', 'anonymous', ...CLI_MUTUAL_OPTIONS]), - 'update': new Set(['uid', 'gid', 'user', 'anonymous', ...CLI_MUTUAL_OPTIONS]), + 'add': new Set(['uid', 'gid', 'user', 'supplemental_groups', 'anonymous', ...CLI_MUTUAL_OPTIONS]), + 'update': new Set(['uid', 'gid', 'user', 'supplemental_groups', 'anonymous', ...CLI_MUTUAL_OPTIONS]), 'delete': new Set(['anonymous', ...CLI_MUTUAL_OPTIONS]), 'status': new Set(['anonymous', ...CLI_MUTUAL_OPTIONS]), }; diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js index 2338bc8251..c204ef63ac 100644 --- a/src/manage_nsfs/manage_nsfs_help_utils.js +++ b/src/manage_nsfs/manage_nsfs_help_utils.js @@ -78,6 +78,7 @@ Flags: --name Set the name for the account --uid Set the User Identifier (UID) (UID and GID can be replaced by --user option) --gid Set the Group Identifier (GID) (UID and GID can be replaced by --user option) +--supplemental_groups (optional) Set the supplemntal group list (List of GID) seperated by comma (,) example: '212,211,202' --new_buckets_path (optional) Set the filesystem's root path where each subdirectory is a bucket --user (optional) Set the OS user name (instead of UID and GID) --access_key (optional) Set the access key for the account (default is generated) @@ -98,6 +99,7 @@ Flags: --new_name (optional) Update the account name --uid (optional) Update the User Identifier (UID) --gid (optional) Update the Group Identifier (GID) +--supplemental_groups (optional) Update the list of supplemental groups (List of GID) seperated by comma(,) (example: 211,202,23) (will override existing list) --new_buckets_path (optional) Update the filesystem's root path where each subdirectory is a bucket --user (optional) Update the OS user name (instead of uid and gid) --regenerate (optional) Update automatically generated access key and secret key @@ -303,13 +305,13 @@ But updates of the config directory will be blocked during the upgrade of the co 'upgrade start' should be executed on one node, the config directory changes will be available for all the nodes of the cluster. Usage: - + noobaa-cli upgrade start [flags] Flags: --expected_version The expected target version of the upgrade ---expected_hosts The expected hosts running NooBaa NC, a string of hosts separated by , +--expected_hosts The expected hosts running NooBaa NC, a string of hosts separated by , --skip_verification (optional) skip verification of the hosts package version WARNING: can cause corrupted config dir files created by hosts running old code --custom_upgrade_scripts_dir (optional) custom upgrade scripts dir, use for running custom config dir upgrade scripts diff --git a/src/manage_nsfs/manage_nsfs_validations.js b/src/manage_nsfs/manage_nsfs_validations.js index af837a44de..3093aad47d 100644 --- a/src/manage_nsfs/manage_nsfs_validations.js +++ b/src/manage_nsfs/manage_nsfs_validations.js @@ -15,12 +15,13 @@ const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, FROM_FILE, BOOLEAN_STRING_VA GLACIER_ACTIONS, LIST_UNSETABLE_OPTIONS, ANONYMOUS, DIAGNOSE_ACTIONS, UPGRADE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants'); const iam_utils = require('../endpoint/iam/iam_utils'); const { check_root_account_owns_user } = require('../nc/nc_utils'); +const { isNumber } = require('lodash'); ///////////////////////////// //// GENERAL VALIDATIONS //// ///////////////////////////// -/** +/** * validate_input_types checks if input option are valid. * if the the user uses from_file then the validation is on the file (in different iteration) * @param {string} type @@ -175,7 +176,11 @@ function validate_options_type_by_value(input_options_with_data) { if ((option === 'bucket_policy' || option === 'notifications') && type_of_value === 'object') { continue; } - const details = `type of flag ${option} should be ${type_of_option}`; + //special case for supplemental groups + if (option === 'supplemental_groups' && validate_supplemental_groups(value)) { + continue; + } + const details = `type of flag ${option} should be ${type_of_option} ${type_of_value} ${value}`; throw_cli_error(ManageCLIError.InvalidArgumentType, details); } } @@ -197,6 +202,31 @@ function validate_boolean_string_value(value) { return false; } +/** + * validates supplemental groups array. verifies that string is comma seperated positive numbers. string should not + * begin or end with a comma. if only one number it should be positive + * @param {number|string} value + */ +function validate_supplemental_groups(value) { + if (value && typeof value === 'string') { + const regex = /^[0-9]+(,[0-9]+)+$/; + if (!regex.test(value)) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + return true; + } else if (typeof value === 'number') { + return value > 0; + } else if (value && typeof value === 'object') { + for (const entry of Object.values(value)) { + if (isNaN(Number(entry)) || Number(entry) < 0) { + return false; + } + } + return true; + } + return false; +} + /** * validate_min_flags_for_update validates that we have at least one flag of a property to update * @param {string} type diff --git a/src/native/fs/fs_napi.cpp b/src/native/fs/fs_napi.cpp index af7bfcf137..4828e5d508 100644 --- a/src/native/fs/fs_napi.cpp +++ b/src/native/fs/fs_napi.cpp @@ -537,6 +537,55 @@ load_xattr_get_keys(Napi::Object& options, std::vector& _xattr_get_ } } +/** +* converts Napi::Array of numbers to std::vector +* typename T - type of the vector to convert to (e.g int, uint, gid_t) +*/ +template +static std::vector convert_nappi_array_to_number_vector(const Napi::Array& arr) { + std::vector new_vector; + const std::size_t arr_length = arr.Length(); + for (std::size_t i = 0; i < arr_length; ++i) { + new_vector.push_back(static_cast(arr[i]).ToNumber()); + } + return new_vector; +} + +/** + * converts std::vector to comma seperated string so it can be printed to logs + */ +template +static std::string stringfy_vector(std::vector& vec) { + std::stringstream ss; + std::size_t size = vec.size(); + for(std::size_t i = 0; i < size; ++i) { + if (i > 0) ss << ','; + ss << vec[i]; + } + return ss.str(); +} + +/** + * converts std::vector to comma seperated string so it can be printed to logs + */ +template +static std::string stringfy_array(void* arr, size_t size) { + const T* typed_arr = static_cast(arr); + std::stringstream ss; + for(std::size_t i = 0; i < size; ++i) { + if (i > 0) ss << ','; + ss << typed_arr[i]; + } + return ss.str(); +} + +static std::string get_groups_as_string() { + const size_t max_group_size = 12; // arbitary size. c does not allow variable length array + gid_t new_groups[max_group_size]; + size_t new_groups_size = getgroups(max_group_size, new_groups); + return stringfy_array(new_groups, new_groups_size); +} + /** * FSWorker is a general async worker for our fs operations */ @@ -560,6 +609,7 @@ struct FSWorker : public Napi::AsyncWorker double _took_time; Napi::FunctionReference _report_fs_stats; bool _should_add_thread_capabilities; + std::vector _supplemental_groups; // executes the ctime check in the stat and read file fuctions // NOTE: If _do_ctime_check = false, then some functions will fallback to using mtime check @@ -576,6 +626,7 @@ struct FSWorker : public Napi::AsyncWorker , _warn_threshold_ms(0) , _took_time(0) , _should_add_thread_capabilities(false) + , _supplemental_groups() , _do_ctime_check(false) { for (int i = 0; i < (int)info.Length(); ++i) _args_ref.Set(i, info[i]); @@ -586,6 +637,9 @@ struct FSWorker : public Napi::AsyncWorker if (fs_context.Get("backend").ToBoolean()) { _backend = fs_context.Get("backend").ToString(); } + if(fs_context.Has("supplemental_groups")) { + _supplemental_groups = convert_nappi_array_to_number_vector(fs_context.Get("supplemental_groups").As()); + } if (fs_context.Get("warn_threshold_ms").ToBoolean()) { _warn_threshold_ms = fs_context.Get("warn_threshold_ms").ToNumber(); } @@ -604,10 +658,12 @@ struct FSWorker : public Napi::AsyncWorker virtual void Work() = 0; void Execute() override { - DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(_backend)); + const std::string supplemental_groups = stringfy_vector(_supplemental_groups); + DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(_backend) << DVAL(supplemental_groups)); ThreadScope tx; - tx.set_user(_uid, _gid); - DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid())); + tx.set_user(_uid, _gid, _supplemental_groups); + std::string new_supplemental_groups = get_groups_as_string(); + DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid()) << DVAL(new_supplemental_groups)); if(_should_add_thread_capabilities) { tx.add_thread_capabilities(); diff --git a/src/native/util/os.h b/src/native/util/os.h index c8064e8982..7115a9a0ae 100644 --- a/src/native/util/os.h +++ b/src/native/util/os.h @@ -31,10 +31,11 @@ class ThreadScope restore_user(); } - void set_user(uid_t uid, gid_t gid) + void set_user(uid_t uid, gid_t gid, std::vector& groups) { _uid = uid; _gid = gid; + _groups = groups; change_user(); } @@ -49,6 +50,7 @@ class ThreadScope void restore_user(); uid_t _uid; gid_t _gid; + std::vector _groups; }; } // namespace noobaa diff --git a/src/native/util/os_darwin.cpp b/src/native/util/os_darwin.cpp index bb14d9c920..eeb6fe2d8f 100644 --- a/src/native/util/os_darwin.cpp +++ b/src/native/util/os_darwin.cpp @@ -3,6 +3,8 @@ #include "common.h" #include // for KAUTH_UID_NONE +#include +#include namespace noobaa { @@ -37,12 +39,25 @@ get_current_uid() const uid_t ThreadScope::orig_uid = getuid(); const gid_t ThreadScope::orig_gid = getgid(); +const std::vector ThreadScope::orig_groups = [] { + std::vector groups(NGROUPS_MAX); + int r = getgroups(NGROUPS_MAX, &groups[0]); + groups.resize(r); + return groups; +}(); + void ThreadScope::change_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(_uid, _gid)); + if (_groups.empty()) { + MUST_SYS(syscall(setgroups, 0, NULL)); + } + else { + MUST_SYS(syscall(setgroups, _groups.size(), &_groups[0])); + } } } @@ -51,6 +66,7 @@ ThreadScope::restore_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(KAUTH_UID_NONE, KAUTH_UID_NONE)); + MUST_SYS(syscall(SYS_setgroups, orig_groups.size(), &orig_groups[0])); } } diff --git a/src/native/util/os_linux.cpp b/src/native/util/os_linux.cpp index 5519db80fa..d94fb5d04c 100644 --- a/src/native/util/os_linux.cpp +++ b/src/native/util/os_linux.cpp @@ -44,8 +44,13 @@ void ThreadScope::change_user() { if (_uid != orig_uid || _gid != orig_gid) { + if (_groups.empty()) { + MUST_SYS(syscall(SYS_setgroups, 0, NULL)); + } + else { + MUST_SYS(syscall(SYS_setgroups, _groups.size(), &_groups[0])); + } // must change gid first otherwise will fail on permission - MUST_SYS(syscall(SYS_setgroups, 0, NULL)); MUST_SYS(syscall(SYS_setresgid, -1, _gid, -1)); MUST_SYS(syscall(SYS_setresuid, -1, _uid, -1)); } @@ -61,7 +66,7 @@ ThreadScope::restore_user() // must restore uid first otherwise will fail on permission MUST_SYS(syscall(SYS_setresuid, -1, orig_uid, -1)); MUST_SYS(syscall(SYS_setresgid, -1, orig_gid, -1)); - MUST_SYS(syscall(SYS_setgroups, ThreadScope::orig_groups.size(), &ThreadScope::orig_groups[0])); + MUST_SYS(syscall(SYS_setgroups, orig_groups.size(), &orig_groups[0])); } } diff --git a/src/sdk/accountspace_fs.js b/src/sdk/accountspace_fs.js index 87d3b7b723..8d5b92c913 100644 --- a/src/sdk/accountspace_fs.js +++ b/src/sdk/accountspace_fs.js @@ -503,6 +503,7 @@ class AccountSpaceFS { distinguished_name: distinguished_name, uid: distinguished_name ? undefined : requesting_account.nsfs_account_config.uid, gid: distinguished_name ? undefined : requesting_account.nsfs_account_config.gid, + supplemental_groups: requesting_account.nsfs_account_config.supplemental_groups, new_buckets_path: requesting_account.nsfs_account_config.new_buckets_path, fs_backend: requesting_account.nsfs_account_config.fs_backend, } diff --git a/src/sdk/nb.d.ts b/src/sdk/nb.d.ts index 29abd76fb4..4c25e49663 100644 --- a/src/sdk/nb.d.ts +++ b/src/sdk/nb.d.ts @@ -1031,6 +1031,7 @@ interface NativeFSContext { uid?: number; gid?: number; backend?: string; + supplemental_groups?: number[]; warn_threshold_ms?: number; report_fs_stats?: Function; do_ctime_check?: boolean; diff --git a/src/server/system_services/schemas/nsfs_account_schema.js b/src/server/system_services/schemas/nsfs_account_schema.js index 9224c3eb95..ef2fe58abd 100644 --- a/src/server/system_services/schemas/nsfs_account_schema.js +++ b/src/server/system_services/schemas/nsfs_account_schema.js @@ -81,6 +81,12 @@ module.exports = { properties: { uid: { type: 'number' }, gid: { type: 'number' }, + supplemental_groups: { + type: 'array', + items: { + type: 'number' + } + }, new_buckets_path: { type: 'string' }, fs_backend: { $ref: 'common_api#/definitions/fs_backend' diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js index 5142207789..3f1d4d1bd7 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js @@ -504,6 +504,69 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, action, account_options); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli account add - cli create account with supplemental groups)', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303,211"; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(supplemental_groups.split(',').map(val => Number(val))); + }); + + it('cli account add - cli create account with supplemental groups - single group (string))', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303"; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual([Number(supplemental_groups)]); + }); + + it('cli account add - cli create account with supplemental groups - single group (number))', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = 303; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual([supplemental_groups]); + }); + + it('cli account add - cli create account with supplemental groups - invalid list', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303l,201"; //group cannot contain charecters + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + const res = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); + + it('cli account add - cli create account with supplemental groups - invalid list end with comma', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303,"; //group cannot end with comma + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + const res = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); }); describe('cli update account', () => { @@ -1004,6 +1067,33 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, ACTIONS.UPDATE, account_options); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli account add - cli update account with supplemental groups)', async function() { + const { name } = defaults; + const supplemental_groups = "303,211"; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(supplemental_groups.split(',').map(val => Number(val))); + }); + + it('cli account add - cli update account with supplemental groups - single group (string))', async function() { + const { name } = defaults; + const supplemental_groups = "303"; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual([Number(supplemental_groups)]); + }); + + it('cli account add - cli update account with supplemental groups - single group (number))', async function() { + const { name } = defaults; + const supplemental_groups = 303; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual([supplemental_groups]); + }); }); describe('cli update account (has distinguished name)', () => { diff --git a/src/test/unit_tests/test_nsfs_access.js b/src/test/unit_tests/test_nsfs_access.js index 5353012ae6..32d3e88962 100644 --- a/src/test/unit_tests/test_nsfs_access.js +++ b/src/test/unit_tests/test_nsfs_access.js @@ -18,8 +18,10 @@ mocha.describe('new tests check', async function() { const p = '/tmp/dir/'; const root_dir = 'root_dir'; const non_root_dir = 'non_root_dir'; + const non_root_dir2 = 'non_root_dir2'; const full_path_root = path.join(p, root_dir); const full_path_non_root = path.join(p, non_root_dir); + const full_path_non_root2 = path.join(p, non_root_dir2); const ROOT_FS_CONFIG = { uid: process.getuid(), @@ -40,12 +42,22 @@ mocha.describe('new tests check', async function() { backend: '', warn_threshold_ms: 100, }; + + const NON_ROOT3_FS_CONFIG = { + uid: 1574, + gid: 1574, + backend: '', + supplemental_groups: [1572, 1577], + warn_threshold_ms: 100, + }; mocha.before(async function() { if (test_utils.invalid_nsfs_root_permissions()) this.skip(); // eslint-disable-line no-invalid-this - await fs_utils.create_fresh_path(p, 0o770); + await fs_utils.create_fresh_path(p, 0o777); await fs_utils.file_must_exist(p); await fs_utils.create_fresh_path(full_path_root, 0o770); await fs_utils.file_must_exist(full_path_root); + await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, full_path_non_root, 0o770); + await nb_native().fs.mkdir(NON_ROOT2_FS_CONFIG, full_path_non_root2, 0o770); }); @@ -66,35 +78,42 @@ mocha.describe('new tests check', async function() { } }); mocha.it('NON ROOT 1 mkdir - failure', async function() { + const new_folder = "folder1"; + const new_path = path.join(full_path_root, new_folder); try { - const mkdir_res = await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, full_path_non_root, 0o770); + const mkdir_res = await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, new_path, 0o770); assert.fail(`non root has access to mkdir under root dir ${mkdir_res}`); } catch (err) { assert.equal(err.code, 'EACCES'); } }); mocha.it('ROOT readdir - dir created by non dir - success', async function() { - try { - const root_entries = await nb_native().fs.readdir(ROOT_FS_CONFIG, full_path_non_root); - assert.fail(`root has access to a folder that should not exist - ${root_entries}`); - } catch (err) { - assert.equal(err.code, 'ENOENT'); - } + await nb_native().fs.readdir(ROOT_FS_CONFIG, full_path_non_root); }); mocha.it('NON ROOT 1 readdir - success', async function() { + const non_root_entries = await nb_native().fs.readdir(NON_ROOT1_FS_CONFIG, full_path_non_root); + assert.equal(non_root_entries && non_root_entries.length, 0); + }); + + mocha.it('NON ROOT 2 readdir - failure', async function() { try { - const non_root_entries = await nb_native().fs.readdir(NON_ROOT1_FS_CONFIG, full_path_non_root); - assert.fail(`non root 1 has access to a folder created by root with 770 perm - ${p} - ${non_root_entries}`); + const non_root_entries = await nb_native().fs.readdir(NON_ROOT2_FS_CONFIG, full_path_non_root); + assert.fail(`non root 2 has access to a folder created by root with 770 perm - ${p} ${non_root_entries}`); } catch (err) { assert.equal(err.code, 'EACCES'); } }); - mocha.it('NON ROOT 2 readdir - failure', async function() { + mocha.it('NON ROOT with suplemental group - success', async function() { + const non_root_entries = await nb_native().fs.readdir(NON_ROOT3_FS_CONFIG, full_path_non_root); + assert.equal(non_root_entries && non_root_entries.length, 0); + }); + + mocha.it('NON ROOT with different suplemental group - failure', async function() { try { - const non_root_entries = await nb_native().fs.readdir(NON_ROOT2_FS_CONFIG, full_path_non_root); - assert.fail(`non root 2 has access to a folder created by root with 770 perm - ${p} ${non_root_entries}`); + const non_root_entries = await nb_native().fs.readdir(NON_ROOT3_FS_CONFIG, full_path_non_root2); + assert.fail(`non root 3 has access to a folder created by user with gid not insupplemental groups - ${p} ${non_root_entries}`); } catch (err) { assert.equal(err.code, 'EACCES'); } diff --git a/src/util/native_fs_utils.js b/src/util/native_fs_utils.js index 2608f0fe4c..19875da816 100644 --- a/src/util/native_fs_utils.js +++ b/src/util/native_fs_utils.js @@ -520,7 +520,8 @@ async function get_fs_context(nsfs_account_config, fs_backend) { uid: (account_ids_by_dn && account_ids_by_dn.uid) ?? nsfs_account_config.uid, gid: (account_ids_by_dn && account_ids_by_dn.gid) ?? nsfs_account_config.gid, warn_threshold_ms: config.NSFS_WARN_THRESHOLD_MS, - backend: fs_backend + backend: fs_backend, + ...(nsfs_account_config.supplemental_groups && {supplemental_groups: nsfs_account_config.supplemental_groups}) }; }