Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create and receive mozmail relay alias #993

Merged
merged 38 commits into from
Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
1af261e
Cast boolean to ADMIN_ENABLED var
say-yawn Jul 27, 2021
9a98ad0
Uniform method to get Relay email domain
say-yawn Jul 29, 2021
94b06cf
Add domain field in Relay Address model
say-yawn Jul 29, 2021
0ab81b7
Get the RelayAddress with appropriate domain
say-yawn Jul 29, 2021
c8a0310
Make RelayAddress with different domains
say-yawn Jul 29, 2021
3a45102
Delete RelayAddress and hash the domain
say-yawn Jul 29, 2021
2022c5b
Logger info
say-yawn Aug 5, 2021
e6cbcce
Use plus rather than append
say-yawn Aug 5, 2021
53c061e
Use mozmail.com based on env var
say-yawn Aug 5, 2021
2dc05d3
Pass domain in retries
say-yawn Aug 5, 2021
7742448
Use index to grab first item
say-yawn Aug 5, 2021
e89525a
Domain not in profile
say-yawn Aug 5, 2021
5480991
Get domain from relay address object
say-yawn Aug 5, 2021
2e08f7a
Remove loggers
say-yawn Aug 5, 2021
10d63fc
Pass domain portion for address hash
say-yawn Aug 12, 2021
7d1f550
Fix views tests on emails app
say-yawn Aug 12, 2021
55b43b2
Fix get email domain from settings
say-yawn Aug 12, 2021
b93e80f
Add get email domain test
say-yawn Aug 12, 2021
6080e2d
Get domain from relay address when copying
say-yawn Aug 12, 2021
42f78f7
Revert changes to deleted DomainAddress
say-yawn Aug 12, 2021
c7ac965
Test address hash with non-default domain
say-yawn Aug 12, 2021
1c76a38
Test domain field on RelayAddress
say-yawn Aug 12, 2021
9aa604f
Fix flake8 errors on emails
say-yawn Aug 12, 2021
92dce9c
Make static migration
say-yawn Aug 18, 2021
d525c8f
Move domains to utils
say-yawn Aug 18, 2021
75ba71f
Add domains environment variables to sample
say-yawn Aug 18, 2021
4b9ef53
Fix emails/views with new domains logic
say-yawn Aug 18, 2021
3c6c411
Remove unneeded code snippet
say-yawn Aug 18, 2021
36588a9
Test get_domains_from_settings
say-yawn Aug 18, 2021
000da4f
Return domain value on profile
say-yawn Aug 18, 2021
74dad3f
Fix broken tests
say-yawn Aug 18, 2021
ee9a67c
Use Firefox address on address_hash logic
say-yawn Aug 18, 2021
8cd6652
Enable mozmail for Mozillians
say-yawn Aug 18, 2021
b74c38b
Pass domain in the parameter
say-yawn Aug 20, 2021
bfc8c84
Display correct domain for alias
say-yawn Aug 24, 2021
279cd39
Use mozmail domain when TEST_MOZMAIL is True
say-yawn Aug 24, 2021
7d05ba2
Return domain of RelayAddress
say-yawn Aug 24, 2021
8673e35
Return full_address for RelayAddress
say-yawn Aug 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env-dist
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ SECRET_KEY=unsafe-secret-key-for-dev-envs
ADMIN_ENABLED=
DEBUG=True
DJANGO_INTERNAL_IPS=127.0.0.1, localhost
TEST_MOZMAIL=True
RELAY_FIREFOX_DOMAIN="relay.firefox.com"
MOZMAIL_DOMAIN="mozmail.com"
SENTRY_DSN=""
SERVE_ADDON="private_relay.zip"
AWS_REGION="us-east-1"
Expand Down
8 changes: 2 additions & 6 deletions emails/context_processors.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
from email.utils import parseaddr

from django.conf import settings
from emails.utils import get_email_domain_from_settings


