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