NSFS | NC | add option to set account supplemental groups

docs/NooBaaNonContainerized/NooBaaCLI.md

@@ -84,6 +84,11 @@ noobaa-cli account add --name <account_name> --uid <uid> --gid <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 <account_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.

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'
+                                }
+                            },
                         }
                     },
                 },

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',

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;

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,
+});
 
 
 ////////////////////////

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;

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]),
 };

src/manage_nsfs/manage_nsfs_help_utils.js

@@ -78,6 +78,7 @@ Flags:
 --name <string>                                                         Set the name for the account
 --uid <number>                                                          Set the User Identifier (UID) (UID and GID can be replaced by --user option)
 --gid <number>                                                          Set the Group Identifier (GID) (UID and GID can be replaced by --user option)
+--supplemental_groups <number[]>                      (optional)        Set the supplemntal group list (List of GID) seperated by comma (,) example: '212,211,202'
 --new_buckets_path <string>                           (optional)        Set the filesystem's root path where each subdirectory is a bucket
 --user <string>                                       (optional)        Set the OS user name (instead of UID and GID)
 --access_key <string>                                 (optional)        Set the access key for the account (default is generated)
@@ -98,6 +99,7 @@ Flags:
 --new_name <string>                                   (optional)        Update the account name
 --uid <number>                                        (optional)        Update the User Identifier (UID)
 --gid <number>                                        (optional)        Update the Group Identifier (GID)
+--supplemental_groups <number[]>                      (optional)        Update the list of supplemental groups (List of GID) seperated by comma(,) (example: 211,202,23) (will override existing list)
 --new_buckets_path <string>                           (optional)        Update the filesystem's root path where each subdirectory is a bucket
 --user <string>                                       (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                             <string>                             The expected target version of the upgrade
---expected_hosts                               <string>                             The expected hosts running NooBaa NC, a string of hosts separated by , 
+--expected_hosts                               <string>                             The expected hosts running NooBaa NC, a string of hosts separated by ,
 --skip_verification                            <boolean>        (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                   <string>         (optional)          custom upgrade scripts dir, use for running custom config dir upgrade scripts

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

src/native/fs/fs_napi.cpp

@@ -537,6 +537,55 @@ load_xattr_get_keys(Napi::Object& options, std::vector<std::string>& _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<typename T>
+static std::vector<T> convert_nappi_array_to_number_vector(const Napi::Array& arr) {
+    std::vector<T> 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<Napi::Value>(arr[i]).ToNumber());
+    }
+    return new_vector;
+}
+
+/**
+ * converts std::vector to comma seperated string so it can be printed to logs
+ */
+template<typename T>
+static std::string stringfy_vector(std::vector<T>& 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<typename T>
+static std::string stringfy_array(void* arr, size_t size) {
+    const T* typed_arr = static_cast<T*>(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<gid_t>(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<gid_t> _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<gid_t>(fs_context.Get("supplemental_groups").As<Napi::Array>());
+            }
             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();

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<gid_t>& 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<gid_t> _groups;
 };
 
 } // namespace noobaa

src/native/util/os_darwin.cpp

@@ -3,6 +3,8 @@
 
 #include "common.h"
 #include <sys/kauth.h> // for KAUTH_UID_NONE
+#include <sys/param.h>
+#include <unistd.h>
 
 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<gid_t> ThreadScope::orig_groups = [] {
+    std::vector<gid_t> 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]));
     }
 }