From b13c40872e642d99f5ca9276d90ff1e348987657 Mon Sep 17 00:00:00 2001 From: Eric <34304046+EricTRL@users.noreply.github.com> Date: Sat, 2 Sep 2023 11:14:36 +0200 Subject: [PATCH 1/3] Fix filter_active duplicate entries Member.objects.filter_active should not return duplicates of members with a membership in multiple active years. --- membership_file/models.py | 2 +- membership_file/tests/tests_model.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/membership_file/models.py b/membership_file/models.py index 7dca4e2b..78f5b2ed 100644 --- a/membership_file/models.py +++ b/membership_file/models.py @@ -33,7 +33,7 @@ def filter_active(self): # Active membership year set; only return members registered in these years. # Honorary members are always active, regardless of active years - return filter.filter(models.Q(memberyear__in=active_years) | models.Q(is_honorary_member=True)) + return filter.filter(models.Q(memberyear__in=active_years) | models.Q(is_honorary_member=True)).distinct() # The Member model represents a Member in the membership file diff --git a/membership_file/tests/tests_model.py b/membership_file/tests/tests_model.py index 1f5338c8..44ce1713 100644 --- a/membership_file/tests/tests_model.py +++ b/membership_file/tests/tests_model.py @@ -70,6 +70,17 @@ def test_filter_active_active_year(self): self.assertIn(self._honorary, active_members) self.assertEqual(len(active_members), 2) + def test_filter_active_duplicates(self): + """Tests if filter_active() accounts for duplicates when multiple years are active simultaneously""" + year = MemberYear.objects.create(name="1980", is_active=True) + year.members.set([self._active, self._deregistered, self._pending_deletion]) + + active_members = Member.objects.filter_active() + self.assertIn(self._active, active_members) + self.assertNotIn(self._inactive, active_members) + self.assertIn(self._honorary, active_members) + self.assertEqual(len(active_members), 2) + # Tests methods related to the Member model class MemberModelTest(TestCase): From 0a32facc3b6713840f71972075016911c664ada9 Mon Sep 17 00:00:00 2001 From: Eric <34304046+EricTRL@users.noreply.github.com> Date: Sat, 2 Sep 2023 11:31:29 +0200 Subject: [PATCH 2/3] Dev Rspamd settings Make sure that Rspamd settings set up during development do not override settings set up in a production environment. --- mailcow_integration/squire_mailcow.py | 8 ++++---- mailcow_integration/tests/api/test_interfaces.py | 4 ++-- mailcow_integration/tests/tests_squire_mailcow.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mailcow_integration/squire_mailcow.py b/mailcow_integration/squire_mailcow.py index 63fe67af..70abc01b 100644 --- a/mailcow_integration/squire_mailcow.py +++ b/mailcow_integration/squire_mailcow.py @@ -1,10 +1,10 @@ from enum import Enum import re -from typing import Dict, Generator, Optional, List, Tuple +from typing import Dict, Optional, List, Tuple from django.apps import apps from django.conf import settings -from django.db.models import Q, QuerySet, Exists, OuterRef +from django.db.models import QuerySet, Exists, OuterRef from django.template.loader import get_template from mailcow_integration.api.client import MailcowAPIClient @@ -28,7 +28,7 @@ def get_mailcow_manager() -> Optional["SquireMailcowManager"]: class AliasCategory(Enum): """Squire's Mailcow Aliases can exist in different forms. 1. Member aliases are used to email all Squire Members active in the current year. - 2. Global committee aliases are used to email all committees (Committtees and orders) + 2. Global committee aliases are used to email all committees (Committees and orders) registered in Squire. 3. Committee aliases are used to email a specific committee (or order). """ @@ -51,7 +51,7 @@ class SquireMailcowManager: it also allows manual overrides by Mailcow admins. """ - SQUIRE_MANAGE_INDICATOR = "[MANAGED BY SQUIRE]" + SQUIRE_MANAGE_INDICATOR = "[MANAGED BY SQUIRE]" if not settings.DEBUG else "[DEV][MANAGED BY SQUIRE]" INTERNAL_ALIAS_SETTING_NAME = "%s Internal Alias" % SQUIRE_MANAGE_INDICATOR ALIAS_COMMITTEE_PUBLIC_COMMENT = "%s Committee Alias" % SQUIRE_MANAGE_INDICATOR ALIAS_MEMBERS_PUBLIC_COMMENT = "%s Members Alias" % SQUIRE_MANAGE_INDICATOR diff --git a/mailcow_integration/tests/api/test_interfaces.py b/mailcow_integration/tests/api/test_interfaces.py index 5396c80d..ef0940d9 100644 --- a/mailcow_integration/tests/api/test_interfaces.py +++ b/mailcow_integration/tests/api/test_interfaces.py @@ -19,7 +19,7 @@ def get_alias_json(): "in_primary_domain": "", "id": 42, "domain": "example.com", - "public_comment": "[MANAGED BY SQUIRE] Members Alias", + "public_comment": "[DEV][MANAGED BY SQUIRE] Members Alias", "private_comment": None, "goto": "bar@example.com,baz@example.com", "address": "foo@example.com", @@ -83,7 +83,7 @@ def get_rspamd_json(): return deepcopy( { "id": 1, - "desc": "[MANAGED BY SQUIRE] Internal Alias", + "desc": "[DEV][MANAGED BY SQUIRE] Internal Alias", "content": "# MANAGED BY SQUIRE - DO NOT MODIFY;\r\nfoo faa rules;\r\nyet another rule;\r\n]", "active": 0, } diff --git a/mailcow_integration/tests/tests_squire_mailcow.py b/mailcow_integration/tests/tests_squire_mailcow.py index 619bdad5..7a3e795f 100644 --- a/mailcow_integration/tests/tests_squire_mailcow.py +++ b/mailcow_integration/tests/tests_squire_mailcow.py @@ -155,8 +155,8 @@ def test_get_active_committees(self): [ # NOTE: Wrap list in iterable to match function signature (it returns a generator, not a list) RspamdSettings(999, "Other rule", "RULE 1", True), - RspamdSettings(234, "[MANAGED BY SQUIRE] Other thing", "RULE 2", True), - RspamdSettings(567, "[MANAGED BY SQUIRE] Internal Alias", "RULE 3", True), + RspamdSettings(234, "[DEV][MANAGED BY SQUIRE] Other thing", "RULE 2", True), + RspamdSettings(567, "[DEV][MANAGED BY SQUIRE] Internal Alias", "RULE 3", True), RspamdSettings(890, "Another rule", "RULE 4", True), ] ), @@ -171,7 +171,7 @@ def test_get_rspamd_setting(self, mock_get: Mock): with patch( "mailcow_integration.squire_mailcow.SquireMailcowManager.INTERNAL_ALIAS_SETTING_NAME", - "[MANAGED BY SQUIRE] fake name", + "[DEV][MANAGED BY SQUIRE] fake name", ): # Use cache; old setting should still be found self.assertEqual( From 170f87d623d1bb78b13dc50e758350f6226dfc72 Mon Sep 17 00:00:00 2001 From: Eric <34304046+EricTRL@users.noreply.github.com> Date: Sat, 2 Sep 2023 13:33:01 +0200 Subject: [PATCH 3/3] Multiple Rspamd rules Modified Rspamd configuration because internal messages were being rejected. Now utilises two rules opposed to one. Modified template, logic, and caching behaviour to account for two rules opposed to one. --- mailcow_integration/admin_status/views.py | 9 +- mailcow_integration/squire_mailcow.py | 103 +++++++++++------- .../admin_status/status.html | 57 ++++++---- .../mailcow_integration/internal_mailbox.conf | 11 -- .../internal_mailbox_allow.conf | 13 +++ .../internal_mailbox_block.conf | 13 +++ .../snippets/rspamd_setting.html | 14 +++ .../tests/tests_squire_mailcow.py | 62 ++++++----- .../tests/tests_view_status.py | 3 +- 9 files changed, 180 insertions(+), 105 deletions(-) delete mode 100644 mailcow_integration/templates/mailcow_integration/internal_mailbox.conf create mode 100644 mailcow_integration/templates/mailcow_integration/internal_mailbox_allow.conf create mode 100644 mailcow_integration/templates/mailcow_integration/internal_mailbox_block.conf create mode 100644 mailcow_integration/templates/mailcow_integration/snippets/rspamd_setting.html diff --git a/mailcow_integration/admin_status/views.py b/mailcow_integration/admin_status/views.py index cef4cd44..71918219 100644 --- a/mailcow_integration/admin_status/views.py +++ b/mailcow_integration/admin_status/views.py @@ -369,7 +369,7 @@ def get_context_data(self, **kwargs): aliases = list(self.mailcow_manager.get_alias_all(use_cache=False)) mailboxes = list(self.mailcow_manager.get_mailbox_all(use_cache=False)) # Force cache update; we don't care about the result - self.mailcow_manager.get_internal_alias_rspamd_setting(use_cache=False) + self.mailcow_manager.get_internal_alias_rspamd_settings(use_cache=False) except MailcowAuthException as e: context["error"] = "No valid API key set." except MailcowAPIReadWriteAccessDenied as e: @@ -388,9 +388,10 @@ def get_context_data(self, **kwargs): context["unused_aliases"] = self._init_unused_squire_addresses_list( aliases, context["member_aliases"], context["committee_aliases"], context["global_committee_aliases"] ) - context["internal_alias_rspamd_setting"] = self.mailcow_manager.get_internal_alias_rspamd_setting( - use_cache=False - ) + ( + context["internal_alias_rspamd_setting_allow"], + context["internal_alias_rspamd_setting_block"], + ) = self.mailcow_manager.get_internal_alias_rspamd_settings() context["mailcow_host"] = self.mailcow_manager.mailcow_host return context diff --git a/mailcow_integration/squire_mailcow.py b/mailcow_integration/squire_mailcow.py index 70abc01b..af6c3b48 100644 --- a/mailcow_integration/squire_mailcow.py +++ b/mailcow_integration/squire_mailcow.py @@ -52,10 +52,12 @@ class SquireMailcowManager: """ SQUIRE_MANAGE_INDICATOR = "[MANAGED BY SQUIRE]" if not settings.DEBUG else "[DEV][MANAGED BY SQUIRE]" - INTERNAL_ALIAS_SETTING_NAME = "%s Internal Alias" % SQUIRE_MANAGE_INDICATOR - ALIAS_COMMITTEE_PUBLIC_COMMENT = "%s Committee Alias" % SQUIRE_MANAGE_INDICATOR - ALIAS_MEMBERS_PUBLIC_COMMENT = "%s Members Alias" % SQUIRE_MANAGE_INDICATOR - ALIAS_GLOBAL_COMMITTEE_PUBLIC_COMMENT = "%s Global Committee Alias" % SQUIRE_MANAGE_INDICATOR + INTERNAL_ALIAS_SETTING_NAME = SQUIRE_MANAGE_INDICATOR + " Internal Alias (%s)" + INTERNAL_ALIAS_SETTING_WHITELIST_NAME = "Authenticated" + INTERNAL_ALIAS_SETTING_BLACKLIST_NAME = "Reject" + ALIAS_COMMITTEE_PUBLIC_COMMENT = f"{SQUIRE_MANAGE_INDICATOR} Committee Alias" + ALIAS_MEMBERS_PUBLIC_COMMENT = f"{SQUIRE_MANAGE_INDICATOR} Members Alias" + ALIAS_GLOBAL_COMMITTEE_PUBLIC_COMMENT = f"{SQUIRE_MANAGE_INDICATOR} Global Committee Alias" def __init__(self, mailcow_host: str, mailcow_api_key: str): self._client = MailcowAPIClient(mailcow_host, mailcow_api_key) @@ -76,7 +78,8 @@ def __init__(self, mailcow_host: str, mailcow_api_key: str): ] + settings.COMMITTEE_CONFIGS["global_addresses"] # Caches - self._internal_rspamd_setting: Optional[RspamdSettings] = None + self._internal_rspamd_setting_whitelist: Optional[RspamdSettings] = None + self._internal_rspamd_setting_blacklist: Optional[RspamdSettings] = None self._alias_cache: Optional[List[MailcowAlias]] = None self._mailbox_cache: Optional[List[MailcowMailbox]] = None self._alias_map_cache: Optional[Dict[str, MailcowAlias]] = None @@ -107,28 +110,38 @@ def clean_emails_flat(self, queryset: QuerySet, email_field="email", **kwargs) - queryset = self.clean_emails(queryset, email_field, **kwargs) return list(queryset.values_list(email_field, flat=True)) - def get_internal_alias_rspamd_setting(self, use_cache=True) -> Optional[RspamdSettings]: - """Gets the Rspamd setting (if it exists) that disallows external domains + def get_internal_alias_rspamd_settings( + self, use_cache=True + ) -> Tuple[Optional[RspamdSettings], Optional[RspamdSettings]]: + """Gets the Rspamd settings (if it exists) that disallows external domains to send emails to a specific set of email addresses. Squire recognises which Rspamd setting to find based on the setting's name. See `self.INTERNAL_ALIAS_SETTING_NAME` """ - if self._internal_rspamd_setting is not None and use_cache: - return self._internal_rspamd_setting + if ( + self._internal_rspamd_setting_whitelist is not None + and self._internal_rspamd_setting_blacklist is not None + and use_cache + ): + return self._internal_rspamd_setting_whitelist, self._internal_rspamd_setting_blacklist + + self._internal_rspamd_setting_whitelist = None + self._internal_rspamd_setting_blacklist = None # Fetch all Rspamd settings settings = self._client.get_rspamd_setting_all() for setting in settings: # Setting description matches the one we normally set - if setting.desc == self.INTERNAL_ALIAS_SETTING_NAME: - self._internal_rspamd_setting = setting - return setting - return None + if setting.desc == self.INTERNAL_ALIAS_SETTING_NAME % self.INTERNAL_ALIAS_SETTING_WHITELIST_NAME: + self._internal_rspamd_setting_whitelist = setting + elif setting.desc == self.INTERNAL_ALIAS_SETTING_NAME % self.INTERNAL_ALIAS_SETTING_BLACKLIST_NAME: + self._internal_rspamd_setting_blacklist = setting + return (self._internal_rspamd_setting_whitelist, self._internal_rspamd_setting_blacklist) def is_address_internal(self, address: str) -> bool: """Whether an alias address is made internal by means of an Rspamd setting""" - setting = self.get_internal_alias_rspamd_setting() - if setting is None or not setting.active: + setting_w, setting_b = self.get_internal_alias_rspamd_settings() + if setting_w is None or not setting_w.active or setting_b is None or not setting_b.active: return False prefix = re.escape('rcpt = "/^(') @@ -136,39 +149,29 @@ def is_address_internal(self, address: str) -> bool: wildcard = '[^"\n]*' # Double escape since we're using regex to find a match ourselves address = re.escape(re.escape(address)) - mtch = re.search(f"{prefix}{wildcard}{address}{wildcard}{suffix}", setting.content) - return mtch is not None + mtch_w = re.search(f"{prefix}{wildcard}{address}{wildcard}{suffix}", setting_w.content) + mtch_b = re.search(f"{prefix}{wildcard}{address}{wildcard}{suffix}", setting_b.content) + return mtch_w is not None and mtch_b is not None - def update_internal_addresses(self) -> None: - """Makes a list of member aliases 'internal'. That is, these aliases can only - be emailed from within one of the domains set up in Mailcow. - See `templates/internal_mailbox.conf` for the Rspamd configuration used to - achieve this. - - Example: - `@example.com` is a domain set up in Mailcow. - Using this function to make `members@example.com` interal ensures that - `foo@spam.com` cannot send emails to `members@example.com` (those are - discarded), while `importantperson@example.com` can send emails to - `members@example.com`. Spoofed sender addresses are properly discarded - as well. - """ - # Escape addresses - addresses = self.INTERNAL_ALIAS_ADDRESSES - addresses = list(map(lambda addr: re.escape(addr), addresses)) - - # Fetch existing rspamd settings - setting = self.get_internal_alias_rspamd_setting(use_cache=False) + def update_internal_alias_setting(self, addresses: List[str], setting: RspamdSettings, is_whitelist_setting: bool): + """Updates the allow/block setting""" if setting is not None and setting.active and f'rcpt = "/^({"|".join(addresses)})$/"' in setting.content: # Setting already exists, is active, and is up-to-date; no need to do anything return # Setting emails are different than from what we expect, or the setting # does not yet exist - template = get_template("mailcow_integration/internal_mailbox.conf") + subtemplate_name = "allow" if is_whitelist_setting else "block" + subsetting_name = ( + self.INTERNAL_ALIAS_SETTING_WHITELIST_NAME + if is_whitelist_setting + else self.INTERNAL_ALIAS_SETTING_BLACKLIST_NAME + ) + + template = get_template("mailcow_integration/internal_mailbox_%s.conf" % subtemplate_name) setting_content = template.render({"addresses": addresses}) id = setting.id if setting is not None else None - setting = RspamdSettings(id, self.INTERNAL_ALIAS_SETTING_NAME, setting_content, True) + setting = RspamdSettings(id, self.INTERNAL_ALIAS_SETTING_NAME % subsetting_name, setting_content, True) if setting.id is None: # Setting does not yet exist @@ -177,6 +180,28 @@ def update_internal_addresses(self) -> None: # Setting exists but should be updated self._client.update_rspamd_setting(setting) + def update_internal_addresses(self) -> None: + """Makes specific member aliases 'internal'. That is, these aliases can only + be emailed from within one of the domains set up in Mailcow. + See `templates/internal_mailbox_.conf` for the Rspamd configuration used to + achieve this. + + Example: + `@example.com` is a domain set up in Mailcow + members@example.com is an internal member address according to Squire's mailcowconfig.json + Using this function ensures that `foo@spam.com` (note the domain) cannot send emails to + `members@example.com` (those are rejected), while `importantperson@example.com` (note the domain) + can send emails to `members@example.com`. + Spoofed sender addresses are properly discarded as well. + """ + # Escape addresses + addresses = self.INTERNAL_ALIAS_ADDRESSES + addresses = list(map(lambda addr: re.escape(addr), addresses)) + + setting_w, setting_b = self.get_internal_alias_rspamd_settings(use_cache=False) + self.update_internal_alias_setting(addresses, setting_w, True) + self.update_internal_alias_setting(addresses, setting_b, False) + def get_alias_all(self, use_cache=True) -> List[MailcowAlias]: """Gets all email aliases""" if use_cache and self._alias_cache is not None: diff --git a/mailcow_integration/templates/mailcow_integration/admin_status/status.html b/mailcow_integration/templates/mailcow_integration/admin_status/status.html index 857cbe60..d392f1ac 100644 --- a/mailcow_integration/templates/mailcow_integration/admin_status/status.html +++ b/mailcow_integration/templates/mailcow_integration/admin_status/status.html @@ -29,7 +29,7 @@