def relay_from_domain(request):
display_name, address = parseaddr(settings.RELAY_FROM_ADDRESS)
relay_from_domain = address.split('@')[1]
return {'RELAY_DOMAIN': relay_from_domain}
return {'RELAY_DOMAIN': get_email_domain_from_settings()}
18 changes: 18 additions & 0 deletions emails/migrations/0018_relayaddress_domain.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 2.2.24 on 2021-08-18 15:08

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('emails', '0017_remove_unique_from_address'),
]

operations = [
migrations.AddField(
model_name='relayaddress',
name='domain',
field=models.PositiveSmallIntegerField(choices=[(1, 'RELAY_FIREFOX_DOMAIN'), (2, 'MOZMAIL_DOMAIN')], default=1),
),
]
48 changes: 41 additions & 7 deletions emails/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.contrib.auth.models import User
from django.db import models

from emails.utils import get_domains_from_settings

emails_config = apps.get_app_config('emails')

Expand All @@ -19,6 +20,12 @@
NOT_PREMIUM_USER_ERR_MSG = 'You must be a premium subscriber to {}.'
TRY_DIFFERENT_VALUE_ERR_MSG = '{} could not be created, try using a different value.'

DOMAINS = get_domains_from_settings()
DOMAIN_CHOICES = [(1, 'RELAY_FIREFOX_DOMAIN'), (2, 'MOZMAIL_DOMAIN')]
DEFAULT_DOMAIN = settings.RELAY_FIREFOX_DOMAIN
if settings.TEST_MOZMAIL:
DEFAULT_DOMAIN = settings.MOZMAIL_DOMAIN


class Profile(models.Model):
user = models.ForeignKey(User, on_delete=models.CASCADE)
Expand Down Expand Up @@ -145,14 +152,18 @@ def add_subdomain(self, subdomain):
return subdomain


def address_hash(address, subdomain=None):
def address_hash(address, subdomain=None, domain=DEFAULT_DOMAIN):
if subdomain:
return sha256(
f'{address}@{subdomain}'.encode('utf-8')
).hexdigest()
return sha256(
if domain == settings.RELAY_FIREFOX_DOMAIN:
return sha256(
f'{address}'.encode('utf-8')
).hexdigest()
return sha256(
f'{address}@{domain}'.encode('utf-8')
).hexdigest()


def address_default():
Expand All @@ -166,6 +177,18 @@ def has_bad_words(value):
)


def get_domain_numerical(domain_address):
# get domain name from the address
domains_keys = list(DOMAINS.keys())
domains_values = list(DOMAINS.values())
domain_name = domains_keys[domains_values.index(domain_address)]
# get domain numerical value from domain name
choices = dict(DOMAIN_CHOICES)
choices_keys = list(choices.keys())
choices_values = list(choices.values())
return choices_keys[choices_values.index(domain_name)]


