Model unit test

src/registrar/fixtures_users.py

@@ -48,6 +48,7 @@ class UserFixture:
             "username": "eb2214cd-fc0c-48c0-9dbd-bc4cd6820c74",
             "first_name": "Alysia",
             "last_name": "Broddrick",
+            "email": "abroddrick@truss.works",
         },
         {
             "username": "8f8e7293-17f7-4716-889b-1990241cbd39",
@@ -64,6 +65,7 @@ class UserFixture:
             "username": "83c2b6dd-20a2-4cac-bb40-e22a72d2955c",
             "first_name": "Cameron",
             "last_name": "Dixon",
+            "email": "cameron.dixon@cisa.dhs.gov",
         },
         {
             "username": "0353607a-cbba-47d2-98d7-e83dcd5b90ea",

src/registrar/models/allowed_email.py

@@ -20,33 +20,37 @@ class AllowedEmail(TimeStampedModel):
     @classmethod
     def is_allowed_email(cls, email):
         """Given an email, check if this email exists within our AllowEmail whitelist"""
-        print(f"the email is: {email}")
+
         if not email:
             return False
 
         # Split the email into a local part and a domain part
-        local, domain = email.split('@')
+        local, domain = email.split("@")
+
+        # If the email exists within the whitelist, then do nothing else.
+        email_exists = cls.objects.filter(email__iexact=email).exists()
+        if email_exists:
+            return True
 
         # Check if there's a '+' in the local part
         if "+" in local:
             base_local = local.split("+")[0]
-            base_email = f"{base_local}@{domain}"
+            base_email_exists = cls.objects.filter(
-            allowed_emails = cls.objects.filter(
+                Q(email__iexact=f"{base_local}@{domain}")
-                Q(email__iexact=base_email) |
+            ).exists()
-                Q(email__iexact=email)
-            )
 
-            # The string must start with the local, and the plus must be a digit
+            # Given an example email, such as "joe.smoe+1@igorville.com"
-            # and occur immediately after the local. The domain should still exist in the email.
+            # The full regex statement will be: "^joe.smoe\\+\\d+@igorville.com$"
             pattern = f'^{re.escape(base_local)}\\+\\d+@{re.escape(domain)}$'
-
+            return base_email_exists and re.match(pattern, email)
-            # If the base email exists AND the email matches our expected regex,
-            # then we can let the email through.
-            return allowed_emails.exists() and re.match(pattern, email)
         else:
-            # If no '+' exists in the email, just do an exact match
+            # Edge case, the +1 record exists but the base does not,
-            allowed_emails = cls.objects.filter(email__iexact=email)
+            # and the record we are checking is the base record.
-            return allowed_emails.exists()
+            pattern = f'^{re.escape(local)}\\+\\d+@{re.escape(domain)}$'
+            plus_email_exists = cls.objects.filter(
+                Q(email__iregex=pattern)
+            ).exists()
+            return plus_email_exists
 
     def __str__(self):
         return str(self.email)

src/registrar/tests/test_models.py

@@ -17,6 +17,7 @@ from registrar.models import (
     DomainInvitation,
     UserDomainRole,
     FederalAgency,
+    AllowedEmail,
 )
 
 import boto3_mocking
@@ -2328,33 +2329,103 @@ class TestAllowedEmail(TestCase):
     @less_console_noise_decorator
     def setUp(self):
         self.email = "mayor@igorville.gov"
-        self.domain_name = "igorvilleInTransition.gov"
+        self.email_2 = "cake@igorville.gov"
-        self.domain, _ = Domain.objects.get_or_create(name="igorville.gov")
+        self.plus_email = "mayor+1@igorville.gov"
-        self.user, _ = User.objects.get_or_create(email=self.email)
+        self.invalid_plus_email = "1+mayor@igorville.gov"
 
     def tearDown(self):
         super().tearDown()
-        Domain.objects.all().delete()
+        AllowedEmail.objects.all().delete()
-        DomainInvitation.objects.all().delete()
-        DomainInformation.objects.all().delete()
-        DomainRequest.objects.all().delete()
-        DraftDomain.objects.all().delete()
-        TransitionDomain.objects.all().delete()
-        Portfolio.objects.all().delete()
-        User.objects.all().delete()
-        UserDomainRole.objects.all().delete()
-    
 
-    # Test for a normal email defined in the whitelist
+    def test_email_in_whitelist(self):
-    # Test for a normal email NOT defined in the whitelist
+        """Test for a normal email defined in the whitelist"""
+        AllowedEmail.objects.create(email=self.email)
+        is_allowed = AllowedEmail.is_allowed_email(self.email)
+        self.assertTrue(is_allowed)
 
-    # Test for a +1 email defined in the whitelist
+    def test_email_not_in_whitelist(self):
-    # Test for a +1 email NOT defined in the whitelist
+        """Test for a normal email NOT defined in the whitelist"""
+        # Check a email not in the list
+        is_allowed = AllowedEmail.is_allowed_email(self.email_2)
+        self.assertFalse(AllowedEmail.objects.filter(email=self.email_2).exists())
+        self.assertFalse(is_allowed)
 
-    # Test for a +1 email NOT defined in the whitelist, but the normal is defined
+    def test_plus_email_in_whitelist(self):
-    # Test for a +1 email defined in the whitelist, but the normal is NOT defined
+        """Test for a +1 email defined in the whitelist"""
-    # Test for a +1 email NOT defined in the whitelist and NOT defined in the normal
+        AllowedEmail.objects.create(email=self.plus_email)
+        plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email)
+        self.assertTrue(plus_email_allowed)
 
