From a0facd5236bf6308d98d0d28a76c9b434681b71c Mon Sep 17 00:00:00 2001 From: Andrew Williamson Date: Wed, 10 Aug 2016 13:04:57 -0700 Subject: [PATCH] Add API endpoint to process POST email content and add to activity log; emails received send notification emails; review emails and notification emails can be replied to. --- docs/topics/api/activity.rst | 93 +++++++++ docs/topics/api/addons.rst | 60 ------ docs/topics/api/index.rst | 1 + requirements/prod.txt | 7 +- settings.py | 3 + src/olympia/activity/models.py | 42 ++++ src/olympia/activity/tasks.py | 14 ++ .../templates/activity/emails/developer.txt | 12 ++ .../activity/tests/emails/message.json | 97 +++++++++ src/olympia/activity/tests/test_models.py | 40 ++++ src/olympia/activity/tests/test_utils.py | 196 ++++++++++++++++++ src/olympia/activity/tests/test_views.py | 73 ++++++- src/olympia/activity/urls.py | 7 + src/olympia/activity/utils.py | 182 ++++++++++++++++ src/olympia/activity/views.py | 50 +++++ src/olympia/amo/log.py | 8 + src/olympia/amo/tasks.py | 10 +- src/olympia/amo/tests/test_send_mail.py | 8 + src/olympia/amo/utils.py | 3 +- src/olympia/api/urls.py | 1 + src/olympia/editors/helpers.py | 51 ++--- src/olympia/editors/tests/test_helpers.py | 39 +++- src/olympia/lib/settings_base.py | 9 + .../migrations/897-activity-email-waffle.sql | 2 + .../898-log-activity-tokens-table.sql | 14 ++ 25 files changed, 923 insertions(+), 99 deletions(-) create mode 100644 docs/topics/api/activity.rst create mode 100644 src/olympia/activity/models.py create mode 100644 src/olympia/activity/tasks.py create mode 100644 src/olympia/activity/templates/activity/emails/developer.txt create mode 100644 src/olympia/activity/tests/emails/message.json create mode 100644 src/olympia/activity/tests/test_models.py create mode 100644 src/olympia/activity/tests/test_utils.py create mode 100644 src/olympia/activity/urls.py create mode 100644 src/olympia/activity/utils.py create mode 100644 src/olympia/migrations/897-activity-email-waffle.sql create mode 100644 src/olympia/migrations/898-log-activity-tokens-table.sql diff --git a/docs/topics/api/activity.rst b/docs/topics/api/activity.rst new file mode 100644 index 0000000000..8944a9da78 --- /dev/null +++ b/docs/topics/api/activity.rst @@ -0,0 +1,93 @@ +======== +Activity +======== + +.. note:: + + These APIs are experimental and are currently being worked on. Endpoints + may change without warning. The only authentication method available at + the moment is :ref:`the internal one`. + + +----------------- +Review Notes List +----------------- + +.. _review-notes-version-list: + +This endpoint allows you to list the approval/rejection review history for a version of an add-on. + +.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/ + + .. note:: + All add-ons require authentication and either + reviewer permissions or a user account listed as a developer of the + add-on. + + :>json int count: The number of versions for this add-on. + :>json string next: The URL of the next page of results. + :>json string previous: The URL of the previous page of results. + :>json array results: An array of :ref:`per version review notes`. + + +------------------- +Review Notes Detail +------------------- + +.. _review-notes-version-detail: + +This endpoint allows you to fetch a single review note for a specific version of an add-on. + +.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/(int:id)/ + + .. _review-notes-version-detail-object: + + :>json int id: The id for a review note. + :>json string action: The :ref:`type of review note`. + :>json string action_label: The text label of the action. + :>json int user.id: The id of the reviewer or author who left the review note. + :>json string user.name: The name of the reviewer or author. + :>json string user.url: The link to the profile page for of the reviewer or author. + :>json string comments: The text content of the review note. + :>json string date: The date the review note was created. + + +.. _review-note-action: + + Possible values for the ``action`` field: + + ========================== ========================================================== + Value Description + ========================== ========================================================== + approved Version, or file in the version, was approved + rejected Version, or file in the version, was rejected + review-requested Developer requested review + more-information-requested Reviewer requested more information from developer + super-review-requested Add-on was referred to an admin for attention + comment Reviewer added comment for other reviewers + review-note Generic review comment + ========================== ========================================================== + + +----------------------- +Incoming Mail End-point +----------------------- + +.. _activity_mail: + +This endpoint allows a mail server or similar to submit a json object containing single email into AMO which will be processed. +The only type of email currently supported is a reply to an activity email (e.g an add-on review, or a reply to an add-on review). +Any other content or invalid emails will be discarded. + +.. http:post:: /api/v3/activity/mail + + .. note:: + This API endpoint uses a custom authentication method. + The value `SecretKey` in the submitted json must match one defined in `settings.INBOUND_EMAIL_SECRET_KEY`. + The IP address of the request must match one defined in `settings.ALLOWED_CLIENTS_EMAIL_API`, if defined. + + :json string version: The version number string for the version. ------------------ -Review Notes List ------------------ - -.. _review-notes-version-list: - -This endpoint allows you to list the approval/rejection review history for a version of an add-on. - -.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/ - - .. note:: - All add-ons require authentication and either - reviewer permissions or a user account listed as a developer of the - add-on. - - :>json int count: The number of versions for this add-on. - :>json string next: The URL of the next page of results. - :>json string previous: The URL of the previous page of results. - :>json array results: An array of :ref:`per version review notes`. - - -------------------- -Review Notes Detail -------------------- - -.. _review-notes-version-detail: - -This endpoint allows you to fetch a single review note for a specific version of an add-on. - -.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/(int:id)/ - - .. _review-notes-version-detail-object: - - :>json int id: The id for a review note. - :>json string action: The :ref:`type of review note`. - :>json string .action_label: The text label of the action. - :>json int user.id: The id of the reviewer or author who left the review note. - :>json string user.name: The name of the reviewer or author. - :>json string user.url: The link to the profile page for of the reviewer or author. - :>json string comments: The text content of the review note. - :>json string date: The date the review note was created. - - -.. _review-note-action: - - Possible values for the ``action`` field: - - ========================== ========================================================== - Value Description - ========================== ========================================================== - approved Version, or file in the version, was approved - rejected Version, or file in the version, was rejected - review-requested Developer requested review - more-information-requested Reviewer requested more information from developer - super-review-requested Add-on was referred to an admin for attention - comment Reviewer added comment for other reviewers - review-note Generic review comment - ========================== ========================================================== - - ---------------------------- Add-on Feature Compatibility ---------------------------- diff --git a/docs/topics/api/index.rst b/docs/topics/api/index.rst index 50a8da9408..058f14e7c8 100644 --- a/docs/topics/api/index.rst +++ b/docs/topics/api/index.rst @@ -33,6 +33,7 @@ using the API. auth auth_internal accounts + activity addons categories discovery diff --git a/requirements/prod.txt b/requirements/prod.txt index 5ba6818c92..e1f4627990 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -126,6 +126,7 @@ django-tables2==1.1.2 \ django-waffle==0.11 \ --hash=sha256:9cd9e3a976849a3cd816830d7811b93543c1dc9da9f3cb1ccc2f9da831b8b025 \ --hash=sha256:914b4b874fa6250a0897bc30b67e02034d92a7235ab72a91bf6da49b71751de4 +# djangorestframework is required by drf-nested-routers djangorestframework==3.3.3 \ --hash=sha256:4f47056ad798103fc9fb049dff8a67a91963bd215d31bad12ad72b891559ab16 \ --hash=sha256:f606f2bb4e9bb320937cb6ccce299991b2d302f5cc705a671dffca491e55935c @@ -144,6 +145,8 @@ elasticsearch==1.1.1 \ elasticsearch-dsl==0.0.11 \ --hash=sha256:663fb62ad39200c7d903e973aa0aa693578613264d83796455cbf4cd172bd878 \ --hash=sha256:59a76c4142478a1952bba6f9a9ca4fc7b029afb619e8ffcf0d135ce37ea692da +email-reply-parser==0.3.0 \ + --hash=sha256:83cd931e40b5dbef0a69462d9392d71610afbae729a135706f09255f78c08043 # enum34 is required by cryptography enum34==1.1.6 \ --hash=sha256:6bd0f6ad48ec2aa117d3d141940d484deccda84d4fcd884f5c3d93c23ecd8c79 \ @@ -163,7 +166,7 @@ funcsigs==1.0.2 \ google-api-python-client==1.2 \ --hash=sha256:3cb3f39c4a634950aee34f52e2a160b9a064b15210f7196ba364f670780aa675 \ --hash=sha256:f6506e837a7401ecd97cad45900716eb12fb480f303d0dee5c61e8a4b16ff5ec -# html5lib is required by bleach +# html5lib is required by bleach, rdflib html5lib==0.9999999 \ --hash=sha256:2612a191a8d5842bfa057e41ba50bbb9dcb722419d2408c78cff4758d0754868 # httplib2 is required by google-api-python-client @@ -265,7 +268,7 @@ rdflib==3.4.0 \ --hash=sha256:78d5f11a7001661d7637f9e61554a5f8971e197f3f6d17ba5e4039b0668116cf redis==2.8.0 \ --hash=sha256:5a34f92937cacb4082f5834d2ce8b710b791342d17d1769b998327e6479e2b24 -# requests is required by amo-validator, codecov, nobot +# requests is required by amo-validator, django-mozilla-product-details, nobot requests==2.11.1 \ --hash=sha256:545c4855cd9d7c12671444326337013766f4eea6068c3f0307fb2dc2696d580e \ --hash=sha256:5acf980358283faba0b897c73959cecf8b841205bb4b2ad3ef545f46eae1a133 diff --git a/settings.py b/settings.py index d19a85f392..629373d144 100644 --- a/settings.py +++ b/settings.py @@ -121,6 +121,9 @@ CSP_FRAME_SRC += ('https://www.sandbox.paypal.com',) CSP_IMG_SRC += (HTTP_GA_SRC,) CSP_SCRIPT_SRC += (HTTP_GA_SRC, "'self'") +# Auth token required to authorize inbound email. +INBOUND_EMAIL_SECRET_KEY = 'totally-unsecure-string-for-local-development-goodness' + # If you have settings you want to overload, put them in a local_settings.py. try: from local_settings import * # noqa diff --git a/src/olympia/activity/models.py b/src/olympia/activity/models.py new file mode 100644 index 0000000000..de2fe838fa --- /dev/null +++ b/src/olympia/activity/models.py @@ -0,0 +1,42 @@ +import uuid + +from django.db import models + +import commonware.log + +from olympia.amo.models import ModelBase +from olympia.versions.models import Version + +log = commonware.log.getLogger('z.devhub') + +# Number of times a token can be used. +MAX_TOKEN_USE_COUNT = 100 + + +class ActivityLogToken(ModelBase): + version = models.ForeignKey(Version, related_name='token') + user = models.ForeignKey('users.UserProfile', + related_name='activity_log_tokens') + uuid = models.UUIDField(default=lambda: uuid.uuid4().hex, unique=True) + use_count = models.IntegerField( + default=0, + help_text='Stores the number of times the token has been used') + + class Meta: + db_table = 'log_activity_tokens' + unique_together = ('version', 'user') + + def is_expired(self): + return self.use_count >= MAX_TOKEN_USE_COUNT + + def is_valid(self): + return (not self.is_expired() and + self.version.addon.latest_version == self.version) + + def expire(self): + self.update(use_count=MAX_TOKEN_USE_COUNT) + + def increment_use(self): + self.__class__.objects.filter(pk=self.pk).update( + use_count=models.expressions.F('use_count') + 1) + self.use_count = self.use_count + 1 diff --git a/src/olympia/activity/tasks.py b/src/olympia/activity/tasks.py new file mode 100644 index 0000000000..6ca118f40a --- /dev/null +++ b/src/olympia/activity/tasks.py @@ -0,0 +1,14 @@ +import commonware.log +from olympia.amo.celery import task +from olympia.activity.utils import add_email_to_activity_log + + +log = commonware.log.getLogger('z.task') + + +@task +def process_email(message, **kwargs): + """Parse emails and save activity log entry.""" + res = add_email_to_activity_log(message) + if not res: + log.error('Failed to save email.') diff --git a/src/olympia/activity/templates/activity/emails/developer.txt b/src/olympia/activity/templates/activity/emails/developer.txt new file mode 100644 index 0000000000..70efcc8f1b --- /dev/null +++ b/src/olympia/activity/templates/activity/emails/developer.txt @@ -0,0 +1,12 @@ +Hello, + +A reply has been added to the review of version {{ number }} of add-on {{ name }}. + +{{ author }} wrote: + +{{ comments }} + +If you want to respond to this email please reply to this email or visit {{ url }}. If you need to send file attachments, please include the address amo-editors@mozilla.org and mention it in your reply. +-- +Mozilla Add-ons +{{ SITE_URL }} diff --git a/src/olympia/activity/tests/emails/message.json b/src/olympia/activity/tests/emails/message.json new file mode 100644 index 0000000000..488000790b --- /dev/null +++ b/src/olympia/activity/tests/emails/message.json @@ -0,0 +1,97 @@ +{ + "SecretKey":"SOME SECRET KEY", + "Message":{ + "TextCharSet":"us-ascii", + "HtmlCharSet":"us-ascii", + "EmbeddedMedia":null, + "MailingId":null, + "MessageId":null, + "Subject":"This is the subject of a test message.", + "TextBody":"This is a developer reply to an AMO. It's nice.", + "HtmlBody":"\u003chtml xmlns:v=\"urn:schemas-microsoft-com:vml\" xmlns:o=\"urn:schemas-microsoft-com:office:office\" xmlns:w=\"urn:schemas-microsoft-com:office:word\" xmlns:m=\"http://schemas.microsoft.com/office/2004/12/omml\" xmlns=\"http://www.w3.org/TR/REC-html40\"\u003e\u003chead\u003e\u003cMETA HTTP-EQUIV=\"Content-Type\" CONTENT=\"text/html; charset=us-ascii\"\u003e\u003cmeta name=Generator content=\"Microsoft Word 15 (filtered medium)\"\u003e\u003cstyle\u003e\u003c!--\r\n/* Font Definitions */\r\n@font-face\r\n\t{font-family:\"Cambria Math\";\r\n\tpanose-1:2 4 5 3 5 4 6 3 2 4;}\r\n@font-face\r\n\t{font-family:Calibri;\r\n\tpanose-1:2 15 5 2 2 2 4 3 2 4;}\r\n/* Style Definitions */\r\np.MsoNormal, li.MsoNormal, div.MsoNormal\r\n\t{margin:0in;\r\n\tmargin-bottom:.0001pt;\r\n\tfont-size:11.0pt;\r\n\tfont-family:\"Calibri\",\"sans-serif\";}\r\na:link, span.MsoHyperlink\r\n\t{mso-style-priority:99;\r\n\tcolor:#0563C1;\r\n\ttext-decoration:underline;}\r\na:visited, span.MsoHyperlinkFollowed\r\n\t{mso-style-priority:99;\r\n\tcolor:#954F72;\r\n\ttext-decoration:underline;}\r\nspan.EmailStyle17\r\n\t{mso-style-type:personal-compose;\r\n\tfont-family:\"Calibri\",\"sans-serif\";\r\n\tcolor:windowtext;}\r\n.MsoChpDefault\r\n\t{mso-style-type:export-only;\r\n\tfont-family:\"Calibri\",\"sans-serif\";}\r\n@page WordSection1\r\n\t{size:8.5in 11.0in;\r\n\tmargin:1.0in 1.0in 1.0in 1.0in;}\r\ndiv.WordSection1\r\n\t{page:WordSection1;}\r\n--\u003e\u003c/style\u003e\u003c!--[if gte mso 9]\u003e\u003cxml\u003e\r\n\u003co:shapedefaults v:ext=\"edit\" spidmax=\"1026\" /\u003e\r\n\u003c/xml\u003e\u003c![endif]--\u003e\u003c!--[if gte mso 9]\u003e\u003cxml\u003e\r\n\u003co:shapelayout v:ext=\"edit\"\u003e\r\n\u003co:idmap v:ext=\"edit\" data=\"1\" /\u003e\r\n\u003c/o:shapelayout\u003e\u003c/xml\u003e\u003c![endif]--\u003e\u003c/head\u003e\u003cbody lang=EN-US link=\"#0563C1\" vlink=\"#954F72\"\u003e\u003cdiv class=WordSection1\u003e\u003cp class=MsoNormal\u003eThis is the body of the test message.\u003co:p\u003e\u003c/o:p\u003e\u003c/p\u003e\u003c/div\u003e\u003c/body\u003e\u003c/html\u003e", + "CustomHeaders":[ + { + "Name":"Received", + "Value":"from smtp176.dfw.emailsrvr.com ([67.192.241.176]) by mx.socketlabs.com with ESMTP; Mon, 20 May 2013 10:01:04 -0400" + }, + { + "Name":"Received", + "Value":"from localhost (localhost.localdomain [127.0.0.1])by smtp7.relay.dfw1a.emailsrvr.com (SMTP Server) with ESMTP id F39D3258692for \u003ctest@customerexample.com\u003e; Mon, 20 May 2013 10:00:47 -0400 (EDT)" + }, + { + "Name":"Received", + "Value":"from smtp192.mex07a.mlsrvr.com (unknown [67.192.133.128])by smtp7.relay.dfw1a.emailsrvr.com (SMTP Server) with ESMTPS id 3E6022586DEfor \u003ctest@customerexample.com\u003e; Mon, 20 May 2013 10:00:45 -0400 (EDT)" + }, + { + "Name":"Received", + "Value":"from DFW1MBX19.mex07a.mlsrvr.com ([192.168.1.230]) byDFW1HUB14.mex07a.mlsrvr.com ([fe80::222:19ff:fe00:52bd%11]) with mapi; Mon,20 May 2013 09:00:45 -0500" + }, + { + "Name":"X-Virus-Scanned", + "Value":"OK" + }, + { + "Name":"Date", + "Value":"Mon, 20 May 2013 09:00:52 -0500" + }, + { + "Name":"Thread-Topic", + "Value":"This is the subject of a test message." + }, + { + "Name":"Thread-Index", + "Value":"Ac5VYnBo0JcanJnQTWSVA0k86Viq/Q==" + }, + { + "Name":"Message-ID", + "Value":"\u003cCC49375B755FC1499ECDFBA782412D5F08F64FD3E6@DFW1MBX19.mex07a.mlsrvr.com\u003e" + }, + { + "Name":"Accept-Language", + "Value":"en-US" + }, + { + "Name":"Content-Language", + "Value":"en-US" + }, + { + "Name":"X-MS-Has-Attach", + "Value":null + }, + { + "Name":"X-MS-TNEF-Correlator", + "Value":null + }, + { + "Name":"acceptlanguage", + "Value":"en-US" + }, + { + "Name":"Content-Type", + "Value":"multipart/alternative;boundary=\"_000_CC49375B755FC1499ECDFBA782412D5F08F64FD3E6DFW1MBX19mex0_\"" + }, + { + "Name":"MIME-Version", + "Value":"1.0" + } + ], + "To":[{ + "EmailAddress":"reviewreply+5a0b8a83d501412589cc5d562334b46b@addons.mozilla.org", + "FriendlyName":"AMO Reply" + }], + "Cc":null, + "Bcc":null, + "From":{ + "EmailAddress":"sender@example.com", + "FriendlyName":"Example Sender" + }, + "ReplyTo":null, + "Attachments":null + }, + "InboundMailFrom":"sender@example.com", + "InboundRcptTo":"test@customerexample.com", + "InboundIpAddress":"10.10.10.10", + "ErrorLog":null, + "SpamScore":0, + "SpamDetails":null +} \ No newline at end of file diff --git a/src/olympia/activity/tests/test_models.py b/src/olympia/activity/tests/test_models.py new file mode 100644 index 0000000000..452b5234df --- /dev/null +++ b/src/olympia/activity/tests/test_models.py @@ -0,0 +1,40 @@ +from olympia.activity.models import ActivityLogToken, MAX_TOKEN_USE_COUNT +from olympia.amo.tests import addon_factory, user_factory, TestCase + + +class TestActivityLogToken(TestCase): + def setUp(self): + super(TestActivityLogToken, self).setUp() + self.addon = addon_factory() + self.version = self.addon.latest_version + self.user = user_factory() + self.token = ActivityLogToken.objects.create( + version=self.version, user=self.user) + + def test_validity_use_expiry(self): + assert self.token.use_count == 0 + self.token.increment_use() + assert self.token.use_count == 1 + assert not self.token.is_expired() + self.token.expire() + assert self.token.use_count == MAX_TOKEN_USE_COUNT + # Being expired is invalid too. + assert self.token.is_expired() + # But the version is still the latest version. + assert self.version == self.addon.latest_version + assert not self.token.is_valid() + + def test_increment_use(self): + assert self.token.use_count == 0 + self.token.increment_use() + assert self.token.use_count == 1 + token_from_db = ActivityLogToken.objects.get( + version=self.version, user=self.user) + assert token_from_db.use_count == 1 + + def test_validity_version_out_of_date(self): + self.addon._latest_version = None + # The token isn't expired. + assert not self.token.is_expired() + # But is invalid, because the version isn't the latest version. + assert not self.token.is_valid() diff --git a/src/olympia/activity/tests/test_utils.py b/src/olympia/activity/tests/test_utils.py new file mode 100644 index 0000000000..c7eae199a1 --- /dev/null +++ b/src/olympia/activity/tests/test_utils.py @@ -0,0 +1,196 @@ +# -*- coding: utf-8 -*- +import json +import mock +import os + +from django.conf import settings +from django.core import mail + +import pytest + +from olympia import amo +from olympia.amo.helpers import absolutify +from olympia.amo.tests import addon_factory, user_factory, TestCase +from olympia.amo.urlresolvers import reverse +from olympia.activity.models import ActivityLogToken, MAX_TOKEN_USE_COUNT +from olympia.activity.utils import ( + add_email_to_activity_log, log_and_notify, send_activity_mail, + ActivityEmailParser) +from olympia.devhub.models import ActivityLog + + +TESTS_DIR = os.path.dirname(os.path.abspath(__file__)) +sample_message = os.path.join(TESTS_DIR, 'emails', 'message.json') + + +class TestEmailParser(TestCase): + + def test_basic_email(self): + message = json.loads(open(sample_message).read()) + email_text = message['Message'] + parser = ActivityEmailParser(email_text) + assert parser.get_uuid() == '5a0b8a83d501412589cc5d562334b46b' + assert parser.get_body() == ( + 'This is a developer reply to an AMO. It\'s nice.') + + +class TestAddEmailToActivityLog(TestCase): + def setUp(self): + self.addon = addon_factory(name='Badger', status=amo.STATUS_NOMINATED) + self.profile = user_factory() + self.token = ActivityLogToken.objects.create( + version=self.addon.current_version, user=self.profile) + self.token.update(uuid='5a0b8a83d501412589cc5d562334b46b') + message = json.loads(open(sample_message).read()) + self.message = message['Message'] + + def test_developer_comment(self): + self.profile.addonuser_set.create(addon=self.addon) + note = add_email_to_activity_log(self.message) + assert note.log == amo.LOG.DEVELOPER_REPLY_VERSION + self.token.refresh_from_db() + assert self.token.use_count == 1 + + def test_reviewer_comment(self): + self.grant_permission(self.profile, 'Addons:Review') + note = add_email_to_activity_log(self.message) + assert note.log == amo.LOG.REVIEWER_REPLY_VERSION + self.token.refresh_from_db() + assert self.token.use_count == 1 + + def test_with_max_count_token(self): + # Test with an invalid token. + self.token.update(use_count=MAX_TOKEN_USE_COUNT + 1) + assert not add_email_to_activity_log(self.message) + self.token.refresh_from_db() + assert self.token.use_count == MAX_TOKEN_USE_COUNT + 1 + + def test_with_unpermitted_token(self): + """Test when the token user doesn't have a permission to add a note.""" + assert not add_email_to_activity_log(self.message) + self.token.refresh_from_db() + assert self.token.use_count == 0 + + def test_non_existent_token(self): + self.token.update(uuid='12345678901234567890123456789012') + assert not add_email_to_activity_log(self.message) + + def test_with_invalid_msg(self): + assert not add_email_to_activity_log('youtube?v=dQw4w9WgXcQ') + + +class TestLogAndNotify(TestCase): + + def setUp(self): + self.developer = user_factory() + self.developer2 = user_factory() + self.reviewer = user_factory() + self.grant_permission(self.reviewer, 'Addons:Review', + 'Addon Reviewers') + self.senior_reviewer = user_factory() + self.grant_permission(self.senior_reviewer, 'Addons:Edit', + 'Senior Addon Reviewers') + self.grant_permission(self.senior_reviewer, 'Addons:Review', + 'Senior Addon Reviewers') + + self.addon = addon_factory() + self.addon.addonuser_set.create(user=self.developer) + self.addon.addonuser_set.create(user=self.developer2) + + def _create(self, action, author=None): + author = author or self.reviewer + details = { + 'comments': u'I spy, with my líttle €ye...', + 'version': self.addon.latest_version.version} + return amo.log(action, self.addon, self.addon.latest_version, + user=author, details=details, created=self.days_ago(1)) + + def _recipients(self, email_mock): + recipients = [] + for call in email_mock.call_args_list: + recipients += call[1]['recipient_list'] + [reply_to] = call[1]['reply_to'] + assert reply_to.startswith('reviewreply+') + assert reply_to.endswith(settings.INBOUND_EMAIL_DOMAIN) + return recipients + + def _check_email(self, call, url): + assert call[0][0] == ( + 'Mozilla Add-ons: %s Updated' % self.addon.name) + assert ('visit %s' % url) in call[0][1] + + @mock.patch('olympia.activity.utils.send_mail') + def test_developer_reply(self, send_mail_mock): + # One from the reviewer. + self._create(amo.LOG.REJECT_VERSION, self.reviewer) + # One from the developer. So the developer is on the 'thread' + self._create(amo.LOG.DEVELOPER_REPLY_VERSION, self.developer) + action = amo.LOG.DEVELOPER_REPLY_VERSION + comments = u'Thïs is á reply' + version = self.addon.latest_version + log_and_notify(action, comments, self.developer, version) + + logs = ActivityLog.objects.filter(action=action.id) + assert len(logs) == 2 # We added one above. + assert logs[0].details['comments'] == u'Thïs is á reply' + + assert send_mail_mock.call_count == 2 # One author, one reviewer. + recipients = self._recipients(send_mail_mock) + assert self.reviewer.email in recipients + assert self.developer2.email in recipients + # The developer who sent it doesn't get their email back. + assert self.developer.email not in recipients + + self._check_email(send_mail_mock.call_args_list[0], + self.addon.get_dev_url('versions')) + review_url = absolutify( + reverse('editors.review', args=[self.addon.pk], add_prefix=False)) + self._check_email(send_mail_mock.call_args_list[1], + review_url) + + @mock.patch('olympia.activity.utils.send_mail') + def test_reviewer_reply(self, send_mail_mock): + # One from the reviewer. + self._create(amo.LOG.REJECT_VERSION, self.reviewer) + # One from the developer. + self._create(amo.LOG.DEVELOPER_REPLY_VERSION, self.developer) + action = amo.LOG.REVIEWER_REPLY_VERSION + comments = u'Thîs ïs a revïewer replyîng' + version = self.addon.latest_version + log_and_notify(action, comments, self.reviewer, version) + + logs = ActivityLog.objects.filter(action=action.id) + assert len(logs) == 1 + assert logs[0].details['comments'] == u'Thîs ïs a revïewer replyîng' + + assert send_mail_mock.call_count == 2 # Both authors. + recipients = self._recipients(send_mail_mock) + assert self.developer.email in recipients + assert self.developer2.email in recipients + # The reviewer who sent it doesn't get their email back. + assert self.reviewer.email not in recipients + + self._check_email(send_mail_mock.call_args_list[0], + self.addon.get_dev_url('versions')) + self._check_email(send_mail_mock.call_args_list[1], + self.addon.get_dev_url('versions')) + + +@pytest.mark.django_db +def test_send_activity_mail(): + subject = u'This ïs ã subject' + message = u'And... this ïs a messãge!' + addon = addon_factory() + user = user_factory() + recipients = [user, ] + from_email = 'bob@bob.bob' + send_activity_mail(subject, message, addon.latest_version, recipients, + from_email) + + assert len(mail.outbox) == 1 + assert mail.outbox[0].body == message + assert mail.outbox[0].subject == subject + + uuid = addon.latest_version.token.get(user=user).uuid.hex + reply_email = 'reviewreply+%s@%s' % (uuid, settings.INBOUND_EMAIL_DOMAIN) + assert mail.outbox[0].reply_to == [reply_email] diff --git a/src/olympia/activity/tests/test_views.py b/src/olympia/activity/tests/test_views.py index b3213a25ec..c225529382 100644 --- a/src/olympia/activity/tests/test_views.py +++ b/src/olympia/activity/tests/test_views.py @@ -1,13 +1,21 @@ # -*- coding: utf-8 -*- import json +import mock +import StringIO + +from django.test.utils import override_settings from olympia import amo +from olympia.activity.models import ActivityLogToken from olympia.activity.tests.test_serializers import LogMixin +from olympia.activity.tests.test_utils import sample_message +from olympia.activity.views import inbound_email, EmailCreationPermission from olympia.amo.tests import ( - addon_factory, APITestClient, user_factory, TestCase) + addon_factory, APITestClient, req_factory_factory, user_factory, TestCase) from olympia.amo.urlresolvers import reverse from olympia.addons.models import AddonUser from olympia.addons.utils import generate_addon_guid +from olympia.devhub.models import ActivityLog from olympia.users.models import UserProfile @@ -198,3 +206,66 @@ class TestReviewNotesViewSetList(ReviewNotesViewSetDetailMixin, TestCase): self.url = reverse('version-reviewnotes-list', kwargs={ 'addon_pk': addon_pk or self.addon.pk, 'version_pk': version_pk or self.version.pk}) + + +@override_settings(ALLOWED_CLIENTS_EMAIL_API=['10.10.10.10']) +@override_settings(INBOUND_EMAIL_SECRET_KEY='SOME SECRET KEY') +class TestEmailApi(TestCase): + + def get_request(self, data): + datastr = json.dumps(data) + req = req_factory_factory(reverse('inbound-email-api')) + req.META['REMOTE_ADDR'] = '10.10.10.10' + req.META['CONTENT_LENGTH'] = len(datastr) + req.META['CONTENT_TYPE'] = 'application/json' + req.POST = data + req.method = 'POST' + req._stream = StringIO.StringIO(datastr) + return req + + def test_basic(self): + user = user_factory() + self.grant_permission(user, '*:*') + addon = addon_factory() + req = self.get_request( + json.loads(open(sample_message).read())) + + ActivityLogToken.objects.create( + user=user, version=addon.latest_version, + uuid='5a0b8a83d501412589cc5d562334b46b') + + res = inbound_email(req) + assert res.status_code == 201 + logs = ActivityLog.objects.for_addons(addon) + assert logs.count() == 1 + assert logs.get(action=amo.LOG.REVIEWER_REPLY_VERSION.id) + + def test_allowed(self): + assert EmailCreationPermission().has_permission( + self.get_request({'SecretKey': 'SOME SECRET KEY'}), None) + + def test_ip_denied(self): + req = self.get_request({'SecretKey': 'SOME SECRET KEY'}) + req.META['REMOTE_ADDR'] = '10.10.10.1' + assert not EmailCreationPermission().has_permission(req, None) + + def test_no_postfix_token(self): + req = self.get_request({}) + assert not EmailCreationPermission().has_permission(req, None) + + def test_postfix_token_denied(self): + req = self.get_request({'SecretKey': 'WRONG SECRET'}) + assert not EmailCreationPermission().has_permission(req, None) + + @mock.patch('olympia.activity.tasks.process_email.apply_async') + def test_successful(self, _mock): + req = self.get_request( + {'SecretKey': 'SOME SECRET KEY', 'Message': 'something'}) + res = inbound_email(req) + _mock.assert_called_with(('something',)) + assert res.status_code == 201 + + def test_bad_request(self): + """Test with no email body.""" + res = inbound_email(self.get_request({'SecretKey': 'SOME SECRET KEY'})) + assert res.status_code == 400 diff --git a/src/olympia/activity/urls.py b/src/olympia/activity/urls.py new file mode 100644 index 0000000000..f6d35a0291 --- /dev/null +++ b/src/olympia/activity/urls.py @@ -0,0 +1,7 @@ +from django.conf.urls import url + +from . import views + +urlpatterns = [ + url(r'^mail/', views.inbound_email, name='inbound-email-api'), +] diff --git a/src/olympia/activity/utils.py b/src/olympia/activity/utils.py new file mode 100644 index 0000000000..9e85b701ad --- /dev/null +++ b/src/olympia/activity/utils.py @@ -0,0 +1,182 @@ +import datetime +import logging +import re + +from django.conf import settings +from django.template import Context, loader + +from email_reply_parser import EmailReplyParser + +from olympia import amo +from olympia.access import acl +from olympia.activity.models import ActivityLogToken +from olympia.amo.helpers import absolutify +from olympia.amo.urlresolvers import reverse +from olympia.amo.utils import send_mail +from olympia.devhub.models import ActivityLog + +log = logging.getLogger('z.amo.activity') + +# Prefix of the reply to address in devcomm emails. +REPLY_TO_PREFIX = 'reviewreply+' + + +class ActivityEmailError(ValueError): + pass + + +class ActivityEmailEncodingError(ActivityEmailError): + pass + + +class ActivityEmailUUIDError(ActivityEmailError): + pass + + +class ActivityEmailParser(object): + """Utility to parse email replies.""" + address_prefix = REPLY_TO_PREFIX + + def __init__(self, message): + if (not type(message) is dict or 'TextBody' not in message): + log.exception('ActivityEmailParser didn\'t get a valid message.') + raise ActivityEmailEncodingError( + 'Invalid or malformed json message object') + + self.email = message + reply = self._extra_email_reply_parse(self.email['TextBody']) + self.reply = EmailReplyParser.read(reply).reply + + def _extra_email_reply_parse(self, email): + """ + Adds an extra case to the email reply parser where the reply is + followed by headers like "From: amo-editors@mozilla.org" and + strips that part out. + """ + email_header_re = re.compile('From: [^@]+@[^@]+\.[^@]+') + split_email = email_header_re.split(email) + if split_email[0].startswith('From: '): + # In case, it's a bottom reply, return everything. + return email + else: + # Else just return the email reply portion. + return split_email[0] + + def get_uuid(self): + for to in self.email.get('To', []): + address = to.get('EmailAddress', '') + if address.startswith(self.address_prefix): + # Strip everything between "reviewreply+" and the "@" sign. + return address[len(self.address_prefix):].split('@')[0] + log.exception( + 'TO: address missing or not related to activity emails. (%s)' + % self.email) + raise ActivityEmailUUIDError( + 'TO: address doesn\'t contain activity email uuid (%s)' + % self.email) + + def get_body(self): + return self.reply + + +def add_email_to_activity_log(message): + log.debug("Saving from email reply") + + try: + parser = ActivityEmailParser(message) + uuid = parser.get_uuid() + token = ActivityLogToken.objects.get(uuid=uuid) + except ActivityLogToken.DoesNotExist: + log.error('An email was skipped with non-existing uuid %s.' % uuid) + return False + except ActivityEmailError: + # We logged already when the exception occurred. + return False + + version = token.version + user = token.user + if token.is_valid(): + log_type = None + + review_perm = 'Review' if version.addon.is_listed else 'ReviewUnlisted' + if version.addon.authors.filter(pk=user.pk).exists(): + log_type = amo.LOG.DEVELOPER_REPLY_VERSION + elif acl.action_allowed_user(user, 'Addons', review_perm): + log_type = amo.LOG.REVIEWER_REPLY_VERSION + + if log_type: + note = log_and_notify(log_type, parser.get_body(), user, version) + log.info('A new note has been created (from %s using tokenid %s).' + % (user.id, uuid)) + token.increment_use() + return note + else: + log.error('%s did not have perms to reply to email thread %s.' + % (user.email, version.id)) + else: + log.error('%s tried to use an invalid activity email token for ' + 'version %s.' % (user.email, version.id)) + + return False + + +def log_and_notify(action, comments, note_creator, version): + log_kwargs = { + 'user': note_creator, + 'created': datetime.datetime.now(), + 'details': { + 'comments': comments, + 'version': version.version}} + note = amo.log(action, version.addon, version, **log_kwargs) + + # Collect reviewers/others involved with this version. + log_users = [ + alog.user for alog in ActivityLog.objects.for_version(version)] + # Collect add-on authors (excl. the person who sent the email.) + addon_authors = set(version.addon.authors.all()) - {note_creator} + # Collect reviewers on the thread (again, excl. the email sender) + reviewer_recipients = set(log_users) - addon_authors - {note_creator} + author_context_dict = { + 'name': version.addon.name, + 'number': version.version, + 'author': note_creator.name, + 'comments': comments, + 'url': version.addon.get_dev_url('versions'), + 'SITE_URL': settings.SITE_URL, + } + reviewer_context_dict = author_context_dict.copy() + reviewer_context_dict['url'] = absolutify( + reverse('editors.review', args=[version.addon.pk], add_prefix=False)) + + # Not being localised because we don't know the recipients locale. + subject = 'Mozilla Add-ons: %s Updated' % version.addon.name + template = loader.get_template('activity/emails/developer.txt') + send_activity_mail( + subject, template.render(Context(author_context_dict)), version, + addon_authors, settings.EDITORS_EMAIL) + send_activity_mail( + subject, template.render(Context(reviewer_context_dict)), version, + reviewer_recipients, settings.EDITORS_EMAIL) + return note + + +def send_activity_mail(subject, message, version, recipients, from_email, + perm_setting=None): + for recipient in recipients: + token, created = ActivityLogToken.objects.get_or_create( + version=version, user=recipient) + if not created: + token.update(use_count=0) + else: + # We need .uuid to be a real UUID not just a str. + token.reload() + log.info('Created token with UUID %s for user: %s.' % ( + token.uuid, recipient.id)) + reply_to = "%s%s@%s" % ( + REPLY_TO_PREFIX, token.uuid.hex, settings.INBOUND_EMAIL_DOMAIN) + log.info('Sending activity email to %s for %s version %s' % ( + recipient, version.addon.pk, version.pk)) + send_mail( + subject, message, recipient_list=[recipient.email], + from_email=from_email, use_blacklist=False, + perm_setting=perm_setting, reply_to=[reply_to]) diff --git a/src/olympia/activity/views.py b/src/olympia/activity/views.py index 24f647f3e2..3e548bfe9e 100644 --- a/src/olympia/activity/views.py +++ b/src/olympia/activity/views.py @@ -1,10 +1,20 @@ +import json +import logging + +from django.conf import settings from django.shortcuts import get_object_or_404 +from rest_framework import status +from rest_framework.decorators import (api_view, authentication_classes, + permission_classes) +from rest_framework.exceptions import ParseError from rest_framework.mixins import ListModelMixin, RetrieveModelMixin +from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet from olympia import amo from olympia.activity.serializers import ActivityLogSerializer +from olympia.activity.tasks import process_email from olympia.addons.views import AddonChildMixin from olympia.api.permissions import ( AllowAddonAuthor, AllowReviewer, AllowReviewerUnlisted, AnyOf) @@ -50,3 +60,43 @@ class VersionReviewNotesViewSet(AddonChildMixin, RetrieveModelMixin, if not latest_reply: return version_qs return version_qs.filter(created__gt=latest_reply.created) + + +log = logging.getLogger('z.amo.mail') + + +class EmailCreationPermission(object): + """Permit if client's IP address is allowed.""" + + def has_permission(self, request, view): + try: + # request.data isn't available at this point. + data = json.loads(request.body) + except ValueError: + data = {} + + secret_key = data.get('SecretKey', '') + if not secret_key == settings.INBOUND_EMAIL_SECRET_KEY: + log.info('Invalid secret key [%s] provided' % (secret_key,)) + return False + + remote_ip = request.META.get('REMOTE_ADDR', '') + allowed_ips = settings.ALLOWED_CLIENTS_EMAIL_API + if allowed_ips and remote_ip not in allowed_ips: + log.info('Request from invalid ip address [%s]' % (remote_ip,)) + return False + + return True + + +@api_view(['POST']) +@authentication_classes(()) +@permission_classes((EmailCreationPermission,)) +def inbound_email(request): + message = request.data.get('Message', None) + if not message: + raise ParseError( + detail='Message not present in the POST data.') + + process_email.apply_async((message,)) + return Response(status=status.HTTP_201_CREATED) diff --git a/src/olympia/amo/log.py b/src/olympia/amo/log.py index 95637934c3..ad80d3c8fb 100644 --- a/src/olympia/amo/log.py +++ b/src/olympia/amo/log.py @@ -658,6 +658,14 @@ class DEVELOPER_REPLY_VERSION(_LOG): review_queue = True +class REVIEWER_REPLY_VERSION(_LOG): + id = 141 + format = _(u'Reply by reviewer on {addon} {version}.') + short = _(u'Reviewer Reply') + keep = True + review_queue = True + + LOGS = [x for x in vars().values() if isclass(x) and issubclass(x, _LOG) and x != _LOG] # Make sure there's no duplicate IDs. diff --git a/src/olympia/amo/tasks.py b/src/olympia/amo/tasks.py index e88ac38e6a..cd99a188ac 100644 --- a/src/olympia/amo/tasks.py +++ b/src/olympia/amo/tasks.py @@ -22,12 +22,14 @@ log = commonware.log.getLogger('z.task') def send_email(recipient, subject, message, from_email=None, html_message=None, attachments=None, real_email=False, cc=None, headers=None, fail_silently=False, async=False, - max_retries=None, **kwargs): + max_retries=None, reply_to=None, **kwargs): backend = EmailMultiAlternatives if html_message else EmailMessage connection = get_email_backend(real_email) - result = backend(subject, message, - from_email, recipient, cc=cc, connection=connection, - headers=headers, attachments=attachments) + + result = backend(subject, message, from_email, to=recipient, cc=cc, + connection=connection, headers=headers, + attachments=attachments, reply_to=reply_to) + if html_message: result.attach_alternative(html_message, 'text/html') try: diff --git a/src/olympia/amo/tests/test_send_mail.py b/src/olympia/amo/tests/test_send_mail.py index a6db1d6aab..a7d70a7360 100644 --- a/src/olympia/amo/tests/test_send_mail.py +++ b/src/olympia/amo/tests/test_send_mail.py @@ -250,6 +250,14 @@ class TestSendMail(BaseTestCase): assert headers['X-Auto-Response-Suppress'] == 'RN, NRN, OOF, AutoReply' assert headers['Auto-Submitted'] == 'auto-generated' + def test_reply_to(self): + send_mail('subject', 'test body', from_email='a@example.com', + recipient_list=['b@example.com'], reply_to=['c@example.com']) + + headers = mail.outbox[0].extra_headers + assert mail.outbox[0].reply_to == ['c@example.com'] + assert headers['Auto-Submitted'] == 'auto-generated' # Still there. + def make_backend_class(self, error_order): throw_error = iter(error_order) diff --git a/src/olympia/amo/utils.py b/src/olympia/amo/utils.py index 81dfad53b6..05fa9b0b47 100644 --- a/src/olympia/amo/utils.py +++ b/src/olympia/amo/utils.py @@ -180,7 +180,7 @@ def send_mail(subject, message, from_email=None, recipient_list=None, fail_silently=False, use_blacklist=True, perm_setting=None, manage_url=None, headers=None, cc=None, real_email=False, html_message=None, attachments=None, async=False, - max_retries=None): + max_retries=None, reply_to=None): """ A wrapper around django.core.mail.EmailMessage. @@ -246,6 +246,7 @@ def send_mail(subject, message, from_email=None, recipient_list=None, 'html_message': html_message, 'max_retries': max_retries, 'real_email': real_email, + 'reply_to': reply_to, } kwargs.update(options) # Email subject *must not* contain newlines diff --git a/src/olympia/api/urls.py b/src/olympia/api/urls.py index feea428d5f..f7b8de6648 100644 --- a/src/olympia/api/urls.py +++ b/src/olympia/api/urls.py @@ -16,4 +16,5 @@ urlpatterns = patterns( url(r'^v3/internal/', include('olympia.internal_tools.urls')), url(r'^v3/', include('olympia.signing.urls')), url(r'^v3/statistics/', include('olympia.stats.api_urls')), + url(r'^v3/activity', include('olympia.activity.urls')), ) diff --git a/src/olympia/editors/helpers.py b/src/olympia/editors/helpers.py index 53a80692f5..8781a5844d 100644 --- a/src/olympia/editors/helpers.py +++ b/src/olympia/editors/helpers.py @@ -17,6 +17,7 @@ from jingo import register from olympia import amo from olympia.access import acl from olympia.access.models import GroupUser +from olympia.activity.utils import send_activity_mail from olympia.addons.helpers import new_context from olympia.addons.models import Addon from olympia.amo.helpers import absolutify, breadcrumbs, page_title @@ -457,13 +458,6 @@ PENDING_STATUSES = (amo.STATUS_BETA, amo.STATUS_DISABLED, amo.STATUS_NULL, amo.STATUS_PENDING, amo.STATUS_PUBLIC) -def send_mail(template, subject, emails, context, perm_setting=None): - template = loader.get_template(template) - amo_send_mail(subject, template.render(Context(context, autoescape=False)), - recipient_list=emails, from_email=settings.EDITORS_EMAIL, - use_blacklist=False, perm_setting=perm_setting) - - @register.function def get_position(addon): if addon.is_persona() and addon.is_pending(): @@ -701,9 +695,8 @@ class ReviewBase(object): 'details': details} amo.log(action, *args, **kwargs) - def notify_email(self, template, subject): + def notify_email(self, template, subject, perm_setting='editor_reviewed'): """Notify the authors that their addon has been reviewed.""" - emails = [a.email for a in self.addon.authors.all()] data = self.data.copy() if self.data else {} data.update(self.get_context_data()) data['tested'] = '' @@ -716,8 +709,20 @@ class ReviewBase(object): data['tested'] = 'Tested with %s' % app subject = subject % (data['name'], self.version.version if self.version else '') - send_mail('editors/emails/%s.ltxt' % template, subject, - emails, Context(data), perm_setting='editor_reviewed') + + message = loader.get_template( + 'editors/emails/%s.ltxt' % template).render( + Context(data, autoescape=False)) + if not waffle.switch_is_active('activity-email'): + emails = [a.email for a in self.addon.authors.all()] + amo_send_mail( + subject, message, recipient_list=emails, + from_email=settings.EDITORS_EMAIL, use_blacklist=False, + perm_setting=perm_setting) + else: + send_activity_mail( + subject, message, self.version, self.addon.authors.all(), + settings.EDITORS_EMAIL, perm_setting) def get_context_data(self): addon_url = self.addon.get_url_path(add_prefix=False) @@ -744,30 +749,28 @@ class ReviewBase(object): def request_information(self): """Send a request for information to the authors.""" - emails = [a.email for a in self.addon.authors.all()] self.log_action(amo.LOG.REQUEST_INFORMATION) if self.version: kw = {'has_info_request': True} if not self.addon.is_listed and not self.version.reviewed: kw['reviewed'] = datetime.datetime.now() self.version.update(**kw) - log.info(u'Sending request for information for %s to %s' % - (self.addon, emails)) - data = self.get_context_data() - subject = u'Mozilla Add-ons: %s %s' % ( - data['name'], self.version.version if self.version else '') - send_mail('editors/emails/info.ltxt', subject, - emails, Context(data), - perm_setting='individual_contact') + log.info(u'Sending request for information for %s to authors' % + self.addon) + subject = u'Mozilla Add-ons: %s %s' + self.notify_email('info', subject, perm_setting='individual_contact') def send_super_mail(self): self.log_action(amo.LOG.REQUEST_SUPER_REVIEW) log.info(u'Super review requested for %s' % (self.addon)) data = self.get_context_data() - send_mail('editors/emails/super_review.ltxt', - u'Super review requested: %s' % (data['name']), - [settings.SENIOR_EDITORS_EMAIL], - Context(data)) + message = (loader + .get_template('editors/emails/super_review.ltxt') + .render(Context(data, autoescape=False))) + amo_send_mail(u'Super review requested: %s' % (data['name']), message, + recipient_list=[settings.SENIOR_EDITORS_EMAIL], + from_email=settings.EDITORS_EMAIL, + use_blacklist=False) def process_comment(self): if self.version: diff --git a/src/olympia/editors/tests/test_helpers.py b/src/olympia/editors/tests/test_helpers.py index f778dde107..2512235545 100644 --- a/src/olympia/editors/tests/test_helpers.py +++ b/src/olympia/editors/tests/test_helpers.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from datetime import datetime, timedelta +from django.conf import settings from django.core import mail from django.core.files.storage import default_storage as storage from django.utils import translation @@ -11,6 +12,7 @@ from pyquery import PyQuery as pq from waffle.testutils import override_flag, override_switch from olympia import amo +from olympia.activity.models import ActivityLogToken from olympia.amo.tests import TestCase from olympia.addons.models import Addon from olympia.amo.urlresolvers import reverse @@ -466,9 +468,9 @@ class TestReviewHelper(TestCase): self.helper.handler.log_action(amo.LOG.APPROVE_VERSION) assert self.check_log_count(amo.LOG.APPROVE_VERSION.id) == 1 - def test_notify_email( - self, base_fragment='reply to this email or join #amo-editors'): + def test_notify_email(self): self.helper.set_data(self.get_data()) + base_fragment = 'reply to this email or join #amo-editors' for template in ['nominated_to_nominated', 'nominated_to_preliminary', 'nominated_to_public', 'nominated_to_sandbox', 'pending_to_preliminary', 'pending_to_public', @@ -483,8 +485,26 @@ class TestReviewHelper(TestCase): @override_switch('activity-email', active=True) def test_notify_email_activity_email(self): - self.test_notify_email( - base_fragment='If you need to send file attachments') + self.helper.set_data(self.get_data()) + base_fragment = 'If you need to send file attachments' + user = self.addon.listed_authors[0] + ActivityLogToken.objects.create(version=self.version, user=user) + uuid = self.version.token.get(user=user).uuid.hex + reply_email = ( + 'reviewreply+%s@%s' % (uuid, settings.INBOUND_EMAIL_DOMAIN)) + + for template in ['nominated_to_nominated', 'nominated_to_preliminary', + 'nominated_to_public', 'nominated_to_sandbox', + 'pending_to_preliminary', 'pending_to_public', + 'pending_to_sandbox', 'preliminary_to_preliminary', + 'author_super_review', 'unlisted_to_reviewed', + 'unlisted_to_reviewed_auto', + 'unlisted_to_sandbox']: + mail.outbox = [] + self.helper.handler.notify_email(template, 'Sample subject %s, %s') + assert len(mail.outbox) == 1 + assert base_fragment in mail.outbox[0].body + assert mail.outbox[0].reply_to == [reply_email] def test_email_links(self): expected = { @@ -1155,11 +1175,16 @@ def test_page_title_unicode(): def test_send_email_autoescape(): - # Make sure HTML is not auto-escaped. + mock_request = Mock() + mock_request.user = None + base = helpers.ReviewBase(mock_request, None, None, '') s = 'woo&&<>\'""' ctx = dict(name=s, review_url=s, reviewer=s, comments=s, SITE_URL=s) - helpers.send_mail('editors/emails/super_review.ltxt', - 'aww yeah', ['xx'], ctx) + base.get_context_data = Mock(name='get_context_data', return_value=ctx) + base.data = {'comments': ''} + + # Make sure HTML is not auto-escaped. + base.send_super_mail() assert len(mail.outbox) == 1 assert mail.outbox[0].body.count(s) == len(ctx) diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index 8001f27bc8..09017715f3 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -212,6 +212,15 @@ MOBILE_DOMAIN = 'm.%s' % DOMAIN # The full url of the mobile site. MOBILE_SITE_URL = 'http://%s' % MOBILE_DOMAIN +# Filter IP addresses of the allowed clients that can post email +# through the API. +ALLOWED_CLIENTS_EMAIL_API = env.list('ALLOWED_CLIENTS_EMAIL_API', default=[]) + +# Auth token required to authorize inbound email. +INBOUND_EMAIL_SECRET_KEY = env('INBOUND_EMAIL_SECRET_KEY', default='') + +INBOUND_EMAIL_DOMAIN = DOMAIN + # Absolute path to the directory that holds media. # Example: "/home/media/media.lawrence.com/" MEDIA_ROOT = path('user-media') diff --git a/src/olympia/migrations/897-activity-email-waffle.sql b/src/olympia/migrations/897-activity-email-waffle.sql new file mode 100644 index 0000000000..fe30da54d9 --- /dev/null +++ b/src/olympia/migrations/897-activity-email-waffle.sql @@ -0,0 +1,2 @@ +INSERT INTO waffle_switch (name, active, note, created, modified) +VALUES ('activity-email', 0, 'Review emails have "reviewreply+...@" reply-to headers.', NOW(), NOW()); diff --git a/src/olympia/migrations/898-log-activity-tokens-table.sql b/src/olympia/migrations/898-log-activity-tokens-table.sql new file mode 100644 index 0000000000..7a204e39cf --- /dev/null +++ b/src/olympia/migrations/898-log-activity-tokens-table.sql @@ -0,0 +1,14 @@ +CREATE TABLE `log_activity_tokens` ( + `id` int(11) unsigned AUTO_INCREMENT NOT NULL PRIMARY KEY, + `created` datetime NOT NULL, + `modified` datetime NOT NULL, + `version_id` int(11) NOT NULL, + `user_id` int(11) NOT NULL, + `uuid` char(32) NOT NULL UNIQUE, + `use_count` integer UNSIGNED NOT NULL +) DEFAULT CHARSET=utf8; + +ALTER TABLE `log_activity_tokens` ADD CONSTRAINT `log_activity_tokens_user` +FOREIGN KEY (`user_id`) REFERENCES `users` (`id`); +ALTER TABLE `log_activity_tokens` ADD CONSTRAINT `log_activity_tokens_version` +FOREIGN KEY (`version_id`) REFERENCES `versions` (`id`);