Skip to content

Commit 2a4e196

Browse files
shiradyshirady
authored
Merge pull request #9274 from shirady/iam-user-policy-document-validations
IAM | User Inline Policy Document Additional Validation
2 parents 2017b80 + 860a611 commit 2a4e196

File tree

4 files changed

+312
-236
lines changed

4 files changed

+312
-236
lines changed

‎src/endpoint/iam/iam_utils.js‎

Lines changed: 112 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
const _ = require('lodash');
55
const s3_utils = require('../s3/s3_utils');
66
const { IamError } = require('./iam_errors');
7-
const { AWS_IAM_PATH_REGEXP, AWS_IAM_LIST_MARKER, AWS_IAM_ACCESS_KEY_INPUT_REGEXP } = require('../../util/string_utils');
7+
const { AWS_IAM_PATH_REGEXP, AWS_IAM_LIST_MARKER, AWS_IAM_ACCESS_KEY_INPUT_REGEXP,
8+
AWS_POLICY_NAME_REGEXP, AWS_POLICY_DOCUMENT_REGEXP } = require('../../util/string_utils');
89
const iam_constants = require('./iam_constants');
910
const { RpcError } = require('../../rpc');
1011
const validation_utils = require('../../util/validation_utils');
@@ -398,9 +399,9 @@ function validate_put_user_policy(params) {
398399
check_required_username(params);
399400
validation_utils.validate_username(params.username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
400401
check_required_policy_name(params);
401-
validation_utils.validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
402+
validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
402403
check_required_policy_document(params);
403-
validation_utils.validate_policy_document(params.policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
404+
validate_policy_document(params.policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
404405
} catch (err) {
405406
translate_rpc_error(err);
406407
}
@@ -415,7 +416,7 @@ function validate_get_user_policy(params) {
415416
check_required_username(params);
416417
validation_utils.validate_username(params.username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
417418
check_required_policy_name(params);
418-
validation_utils.validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
419+
validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
419420
} catch (err) {
420421
translate_rpc_error(err);
421422
}
@@ -430,7 +431,7 @@ function validate_delete_user_policy(params) {
430431
check_required_username(params);
431432
validation_utils.validate_username(params.username, iam_constants.IAM_PARAMETER_NAME.USERNAME);
432433
check_required_policy_name(params);
433-
validation_utils.validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
434+
validate_policy_name(params.policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
434435
} catch (err) {
435436
translate_rpc_error(err);
436437
}
@@ -593,6 +594,110 @@ function validate_status(input_status) {
593594
return true;
594595
}
595596

597+
/**
598+
* validate_policy_document will validate:
599+
* 1. type
600+
* 2. length
601+
* 3. regex (from AWS docs)
602+
* 4. valid JSON (like we do in bucket policy)
603+
* 5. structure - basic (currently - only that we don't have Principal NotPrincipal)
604+
* @param {string} input_policy_document
605+
* @param {string} parameter_name
606+
*/
607+
function validate_policy_document(input_policy_document, parameter_name = iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT) {
608+
try {
609+
if (input_policy_document === undefined) return;
610+
// type check
611+
validation_utils._type_check_input('string', input_policy_document, parameter_name);
612+
// length check
613+
const min_length = 1;
614+
const max_length = 131072;
615+
const input_length = input_policy_document.length;
616+
const is_valid_policy_document_length = input_length >= min_length && input_length <= max_length;
617+
// regex check
618+
const is_valid_policy_document = AWS_POLICY_DOCUMENT_REGEXP.test(input_policy_document);
619+
if (!is_valid_policy_document_length || !is_valid_policy_document) {
620+
const { code, http_code, type } = IamError.MalformedPolicyDocument;
621+
const message_with_details = 'Syntax errors in policy.';
622+
throw new IamError({ code, message: message_with_details, http_code, type });
623+
}
624+
// valid JSON check
625+
const policy_document = _validate_json_policy_document(input_policy_document);
626+
_validate_policy_document_iam_structure(policy_document);
627+
return true;
628+
} catch (err) {
629+
translate_rpc_error(err);
630+
}
631+
}
632+
633+
/**
634+
* validate_policy_name will validate:
635+
* 1. type
636+
* 2. length
637+
* 3. regex (from AWS docs)
638+
* @param {string} input_policy_name
639+
* @param {string} parameter_name
640+
*/
641+
function validate_policy_name(input_policy_name, parameter_name = iam_constants.IAM_PARAMETER_NAME.POLICY_NAME) {
642+
try {
643+
if (input_policy_name === undefined) return;
644+
// type check
645+
validation_utils._type_check_input('string', input_policy_name, parameter_name);
646+
// length check
647+
const min_length = 1;
648+
const max_length = 128;
649+
validation_utils._length_check_input(min_length, max_length, input_policy_name, parameter_name);
650+
// regex check
651+
const is_valid_policy_name = AWS_POLICY_NAME_REGEXP.test(input_policy_name);
652+
if (!is_valid_policy_name) {
653+
const message_with_details = `The specified value for ${_.lowerFirst(parameter_name)} is invalid. ` +
654+
`It must contain only alphanumeric characters and/or the following: +=,.@_-`;
655+
const { code, http_code, type } = IamError.ValidationError;
656+
throw new IamError({ code, message: message_with_details, http_code, type });
657+
}
658+
return true;
659+
} catch (err) {
660+
translate_rpc_error(err);
661+
}
662+
}
663+
664+
/**
665+
* _validate_json_policy_document will validate that the policy document is valid JSON
666+
* @param {string} input_policy_document
667+
*/
668+
function _validate_json_policy_document(input_policy_document) {
669+
try {
670+
return JSON.parse(input_policy_document);
671+
} catch (error) {
672+
const { code, http_code, type } = IamError.MalformedPolicyDocument;
673+
const message_with_details = 'Syntax errors in policy.';
674+
throw new IamError({ code, message: message_with_details, http_code, type });
675+
}
676+
}
677+
678+
/**
679+
* The function will validate the policy document basic structure
680+
* (currently - only that we don't have Principal NotPrincipal in every Statement)
681+
* @param {object} policy_document
682+
*/
683+
684+
function _validate_policy_document_iam_structure(policy_document) {
685+
// as we check this before the schema check - here we ensure that we have the Statement as array and it is iterable
686+
if (!policy_document.Statement || !Array.isArray(policy_document.Statement)) {
687+
const { code, http_code, type } = IamError.MalformedPolicyDocument;
688+
const message_with_details = 'Syntax errors in policy.';
689+
throw new IamError({ code, message: message_with_details, http_code, type });
690+
}
691+
for (const statement of policy_document.Statement) {
692+
const statement_principal = statement.Principal || statement.NotPrincipal;
693+
if (statement_principal) {
694+
const { code, http_code, type } = IamError.MalformedPolicyDocument;
695+
const message_with_details = 'Policy document should not specify a principal.';
696+
throw new IamError({ code, message: message_with_details, http_code, type });
697+
}
698+
}
699+
}
700+
596701
/**
597702
* translate_rpc_error is used to translate the RPC error in-place
598703
* @param {{ rpc_code: string; message: string; }} err
@@ -719,6 +824,8 @@ exports.validate_iam_path = validate_iam_path;
719824
exports.validate_marker = validate_marker;
720825
exports.validate_access_key_id = validate_access_key_id;
721826
exports.validate_status = validate_status;
827+
exports.validate_policy_name = validate_policy_name;
828+
exports.validate_policy_document = validate_policy_document;
722829
exports.parse_indexed_members = parse_indexed_members;
723830
exports.parse_tags_from_request_body = parse_tags_from_request_body;
724831
exports.parse_tag_keys_from_request_body = parse_tag_keys_from_request_body;

‎src/test/unit_tests/api/iam/test_iam_utils.test.js‎

Lines changed: 197 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ describe('parse_max_items', () => {
182182
describe('validate_user_input_iam', () => {
183183
describe('validate_iam_path', () => {
184184
const max_length = 512;
185-
it('should return true when path is undefined', () => {
185+
it('should return void when path is undefined', () => {
186186
let dummy_path;
187187
const res = iam_utils.validate_iam_path(dummy_path);
188188
expect(res).toBeUndefined();
@@ -313,7 +313,7 @@ describe('validate_user_input_iam', () => {
313313

314314
describe('validate_marker', () => {
315315
const min_length = 1;
316-
it('should return true when marker is undefined', () => {
316+
it('should return void when marker is undefined', () => {
317317
let dummy_marker;
318318
const res = iam_utils.validate_marker(dummy_marker);
319319
expect(res).toBeUndefined();
@@ -407,7 +407,7 @@ describe('validate_user_input_iam', () => {
407407
describe('validate_access_key_id', () => {
408408
const min_length = 16;
409409
const max_length = 128;
410-
it('should return true when access_key is undefined', () => {
410+
it('should return void when access_key is undefined', () => {
411411
let dummy_access_key;
412412
const res = iam_utils.validate_access_key_id(dummy_access_key);
413413
expect(res).toBeUndefined();
@@ -511,7 +511,7 @@ describe('validate_user_input_iam', () => {
511511
});
512512

513513
describe('validate_status', () => {
514-
it('should return true when status is undefined', () => {
514+
it('should return void when status is undefined', () => {
515515
let dummy_status;
516516
const res = iam_utils.validate_status(dummy_status);
517517
expect(res).toBeUndefined();
@@ -547,4 +547,197 @@ describe('validate_user_input_iam', () => {
547547
});
548548

549549
});
550+
551+
describe('validate_policy_name', () => {
552+
const min_length = 1;
553+
const max_length = 128;
554+
it('should return void when policy_name is undefined', () => {
555+
let dummy_policy_name;
556+
const res = iam_utils.validate_policy_name(dummy_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
557+
expect(res).toBeUndefined();
558+
});
559+
560+
it('should return true when policy_name is at the min or max length', () => {
561+
expect(iam_utils.validate_policy_name('a', iam_constants.IAM_PARAMETER_NAME.POLICY_NAME)).toBe(true);
562+
expect(iam_utils.validate_policy_name('a'.repeat(max_length), iam_constants.IAM_PARAMETER_NAME.POLICY_NAME)).toBe(true);
563+
});
564+
565+
it('should return true when policy_name is within the length constraint', () => {
566+
expect(iam_utils.validate_policy_name('a'.repeat(min_length + 1), iam_constants.IAM_PARAMETER_NAME.POLICY_NAME)).toBe(true);
567+
expect(iam_utils.validate_policy_name('a'.repeat(max_length - 1), iam_constants.IAM_PARAMETER_NAME.POLICY_NAME)).toBe(true);
568+
});
569+
570+
it('should return true when policy_name is valid CamelCase', () => {
571+
const dummy_policy_name = 'CreateBucketPolicy';
572+
const res = iam_utils.validate_policy_name(dummy_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
573+
expect(res).toBe(true);
574+
});
575+
576+
it('should return true when policy_name is valid SnakeCase', () => {
577+
const dummy_policy_name = 'create_bucket_policy';
578+
const res = iam_utils.validate_policy_name(dummy_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
579+
expect(res).toBe(true);
580+
});
581+
582+
it('should throw error when policy_name is invalid - contains invalid character', () => {
583+
try {
584+
iam_utils.validate_policy_name('{}', iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
585+
throw new NoErrorThrownError();
586+
} catch (err) {
587+
expect(err).toBeInstanceOf(IamError);
588+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
589+
}
590+
});
591+
592+
it('should throw error when policy_name is too short', () => {
593+
try {
594+
const dummy_policy_name = '';
595+
iam_utils.validate_policy_name(dummy_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
596+
throw new NoErrorThrownError();
597+
} catch (err) {
598+
expect(err).toBeInstanceOf(IamError);
599+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
600+
}
601+
});
602+
603+
it('should throw error when dummy_policy_name is too long', () => {
604+
try {
605+
const dummy_policy_name = 'A'.repeat(max_length + 1);
606+
iam_utils.validate_policy_name(dummy_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
607+
throw new NoErrorThrownError();
608+
} catch (err) {
609+
expect(err).toBeInstanceOf(IamError);
610+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
611+
}
612+
});
613+
614+
it('should throw error for invalid input types (null) in policy_name', () => {
615+
try {
616+
// @ts-ignore
617+
const invalid_policy_name = null;
618+
iam_utils.validate_policy_name(invalid_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
619+
throw new NoErrorThrownError();
620+
} catch (err) {
621+
expect(err).toBeInstanceOf(IamError);
622+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
623+
}
624+
});
625+
626+
it('should throw error for invalid input types (number) in policy_name', () => {
627+
try {
628+
const invalid_policy_name = 1;
629+
// @ts-ignore
630+
iam_utils.validate_policy_name(invalid_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
631+
throw new NoErrorThrownError();
632+
} catch (err) {
633+
expect(err).toBeInstanceOf(IamError);
634+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
635+
}
636+
});
637+
638+
it('should throw error for invalid input types (object) in policy_name', () => {
639+
try {
640+
const invalid_policy_name = {};
641+
// @ts-ignore
642+
iam_utils.validate_policy_name(invalid_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
643+
throw new NoErrorThrownError();
644+
} catch (err) {
645+
expect(err).toBeInstanceOf(IamError);
646+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
647+
}
648+
});
649+
650+
it('should throw error for invalid input types (boolean) in policy_name', () => {
651+
try {
652+
const invalid_policy_name = false;
653+
// @ts-ignore
654+
iam_utils.validate_policy_name(invalid_policy_name, iam_constants.IAM_PARAMETER_NAME.POLICY_NAME);
655+
throw new NoErrorThrownError();
656+
} catch (err) {
657+
expect(err).toBeInstanceOf(IamError);
658+
expect(err).toHaveProperty('code', IamError.ValidationError.code);
659+
}
660+
});
661+
});
662+
663+
describe('validate_policy_document', () => {
664+
it('should return void when policy_document is undefined', () => {
665+
let dummy_policy_document;
666+
const res = iam_utils.validate_policy_document(dummy_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
667+
expect(res).toBeUndefined();
668+
});
669+
670+
it('should return true when policy_document is valid JSON string', () => {
671+
const dummy_policy_document = JSON.stringify({
672+
Version: '2012-10-17',
673+
Statement: [{
674+
Effect: 'Allow',
675+
Action: 's3:GetBucketLocation',
676+
Resource: '*'
677+
}]
678+
});
679+
const res = iam_utils.validate_policy_document(dummy_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
680+
expect(res).toBe(true);
681+
});
682+
683+
it('should throw error when policy_document is invalid JSON string', () => {
684+
try {
685+
const invalid_policy_document = 'hello';
686+
iam_utils.validate_policy_document(invalid_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
687+
throw new NoErrorThrownError();
688+
} catch (err) {
689+
expect(err).toBeInstanceOf(IamError);
690+
expect(err).toHaveProperty('code', IamError.MalformedPolicyDocument.code);
691+
}
692+
});
693+
694+
it('should throw error when policy_document is empty', () => {
695+
try {
696+
const invalid_policy_document = '';
697+
iam_utils.validate_policy_document(invalid_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
698+
throw new NoErrorThrownError();
699+
} catch (err) {
700+
expect(err).toBeInstanceOf(IamError);
701+
expect(err).toHaveProperty('code', IamError.MalformedPolicyDocument.code);
702+
}
703+
});
704+
705+
it('should throw error when policy_document has Principal field', () => {
706+
try {
707+
const dummy_policy_document = JSON.stringify({
708+
Version: '2012-10-17',
709+
Statement: [{
710+
Effect: 'Allow',
711+
Action: 's3:GetBucketLocation',
712+
Resource: '*',
713+
Principal: '*' // in IAM user inline policies we don't have the principal (user policy is embed to user)
714+
}]
715+
});
716+
iam_utils.validate_policy_document(dummy_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
717+
throw new NoErrorThrownError();
718+
} catch (err) {
719+
expect(err).toBeInstanceOf(IamError);
720+
expect(err).toHaveProperty('code', IamError.MalformedPolicyDocument.code);
721+
}
722+
});
723+
724+
it('should throw error when policy_document has NotPrincipal field', () => {
725+
try {
726+
const dummy_policy_document = JSON.stringify({
727+
Version: '2012-10-17',
728+
Statement: [{
729+
Effect: 'Allow',
730+
Action: 's3:GetBucketLocation',
731+
Resource: '*',
732+
NotPrincipal: '*' // in IAM user inline policies we don't have the principal (user policy is embed to user)
733+
}]
734+
});
735+
iam_utils.validate_policy_document(dummy_policy_document, iam_constants.IAM_PARAMETER_NAME.POLICY_DOCUMENT);
736+
throw new NoErrorThrownError();
737+
} catch (err) {
738+
expect(err).toBeInstanceOf(IamError);
739+
expect(err).toHaveProperty('code', IamError.MalformedPolicyDocument.code);
740+
}
741+
});
742+
});
550743
});

0 commit comments

Comments
 (0)