Repository: bridgecrewio/checkov

Issue:
Add new check: IAM Policies should not be attached to a user
Attach policies only to groups or roles 

https://www.terraform.io/docs/providers/aws/r/iam_user_policy.html
https://www.terraform.io/docs/providers/aws/r/iam_user_policy_attachment.html
users: https://www.terraform.io/docs/providers/aws/r/iam_policy_attachment.html

Policies / ManagedPolicyArns: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iam-user.html
Users: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-policy.html



Discussion:

Patch:
diff --git a/checkov/terraform/checks/resource/aws/IAMPolicyAttachedToGroupOrRoles.py b/checkov/terraform/checks/resource/aws/IAMPolicyAttachedToGroupOrRoles.py
index d853428ee..cef1f82a7 100644
--- a/checkov/terraform/checks/resource/aws/IAMPolicyAttachedToGroupOrRoles.py
+++ b/checkov/terraform/checks/resource/aws/IAMPolicyAttachedToGroupOrRoles.py
@@ -12,8 +12,9 @@ class IAMPolicyAttachedToGroupOrRoles(BaseResourceCheck):
         super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
 
     def scan_resource_conf(self, conf):
-        if 'user' in conf.keys() or 'users' in conf.keys():
+        if 'user' in conf.keys() or ('users' in conf.keys() and len(conf['users'][0]) > 0):
             return CheckResult.FAILED
         return CheckResult.PASSED
 
+
 check = IAMPolicyAttachedToGroupOrRoles()


Test patch:
diff --git a/tests/terraform/checks/resource/aws/test_IAMPolicyAttachedToGroupOrRoles.py b/tests/terraform/checks/resource/aws/test_IAMPolicyAttachedToGroupOrRoles.py
index 285dece4f..9f02a4ebe 100644
--- a/tests/terraform/checks/resource/aws/test_IAMPolicyAttachedToGroupOrRoles.py
+++ b/tests/terraform/checks/resource/aws/test_IAMPolicyAttachedToGroupOrRoles.py
@@ -13,10 +13,23 @@ class TestIAMPolicyAttachedToGroupOrRoles(unittest.TestCase):
 
     def test_failure_2(self):
         resource_conf = {'name': ['test'], 'user': ['${aws_iam_user.lb.name}'], 'policy': [
-            '{\n  "Version": "2012-10-17",\n  "Statement": [\n    {\n      "Action": [\n        "ec2:Describe*"\n      ],\n      "Effect": "Allow",\n      "Resource": "*"\n    }\n  ]\n}']}
+            '{\n  "Version": "2012-10-17",\n  "Statement": [\n    {\n      "Action": [\n        "ec2:Describe*"\n      ],\n      "Effect": "Allow",'
+            '\n      "Resource": "*"\n    }\n  ]\n}']}
         scan_result = check.scan_resource_conf(conf=resource_conf)
         self.assertEqual(CheckResult.FAILED, scan_result)
 
+    def test_failure_3(self):
+        conf = {'name': ['test-attachment'], 'users': [['${aws_iam_user.user.name}']], 'roles': [['${aws_iam_role.role.name}']],
+                'groups': [['${aws_iam_group.group.name}']], 'policy_arn': ['${aws_iam_policy.policy.arn}']}
+        scan_result = check.scan_resource_conf(conf=conf)
+        self.assertEqual(CheckResult.FAILED, scan_result)
+
+    def test_success(self):
+        conf = {'name': ['test-attachment'], 'users': [[]], 'roles': [['${aws_iam_role.role.name}']],
+                'groups': [['${aws_iam_group.group.name}']], 'policy_arn': ['${aws_iam_policy.policy.arn}']}
+        scan_result = check.scan_resource_conf(conf=conf)
+        self.assertEqual(CheckResult.PASSED, scan_result)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/terraform/test_scanner_registry.py b/tests/terraform/test_scanner_registry.py
index e9052dce7..ef5a0d615 100644
--- a/tests/terraform/test_scanner_registry.py
+++ b/tests/terraform/test_scanner_registry.py
@@ -14,12 +14,13 @@ class TestScannerRegistry(unittest.TestCase):
 
     def test_non_colliding_check_ids(self):
         check_id_check_class_map = {}
-        for (resource_type,checks) in registry.checks.items():
+        for (resource_type, checks) in registry.checks.items():
             for check in checks:
-                check_id_check_class_map.setdefault(check.id,[]).append(check)
+                check_id_check_class_map.setdefault(check.id, []).append(check)
+
+        for check_id, check_classes in check_id_check_class_map.items():
+            self.assertEqual(len(set(check_classes)), 1)
 
-        for check_id,check_classes in check_id_check_class_map.items():
-            self.assertEqual(len(set(check_classes)),1)
 
 if __name__ == '__main__':
     unittest.main()