From 6faaa8bb334f2c5bf8f185ec04a5039cdca1aae7 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Wed, 18 Sep 2024 16:52:52 -0500 Subject: [PATCH 1/8] MPP-3119: complaint notification disables mask --- emails/tests/views_tests.py | 47 ++++++++++++++++++++++++++++++++----- emails/views.py | 37 +++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index 114adc1e1b..6a43e290aa 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -1052,7 +1052,12 @@ def test_sns_message_with_hard_bounce_and_optout(self) -> None: @override_settings(STATSD_ENABLED=True) class ComplaintHandlingTest(TestCase): - """Test Complaint notifications and events.""" + """ + Test Complaint notifications and events. + + Example derived from: + https://docs.aws.amazon.com/ses/latest/dg/notification-contents.html#complaint-object + """ def setUp(self): self.user = baker.make(User, email="relayuser@test.com") @@ -1076,11 +1081,11 @@ def setUp(self): def test_notification_type_complaint(self): """ - A notificationType of complaint increments a counter, logs details, and - returns 200. - - Example derived from: - https://docs.aws.amazon.com/ses/latest/dg/notification-contents.html#complaint-object + A notificationType of complaint: + 1. increments a counter + 2. logs details, + 3. sets the user profile's auto_block_spam = True, and + 4. returns 200. """ assert self.user.profile.auto_block_spam is False @@ -1125,6 +1130,36 @@ def test_complaint_log_with_optout(self) -> None: assert log_data["user_match"] == "found" assert not log_data["fxa_id"] + def test_complaint_disables_mask(self): + """ + A notificationType of complaint: + 1. sets enabled=False on the mask, and + 2. returns 200. + """ + self.ra = baker.make( + RelayAddress, user=self.user, address="ebsbdsan7", domain=2 + ) + + # The top-level JSON object for complaints includes a "mail" field + # which contains information about the original mail to which the notification + # pertains. So, add a "mail" field with content from our russian_spam fixture + russian_spam_notification = create_notification_from_email( + EMAIL_INCOMING["russian_spam"] + ) + spam_mail_content = json.loads( + russian_spam_notification.get("Message", "") + ).get("mail", {}) + complaint_body_message = json.loads(self.complaint_body["Message"]) + complaint_body_message["mail"] = spam_mail_content + complaint_body_with_spam_mail = {"Message": json.dumps(complaint_body_message)} + assert self.ra.enabled is True + + response = _sns_notification(complaint_body_with_spam_mail) + assert response.status_code == 200 + + self.ra.refresh_from_db() + assert self.ra.enabled is False + class SNSNotificationRemoveEmailsInS3Test(TestCase): def setUp(self) -> None: diff --git a/emails/views.py b/emails/views.py index 64d0108168..69fb8b64aa 100644 --- a/emails/views.py +++ b/emails/views.py @@ -1368,7 +1368,9 @@ def _handle_reply( return HttpResponse("Sent email to final recipient.", status=200) -def _get_domain_address(local_portion: str, domain_portion: str) -> DomainAddress: +def _get_domain_address( + local_portion: str, domain_portion: str, create: bool = True +) -> DomainAddress: """ Find or create the DomainAddress for the parts of an email address. @@ -1396,6 +1398,8 @@ def _get_domain_address(local_portion: str, domain_portion: str) -> DomainAddres user=locked_profile.user, address=local_portion, domain=domain_numerical ).first() if domain_address is None: + if not create: + raise ObjectDoesNotExist("Address does not exist") # TODO: Consider flows when a user generating alias on a fly # was unable to receive an email due to user no longer being a # premium user as seen in exception thrown on make_domain_address @@ -1414,12 +1418,12 @@ def _get_domain_address(local_portion: str, domain_portion: str) -> DomainAddres raise e -def _get_address(address: str) -> RelayAddress | DomainAddress: +def _get_address(address: str, create: bool = True) -> RelayAddress | DomainAddress: """ Find or create the RelayAddress or DomainAddress for an email address. - If an unknown email address is for a valid subdomain, a new DomainAddress - will be created. + If an unknown email address is for a valid subdomain, and create is True, + a new DomainAddress will be created. On failure, raises exception based on Django's ObjectDoesNotExist: * RelayAddress.DoesNotExist - looks like RelayAddress, deleted or does not exist @@ -1445,6 +1449,8 @@ def _get_address(address: str) -> RelayAddress | DomainAddress: ) return relay_address except RelayAddress.DoesNotExist as e: + if not create: + raise e try: DeletedAddress.objects.get( address_hash=address_hash(local_address, domain=domain) @@ -1572,6 +1578,11 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: """ Handle an AWS SES complaint notification. + Sets the user's auto_block_spam flag to True. + + Disables the mask thru which the spam mail was forwarded, and sends an email to the + user to notify them the mask is disabled and can be re-enabled on their dashboard. + For more information, see: https://docs.aws.amazon.com/ses/latest/dg/notification-contents.html#complaint-object @@ -1580,7 +1591,7 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: * 200 response if all match or none are given Emits a counter metric "email_complaint" with these tags: - * complaint_subtype: 'onaccounsuppressionlist', or 'none' if omitted + * complaint_subtype: 'onaccountsuppressionlist', or 'none' if omitted * complaint_feedback - feedback enumeration from ISP or 'none' * user_match: 'found', 'missing', error states 'no_address' and 'no_recipients' * relay_action: 'no_action', 'auto_block_spam' @@ -1634,6 +1645,22 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: profile.auto_block_spam = True profile.save() + for destination_address in message_json.get("mail", {}).get("destination", []): + try: + address = _get_address(destination_address, False) + address.enabled = False + address.save() + # TODO: email the user that we disabled the mask + except ( + ObjectDoesNotExist, + RelayAddress.DoesNotExist, + DomainAddress.DoesNotExist, + ): + logger.error( + "Received a complaint from a destination address that does not match " + "a Relay address.", + ) + if not complaint_data: # Data when there are no identified recipients complaint_data = [{"user_match": "no_recipients", "relay_action": "no_action"}] From a156885df2796baaafe6d3b5c5066e37141c2519 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Thu, 19 Sep 2024 16:15:33 -0500 Subject: [PATCH 2/8] MPP-3119: add disabled_mask_for_spam email --- .../templates/emails/direct_email_footer.html | 19 ++ .../templates/emails/direct_email_header.html | 173 +++++++++++ .../emails/disabled_mask_for_spam.html | 32 ++ .../emails/disabled_mask_for_spam.txt | 11 + .../emails/reply_requires_premium.html | 190 +----------- .../disabled_mask_for_spam_expected.email | 289 ++++++++++++++++++ ...eply_requires_premium_first_expected.email | 5 +- ...ply_requires_premium_second_expected.email | 5 +- emails/tests/views_tests.py | 47 +++ emails/urls.py | 1 + emails/views.py | 68 ++++- privaterelay/pending_locales/en/pending.ftl | 18 ++ 12 files changed, 668 insertions(+), 190 deletions(-) create mode 100644 emails/templates/emails/direct_email_footer.html create mode 100644 emails/templates/emails/direct_email_header.html create mode 100644 emails/templates/emails/disabled_mask_for_spam.html create mode 100644 emails/templates/emails/disabled_mask_for_spam.txt create mode 100644 emails/tests/fixtures/disabled_mask_for_spam_expected.email diff --git a/emails/templates/emails/direct_email_footer.html b/emails/templates/emails/direct_email_footer.html new file mode 100644 index 0000000000..0320c0bb6a --- /dev/null +++ b/emails/templates/emails/direct_email_footer.html @@ -0,0 +1,19 @@ +{% comment %} + Note that Django only loads strings from some Fluent files. + See privaterelay/ftl_bundles.py. +{% endcomment %} +{% load ftl %} + + + + + + + + + diff --git a/emails/templates/emails/direct_email_header.html b/emails/templates/emails/direct_email_header.html new file mode 100644 index 0000000000..8e798c200c --- /dev/null +++ b/emails/templates/emails/direct_email_header.html @@ -0,0 +1,173 @@ +{% comment %} + Note that Django only loads strings from some Fluent files. + See privaterelay/ftl_bundles.py. +{% endcomment %} +{% load ftl %} + + + + + + + + + + + + + + + + + + +
+ warning icon + + {% ftlmsg 'upgrade-for-more-protection' %} +
diff --git a/emails/templates/emails/disabled_mask_for_spam.html b/emails/templates/emails/disabled_mask_for_spam.html new file mode 100644 index 0000000000..31d645037f --- /dev/null +++ b/emails/templates/emails/disabled_mask_for_spam.html @@ -0,0 +1,32 @@ +{% comment %} + Note that Django only loads strings from some Fluent files. + See privaterelay/ftl_bundles.py. +{% endcomment %} +{% load ftl %} +{% load email_extras %} +{% withftl bundle='privaterelay.ftl_bundles.main' language=language %} + +{% include "emails/direct_email_header.html" %} + + + + + +
+ {% with mask|striptags|urlencode as mask_url %} +

