Merge pull request #108 from CodethinkLabs/bobclough/suitecrm_disable…

…_users

target_suitecrm: add functionality to disable accounts instead of delete

lifecycle/target_suitecrm.py

@@ -40,6 +40,7 @@ def __init__(self, *args):
         "users_cleanup",
         "excluded_usernames",
         "admin_groups",
+        "delete_absent_users",
     }
 
     supported_user_fields = {
@@ -60,6 +61,7 @@ def __init__(self, *args):
         "api_page_size": 20,
         "stages": ["users_create", "users_sync", "users_disable", "users_cleanup"],
         "excluded_usernames": [],
+        "delete_absent_users": True,
     }
 
     def process_groups_patterns(self, groups_patterns: list[str]) -> list[str]:
@@ -477,23 +479,57 @@ def users_cleanup(self, diff: ModelDifference):
         logging.debug("Excluded usernames: %s", self.config["excluded_usernames"])
         for user in diff.removed_users.values():
             _id = self._users_data[user.username]["id"]
-            logging.debug(
-                "Attempting to delete: %s. Is user excluded: %s",
-                user.username,
-                user.username in self.config["excluded_usernames"],
-            )
-            if user.username not in self.config["excluded_usernames"]:
-                deletion_record = {
-                    "data": {
-                        "type": "User",
-                        "id": _id,
-                        "attributes": {
-                            "deleted": 1,
-                        },
+            if self.config["delete_absent_users"]:
+                if user.username not in self.config["excluded_usernames"]:
+                    logging.debug(
+                        "Attempting to delete %s.",
+                        user.username,
+                    )
+                    deletion_record = {
+                        "data": {
+                            "type": "User",
+                            "id": _id,
+                            "attributes": {
+                                "deleted": 1,
+                            },
+                        }
                     }
-                }
-                logging.debug("Deleting user: %s", user.username)
-                self._request("/Api/V8/module", method="PATCH", json=deletion_record)
+                    logging.debug("Deleting user: %s", user.username)
+                    self._request(
+                        "/Api/V8/module", method="PATCH", json=deletion_record
+                    )
+                else:
+                    logging.debug(
+                        "Not attempting to delete %s as they are in excluded_usernames",
+                        user.username,
+                    )
+            else:
+                if not user.locked:
+                    if user.username not in self.config["excluded_usernames"]:
+                        logging.debug("Attempting to disable: %s.", user.username)
+                        disablement_record = {
+                            "data": {
+                                "type": "User",
+                                "id": _id,
+                                "attributes": {
+                                    "status": "Inactive",
+                                },
+                            }
+                        }
+                        logging.debug("Disabling account for %s", user.username)
+                        self._request(
+                            "/Api/V8/module", method="PATCH", json=disablement_record
+                        )
+                    else:
+                        logging.debug(
+                            "Not attempting to disable %s as they are in excluded_usernames",
+                            user.username,
+                        )
+                else:
+                    logging.debug(
+                        "Not attempting to disable %s as they are already locked",
+                        user.username,
+                    )
 
     def _sync_emails_for_users(self, diff: ModelDifference):
         for user in diff.changed_users.values():

setup.py

@@ -15,6 +15,7 @@
             "pytest-cov",
             "pytest-mock",
             "pytest-pylint",
+            "black",
         ],
     },
     python_requires=">=3.7",

tests/test_target_suitecrm.py

@@ -13,14 +13,30 @@
 
 
 @pytest.fixture(name="basic_config")
-def fixture_config():
+def fixture_basic_config():
     """Create a config"""
     config = {
         "url": "127.0.0.1:8080",
         "api_username": "user",
         "api_password": "bitnami",
         "api_client_id": "asd",
         "api_client_secret": "secret",
+        "excluded_usernames": ["excluded"],
+    }
+    return config
+
+
+@pytest.fixture(name="users_disable_config")
+def fixture_users_disable_config():
+    """Create a config"""
+    config = {
+        "url": "127.0.0.1:8080",
+        "api_username": "user",
+        "api_password": "bitnami",
+        "api_client_id": "asd",
+        "api_client_secret": "secret",
+        "delete_absent_users": False,
+        "excluded_usernames": ["excluded"],
     }
     return config
 
@@ -376,26 +392,123 @@ def test_user_delete(basic_target, suitecrm_server):
             "status": "Active",
         }
     )