Mailcow Server Status

Member Aliases

- + {% if error %} {% else %} @@ -51,34 +51,47 @@

Member Aliases

- {% if not internal_alias_rspamd_setting %} -
diff --git a/mailcow_integration/templates/mailcow_integration/internal_mailbox.conf b/mailcow_integration/templates/mailcow_integration/internal_mailbox.conf deleted file mode 100644 index afbf5530..00000000 --- a/mailcow_integration/templates/mailcow_integration/internal_mailbox.conf +++ /dev/null @@ -1,11 +0,0 @@ -# MANAGED BY SQUIRE - DO NOT MODIFY -# Last updated on {% now "Y-m-d H:i:s.u" %} -priority = 10; -rcpt = "/^({{ addresses|join:'|' }})$/" -local = false; -apply "default" { - MAILCOW_BLACK = 9999.0; -} -symbols [ - "MAILCOW_BLACK" -] diff --git a/mailcow_integration/templates/mailcow_integration/internal_mailbox_allow.conf b/mailcow_integration/templates/mailcow_integration/internal_mailbox_allow.conf new file mode 100644 index 00000000..7e836a59 --- /dev/null +++ b/mailcow_integration/templates/mailcow_integration/internal_mailbox_allow.conf @@ -0,0 +1,13 @@ +# MANAGED BY SQUIRE - DO NOT MODIFY +# Last updated on {% now "Y-m-d H:i:s.u" %} +# This rule matches any internal address + # If this rule matches, NO FURTHER RULES ARE CHECKED +priority = 11; +rcpt = "/^({{ addresses|join:'|' }})$/"; +authenticated = yes; +apply "default" { + INTERNAL_MAILBOX_TO_ALIAS = -9999.0; +} +symbols [ + "INTERNAL_MAILBOX_TO_ALIAS" +] diff --git a/mailcow_integration/templates/mailcow_integration/internal_mailbox_block.conf b/mailcow_integration/templates/mailcow_integration/internal_mailbox_block.conf new file mode 100644 index 00000000..e0c55507 --- /dev/null +++ b/mailcow_integration/templates/mailcow_integration/internal_mailbox_block.conf @@ -0,0 +1,13 @@ +# MANAGED BY SQUIRE - DO NOT MODIFY +# Last updated on {% now "Y-m-d H:i:s.u" %} +# This rule blocks all emails sent to this address. +# Note how this rule has a lower priority than the allow-rule +priority = 10; +rcpt = "/^({{ addresses|join:'|' }})$/"; + +apply "default" { + EXTERNAL_MAILBOX_TO_ALIAS = 9999.0; +} +symbols [ + "EXTERNAL_MAILBOX_TO_ALIAS" +] diff --git a/mailcow_integration/templates/mailcow_integration/snippets/rspamd_setting.html b/mailcow_integration/templates/mailcow_integration/snippets/rspamd_setting.html new file mode 100644 index 00000000..5a98cc73 --- /dev/null +++ b/mailcow_integration/templates/mailcow_integration/snippets/rspamd_setting.html @@ -0,0 +1,14 @@ +