-    # Test for an invalid email that contains a '+'
+    def test_plus_email_not_in_whitelist(self):
+        """Test for a +1 email not defined in the whitelist"""
+        # This email should not be allowed. 
+        # Checks that we do more than just a regex check on the record.
+        plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email)
+        self.assertFalse(plus_email_allowed)
 
-    # TODO: We need a small test for domain request admin
+    def test_plus_email_not_in_whitelist_but_base_email_is(self):
+        """
+        Test for a +1 email NOT defined in the whitelist, but the normal one is defined.
+        Example:
+        normal (in whitelist) - joe@igorville.com
+        +1 email (not in whitelist) - joe+1@igorville.com
+        """
+        AllowedEmail.objects.create(email=self.email)
+        base_email_allowed = AllowedEmail.is_allowed_email(self.email)
+        self.assertTrue(base_email_allowed)
+
+        # The plus email should also be allowed
+        plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email)
+        self.assertTrue(plus_email_allowed)
+
+        # This email shouldn't exist in the DB
+        self.assertFalse(AllowedEmail.objects.filter(email=self.plus_email).exists())
+
+    def test_plus_email_in_whitelist_but_base_email_is_not(self):
+        """
+        Test for a +1 email defined in the whitelist, but the normal is NOT defined.
+        Example:
+        normal (not in whitelist) - joe@igorville.com
+        +1 email (in whitelist) - joe+1@igorville.com
+        """
+        AllowedEmail.objects.create(email=self.plus_email)
+        plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email)
+        self.assertTrue(plus_email_allowed)
+
+        # The base email should also be allowed
+        base_email_allowed = AllowedEmail.is_allowed_email(self.email)
+        self.assertTrue(base_email_allowed)
+
+        # This email shouldn't exist in the DB
+        self.assertFalse(AllowedEmail.objects.filter(email=self.email).exists())
+
+    def test_invalid_regex_for_plus_email(self):
+        """
+        Test for an invalid email that contains a '+'.
+        This base email should still pass, but the regex rule should not.
+
+        Our regex should only pass for emails that end with a '+'
+        Example:
+        Invalid email - 1+joe@igorville.com
+        Valid email: - joe+1@igorville.com
+        """
+        AllowedEmail.objects.create(email=self.invalid_plus_email)
+        invalid_plus_email = AllowedEmail.is_allowed_email(self.invalid_plus_email)
+        # We still expect that this will pass, it exists in the db
+        self.assertTrue(invalid_plus_email)
+
+        # The base email SHOULD NOT pass, as it doesn't match our regex
+        base_email = AllowedEmail.is_allowed_email(self.email)
+        self.assertFalse(base_email)
+
+        # For good measure, also check the other plus email
+        regular_plus_email = AllowedEmail.is_allowed_email(self.plus_email)
+        self.assertFalse(regular_plus_email)
+
+    # TODO: We need a small test for domain request admin
+    # We also need a basic test in test_emails based off the mocked is_allowed_email value.
+    # This will be simpler 
+    # def test_email_in_whitelist_in_prod(self):
+    #     """Tests that the whitelist does nothing when we are in production"""
+    #     allowed_email = AllowedEmail.objects.create(email=self.email)
+    #     self.assertEqual(allowed_email.is_allowed_email(), True)