From ccdcb7ca6018e38e9480814f87a3e3baeba15760 Mon Sep 17 00:00:00 2001 From: Chris Murton Date: Tue, 22 Oct 2019 14:27:49 +0100 Subject: [PATCH 1/2] Add conflict detection on IAM delete_role and delete_user --- moto/iam/models.py | 40 +++++++++++++++++++++++++++++--------- tests/test_iam/test_iam.py | 39 ++++++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/moto/iam/models.py b/moto/iam/models.py index ba7d0ac8..ca3ae00a 100644 --- a/moto/iam/models.py +++ b/moto/iam/models.py @@ -742,11 +742,25 @@ class IAMBackend(BaseBackend): raise IAMNotFoundException("Role {0} not found".format(arn)) def delete_role(self, role_name): - for role in self.get_roles(): - if role.name == role_name: - del self.roles[role.id] - return - raise IAMNotFoundException("Role {0} not found".format(role_name)) + role = self.get_role(role_name) + for instance_profile in self.get_instance_profiles(): + for role in instance_profile.roles: + if role.name == role_name: + raise IAMConflictException( + code="DeleteConflict", + message="Cannot delete entity, must remove roles from instance profile first." + ) + if role.managed_policies: + raise IAMConflictException( + code="DeleteConflict", + message="Cannot delete entity, must detach all policies first." + ) + if role.policies: + raise IAMConflictException( + code="DeleteConflict", + message="Cannot delete entity, must delete policies first." + ) + del self.roles[role.id] def get_roles(self): return self.roles.values() @@ -1251,10 +1265,18 @@ class IAMBackend(BaseBackend): return user.mfa_devices.values() def delete_user(self, user_name): - try: - del self.users[user_name] - except KeyError: - raise IAMNotFoundException("User {0} not found".format(user_name)) + user = self.get_user(user_name) + if user.managed_policies: + raise IAMConflictException( + code="DeleteConflict", + message="Cannot delete entity, must detach all policies first." + ) + if user.policies: + raise IAMConflictException( + code="DeleteConflict", + message="Cannot delete entity, must delete policies first." + ) + del self.users[user_name] def report_generated(self): return self.credential_report diff --git a/tests/test_iam/test_iam.py b/tests/test_iam/test_iam.py index 2374fb59..eb69852a 100644 --- a/tests/test_iam/test_iam.py +++ b/tests/test_iam/test_iam.py @@ -214,16 +214,39 @@ def test_update_login_profile(): def test_delete_role(): conn = boto3.client('iam', region_name='us-east-1') - with assert_raises(ClientError): + with assert_raises(conn.exceptions.NoSuchEntityException): conn.delete_role(RoleName="my-role") + # Test deletion failure with a managed policy conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") - role = conn.get_role(RoleName="my-role") - role.get('Role').get('Arn').should.equal('arn:aws:iam::123456789012:role/my-path/my-role') - + response = conn.create_policy(PolicyName="my-managed-policy", PolicyDocument=MOCK_POLICY) + conn.attach_role_policy(PolicyArn=response['Policy']['Arn'], RoleName="my-role") + with assert_raises(conn.exceptions.DeleteConflictException): + conn.delete_role(RoleName="my-role") + conn.detach_role_policy(PolicyArn=response['Policy']['Arn'], RoleName="my-role") + conn.delete_policy(PolicyArn=response['Policy']['Arn']) conn.delete_role(RoleName="my-role") + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_role(RoleName="my-role") - with assert_raises(ClientError): + # Test deletion failure with an inline policy + conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") + conn.put_role_policy(RoleName="my-role", PolicyName="my-role-policy", PolicyDocument=MOCK_POLICY) + with assert_raises(conn.exceptions.DeleteConflictException): + conn.delete_role(RoleName="my-role") + conn.delete_role_policy(RoleName="my-role", PolicyName="my-role-policy") + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_role(RoleName="my-role") + + # Test deletion failure with attachment to an instance profile + conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") + conn.create_instance_profile("my-profile", path="my-path") + conn.add_role_to_instance_profile(InstanceProfileName="my-profile", RoleName="my-role") + with assert_raises(conn.exceptions.DeleteConflictException): + conn.delete_role(RoleName="my-role") + conn.remove_role_from_instance_profile(InstanceProfileName="my-profile", RoleName="my-role") + conn.delete_role(RoleName="my-role") + with assert_raises(conn.exceptions.NoSuchEntityException): conn.get_role(RoleName="my-role") @@ -739,6 +762,12 @@ def test_delete_user(): conn.delete_user(UserName='my-user') conn.create_user(UserName='my-user') [user['UserName'] for user in conn.list_users()['Users']].should.equal(['my-user']) + + conn.put_user_policy(UserName='my-user', PolicyName='my-user-policy', PolicyDocument=MOCK_POLICY) + with assert_raises(ClientError): + conn.delete_user(UserName='my-user') + conn.delete_user_policy(UserName='my-user', PolicyName='my-user-policy') + conn.delete_user(UserName='my-user') assert conn.list_users()['Users'].should.be.empty From f0b22fcd2f775ac11c2b48965927dd492d0b50f6 Mon Sep 17 00:00:00 2001 From: Chris Murton Date: Tue, 22 Oct 2019 15:28:59 +0100 Subject: [PATCH 2/2] Fix absent role deletion, Add more delete_user tests, add no conflict deletion testing --- tests/test_iam/test_iam.py | 45 +++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/tests/test_iam/test_iam.py b/tests/test_iam/test_iam.py index eb69852a..e46c7dd6 100644 --- a/tests/test_iam/test_iam.py +++ b/tests/test_iam/test_iam.py @@ -235,12 +235,13 @@ def test_delete_role(): with assert_raises(conn.exceptions.DeleteConflictException): conn.delete_role(RoleName="my-role") conn.delete_role_policy(RoleName="my-role", PolicyName="my-role-policy") + conn.delete_role(RoleName="my-role") with assert_raises(conn.exceptions.NoSuchEntityException): conn.get_role(RoleName="my-role") # Test deletion failure with attachment to an instance profile conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") - conn.create_instance_profile("my-profile", path="my-path") + conn.create_instance_profile(InstanceProfileName="my-profile") conn.add_role_to_instance_profile(InstanceProfileName="my-profile", RoleName="my-role") with assert_raises(conn.exceptions.DeleteConflictException): conn.delete_role(RoleName="my-role") @@ -249,6 +250,12 @@ def test_delete_role(): with assert_raises(conn.exceptions.NoSuchEntityException): conn.get_role(RoleName="my-role") + # Test deletion with no conflicts + conn.create_role(RoleName="my-role", AssumeRolePolicyDocument="some policy", Path="/my-path/") + conn.delete_role(RoleName="my-role") + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_role(RoleName="my-role") + @mock_iam_deprecated() def test_list_instance_profiles(): @@ -758,18 +765,40 @@ def test_delete_user_deprecated(): @mock_iam() def test_delete_user(): conn = boto3.client('iam', region_name='us-east-1') - with assert_raises(ClientError): + with assert_raises(conn.exceptions.NoSuchEntityException): conn.delete_user(UserName='my-user') - conn.create_user(UserName='my-user') - [user['UserName'] for user in conn.list_users()['Users']].should.equal(['my-user']) - conn.put_user_policy(UserName='my-user', PolicyName='my-user-policy', PolicyDocument=MOCK_POLICY) - with assert_raises(ClientError): + # Test deletion failure with a managed policy + conn.create_user(UserName='my-user') + response = conn.create_policy(PolicyName="my-managed-policy", PolicyDocument=MOCK_POLICY) + conn.attach_user_policy(PolicyArn=response['Policy']['Arn'], UserName="my-user") + with assert_raises(conn.exceptions.DeleteConflictException): + conn.delete_user(UserName='my-user') + conn.detach_user_policy(PolicyArn=response['Policy']['Arn'], UserName="my-user") + conn.delete_policy(PolicyArn=response['Policy']['Arn']) + conn.delete_user(UserName='my-user') + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_user(UserName='my-user') + + # Test deletion failure with an inline policy + conn.create_user(UserName='my-user') + conn.put_user_policy( + UserName='my-user', + PolicyName='my-user-policy', + PolicyDocument=MOCK_POLICY + ) + with assert_raises(conn.exceptions.DeleteConflictException): conn.delete_user(UserName='my-user') conn.delete_user_policy(UserName='my-user', PolicyName='my-user-policy') - conn.delete_user(UserName='my-user') - assert conn.list_users()['Users'].should.be.empty + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_user(UserName='my-user') + + # Test deletion with no conflicts + conn.create_user(UserName='my-user') + conn.delete_user(UserName='my-user') + with assert_raises(conn.exceptions.NoSuchEntityException): + conn.get_user(UserName='my-user') @mock_iam_deprecated()