class CannotMakeSubdomainException(Exception):
"""Exception raised by Profile due to error on subdomain creation.

Expand Down Expand Up @@ -193,6 +216,7 @@ class RelayAddress(models.Model):
address = models.CharField(
max_length=64, default=address_default, unique=True
)
domain = models.PositiveSmallIntegerField(choices=DOMAIN_CHOICES, default=1)
enabled = models.BooleanField(default=True)
description = models.CharField(max_length=64, blank=True)
created_at = models.DateTimeField(auto_now_add=True, db_index=True)
Expand All @@ -208,7 +232,7 @@ def __str__(self):
def delete(self, *args, **kwargs):
# TODO: create hard bounce receipt rule in AWS for the address
deleted_address = DeletedAddress.objects.create(
address_hash=address_hash(self.address),
address_hash=address_hash(self.address, domain=self.domain_value),
num_forwarded=self.num_forwarded,
num_blocked=self.num_blocked,
num_spam=self.num_spam,
Expand All @@ -220,7 +244,7 @@ def delete(self, *args, **kwargs):
profile.save()
return super(RelayAddress, self).delete(*args, **kwargs)

def make_relay_address(user_profile, num_tries=0):
def make_relay_address(user_profile, num_tries=0, domain=DEFAULT_DOMAIN):
if (
user_profile.at_max_free_aliases
and not user_profile.has_unlimited
Expand All @@ -231,17 +255,27 @@ def make_relay_address(user_profile, num_tries=0):
)
if num_tries >= 5:
raise CannotMakeAddressException
relay_address = RelayAddress.objects.create(user=user_profile.user)
# only use the numerical value of the domain when creating the alias
domain_numerical = get_domain_numerical(domain)
relay_address = RelayAddress.objects.create(user=user_profile.user, domain=domain_numerical)
address_contains_badword = has_bad_words(relay_address.address)
address_already_deleted = DeletedAddress.objects.filter(
address_hash=address_hash(relay_address.address)
address_hash=address_hash(relay_address.address, domain=domain)
).count()
if address_already_deleted > 0 or address_contains_badword:
relay_address.delete()
num_tries += 1
return RelayAddress.make_relay_address(user_profile, num_tries)
return RelayAddress.make_relay_address(user_profile, num_tries, domain)
return relay_address

@property
def domain_value(self):
return DOMAINS.get(self.get_domain_display())

@property
def full_address(self):
return '%s@%s' % (self.address, self.domain_value)


class DeletedAddress(models.Model):
address_hash = models.CharField(max_length=64, db_index=True)
Expand Down
58 changes: 53 additions & 5 deletions emails/tests/models_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@

from django.conf import settings
from django.contrib.auth.models import User
from django.test import TestCase
from django.test import (
override_settings,
TestCase,
)

from allauth.socialaccount.models import SocialAccount

Expand All @@ -18,13 +21,16 @@
CannotMakeSubdomainException,
DeletedAddress,
DomainAddress,
get_domain_numerical,
has_bad_words,
NOT_PREMIUM_USER_ERR_MSG,
Profile,
RelayAddress,
TRY_DIFFERENT_VALUE_ERR_MSG,
)

TEST_DOMAINS = {'RELAY_FIREFOX_DOMAIN': 'default.com', 'MOZMAIL_DOMAIN': 'test.com'}


class MiscEmailModelsTest(TestCase):
def test_has_bad_words_with_bad_words(self):
Expand All @@ -33,17 +39,36 @@ def test_has_bad_words_with_bad_words(self):
def test_has_bad_words_without_bad_words(self):
assert not has_bad_words('happy')

def test_address_hash_without_subdomain(self):
@override_settings(TEST_MOZMAIL=False, RELAY_FIREFOX_DOMAIN='firefox.com')
def test_address_hash_without_subdomain_domain_firefox(self):
address = 'aaaaaaaaa'
expected_hash = sha256(f'{address}'.encode('utf-8')).hexdigest()
assert address_hash(address) == expected_hash
assert address_hash(address, domain='firefox.com') == expected_hash

@override_settings(TEST_MOZMAIL=False, RELAY_FIREFOX_DOMAIN='firefox.com')
def test_address_hash_without_subdomain_domain_not_firefoxz(self):
non_default = 'test.com'
address = 'aaaaaaaaa'
expected_hash = sha256(f'{address}@{non_default}'.encode('utf-8')).hexdigest()
assert address_hash(address, domain=non_default) == expected_hash

def test_address_hash_with_subdomain(self):
address = 'aaaaaaaaa'
subdomain = 'test'
expected_hash = sha256(f'{address}@{subdomain}'.encode('utf-8')).hexdigest()
assert address_hash(address, subdomain) == expected_hash

def test_address_hash_with_additional_domain(self):
address = 'aaaaaaaaa'
test_domain = 'test.com'
expected_hash = sha256(f'{address}@{test_domain}'.encode('utf-8')).hexdigest()
assert address_hash(address, domain=test_domain) == expected_hash

@patch('emails.models.DOMAINS', TEST_DOMAINS)
def test_get_domain_numerical(self):
assert get_domain_numerical('default.com') == 1
assert get_domain_numerical('test.com') == 2


class RelayAddressTest(TestCase):
def setUp(self):
Expand Down Expand Up @@ -99,6 +124,16 @@ def test_make_relay_address_non_premium_user_cannot_pass_limit(self):
return
self.fail("Should have raised CannotMakeSubdomainException")

@patch('emails.models.DOMAINS', TEST_DOMAINS)
def test_make_relay_address_with_specified_domain(self):
relay_address = RelayAddress.make_relay_address(self.user_profile, domain='test.com')
assert relay_address.domain == 2
assert relay_address.get_domain_display() == 'MOZMAIL_DOMAIN'
assert relay_address.domain_value == 'test.com'

@override_settings(TEST_MOZMAIL=False, RELAY_FIREFOX_DOMAIN=TEST_DOMAINS['RELAY_FIREFOX_DOMAIN'])
@patch('emails.models.DOMAINS', TEST_DOMAINS)
@patch('emails.models.DEFAULT_DOMAIN', TEST_DOMAINS['RELAY_FIREFOX_DOMAIN'])
def test_delete_adds_deleted_address_object(self):
relay_address = baker.make(RelayAddress)
address_hash = sha256(
Expand All @@ -110,13 +145,27 @@ def test_delete_adds_deleted_address_object(self):
).count()
assert deleted_count == 1

@patch('emails.models.DOMAINS', TEST_DOMAINS)
def test_delete_mozmail_deleted_address_object(self):
relay_address = baker.make(RelayAddress, domain=2)
address_hash = sha256(
f'{relay_address.address}@{relay_address.domain_value}'.encode('utf-8')
).hexdigest()
relay_address.delete()
deleted_count = DeletedAddress.objects.filter(
address_hash=address_hash
).count()
assert deleted_count == 1

# trigger a collision by making address_default always return 'aaaaaaaaa'
@override_settings(RELAY_FIREFOX_DOMAIN='default.com')
@patch.multiple('string', ascii_lowercase='a', digits='')
@patch('emails.models.DOMAINS', TEST_DOMAINS)
def test_make_relay_address_doesnt_make_dupe_of_deleted(self):
test_hash = sha256('aaaaaaaaa'.encode('utf-8')).hexdigest()
DeletedAddress.objects.create(address_hash=test_hash)
try:
RelayAddress.make_relay_address(self.user_profile)
RelayAddress.make_relay_address(self.user_profile, domain='default.com')
except CannotMakeAddressException:
return
self.fail("Should have raise CannotMakeAddressException")
Expand Down Expand Up @@ -399,7 +448,6 @@ def test_display_name_does_not_exist(self):
)
profile = Profile.objects.get(user=social_account.user)
assert profile.display_name == None



class DomainAddressTest(TestCase):
Expand Down
17 changes: 13 additions & 4 deletions emails/tests/utils_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from emails.utils import (
generate_relay_From,
get_domains_from_settings,
get_email_domain_from_settings,
)

Expand Down Expand Up @@ -66,12 +67,20 @@ def test_generate_relay_From_with_linebreak_chars(self):
)
assert formatted_from_address == expected_formatted_from

@override_settings(ON_HEROKU=True, SITE_ORIGIN='https://test.com')
def test_get_email_domain_from_settings_on_heroku(self):
@override_settings(ON_HEROKU=True, SITE_ORIGIN='https://test.com', TEST_MOZMAIL=False)
def test_get_email_domain_from_settings_on_heroku_test_mozmail_false(self):
email_domain = get_email_domain_from_settings()
assert 'mail.test.com' == email_domain

@override_settings(ON_HEROKU=False, SITE_ORIGIN='https://test.com')
def test_get_email_domain_from_settings_not_on_heroku(self):
@override_settings(ON_HEROKU=False, SITE_ORIGIN='https://test.com', TEST_MOZMAIL=False)
def test_get_email_domain_from_settings_not_on_heroku_test_mozmail_false(self):
email_domain = get_email_domain_from_settings()
assert 'test.com' == email_domain

@override_settings(RELAY_FIREFOX_DOMAIN='firefox.com', MOZMAIL_DOMAIN='mozmail.com')
def test_get_domains_from_settings(self):
domains = get_domains_from_settings()
assert domains == {
'RELAY_FIREFOX_DOMAIN': 'firefox.com',
'MOZMAIL_DOMAIN': 'mozmail.com'
}
32 changes: 24 additions & 8 deletions emails/tests/views_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
)
from emails.views import _get_address

TEST_DOMAINS = {'RELAY_FIREFOX_DOMAIN': 'default.com', 'MOZMAIL_DOMAIN': 'test.com'}


def _get_bounce_payload(bounce_type):
f_path = (
Expand Down Expand Up @@ -89,9 +91,14 @@ def test_get_address_with_domain_address(self, _get_domain_address_mocked):
)
assert actual == expected

def test_get_address_with_relay_address(self):
@patch('emails.models.DOMAINS', TEST_DOMAINS)
@patch('emails.views.get_domains_from_settings')
def test_get_address_with_relay_address(self, domains_mocked):
domains_mocked.return_value = TEST_DOMAINS
local_portion = 'foo'
relay_address = baker.make(RelayAddress, address=local_portion)
relay_address = baker.make(
RelayAddress, address=local_portion, domain=2
)

actual = _get_address(
to_address=f'{self.local_portion}@{self.service_domain}',
Expand All @@ -100,24 +107,30 @@ def test_get_address_with_relay_address(self):
)
assert actual == relay_address

@patch('emails.models.DOMAINS', TEST_DOMAINS)
@patch('emails.views.incr_if_enabled')
def test_get_address_with_deleted_relay_address(self, incr_mocked):
hashed_address = address_hash(self.local_portion)
@patch('emails.views.get_domains_from_settings')
def test_get_address_with_deleted_relay_address(self, domains_mocked, incr_mocked):
domains_mocked.return_value = TEST_DOMAINS
hashed_address = address_hash(self.local_portion, domain=self.service_domain)
baker.make(DeletedAddress, address_hash=hashed_address)

try:
_get_address(
to_address=f'{self.local_portion}@{self.service_domain}',
local_portion=self.local_portion,
domain_portion=f'{self.service_domain}'
domain_portion=self.service_domain
)
except Exception as e:
assert e.args[0] == 'Address does not exist'
incr_mocked.assert_called_once_with('email_for_deleted_address', 1)

@patch('emails.models.DOMAINS', TEST_DOMAINS)
@patch('emails.views.get_domains_from_settings')
@patch('emails.views.incr_if_enabled')
@patch('emails.views.logger')
def test_get_address_with_relay_address_does_not_exist(self, logging_mocked, incr_mocked):
def test_get_address_with_relay_address_does_not_exist(self, logging_mocked, incr_mocked, domains_mocked):
domains_mocked.return_value = TEST_DOMAINS
try:
_get_address(
to_address=f'{self.local_portion}@{self.service_domain}',
Expand All @@ -132,9 +145,12 @@ def test_get_address_with_relay_address_does_not_exist(self, logging_mocked, inc
)
incr_mocked.assert_called_once_with('email_for_unknown_address', 1)

@patch('emails.models.DOMAINS', TEST_DOMAINS)
@patch('emails.views.incr_if_enabled')
def test_get_address_with_deleted_relay_address_multiple(self, incr_mocked):
hashed_address = address_hash(self.local_portion)
@patch('emails.views.get_domains_from_settings')
def test_get_address_with_deleted_relay_address_multiple(self, domains_mocked, incr_mocked):
domains_mocked.return_value = TEST_DOMAINS
hashed_address = address_hash(self.local_portion, domain=self.service_domain)
baker.make(DeletedAddress, address_hash=hashed_address)
baker.make(DeletedAddress, address_hash=hashed_address)

Expand Down
Loading