Skip to content

Commit 924a49e

Browse files
authored
Merge pull request #9323 from shirady/iam-user-name-check
IAM | `CreateUser`, `UpdateUser` - `UserName`, `NewUserName` Check
2 parents f17a42a + 04332d1 commit 924a49e

File tree

7 files changed

+1013
-841
lines changed

7 files changed

+1013
-841
lines changed

src/endpoint/iam/iam_utils.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ function check_iam_path_was_set(iam_path) {
6262
return iam_path && iam_path !== iam_constants.IAM_DEFAULT_PATH;
6363
}
6464

65-
function get_iam_username(requested_account_name) {
66-
return requested_account_name.split(iam_constants.IAM_SPLIT_CHARACTERS)[0];
67-
}
68-
6965
/**
7066
* _create_detailed_message_for_iam_user_access_in_s3 returns a detailed message with details needed for user who
7167
* tried to perform S3 operation
@@ -77,7 +73,7 @@ function get_iam_username(requested_account_name) {
7773
function _create_detailed_message_for_iam_user_access_in_s3(user_account, method, resource_arn) {
7874
const owner_account_id = get_owner_account_id(user_account);
7975
const arn_for_requesting_account = create_arn_for_user(owner_account_id,
80-
get_iam_username(user_account.name.unwrap()), user_account.iam_path);
76+
user_account.name.unwrap(), user_account.iam_path);
8177
const full_action_name = Array.isArray(method) && method.length > 1 ? method[1] : method; // special case for get_object_attributes
8278

8379
const message_start = `User: ${arn_for_requesting_account} is not authorized to perform: ${full_action_name} `;
@@ -863,7 +859,6 @@ exports.create_arn_for_user = create_arn_for_user;
863859
exports.create_arn_for_root = create_arn_for_root;
864860
exports.get_action_message_title = get_action_message_title;
865861
exports.check_iam_path_was_set = check_iam_path_was_set;
866-
exports.get_iam_username = get_iam_username;
867862
exports.parse_max_items = parse_max_items;
868863
exports.validate_params = validate_params;
869864
exports.validate_iam_path = validate_iam_path;

src/sdk/accountspace_fs.js

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -795,15 +795,34 @@ class AccountSpaceFS {
795795
}
796796

797797
async _check_username_already_exists(action, params, requesting_account) {
798-
const owner_account_id = this._get_owner_account_argument(requesting_account, params);
798+
const owner_account_id = this._get_owner_account_argument(requesting_account);
799799
const username = params.username;
800-
const name_exists = await this.config_fs.is_account_exists_by_name(username, owner_account_id);
801-
if (name_exists) {
802-
dbg.error(`AccountSpaceFS.${action} username already exists`, username);
803-
const message_with_details = `User with name ${username} already exists.`;
804-
const { code, http_code, type } = IamError.EntityAlreadyExists;
805-
throw new IamError({ code, message: message_with_details, http_code, type });
800+
const file_name_exists = await this.config_fs.is_account_exists_by_name(username, owner_account_id);
801+
if (file_name_exists) {
802+
this._throw_error_if_account_already_exists(action, username);
803+
}
804+
const is_username_lowercase_exists_under_owner = await this._check_if_account_exists_under_the_owner(
805+
requesting_account, username);
806+
if (is_username_lowercase_exists_under_owner) {
807+
this._throw_error_if_account_already_exists(action, username);
808+
}
809+
}
810+
811+
_throw_error_if_account_already_exists(action, username) {
812+
dbg.error(`AccountSpaceFS.${action} username already exists`, username);
813+
const message_with_details = `User with name ${username} already exists.`;
814+
const { code, http_code, type } = IamError.EntityAlreadyExists;
815+
throw new IamError({ code, message: message_with_details, http_code, type });
816+
}
817+
818+
async _check_if_account_exists_under_the_owner(requesting_account, username) {
819+
const members = await this._list_config_files_for_users(requesting_account, undefined);
820+
for (const member of members) {
821+
if (member.username.toLowerCase() === username.toLowerCase()) {
822+
return true;
823+
}
806824
}
825+
return false;
807826
}
808827

809828
async _copy_data_from_requesting_account_to_account_config(action, requesting_account, params) {

src/sdk/accountspace_nb.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ class AccountSpaceNB {
3737
async create_user(params, account_sdk) {
3838

3939
const requesting_account = system_store.get_account_by_email(account_sdk.requesting_account.email);
40-
const account_name = account_util.get_account_name_from_username(params.username, requesting_account._id.toString());
40+
const account_email_wrapped = account_util.get_account_email_from_username(params.username, requesting_account._id.toString());
4141
const req = {
42-
name: account_name,
43-
email: account_name,
42+
name: params.username, // actual username saved
43+
email: account_email_wrapped, // unique email generated from username lowercase and root account id
4444
has_login: false,
4545
s3_access: true,
4646
allow_bucket_creation: true,

src/server/system_services/account_server.js

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ const check_new_azure_connection_timeout = 20 * 1000;
3737
*
3838
*/
3939
async function create_account(req) {
40-
const action = IAM_ACTIONS.CREATE_USER;
4140
let iam_arn;
4241
if (req.rpc_params.owner) {
43-
const user_name = account_util.get_iam_username(req.rpc_params.email.unwrap());
42+
const action = IAM_ACTIONS.CREATE_USER;
43+
const username = req.rpc_params.name.unwrap();
4444
account_util._check_if_requesting_account_is_root_account(action, req.account,
45-
{ username: user_name, path: req.rpc_params.iam_path });
46-
account_util._check_username_already_exists(action, req.rpc_params.email, user_name);
47-
iam_arn = iam_utils.create_arn_for_user(req.account._id.toString(), user_name,
48-
req.rpc_params.iam_path || IAM_DEFAULT_PATH);
45+
{ username: username, path: req.rpc_params.iam_path });
46+
account_util._check_username_already_exists(action, req.rpc_params.email, username);
47+
iam_arn = iam_utils.create_arn_for_user(req.account._id.toString(), username,
48+
req.rpc_params.iam_path || IAM_DEFAULT_PATH);
4949
} else {
5050
account_util.validate_create_account_permissions(req);
5151
account_util.validate_create_account_params(req);
@@ -1207,7 +1207,7 @@ async function get_user(req) {
12071207
const action = IAM_ACTIONS.GET_USER;
12081208
const requesting_account = req.account;
12091209
const requested_account = account_util.validate_and_return_requested_account(req.rpc_params, action, requesting_account);
1210-
const username = account_util.get_iam_username(req.rpc_params.username || requested_account.name.unwrap());
1210+
const username = requested_account.name.unwrap();
12111211
const iam_arn = iam_utils.create_arn_for_user(requesting_account._id.toString(), username,
12121212
requested_account.iam_path || IAM_DEFAULT_PATH);
12131213
const tags = account_util.get_sorted_list_tags_for_user(requested_account.tagging);
@@ -1228,25 +1228,32 @@ async function update_user(req) {
12281228

12291229
const action = IAM_ACTIONS.UPDATE_USER;
12301230
const requesting_account = system_store.get_account_by_email(req.account.email);
1231-
const old_username = account_util.get_account_name_from_username(req.rpc_params.username, requesting_account._id.toString());
1231+
const old_account_email_wrapped = account_util.get_account_email_from_username(
1232+
req.rpc_params.username, requesting_account._id.toString());
12321233
account_util._check_if_requesting_account_is_root_account(action, requesting_account,
12331234
{ username: req.rpc_params.username, iam_path: req.rpc_params.new_iam_path });
1234-
account_util._check_if_account_exists(action, old_username);
1235-
const requested_account = system_store.get_account_by_email(old_username);
1235+
account_util._check_if_account_exists(action, old_account_email_wrapped, req.rpc_params.username);
1236+
const requested_account = system_store.get_account_by_email(old_account_email_wrapped);
12361237
let iam_path = requested_account.iam_path;
1237-
let user_name = account_util.get_iam_username(requested_account.name.unwrap());
1238+
let user_name = req.rpc_params.username;
12381239
// Change to complete user name
1239-
const new_username = account_util.get_account_name_from_username(req.rpc_params.new_username, requesting_account._id.toString());
1240-
account_util._check_username_already_exists(action, new_username, req.rpc_params.new_username);
1240+
const is_username_update = req.rpc_params.new_username !== undefined &&
1241+
req.rpc_params.new_username !== req.rpc_params.username;
1242+
if (is_username_update) {
1243+
const new_account_email_wrapped = account_util.get_account_email_from_username(
1244+
req.rpc_params.new_username,
1245+
requesting_account._id.toString());
1246+
account_util._check_username_already_exists(action, new_account_email_wrapped, req.rpc_params.new_username);
1247+
}
12411248
account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
12421249
account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
12431250
if (req.rpc_params.new_iam_path) iam_path = req.rpc_params.new_iam_path;
12441251
if (req.rpc_params.new_username) user_name = req.rpc_params.new_username;
12451252
const iam_arn = iam_utils.create_arn_for_user(requesting_account._id.toString(), user_name, iam_path);
1246-
const new_account_name = account_util.get_account_name_from_username(user_name, requesting_account._id.toString());
1253+
const new_account_email_wrapped = account_util.get_account_email_from_username(user_name, requesting_account._id.toString());
12471254
const updates = {
1248-
name: new_account_name,
1249-
email: new_account_name,
1255+
name: user_name,
1256+
email: new_account_email_wrapped,
12501257
iam_path: iam_path,
12511258
};
12521259
await system_store.make_changes({
@@ -1270,10 +1277,10 @@ async function delete_user(req) {
12701277

12711278
const action = IAM_ACTIONS.DELETE_USER;
12721279
const requesting_account = req.account;
1273-
const username = account_util.get_account_name_from_username(req.rpc_params.username, requesting_account._id.toString());
1280+
const account_email_wrapped = account_util.get_account_email_from_username(req.rpc_params.username, requesting_account._id.toString());
12741281
account_util._check_if_requesting_account_is_root_account(action, requesting_account, { username: req.rpc_params.username });
1275-
account_util._check_if_account_exists(action, username);
1276-
const requested_account = system_store.get_account_by_email(username);
1282+
account_util._check_if_account_exists(action, account_email_wrapped, req.rpc_params.username);
1283+
const requested_account = system_store.get_account_by_email(account_email_wrapped);
12771284
account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
12781285
account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
12791286
account_util._check_if_user_does_not_have_resources_before_deletion(action, requested_account);
@@ -1295,7 +1302,7 @@ async function list_users(req) {
12951302
return owner_account_id === requesting_account._id.toString();
12961303
});
12971304
let members = _.map(requesting_account_iam_users, function(iam_user) {
1298-
const iam_username = account_util.get_iam_username(iam_user.name.unwrap());
1305+
const iam_username = iam_user.name.unwrap();
12991306
const iam_path = iam_user.iam_path || IAM_DEFAULT_PATH;
13001307
let member;
13011308
// Check the iam_path_prefix and add only those satify the iam_path if exists
@@ -1330,12 +1337,12 @@ async function create_access_key(req) {
13301337
iam_access_key = await account_util.generate_account_keys(account_req);
13311338
} catch (err) {
13321339
dbg.error(`AccountSpaceNB.${action} error: `, err);
1333-
const message_with_details = `Create accesskey failed for the user with name ${account_util.get_iam_username(requested_account.email.unwrap())}.`;
1340+
const message_with_details = `Create accesskey failed for the user with name ${requested_account.name.unwrap()}.`;
13341341
throw new RpcError('INTERNAL_FAILURE', message_with_details, 500);
13351342
}
13361343

13371344
return {
1338-
username: account_util.get_iam_username(requested_account.name.unwrap()),
1345+
username: requested_account.name.unwrap(),
13391346
access_key: iam_access_key.access_key.unwrap(),
13401347
create_date: iam_access_key.creation_date,
13411348
status: ACCESS_KEY_STATUS_ENUM.ACTIVE,
@@ -1387,16 +1394,16 @@ async function update_access_key(req) {
13871394
async function get_access_key_last_used(req) {
13881395
const action = IAM_ACTIONS.GET_ACCESS_KEY_LAST_USED;
13891396
const requesting_account = req.account;
1397+
const requested_account = account_util.validate_and_return_requested_account(req.rpc_params, action, requesting_account);
13901398
const dummy_region = 'us-west-2';
13911399
const dummy_service_name = 's3';
13921400
account_util._check_access_key_belongs_to_account(action, requesting_account, req.rpc_params.access_key);
13931401
// TODO: Need to return valid last_used_date date, Low priority.
1394-
const username = account_util._returned_username(requesting_account, requesting_account.name.unwrap(), false);
13951402
return {
13961403
region: dummy_region, // GAP
13971404
last_used_date: Date.now(), // GAP
13981405
service_name: dummy_service_name, // GAP
1399-
username: username ? account_util.get_iam_username(username) : undefined,
1406+
username: requested_account.name.unwrap(),
14001407
};
14011408
}
14021409

@@ -1425,12 +1432,12 @@ async function delete_access_key(req) {
14251432
async function tag_user(req) {
14261433
const action = IAM_ACTIONS.TAG_USER;
14271434
const requesting_account = req.account;
1428-
const username = account_util.get_account_name_from_username(req.rpc_params.username, requesting_account._id.toString());
1435+
const account_email_wrapped = account_util.get_account_email_from_username(req.rpc_params.username, requesting_account._id.toString());
14291436

14301437
account_util._check_if_requesting_account_is_root_account(action, requesting_account, { username: req.rpc_params.username });
1431-
account_util._check_if_account_exists(action, username);
1438+
account_util._check_if_account_exists(action, account_email_wrapped, req.rpc_params.username);
14321439

1433-
const requested_account = system_store.get_account_by_email(username);
1440+
const requested_account = system_store.get_account_by_email(account_email_wrapped);
14341441
account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
14351442
account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
14361443

@@ -1467,12 +1474,12 @@ async function tag_user(req) {
14671474
async function untag_user(req) {
14681475
const action = IAM_ACTIONS.UNTAG_USER;
14691476
const requesting_account = req.account;
1470-
const username = account_util.get_account_name_from_username(req.rpc_params.username, requesting_account._id.toString());
1477+
const account_email_wrapped = account_util.get_account_email_from_username(req.rpc_params.username, requesting_account._id.toString());
14711478

14721479
account_util._check_if_requesting_account_is_root_account(action, requesting_account, { username: req.rpc_params.username });
1473-
account_util._check_if_account_exists(action, username);
1480+
account_util._check_if_account_exists(action, account_email_wrapped, req.rpc_params.username);
14741481

1475-
const requested_account = system_store.get_account_by_email(username);
1482+
const requested_account = system_store.get_account_by_email(account_email_wrapped);
14761483
account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
14771484
account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
14781485

@@ -1496,12 +1503,12 @@ async function untag_user(req) {
14961503
async function list_user_tags(req) {
14971504
const action = IAM_ACTIONS.LIST_USER_TAGS;
14981505
const requesting_account = req.account;
1499-
const username = account_util.get_account_name_from_username(req.rpc_params.username, requesting_account._id.toString());
1506+
const account_email_wrapped = account_util.get_account_email_from_username(req.rpc_params.username, requesting_account._id.toString());
15001507

15011508
account_util._check_if_requesting_account_is_root_account(action, requesting_account, { username: req.rpc_params.username });
1502-
account_util._check_if_account_exists(action, username);
1509+
account_util._check_if_account_exists(action, account_email_wrapped, req.rpc_params.username);
15031510

1504-
const requested_account = system_store.get_account_by_email(username);
1511+
const requested_account = system_store.get_account_by_email(account_email_wrapped);
15051512
account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
15061513
account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
15071514

src/server/system_services/bucket_server.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ async function account_exists_by_principal_arn(principal_as_string) {
539539
} else if (arn_sufix && arn_sufix.startsWith(user_sufix)) {
540540
const arn_path_parts = principal_as_string.split('/');
541541
const iam_user_name = arn_path_parts[arn_path_parts.length - 1].trim();
542-
return system_store.get_account_by_email(new SensitiveString(`${iam_user_name}:${account_id}`));
542+
return system_store.get_account_by_email(new SensitiveString(`${iam_user_name.toLowerCase()}:${account_id}`));
543543
} //else {
544544
// wrong principal ARN should not return anything.
545545
//}

0 commit comments

Comments
 (0)