From 9dd047553e8c1fc35327389b6ff855aa152885d0 Mon Sep 17 00:00:00 2001 From: Chris Amico Date: Wed, 5 Aug 2026 12:52:23 -0400 Subject: [PATCH 1/2] Add configurable email sending for Klaxon add-on --- config/settings/base.py | 8 ++ documentcloud/addons/models.py | 13 ++- documentcloud/addons/tests/test_models.py | 56 +++++++++++++ documentcloud/core/mail.py | 9 +- documentcloud/templates/core/email/base.html | 6 +- documentcloud/users/tests/test_views.py | 88 ++++++++++++++++++++ documentcloud/users/views.py | 12 +++ 7 files changed, 187 insertions(+), 5 deletions(-) create mode 100644 documentcloud/addons/tests/test_models.py diff --git a/config/settings/base.py b/config/settings/base.py index 970acd14..214aaac2 100644 --- a/config/settings/base.py +++ b/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 "), +} + + # django-parler # ------------------------------------------------------------------------------ INSTALLED_APPS += ["parler"] diff --git a/documentcloud/addons/models.py b/documentcloud/addons/models.py index 8f67f312..02db0028 100644 --- a/documentcloud/addons/models.py +++ b/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( diff --git a/documentcloud/addons/tests/test_models.py b/documentcloud/addons/tests/test_models.py new file mode 100644 index 00000000..c106e5bb --- /dev/null +++ b/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"} + ) diff --git a/documentcloud/core/mail.py b/documentcloud/core/mail.py index ef06b2e6..1686a8fb 100644 --- a/documentcloud/core/mail.py +++ b/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,8 @@ 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) or settings.DEFAULT_FROM_EMAIL + super().__init__(from_email=from_email, **kwargs) # set up who we are sending the email to if user: self.to.append(user.email) @@ -35,10 +37,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(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) diff --git a/documentcloud/templates/core/email/base.html b/documentcloud/templates/core/email/base.html index b93be9e7..14bea038 100644 --- a/documentcloud/templates/core/email/base.html +++ b/documentcloud/templates/core/email/base.html @@ -218,7 +218,7 @@ -->
-

DocumentCloud

+

{{ from_name }}

{{ subject }}

@@ -234,8 +234,8 @@

DocumentCloud

{{ user.email }}.

{% endif %} - {% blocktrans %} -

Add info@documentcloud.org to your address book to prevent our emails from being marked as spam.

+ {% blocktrans with email=contact_email %} +

Add {{ email }} to your address book to prevent our emails from being marked as spam.

{% endblocktrans %}

MuckRock, 263 Huntington Ave, Boston MA 02115

diff --git a/documentcloud/users/tests/test_views.py b/documentcloud/users/tests/test_views.py index 35ba0550..8cc3b1bf 100644 --- a/documentcloud/users/tests/test_views.py +++ b/documentcloud/users/tests/test_views.py @@ -1,4 +1,5 @@ # 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 @@ -6,6 +7,7 @@ # Standard Library import json import uuid +from email.utils import parseaddr from unittest.mock import MagicMock # Third Party @@ -190,3 +192,89 @@ 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 "} + + +@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 " + # 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 diff --git a/documentcloud/users/views.py b/documentcloud/users/views.py index b0848d5c..a4af25c5 100644 --- a/documentcloud/users/views.py +++ b/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 From 858c2da4254fac20ed8c9e27ad79c60bc97fb168 Mon Sep 17 00:00:00 2001 From: Chris Amico Date: Thu, 6 Aug 2026 10:30:55 -0400 Subject: [PATCH 2/2] Override sender domain for Klaxon --- documentcloud/core/mail.py | 12 ++++-- documentcloud/core/tests.py | 54 +++++++++++++++++++++++++ documentcloud/users/tests/test_views.py | 2 + 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/documentcloud/core/mail.py b/documentcloud/core/mail.py index 1686a8fb..d8942178 100644 --- a/documentcloud/core/mail.py +++ b/documentcloud/core/mail.py @@ -22,8 +22,14 @@ def __init__(self, **kwargs): user = kwargs.pop("user", None) extra_context = kwargs.pop("extra_context", {}) template = kwargs.pop("template", self.template) - from_email = kwargs.pop("from_email", None) or settings.DEFAULT_FROM_EMAIL - super().__init__(from_email=from_email, **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) @@ -39,7 +45,7 @@ def __init__(self, **kwargs): # 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(from_email) + from_name, contact_email = parseaddr(self.from_email) context = { "base_url": settings.DOCCLOUD_URL, "subject": self.subject, diff --git a/documentcloud/core/tests.py b/documentcloud/core/tests.py index e6379117..0db5cb57 100644 --- a/documentcloud/core/tests.py +++ b/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 ", + ) + mail = django_mail.outbox[0] + assert mail.from_email == "Klaxon " + 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] + ) diff --git a/documentcloud/users/tests/test_views.py b/documentcloud/users/tests/test_views.py index 8cc3b1bf..3ae9f1bf 100644 --- a/documentcloud/users/tests/test_views.py +++ b/documentcloud/users/tests/test_views.py @@ -258,6 +258,8 @@ def test_send_addon_sender(self, client, user, mailoutbox): self.send(client) mail = mailoutbox[0] assert mail.from_email == "Klaxon " + # and Mailgun is told to route it over that address's domain + assert mail.envelope_sender == "Klaxon " # the body is branded to match the sender, not to us assert "Klaxon" in mail.body assert "klaxon@example.com" in mail.body