+ warning icon + {% ftlmsg 'relay-disabled-your-mask' %} +

+

+ {% ftlmsg 'relay-received-spam-complaint-html' mask=mask %} {% ftlmsg 'relay-disabled-your-mask-detail-html' mask=mask %} +

+ + {% ftlmsg 're-enable-your-mask' %} + + {% endwith %} +
+ + {% include "emails/direct_email_footer.html" %} + +{% endwithftl %} diff --git a/emails/templates/emails/disabled_mask_for_spam.txt b/emails/templates/emails/disabled_mask_for_spam.txt new file mode 100644 index 0000000000..b75eb7c535 --- /dev/null +++ b/emails/templates/emails/disabled_mask_for_spam.txt @@ -0,0 +1,11 @@ +{% load ftl %} +{% load email_extras %} +{% withftl bundle='privaterelay.ftl_bundles.main' language=language %} +{% ftlmsg 'relay-disabled-your-mask' %} + +{% ftlmsg 'relay-received-spam-complaint' mask=mask %} {% ftlmsg 'relay-disabled-your-mask-detail' mask=mask %} +{% with mask|striptags|urlencode as mask_url %} +{% ftlmsg 're-enable-your-mask' %} +{{ SITE_ORIGIN }}/accounts/profile/#{{ mask_url }} +{% endwith %} +{% endwithftl %} diff --git a/emails/templates/emails/reply_requires_premium.html b/emails/templates/emails/reply_requires_premium.html index 20932d351d..75147724dd 100644 --- a/emails/templates/emails/reply_requires_premium.html +++ b/emails/templates/emails/reply_requires_premium.html @@ -5,176 +5,7 @@ {% load ftl %} {% load email_extras %} {% withftl bundle='privaterelay.ftl_bundles.main' language=language %} - - - - - - - - - - - - - - - - - - - -
- warning icon - - {% ftlmsg 'upgrade-for-more-protection' %} -
- +{% include "emails/direct_email_header.html" %}
@@ -198,20 +29,5 @@

