From 23c4f47635c0534d8448a09dd54a5f4b82966ce1 Mon Sep 17 00:00:00 2001 From: Michael Merrill Date: Fri, 29 Jul 2022 23:25:56 -0400 Subject: [PATCH] Implement IAM back end part of update_assume_role_policy to validate policies (#5340) --- IMPLEMENTATION_COVERAGE.md | 2 +- docs/docs/services/iam.rst | 2 +- moto/iam/models.py | 11 +++- moto/iam/policy_validation.py | 58 +++++++++++++++-- moto/iam/responses.py | 4 +- tests/test_iam/test_iam.py | 99 ++++++++++++++++++++++++++++- tests/test_iam/test_iam_policies.py | 2 +- 7 files changed, 164 insertions(+), 14 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 20f505d6c..d6a415cdd 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -3290,7 +3290,7 @@ - [X] untag_user - [X] update_access_key - [X] update_account_password_policy -- [ ] update_assume_role_policy +- [X] update_assume_role_policy - [X] update_group - [X] update_login_profile - [X] update_open_id_connect_provider_thumbprint diff --git a/docs/docs/services/iam.rst b/docs/docs/services/iam.rst index 87b13f623..eae5de30c 100644 --- a/docs/docs/services/iam.rst +++ b/docs/docs/services/iam.rst @@ -192,7 +192,7 @@ iam - [X] untag_user - [X] update_access_key - [X] update_account_password_policy -- [ ] update_assume_role_policy +- [X] update_assume_role_policy - [X] update_group - [X] update_login_profile - [X] update_open_id_connect_provider_thumbprint diff --git a/moto/iam/models.py b/moto/iam/models.py index 8259a4296..bb1adc10b 100644 --- a/moto/iam/models.py +++ b/moto/iam/models.py @@ -21,7 +21,10 @@ from moto.core.utils import ( iso_8601_datetime_with_milliseconds, BackendDict, ) -from moto.iam.policy_validation import IAMPolicyDocumentValidator +from moto.iam.policy_validation import ( + IAMPolicyDocumentValidator, + IAMTrustPolicyDocumentValidator, +) from moto.utilities.utils import md5_hash from .aws_managed_policies import aws_managed_policies_data @@ -1851,6 +1854,12 @@ class IAMBackend(BaseBackend): def get_roles(self): return self.roles.values() + def update_assume_role_policy(self, role_name, policy_document): + role = self.get_role(role_name) + iam_policy_document_validator = IAMTrustPolicyDocumentValidator(policy_document) + iam_policy_document_validator.validate() + role.assume_role_policy_document = policy_document + def put_role_policy(self, role_name, policy_name, policy_json): role = self.get_role(role_name) diff --git a/moto/iam/policy_validation.py b/moto/iam/policy_validation.py index ee6a33ae2..c77069e70 100644 --- a/moto/iam/policy_validation.py +++ b/moto/iam/policy_validation.py @@ -83,7 +83,7 @@ VALID_RESOURCE_PATH_STARTING_VALUES = { } -class IAMPolicyDocumentValidator: +class BaseIAMPolicyValidator: def __init__(self, policy_document): self._policy_document = policy_document self._policy_json = {} @@ -117,10 +117,6 @@ class IAMPolicyDocumentValidator: self._validate_action_like_exist() except Exception: raise MalformedPolicyDocument("Policy statement must contain actions.") - try: - self._validate_resource_exist() - except Exception: - raise MalformedPolicyDocument("Policy statement must contain resources.") if self._resource_error != "": raise MalformedPolicyDocument(self._resource_error) @@ -521,3 +517,55 @@ class IAMPolicyDocumentValidator: if seconds_with_decimal_fraction_partition[1] == ".": decimal_seconds = seconds_with_decimal_fraction_partition[2] assert 0 <= int(decimal_seconds) <= 999999999 + + +class IAMPolicyDocumentValidator(BaseIAMPolicyValidator): + def __init__(self, policy_document): + super().__init__(policy_document) + + def validate(self): + super().validate() + try: + self._validate_resource_exist() + except Exception: + raise MalformedPolicyDocument("Policy statement must contain resources.") + + +class IAMTrustPolicyDocumentValidator(BaseIAMPolicyValidator): + def __init__(self, policy_document): + super().__init__(policy_document) + + def validate(self): + super().validate() + try: + for statement in self._statements: + if isinstance(statement["Action"], str): + IAMTrustPolicyDocumentValidator._validate_trust_policy_action( + statement["Action"] + ) + else: + for action in statement["Action"]: + IAMTrustPolicyDocumentValidator._validate_trust_policy_action( + action + ) + except Exception: + raise MalformedPolicyDocument( + "Trust Policy statement actions can only be sts:AssumeRole, " + "sts:AssumeRoleWithSAML, and sts:AssumeRoleWithWebIdentity" + ) + try: + self._validate_resource_not_exist() + except Exception: + raise MalformedPolicyDocument("Has prohibited field Resource.") + + def _validate_resource_not_exist(self): + for statement in self._statements: + assert "Resource" not in statement and "NotResource" not in statement + + @staticmethod + def _validate_trust_policy_action(action): + assert action in ( + "sts:AssumeRole", + "sts:AssumeRoleWithSAML", + "sts:AssumeRoleWithWebIdentity", + ) diff --git a/moto/iam/responses.py b/moto/iam/responses.py index 178d19a8d..d9ab5e06a 100644 --- a/moto/iam/responses.py +++ b/moto/iam/responses.py @@ -255,8 +255,8 @@ class IamResponse(BaseResponse): def update_assume_role_policy(self): role_name = self._get_param("RoleName") - role = self.backend.get_role(role_name) - role.assume_role_policy_document = self._get_param("PolicyDocument") + policy_document = self._get_param("PolicyDocument") + self.backend.update_assume_role_policy(role_name, policy_document) template = self.response_template(GENERIC_EMPTY_TEMPLATE) return template.render(name="UpdateAssumeRolePolicy") diff --git a/tests/test_iam/test_iam.py b/tests/test_iam/test_iam.py index 95f39d914..a1c94da95 100644 --- a/tests/test_iam/test_iam.py +++ b/tests/test_iam/test_iam.py @@ -379,14 +379,107 @@ def test_get_role_policy(): @mock_iam -def test_update_assume_role_policy(): +def test_update_assume_role_invalid_policy(): conn = boto3.client("iam", region_name="us-east-1") conn.create_role( RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="my-path" ) - conn.update_assume_role_policy(RoleName="my-role", PolicyDocument="new policy") + with pytest.raises(ClientError) as ex: + conn.update_assume_role_policy(RoleName="my-role", PolicyDocument="new policy") + err = ex.value.response["Error"] + err["Code"].should.equal("MalformedPolicyDocument") + err["Message"].should.contain("Syntax errors in policy.") + + +@mock_iam +def test_update_assume_role_valid_policy(): + conn = boto3.client("iam", region_name="us-east-1") + conn.create_role( + RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="my-path" + ) + policy_document = """ + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": ["ec2.amazonaws.com"] + }, + "Action": ["sts:AssumeRole"] + } + ] + } +""" + conn.update_assume_role_policy(RoleName="my-role", PolicyDocument=policy_document) role = conn.get_role(RoleName="my-role")["Role"] - role["AssumeRolePolicyDocument"].should.equal("new policy") + role["AssumeRolePolicyDocument"]["Statement"][0]["Action"][0].should.equal( + "sts:AssumeRole" + ) + + +@mock_iam +def test_update_assume_role_invalid_policy_bad_action(): + conn = boto3.client("iam", region_name="us-east-1") + conn.create_role( + RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="my-path" + ) + policy_document = """ + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": ["ec2.amazonaws.com"] + }, + "Action": ["sts:BadAssumeRole"] + } + ] + } +""" + + with pytest.raises(ClientError) as ex: + conn.update_assume_role_policy( + RoleName="my-role", PolicyDocument=policy_document + ) + err = ex.value.response["Error"] + err["Code"].should.equal("MalformedPolicyDocument") + err["Message"].should.contain( + "Trust Policy statement actions can only be sts:AssumeRole, " + "sts:AssumeRoleWithSAML, and sts:AssumeRoleWithWebIdentity" + ) + + +@mock_iam +def test_update_assume_role_invalid_policy_with_resource(): + conn = boto3.client("iam", region_name="us-east-1") + conn.create_role( + RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="my-path" + ) + policy_document = """ + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": ["ec2.amazonaws.com"] + }, + "Action": ["sts:AssumeRole"], + "Resource" : "arn:aws:s3:::example_bucket" + } + ] + } + """ + + with pytest.raises(ClientError) as ex: + conn.update_assume_role_policy( + RoleName="my-role", PolicyDocument=policy_document + ) + err = ex.value.response["Error"] + err["Code"].should.equal("MalformedPolicyDocument") + err["Message"].should.contain("Has prohibited field Resource.") @mock_iam diff --git a/tests/test_iam/test_iam_policies.py b/tests/test_iam/test_iam_policies.py index 7aa035e10..4099ac783 100644 --- a/tests/test_iam/test_iam_policies.py +++ b/tests/test_iam/test_iam_policies.py @@ -317,7 +317,7 @@ invalid_policy_document_test_cases = [ "Version": "2012-10-17", "Statement": {"Effect": "Allow", "Action": "invalid"}, }, - "error_message": "Policy statement must contain resources.", + "error_message": "Actions/Conditions must be prefaced by a vendor, e.g., iam, sdb, ec2, etc.", }, { "document": {