From 5f7167ce62c0e6eedde12398e70adcce81c54c72 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Sun, 22 Aug 2021 10:49:48 +0100 Subject: [PATCH] Organisations - Detach policy, and asserts it actually happens (#3759) --- moto/organizations/models.py | 8 ++--- .../test_organizations_boto3.py | 33 ++++++++++++++----- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/moto/organizations/models.py b/moto/organizations/models.py index 708aa1c96..fbfd92b72 100644 --- a/moto/organizations/models.py +++ b/moto/organizations/models.py @@ -589,7 +589,7 @@ class OrganizationsBackend(BaseBackend): ).match(kwargs["TargetId"]): ou = next((ou for ou in self.ou if ou.id == kwargs["TargetId"]), None) if ou is not None: - if ou not in ou.attached_policies: + if policy not in ou.attached_policies: ou.attached_policies.append(policy) policy.attachments.append(ou) else: @@ -602,7 +602,7 @@ class OrganizationsBackend(BaseBackend): (a for a in self.accounts if a.id == kwargs["TargetId"]), None ) if account is not None: - if account not in account.attached_policies: + if policy not in account.attached_policies: account.attached_policies.append(policy) policy.attachments.append(account) else: @@ -866,7 +866,7 @@ class OrganizationsBackend(BaseBackend): if re.match(root_id_regex, target_id) or re.match(ou_id_regex, target_id): ou = next((ou for ou in self.ou if ou.id == target_id), None) if ou is not None: - if ou in ou.attached_policies: + if policy in ou.attached_policies: ou.attached_policies.remove(policy) policy.attachments.remove(ou) else: @@ -879,7 +879,7 @@ class OrganizationsBackend(BaseBackend): (account for account in self.accounts if account.id == target_id), None, ) if account is not None: - if account in account.attached_policies: + if policy in account.attached_policies: account.attached_policies.remove(policy) policy.attachments.remove(account) else: diff --git a/tests/test_organizations/test_organizations_boto3.py b/tests/test_organizations/test_organizations_boto3.py index ee3a92d0c..09cec4529 100644 --- a/tests/test_organizations/test_organizations_boto3.py +++ b/tests/test_organizations/test_organizations_boto3.py @@ -556,15 +556,30 @@ def test_detach_policy(): Name="MockServiceControlPolicy", Type="SERVICE_CONTROL_POLICY", )["Policy"]["PolicySummary"]["Id"] - client.attach_policy(PolicyId=policy_id, TargetId=ou_id) - client.attach_policy(PolicyId=policy_id, TargetId=root_id) - client.attach_policy(PolicyId=policy_id, TargetId=account_id) - response = client.detach_policy(PolicyId=policy_id, TargetId=ou_id) - response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) - response = client.detach_policy(PolicyId=policy_id, TargetId=root_id) - response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) - response = client.detach_policy(PolicyId=policy_id, TargetId=account_id) - response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) + # Attach/List/Detach policy + for name, target in [("OU", ou_id), ("Root", root_id), ("Account", account_id)]: + # + with sure.ensure("We should start with 0 policies"): + get_nonaws_policies(target, client).should.have.length_of(0) + # + client.attach_policy(PolicyId=policy_id, TargetId=target) + with sure.ensure("Expecting 1 policy after creation of target={0}", name): + get_nonaws_policies(target, client).should.have.length_of(1) + # + response = client.detach_policy(PolicyId=policy_id, TargetId=target) + response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) + with sure.ensure("Expecting 0 policies after deletion of target={0}", name): + get_nonaws_policies(target, client).should.have.length_of(0) + + +def get_nonaws_policies(account_id, client): + return [ + p + for p in client.list_policies_for_target( + TargetId=account_id, Filter="SERVICE_CONTROL_POLICY" + )["Policies"] + if not p["AwsManaged"] + ] @mock_organizations