Skip to content

Commit

Permalink
NC | NSFS | IAM | Tech Debts (IAM Integration Tests, Username Validat…
Browse files Browse the repository at this point in the history
…ion Move module and Allow IAM User to Create Bucket)

1. IAM Integration Tests: add the file test_nc_iam_basic_integration.js and make the needed changes in the fiiles nc_coretest.js (add the IAM port), nc_index (add the new test in the CI) and test_utils.js (add the IAM client - like we have S3 client) - the IAM integration tests the APIs of IAM that we support today.
2. Username Validation Move the Module: we have 2 flows noobaa-cli and API (S3, IAM), and don't want to import modules between the flows and only from an above level. Therefore, I moved the function validate_username from the iam_utils to validation_utils, since it used other functions I also had to move them and move the testing file.
3. Allow IAM Users to Create Bucket - we temporarily didn't allow IAM users to create buckets.

Signed-off-by: shirady <[email protected]>
  • Loading branch information
shirady committed Jan 21, 2025
1 parent e3bbfa1 commit d759a0e
Show file tree
Hide file tree
Showing 12 changed files with 684 additions and 330 deletions.
3 changes: 2 additions & 1 deletion src/endpoint/iam/iam_rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ const RPC_ERRORS_TO_IAM = Object.freeze({
INVALID_ACCESS_KEY_ID: IamError.InvalidClientTokenId,
DEACTIVATED_ACCESS_KEY_ID: IamError.InvalidClientTokenIdInactiveAccessKey,
NO_SUCH_ACCOUNT: IamError.AccessDeniedException,
NO_SUCH_ROLE: IamError.AccessDeniedException
NO_SUCH_ROLE: IamError.AccessDeniedException,
VALIDATION_ERROR: IamError.ValidationError,
});

