From 87ed095b60ce15df98ad8f0a07c9ad86e0e7f5dc Mon Sep 17 00:00:00 2001 From: nadav mizrahi Date: Thu, 21 Nov 2024 18:17:51 +0200 Subject: [PATCH] NSFS | NC | add option to set account supplemental groups Signed-off-by: nadav mizrahi --- .../AccountsAndBuckets.md | 6 +- docs/NooBaaNonContainerized/NooBaaCLI.md | 8 + docs/NooBaaNonContainerized/S3Ops.md | 2 +- src/api/account_api.js | 5 +- src/api/common_api.js | 12 +- src/cmd/manage_nsfs.js | 7 +- src/manage_nsfs/manage_nsfs_cli_errors.js | 5 + src/manage_nsfs/manage_nsfs_cli_utils.js | 21 ++- src/manage_nsfs/manage_nsfs_constants.js | 9 +- src/manage_nsfs/manage_nsfs_help_utils.js | 21 ++- src/manage_nsfs/manage_nsfs_validations.js | 41 +++++- src/native/fs/fs_napi.cpp | 50 ++++++- src/native/util/os.h | 8 +- src/native/util/os_darwin.cpp | 30 ++++ src/native/util/os_linux.cpp | 29 ++-- src/sdk/accountspace_fs.js | 1 + src/sdk/nb.d.ts | 1 + .../schemas/nsfs_account_schema.js | 3 + .../test_nc_nsfs_account_cli.test.js | 137 ++++++++++++++++++ .../test_nc_nsfs_anonymous_cli.test.js | 74 ++++++++++ src/test/unit_tests/test_nsfs_access.js | 39 ++++- src/util/native_fs_utils.js | 8 +- 22 files changed, 475 insertions(+), 42 deletions(-) diff --git a/docs/NooBaaNonContainerized/AccountsAndBuckets.md b/docs/NooBaaNonContainerized/AccountsAndBuckets.md index 08b6462e02..4886b35b61 100644 --- a/docs/NooBaaNonContainerized/AccountsAndBuckets.md +++ b/docs/NooBaaNonContainerized/AccountsAndBuckets.md @@ -29,7 +29,11 @@ See all available account properties - [NC Account Schema](../../src/server/syst #### Important properties - `encrypted_secret_key` - Account's secrets will be kept encrypted in the account's configuration file. - - `uid/gid/user` - An account's access key is mapped to a file system uid/gid (or user). Before performing any file system operation, NooBaa switches to the account's UID/GID, ensuring that accounts access to buckets and objects is enforced by the file system. + - `uid/gid/user` - An account's access key is mapped to a file system uid/gid (or user). Before performing any file system operation, NooBaa switches to the account's UID/GID, ensuring that accounts access to buckets and objects is enforced by the file system. + + - `supplemental_groups` - In addition to the account main GID, an account can have supplementary group IDs that are used to determine permissions for accessing files. These GIDs are validated against a files group (GID) permissions. + Note: depending on the file system there may be 'sticky bit' enabled somewhere on the files path. 'sticky bit' is a user ownership access right flag that prevents other users than the file owner and root from deleting or moving files. + In that case some actions will still get access denied regardless of group permissions enabled. sticky bit is denoted by `t` at the end of the permissions list (example: `drwxrwxrwt`). see https://en.wikipedia.org/wiki/Sticky_bit - `new_buckets_path` - When an account creates a bucket using the S3 protocol, NooBaa will create the underlying file system directory. This directory will be created under new_buckets_path. Note that the account must have read and write access to its `new_buckets_path`. Must be an absolute path. diff --git a/docs/NooBaaNonContainerized/NooBaaCLI.md b/docs/NooBaaNonContainerized/NooBaaCLI.md index 40557c3353..9014a142e6 100644 --- a/docs/NooBaaNonContainerized/NooBaaCLI.md +++ b/docs/NooBaaNonContainerized/NooBaaCLI.md @@ -84,6 +84,10 @@ 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: String + - Description: Specifies additional FS groups (GID) a user can be a part of. Allows access to directories/files having one or more of the provided groups. A String of GIDs separated by commas. + - `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 +156,10 @@ 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: String + - Description: Specifies additional FS groups (GID) a user can be a part of. Allows access to directories/files having one or more of the provided groups. A String of GIDs separated by commas. + - `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/docs/NooBaaNonContainerized/S3Ops.md b/docs/NooBaaNonContainerized/S3Ops.md index 8b03c3bc24..03ecdf4ca7 100644 --- a/docs/NooBaaNonContainerized/S3Ops.md +++ b/docs/NooBaaNonContainerized/S3Ops.md @@ -52,7 +52,7 @@ The following lists describe the bucket and object operations available in NooBa - Bucket policies are an access policy option available to grant permission to buckets and objects (see [bucket policy](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucket-policies.html) in AWS documentation). You can use bucket policies to add or deny permissions for the objects in a bucket. Bucket policies can allow or deny requests based on the elements in the policy. - Bucket policies use JSON-based policy language (for more information see [basic elements in bucket policy](https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-policy-language-overview.html) in AWS documentation) - Bucket policy can be added to a bucket using the S3 API or the noobaa-cli. -- Bucket policy is an additional layer of permissions to the FS permissions (UID and GID), which means that if two accounts do not have the same permissions (UID, GID) just setting bucket policy on the bucket is not enough. +- Bucket policy is an additional layer of permissions to the FS permissions (UID, GID, supplemental GIDs), which means that if two accounts do not have the same permissions (UID, GID, supplemental GIDs) just setting bucket policy on the bucket is not enough. #### Bucket Policy in NooBaa CLI 1. Adding a bucket policy: diff --git a/src/api/account_api.js b/src/api/account_api.js index 6e21f0a6e6..cbf152e5f6 100644 --- a/src/api/account_api.js +++ b/src/api/account_api.js @@ -312,7 +312,10 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + $ref: 'common_api#/definitions/supplemental_groups' + }, } }, }, diff --git a/src/api/common_api.js b/src/api/common_api.js index 6a46718c8c..953e1b7722 100644 --- a/src/api/common_api.js +++ b/src/api/common_api.js @@ -1346,6 +1346,13 @@ module.exports = { } } }, + supplemental_groups: { + type: 'array', + items: { + type: 'integer', + 'minimum': 0 + } + }, nsfs_account_config: { oneOf: [{ type: 'object', @@ -1354,7 +1361,10 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + $ref: '#/definitions/supplemental_groups' + }, } }, { type: 'object', diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js index 0efa94a736..982fa41ef6 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 = user_input.supplemental_groups === undefined ? + 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..6ce2c27b46 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 group ids (group id is zero or a positive integer)', + 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..b005fd3ac0 100644 --- a/src/manage_nsfs/manage_nsfs_cli_utils.js +++ b/src/manage_nsfs/manage_nsfs_cli_utils.js @@ -91,7 +91,25 @@ function get_boolean_or_string_value(value) { } /** - * get_options_from_file will read a JSON file that include key-value of the options + * This function parse a comma delimited string of numbers ('0,212,111') to an array of numbers. + * This function assumes string format was validated before calling the function, wrong string format can + * lead to unexpected output (usually array of NaN) + * 1. if the value is a number return array with this number (3 => [3]) + * 2. if the value is a string return an array of numbers ('0,212,111' => [0,212,111]) + * 3. for all other types (including object and undefined) return the value itself + */ +function parse_comma_delimited_string(value) { + if (typeof value === 'number') { + return [value]; + } + if (typeof value === 'string') { + return value.split(',').map(val => Number(val)); + } + return value; +} + +/**_ + * get_options_fromfile 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 +174,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..ae3b374bf0 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]), }; @@ -105,6 +105,7 @@ const OPTION_TYPE = { owner: 'string', uid: 'number', gid: 'number', + supplemental_groups: 'string', new_buckets_path: 'string', user: 'string', access_key: 'string', diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js index d41bd9df16..13e88e0bf1 100644 --- a/src/manage_nsfs/manage_nsfs_help_utils.js +++ b/src/manage_nsfs/manage_nsfs_help_utils.js @@ -8,7 +8,7 @@ Help: "NSFS" (Namespace FileSystem) is a NooBaa system that runs a local S3 endpoint on top of a filesystem. Each subdirectory of the root filesystem represents an S3 bucket. - "noobaa-cli" will provide a command line interface (CLI) to create new accounts and map existing directories + "noobaa-cli" will provide a command line interface (CLI) to create new accounts and map existing directories to NooBaa as buckets. For more information refer to the NooBaa docs. `; @@ -111,10 +111,10 @@ Usage: account add [flags] 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 supplemental group list (List of GIDs) separated by commas (,) 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) @@ -124,7 +124,6 @@ Flags: --force_md5_etag (optional) Set the account to force md5 etag calculation. (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND) --iam_operate_on_root_account (optional) Set the account to create root accounts instead of IAM users in IAM API requests. --from_file (optional) Use details from the JSON file, there is no need to mention all the properties individually in the CLI - `; const ACCOUNT_FLAGS_UPDATE = ` @@ -142,6 +141,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 - it 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 @@ -151,7 +151,6 @@ Flags: --allow_bucket_creation (optional) Update the account to explicitly allow or block bucket creation --force_md5_etag (optional) Update the account to force md5 etag calculation (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND) --iam_operate_on_root_account (optional) Update the account to create root accounts instead of IAM users in IAM API requests. - `; const ACCOUNT_FLAGS_DELETE = ` @@ -334,7 +333,7 @@ List of actions supported: health gather-logs metrics - + `; const DIAGNOSE_HEALTH_OPTIONS = ` @@ -400,7 +399,7 @@ List of actions supported: start status history - + `; const UPGRADE_START_OPTIONS = ` @@ -413,13 +412,13 @@ Help: '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 @@ -453,7 +452,7 @@ Usage: `; -/** +/** * print_usage would print the help according to the arguments that were passed * @param {string} type * @param {string} action @@ -489,7 +488,7 @@ function print_usage(type, action) { process.exit(0); } -/** +/** * print_help_account would print the help options for account * @param {string} action */ @@ -516,7 +515,7 @@ function print_help_account(action) { process.exit(0); } -/** +/** * print_help_bucket would print the help options for bucket * @param {string} action */ diff --git a/src/manage_nsfs/manage_nsfs_validations.js b/src/manage_nsfs/manage_nsfs_validations.js index af837a44de..d711ff8b2e 100644 --- a/src/manage_nsfs/manage_nsfs_validations.js +++ b/src/manage_nsfs/manage_nsfs_validations.js @@ -20,7 +20,7 @@ const { check_root_account_owns_user } = require('../nc/nc_utils'); //// 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 @@ -41,6 +41,7 @@ async function validate_input_types(type, action, argv) { validate_flags_combination(type, action, input_options); validate_flags_value_combination(type, action, input_options_with_data); validate_account_name(type, action, input_options_with_data); + validate_supplemental_groups(input_options_with_data); if (action === ACTIONS.UPDATE) validate_min_flags_for_update(type, input_options_with_data); // currently we use from_file only in add action @@ -52,12 +53,14 @@ async function validate_input_types(type, action, argv) { const details = `${FROM_FILE} should not be passed inside json options`; throw_cli_error(ManageCLIError.InvalidArgument, details); } + console.log("NMNM"); validate_no_extra_options(type, action, input_options_from_file, true); validate_options_type_by_value(input_options_with_data_from_file); validate_identifier(type, action, input_options_with_data_from_file, true); validate_flags_combination(type, action, input_options_from_file); validate_flags_value_combination(type, action, input_options_with_data_from_file); validate_account_name(type, action, input_options_with_data_from_file); + validate_supplemental_groups(input_options_with_data_from_file); return input_options_with_data_from_file; } } @@ -175,7 +178,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' && (type_of_value === 'object') || (type_of_value === 'number')) { + continue; + } + const details = `type of flag ${option} should be ${type_of_option} (and the received value is ${value})`; throw_cli_error(ManageCLIError.InvalidArgumentType, details); } } @@ -197,6 +204,36 @@ function validate_boolean_string_value(value) { return false; } +/** + * validates supplemental groups array. + * string type: is comma seperated positive numbers. should not begin or end with a comma. + * number type: the number should be positive + * object type: should be array of possitive integers + * @param {object} input_options_with_data + */ +function validate_supplemental_groups(input_options_with_data) { + const value = input_options_with_data.supplemental_groups; + if (!value) { + return; + } + if (typeof value === 'string') { + const regex = /^[0-9]+(,[0-9]+)+$/; + if (!regex.test(value)) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + } else if (typeof value === 'number') { + if (value < 0) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + } else if (typeof value === 'object') { + for (const entry of Object.values(value)) { + if (isNaN(Number(entry)) || Number(entry) < 0) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + } + } +} + /** * 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..b5195eb5b8 100644 --- a/src/native/fs/fs_napi.cpp +++ b/src/native/fs/fs_napi.cpp @@ -537,6 +537,43 @@ 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) +* warning: function will only work on vector with numeric types. should not be used with other types +*/ +template +static std::vector +convert_napi_number_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(); +} + + +static std::string get_groups_as_string() { + std::vector groups = ThreadScope::get_process_groups(); + return stringfy_vector(groups); +} + /** * FSWorker is a general async worker for our fs operations */ @@ -560,6 +597,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 +614,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 +625,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_napi_number_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 +646,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..3178ecad8a 100644 --- a/src/native/util/os.h +++ b/src/native/util/os.h @@ -2,6 +2,8 @@ #pragma once #include +#include +#include namespace noobaa { @@ -31,10 +33,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(); } @@ -44,11 +47,14 @@ class ThreadScope const static gid_t orig_gid; const static std::vector orig_groups; + static std::vector get_process_groups(); + private: void change_user(); 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..911f4873c1 100644 --- a/src/native/util/os_darwin.cpp +++ b/src/native/util/os_darwin.cpp @@ -3,6 +3,9 @@ #include "common.h" #include // for KAUTH_UID_NONE +#include +#include +#include namespace noobaa { @@ -37,12 +40,30 @@ get_current_uid() const uid_t ThreadScope::orig_uid = getuid(); const gid_t ThreadScope::orig_gid = getgid(); +const std::vector ThreadScope::orig_groups = get_process_groups(); + +/** + * set the effective uid/gid/supplemental_groups of the current thread using pthread_getugid_np and setgroups + * we have to bypass the libc wrappers because posix requires it to syncronize + * uid, gid & supplemental_groups to all threads which is undesirable in our case. + * this way is deprecated, but we don't seem to have another way. + * pthread_setugid_np sets the effective uid/gid and clears the supplementary groups of the current thread + * See https://www.unix.com/man-page/osx/2/pthread_setugid_np/ + * regarding setgroups When called from a thread running under an assumed per-thread identity, + * this function will operate against the per-thread. so we have to run it after _mac_thread_setugid + * https://github.com/apple/darwin-xnu/blame/main/bsd/kern/kern_prot.c (see setgroups1) + */ void ThreadScope::change_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(_uid, _gid)); + if (!_groups.empty()) { + _groups.push_back(_gid); + std::swap(_groups.front(), _groups.back()); + MUST_SYS(setgroups(_groups.size(), &_groups[0])); + } } } @@ -51,6 +72,7 @@ ThreadScope::restore_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(KAUTH_UID_NONE, KAUTH_UID_NONE)); + MUST_SYS(setgroups(orig_groups.size(), &orig_groups[0])); } } @@ -62,6 +84,14 @@ ThreadScope::add_thread_capabilities() return -1; } +std::vector +ThreadScope::get_process_groups() { + std::vector groups(NGROUPS_MAX); + int r = getgroups(NGROUPS_MAX, &groups[0]); + groups.resize(r); + return groups; +} + } // namespace noobaa #endif diff --git a/src/native/util/os_linux.cpp b/src/native/util/os_linux.cpp index 5519db80fa..fdd9eb0202 100644 --- a/src/native/util/os_linux.cpp +++ b/src/native/util/os_linux.cpp @@ -27,25 +27,24 @@ 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; -}(); +const std::vector ThreadScope::orig_groups = get_process_groups(); /** - * set the effective uid/gid of the current thread using direct syscalls - * unsets the supplementary group IDs for the current thread using direct syscall + * set the effective uid/gid/supplemental_groups of the current thread using direct syscalls * we have to bypass the libc wrappers because posix requires it to syncronize - * uid & gid to all threads which is undesirable in our case. + * uid, gid & supplemental_groups to all threads which is undesirable in our case. */ 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 +60,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])); } } @@ -100,6 +99,14 @@ ThreadScope::add_thread_capabilities() { return 0; } +std::vector +ThreadScope::get_process_groups() { + std::vector groups(NGROUPS_MAX); + int r = getgroups(NGROUPS_MAX, &groups[0]); + groups.resize(r); + return groups; +} + } // namespace noobaa #endif 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 33dad4f39d..a124544134 100644 --- a/src/sdk/nb.d.ts +++ b/src/sdk/nb.d.ts @@ -1032,6 +1032,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..ecc3dcc44b 100644 --- a/src/server/system_services/schemas/nsfs_account_schema.js +++ b/src/server/system_services/schemas/nsfs_account_schema.js @@ -81,6 +81,9 @@ module.exports = { properties: { uid: { type: 'number' }, gid: { type: 'number' }, + supplemental_groups: { + $ref: 'common_api#/definitions/supplemental_groups' + }, 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..3a9187699b 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,72 @@ 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 expected_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(expected_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 expected_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(expected_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 expected_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(expected_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 +1070,36 @@ 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 expected_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(expected_groups); + }); + + it('cli account add - cli update account with supplemental groups - single group (string))', async function() { + const { name } = defaults; + const supplemental_groups = '303'; + const expected_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(expected_groups); + }); + + it('cli account add - cli update account with supplemental groups - single group (number))', async function() { + const { name } = defaults; + const supplemental_groups = 303; + const expected_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(expected_groups); + }); }); describe('cli update account (has distinguished name)', () => { @@ -1765,6 +1861,47 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, action, command_flags); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli create account using from_file with supplemental_groups', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = [212, 112]; + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + await exec_manage_cli(type, action, command_flags); + // compare the details + 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('should fail - cli create account using from_file with invalid supplemental_groups (negative number)', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = [212, -112]; // gid cannot be negative number + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + const res = await exec_manage_cli(type, action, command_flags); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); + + it('should fail - cli create account using from_file with invalid supplemental_groups (string)', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = ["212d", 112]; // gid cannot a non number type + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + const res = await exec_manage_cli(type, action, command_flags); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); }); }); diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_anonymous_cli.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_anonymous_cli.test.js index bc1f93ec08..85e796b7c6 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_anonymous_cli.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_anonymous_cli.test.js @@ -123,6 +123,58 @@ describe('manage nsfs cli anonymous account flow', () => { const resp = await exec_manage_cli(type, action, account_options); expect(JSON.parse(resp.stdout).error.message).toBe(ManageCLIError.InvalidArgument.message); }); + + it('cli create anonymous account with supplemental_groups - string', async () => { + let action = ACTIONS.DELETE; + const { type, anonymous, uid, gid } = defaults; + let account_options = { anonymous, config_root }; + const resp = await exec_manage_cli(type, action, account_options); + const res_json = JSON.parse(resp.trim()); + expect(res_json.response.code).toBe(ManageCLIResponse.AccountDeleted.code); + + action = ACTIONS.ADD; + const supplemental_groups = "0,202,101"; + const expected_groups = [0, 202, 101]; + account_options = { anonymous, config_root, uid, gid, supplemental_groups }; + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(config.ANONYMOUS_ACCOUNT_NAME); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli create anonymous account with supplemental_groups - number', async () => { + let action = ACTIONS.DELETE; + const { type, anonymous, uid, gid } = defaults; + let account_options = { anonymous, config_root }; + const resp = await exec_manage_cli(type, action, account_options); + const res_json = JSON.parse(resp.trim()); + expect(res_json.response.code).toBe(ManageCLIResponse.AccountDeleted.code); + + action = ACTIONS.ADD; + const supplemental_groups = 0; + const expected_groups = [0]; + account_options = { anonymous, config_root, uid, gid, supplemental_groups }; + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(config.ANONYMOUS_ACCOUNT_NAME); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('should fail - cli create anonymous account with invalid supplemental_groups - not a number', async () => { + const action = ACTIONS.ADD; + const { type, uid, gid, anonymous } = defaults; + const supplemental_groups = '303g,202,101'; + const account_options = { anonymous, config_root, uid, gid, supplemental_groups }; + const resp = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(resp.stdout).error.message).toBe(ManageCLIError.InvalidSupplementalGroupsList.message); + }); + + it('should fail - cli create anonymous account with invalid supplemental_groups - negative number', async () => { + const action = ACTIONS.ADD; + const { type, uid, gid, anonymous } = defaults; + const supplemental_groups = "0,-202,101"; + const account_options = { anonymous, config_root, uid, gid, supplemental_groups }; + const resp = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(resp.stdout).error.message).toBe(ManageCLIError.InvalidSupplementalGroupsList.message); + }); }); describe('cli update anonymous account', () => { @@ -179,6 +231,28 @@ describe('manage nsfs cli anonymous account flow', () => { const resp = await exec_manage_cli(type, action, account_update_options); expect(JSON.parse(resp.stdout).error.message).toBe(ManageCLIError.InvalidArgumentType.message); }); + + it('cli update anonymous account with supplemental_groups - string', async () => { + const action = ACTIONS.UPDATE; + const { type, uid, gid, anonymous } = defaults; + const supplemental_groups = '0,202,101'; + const expected_groups = [0, 202, 101]; + const account_update_options = { anonymous, config_root, uid, gid, supplemental_groups }; + await exec_manage_cli(type, action, account_update_options); + const account = await config_fs.get_account_by_name(config.ANONYMOUS_ACCOUNT_NAME); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli update anonymous account with supplemental_groups - number', async () => { + const action = ACTIONS.UPDATE; + const { type, uid, gid, anonymous } = defaults; + const supplemental_groups = 0; + const expected_groups = [0]; + const account_update_options = { anonymous, config_root, uid, gid, supplemental_groups }; + await exec_manage_cli(type, action, account_update_options); + const account = await config_fs.get_account_by_name(config.ANONYMOUS_ACCOUNT_NAME); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); }); describe('cli delete anonymous account', () => { diff --git a/src/test/unit_tests/test_nsfs_access.js b/src/test/unit_tests/test_nsfs_access.js index 5353012ae6..6f038d5e38 100644 --- a/src/test/unit_tests/test_nsfs_access.js +++ b/src/test/unit_tests/test_nsfs_access.js @@ -8,6 +8,7 @@ const assert = require('assert'); const fs_utils = require('../../util/fs_utils'); const nb_native = require('../../util/nb_native'); const test_utils = require('../system_tests/test_utils'); +const fs = require('fs'); const new_umask = process.env.NOOBAA_ENDPOINT_UMASK || 0o000; const old_umask = process.umask(new_umask); @@ -18,8 +19,11 @@ 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_root = path.join(full_path_root, non_root_dir); + const full_path_non_root1 = 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,13 +44,20 @@ 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], //gid of non-root1 and unrelated gid + 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); - }); mocha.after(async function() { @@ -99,6 +110,28 @@ mocha.describe('new tests check', async function() { assert.equal(err.code, 'EACCES'); } }); + + mocha.it('NON ROOT 3 with suplemental group - success', async function() { + await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, full_path_non_root1, 0o770); + //TODO on mac new directories are created with the parents directory GID and not with the process GID. manually change the gid + fs.promises.chown(full_path_non_root1, NON_ROOT1_FS_CONFIG.uid, NON_ROOT1_FS_CONFIG.gid); + //non root3 has non-root1 group as supplemental group, so it should succeed + const non_root_entries = await nb_native().fs.readdir(NON_ROOT3_FS_CONFIG, full_path_non_root1); + assert.equal(non_root_entries && non_root_entries.length, 0); + }); + + mocha.it('NON ROOT 3 suplemental group without the files gid - failure', async function() { + //TODO on mac new directories are created with the parents directory GID and not with the process GID. manually change the gid + fs.promises.chown(full_path_non_root1, NON_ROOT2_FS_CONFIG.uid, NON_ROOT2_FS_CONFIG.gid); + await nb_native().fs.mkdir(NON_ROOT2_FS_CONFIG, full_path_non_root2, 0o770); + try { + //non root3 doesn't have non-root2 group as supplemental group, so it should fail + 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 89dee38835..11911b93f8 100644 --- a/src/util/native_fs_utils.js +++ b/src/util/native_fs_utils.js @@ -516,12 +516,18 @@ async function get_fs_context(nsfs_account_config, fs_backend) { if (nsfs_account_config.distinguished_name) { account_ids_by_dn = await get_user_by_distinguished_name({ distinguished_name: nsfs_account_config.distinguished_name }); } - return { + const fs_context = { 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 }; + + //napi does not accepts undefined value for an array. if supplemental_groups is undefined don't include this property at all + if (nsfs_account_config.supplemental_groups) { + fs_context.supplemental_groups = nsfs_account_config.supplemental_groups; + } + return fs_context; } function validate_bucket_creation(params) {