From e133344846f68b47224a903becd5602631c6d0d8 Mon Sep 17 00:00:00 2001 From: acsbendi Date: Sun, 30 Jun 2019 18:48:27 +0200 Subject: [PATCH] Implemented validating action prefixes. --- moto/iam/policy_validation.py | 34 ++++++++++-- tests/test_iam/test_iam_policies.py | 80 +++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/moto/iam/policy_validation.py b/moto/iam/policy_validation.py index 6b0f6578b..38bb6e944 100644 --- a/moto/iam/policy_validation.py +++ b/moto/iam/policy_validation.py @@ -1,4 +1,5 @@ import json +import re from six import string_types @@ -58,6 +59,8 @@ class IAMPolicyDocumentValidator: except Exception: raise MalformedPolicyDocument("Policy statement must contain resources.") + self._validate_action_prefix() + def _validate_syntax(self): self._policy_json = json.loads(self._policy_document) assert isinstance(self._policy_json, dict) @@ -101,6 +104,8 @@ class IAMPolicyDocumentValidator: assert ("Action" not in statement or "NotAction" not in statement) IAMPolicyDocumentValidator._validate_effect_syntax(statement) + IAMPolicyDocumentValidator._validate_action_syntax(statement) + IAMPolicyDocumentValidator._validate_not_action_syntax(statement) IAMPolicyDocumentValidator._validate_resource_syntax(statement) IAMPolicyDocumentValidator._validate_not_resource_syntax(statement) IAMPolicyDocumentValidator._validate_condition_syntax(statement) @@ -112,16 +117,24 @@ class IAMPolicyDocumentValidator: assert isinstance(statement["Effect"], string_types) assert statement["Effect"].lower() in [allowed_effect.lower() for allowed_effect in ALLOWED_EFFECTS] + @staticmethod + def _validate_action_syntax(statement): + IAMPolicyDocumentValidator._validate_string_or_list_of_strings_syntax(statement, "Action") + + @staticmethod + def _validate_not_action_syntax(statement): + IAMPolicyDocumentValidator._validate_string_or_list_of_strings_syntax(statement, "NotAction") + @staticmethod def _validate_resource_syntax(statement): - IAMPolicyDocumentValidator._validate_resource_like_syntax(statement, "Resource") + IAMPolicyDocumentValidator._validate_string_or_list_of_strings_syntax(statement, "Resource") @staticmethod def _validate_not_resource_syntax(statement): - IAMPolicyDocumentValidator._validate_resource_like_syntax(statement, "NotResource") + IAMPolicyDocumentValidator._validate_string_or_list_of_strings_syntax(statement, "NotResource") @staticmethod - def _validate_resource_like_syntax(statement, key): + def _validate_string_or_list_of_strings_syntax(statement, key): if key in statement: assert isinstance(statement[key], (string_types, list)) if isinstance(statement[key], list): @@ -155,4 +168,19 @@ class IAMPolicyDocumentValidator: def _validate_action_exist(self): for statement in self._statements: assert "Action" in statement + if isinstance(statement["Action"], list): + assert statement["Action"] + + def _validate_action_prefix(self): + for statement in self._statements: + action_parts = statement["Action"].split(":") + if len(action_parts) == 1: + raise MalformedPolicyDocument("Actions/Conditions must be prefaced by a vendor, e.g., iam, sdb, ec2, etc.") + elif len(action_parts) > 2: + raise MalformedPolicyDocument("Actions/Condition can contain only one colon.") + + vendor_pattern = re.compile(r'[^a-zA-Z0-9\-.]') + if vendor_pattern.search(action_parts[0]): + raise MalformedPolicyDocument("Vendor {vendor} is not valid".format(vendor=action_parts[0])) + diff --git a/tests/test_iam/test_iam_policies.py b/tests/test_iam/test_iam_policies.py index aefacf25e..f1c53a7d0 100644 --- a/tests/test_iam/test_iam_policies.py +++ b/tests/test_iam/test_iam_policies.py @@ -127,6 +127,30 @@ invalid_documents_test_cases = [ }, "error_message": 'Actions/Conditions must be prefaced by a vendor, e.g., iam, sdb, ec2, etc.' }, + { + "document": { + "Version": "2012-10-17", + "Statement": + { + "Effect": "Allow", + "Action": "a a:ListBucket", + "Resource": "arn:aws:s3:::example_bucket" + } + }, + "error_message": 'Vendor a a is not valid' + }, + { + "document": { + "Version": "2012-10-17", + "Statement": + { + "Effect": "Allow", + "Action": "s3:List:Bucket", + "Resource": "arn:aws:s3:::example_bucket" + } + }, + "error_message": 'Actions/Condition can contain only one colon.' + }, { "document": { "Version": "2012-10-17", @@ -149,6 +173,17 @@ invalid_documents_test_cases = [ }, "error_message": 'Resource adf must be in ARN format or "*".' }, + { + "document": { + "Version": "2012-10-17", + "Statement": { + "Effect": "Allow", + "Action": "s3:ListBucket", + "Resource": "" + } + }, + "error_message": 'Resource must be in ARN format or "*".' + }, { "document": { "Version": "2012-10-17", @@ -177,6 +212,16 @@ invalid_documents_test_cases = [ }, "error_message": 'Policy statement must contain resources.' }, + { + "document": { + "Version": "2012-10-17", + "Statement": { + "Effect": "Allow", + "Action": "invalid" + } + }, + "error_message": 'Policy statement must contain resources.' + }, { "document": { "Version": "2012-10-17", @@ -206,6 +251,18 @@ invalid_documents_test_cases = [ }, "error_message": 'Policy statement must contain actions.' }, + { + "document": { + "Version": "2012-10-17", + "Statement": + { + "Effect": "Allow", + "Action": [], + "Resource": "arn:aws:s3:::example_bucket" + } + }, + "error_message": 'Policy statement must contain actions.' + }, { "document": { "Version": "2012-10-17", @@ -283,6 +340,29 @@ invalid_documents_test_cases = [ }, "error_message": 'Syntax errors in policy.' }, + { + "document": { + "Version": "2012-10-17", + "Statement": { + "Effect": "Deny", + "Action": [[]], + "Resource": "arn:aws:s3:::example_bucket" + } + }, + "error_message": 'Syntax errors in policy.' + }, + { + "document": { + "Version": "2012-10-17", + "Statement": + { + "Effect": "Allow", + "Action": {}, + "Resource": "arn:aws:s3:::example_bucket" + } + }, + "error_message": 'Syntax errors in policy.' + }, { "document": { "Version": "2012-10-17",