Repository: salesforce/policy_sentry (python)

Issue:
Library usage: write_policy_with_template does not ignore empty lists
The example [here](https://github.com/salesforce/policy_sentry/blob/master/examples/library-usage/writing/write_policy_with_access_levels.py) works as-is, but if you remove any of the access levels, it breaks. The error indicates that this is because it does not ignore empty lists, even though it ignores empty strings.

```python
from policy_sentry.writing.template import get_crud_template_dict
from policy_sentry.command.write_policy import write_policy_with_template
import json


if __name__ == '__main__':
    crud_template = get_crud_template_dict()
    crud_template['read'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
    crud_template['write'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
    crud_template['list'].append("arn:aws:s3:::mybucket/stuff")
    # by commenting out the line below, you get an IndexError
    # crud_template['permissions-management'].append("arn:aws:kms:us-east-1:123456789012:key/123456")
    crud_template['tagging'].append("arn:aws:ssm:us-east-1:123456789012:parameter/test")
    wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
    crud_template['wildcard-only']['single-actions'].extend(wildcard_actions_to_add)
    policy = write_policy_with_template(crud_template)
    print(json.dumps(policy, indent=4))
```

Resulting error:

```
Traceback (most recent call last):
  File "./examples/library-usage/writing/write_policy_with_access_levels.py", line 18, in <module>
    policy = write_policy_with_template(crud_template)
  File "./policy_sentry/venv/lib/python3.8/site-packages/policy_sentry/command/write_policy.py", line 77, in write_policy_with_template
    policy = sid_group.process_template(cfg, minimize)
  File "./policy_sentry/venv/lib/python3.8/site-packages/policy_sentry/writing/sid_group.py", line 472, in process_template
    and cfg["permissions-management"][0] != ""
IndexError: list index out of range
```

Fix coming shortly.

Pull request:
Fixes #211 - write_policy_with_template issue when used as a library

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 96461973..e53b232c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.8.5 (2020-08-17)
+* Fixes `write_policy_with_template` issue when used as a library (#211)
+
 ## 0.8.4 (2020-08-16)
 * IAM Data refresh
 * Updated docs for query actions (#208)
diff --git a/examples/library-usage/writing/write_policy_with_access_levels.py b/examples/library-usage/writing/write_policy_with_access_levels.py
index 111e599b..1eb1cc99 100755
--- a/examples/library-usage/writing/write_policy_with_access_levels.py
+++ b/examples/library-usage/writing/write_policy_with_access_levels.py
@@ -7,14 +7,13 @@ import json
 
 if __name__ == '__main__':
     crud_template = get_crud_template_dict()
-    wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
     crud_template['read'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
     crud_template['write'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
-    crud_template['list'].append("arn:aws:s3:::example-org-sbx-vmimport/stuff")
+    crud_template['list'].append("arn:aws:s3:::mybucket/stuff")
     crud_template['permissions-management'].append("arn:aws:kms:us-east-1:123456789012:key/123456")
+    wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
     crud_template['wildcard-only']['single-actions'].extend(wildcard_actions_to_add)
     crud_template['tagging'].append("arn:aws:ssm:us-east-1:123456789012:parameter/test")
-    # Modify it
     policy = write_policy_with_template(crud_template)
     print(json.dumps(policy, indent=4))
 
diff --git a/policy_sentry/bin/cli.py b/policy_sentry/bin/cli.py
index 691e2988..cb43b45e 100755
--- a/policy_sentry/bin/cli.py
+++ b/policy_sentry/bin/cli.py
@@ -2,7 +2,7 @@
 """
     Policy Sentry is a tool for generating least-privilege IAM Policies.
 """
-__version__ = "0.8.4"
+__version__ = "0.8.5"
 import click
 from policy_sentry import command
 
diff --git a/policy_sentry/writing/sid_group.py b/policy_sentry/writing/sid_group.py
index 2a099acf..d5c4ac11 100644
--- a/policy_sentry/writing/sid_group.py
+++ b/policy_sentry/writing/sid_group.py
@@ -384,120 +384,88 @@ class SidGroup:
         Returns:
             Dictionary: The rendered IAM JSON Policy
         """
-        if "mode" in cfg.keys():
-            if cfg["mode"] == "crud":
-                logger.debug("CRUD mode selected")
-                check_crud_schema(cfg)
-                if "exclude-actions" in cfg:
-                    if cfg["exclude-actions"]:
-                        if cfg["exclude-actions"][0] != "":
-                            self.add_exclude_actions(cfg["exclude-actions"])
-                if "wildcard-only" in cfg.keys():
-                    if "single-actions" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["single-actions"]:
-                            if cfg["wildcard-only"]["single-actions"][0] != "":
-                                provided_wildcard_actions = cfg["wildcard-only"][
-                                    "single-actions"
-                                ]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(provided_wildcard_actions)}"
-                                )
-                                self.wildcard_only_single_actions = provided_wildcard_actions
-                    if "service-read" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["service-read"]:
-                            if cfg["wildcard-only"]["service-read"][0] != "":
-                                service_read = cfg["wildcard-only"]["service-read"]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(service_read)}"
-                                )
-                                self.wildcard_only_service_read = service_read
-                    if "service-write" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["service-write"]:
-                            if cfg["wildcard-only"]["service-write"][0] != "":
-                                service_write = cfg["wildcard-only"]["service-write"]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(service_write)}"
-                                )
-                                self.wildcard_only_service_write = service_write
-                    if "service-list" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["service-list"]:
-                            if cfg["wildcard-only"]["service-list"][0] != "":
-                                service_list = cfg["wildcard-only"]["service-list"]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(service_list)}"
-                                )
-                                self.wildcard_only_service_list = service_list
-                    if "service-tagging" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["service-tagging"]:
-                            if cfg["wildcard-only"]["service-tagging"][0] != "":
-                                service_tagging = cfg["wildcard-only"][
-                                    "service-tagging"
-                                ]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(service_tagging)}"
-                                )
-                                self.wildcard_only_service_tagging = service_tagging
-                    if "service-permissions-management" in cfg["wildcard-only"]:
-                        if cfg["wildcard-only"]["service-permissions-management"]:
-                            if (
-                                cfg["wildcard-only"]["service-permissions-management"][
-                                    0
-                                ]
-                                != ""
-                            ):
-
-                                service_permissions_management = cfg["wildcard-only"][
-                                    "service-permissions-management"
-                                ]
-                                logger.debug(
-                                    f"Requested wildcard-only actions: {str(service_permissions_management)}"
-                                )
-                                self.wildcard_only_service_permissions_management = service_permissions_management
-                    self.process_wildcard_only_actions()
-                if "read" in cfg.keys():
-                    if cfg["read"] is not None and cfg["read"][0] != "":
-                        logger.debug(f"Requested access to arns: {str(cfg['read'])}")
-                        self.add_by_arn_and_access_level(cfg["read"], "Read")
-                if "write" in cfg.keys():
-                    if cfg["write"] is not None and cfg["write"][0] != "":
-                        logger.debug(f"Requested access to arns: {str(cfg['write'])}")
-                        self.add_by_arn_and_access_level(cfg["write"], "Write")
-                if "list" in cfg.keys():
-                    if cfg["list"] is not None and cfg["list"][0] != "":
-                        logger.debug(f"Requested access to arns: {str(cfg['list'])}")
-                        self.add_by_arn_and_access_level(cfg["list"], "List")
-                if "permissions-management" in cfg.keys():
-                    if (
-                        cfg["permissions-management"] is not None
-                        and cfg["permissions-management"][0] != ""
-                    ):
-                        logger.debug(
-                            f"Requested access to arns: {str(cfg['permissions-management'])}"
-                        )
-                        self.add_by_arn_and_access_level(
-                            cfg["permissions-management"], "Permissions management",
+        if cfg.get("mode") == "crud":
+            logger.debug("CRUD mode selected")
+            check_crud_schema(cfg)
+            # EXCLUDE ACTIONS
+            if cfg.get("exclude-actions"):
+                if cfg.get("exclude-actions")[0] != "":
+                    self.add_exclude_actions(cfg["exclude-actions"])
+            # WILDCARD ONLY SECTION
+            if cfg.get("wildcard-only"):
+                if cfg.get("wildcard-only").get("single-actions"):
+                    if cfg["wildcard-only"]["single-actions"][0] != "":
+                        provided_wildcard_actions = cfg["wildcard-only"]["single-actions"]
+                        logger.debug(f"Requested wildcard-only actions: {str(provided_wildcard_actions)}")
+                        self.wildcard_only_single_actions = provided_wildcard_actions
+                if cfg.get("wildcard-only").get("service-read"):
+                    if cfg["wildcard-only"]["service-read"][0] != "":
+                        service_read = cfg["wildcard-only"]["service-read"]
+                        logger.debug(f"Requested wildcard-only actions: {str(service_read)}")
+                        self.wildcard_only_service_read = service_read
+                if cfg.get("wildcard-only").get("service-write"):
+                    if cfg["wildcard-only"]["service-write"][0] != "":
+                        service_write = cfg["wildcard-only"]["service-write"]
+                        logger.debug(f"Requested wildcard-only actions: {str(service_write)}")
+                        self.wildcard_only_service_write = service_write
+                if cfg.get("wildcard-only").get("service-list"):
+                    if cfg["wildcard-only"]["service-list"][0] != "":
+                        service_list = cfg["wildcard-only"]["service-list"]
+                        logger.debug(f"Requested wildcard-only actions: {str(service_list)}")
+                        self.wildcard_only_service_list = service_list
+                if cfg.get("wildcard-only").get("service-tagging"):
+                    if cfg["wildcard-only"]["service-tagging"][0] != "":
+                        service_tagging = cfg["wildcard-only"]["service-tagging"]
+                        logger.debug(f"Requested wildcard-only actions: {str(service_tagging)}")
+                        self.wildcard_only_service_tagging = service_tagging
+                if cfg.get("wildcard-only").get("service-permissions-management"):
+                    if cfg["wildcard-only"]["service-permissions-management"][0] != "":
+                        service_permissions_management = cfg["wildcard-only"]["service-permissions-management"]
+                        logger.debug(f"Requested wildcard-only actions: {str(service_permissions_management)}")
+                        self.wildcard_only_service_permissions_management = service_permissions_management
+
+            # Process the wildcard-only section
+            self.process_wildcard_only_actions()
+
+            # Standard access levels
+            if cfg.get("read"):
+                if cfg["read"][0] != "":
+                    logger.debug(f"Requested access to arns: {str(cfg['read'])}")
+                    self.add_by_arn_and_access_level(cfg["read"], "Read")
+            if cfg.get("write"):
+                if cfg["write"][0] != "":
+                    logger.debug(f"Requested access to arns: {str(cfg['write'])}")
+                    self.add_by_arn_and_access_level(cfg["write"], "Write")
+            if cfg.get("list"):
+                if cfg["list"][0] != "":
+                    logger.debug(f"Requested access to arns: {str(cfg['list'])}")
+                    self.add_by_arn_and_access_level(cfg["list"], "List")
+            if cfg.get("tagging"):
+                if cfg["tagging"][0] != "":
+                    logger.debug(f"Requested access to arns: {str(cfg['tagging'])}")
+                    self.add_by_arn_and_access_level(cfg["tagging"], "Tagging")
+            if cfg.get("permissions-management"):
+                if cfg["permissions-management"][0] != "":
+                    logger.debug(f"Requested access to arns: {str(cfg['permissions-management'])}")
+                    self.add_by_arn_and_access_level(cfg["permissions-management"], "Permissions management")
+
+            # SKIP RESOURCE CONSTRAINTS
+            if cfg.get("skip-resource-constraints"):
+                if cfg["skip-resource-constraints"][0] != "":
+                    logger.debug(
+                        f"Requested override: the actions {str(cfg['skip-resource-constraints'])} will "
+                        f"skip resource constraints."
+                    )
+                    self.add_skip_resource_constraints(cfg["skip-resource-constraints"])
+                    for skip_resource_constraints_action in self.skip_resource_constraints:
+                        self.add_action_without_resource_constraint(
+                            skip_resource_constraints_action, "SkipResourceConstraints"
                         )
-                if "tagging" in cfg.keys():
-                    if cfg["tagging"] is not None and cfg["tagging"][0] != "":
-                        logger.debug(f"Requested access to arns: {str(cfg['tagging'])}")
-                        self.add_by_arn_and_access_level(cfg["tagging"], "Tagging")
-                if "skip-resource-constraints" in cfg.keys():
-                    if cfg["skip-resource-constraints"]:
-                        if cfg["skip-resource-constraints"][0] != "":
-                            logger.debug(
-                                f"Requested override: the actions {str(cfg['skip-resource-constraints'])} will "
-                                f"skip resource constraints."
-                            )
-                            self.add_skip_resource_constraints(cfg["skip-resource-constraints"])
-                            for skip_resource_constraints_action in self.skip_resource_constraints:
-                                self.add_action_without_resource_constraint(
-                                    skip_resource_constraints_action, "SkipResourceConstraints"
-                                )
-            if cfg["mode"] == "actions":
-                check_actions_schema(cfg)
-                if "actions" in cfg.keys():
-                    if cfg["actions"] is not None and cfg["actions"][0] != "":
-                        self.add_by_list_of_actions(cfg["actions"])
+        elif cfg.get("mode") == "actions":
+            check_actions_schema(cfg)
+            if "actions" in cfg.keys():
+                if cfg["actions"] is not None and cfg["actions"][0] != "":
+                    self.add_by_list_of_actions(cfg["actions"])
 
         rendered_policy = self.get_rendered_policy(minimize)
         return rendered_policy


Test patch:
diff --git a/test/writing/test_write_policy_library_usage.py b/test/writing/test_write_policy_library_usage.py
index f80aba45..5c2f2990 100644
--- a/test/writing/test_write_policy_library_usage.py
+++ b/test/writing/test_write_policy_library_usage.py
@@ -1,7 +1,7 @@
 import unittest
 import json
 
-from policy_sentry.command.write_policy import write_policy
+from policy_sentry.command.write_policy import write_policy_with_template
 from policy_sentry.writing.sid_group import SidGroup
 from policy_sentry.writing.template import (
     get_crud_template_dict,
@@ -62,27 +62,27 @@ desired_crud_policy = {
             ]
         },
         {
-            "Sid": "KmsPermissionsmanagementKey",
+            "Sid": "SsmTaggingParameter",
             "Effect": "Allow",
             "Action": [
-                "kms:CreateGrant",
-                "kms:PutKeyPolicy",
-                "kms:RetireGrant",
-                "kms:RevokeGrant"
+                "ssm:AddTagsToResource",
+                "ssm:RemoveTagsFromResource"
             ],
             "Resource": [
-                "arn:aws:kms:us-east-1:123456789012:key/123456"
+                "arn:aws:ssm:us-east-1:123456789012:parameter/test"
             ]
         },
         {
-            "Sid": "SsmTaggingParameter",
+            "Sid": "KmsPermissionsmanagementKey",
             "Effect": "Allow",
             "Action": [
-                "ssm:AddTagsToResource",
-                "ssm:RemoveTagsFromResource"
+                "kms:CreateGrant",
+                "kms:PutKeyPolicy",
+                "kms:RetireGrant",
+                "kms:RevokeGrant"
             ],
             "Resource": [
-                "arn:aws:ssm:us-east-1:123456789012:parameter/test"
+                "arn:aws:kms:us-east-1:123456789012:key/123456"
             ]
         }
     ]
@@ -181,6 +181,66 @@ class WritePolicyWithLibraryOnly(unittest.TestCase):
         # print("desired_crud_policy")
         # print(json.dumps(desired_crud_policy, indent=4))
         # print("policy")
-        print(json.dumps(policy, indent=4))
+        # print(json.dumps(policy, indent=4))
         self.maxDiff = None
         self.assertDictEqual(desired_crud_policy, policy)
+
+    def test_gh_211_write_with_empty_access_level_lists(self):
+        expected_results = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "MultMultNone",
+                    "Effect": "Allow",
+                    "Action": [
+                        "cloudhsm:DescribeClusters",
+                        "kms:CreateCustomKeyStore"
+                    ],
+                    "Resource": [
+                        "*"
+                    ]
+                },
+                {
+                    "Sid": "SecretsmanagerReadSecret",
+                    "Effect": "Allow",
+                    "Action": [
+                        "secretsmanager:DescribeSecret",
+                        "secretsmanager:GetResourcePolicy",
+                        "secretsmanager:GetSecretValue",
+                        "secretsmanager:ListSecretVersionIds"
+                    ],
+                    "Resource": [
+                        "arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret"
+                    ]
+                },
+                {
+                    "Sid": "SecretsmanagerWriteSecret",
+                    "Effect": "Allow",
+                    "Action": [
+                        "secretsmanager:CancelRotateSecret",
+                        "secretsmanager:DeleteSecret",
+                        "secretsmanager:PutSecretValue",
+                        "secretsmanager:RestoreSecret",
+                        "secretsmanager:RotateSecret",
+                        "secretsmanager:UpdateSecret",
+                        "secretsmanager:UpdateSecretVersionStage"
+                    ],
+                    "Resource": [
+                        "arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret"
+                    ]
+                }
+            ]
+        }
+        crud_template = get_crud_template_dict()
+        crud_template['read'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
+        crud_template['write'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
+        # crud_template['list'].append("arn:aws:s3:::mybucket/stuff")
+        # by commenting out the line below, you should not get an IndexError
+        # crud_template['permissions-management'].append("arn:aws:kms:us-east-1:123456789012:key/123456")
+        # crud_template['tagging'].append("arn:aws:ssm:us-east-1:123456789012:parameter/test")
+        wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
+        crud_template['wildcard-only']['single-actions'].extend(wildcard_actions_to_add)
+        result = write_policy_with_template(crud_template)
+        # print(json.dumps(result, indent=4))
+        self.assertDictEqual(result, expected_results)
+