diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index 5f57dc93b..f50afec8f 100644 --- a/src/registrar/fixtures_users.py +++ b/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", diff --git a/src/registrar/models/allowed_email.py b/src/registrar/models/allowed_email.py index 7cf3df277..7910caf48 100644 --- a/src/registrar/models/allowed_email.py +++ b/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}" - allowed_emails = cls.objects.filter( - Q(email__iexact=base_email) | - Q(email__iexact=email) - ) + base_email_exists = cls.objects.filter( + Q(email__iexact=f"{base_local}@{domain}") + ).exists() - # The string must start with the local, and the plus must be a digit - # and occur immediately after the local. The domain should still exist in the email. + # Given an example email, such as "joe.smoe+1@igorville.com" + # The full regex statement will be: "^joe.smoe\\+\\d+@igorville.com$" pattern = f'^{re.escape(base_local)}\\+\\d+@{re.escape(domain)}$' - - # 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) + return base_email_exists and re.match(pattern, email) else: - # If no '+' exists in the email, just do an exact match - allowed_emails = cls.objects.filter(email__iexact=email) - return allowed_emails.exists() + # Edge case, the +1 record exists but the base does not, + # and the record we are checking is the base record. + 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) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 8c7225841..be5f6da9b 100644 --- a/src/registrar/tests/test_models.py +++ b/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.domain, _ = Domain.objects.get_or_create(name="igorville.gov") - self.user, _ = User.objects.get_or_create(email=self.email) + self.email_2 = "cake@igorville.gov" + self.plus_email = "mayor+1@igorville.gov" + self.invalid_plus_email = "1+mayor@igorville.gov" def tearDown(self): super().tearDown() - Domain.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() - + AllowedEmail.objects.all().delete() - # Test for a normal email defined in the whitelist - # Test for a normal email NOT defined in the whitelist + def test_email_in_whitelist(self): + """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 - # Test for a +1 email NOT defined in the whitelist + def test_email_not_in_whitelist(self): + """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 - # Test for a +1 email defined in the whitelist, but the normal is NOT defined - # Test for a +1 email NOT defined in the whitelist and NOT defined in the normal + def test_plus_email_in_whitelist(self): + """Test for a +1 email defined in the whitelist""" + 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 \ No newline at end of file + 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) \ No newline at end of file