diff --git a/src/api/account_api.js b/src/api/account_api.js index 6e21f0a6e6..b9726764ea 100644 --- a/src/api/account_api.js +++ b/src/api/account_api.js @@ -312,7 +312,8 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { type: 'string'}, } }, }, diff --git a/src/api/common_api.js b/src/api/common_api.js index 454f14811a..42657c0916 100644 --- a/src/api/common_api.js +++ b/src/api/common_api.js @@ -1310,7 +1310,8 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { type: 'string'}, } }, { type: 'object', diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js index 2f5766679c..878084c0a8 100644 --- a/src/cmd/manage_nsfs.js +++ b/src/cmd/manage_nsfs.js @@ -338,6 +338,7 @@ async function fetch_account_data(action, user_input) { distinguished_name: user_input.user, uid: user_input.user ? undefined : user_input.uid, gid: user_input.user ? undefined : user_input.gid, + supplemental_groups: _.isUndefined(user_input.supplemental_groups) ? undefined : String(user_input.supplemental_groups), new_buckets_path: user_input.new_buckets_path, fs_backend: user_input.fs_backend ? String(user_input.fs_backend) : config.NSFS_NC_STORAGE_BACKEND } diff --git a/src/manage_nsfs/manage_nsfs_cli_errors.js b/src/manage_nsfs/manage_nsfs_cli_errors.js index 19ad998768..45634547ce 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 numbers', + 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..9bd06bc0cb 100644 --- a/src/manage_nsfs/manage_nsfs_cli_utils.js +++ b/src/manage_nsfs/manage_nsfs_cli_utils.js @@ -91,7 +91,7 @@ function get_boolean_or_string_value(value) { } /** - * 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 */ diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js index 4e4ae5605a..6cbef8c619 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', 'supplemental_groups', 'user', 'anonymous', ...CLI_MUTUAL_OPTIONS]), + 'update': new Set(['uid', 'gid', 'supplemental_groups', 'user', '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 16a888ca5c..b0ea21eaa0 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 (,) --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('). 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..4ac2650c5d 100644 --- a/src/manage_nsfs/manage_nsfs_validations.js +++ b/src/manage_nsfs/manage_nsfs_validations.js @@ -175,7 +175,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 +201,19 @@ function validate_boolean_string_value(value) { return false; } +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 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..5186704e02 100644 --- a/src/native/fs/fs_napi.cpp +++ b/src/native/fs/fs_napi.cpp @@ -537,6 +537,17 @@ load_xattr_get_keys(Napi::Object& options, std::vector& _xattr_get_ } } +template +static std::vector convert_delimited_string_to_number_vector(const std::string str, const char delimiter) { + std::vector new_vector; + std::stringstream sstream(str); + std::string item; + while (std::getline(sstream, item, delimiter)) { + new_vector.push_back(std::stoi(item)); + } + return new_vector; +} + /** * FSWorker is a general async worker for our fs operations */ @@ -560,6 +571,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 +588,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 +599,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_delimited_string_to_number_vector(fs_context.Get("supplemental_groups").ToString(), ','); + } if (fs_context.Get("warn_threshold_ms").ToBoolean()) { _warn_threshold_ms = fs_context.Get("warn_threshold_ms").ToNumber(); } @@ -606,7 +622,7 @@ struct FSWorker : public Napi::AsyncWorker { DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(_backend)); ThreadScope tx; - tx.set_user(_uid, _gid); + tx.set_user(_uid, _gid, &_supplemental_groups); DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid())); if(_should_add_thread_capabilities) { diff --git a/src/native/util/os.h b/src/native/util/os.h index c8064e8982..7e5755077b 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_linux.cpp b/src/native/util/os_linux.cpp index 5519db80fa..920265fa47 100644 --- a/src/native/util/os_linux.cpp +++ b/src/native/util/os_linux.cpp @@ -43,9 +43,14 @@ const std::vector ThreadScope::orig_groups = [] { void ThreadScope::change_user() { + if (_groups == NULL || _groups->empty()) { + MUST_SYS(syscall(SYS_setgroups, 0, NULL)); + } + else if (_groups != NULL && *_groups != orig_groups) { + MUST_SYS(syscall(SYS_setgroups, _groups->size(), &_groups->at(0))); + } if (_uid != orig_uid || _gid != orig_gid) { // 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,9 @@ 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])); + if(_groups == NULL || orig_groups != *_groups) { + MUST_SYS(syscall(SYS_setgroups, orig_groups.size(), &orig_groups[0])); + } } } diff --git a/src/sdk/nb.d.ts b/src/sdk/nb.d.ts index a98071294a..4ab355277a 100644 --- a/src/sdk/nb.d.ts +++ b/src/sdk/nb.d.ts @@ -1027,6 +1027,7 @@ interface NativeFSContext { uid?: number; gid?: number; backend?: string; + supplemental_groups?: string; 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..30fcfc9d41 100644 --- a/src/server/system_services/schemas/nsfs_account_schema.js +++ b/src/server/system_services/schemas/nsfs_account_schema.js @@ -81,6 +81,7 @@ module.exports = { properties: { uid: { type: 'number' }, gid: { type: 'number' }, + supplemental_groups: { type: 'string' }, 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..568c69db0a 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).toBe(supplemental_groups); + }); + + 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).toBe(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).toBe(supplemental_groups.toString()); + }); + + 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).toBe(supplemental_groups); + }); + + 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).toBe(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).toBe(supplemental_groups.toString()); + }); }); 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..77f365d7d5 100644 --- a/src/test/unit_tests/test_nsfs_access.js +++ b/src/test/unit_tests/test_nsfs_access.js @@ -40,12 +40,21 @@ 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", + 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); }); @@ -66,29 +75,22 @@ 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() { - 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}`); - } catch (err) { - assert.equal(err.code, 'EACCES'); - } + 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() { @@ -99,6 +101,11 @@ mocha.describe('new tests check', async function() { assert.equal(err.code, 'EACCES'); } }); + + mocha.it('NON ROOT with suplemental group', 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); + }); });