+{% if not rspamd_setting %} + +{% else %} + {% if not rspamd_setting.active %} + + {% endif %} +
{{rspamd_setting.desc}} – {{rspamd_setting.id}}
+ 
{{rspamd_setting.content }}
+{% endif %} \ No newline at end of file diff --git a/mailcow_integration/tests/tests_squire_mailcow.py b/mailcow_integration/tests/tests_squire_mailcow.py index 7a3e795f..cedb4e08 100644 --- a/mailcow_integration/tests/tests_squire_mailcow.py +++ b/mailcow_integration/tests/tests_squire_mailcow.py @@ -156,16 +156,18 @@ def test_get_active_committees(self): # NOTE: Wrap list in iterable to match function signature (it returns a generator, not a list) RspamdSettings(999, "Other rule", "RULE 1", True), RspamdSettings(234, "[DEV][MANAGED BY SQUIRE] Other thing", "RULE 2", True), - RspamdSettings(567, "[DEV][MANAGED BY SQUIRE] Internal Alias", "RULE 3", True), + RspamdSettings(567, "[DEV][MANAGED BY SQUIRE] Internal Alias (Authenticated)", "RULE 3", True), + RspamdSettings(165, "[DEV][MANAGED BY SQUIRE] Internal Alias (Reject)", "RULE 4", True), RspamdSettings(890, "Another rule", "RULE 4", True), ] ), ) def test_get_rspamd_setting(self, mock_get: Mock): """Tests whether the correct Rspamd setting can be found by Squire""" - setting = self.squire_mailcow_manager.get_internal_alias_rspamd_setting() - self.assertIsNotNone(setting) - self.assertEqual(setting.id, 567) + setting_w, setting_b = self.squire_mailcow_manager.get_internal_alias_rspamd_settings() + self.assertIsNotNone(setting_w) + self.assertEqual(setting_w.id, 567) + self.assertEqual(setting_b.id, 165) mock_get.assert_called_once() @@ -174,13 +176,15 @@ def test_get_rspamd_setting(self, mock_get: Mock): "[DEV][MANAGED BY SQUIRE] fake name", ): # Use cache; old setting should still be found - self.assertEqual( - self.squire_mailcow_manager.get_internal_alias_rspamd_setting(use_cache=True).id, setting.id - ) + cached_w, cached_b = self.squire_mailcow_manager.get_internal_alias_rspamd_settings(use_cache=True) + self.assertEqual(cached_w.id, setting_w.id) + self.assertEqual(cached_b.id, setting_b.id) # No cache # Setting should not be found if no descriptions match - self.assertIsNone(self.squire_mailcow_manager.get_internal_alias_rspamd_setting(use_cache=False)) + new_w, new_b = self.squire_mailcow_manager.get_internal_alias_rspamd_settings(use_cache=False) + self.assertIsNone(new_w) + self.assertIsNone(new_b) def test_is_address_internal(self): """Tests whether the internal-status of an address is correctly determined""" @@ -190,15 +194,16 @@ def test_is_address_internal(self): # Setting is not set with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=None, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(None, None), ) as setting_prop: self.assertFalse(self.squire_mailcow_manager.is_address_internal("foo@example.com")) setting_prop.assert_called_once() + # Setting is inactive with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=setting, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(setting, setting), ) as setting_prop: # Setting is inactive self.assertFalse(self.squire_mailcow_manager.is_address_internal("foo@example.com")) @@ -245,11 +250,11 @@ def test_update_internal_adresses(self, mock_update: Mock, mock_create: Mock): # Setting is active and rule addresses are up-to-date with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=setting, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(setting, setting), ) as setting_prop: - # Verify return value, and that the property is only called once. - self.assertIsNone(self.squire_mailcow_manager.update_internal_addresses()) + # Verify that the property is only called once. + self.squire_mailcow_manager.update_internal_addresses() setting_prop.assert_called_once() # Create/update methods should not be called mock_create.assert_not_called() @@ -258,13 +263,14 @@ def test_update_internal_adresses(self, mock_update: Mock, mock_create: Mock): # Setting is inactive setting.active = False with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=setting, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(setting, setting), ): self.squire_mailcow_manager.update_internal_addresses() - # Create/update methods should not be called + # Create method should not be called mock_create.assert_not_called() - mock_update.assert_called_once() + # Update called twice (once for each rule) + self.assertEqual(mock_update.call_count, 2) # Updated rule should be active self.assertTrue(mock_update.call_args.args[0].active) @@ -273,25 +279,25 @@ def test_update_internal_adresses(self, mock_update: Mock, mock_create: Mock): setting.active = True setting.content = "out of date" with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=setting, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(setting, setting), ): self.squire_mailcow_manager.update_internal_addresses() - # Create/update methods should not be called + # Create method should not be called mock_create.assert_not_called() - mock_update.assert_called_once() + self.assertEqual(mock_update.call_count, 2) # Updated rule should have correct addresses set self.assertIn('"/^(foo@example\\.com|bar@example\\.com)$/"', mock_update.call_args.args[0].content) # Setting does not exist mock_update.reset_mock() with patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", - return_value=None, + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(None, None), ): self.squire_mailcow_manager.update_internal_addresses() - # Create/update methods should not be called - mock_create.assert_called_once() + # Update method should not be called + self.assertEqual(mock_create.call_count, 2) mock_update.assert_not_called() ################ diff --git a/mailcow_integration/tests/tests_view_status.py b/mailcow_integration/tests/tests_view_status.py index e827848e..d2a76e56 100644 --- a/mailcow_integration/tests/tests_view_status.py +++ b/mailcow_integration/tests/tests_view_status.py @@ -596,7 +596,8 @@ class MailcowContextDataTests(MailcowStatusViewTests): @patch("mailcow_integration.squire_mailcow.SquireMailcowManager.get_alias_all", return_value=[1]) @patch("mailcow_integration.squire_mailcow.SquireMailcowManager.get_mailbox_all", return_value=[2]) @patch( - "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_setting", return_value=None + "mailcow_integration.squire_mailcow.SquireMailcowManager.get_internal_alias_rspamd_settings", + return_value=(None, None), ) def test_get_context_data_valid( self,