- - - - - - - - - - - -{% endwithftl %} \ No newline at end of file +{% include "emails/direct_email_footer.html" %} +{% endwithftl %} diff --git a/emails/tests/fixtures/disabled_mask_for_spam_expected.email b/emails/tests/fixtures/disabled_mask_for_spam_expected.email new file mode 100644 index 0000000000..3eb2776de8 --- /dev/null +++ b/emails/tests/fixtures/disabled_mask_for_spam_expected.email @@ -0,0 +1,289 @@ +Subject: =?utf-8?q?=E2=81=A8Firefox_Relay=E2=81=A9?= has disabled one of your + email masks. +From: reply@relay.example.com +To: +MIME-Version: 1.0 +Content-Type: multipart/alternative; + boundary="==[BOUNDARY0]==" + +--==[BOUNDARY0]== +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: quoted-printable + + + + +=E2=81=A8Firefox Relay=E2=81=A9 has disabled one of your email masks. + +=E2=81=A8Firefox Relay=E2=81=A9 received a spam complaint for an email sent t= +o =E2=81=A8w41fwbt4q@test.com=E2=81=A9. This usually happens if you or your e= +mail provider mark an email as spam. To prevent further spam, =E2=81=A8Firefo= +x Relay=E2=81=A9 has disabled your =E2=81=A8w41fwbt4q@test.com=E2=81=A9 mask. + +Visit your =E2=81=A8Firefox Relay=E2=81=A9 dashboard to re-enable this mask. +http://127.0.0.1:8000/accounts/profile/#w41fwbt4q%40test.com + + + +--==[BOUNDARY0]== +Content-Type: text/html; charset="utf-8" +Content-Transfer-Encoding: quoted-printable +MIME-Version: 1.0 + + + + + + + + + + + + + + + + + + + + + + + + + + +
=20 + 3D"= =20 + =20 + Upgrade for more protection=20 +
=20 + + + + + =20 + +
+ =20 +

=20 + 3D"warning + =E2=81=A8Firefox Relay=E2=81=A9 has disabled one of your = +email masks. +

+

+ Firefox Relay received a spam complaint for an email sent to = +w41fwbt4q@test.com. This usually happens if you or your emai= +l provider mark an email as spam. To prevent further spam, Firefox Relay has = +disabled your w41fwbt4q@test.com mask. +

