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

@@ -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
 </details>
 
 ## outposts

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."
+        )

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)

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)

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)
+    new_policies = client.list_policies(Filter="SERVICE_CONTROL_POLICY")["Policies"]
-    response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200)
+    new_policies.should.have.length_of(2)
-    response = client.attach_policy(PolicyId=policy_id, TargetId=ou_id)
+    response = client.delete_policy(PolicyId=policy_id)
-    response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200)
-    response = client.attach_policy(PolicyId=policy_id, TargetId=account_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")