+    server.create_user(
+        {
+            "user_name": "excluded",
+            "first_name": "Ernie",
+            "last_name": "Excluded",
+            "email1": "[email protected]",
+            "status": "Active",
+        }
+    )
 
+    remaining_user = User(
+        "adalice", forename="Ad", surname="Alice", email=("[email protected]",)
+    )
     deleted_user = User(
         "basicuser", forename="Basic", surname="Bob", email=("[email protected]",)
     )
+    excluded_user = User(
+        "excluded",
+        forename="Ernie",
+        surname="Excluded",
+        email=("[email protected]",),
+    )
+
+    diff = ModelDifference(
+        source_users={
+            "adAlice": remaining_user,
+            "basicuser": deleted_user,
+            "excluded": excluded_user,
+        },
+        target_users={"adAlice": remaining_user},
+        added_users={},
+        changed_users={},
+        unchanged_users={"adAlice": remaining_user},
+        removed_users={"basicuser": deleted_user, "excluded": excluded_user},
+    )
+
+    basic_target.users_cleanup(diff)
+    users = server.search_by_type("User")
+    assert any(user["attributes"]["first_name"] == "Ad" for user in users)
+    assert any(user["attributes"]["first_name"] == "Ernie" for user in users)
+
+
+def test_users_disable(users_disable_target, suitecrm_server):
+    """Delete a user and check it's been deleted"""
+    server = suitecrm_server([])
+    server.create_user(
+        {
+            "user_name": "adalice",
+            "first_name": "Ad",
+            "last_name": "Alice",
+            "email1": "[email protected]",
+            "status": "Active",
+        }
+    )
+    server.create_user(
+        {
+            "user_name": "basicuser",
+            "first_name": "Basic",
+            "last_name": "Bob",
+            "full_name": "Basic Bob",
+            "email1": "[email protected]",
+            "status": "Active",
+        }
+    )
+    server.create_user(
+        {
+            "user_name": "excluded",
+            "first_name": "Ernie",
+            "last_name": "Excluded",
+            "email1": "[email protected]",
+            "status": "Active",
+        }
+    )
+
     remaining_user = User(
         "adalice", forename="Ad", surname="Alice", email=("[email protected]",)
     )
+    deleted_user = User(
+        "basicuser", forename="Basic", surname="Bob", email=("[email protected]",)
+    )
+    excluded_user = User(
+        "excluded",
+        forename="Ernie",
+        surname="Excluded",
+        email=("[email protected]",),
+    )
 
     diff = ModelDifference(
-        source_users={"basicuser": deleted_user, "adAlice": remaining_user},
+        source_users={
+            "adAlice": remaining_user,
+            "basicuser": deleted_user,
+            "excluded": excluded_user,
+        },
         target_users={"adAlice": remaining_user},
         added_users={},
         changed_users={},
         unchanged_users={"adAlice": remaining_user},
-        removed_users={"basicuser": deleted_user},
+        removed_users={"basicuser": deleted_user, "excluded": excluded_user},
     )
 
-    basic_target.users_cleanup(diff)
+    users_disable_target.users_cleanup(diff)
     users = server.search_by_type("User")
-    assert users[0]["attributes"]["first_name"] == "Ad"
+    assert any(
+        user["attributes"]["first_name"] == "Ad"
+        and user["attributes"]["status"] == "Active"
+        for user in users
+    )
+    assert any(
+        user["attributes"]["first_name"] == "Basic"
+        and user["attributes"]["status"] == "Inactive"
+        for user in users
+    )
+    assert any(
+        user["attributes"]["first_name"] == "Ernie"
+        and user["attributes"]["status"] == "Active"
+        for user in users
+    )
 
 
 def test_groups_emails_sync_no_changes(basic_config, suitecrm_server):
@@ -595,3 +708,10 @@ def fixture_basic_target(basic_config):
     """Create a TargetSuiteCRM with default config"""
     target = TargetSuiteCRM(basic_config, None)
     return target
+
+
+@pytest.fixture(name="users_disable_target")
+def fixture_users_disable_target(users_disable_config):
+    """Create a TargetSuiteCRM with config set up to disable users instead of deleting them"""
+    target = TargetSuiteCRM(users_disable_config, None)
+    return target