Skip to content

Commit 1d470eb

Browse files
kmcquadekmcquade
authored
Fixes #211 - write_policy_with_template issue when used as a library (#212)
1 parent 9620b99 commit 1d470eb

File tree

5 files changed

+159
-129
lines changed

5 files changed

+159
-129
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Changelog
22

3+
## 0.8.5 (2020-08-17)
4+
* Fixes `write_policy_with_template` issue when used as a library (#211)
5+
36
## 0.8.4 (2020-08-16)
47
* IAM Data refresh
58
* Updated docs for query actions (#208)

‎examples/library-usage/writing/write_policy_with_access_levels.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77

88
if __name__ == '__main__':
99
crud_template = get_crud_template_dict()
10-
wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
1110
crud_template['read'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
1211
crud_template['write'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
13-
crud_template['list'].append("arn:aws:s3:::example-org-sbx-vmimport/stuff")
12+
crud_template['list'].append("arn:aws:s3:::mybucket/stuff")
1413
crud_template['permissions-management'].append("arn:aws:kms:us-east-1:123456789012:key/123456")
14+
wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
1515
crud_template['wildcard-only']['single-actions'].extend(wildcard_actions_to_add)
1616
crud_template['tagging'].append("arn:aws:ssm:us-east-1:123456789012:parameter/test")
17-
# Modify it
1817
policy = write_policy_with_template(crud_template)
1918
print(json.dumps(policy, indent=4))
2019

‎policy_sentry/bin/cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"""
33
Policy Sentry is a tool for generating least-privilege IAM Policies.
44
"""
5-
__version__ = "0.8.4"
5+
__version__ = "0.8.5"
66
import click
77
from policy_sentry import command
88

‎policy_sentry/writing/sid_group.py

Lines changed: 81 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -384,120 +384,88 @@ def process_template(self, cfg, minimize=None):
384384
Returns:
385385
Dictionary: The rendered IAM JSON Policy
386386
"""
387-
if "mode" in cfg.keys():
388-
if cfg["mode"] == "crud":
389-
logger.debug("CRUD mode selected")
390-
check_crud_schema(cfg)
391-
if "exclude-actions" in cfg:
392-
if cfg["exclude-actions"]:
393-
if cfg["exclude-actions"][0] != "":
394-
self.add_exclude_actions(cfg["exclude-actions"])
395-
if "wildcard-only" in cfg.keys():
396-
if "single-actions" in cfg["wildcard-only"]:
397-
if cfg["wildcard-only"]["single-actions"]:
398-
if cfg["wildcard-only"]["single-actions"][0] != "":
399-
provided_wildcard_actions = cfg["wildcard-only"][
400-
"single-actions"
401-
]
402-
logger.debug(
403-
f"Requested wildcard-only actions: {str(provided_wildcard_actions)}"
404-
)
405-
self.wildcard_only_single_actions = provided_wildcard_actions
406-
if "service-read" in cfg["wildcard-only"]:
407-
if cfg["wildcard-only"]["service-read"]:
408-
if cfg["wildcard-only"]["service-read"][0] != "":
409-
service_read = cfg["wildcard-only"]["service-read"]
410-
logger.debug(
411-
f"Requested wildcard-only actions: {str(service_read)}"
412-
)
413-
self.wildcard_only_service_read = service_read
414-
if "service-write" in cfg["wildcard-only"]:
415-
if cfg["wildcard-only"]["service-write"]:
416-
if cfg["wildcard-only"]["service-write"][0] != "":
417-
service_write = cfg["wildcard-only"]["service-write"]
418-
logger.debug(
419-
f"Requested wildcard-only actions: {str(service_write)}"
420-
)
421-
self.wildcard_only_service_write = service_write
422-
if "service-list" in cfg["wildcard-only"]:
423-
if cfg["wildcard-only"]["service-list"]:
424-
if cfg["wildcard-only"]["service-list"][0] != "":
425-
service_list = cfg["wildcard-only"]["service-list"]
426-
logger.debug(
427-
f"Requested wildcard-only actions: {str(service_list)}"
428-
)
429-
self.wildcard_only_service_list = service_list
430-
if "service-tagging" in cfg["wildcard-only"]:
431-
if cfg["wildcard-only"]["service-tagging"]:
432-
if cfg["wildcard-only"]["service-tagging"][0] != "":
433-
service_tagging = cfg["wildcard-only"][
434-
"service-tagging"
435-
]
436-
logger.debug(
437-
f"Requested wildcard-only actions: {str(service_tagging)}"
438-
)
439-
self.wildcard_only_service_tagging = service_tagging
440-
if "service-permissions-management" in cfg["wildcard-only"]:
441-
if cfg["wildcard-only"]["service-permissions-management"]:
442-
if (
443-
cfg["wildcard-only"]["service-permissions-management"][
444-
0
445-
]
446-
!= ""
447-
):
448-
449-
service_permissions_management = cfg["wildcard-only"][
450-
"service-permissions-management"
451-
]
452-
logger.debug(
453-
f"Requested wildcard-only actions: {str(service_permissions_management)}"
454-
)
455-
self.wildcard_only_service_permissions_management = service_permissions_management
456-
self.process_wildcard_only_actions()
457-
if "read" in cfg.keys():
458-
if cfg["read"] is not None and cfg["read"][0] != "":
459-
logger.debug(f"Requested access to arns: {str(cfg['read'])}")
460-
self.add_by_arn_and_access_level(cfg["read"], "Read")
461-
if "write" in cfg.keys():
462-
if cfg["write"] is not None and cfg["write"][0] != "":
463-
logger.debug(f"Requested access to arns: {str(cfg['write'])}")
464-
self.add_by_arn_and_access_level(cfg["write"], "Write")
465-
if "list" in cfg.keys():
466-
if cfg["list"] is not None and cfg["list"][0] != "":
467-
logger.debug(f"Requested access to arns: {str(cfg['list'])}")
468-
self.add_by_arn_and_access_level(cfg["list"], "List")
469-
if "permissions-management" in cfg.keys():
470-
if (
471-
cfg["permissions-management"] is not None
472-
and cfg["permissions-management"][0] != ""
473-
):
474-
logger.debug(
475-
f"Requested access to arns: {str(cfg['permissions-management'])}"
476-
)
477-
self.add_by_arn_and_access_level(
478-
cfg["permissions-management"], "Permissions management",
387+
if cfg.get("mode") == "crud":
388+
logger.debug("CRUD mode selected")
389+
check_crud_schema(cfg)
390+
# EXCLUDE ACTIONS
391+
if cfg.get("exclude-actions"):
392+
if cfg.get("exclude-actions")[0] != "":
393+
self.add_exclude_actions(cfg["exclude-actions"])
394+
# WILDCARD ONLY SECTION
395+
if cfg.get("wildcard-only"):
396+
if cfg.get("wildcard-only").get("single-actions"):
397+
if cfg["wildcard-only"]["single-actions"][0] != "":
398+
provided_wildcard_actions = cfg["wildcard-only"]["single-actions"]
399+
logger.debug(f"Requested wildcard-only actions: {str(provided_wildcard_actions)}")
400+
self.wildcard_only_single_actions = provided_wildcard_actions
401+
if cfg.get("wildcard-only").get("service-read"):
402+
if cfg["wildcard-only"]["service-read"][0] != "":
403+
service_read = cfg["wildcard-only"]["service-read"]
404+
logger.debug(f"Requested wildcard-only actions: {str(service_read)}")
405+
self.wildcard_only_service_read = service_read
406+
if cfg.get("wildcard-only").get("service-write"):
407+
if cfg["wildcard-only"]["service-write"][0] != "":
408+
service_write = cfg["wildcard-only"]["service-write"]
409+
logger.debug(f"Requested wildcard-only actions: {str(service_write)}")
410+
self.wildcard_only_service_write = service_write
411+
if cfg.get("wildcard-only").get("service-list"):
412+
if cfg["wildcard-only"]["service-list"][0] != "":
413+
service_list = cfg["wildcard-only"]["service-list"]
414+
logger.debug(f"Requested wildcard-only actions: {str(service_list)}")
415+
self.wildcard_only_service_list = service_list
416+
if cfg.get("wildcard-only").get("service-tagging"):
417+
if cfg["wildcard-only"]["service-tagging"][0] != "":
418+
service_tagging = cfg["wildcard-only"]["service-tagging"]
419+
logger.debug(f"Requested wildcard-only actions: {str(service_tagging)}")
420+
self.wildcard_only_service_tagging = service_tagging
421+
if cfg.get("wildcard-only").get("service-permissions-management"):
422+
if cfg["wildcard-only"]["service-permissions-management"][0] != "":
423+
service_permissions_management = cfg["wildcard-only"]["service-permissions-management"]
424+
logger.debug(f"Requested wildcard-only actions: {str(service_permissions_management)}")
425+
self.wildcard_only_service_permissions_management = service_permissions_management
426+
427+
# Process the wildcard-only section
428+
self.process_wildcard_only_actions()
429+
430+
# Standard access levels
431+
if cfg.get("read"):
432+
if cfg["read"][0] != "":
433+
logger.debug(f"Requested access to arns: {str(cfg['read'])}")
434+
self.add_by_arn_and_access_level(cfg["read"], "Read")
435+
if cfg.get("write"):
436+
if cfg["write"][0] != "":
437+
logger.debug(f"Requested access to arns: {str(cfg['write'])}")
438+
self.add_by_arn_and_access_level(cfg["write"], "Write")
439+
if cfg.get("list"):
440+
if cfg["list"][0] != "":
441+
logger.debug(f"Requested access to arns: {str(cfg['list'])}")
442+
self.add_by_arn_and_access_level(cfg["list"], "List")
443+
if cfg.get("tagging"):
444+
if cfg["tagging"][0] != "":
445+
logger.debug(f"Requested access to arns: {str(cfg['tagging'])}")
446+
self.add_by_arn_and_access_level(cfg["tagging"], "Tagging")
447+
if cfg.get("permissions-management"):
448+
if cfg["permissions-management"][0] != "":
449+
logger.debug(f"Requested access to arns: {str(cfg['permissions-management'])}")
450+
self.add_by_arn_and_access_level(cfg["permissions-management"], "Permissions management")
451+
452+
# SKIP RESOURCE CONSTRAINTS
453+
if cfg.get("skip-resource-constraints"):
454+
if cfg["skip-resource-constraints"][0] != "":
455+
logger.debug(
456+
f"Requested override: the actions {str(cfg['skip-resource-constraints'])} will "
457+
f"skip resource constraints."
458+
)
459+
self.add_skip_resource_constraints(cfg["skip-resource-constraints"])
460+
for skip_resource_constraints_action in self.skip_resource_constraints:
461+
self.add_action_without_resource_constraint(
462+
skip_resource_constraints_action, "SkipResourceConstraints"
479463
)
480-
if "tagging" in cfg.keys():
481-
if cfg["tagging"] is not None and cfg["tagging"][0] != "":
482-
logger.debug(f"Requested access to arns: {str(cfg['tagging'])}")
483-
self.add_by_arn_and_access_level(cfg["tagging"], "Tagging")
484-
if "skip-resource-constraints" in cfg.keys():
485-
if cfg["skip-resource-constraints"]:
486-
if cfg["skip-resource-constraints"][0] != "":
487-
logger.debug(
488-
f"Requested override: the actions {str(cfg['skip-resource-constraints'])} will "
489-
f"skip resource constraints."
490-
)
491-
self.add_skip_resource_constraints(cfg["skip-resource-constraints"])
492-
for skip_resource_constraints_action in self.skip_resource_constraints:
493-
self.add_action_without_resource_constraint(
494-
skip_resource_constraints_action, "SkipResourceConstraints"
495-
)
496-
if cfg["mode"] == "actions":
497-
check_actions_schema(cfg)
498-
if "actions" in cfg.keys():
499-
if cfg["actions"] is not None and cfg["actions"][0] != "":
500-
self.add_by_list_of_actions(cfg["actions"])
464+
elif cfg.get("mode") == "actions":
465+
check_actions_schema(cfg)
466+
if "actions" in cfg.keys():
467+
if cfg["actions"] is not None and cfg["actions"][0] != "":
468+
self.add_by_list_of_actions(cfg["actions"])
501469

502470
rendered_policy = self.get_rendered_policy(minimize)
503471
return rendered_policy

‎test/writing/test_write_policy_library_usage.py

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import unittest
22
import json
33

4-
from policy_sentry.command.write_policy import write_policy
4+
from policy_sentry.command.write_policy import write_policy_with_template
55
from policy_sentry.writing.sid_group import SidGroup
66
from policy_sentry.writing.template import (
77
get_crud_template_dict,
@@ -62,27 +62,27 @@
6262
]
6363
},
6464
{
65-
"Sid": "KmsPermissionsmanagementKey",
65+
"Sid": "SsmTaggingParameter",
6666
"Effect": "Allow",
6767
"Action": [
68-
"kms:CreateGrant",
69-
"kms:PutKeyPolicy",
70-
"kms:RetireGrant",
71-
"kms:RevokeGrant"
68+
"ssm:AddTagsToResource",
69+
"ssm:RemoveTagsFromResource"
7270
],
7371
"Resource": [
74-
"arn:aws:kms:us-east-1:123456789012:key/123456"
72+
"arn:aws:ssm:us-east-1:123456789012:parameter/test"
7573
]
7674
},
7775
{
78-
"Sid": "SsmTaggingParameter",
76+
"Sid": "KmsPermissionsmanagementKey",
7977
"Effect": "Allow",
8078
"Action": [
81-
"ssm:AddTagsToResource",
82-
"ssm:RemoveTagsFromResource"
79+
"kms:CreateGrant",
80+
"kms:PutKeyPolicy",
81+
"kms:RetireGrant",
82+
"kms:RevokeGrant"
8383
],
8484
"Resource": [
85-
"arn:aws:ssm:us-east-1:123456789012:parameter/test"
85+
"arn:aws:kms:us-east-1:123456789012:key/123456"
8686
]
8787
}
8888
]
@@ -181,6 +181,66 @@ def test_write_crud_policy_with_library_only(self):
181181
# print("desired_crud_policy")
182182
# print(json.dumps(desired_crud_policy, indent=4))
183183
# print("policy")
184-
print(json.dumps(policy, indent=4))
184+
# print(json.dumps(policy, indent=4))
185185
self.maxDiff = None
186186
self.assertDictEqual(desired_crud_policy, policy)
187+
188+
def test_gh_211_write_with_empty_access_level_lists(self):
189+
expected_results = {
190+
"Version": "2012-10-17",
191+
"Statement": [
192+
{
193+
"Sid": "MultMultNone",
194+
"Effect": "Allow",
195+
"Action": [
196+
"cloudhsm:DescribeClusters",
197+
"kms:CreateCustomKeyStore"
198+
],
199+
"Resource": [
200+
"*"
201+
]
202+
},
203+
{
204+
"Sid": "SecretsmanagerReadSecret",
205+
"Effect": "Allow",
206+
"Action": [
207+
"secretsmanager:DescribeSecret",
208+
"secretsmanager:GetResourcePolicy",
209+
"secretsmanager:GetSecretValue",
210+
"secretsmanager:ListSecretVersionIds"
211+
],
212+
"Resource": [
213+
"arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret"
214+
]
215+
},
216+
{
217+
"Sid": "SecretsmanagerWriteSecret",
218+
"Effect": "Allow",
219+
"Action": [
220+
"secretsmanager:CancelRotateSecret",
221+
"secretsmanager:DeleteSecret",
222+
"secretsmanager:PutSecretValue",
223+
"secretsmanager:RestoreSecret",
224+
"secretsmanager:RotateSecret",
225+
"secretsmanager:UpdateSecret",
226+
"secretsmanager:UpdateSecretVersionStage"
227+
],
228+
"Resource": [
229+
"arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret"
230+
]
231+
}
232+
]
233+
}
234+
crud_template = get_crud_template_dict()
235+
crud_template['read'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
236+
crud_template['write'].append("arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret")
237+
# crud_template['list'].append("arn:aws:s3:::mybucket/stuff")
238+
# by commenting out the line below, you should not get an IndexError
239+
# crud_template['permissions-management'].append("arn:aws:kms:us-east-1:123456789012:key/123456")
240+
# crud_template['tagging'].append("arn:aws:ssm:us-east-1:123456789012:parameter/test")
241+
wildcard_actions_to_add = ["kms:createcustomkeystore", "cloudhsm:describeclusters"]
242+
crud_template['wildcard-only']['single-actions'].extend(wildcard_actions_to_add)
243+
result = write_policy_with_template(crud_template)
244+
# print(json.dumps(result, indent=4))
245+
self.assertDictEqual(result, expected_results)
246+

0 commit comments

Comments
 (0)