Merge pull request noobaa#8195 from shirady/nsfs-iam-minor-cli-changes

NC | NSFS | CLI Basic Support of IAM Accounts Configs

src/cmd/manage_nsfs.js

@@ -347,14 +347,15 @@ async function fetch_account_data(action, user_input) {
     }
 
     // override values
-    // access_key as SensitiveString
     if (has_access_keys(data.access_keys)) {
+        // access_key as SensitiveString
         data.access_keys[0].access_key = _.isUndefined(data.access_keys[0].access_key) ? undefined :
             new SensitiveString(String(data.access_keys[0].access_key));
         // secret_key as SensitiveString
         data.access_keys[0].secret_key = _.isUndefined(data.access_keys[0].secret_key) ? undefined :
             new SensitiveString(String(data.access_keys[0].secret_key));
     }
+    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;
     // new_buckets_path deletion specified with empty string ''
@@ -453,9 +454,14 @@ async function update_account(data, is_flag_iam_operate_on_root_account) {
     const new_name = data.new_name;
     const cur_access_key = has_access_keys(data.access_keys) ? data.access_keys[0].access_key : undefined;
     const update_name = new_name && cur_name && data.new_name !== cur_name;
-    const update_access_key = data.new_access_key && cur_access_key && data.new_access_key !== cur_access_key;
+    const update_access_key = data.new_access_key && cur_access_key && data.new_access_key.unwrap() !== cur_access_key.unwrap();
 
     if (!update_name && !update_access_key) {
+        if (data.new_access_key) {
+            // the user set the same access-key as was before
+            data.access_keys[0] = _.pick(data.access_keys[0], ['access_key', 'secret_key']);
+            data = _.omit(data, ['new_access_key']);
+        }
         const account_config_path = get_config_file_path(global_config.accounts_dir_path, data.name);
         const encrypted_account = await nc_mkm.encrypt_access_keys(data);
         data.master_key_id = encrypted_account.master_key_id;
@@ -473,7 +479,10 @@ async function update_account(data, is_flag_iam_operate_on_root_account) {
     const data_name = new_name || cur_name;
     data.name = data_name;
     data.email = data_name; // saved internally
-    data.access_keys[0].access_key = data.new_access_key || cur_access_key;
+    data.access_keys[0] = {
+        access_key: data.new_access_key || cur_access_key,
+        secret_key: data.access_keys[0].secret_key,
+    };
     const cur_account_config_path = get_config_file_path(global_config.accounts_dir_path, cur_name);
     const new_account_config_path = get_config_file_path(global_config.accounts_dir_path, data.name);
     const new_account_relative_config_path = get_config_file_path(global_config.accounts_dir_relative_path, data.name);

src/endpoint/iam/iam_utils.js

@@ -417,13 +417,20 @@ function validate_username(input_username, parameter_name = iam_constants.USERNA
             const { code, http_code, type } = IamError.ValidationError;
             throw new IamError({ code, message: message_with_details, http_code, type });
     }
+    // internal limitations
     const invalid_internal_names = new Set(['anonymous', '/', '.']);
     if (invalid_internal_names.has(input_username)) {
         const message_with_details = `The specified value for ${_.lowerFirst(parameter_name)} is invalid. ` +
         `Should not be one of: ${[...invalid_internal_names].join(' ').toString()}`;
         const { code, http_code, type } = IamError.ValidationError;
         throw new IamError({ code, message: message_with_details, http_code, type });
     }
+    if (input_username !== input_username.trim()) {
+        const message_with_details = `The specified value for ${_.lowerFirst(parameter_name)} is invalid. ` +
+        `Must not contain leading or trailing spaces`;
+        const { code, http_code, type } = IamError.ValidationError;
+        throw new IamError({ code, message: message_with_details, http_code, type });
+    }
     return true;
 }
 

src/manage_nsfs/manage_nsfs_cli_errors.js

@@ -208,6 +208,13 @@ ManageCLIError.AccountDeleteForbiddenHasBuckets = Object.freeze({
     http_code: 403,
 });
 
+ManageCLIError.AccountDeleteForbiddenHasIAMAccounts = Object.freeze({
+    code: 'AccountDeleteForbiddenHasIAMAccounts',
+    message: 'Cannot delete account that is owner of IAM accounts. ' +
+        'You must delete all IAM accounts before deleting the root account',
+    http_code: 403,
+});
+
 ManageCLIError.AccountCannotCreateRootAccountsRequesterIAMUser = Object.freeze({
     code: 'AccountCannotCreateRootAccounts',
     message: 'Cannot update account to have iam_operate_on_root_account. ' +
@@ -339,13 +346,20 @@ ManageCLIError.BucketAlreadyExists = Object.freeze({
     http_code: 409,
 });
 
-ManageCLIError.BucketSetForbiddenNoBucketOwner = Object.freeze({
-    code: 'BucketSetForbiddenNoBucketOwner',
+ManageCLIError.BucketSetForbiddenBucketOwnerNotExists = Object.freeze({
+    code: 'BucketSetForbiddenBucketOwnerNotExists',
     message: 'The bucket owner you set for the bucket does not exist. ' +
         'Please set the bucket owner from existing account',
     http_code: 403,
 });
 
+ManageCLIError.BucketSetForbiddenBucketOwnerIsIAMAccount = Object.freeze({
+    code: 'BucketSetForbiddenBucketOwnerIsIAMAccount',
+    message: 'The bucket owner you set for the bucket is an IAM account. ' +
+        'Please set root account as bucket owner',
+    http_code: 403,
+});
+
 ManageCLIError.BucketCreationNotAllowed = Object.freeze({
     code: 'BucketCreationNotAllowed',
     message: 'Not allowed to create new buckets',
@@ -432,7 +446,8 @@ const NSFS_CLI_ERROR_EVENT_MAP = {
     AccountNameAlreadyExists: NoobaaEvent.ACCOUNT_ALREADY_EXISTS,
     AccountDeleteForbiddenHasBuckets: NoobaaEvent.ACCOUNT_DELETE_FORBIDDEN,
     BucketAlreadyExists: NoobaaEvent.BUCKET_ALREADY_EXISTS,
-    BucketSetForbiddenNoBucketOwner: NoobaaEvent.UNAUTHORIZED,
+    BucketSetForbiddenBucketOwnerNotExists: NoobaaEvent.UNAUTHORIZED, // GAP - add event
+    BucketSetForbiddenBucketOwnerIsIAMAccount: NoobaaEvent.UNAUTHORIZED, // // GAP - add event
     LoggingExportFailed: NoobaaEvent.LOGGING_FAILED,
 };
 

src/manage_nsfs/manage_nsfs_cli_utils.js

@@ -120,7 +120,7 @@ async function get_bucket_owner_account(global_config, bucket_owner) {
     } catch (err) {
         if (err.code === 'ENOENT') {
             const detail_msg = `bucket owner ${bucket_owner} does not exists`;
-            throw_cli_error(ManageCLIError.BucketSetForbiddenNoBucketOwner, detail_msg, {bucket_owner: bucket_owner});
+            throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, {bucket_owner: bucket_owner});
         }
         throw err;
     }

src/manage_nsfs/manage_nsfs_validations.js

@@ -17,6 +17,7 @@ const { throw_cli_error, get_config_file_path, get_bucket_owner_account,
     check_root_account_owns_user, get_config_data_if_exists } = require('../manage_nsfs/manage_nsfs_cli_utils');
 const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, FROM_FILE, BOOLEAN_STRING_VALUES, BOOLEAN_STRING_OPTIONS,
     GLACIER_ACTIONS, LIST_UNSETABLE_OPTIONS, ANONYMOUS } = require('../manage_nsfs/manage_nsfs_constants');
+const iam_utils = require('../endpoint/iam/iam_utils');
 
 /////////////////////////////
 //// GENERAL VALIDATIONS ////
@@ -42,6 +43,7 @@ async function validate_input_types(type, action, argv) {
     validate_identifier(type, action, input_options_with_data, false);
     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);
     if (action === ACTIONS.UPDATE) validate_min_flags_for_update(type, input_options_with_data);
 
     // currently we use from_file only in add action
@@ -57,7 +59,8 @@ async function validate_input_types(type, action, argv) {
         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);
+        validate_flags_value_combination(type, action, input_options_with_data_from_file);
+        validate_account_name(type, action, input_options_with_data_from_file);
         return input_options_with_data_from_file;
     }
 }
@@ -259,6 +262,36 @@ function validate_flags_value_combination(type, action, input_options_with_data)
     }
 }
 
+/**
+ * validate_account_name
+ * We check the name only on new accounts (account add) or accounts' rename (account update) - 
+ * in name and new_name we allow type number, hence convert it to string (it is saved converted in fetch_account_data)
+ * In case, we had already an account with invalid name, it can be changed to a valid name
+ * (current name is not validated)
+ * @param {string} type
+ * @param {string} action
+ * @param {object} input_options_with_data
+ */
+function validate_account_name(type, action, input_options_with_data) {
+    if (type !== TYPES.ACCOUNT) return;
+    let account_name;
+    try {
+        if (action === ACTIONS.ADD) {
+            account_name = String(input_options_with_data.name);
+            iam_utils.validate_username(account_name, 'name');
+        } else if (action === ACTIONS.UPDATE && input_options_with_data.new_name !== undefined) {
+            account_name = String(input_options_with_data.new_name);
+            iam_utils.validate_username(account_name, 'new_name');
+        }
+    } catch (err) {
+        if (err instanceof ManageCLIError) throw err;
+        // we receive IAMError and replace it to ManageCLIError
+        // we do not use the mapping errors because it is a general error ValidationError
+        const detail = err.message;
+        throw_cli_error(ManageCLIError.InvalidAccountName, detail);
+    }
+}
+
 /////////////////////////////
 //// BUCKET VALIDATIONS /////
 /////////////////////////////
@@ -285,7 +318,6 @@ async function validate_bucket_args(global_config, data, action) {
     if (action === ACTIONS.ADD || action === ACTIONS.UPDATE) {
         if (action === ACTIONS.ADD) native_fs_utils.validate_bucket_creation({ name: data.name });
         if (action === ACTIONS.UPDATE && !_.isUndefined(data.new_name)) native_fs_utils.validate_bucket_creation({ name: data.new_name });
-
         if (action === ACTIONS.ADD && _.isUndefined(data.bucket_owner)) throw_cli_error(ManageCLIError.MissingBucketOwnerFlag);
         if (!data.path) throw_cli_error(ManageCLIError.MissingBucketPathFlag);
         // fs_backend='' used for deletion of the fs_backend property
@@ -314,6 +346,10 @@ async function validate_bucket_args(global_config, data, action) {
         }
             data.owner_account = account._id; // TODO move this assignment to better place
         }
+        if (account.owner) {
+            const detail_msg = `account ${data.bucket_owner} is IAM account`;
+            throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerIsIAMAccount, detail_msg, {bucket_owner: data.bucket_owner});
+        }
         if (data.s3_policy) {
             try {
                 await bucket_policy_utils.validate_s3_policy(data.s3_policy, data.name,
@@ -402,7 +438,23 @@ async function validate_account_args(global_config, data, action, is_flag_iam_op
         }
     }
     if (action === ACTIONS.DELETE) {
-        await validate_delete_account(global_config, data.name);
+        await validate_account_resources_before_deletion(global_config, data);
+    }
+}
+
+/**
+ * validate_account_resources_before_deletion will validate that the account to be deleted
+ * doesn't have resources related to it
+ * 1 - buckets that it owns
+ * 2 - accounts that it owns
+ * @param {object} global_config
+ * @param {object} data
+ */
+async function validate_account_resources_before_deletion(global_config, data) {
+    await validate_account_not_owns_buckets(global_config, data.name);
+    // If it is root account (not owned by other account) then we check that it doesn't owns IAM accounts
+    if (data.owner === undefined) {
+        await check_if_root_account_does_not_have_IAM_users(global_config, data, ACTIONS.DELETE);
     }
 }
 
@@ -437,7 +489,7 @@ function _validate_access_keys(access_key, secret_key) {
  * @param {object} global_config
  * @param {string} account_name
  */
-async function validate_delete_account(global_config, account_name) {
+async function validate_account_not_owns_buckets(global_config, account_name) {
     const fs_context = native_fs_utils.get_process_fs_context(global_config.config_root_backend);
     const entries = await nb_native().fs.readdir(fs_context, global_config.buckets_dir_path);
     await P.map_with_concurrency(10, entries, async entry => {
@@ -458,8 +510,9 @@ async function validate_delete_account(global_config, account_name) {
 /**
  * @param {object} global_config
  * @param {object} account_to_check
+ * @param {string} action
  */
-async function check_if_root_account_does_not_have_IAM_users(global_config, account_to_check) {
+async function check_if_root_account_does_not_have_IAM_users(global_config, account_to_check, action) {
     const fs_context = native_fs_utils.get_process_fs_context(global_config.config_root_backend);
     const entries = await nb_native().fs.readdir(fs_context, global_config.accounts_dir_path);
     await P.map_with_concurrency(10, entries, async entry => {
@@ -469,8 +522,12 @@ async function check_if_root_account_does_not_have_IAM_users(global_config, acco
             if (entry.name.includes(config.NSFS_TEMP_CONF_DIR_NAME)) return undefined;
             const is_root_account_owns_user = check_root_account_owns_user(account_to_check, account_data);
             if (is_root_account_owns_user) {
-                    const detail_msg = `Account ${account_to_check.name} has IAM account ${account_data.name}`;
-                    throw_cli_error(ManageCLIError.AccountCannotBeRootAccountsManager, detail_msg);
+                const detail_msg = `Account ${account_to_check.name} has IAM account ${account_data.name}`;
+                if (action === ACTIONS.DELETE) {
+                    throw_cli_error(ManageCLIError.AccountDeleteForbiddenHasIAMAccounts, detail_msg);
+                }
+                // else it is called with action ACTIONS.UPDATE
+                throw_cli_error(ManageCLIError.AccountCannotBeRootAccountsManager, detail_msg);
             }
             return account_data;
         }
@@ -488,7 +545,7 @@ async function validate_root_accounts_manager_update(global_config, account) {
     if (account.owner) {
         throw_cli_error(ManageCLIError.AccountCannotCreateRootAccountsRequesterIAMUser);
     }
-    await check_if_root_account_does_not_have_IAM_users(global_config, account);
+    await check_if_root_account_does_not_have_IAM_users(global_config, account, ACTIONS.UPDATE);
 }
 
 ///////////////////////////////////

src/test/unit_tests/jest_tests/test_iam_utils.test.js

@@ -337,7 +337,7 @@ describe('validate_user_input_iam', () => {
             }
         });
 
-        it('should throw error when username is invalid - internal limitation', () => {
+        it('should throw error when username is invalid - internal limitation (anonymous)', () => {
             try {
                 iam_utils.validate_username('anonymous', iam_constants.USERNAME);
                 throw new NoErrorThrownError();
@@ -347,6 +347,16 @@ describe('validate_user_input_iam', () => {
             }
         });
 
+        it('should throw error when username is invalid - internal limitation (with leading or trailing spaces)', () => {
+            try {
+                iam_utils.validate_username('    name-with-spaces    ', iam_constants.USERNAME);
+                throw new NoErrorThrownError();
+            } catch (err) {
+                expect(err).toBeInstanceOf(IamError);
+                expect(err).toHaveProperty('code', IamError.ValidationError.code);
+            }
+        });
+
         it('should throw error when username is too short', () => {
             try {
                 const dummy_username = '';