Fix the concept of "admin" in aws iam tests (#152)

Originally, a user was an "admin" if they had a policy that included the
word "admin" in it. That kinda worked sometimes, but needed to be
replaced.

Now, in the config file you can list the admin policies and groups and a
user or role will be matched against these lists.

README.md

@@ -211,17 +211,27 @@ regressions:
     test_param_id: '*mycustomgroup'
     comment: this was remediated by ops team
 aws:
+  admin_groups:
+    - "Administrators"
+  admin_policies:
+    - "AWSAdminRequireMFA"
   user_is_inactive:
     no_activity_since:
       years: 1
       months: 0
     created_after:
       weeks: 1
+  access_key_expires_after:
+    years: 1
+    months: 0
   required_tags:
     - Name
     - Type
     - App
     - Env
+  required_amis:
+    - ami-00000000000000000
+    - ami-55555555555555555
   whitelisted_ports_global:
     - 25
   whitelisted_ports:
@@ -229,6 +239,12 @@ aws:
       ports:
         - 22
         - 2222
+gsuite:
+  domain: 'example.com'
+  user_is_inactive:
+    no_activity_since:
+      years: 1
+      months: 0
 pagerduty:
   users_with_remote_access_monitoring: 'pd_users.json'
   bastion_users: 'hierahash/*hierahash.json'

aws/iam/resources.py

@@ -1,6 +1,8 @@
 import csv
 import time
 
+import pytest
+
 from conftest import botocore_client
 
 
@@ -15,7 +17,9 @@ def iam_users():
 
 
 def iam_admin_users():
-    return [user for user in iam_users_with_policies() if user_is_admin(user)]
+    return [
+        user for user in iam_users_with_policies_and_groups() if user_is_admin(user)
+    ]
 
 
 def iam_inline_policies(username):
@@ -114,6 +118,15 @@ def iam_users_with_policies():
     ]
 
 
+def iam_users_with_policies_and_groups():
+    """Users with thier associated Policies and Groups
+    """
+    return [
+        {**{"Groups": iam_user_groups(username=user["UserName"])}, **user}
+        for user in iam_users_with_policies()
+    ]
+
+
 def iam_admin_login_profiles():
     "http://botocore.readthedocs.io/en/latest/reference/services/iam.html#IAM.Client.get_login_profile"
     return iam_login_profiles(iam_admin_users())
@@ -277,18 +290,25 @@ def iam_admin_users_with_credential_report():
     return admins
 
 
-# FIXME
-# Substring matching is _not_ enough of a check, but works for testing.
-# The truth is that we probably shouldn't depend too much on the concept
-# of an "admin" in AWS, since that's not how the ACL's really work. We
-# should probably move towards concepts like "has write access", "can
-# read secrets", etc.
 def user_is_admin(user):
     for policy in user["Policies"]:
         if isinstance(policy, dict):
-            if "admin" in policy.get("PolicyName", "").lower():
+            if (
+                policy.get("PolicyName", "")
+                in pytest.config.custom_config.aws.admin_policies
+            ):
                 return True
 
+    for group in user.get("Groups", []):
+        if isinstance(group, dict):
+            if (
+                group.get("GroupName", "")
+                in pytest.config.custom_config.aws.admin_groups
+            ):
+                return True
+
+    return False
+
 
 def get_all_users_that_can_access_aws_account():
     """

config.yaml.example

@@ -22,6 +22,10 @@ regressions:
     test_param_id: '*mycustomgroup'
     comment: this was remediated by ops team
 aws:
+  admin_groups:
+    - "Administrators"
+  admin_policies:
+    - "AWSAdminRequireMFA"
   user_is_inactive:
     no_activity_since:
       years: 1

custom_config.py

@@ -64,6 +64,8 @@ class AWSConfig(CustomConfigMixin):
         self.whitelisted_ports_global = set(config.get("whitelisted_ports_global", []))
         self.whitelisted_ports = config.get("whitelisted_ports", [])
         self.access_key_expires_after = config.get("access_key_expires_after", None)
+        self.admin_policies = frozenset(config.get("admin_policies", []))
+        self.admin_groups = frozenset(config.get("admin_groups", []))
         super().__init__(config)
 
     def get_whitelisted_ports(self, test_id):