Add configurable email sending for Klaxon add-on

config/settings/base.py

@@ -647,6 +647,14 @@
 ADDON_DASH_FAIL_LIMIT = env.int("ADDON_DASH_FAIL_LIMIT", default=5)
 ADDON_DASH_DAYS = env.list("ADDON_DASH_DAYS", default=[30, 7])
 
+# Klaxon
+KLAXON_ADDON_ID = env.int("KLAXON_ADDON_ID", default=0)
+# permission granted to an add-on's token -> the From address it may use
+ADDON_MAIL_FROM = {
+    "klaxon": env("KLAXON_FROM_EMAIL", default="Klaxon <no-reply@klaxoncloud.org>"),
+}
+
+
 # django-parler
 # ------------------------------------------------------------------------------
 INSTALLED_APPS += ["parler"]

documentcloud/addons/models.py

@@ -126,12 +126,23 @@ class AddOn(models.Model):
     def __str__(self):
         return self.name if self.name else "- Nameless Add-On -"
 
+    @property
+    def token_permissions(self):
+        """Extra permissions to embed in this add-on's JWT"""
+        if settings.KLAXON_ADDON_ID and self.pk == settings.KLAXON_ADDON_ID:
+            return ["klaxon"]
+        return []
+
     def get_tokens(self, user):
         """Get a JWT refresh token an access token from squarelet for the
         add-on to be able to authenticate itself to the DocumentCloud API
         """
+        params = {}
+        if self.token_permissions:
+            params["permissions"] = " ".join(self.token_permissions)
+
         try:
