From cdc4385e2a0e86e3c24c1fccc4f93d474f3ebb7e Mon Sep 17 00:00:00 2001 From: Hector Acosta Date: Mon, 27 Jul 2020 06:32:11 -0500 Subject: [PATCH] Various changes to organizations endpoint (#3175) * Raise DuplicatePolicyException when a policy with the same name exists * Implement update_policy * Implement delete_policy --- IMPLEMENTATION_COVERAGE.md | 4 +- moto/organizations/exceptions.py | 9 ++ moto/organizations/models.py | 39 ++++++++- moto/organizations/responses.py | 9 ++ .../test_organizations_boto3.py | 86 +++++++++++++++++-- 5 files changed, 139 insertions(+), 8 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 8744f4759..d2696e6af 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -6113,7 +6113,7 @@ - [ ] decline_handshake - [ ] delete_organization - [ ] delete_organizational_unit -- [ ] delete_policy +- [X] delete_policy - [ ] deregister_delegated_administrator - [X] describe_account - [X] describe_create_account_status @@ -6152,7 +6152,7 @@ - [X] tag_resource - [X] untag_resource - [X] update_organizational_unit -- [ ] update_policy +- [X] update_policy ## outposts diff --git a/moto/organizations/exceptions.py b/moto/organizations/exceptions.py index 3649e3a13..036eeccbc 100644 --- a/moto/organizations/exceptions.py +++ b/moto/organizations/exceptions.py @@ -17,3 +17,12 @@ class DuplicateOrganizationalUnitException(JsonRESTError): "DuplicateOrganizationalUnitException", "An OU with the same name already exists.", ) + + +class DuplicatePolicyException(JsonRESTError): + code = 400 + + def __init__(self): + super(DuplicatePolicyException, self).__init__( + "DuplicatePolicyException", "A policy with the same name already exists." + ) diff --git a/moto/organizations/models.py b/moto/organizations/models.py index d538ec1b8..6c1dab15d 100644 --- a/moto/organizations/models.py +++ b/moto/organizations/models.py @@ -11,6 +11,7 @@ from moto.organizations import utils from moto.organizations.exceptions import ( InvalidInputException, DuplicateOrganizationalUnitException, + DuplicatePolicyException, ) @@ -409,6 +410,9 @@ class OrganizationsBackend(BaseBackend): def create_policy(self, **kwargs): new_policy = FakeServiceControlPolicy(self.org, **kwargs) + for policy in self.policies: + if kwargs["Name"] == policy.name: + raise DuplicatePolicyException self.policies.append(new_policy) return new_policy.describe() @@ -426,8 +430,26 @@ class OrganizationsBackend(BaseBackend): raise RESTError("InvalidInputException", "You specified an invalid value.") return policy.describe() + def get_policy_by_id(self, policy_id): + policy = next( + (policy for policy in self.policies if policy.id == policy_id), None + ) + if policy is None: + raise RESTError( + "PolicyNotFoundException", + "We can't find a policy with the PolicyId that you specified.", + ) + return policy + + def update_policy(self, **kwargs): + policy = self.get_policy_by_id(kwargs["PolicyId"]) + policy.name = kwargs.get("Name", policy.name) + policy.description = kwargs.get("Description", policy.description) + policy.content = kwargs.get("Content", policy.content) + return policy.describe() + def attach_policy(self, **kwargs): - policy = next((p for p in self.policies if p.id == kwargs["PolicyId"]), None) + policy = self.get_policy_by_id(kwargs["PolicyId"]) if re.compile(utils.ROOT_ID_REGEX).match(kwargs["TargetId"]) or re.compile( utils.OU_ID_REGEX ).match(kwargs["TargetId"]): @@ -462,6 +484,21 @@ class OrganizationsBackend(BaseBackend): Policies=[p.describe()["Policy"]["PolicySummary"] for p in self.policies] ) + def delete_policy(self, **kwargs): + for idx, policy in enumerate(self.policies): + if policy.id == kwargs["PolicyId"]: + if self.list_targets_for_policy(PolicyId=policy.id)["Targets"]: + raise RESTError( + "PolicyInUseException", + "The policy is attached to one or more entities. You must detach it from all roots, OUs, and accounts before performing this operation.", + ) + del self.policies[idx] + return + raise RESTError( + "PolicyNotFoundException", + "We can't find a policy with the PolicyId that you specified.", + ) + def list_policies_for_target(self, **kwargs): if re.compile(utils.OU_ID_REGEX).match(kwargs["TargetId"]): obj = next((ou for ou in self.ou if ou.id == kwargs["TargetId"]), None) diff --git a/moto/organizations/responses.py b/moto/organizations/responses.py index 616deacbc..a2bd028d9 100644 --- a/moto/organizations/responses.py +++ b/moto/organizations/responses.py @@ -105,6 +105,11 @@ class OrganizationsResponse(BaseResponse): self.organizations_backend.describe_policy(**self.request_params) ) + def update_policy(self): + return json.dumps( + self.organizations_backend.update_policy(**self.request_params) + ) + def attach_policy(self): return json.dumps( self.organizations_backend.attach_policy(**self.request_params) @@ -115,6 +120,10 @@ class OrganizationsResponse(BaseResponse): self.organizations_backend.list_policies(**self.request_params) ) + def delete_policy(self): + self.organizations_backend.delete_policy(**self.request_params) + return json.dumps({}) + def list_policies_for_target(self): return json.dumps( self.organizations_backend.list_policies_for_target(**self.request_params) diff --git a/tests/test_organizations/test_organizations_boto3.py b/tests/test_organizations/test_organizations_boto3.py index c2327dc40..5f14d83a5 100644 --- a/tests/test_organizations/test_organizations_boto3.py +++ b/tests/test_organizations/test_organizations_boto3.py @@ -420,18 +420,56 @@ def test_attach_policy(): account_id = client.create_account(AccountName=mockname, Email=mockemail)[ "CreateAccountStatus" ]["AccountId"] + + +@mock_organizations +def test_delete_policy(): + client = boto3.client("organizations", region_name="us-east-1") + org = client.create_organization(FeatureSet="ALL")["Organization"] + base_policies = client.list_policies(Filter="SERVICE_CONTROL_POLICY")["Policies"] + base_policies.should.have.length_of(1) policy_id = client.create_policy( Content=json.dumps(policy_doc01), Description="A dummy service control policy", Name="MockServiceControlPolicy", Type="SERVICE_CONTROL_POLICY", )["Policy"]["PolicySummary"]["Id"] - response = client.attach_policy(PolicyId=policy_id, TargetId=root_id) - response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) - response = client.attach_policy(PolicyId=policy_id, TargetId=ou_id) - response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) - response = client.attach_policy(PolicyId=policy_id, TargetId=account_id) + new_policies = client.list_policies(Filter="SERVICE_CONTROL_POLICY")["Policies"] + new_policies.should.have.length_of(2) + response = client.delete_policy(PolicyId=policy_id) response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) + new_policies = client.list_policies(Filter="SERVICE_CONTROL_POLICY")["Policies"] + new_policies.should.equal(base_policies) + new_policies.should.have.length_of(1) + + +@mock_organizations +def test_delete_policy_exception(): + client = boto3.client("organizations", region_name="us-east-1") + org = client.create_organization(FeatureSet="ALL")["Organization"] + non_existent_policy_id = utils.make_random_service_control_policy_id() + with assert_raises(ClientError) as e: + response = client.delete_policy(PolicyId=non_existent_policy_id) + ex = e.exception + ex.operation_name.should.equal("DeletePolicy") + ex.response["Error"]["Code"].should.equal("400") + ex.response["Error"]["Message"].should.contain("PolicyNotFoundException") + + # Attempt to delete an attached policy + policy_id = client.create_policy( + Content=json.dumps(policy_doc01), + Description="A dummy service control policy", + Name="MockServiceControlPolicy", + Type="SERVICE_CONTROL_POLICY", + )["Policy"]["PolicySummary"]["Id"] + root_id = client.list_roots()["Roots"][0]["Id"] + client.attach_policy(PolicyId=policy_id, TargetId=root_id) + with assert_raises(ClientError) as e: + response = client.delete_policy(PolicyId=policy_id) + ex = e.exception + ex.operation_name.should.equal("DeletePolicy") + ex.response["Error"]["Code"].should.equal("400") + ex.response["Error"]["Message"].should.contain("PolicyInUseException") @mock_organizations @@ -479,6 +517,44 @@ def test_attach_policy_exception(): ex.response["Error"]["Message"].should.contain("InvalidInputException") +@mock_organizations +def test_update_policy(): + client = boto3.client("organizations", region_name="us-east-1") + org = client.create_organization(FeatureSet="ALL")["Organization"] + + policy_dict = dict( + Content=json.dumps(policy_doc01), + Description="A dummy service control policy", + Name="MockServiceControlPolicy", + Type="SERVICE_CONTROL_POLICY", + ) + policy_id = client.create_policy(**policy_dict)["Policy"]["PolicySummary"]["Id"] + + for key in ("Description", "Name"): + response = client.update_policy(**{"PolicyId": policy_id, key: "foobar"}) + policy = client.describe_policy(PolicyId=policy_id) + policy["Policy"]["PolicySummary"][key].should.equal("foobar") + validate_service_control_policy(org, response["Policy"]) + + response = client.update_policy(PolicyId=policy_id, Content="foobar") + policy = client.describe_policy(PolicyId=policy_id) + policy["Policy"]["Content"].should.equal("foobar") + validate_service_control_policy(org, response["Policy"]) + + +@mock_organizations +def test_update_policy_exception(): + client = boto3.client("organizations", region_name="us-east-1") + org = client.create_organization(FeatureSet="ALL")["Organization"] + non_existent_policy_id = utils.make_random_service_control_policy_id() + with assert_raises(ClientError) as e: + response = client.update_policy(PolicyId=non_existent_policy_id) + ex = e.exception + ex.operation_name.should.equal("UpdatePolicy") + ex.response["Error"]["Code"].should.equal("400") + ex.response["Error"]["Message"].should.contain("PolicyNotFoundException") + + @mock_organizations def test_list_polices(): client = boto3.client("organizations", region_name="us-east-1")