From dbeba8bbae3a9c6ae2bd9c3c1ec2bbb3988d768e Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 23 Mar 2023 10:03:36 +1000
Subject: [PATCH 1/2] Factor out the bot note into a helper function

---
 damspam/__init__.py | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/damspam/__init__.py b/damspam/__init__.py
index e4f8f1f..63d55cf 100644
--- a/damspam/__init__.py
+++ b/damspam/__init__.py
@@ -49,6 +49,16 @@ def not_read_only(ro: Union[Any, bool], message) -> bool:
     return not readonly
 
 
+def create_bot_note(
+    ro: Union[Any, bool], issue: gitlab.v4.objects.ProjectIssue, log_message: str
+):
+    """
+    If not in readonly mode, create a note with the standard bot message.
+    """
+    if not_read_only(ro, log_message):
+        issue.notes.create({"body": BOT_MESSAGE})
+
+
 @attr.s
 class Payload:
     """
@@ -427,21 +437,22 @@ class DamSpamIssue:
             return
 
         if self.issue.author["state"] != "active":
-            if not_read_only(
+            create_bot_note(
                 self,
+                self.issue,
                 f"Author '{self.issue.author['name']}' is already {self.issue.author['state']}",
-            ):
-                self.issue.notes.create({"body": BOT_MESSAGE})
+            )
             return
 
         # Safety check: if we have more than 2 commenters on the issue, we
         # say we don't know what to do.
         commenters = {n.author["id"] for n in self.issue.notes.list()}
         if len(commenters) > 2:
-            if not_read_only(
-                self, "Three or more people involved, please report directly"
-            ):
-                self.issue.notes.create({"body": BOT_MESSAGE})
+            create_bot_note(
+                self,
+                self.issue,
+                "Three or more people involved, please report directly",
+            )
             return
 
         # Block the user first, ideally that way they don't get notified about the rest of the
@@ -459,11 +470,11 @@ class DamSpamIssue:
             now = datetime.now(tz=timezone.utc)
             dt = timedelta(weeks=26)
             if created_at < now - dt:
-                if not_read_only(
+                create_bot_note(
                     self,
+                    self.issue,
                     f"Account of alleged spammer {spammer.name} is more than 6m old, skipping",
-                ):
-                    self.issue.notes.create({"body": BOT_MESSAGE})
+                )
                 return
         except (ValueError, TypeError) as e:
             logger.error(f"Unable to parse created_at from spammer: {e}")
-- 
GitLab


From 64aeaf45e97d389dc1de7311f69a93ab2855f45c Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 23 Mar 2023 09:58:35 +1000
Subject: [PATCH 2/2] Protect the ghost user too

Add a new ProtectedUserList singleton that gives us access/lookup for
the various protected users we want.
---
 damspam/__init__.py   | 70 +++++++++++++++++++++++++++++++++++++++----
 tests/test_damspam.py | 21 +++++++++++++
 2 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/damspam/__init__.py b/damspam/__init__.py
index 63d55cf..bff792d 100644
--- a/damspam/__init__.py
+++ b/damspam/__init__.py
@@ -13,8 +13,8 @@ import logging
 logging.basicConfig(format="%(levelname)7s| %(name)s: %(message)s")
 logger = logging.getLogger("damspam")
 
-SPAMBOT_USER_ID = 85713
-
+SPAMBOT_USER_ID = 85713  # @spambot
+GHOST_USER_ID = 2  # @ghost
 
 """
 We only use one label across all projects. No way to change this so we don't
@@ -59,6 +59,60 @@ def create_bot_note(
         issue.notes.create({"body": BOT_MESSAGE})
 
 
+@attr.s
+class ProtectedUser:
+    id: int = attr.ib()
+    name: str = attr.ib()
+
+
+@attr.s
+class ProtectedUserList:
+    """This is a singleton class"""
+
+    _protected_users: dict[int, ProtectedUser] = attr.ib()
+
+    _instance: "ProtectedUserList" = None  # type: ignore
+
+    @classmethod
+    def instance(cls) -> "ProtectedUserList":
+        if not cls._instance:
+            cls._instance = ProtectedUserList(
+                protected_users={  # type: ignore
+                    u.id: u
+                    for u in [
+                        ProtectedUser(GHOST_USER_ID, "ghost"),
+                        ProtectedUser(SPAMBOT_USER_ID, "spambot"),
+                    ]
+                },
+            )
+        return cls._instance
+
+    @property
+    def ghost(self) -> ProtectedUser:
+        return self._protected_users[GHOST_USER_ID]
+
+    @property
+    def spambot(self) -> ProtectedUser:
+        return self._protected_users[SPAMBOT_USER_ID]
+
+    def lookup_user_by_id(self, id: int) -> Optional[ProtectedUser]:
+        return self._protected_users.get(id, None)
+
+    def lookup_user_by_name(self, name: str) -> Optional[ProtectedUser]:
+        try:
+            return next(
+                filter(
+                    lambda u: u is not None and u.name == name,
+                    self._protected_users.values(),
+                )
+            )
+        except StopIteration:
+            return None
+
+    def is_protected(self, id: int) -> bool:
+        return id in self._protected_users
+
+
 @attr.s
 class Payload:
     """
@@ -432,8 +486,11 @@ class DamSpamIssue:
         confidential and assigned to the spambot.
         """
 
-        if self.issue.assignee == SPAMBOT_USER_ID:
-            logger.info("Issue already assigned to spambot, skipping")
+        protected_user = ProtectedUserList.instance().lookup_user_by_id(
+            self.issue.assignee
+        )
+        if protected_user is not None:
+            logger.info(f"Issue already assigned to {protected_user.name}, skipping")
             return
 
         if self.issue.author["state"] != "active":
@@ -562,8 +619,9 @@ class DamSpamUser:
     readonly: bool = attr.ib(default=False)
 
     def delete(self):
-        if self.user.id == SPAMBOT_USER_ID:
-            logger.error("Nice try, buddy")
+        protected_user = ProtectedUserList.instance().lookup_user_by_id(self.user.id)
+        if protected_user is not None:
+            logger.error(f"User @{protected_user.name} cannot be deleted.")
             return
 
         if self.user.is_admin:
diff --git a/tests/test_damspam.py b/tests/test_damspam.py
index da6504e..bd18f9f 100644
--- a/tests/test_damspam.py
+++ b/tests/test_damspam.py
@@ -10,6 +10,7 @@ from damspam import (
     DamSpamUser,
     DamSpamUserList,
     BuilderError,
+    ProtectedUserList,
     SPAMBOT_USER_ID,
     BOT_MESSAGE,
 )
@@ -476,3 +477,23 @@ def test_list_spammers(readonly):
     assert len(spammer.issues) == 1, "Expected only one spam issue"
     spam = spammer.issues[0]
     assert spam.id == 99
+
+
+@pytest.mark.parametrize("username", ("ghost", "spambot"))
+def test_protected_users(username):
+    userlist = ProtectedUserList.instance()
+    if username == "ghost":
+        protected = userlist.ghost
+    elif username == "spambot":
+        protected = userlist.spambot
+    else:
+        assert False, "Unexpected argument"
+    assert protected is not None
+    assert protected.name == username
+    assert userlist.lookup_user_by_name(username) is not None
+
+    # other names are not protected
+    not_protected = userlist.lookup_user_by_name("foobar")
+    assert not_protected is None
+    not_protected = userlist.lookup_user_by_id(123)
+    assert not_protected is None
-- 
GitLab