const ACTIONS = Object.freeze({
Expand Down
317 changes: 139 additions & 178 deletions src/endpoint/iam/iam_utils.js

Large diffs are not rendered by default.

9 changes: 4 additions & 5 deletions src/manage_nsfs/manage_nsfs_validations.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const { throw_cli_error, get_options_from_file, get_boolean_or_string_value, get
is_name_update, is_access_key_update } = 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, 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 { validate_username } = require('../util/validation_utils');

/////////////////////////////
//// GENERAL VALIDATIONS ////
Expand Down Expand Up @@ -317,15 +317,14 @@ function validate_account_name(type, action, input_options_with_data) {
try {
if (action === ACTIONS.ADD) {
account_name = String(input_options_with_data.name);
iam_utils.validate_username(account_name, 'name');
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');
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
// we replace it to ManageCLIError
const detail = err.message;
throw_cli_error(ManageCLIError.InvalidAccountName, detail);
}
Expand Down
7 changes: 1 addition & 6 deletions src/sdk/bucketspace_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,6 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
if (!sdk.requesting_account.allow_bucket_creation) {
throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets');
}
// currently we do not allow IAM account to create a bucket (temporary)
if (sdk.requesting_account.owner !== undefined) {
dbg.warn('create_bucket: account is IAM account (currently not allowed to create buckets)');
throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets');
}
if (!sdk.requesting_account.nsfs_account_config || !sdk.requesting_account.nsfs_account_config.new_buckets_path) {
throw new RpcError('MISSING_NSFS_ACCOUNT_CONFIGURATION');
}
Expand Down Expand Up @@ -345,7 +340,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
_id: mongo_utils.mongoObjectId(),
name,
tag: js_utils.default_value(tag, undefined),
owner_account: account._id,
owner_account: account.owner ? account.owner : account._id, // The account is the owner of the buckets that were created by it or by its users.
creator: account._id,
versioning: config.NSFS_VERSIONING_ENABLED && lock_enabled ? 'ENABLED' : 'DISABLED',
object_lock_configuration: config.WORM_ENABLED ? {
Expand Down
17 changes: 17 additions & 0 deletions src/test/system_tests/test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ const fs = require('fs');
const _ = require('lodash');
const path = require('path');
const http = require('http');
const https = require('https');
const P = require('../../util/promise');
const config = require('../../../config');
const { S3 } = require('@aws-sdk/client-s3');
const { IAMClient } = require('@aws-sdk/client-iam');
const os_utils = require('../../util/os_utils');
const fs_utils = require('../../util/fs_utils');
const nb_native = require('../../util/nb_native');
Expand Down Expand Up @@ -416,6 +418,20 @@ function generate_s3_client(access_key, secret_key, endpoint) {
endpoint
});
}

function generate_iam_client(access_key, secret_key, endpoint) {
const httpsAgent = new https.Agent({ rejectUnauthorized: false }); // disable SSL certificate validation
return new IAMClient({
region: config.DEFAULT_REGION,
credentials: {
accessKeyId: access_key,
secretAccessKey: secret_key,
},
endpoint,
requestHandler: new NodeHttpHandler({ httpsAgent }),
});
}

/**
* generate_nsfs_account generate an nsfs account and returns its credentials
* if the admin flag is received (in the options object) the function will not create
Expand Down Expand Up @@ -720,6 +736,7 @@ exports.empty_and_delete_buckets = empty_and_delete_buckets;
exports.disable_accounts_s3_access = disable_accounts_s3_access;
exports.generate_s3_policy = generate_s3_policy;
exports.generate_s3_client = generate_s3_client;
exports.generate_iam_client = generate_iam_client;
exports.invalid_nsfs_root_permissions = invalid_nsfs_root_permissions;
exports.get_coretest_path = get_coretest_path;
exports.exec_manage_cli = exec_manage_cli;
Expand Down
126 changes: 0 additions & 126 deletions src/test/unit_tests/jest_tests/test_iam_utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,132 +312,6 @@ describe('validate_user_input_iam', () => {
});
});

describe('validate_username', () => {
const min_length = 1;
const max_length = 64;
it('should return true when username is undefined', () => {
let dummy_username;
const res = iam_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
expect(res).toBeUndefined();
});

it('should return true when username is at the min or max length', () => {
expect(iam_utils.validate_username('a', iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
expect(iam_utils.validate_username('a'.repeat(max_length), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
});

it('should return true when username is within the length constraint', () => {
expect(iam_utils.validate_username('a'.repeat(min_length + 1), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
expect(iam_utils.validate_username('a'.repeat(max_length - 1), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
});

it('should return true when username is valid', () => {
const dummy_username = 'Robert';
const res = iam_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
expect(res).toBe(true);
});

it('should throw error when username is invalid - contains invalid character', () => {
try {
iam_utils.validate_username('{}', iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

it('should throw error when username is invalid - internal limitation (anonymous)', () => {
try {
iam_utils.validate_username('anonymous', iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

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.IAM_PARAMETER_NAME.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 = '';
iam_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.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 long', () => {
try {
const dummy_username = 'A'.repeat(max_length + 1);
iam_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

it('should throw error for invalid input types (null)', () => {
try {
// @ts-ignore
const invalid_username = null;
iam_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

it('should throw error for invalid input types (number)', () => {
try {
const invalid_username = 1;
// @ts-ignore
iam_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

it('should throw error for invalid input types (object)', () => {
try {
const invalid_username = {};
// @ts-ignore
iam_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});

it('should throw error for invalid input types (boolean)', () => {
try {
const invalid_username = false;
// @ts-ignore
iam_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(IamError);
expect(err).toHaveProperty('code', IamError.ValidationError.code);
}
});
});

describe('validate_marker', () => {
const min_length = 1;
it('should return true when marker is undefined', () => {
Expand Down
138 changes: 138 additions & 0 deletions src/test/unit_tests/jest_tests/test_validation_utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/* Copyright (C) 2024 NooBaa */
/* eslint-disable max-lines-per-function */
'use strict';
const validation_utils = require('../../../util/validation_utils');
const iam_constants = require('../../../endpoint/iam/iam_constants');
const RpcError = require('../../../rpc/rpc_error');

class NoErrorThrownError extends Error {}

describe('validate_user_input', () => {
const RPC_CODE_VALIDATION_ERROR = 'VALIDATION_ERROR';
describe('validate_username', () => {
const min_length = 1;
const max_length = 64;
it('should return true when username is undefined', () => {
let dummy_username;
const res = validation_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
expect(res).toBeUndefined();
});

it('should return true when username is at the min or max length', () => {
expect(validation_utils.validate_username('a', iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
expect(validation_utils.validate_username('a'.repeat(max_length), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
});

it('should return true when username is within the length constraint', () => {
expect(validation_utils.validate_username('a'.repeat(min_length + 1), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
expect(validation_utils.validate_username('a'.repeat(max_length - 1), iam_constants.IAM_PARAMETER_NAME.USERNAME)).toBe(true);
});

it('should return true when username is valid', () => {
const dummy_username = 'Robert';
const res = validation_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
expect(res).toBe(true);
});

it('should throw error when username is invalid - contains invalid character', () => {
try {
validation_utils.validate_username('{}', iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});


it('should throw error when username is invalid - internal limitation (anonymous)', () => {
try {
validation_utils.validate_username('anonymous', iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error when username is invalid - internal limitation (with leading or trailing spaces)', () => {
try {
validation_utils.validate_username(' name-with-spaces ', iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error when username is too short', () => {
try {
const dummy_username = '';
validation_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error when username is too long', () => {
try {
const dummy_username = 'A'.repeat(max_length + 1);
validation_utils.validate_username(dummy_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error for invalid input types (null)', () => {
try {
// @ts-ignore
const invalid_username = null;
validation_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error for invalid input types (number)', () => {
try {
const invalid_username = 1;
// @ts-ignore
validation_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error for invalid input types (object)', () => {
try {
const invalid_username = {};
// @ts-ignore
validation_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});

it('should throw error for invalid input types (boolean)', () => {
try {
const invalid_username = false;
// @ts-ignore
validation_utils.validate_username(invalid_username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
throw new NoErrorThrownError();
} catch (err) {
expect(err).toBeInstanceOf(RpcError);
expect(err.rpc_code).toBe(RPC_CODE_VALIDATION_ERROR);
}
});
});
});
Loading

0 comments on commit d759a0e

Please sign in to comment.