Merge pull request #4254 from mozilla/MPP-3482-standardize-delete-domain-mask

MPP-3482 standardize delete domain mask

emails/models.py

@@ -22,6 +22,7 @@ from django.utils.translation.trans_real import (
 from rest_framework.authtoken.models import Token
 
 from api.exceptions import ErrorContextType, RelayAPIException
+from emails.apps import EmailsConfig
 from privaterelay.plans import get_premium_countries
 from privaterelay.utils import (
     AcceptLanguageError,
@@ -516,7 +517,8 @@ def has_bad_words(value):
     return False
 
 
-def is_blocklisted(value):
+def is_blocklisted(value: str) -> bool:
+    assert isinstance(emails_config, EmailsConfig)
     return any(blockedword == value for blockedword in emails_config.blocklist)
 
 
@@ -665,7 +667,9 @@ class RelayAddress(models.Model):
                 locked_profile = Profile.objects.select_for_update().get(user=self.user)
                 check_user_can_make_another_address(locked_profile)
                 while True:
-                    if valid_address(self.address, self.domain_value):
+                    address_is_allowed = not is_blocklisted(self.address)
+                    address_is_valid = valid_address(self.address, self.domain_value)
+                    if address_is_valid and address_is_allowed:
                         break
                     self.address = address_default()
                 locked_profile.update_abuse_metric(address_created=True)
@@ -703,17 +707,15 @@ def valid_address_pattern(address):
     return valid_address_pattern.match(address) is not None
 
 
-def valid_address(address: str, domain: str) -> bool:
+def valid_address(address: str, domain: str, subdomain: str | None = None) -> bool:
     address_pattern_valid = valid_address_pattern(address)
     address_contains_badword = has_bad_words(address)
-    address_is_blocklisted = is_blocklisted(address)
     address_already_deleted = DeletedAddress.objects.filter(
-        address_hash=address_hash(address, domain=domain)
+        address_hash=address_hash(address, domain=domain, subdomain=subdomain)
     ).count()
     if (
         address_already_deleted > 0
         or address_contains_badword
-        or address_is_blocklisted
         or not address_pattern_valid
     ):
         return False
@@ -772,9 +774,10 @@ class DomainAddress(models.Model):
         user_profile = self.user.profile
         if self._state.adding:
             check_user_can_make_domain_address(user_profile)
-            pattern_valid = valid_address_pattern(self.address)
-            address_contains_badword = has_bad_words(self.address)
-            if not pattern_valid or address_contains_badword:
+            domain_address_valid = valid_address(
+                self.address, self.domain_value, user_profile.subdomain
+            )
+            if not domain_address_valid:
                 raise DomainAddrUnavailableException(unavailable_address=self.address)
             user_profile.update_abuse_metric(address_created=True)
         if not user_profile.has_premium:
@@ -788,7 +791,9 @@ class DomainAddress(models.Model):
         return Profile.objects.get(user=self.user)
 
     @staticmethod
-    def make_domain_address(user_profile, address=None, made_via_email=False):
+    def make_domain_address(
+        user_profile: Profile, address: str | None = None, made_via_email: bool = False
+    ) -> "DomainAddress":
         check_user_can_make_domain_address(user_profile)
 
         address_contains_badword = False
@@ -799,6 +804,7 @@ class DomainAddress(models.Model):
             # DomainAlias will be a feature
             address = address_default()
             # Only check for bad words if randomly generated
+        assert isinstance(address, str)
 
         domain_address = DomainAddress.objects.create(
             user=user_profile.user,

emails/tests/models_tests.py

@@ -21,6 +21,8 @@ from ..models import (
     CannotMakeSubdomainException,
     DeletedAddress,
     DomainAddress,
+    DomainAddrUnavailableException,
+    emails_config,
     get_domains_from_settings,
     get_domain_numerical,
     has_bad_words,
@@ -120,6 +122,10 @@ class MiscEmailModelsTest(TestCase):
     def test_is_blocklisted_without_blocked_words(self):
         assert not is_blocklisted("non-blocked-word")
 
+    @patch.object(emails_config, "blocklist", ["blocked-word"])
+    def test_is_blocklisted_with_mocked_blocked_words(self) -> None:
+        assert is_blocklisted("blocked-word")
+
     @override_settings(RELAY_FIREFOX_DOMAIN="firefox.com")
     def test_address_hash_without_subdomain_domain_firefox(self):
         address = "aaaaaaaaa"
@@ -260,6 +266,28 @@ class RelayAddressTest(TestCase):
         relay_address.delete()
         assert not valid_address(relay_address.address, relay_address.domain_value)
 
+    def test_dupe_domain_address_of_deleted_invalid(self):
+        user = make_premium_test_user()
+        user_profile = user.profile
+        user_profile.subdomain = "test"
+        user_profile.save()
+        address = "same-address"
+        domain_address = DomainAddress.make_domain_address(
+            user_profile, address=address
+        )
+        domain_address.delete()
+        assert not valid_address(
+            address, domain_address.domain_value, user_profile.subdomain
+        )
+
+    @patch.object(emails_config, "blocklist", ["blocked-word"])
+    def test_address_contains_blocklist_invalid(self) -> None:
+        blocked_word = "blocked-word"
+        relay_address = RelayAddress.objects.create(
+            user=baker.make(User), address=blocked_word
+        )
+        assert not relay_address.address == blocked_word
+
     def test_free_user_cant_set_block_list_emails(self):
         relay_address = RelayAddress.objects.create(user=self.user)
         relay_address.block_list_emails = True
@@ -1012,16 +1040,20 @@ class DomainAddressTest(TestCase):
             DomainAddress.make_domain_address(user_profile, "test-nosubdomain")
         assert exc_info.value.get_codes() == "need_subdomain"
 
-    @patch.multiple("string", ascii_lowercase="a", digits="")
-    def test_make_domain_address_makes_dupe_of_deleted(self):
-        test_hash = address_hash("aaaaaaaaa", self.subdomain)
-        DeletedAddress.objects.create(address_hash=test_hash)
-        domain_address = DomainAddress.make_domain_address(self.user_profile)
+    def test_make_domain_address_cannot_make_dupe_of_deleted(self):
+        address = "same-address"
+        domain_address = DomainAddress.make_domain_address(
+            self.user_profile, address=address
+        )
         domain_address_hash = address_hash(
             domain_address.address,
             domain_address.user_profile.subdomain,
             domain_address.domain_value,
         )
+        domain_address.delete()
+        with pytest.raises(DomainAddrUnavailableException) as exc_info:
+            DomainAddress.make_domain_address(self.user_profile, address=address)
+        assert exc_info.value.get_codes() == "address_unavailable"
         assert (
             DeletedAddress.objects.filter(address_hash=domain_address_hash).count() == 1
         )