Merge pull request #2504 from csmurton/add-iam-delete-entity-constraints

Conflict detection for IAM delete_role and delete_user

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():
+        role = self.get_role(role_name)
-            if role.name == role_name:
+        for instance_profile in self.get_instance_profiles():
-                del self.roles[role.id]
+            for role in instance_profile.roles:
-                return
+                if role.name == role_name:
-        raise IAMNotFoundException("Role {0} not found".format(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:
+        user = self.get_user(user_name)
-            del self.users[user_name]
+        if user.managed_policies:
-        except KeyError:
+            raise IAMConflictException(
-            raise IAMNotFoundException("User {0} not found".format(user_name))
+                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

tests/test_iam/test_iam.py

@@ -214,16 +214,46 @@ 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")
+    response = conn.create_policy(PolicyName="my-managed-policy", PolicyDocument=MOCK_POLICY)
-    role.get('Role').get('Arn').should.equal('arn:aws:iam::123456789012:role/my-path/my-role')
+    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")
+    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(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")
+    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")
+
+    # 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")
 
 
@@ -735,12 +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')
+
+    # Test deletion failure with a managed policy
     conn.create_user(UserName='my-user')
-    [user['UserName'] for user in conn.list_users()['Users']].should.equal(['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')
-    assert conn.list_users()['Users'].should.be.empty
+    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')
+    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()