=20 + + Visit your =E2=81=A8Firefox Relay=E2=81=A9 dashboard to r= +e-enable this mask. + + =20 +
+ + =20 + + + + + + =20 + +
=20 + 3D"= =20 + =20 + Upgrade to =E2=81=A8Firefox = +Relay Premium=E2=81=A9=20 + Manage your masks=20 +
=20 + + + + + + +--==[BOUNDARY0]==-- diff --git a/emails/tests/fixtures/reply_requires_premium_first_expected.email b/emails/tests/fixtures/reply_requires_premium_first_expected.email index bfab7be2e5..13227fe6e1 100644 --- a/emails/tests/fixtures/reply_requires_premium_first_expected.email +++ b/emails/tests/fixtures/reply_requires_premium_first_expected.email @@ -38,6 +38,7 @@ MIME-Version: 1.0 + @@ -223,7 +224,7 @@ ail" style=3D"color: white;">Upgrade for more protection=20 =20 - =20 + @@ -281,4 +282,6 @@ dium=3Demail" style=3D"color: white;">Manage your masks=20 + + --==[BOUNDARY0]==-- diff --git a/emails/tests/fixtures/reply_requires_premium_second_expected.email b/emails/tests/fixtures/reply_requires_premium_second_expected.email index a8a81d85bc..dee7b0bd31 100644 --- a/emails/tests/fixtures/reply_requires_premium_second_expected.email +++ b/emails/tests/fixtures/reply_requires_premium_second_expected.email @@ -37,6 +37,7 @@ MIME-Version: 1.0 + @@ -222,7 +223,7 @@ ail" style=3D"color: white;">Upgrade for more protection=20
=20 - =20 + @@ -280,4 +281,6 @@ dium=3Demail" style=3D"color: white;">Manage your masks=20 + + --==[BOUNDARY0]==-- diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index 6a43e290aa..90fa3bec29 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -12,6 +12,7 @@ from unittest.mock import Mock, patch from uuid import uuid4 +from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist from django.http import HttpResponse @@ -45,6 +46,7 @@ from emails.views import ( EmailDroppedReason, ReplyHeadersNotFound, + _build_disabled_mask_for_spam_email, _build_reply_requires_premium_email, _get_address, _get_keys_from_headers, @@ -1051,6 +1053,7 @@ def test_sns_message_with_hard_bounce_and_optout(self) -> None: @override_settings(STATSD_ENABLED=True) +@override_settings(RELAY_FROM_ADDRESS="reply@relay.example.com") class ComplaintHandlingTest(TestCase): """ Test Complaint notifications and events. @@ -1078,6 +1081,12 @@ def setUp(self): }, } self.complaint_body = {"Message": json.dumps(complaint)} + ses_client_patcher = patch( + "emails.apps.EmailsConfig.ses_client", + spec_set=["send_raw_email"], + ) + self.mock_ses_client = ses_client_patcher.start() + self.addCleanup(ses_client_patcher.stop) def test_notification_type_complaint(self): """ @@ -1158,7 +1167,45 @@ def test_complaint_disables_mask(self): assert response.status_code == 200 self.ra.refresh_from_db() + source = self.mock_ses_client.send_raw_email.call_args.kwargs["Source"] + destinations = self.mock_ses_client.send_raw_email.call_args.kwargs["Destinations"] + raw_message = self.mock_ses_client.send_raw_email.call_args.kwargs["RawMessage"] + data_without_newlines = raw_message["Data"].replace("\n", "") + assert self.ra.enabled is False + self.mock_ses_client.send_raw_email.assert_called_once() + assert source == settings.RELAY_FROM_ADDRESS + assert destinations == [self.ra.user.email] + assert "To prevent further spam" in data_without_newlines + assert self.ra.full_address in data_without_newlines + + # re-enable the mask for other tests + self.ra.enabled = True + self.ra.save() + self.ra.refresh_from_db() + + def test_build_disabled_mask_for_spam_email(self): + free_user = make_free_test_user() + test_mask_address = "w41fwbt4q" + relay_address = baker.make( + RelayAddress, user=free_user, address=test_mask_address, domain=2 + ) + + original_spam_email: dict = {"mask": relay_address.full_address} + + msg = _build_disabled_mask_for_spam_email(relay_address, original_spam_email) + + assert msg["Subject"] == main.format("relay-disabled-your-mask") + assert msg["From"] == settings.RELAY_FROM_ADDRESS + assert msg["To"] == free_user.email + + text_content, html_content = get_text_and_html_content(msg) + assert test_mask_address in text_content + assert test_mask_address in html_content + + assert_email_equals_fixture( + msg.as_string(), "disabled_mask_for_spam", replace_mime_boundaries=True + ) class SNSNotificationRemoveEmailsInS3Test(TestCase): diff --git a/emails/urls.py b/emails/urls.py index d0a9f0ca15..025443752b 100644 --- a/emails/urls.py +++ b/emails/urls.py @@ -13,4 +13,5 @@ path("first_time_user_test", views.first_time_user_test), path("reply_requires_premium_test", views.reply_requires_premium_test), path("first_forwarded_email", views.first_forwarded_email_test), + path("disabled_mask_for_spam_test", views.disabled_mask_for_spam_test), ] diff --git a/emails/views.py b/emails/views.py index 69fb8b64aa..5a8a9d0466 100644 --- a/emails/views.py +++ b/emails/views.py @@ -139,6 +139,32 @@ def reply_requires_premium_test(request): return render(request, "emails/reply_requires_premium.html", email_context) +def disabled_mask_for_spam_test(request): + """ + Demonstrate rendering of the "Disabled mask for spam" email. + + Settings like language can be given in the querystring, otherwise settings + come from a random free profile. + """ + mask = "abc123456@mozmail.com" + email_context = { + "mask": mask, + "SITE_ORIGIN": settings.SITE_ORIGIN, + } + for param in request.GET: + email_context[param] = request.GET.get(param) + + for param in request.GET: + if param == "content-type" and request.GET[param] == "text/plain": + return render( + request, + "emails/disabled_mask_for_spam.txt", + email_context, + "text/plain; charset=utf-8", + ) + return render(request, "emails/disabled_mask_for_spam.html", email_context) + + def first_forwarded_email_test(request: HttpRequest) -> HttpResponse: # TO DO: Update with correct context when trigger is created first_forwarded_email_html = render_to_string( @@ -1574,6 +1600,46 @@ def _handle_bounce(message_json: AWS_SNSMessageJSON) -> HttpResponse: return HttpResponse("OK", status=200) +def _build_disabled_mask_for_spam_email( + mask: RelayAddress | DomainAddress, original_spam_email: dict +) -> EmailMessage: + ctx = { + "mask": mask.full_address, + "spam_email": original_spam_email, + "SITE_ORIGIN": settings.SITE_ORIGIN, + } + html_body = render_to_string("emails/disabled_mask_for_spam.html", ctx) + text_body = render_to_string("emails/disabled_mask_for_spam.txt", ctx) + + # Create the message + msg = EmailMessage() + msg["Subject"] = ftl_bundle.format("relay-disabled-your-mask") + msg["From"] = settings.RELAY_FROM_ADDRESS + msg["To"] = mask.user.email + msg.set_content(text_body) + msg.add_alternative(html_body, subtype="html") + return msg + + +def _send_disabled_mask_for_spam_email( + mask: RelayAddress | DomainAddress, original_spam_email: dict +) -> None: + msg = _build_disabled_mask_for_spam_email(mask, original_spam_email) + if not settings.RELAY_FROM_ADDRESS: + raise ValueError( + "Must set settings.RELAY_FROM_ADDRESS to send disabled_mask_for_spam email." + ) + try: + ses_send_raw_email( + source_address=settings.RELAY_FROM_ADDRESS, + destination_address=mask.user.email, + message=msg, + ) + except ClientError as e: + logger.error("reply_not_allowed_ses_client_error", extra=e.response["Error"]) + incr_if_enabled("free_user_reply_attempt", 1) + + def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: """ Handle an AWS SES complaint notification. @@ -1650,7 +1716,7 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: address = _get_address(destination_address, False) address.enabled = False address.save() - # TODO: email the user that we disabled the mask + _send_disabled_mask_for_spam_email(address, message_json.get("mail", {})) except ( ObjectDoesNotExist, RelayAddress.DoesNotExist, diff --git a/privaterelay/pending_locales/en/pending.ftl b/privaterelay/pending_locales/en/pending.ftl index 5f35a2cc62..2c9d4b9b24 100644 --- a/privaterelay/pending_locales/en/pending.ftl +++ b/privaterelay/pending_locales/en/pending.ftl @@ -3,3 +3,21 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. # This is the Django equivalent of frontend/pendingTranslations.ftl + +## Email sent to users when Relay disables their mask after the user marks a forwarded +## email as spam. + +relay-disabled-your-mask = { -brand-name-firefox-relay } has disabled one of your email masks. +# Variables +# $mask (string) - the Relay email mask that sent a spam complaint +relay-received-spam-complaint-html = { -brand-name-firefox-relay } received a spam complaint for an email sent to { $mask }. This usually happens if you or your email provider mark an email as spam. +# Variables +# $mask (string) - the Relay email mask that sent a spam complaint +relay-received-spam-complaint = { -brand-name-firefox-relay } received a spam complaint for an email sent to { $mask }. This usually happens if you or your email provider mark an email as spam. +# Variables +# $mask (string) - the Relay email mask that sent a spam complaint +relay-disabled-your-mask-detail-html = To prevent further spam, { -brand-name-firefox-relay } has disabled your { $mask } mask. +# Variables +# $mask (string) - the Relay email mask that sent a spam complaint +relay-disabled-your-mask-detail = To prevent further spam, { -brand-name-firefox-relay } has disabled your { $mask } mask. +re-enable-your-mask = Visit your { -brand-name-firefox-relay } dashboard to re-enable this mask. From 324695dde5395cac287bcad831cfbc4de6885274 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Tue, 24 Sep 2024 07:36:15 -0500 Subject: [PATCH 3/8] MPP-3119: add disable_mask_on_complaint waffle flag --- emails/tests/views_tests.py | 6 +++++- emails/views.py | 43 ++++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index 90fa3bec29..933eeb7816 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -24,6 +24,7 @@ from markus.main import MetricsRecord from markus.testing import MetricsMock from model_bakery import baker +from waffle.testutils import override_flag from emails.models import ( DeletedAddress, @@ -1139,6 +1140,7 @@ def test_complaint_log_with_optout(self) -> None: assert log_data["user_match"] == "found" assert not log_data["fxa_id"] + @override_flag("disable_mask_on_complaint", active=True) def test_complaint_disables_mask(self): """ A notificationType of complaint: @@ -1168,7 +1170,9 @@ def test_complaint_disables_mask(self): self.ra.refresh_from_db() source = self.mock_ses_client.send_raw_email.call_args.kwargs["Source"] - destinations = self.mock_ses_client.send_raw_email.call_args.kwargs["Destinations"] + destinations = self.mock_ses_client.send_raw_email.call_args.kwargs[ + "Destinations" + ] raw_message = self.mock_ses_client.send_raw_email.call_args.kwargs["RawMessage"] data_without_newlines = raw_message["Data"].replace("\n", "") diff --git a/emails/views.py b/emails/views.py index 5a8a9d0466..762468a274 100644 --- a/emails/views.py +++ b/emails/views.py @@ -31,7 +31,7 @@ from decouple import strtobool from markus.utils import generate_tag from sentry_sdk import capture_message -from waffle import sample_is_active +from waffle import get_waffle_flag_model, sample_is_active from privaterelay.ftl_bundles import main as ftl_bundle from privaterelay.models import Profile @@ -1640,6 +1640,24 @@ def _send_disabled_mask_for_spam_email( incr_if_enabled("free_user_reply_attempt", 1) +def _disable_masks_for_complaint(message_json: dict) -> None: + for destination_address in message_json.get("mail", {}).get("destination", []): + try: + address = _get_address(destination_address, False) + address.enabled = False + address.save() + _send_disabled_mask_for_spam_email(address, message_json.get("mail", {})) + except ( + ObjectDoesNotExist, + RelayAddress.DoesNotExist, + DomainAddress.DoesNotExist, + ): + logger.error( + "Received a complaint from a destination address that does not match " + "a Relay address.", + ) + + def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: """ Handle an AWS SES complaint notification. @@ -1711,21 +1729,16 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: profile.auto_block_spam = True profile.save() - for destination_address in message_json.get("mail", {}).get("destination", []): - try: - address = _get_address(destination_address, False) - address.enabled = False - address.save() - _send_disabled_mask_for_spam_email(address, message_json.get("mail", {})) - except ( - ObjectDoesNotExist, - RelayAddress.DoesNotExist, - DomainAddress.DoesNotExist, - ): - logger.error( - "Received a complaint from a destination address that does not match " - "a Relay address.", + disable_mask_on_complaint_flag, _ = get_waffle_flag_model().objects.get_or_create( + name="disable_mask_on_complaint", + defaults={ + "note": ( + "MPP-3119: When a Relay user marks an email as spam, disable the mask." ) + }, + ) + if disable_mask_on_complaint_flag.is_active_for_user(user): + _disable_masks_for_complaint(message_json) if not complaint_data: # Data when there are no identified recipients From 5d2e79df59aa660422e44018406d5793651e26e2 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Tue, 24 Sep 2024 11:16:54 -0500 Subject: [PATCH 4/8] MPP-3119: use flag_is_active_in_task and move to check each address user --- emails/views.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/emails/views.py b/emails/views.py index 762468a274..9119ae8e17 100644 --- a/emails/views.py +++ b/emails/views.py @@ -1641,12 +1641,23 @@ def _send_disabled_mask_for_spam_email( def _disable_masks_for_complaint(message_json: dict) -> None: + _, _ = get_waffle_flag_model().objects.get_or_create( + name="disable_mask_on_complaint", + defaults={ + "note": ( + "MPP-3119: When a Relay user marks an email as spam, disable the mask." + ) + }, + ) for destination_address in message_json.get("mail", {}).get("destination", []): try: address = _get_address(destination_address, False) - address.enabled = False - address.save() - _send_disabled_mask_for_spam_email(address, message_json.get("mail", {})) + if flag_is_active_in_task("disable_mask_on_complaint", address.user): + address.enabled = False + address.save() + _send_disabled_mask_for_spam_email( + address, message_json.get("mail", {}) + ) except ( ObjectDoesNotExist, RelayAddress.DoesNotExist, @@ -1729,16 +1740,7 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: profile.auto_block_spam = True profile.save() - disable_mask_on_complaint_flag, _ = get_waffle_flag_model().objects.get_or_create( - name="disable_mask_on_complaint", - defaults={ - "note": ( - "MPP-3119: When a Relay user marks an email as spam, disable the mask." - ) - }, - ) - if disable_mask_on_complaint_flag.is_active_for_user(user): - _disable_masks_for_complaint(message_json) + _disable_masks_for_complaint(message_json) if not complaint_data: # Data when there are no identified recipients From ab264a30822b5d16c6edb56b9c37a57ff56377f3 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Fri, 27 Sep 2024 10:00:08 -0500 Subject: [PATCH 5/8] MPP-3119: moves _disable_masks_for_complaint() into complained_recipients loop To ensure the Relay mask belongs to the user from whom we received the spam complaint, moves _disable_masks_for_complaint() into complained_recipients loop. Also, change _disable_masks_for_complaint to search the original mail "source" field for a Relay mask address to find the mask thru which the spam email was sent. --- emails/tests/views_tests.py | 3 +++ emails/views.py | 35 +++++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index 933eeb7816..1b49db83c2 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -1160,6 +1160,9 @@ def test_complaint_disables_mask(self): spam_mail_content = json.loads( russian_spam_notification.get("Message", "") ).get("mail", {}) + spam_mail_content["source"] = ( + f"hello@ac.spam.example.com [via Relay] <{self.ra.full_address}>" + ) complaint_body_message = json.loads(self.complaint_body["Message"]) complaint_body_message["mail"] = spam_mail_content complaint_body_with_spam_mail = {"Message": json.dumps(complaint_body_message)} diff --git a/emails/views.py b/emails/views.py index 9119ae8e17..298956b814 100644 --- a/emails/views.py +++ b/emails/views.py @@ -1640,19 +1640,42 @@ def _send_disabled_mask_for_spam_email( incr_if_enabled("free_user_reply_attempt", 1) -def _disable_masks_for_complaint(message_json: dict) -> None: +def _disable_masks_for_complaint(message_json: dict, user: User) -> None: + """ + See https://docs.aws.amazon.com/ses/latest/dg/notification-contents.html#mail-object + + message_json.mail.source contains the envelope MAIL FROM address from which the + original message was sent. + + Relay sends emails From: "original-From" . + + So, we can find the mask that sent the spam email by parsing the source value. + """ + flag_name = "disable_mask_on_complaint" _, _ = get_waffle_flag_model().objects.get_or_create( - name="disable_mask_on_complaint", + name=flag_name, defaults={ "note": ( "MPP-3119: When a Relay user marks an email as spam, disable the mask." ) }, ) - for destination_address in message_json.get("mail", {}).get("destination", []): + source = message_json.get("mail", {}).get("source", "") + # parseaddr is confused by 2 email addresses in the value, so use this + # regular expression to extract the mask address by searching for any relay domains + email_domains = get_domains_from_settings().values() + domain_pattern = "|".join(re.escape(domain) for domain in email_domains) + email_regex = rf"[\w\.-]+@(?:{domain_pattern})" + matches = re.findall(email_regex, source) + if not matches: + return + for mask_address in matches: try: - address = _get_address(destination_address, False) - if flag_is_active_in_task("disable_mask_on_complaint", address.user): + address = _get_address(mask_address, False) + # ensure the mask belongs to the user for whom Relay received a complaint + if address.user != user: + continue + if flag_is_active_in_task(flag_name, address.user): address.enabled = False address.save() _send_disabled_mask_for_spam_email( @@ -1740,7 +1763,7 @@ def _handle_complaint(message_json: AWS_SNSMessageJSON) -> HttpResponse: profile.auto_block_spam = True profile.save() - _disable_masks_for_complaint(message_json) + _disable_masks_for_complaint(message_json, user) if not complaint_data: # Data when there are no identified recipients From 53855a3d12dcede9d3b026bdd42e2ec125c9766f Mon Sep 17 00:00:00 2001 From: groovecoder Date: Fri, 27 Sep 2024 13:01:31 -0500 Subject: [PATCH 6/8] MPP-3119: fix test_build_disabled_mask_for_spam_email user to have a real email address --- emails/tests/fixtures/disabled_mask_for_spam_expected.email | 2 +- emails/tests/views_tests.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/emails/tests/fixtures/disabled_mask_for_spam_expected.email b/emails/tests/fixtures/disabled_mask_for_spam_expected.email index 3eb2776de8..7b2420cbd8 100644 --- a/emails/tests/fixtures/disabled_mask_for_spam_expected.email +++ b/emails/tests/fixtures/disabled_mask_for_spam_expected.email @@ -1,7 +1,7 @@ Subject: =?utf-8?q?=E2=81=A8Firefox_Relay=E2=81=A9?= has disabled one of your email masks. From: reply@relay.example.com -To: +To: testreal@email.com MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="==[BOUNDARY0]==" diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index 1b49db83c2..f5a1bddd1e 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -1192,7 +1192,7 @@ def test_complaint_disables_mask(self): self.ra.refresh_from_db() def test_build_disabled_mask_for_spam_email(self): - free_user = make_free_test_user() + free_user = make_free_test_user("testreal@email.com") test_mask_address = "w41fwbt4q" relay_address = baker.make( RelayAddress, user=free_user, address=test_mask_address, domain=2 From 7011fe3c686707a1e424af934ef4fe8ed96e7e08 Mon Sep 17 00:00:00 2001 From: groovecoder Date: Mon, 30 Sep 2024 13:08:04 -0500 Subject: [PATCH 7/8] MPP-3119: skip sending disabled mask email if user already disabled the mask --- emails/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/emails/views.py b/emails/views.py index 298956b814..5812d915a4 100644 --- a/emails/views.py +++ b/emails/views.py @@ -1672,8 +1672,9 @@ def _disable_masks_for_complaint(message_json: dict, user: User) -> None: for mask_address in matches: try: address = _get_address(mask_address, False) - # ensure the mask belongs to the user for whom Relay received a complaint - if address.user != user: + # ensure the mask belongs to the user for whom Relay received a complaint, + # and that they haven't already disabled the mask themselves. + if address.user != user or address.enabled is False: continue if flag_is_active_in_task(flag_name, address.user): address.enabled = False From add4afd286d1a42da8f209061b524ab83f3a811d Mon Sep 17 00:00:00 2001 From: groovecoder Date: Mon, 30 Sep 2024 13:46:52 -0500 Subject: [PATCH 8/8] MPP-3119: add waffle_flag_by_fxa_uid command --- .../commands/waffle_flag_by_fxa_uid.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 privaterelay/management/commands/waffle_flag_by_fxa_uid.py diff --git a/privaterelay/management/commands/waffle_flag_by_fxa_uid.py b/privaterelay/management/commands/waffle_flag_by_fxa_uid.py new file mode 100644 index 0000000000..d20dceafbf --- /dev/null +++ b/privaterelay/management/commands/waffle_flag_by_fxa_uid.py @@ -0,0 +1,25 @@ +from typing import Any + +from django.core.management.base import CommandParser + +from allauth.socialaccount.models import SocialAccount +from waffle.management.commands.waffle_flag import Command as FlagCommand + + +class Command(FlagCommand): + def add_arguments(self, parser: CommandParser) -> None: + parser.add_argument( + "--fxa", + action="append", + default=list(), + help="Turn on the flag for listed FXA uids.", + ) + return super().add_arguments(parser) + + def handle(self, *args: Any, **options: Any) -> None: + if "fxa" in options: + uids: list[str] = options.get("fxa", []) + for uid in uids: + social_account = SocialAccount.objects.get(uid=uid, provider="fxa") + options["user"].append(social_account.user.email) + return super().handle(*args, **options)