-            resp = squarelet_get(f"/api/refresh_tokens/{user.uuid}/")
+            resp = squarelet_get(f"/api/refresh_tokens/{user.uuid}/", params=params)
             resp.raise_for_status()
         except requests.exceptions.RequestException as exc:
             logger.warning(

documentcloud/addons/tests/test_models.py

@@ -0,0 +1,56 @@
+# Django
+from django.test.utils import override_settings
+
+# Standard Library
+from unittest.mock import patch
+
+# Third Party
+import pytest
+
+# DocumentCloud
+from documentcloud.addons.tests.factories import AddOnFactory
+
+TOKENS = {"access_token": "access", "refresh_token": "refresh"}
+
+
+@pytest.mark.django_db()
+class TestAddOnTokens:
+    """Klaxon's token carries a permission, so that the API can recognize its
+    email as coming from Klaxon - the token is all the API ever sees.
+    """
+
+    def test_token_permissions_klaxon(self):
+        addon = AddOnFactory()
+        with override_settings(KLAXON_ADDON_ID=addon.pk):
+            assert addon.token_permissions == ["klaxon"]
+
+    def test_token_permissions_other_addon(self):
+        addon, klaxon = AddOnFactory.create_batch(2)
+        with override_settings(KLAXON_ADDON_ID=klaxon.pk):
+            assert addon.token_permissions == []
+
+    @override_settings(KLAXON_ADDON_ID=0)
+    def test_token_permissions_unconfigured(self):
+        """An unset Klaxon ID must never match an add-on"""
+        addon = AddOnFactory()
+        assert addon.token_permissions == []
+
+    @override_settings(KLAXON_ADDON_ID=0)
+    def test_get_tokens(self, user):
+        """An add-on with no permissions asks Squarelet for a plain token"""
+        addon = AddOnFactory()
+        with patch("documentcloud.addons.models.squarelet_get") as mock_get:
+            mock_get.return_value.json.return_value = TOKENS
+            assert addon.get_tokens(user) == TOKENS
+        mock_get.assert_called_once_with(f"/api/refresh_tokens/{user.uuid}/", params={})
+
+    def test_get_tokens_klaxon(self, user):
+        """Klaxon's token is requested with the klaxon permission"""
+        addon = AddOnFactory()
+        with override_settings(KLAXON_ADDON_ID=addon.pk):
+            with patch("documentcloud.addons.models.squarelet_get") as mock_get:
+                mock_get.return_value.json.return_value = TOKENS
+                assert addon.get_tokens(user) == TOKENS
+        mock_get.assert_called_once_with(
+            f"/api/refresh_tokens/{user.uuid}/", params={"permissions": "klaxon"}
+        )

documentcloud/core/mail.py

@@ -5,6 +5,7 @@
 
 # Standard Library
 import logging
+from email.utils import parseaddr
 
 # Third Party
 from html2text import html2text
@@ -21,7 +22,14 @@ def __init__(self, **kwargs):
         user = kwargs.pop("user", None)
         extra_context = kwargs.pop("extra_context", {})
         template = kwargs.pop("template", self.template)
-        super().__init__(**kwargs)
+        from_email = kwargs.pop("from_email", None)
+        super().__init__(from_email=from_email or settings.DEFAULT_FROM_EMAIL, **kwargs)
+
+        # MAILGUN_SENDER_DOMAIN pins the sending domain globally, ignoring the
+        # From address - override it only when sending as somebody else
+        if from_email:
+            self.envelope_sender = from_email
+
         # set up who we are sending the email to
         if user:
             self.to.append(user.email)
@@ -35,10 +43,15 @@ def __init__(self, **kwargs):
         # always BCC diagnostics
         self.bcc.append("diagnostics@muckrock.com")
 
+        # brand the email to match whoever it is being sent as, so an Add-On
+        # sending under its own address does not look like it came from us
+        from_name, contact_email = parseaddr(self.from_email)
         context = {
             "base_url": settings.DOCCLOUD_URL,
             "subject": self.subject,
             "user": user,
+            "from_name": from_name or contact_email,
+            "contact_email": contact_email,
         }
         context.update(extra_context)
         html = render_to_string(template, context)

documentcloud/core/tests.py

@@ -2,6 +2,7 @@
 from django.conf import settings
 from django.contrib.flatpages.models import FlatPage
 from django.contrib.sites.models import Site
+from django.core import mail as django_mail
 from django.db import transaction
 from django.test import TestCase
 from django.urls import reverse
@@ -12,6 +13,7 @@
 import hmac
 import time
 import uuid
+from email.utils import parseaddr
 from unittest import mock
 
 # Third Party
@@ -21,6 +23,7 @@
 
 # DocumentCloud
 from documentcloud.core.authentication import SquareletJWTAuthentication
+from documentcloud.core.mail import send_mail
 from documentcloud.users.tests.factories import UserFactory
 
 
@@ -186,3 +189,54 @@ def test_disable_create_skips_provisioning(self, mock_get, mock_update):
 
         mock_get.assert_not_called()
         mock_update.assert_not_called()
+
+
+@pytest.mark.django_db()
+class TestEmailSenderDomain:
+    """Which domain Mailgun sends over
+
+    Anymail only intuits the sending domain from the From address when
+    MAILGUN_SENDER_DOMAIN is unset, and production sets it.  So mail sent under
+    an Add-On's own address has to override it per-message, via the envelope
+    sender, or it goes out over our domain and fails DMARC alignment.
+    """
+
+    def domain(self, mail):
+        """The domain Anymail would route this message over"""
+        return parseaddr(mail.envelope_sender)[1].rpartition("@")[2]
+
+    def test_default_sender_keeps_configured_domain(self, user):
+        """Our own mail sets no envelope sender, so the global setting stands"""
+        send_mail(subject="Hello", user=user, template="core/email/base.html")
+        mail = django_mail.outbox[0]
+        assert not hasattr(mail, "envelope_sender")
+
+    def test_addon_sender_overrides_domain(self, user):
+        """An Add-On's mail is routed over its own domain"""
+        send_mail(
+            subject="Site changed",
+            user=user,
+            template="core/email/base.html",
+            from_email="Klaxon <no-reply@klaxoncloud.org>",
+        )
+        mail = django_mail.outbox[0]
+        assert mail.from_email == "Klaxon <no-reply@klaxoncloud.org>"
+        assert self.domain(mail) == "klaxoncloud.org"
+
+    def test_addon_sender_domain_is_routable(self, user):
+        """Mailgun 200s with a junk body if the domain contains a slash, and
+        Anymail guards against it - the address must not smuggle one through
+        """
+        send_mail(
+            subject="Site changed",
+            user=user,
+            template="core/email/base.html",
+            from_email=settings.ADDON_MAIL_FROM["klaxon"],
+        )
+        domain = self.domain(django_mail.outbox[0])
+        assert domain
+        assert "/" not in domain
+        assert (
+            domain
+            == parseaddr(settings.ADDON_MAIL_FROM["klaxon"])[1].rpartition("@")[2]
+        )

documentcloud/templates/core/email/base.html

@@ -218,7 +218,7 @@
                         </a>
                         -->
                         <div class="header--text">
-                          <h1>DocumentCloud</h1>
+                          <h1>{{ from_name }}</h1>
                           <p>{{ subject }}</p>
                         </div>
                       </div>
@@ -234,8 +234,8 @@ <h1>DocumentCloud</h1>
                           <a href="mailto:{{ user.email }}">{{ user.email }}</a>.
                         </p>
                       {% endif %}
-                      {% blocktrans %}
-                        <p>Add <a href="mailto:info@documentcloud.org">info@documentcloud.org</a> to your address book to prevent our emails from being marked as spam.</p>
+                      {% blocktrans with email=contact_email %}
+                        <p>Add <a href="mailto:{{ email }}">{{ email }}</a> to your address book to prevent our emails from being marked as spam.</p>
                       {% endblocktrans %}
                       <p>MuckRock, 263 Huntington Ave, Boston MA 02115</p>
                     </div>

documentcloud/users/tests/test_views.py

@@ -1,11 +1,13 @@
 # Django
+from django.conf import settings
 from django.db import connection, reset_queries
 from django.test.utils import override_settings
 from rest_framework import status
 
 # Standard Library
 import json
 import uuid
+from email.utils import parseaddr
 from unittest.mock import MagicMock
 
 # Third Party
@@ -190,3 +192,91 @@ def test_retrieve_another_user_email(self, client):
         assert response.status_code == status.HTTP_200_OK
         response_json = json.loads(response.content)
         assert "email" not in response_json
+
+
+# Add-Ons which are allowed their own sender, keyed by the permission their
+# token carries.  Overridden in tests so they do not depend on the deployed
+# addresses.
+ADDON_MAIL_FROM = {"klaxon": "Klaxon <klaxon@example.com>"}
+
+
+@pytest.mark.django_db()
+class TestMessageAPI:
+    """Add-Ons email their own user through this endpoint
+
+    The API cannot tell which add-on is calling - it only ever sees the user's
+    token - so an add-on which may send under its own name is identified by a
+    permission embedded in that token when it is issued.
+    """
+
+    def send(self, client, **kwargs):
+        return client.post(
+            "/api/messages/",
+            {"subject": "Site changed", "content": "Something happened"},
+            **kwargs,
+        )
+
+    def test_send(self, client, user, mailoutbox):
+        """You may email yourself"""
+        client.force_authenticate(user=user)
+        response = self.send(client)
+        assert response.status_code == status.HTTP_200_OK
+        assert len(mailoutbox) == 1
+        mail = mailoutbox[0]
+        assert mail.subject == "Site changed"
+        assert mail.to == [user.email]
+        assert "Something happened" in mail.body
+
+    def test_send_anonymous(self, client, mailoutbox):
+        """You must be logged in to send a message"""
+        response = self.send(client)
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert not mailoutbox
+
+    def test_send_invalid(self, client, user, mailoutbox):
+        """Both subject and content are required"""
+        client.force_authenticate(user=user)
+        response = client.post("/api/messages/", {"subject": "Site changed"})
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert not mailoutbox
+
+    def test_send_default_sender(self, client, user, mailoutbox):
+        """Without a mail permission, the message comes from us"""
+        client.force_authenticate(user=user)
+        self.send(client)
+        mail = mailoutbox[0]
+        assert mail.from_email == settings.DEFAULT_FROM_EMAIL
+        # the body is branded to match the sender
+        from_name, contact_email = parseaddr(settings.DEFAULT_FROM_EMAIL)
+        assert from_name in mail.body
+        assert contact_email in mail.body
+
+    @override_settings(ADDON_MAIL_FROM=ADDON_MAIL_FROM)
+    def test_send_addon_sender(self, client, user, mailoutbox):
+        """A token with a mail permission sends under that add-on's name"""
+        client.force_authenticate(user=user, token={"permissions": ["klaxon"]})
+        self.send(client)
+        mail = mailoutbox[0]
+        assert mail.from_email == "Klaxon <klaxon@example.com>"
+        # and Mailgun is told to route it over that address's domain
+        assert mail.envelope_sender == "Klaxon <klaxon@example.com>"
+        # the body is branded to match the sender, not to us
+        assert "Klaxon" in mail.body
+        assert "klaxon@example.com" in mail.body
+        assert parseaddr(settings.DEFAULT_FROM_EMAIL)[1] not in mail.body
+        # the recipient is still the token's own user
+        assert mail.to == [user.email]
+
+    @override_settings(ADDON_MAIL_FROM=ADDON_MAIL_FROM)
+    def test_send_unrelated_permission(self, client, user, mailoutbox):
+        """A permission with no sender configured uses the default"""
+        client.force_authenticate(user=user, token={"permissions": ["processing"]})
+        self.send(client)
+        assert mailoutbox[0].from_email == settings.DEFAULT_FROM_EMAIL
+
+    @override_settings(ADDON_MAIL_FROM=ADDON_MAIL_FROM)
+    def test_send_no_permissions_claim(self, client, user, mailoutbox):
+        """A token without a permissions claim at all uses the default"""
+        client.force_authenticate(user=user, token={})
+        self.send(client)
+        assert mailoutbox[0].from_email == settings.DEFAULT_FROM_EMAIL

documentcloud/users/views.py

@@ -1,4 +1,5 @@
 # Django
+from django.conf import settings
 from rest_framework import mixins, permissions, serializers, viewsets
 from rest_framework.decorators import action
 from rest_framework.permissions import IsAuthenticated
@@ -198,10 +199,21 @@ def post(self, request, format=None):
         # pylint: disable=redefined-builtin, unused-argument
         serializer = MessageSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
+        _permission, from_email = self._sender(request)
         send_mail(
             subject=serializer.validated_data["subject"],
             user=request.user,
             template="core/email/base.html",
+            from_email=from_email,
             extra_context={"content": serializer.validated_data["content"]},
         )
         return Response(serializer.data)
+
+    def _sender(self, request):
+        """Add-ons whose token carries a mail permission send from their own address"""
+        auth = getattr(request, "auth", None)
+        permissions_ = auth.get("permissions", []) if auth is not None else []
+        for permission in permissions_:
+            if permission in settings.ADDON_MAIL_FROM:
+                return permission, settings.ADDON_MAIL_FROM[permission]
+        return None, None