From 38866bfcef28525a48035e023931aa1fbe1b35c6 Mon Sep 17 00:00:00 2001 From: Mike Grima Date: Wed, 21 Aug 2019 12:24:23 -0700 Subject: [PATCH] Fixed some IAM APIs for tagging and role descriptions --- moto/iam/models.py | 50 ++++++++++++---------- moto/iam/responses.py | 24 ++++++++++- tests/test_iam/test_iam.py | 86 +++++++++++++++++++++++++++++++++++++- 3 files changed, 136 insertions(+), 24 deletions(-) diff --git a/moto/iam/models.py b/moto/iam/models.py index 21bb87e02..d76df8a28 100644 --- a/moto/iam/models.py +++ b/moto/iam/models.py @@ -161,7 +161,7 @@ class InlinePolicy(Policy): class Role(BaseModel): - def __init__(self, role_id, name, assume_role_policy_document, path, permissions_boundary): + def __init__(self, role_id, name, assume_role_policy_document, path, permissions_boundary, description, tags): self.id = role_id self.name = name self.assume_role_policy_document = assume_role_policy_document @@ -169,8 +169,8 @@ class Role(BaseModel): self.policies = {} self.managed_policies = {} self.create_date = datetime.utcnow() - self.tags = {} - self.description = "" + self.tags = tags + self.description = description self.permissions_boundary = permissions_boundary @property @@ -185,7 +185,9 @@ class Role(BaseModel): role_name=resource_name, assume_role_policy_document=properties['AssumeRolePolicyDocument'], path=properties.get('Path', '/'), - permissions_boundary=properties.get('PermissionsBoundary', '') + permissions_boundary=properties.get('PermissionsBoundary', ''), + description=properties.get('Description', ''), + tags=properties.get('Tags', {}) ) policies = properties.get('Policies', []) @@ -635,12 +637,13 @@ class IAMBackend(BaseBackend): return policies, marker - def create_role(self, role_name, assume_role_policy_document, path, permissions_boundary): + def create_role(self, role_name, assume_role_policy_document, path, permissions_boundary, description, tags): role_id = random_resource_id() if permissions_boundary and not self.policy_arn_regex.match(permissions_boundary): raise RESTError('InvalidParameterValue', 'Value ({}) for parameter PermissionsBoundary is invalid.'.format(permissions_boundary)) - role = Role(role_id, role_name, assume_role_policy_document, path, permissions_boundary) + clean_tags = self._tag_verification(tags) + role = Role(role_id, role_name, assume_role_policy_document, path, permissions_boundary, description, clean_tags) self.roles[role_id] = role return role @@ -691,6 +694,23 @@ class IAMBackend(BaseBackend): role = self.get_role(role_name) return role.policies.keys() + def _tag_verification(self, tags): + if len(tags) > 50: + raise TooManyTags(tags) + + tag_keys = {} + for tag in tags: + # Need to index by the lowercase tag key since the keys are case insensitive, but their case is retained. + ref_key = tag['Key'].lower() + self._check_tag_duplicate(tag_keys, ref_key) + self._validate_tag_key(tag['Key']) + if len(tag['Value']) > 256: + raise TagValueTooBig(tag['Value']) + + tag_keys[ref_key] = tag + + return tag_keys + def _validate_tag_key(self, tag_key, exception_param='tags.X.member.key'): """Validates the tag key. @@ -740,23 +760,9 @@ class IAMBackend(BaseBackend): return tags, marker def tag_role(self, role_name, tags): - if len(tags) > 50: - raise TooManyTags(tags) - + clean_tags = self._tag_verification(tags) role = self.get_role(role_name) - - tag_keys = {} - for tag in tags: - # Need to index by the lowercase tag key since the keys are case insensitive, but their case is retained. - ref_key = tag['Key'].lower() - self._check_tag_duplicate(tag_keys, ref_key) - self._validate_tag_key(tag['Key']) - if len(tag['Value']) > 256: - raise TagValueTooBig(tag['Value']) - - tag_keys[ref_key] = tag - - role.tags.update(tag_keys) + role.tags.update(clean_tags) def untag_role(self, role_name, tag_keys): if len(tag_keys) > 50: diff --git a/moto/iam/responses.py b/moto/iam/responses.py index 7ec6242f6..806dd37f4 100644 --- a/moto/iam/responses.py +++ b/moto/iam/responses.py @@ -178,9 +178,11 @@ class IamResponse(BaseResponse): 'AssumeRolePolicyDocument') permissions_boundary = self._get_param( 'PermissionsBoundary') + description = self._get_param('Description') + tags = self._get_multi_param('Tags.member') role = iam_backend.create_role( - role_name, assume_role_policy_document, path, permissions_boundary) + role_name, assume_role_policy_document, path, permissions_boundary, description, tags) template = self.response_template(CREATE_ROLE_TEMPLATE) return template.render(role=role) @@ -1002,6 +1004,7 @@ CREATE_ROLE_TEMPLATE = """{{ role.arn }} {{ role.name }} {{ role.assume_role_policy_document }} + {{role.description}} {{ role.created_iso_8601 }} {{ role.id }} + {% if role.permissions_boundary %} + + PermissionsBoundaryPolicy + {{ role.permissions_boundary }} + + {% endif %} {% endfor %} diff --git a/tests/test_iam/test_iam.py b/tests/test_iam/test_iam.py index e7507e2e5..fe2117a3a 100644 --- a/tests/test_iam/test_iam.py +++ b/tests/test_iam/test_iam.py @@ -944,7 +944,8 @@ def test_get_account_authorization_details(): }) conn = boto3.client('iam', region_name='us-east-1') - conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") + boundary = 'arn:aws:iam::123456789012:policy/boundary' + conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/", Description='testing', PermissionsBoundary=boundary) conn.create_user(Path='/', UserName='testUser') conn.create_group(Path='/', GroupName='testGroup') conn.create_policy( @@ -985,6 +986,11 @@ def test_get_account_authorization_details(): assert len(result['GroupDetailList']) == 0 assert len(result['Policies']) == 0 assert len(result['RoleDetailList'][0]['InstanceProfileList']) == 1 + assert result['RoleDetailList'][0]['InstanceProfileList'][0]['Roles'][0]['Description'] == 'testing' + assert result['RoleDetailList'][0]['InstanceProfileList'][0]['Roles'][0]['PermissionsBoundary'] == { + 'PermissionsBoundaryType': 'PermissionsBoundaryPolicy', + 'PermissionsBoundaryArn': 'arn:aws:iam::123456789012:policy/boundary' + } assert len(result['RoleDetailList'][0]['Tags']) == 2 assert len(result['RoleDetailList'][0]['RolePolicyList']) == 1 assert len(result['RoleDetailList'][0]['AttachedManagedPolicies']) == 1 @@ -1151,6 +1157,79 @@ def test_delete_saml_provider(): assert not resp['Certificates'] +@mock_iam() +def test_create_role_with_tags(): + """Tests both the tag_role and get_role_tags capability""" + conn = boto3.client('iam', region_name='us-east-1') + conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="{}", Tags=[ + { + 'Key': 'somekey', + 'Value': 'somevalue' + }, + { + 'Key': 'someotherkey', + 'Value': 'someothervalue' + } + ], Description='testing') + + # Get role: + role = conn.get_role(RoleName='my-role')['Role'] + assert len(role['Tags']) == 2 + assert role['Tags'][0]['Key'] == 'somekey' + assert role['Tags'][0]['Value'] == 'somevalue' + assert role['Tags'][1]['Key'] == 'someotherkey' + assert role['Tags'][1]['Value'] == 'someothervalue' + assert role['Description'] == 'testing' + + # Empty is good: + conn.create_role(RoleName="my-role2", AssumeRolePolicyDocument="{}", Tags=[ + { + 'Key': 'somekey', + 'Value': '' + } + ]) + tags = conn.list_role_tags(RoleName='my-role2') + assert len(tags['Tags']) == 1 + assert tags['Tags'][0]['Key'] == 'somekey' + assert tags['Tags'][0]['Value'] == '' + + # Test creating tags with invalid values: + # With more than 50 tags: + with assert_raises(ClientError) as ce: + too_many_tags = list(map(lambda x: {'Key': str(x), 'Value': str(x)}, range(0, 51))) + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=too_many_tags) + assert 'failed to satisfy constraint: Member must have length less than or equal to 50.' \ + in ce.exception.response['Error']['Message'] + + # With a duplicate tag: + with assert_raises(ClientError) as ce: + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=[{'Key': '0', 'Value': ''}, {'Key': '0', 'Value': ''}]) + assert 'Duplicate tag keys found. Please note that Tag keys are case insensitive.' \ + in ce.exception.response['Error']['Message'] + + # Duplicate tag with different casing: + with assert_raises(ClientError) as ce: + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=[{'Key': 'a', 'Value': ''}, {'Key': 'A', 'Value': ''}]) + assert 'Duplicate tag keys found. Please note that Tag keys are case insensitive.' \ + in ce.exception.response['Error']['Message'] + + # With a really big key: + with assert_raises(ClientError) as ce: + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=[{'Key': '0' * 129, 'Value': ''}]) + assert 'Member must have length less than or equal to 128.' in ce.exception.response['Error']['Message'] + + # With a really big value: + with assert_raises(ClientError) as ce: + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=[{'Key': '0', 'Value': '0' * 257}]) + assert 'Member must have length less than or equal to 256.' in ce.exception.response['Error']['Message'] + + # With an invalid character: + with assert_raises(ClientError) as ce: + conn.create_role(RoleName="my-role3", AssumeRolePolicyDocument="{}", Tags=[{'Key': 'NOWAY!', 'Value': ''}]) + assert 'Member must satisfy regular expression pattern: [\\p{L}\\p{Z}\\p{N}_.:/=+\\-@]+' \ + in ce.exception.response['Error']['Message'] + + @mock_iam() def test_tag_role(): """Tests both the tag_role and get_role_tags capability""" @@ -1338,6 +1417,7 @@ def test_update_role_description(): assert response['Role']['RoleName'] == 'my-role' + @mock_iam() def test_update_role(): conn = boto3.client('iam', region_name='us-east-1') @@ -1349,6 +1429,7 @@ def test_update_role(): response = conn.update_role_description(RoleName="my-role", Description="test") assert response['Role']['RoleName'] == 'my-role' + @mock_iam() def test_update_role(): conn = boto3.client('iam', region_name='us-east-1') @@ -1443,6 +1524,8 @@ def test_create_role_no_path(): resp = conn.create_role(RoleName='my-role', AssumeRolePolicyDocument='some policy', Description='test') resp.get('Role').get('Arn').should.equal('arn:aws:iam::123456789012:role/my-role') resp.get('Role').should_not.have.key('PermissionsBoundary') + resp.get('Role').get('Description').should.equal('test') + @mock_iam() def test_create_role_with_permissions_boundary(): @@ -1454,6 +1537,7 @@ def test_create_role_with_permissions_boundary(): 'PermissionsBoundaryArn': boundary } resp.get('Role').get('PermissionsBoundary').should.equal(expected) + resp.get('Role').get('Description').should.equal('test') invalid_boundary_arn = 'arn:aws:iam::123456789:not_a_boundary' with assert_raises(ClientError):