Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSFS | NC | CLI | Account add/update when uid = 0 and gid != 0 bug fix #7788

Merged
merged 1 commit into from
Feb 5, 2024
Merged

NSFS | NC | CLI | Account add/update when uid = 0 and gid != 0 bug fix #7788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 26 additions & 22 deletions src/cmd/manage_nsfs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ async function fetch_account_data(argv, from_file) {
// fs_backend deletion specified with empty string '' (but it is not part of the schema)
fs_backend: data.nsfs_account_config.fs_backend || undefined
},
allow_bucket_creation: !is_undefined(data.nsfs_account_config.new_buckets_path),
allow_bucket_creation: !is_string_undefined(data.nsfs_account_config.new_buckets_path),
// updates of account identifiers
new_name: data.new_name && new SensitiveString(String(data.new_name)),
new_access_key: data.new_access_key && new SensitiveString(String(data.new_access_key))
Expand All @@ -442,7 +442,7 @@ async function fetch_existing_account_data(target) {
} catch (err) {
dbg.log1('NSFS Manage command: Could not find account', target, err);
if (err.code === 'ENOENT') {
if (is_undefined(target.name)) {
if (is_string_undefined(target.name)) {
throw_cli_error(ManageCLIError.NoSuchAccountAccessKey, target.access_keys[0].access_key);
} else {
throw_cli_error(ManageCLIError.NoSuchAccountName, target.name);
Expand Down Expand Up @@ -576,13 +576,13 @@ async function get_account_status(data, show_secrets) {
await validate_account_args(data, ACTIONS.STATUS);

try {
const account_path = is_undefined(data.name) ?
const account_path = is_string_undefined(data.name) ?
get_symlink_config_file_path(access_keys_dir_path, data.access_keys[0].access_key) :
get_config_file_path(accounts_dir_path, data.name);
const config_data = await get_config_data(account_path, show_secrets);
write_stdout_response(ManageCLIResponse.AccountStatus, config_data);
} catch (err) {
if (is_undefined(data.name)) {
if (is_string_undefined(data.name)) {
throw_cli_error(ManageCLIError.NoSuchAccountAccessKey, data.access_keys[0].access_key.unwrap());
} else {
throw_cli_error(ManageCLIError.NoSuchAccountName, data.name.unwrap());
Expand Down Expand Up @@ -715,23 +715,23 @@ async function get_config_data(config_file_path, show_secrets) {
*/
async function validate_bucket_args(data, action) {
if (action === ACTIONS.DELETE || action === ACTIONS.STATUS) {
if (is_undefined(data.name)) throw_cli_error(ManageCLIError.MissingBucketNameFlag);
if (is_string_undefined(data.name)) throw_cli_error(ManageCLIError.MissingBucketNameFlag);
} else {
if (is_undefined(data.name)) throw_cli_error(ManageCLIError.MissingBucketNameFlag);
if (is_string_undefined(data.name)) throw_cli_error(ManageCLIError.MissingBucketNameFlag);
try {
native_fs_utils.validate_bucket_creation({ name: data.name.unwrap() });
} catch (err) {
throw_cli_error(ManageCLIError.InvalidBucketName, data.name.unwrap());
}
if (!is_undefined(data.new_name)) {
if (!is_string_undefined(data.new_name)) {
if (action !== ACTIONS.UPDATE) throw_cli_error(ManageCLIError.InvalidNewNameBucketIdentifier);
try {
native_fs_utils.validate_bucket_creation({ name: data.new_name.unwrap() });
} catch (err) {
throw_cli_error(ManageCLIError.InvalidBucketName, data.new_name.unwrap());
}
}
if (is_undefined(data.system_owner)) throw_cli_error(ManageCLIError.MissingBucketEmailFlag);
if (is_string_undefined(data.system_owner)) throw_cli_error(ManageCLIError.MissingBucketEmailFlag);
if (!data.path) throw_cli_error(ManageCLIError.MissingBucketPathFlag);
const fs_context = native_fs_utils.get_process_fs_context();
const exists = await native_fs_utils.is_path_exists(fs_context, data.path);
Expand Down Expand Up @@ -769,24 +769,24 @@ async function validate_bucket_args(data, action) {
*/
async function validate_account_args(data, action) {
if (action === ACTIONS.STATUS || action === ACTIONS.DELETE) {
if (is_undefined(data.access_keys[0].access_key) && is_undefined(data.name)) {
if (is_string_undefined(data.access_keys[0].access_key) && is_string_undefined(data.name)) {
throw_cli_error(ManageCLIError.MissingIdentifier);
}
} else {
if ((action !== ACTIONS.UPDATE && data.new_name)) throw_cli_error(ManageCLIError.InvalidNewNameAccountIdentifier);
if ((action !== ACTIONS.UPDATE && data.new_access_key)) throw_cli_error(ManageCLIError.InvalidNewAccessKeyIdentifier);
if (is_undefined(data.name)) throw_cli_error(ManageCLIError.MissingAccountNameFlag);
if (is_undefined(data.email)) throw_cli_error(ManageCLIError.MissingAccountEmailFlag);
if (is_string_undefined(data.name)) throw_cli_error(ManageCLIError.MissingAccountNameFlag);
if (is_string_undefined(data.email)) throw_cli_error(ManageCLIError.MissingAccountEmailFlag);

if (is_undefined(data.access_keys[0].secret_key)) throw_cli_error(ManageCLIError.MissingAccountSecretKeyFlag);
if (is_undefined(data.access_keys[0].access_key)) throw_cli_error(ManageCLIError.MissingAccountAccessKeyFlag);
if (data.nsfs_account_config.gid && (is_undefined(data.nsfs_account_config.uid))) {
romayalon marked this conversation as resolved.
Show resolved Hide resolved
if (is_string_undefined(data.access_keys[0].secret_key)) throw_cli_error(ManageCLIError.MissingAccountSecretKeyFlag);
if (is_string_undefined(data.access_keys[0].access_key)) throw_cli_error(ManageCLIError.MissingAccountAccessKeyFlag);
if (data.nsfs_account_config.gid && data.nsfs_account_config.uid === undefined) {
throw_cli_error(ManageCLIError.MissingAccountNSFSConfigUID, data.nsfs_account_config);
}
if (data.nsfs_account_config.uid && (is_undefined(data.nsfs_account_config.gid))) {
if (data.nsfs_account_config.uid && data.nsfs_account_config.gid === undefined) {
throw_cli_error(ManageCLIError.MissingAccountNSFSConfigGID, data.nsfs_account_config);
}
if ((is_undefined(data.nsfs_account_config.distinguished_name) &&
if ((is_string_undefined(data.nsfs_account_config.distinguished_name) &&
(data.nsfs_account_config.uid === undefined || data.nsfs_account_config.gid === undefined))) {
throw_cli_error(ManageCLIError.InvalidAccountNSFSConfig, data.nsfs_account_config);
}
Expand All @@ -795,7 +795,7 @@ async function validate_account_args(data, action) {
throw_cli_error(ManageCLIError.InvalidFSBackend);
}

if (is_undefined(data.nsfs_account_config.new_buckets_path)) {
if (is_string_undefined(data.nsfs_account_config.new_buckets_path)) {
return;
}
const fs_context = native_fs_utils.get_process_fs_context();
Expand Down Expand Up @@ -882,7 +882,7 @@ function validate_options_type(input_options_with_data) {
/// UTILS ///
///////////////////////////////

function is_undefined(value) {
function is_string_undefined(value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it will not be temporary as we want (as mentioned in the comment) I would consider these points:

  • If you can add a JSDoc to the function.
  • I prefer functions with verbs and change the variable name to indicate that it should only be used for names., for example:
Suggested change
function is_string_undefined(value) {
function check_string_undefined(str) {
  • Edge case - what will be when someone uses a bucket or an account with the name 0? (it still can happen, unlike a user with 0 that will be removed).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bucket and account are SensitiveStrings and on SensitiveString we check str.unwrap() === 'undefined'

if (!value) return true;
if ((value instanceof SensitiveString) && value.unwrap() === 'undefined') return true;
return false;
Expand Down Expand Up @@ -930,18 +930,22 @@ function verify_whitelist_ips(ips_to_validate) {

function _validate_access_keys(argv) {
// using the access_key flag requires also using the secret_key flag
if (!is_undefined(argv.access_key) && is_undefined(argv.secret_key)) throw_cli_error(ManageCLIError.MissingAccountSecretKeyFlag);
if (!is_undefined(argv.secret_key) && is_undefined(argv.access_key)) throw_cli_error(ManageCLIError.MissingAccountAccessKeyFlag);
if (!is_string_undefined(argv.access_key) && is_string_undefined(argv.secret_key)) {
throw_cli_error(ManageCLIError.MissingAccountSecretKeyFlag);
}
if (!is_string_undefined(argv.secret_key) && is_string_undefined(argv.access_key)) {
throw_cli_error(ManageCLIError.MissingAccountAccessKeyFlag);
}
// checking the complexity of access_key
if (!is_undefined(argv.access_key) && !string_utils.validate_complexity(argv.access_key, {
if (!is_string_undefined(argv.access_key) && !string_utils.validate_complexity(argv.access_key, {
require_length: 20,
check_uppercase: true,
check_lowercase: false,
check_numbers: true,
check_symbols: false,
})) throw_cli_error(ManageCLIError.AccountAccessKeyFlagComplexity);
// checking the complexity of secret_key
if (!is_undefined(argv.secret_key) && !string_utils.validate_complexity(argv.secret_key, {
if (!is_string_undefined(argv.secret_key) && !string_utils.validate_complexity(argv.secret_key, {
require_length: 40,
check_uppercase: true,
check_lowercase: true,
Expand Down
27 changes: 27 additions & 0 deletions src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,33 @@ describe('manage nsfs cli account flow', () => {
expect(JSON.parse(res.stdout).error.message).toBe(ManageCLIError.InvalidAccountNewBucketsPath.message);
});

it('cli account add - uid is 0, gid is not 0', async function() {
const account_name = 'uid_is_0';
const options = { name: account_name, email: `${account_name}@noobaa.com`, uid: 0, gid: 1001 };
const res = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, { config_root, ...options });
const account = JSON.parse(res).response.reply;
assert_account(account, options, false);
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, { config_root, name: account_name });
});

it('cli account add - uid is not 0, gid is 0', async function() {
const account_name = 'gid_is_0';
const options = { name: account_name, email: `${account_name}@noobaa.com`, uid: 1001, gid: 0 };
const res = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, { config_root, ...options });
const account = JSON.parse(res).response.reply;
assert_account(account, options, false);
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, { config_root, name: account_name });
});

it('cli account add - uid is 0, gid is 0', async function() {
const account_name = 'uid_gid_are_0';
const options = { name: account_name, email: `${account_name}@noobaa.com`, uid: 0, gid: 0 };
const res = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, { config_root, ...options });
const account = JSON.parse(res).response.reply;
assert_account(account, options, false);
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, { config_root, name: account_name });
});

});

describe('cli update account', () => {
Expand Down