Fix blank HTML previews by loading each email from its own view, not srcdoc
The iframe's srcdoc="{{ preview.html }}" relied on Django's autoescaping
to correctly re-encode a document full of its own double-quoted
style="..." attributes, and on browser handling of an escaped, inherited-
CSP srcdoc document that didn't render reliably in practice -- the
panel showed blank. EmailPreviewRenderView now serves each preview's
HTML as an ordinary same-origin response at its own URL, and the
iframe just points `src` at it -- xframe_options_sameorigin overrides
the site-wide X-Frame-Options: DENY (SecurityMiddleware's default,
unset in settings.py) since this response only needs to be framed by
the page linking to it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
@@ -20,7 +20,7 @@
|
|||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div class="bg-subhead p-4" data-view-panel="html">
|
<div class="bg-subhead p-4" data-view-panel="html">
|
||||||
<iframe class="h-[560px] w-full rounded-lg border border-line bg-white" srcdoc="{{ preview.html }}" title="{{ preview.label }}"></iframe>
|
<iframe class="h-[560px] w-full rounded-lg border border-line bg-white" src="{% url 'management:email_preview_render' preview.key %}" loading="lazy" title="{{ preview.label }}"></iframe>
|
||||||
</div>
|
</div>
|
||||||
<pre class="hidden overflow-x-auto p-4.5 font-mono text-sm whitespace-pre-wrap text-ink" data-view-panel="text">{{ preview.text }}</pre>
|
<pre class="hidden overflow-x-auto p-4.5 font-mono text-sm whitespace-pre-wrap text-ink" data-view-panel="text">{{ preview.text }}</pre>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -4779,15 +4779,11 @@ class EmailPreviewListViewTests(ManagementTestBase):
|
|||||||
for preview in EMAIL_PREVIEWS:
|
for preview in EMAIL_PREVIEWS:
|
||||||
self.assertContains(response, preview.label)
|
self.assertContains(response, preview.label)
|
||||||
|
|
||||||
def test_the_html_render_uses_the_clubs_own_branding(self):
|
def test_each_card_links_to_its_own_render_view(self):
|
||||||
self.club.name = "Ajax United"
|
|
||||||
self.club.secondary_color = "#123456"
|
|
||||||
self.club.save(update_fields=["name", "secondary_color"])
|
|
||||||
|
|
||||||
response = self.club_get("email_preview_list")
|
response = self.club_get("email_preview_list")
|
||||||
|
|
||||||
self.assertContains(response, "Ajax United")
|
for preview in EMAIL_PREVIEWS:
|
||||||
self.assertContains(response, "#123456")
|
self.assertContains(response, reverse("management:email_preview_render", args=[preview.key]))
|
||||||
|
|
||||||
def test_the_subject_and_plain_text_body_render(self):
|
def test_the_subject_and_plain_text_body_render(self):
|
||||||
response = self.club_get("email_preview_list")
|
response = self.club_get("email_preview_list")
|
||||||
@@ -4821,6 +4817,58 @@ class EmailPreviewListViewTests(ManagementTestBase):
|
|||||||
self.assertContains(admin_response, reverse("management:email_preview_list"))
|
self.assertContains(admin_response, reverse("management:email_preview_list"))
|
||||||
|
|
||||||
|
|
||||||
|
class EmailPreviewRenderViewTests(ManagementTestBase):
|
||||||
|
"""The actual per-email HTML document each card's iframe loads via `src` --
|
||||||
|
see EmailPreviewListView's own docstring for why this is a same-origin
|
||||||
|
sub-request rather than a srcdoc="..." attribute."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.client.force_login(self.admin_user)
|
||||||
|
|
||||||
|
def test_renders_the_full_html_document(self):
|
||||||
|
response = self.client.get(reverse("management:email_preview_render", args=["dues_invoice"]), HTTP_HOST="ajax-united.rosterchief.app")
|
||||||
|
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
|
self.assertEqual(response["Content-Type"], "text/html; charset=utf-8")
|
||||||
|
self.assertContains(response, "<!DOCTYPE html>")
|
||||||
|
self.assertContains(response, "DUE-2026-00042")
|
||||||
|
|
||||||
|
def test_uses_the_clubs_own_branding(self):
|
||||||
|
self.club.name = "Ajax United"
|
||||||
|
self.club.secondary_color = "#123456"
|
||||||
|
self.club.save(update_fields=["name", "secondary_color"])
|
||||||
|
|
||||||
|
response = self.client.get(reverse("management:email_preview_render", args=["dues_invoice"]), HTTP_HOST="ajax-united.rosterchief.app")
|
||||||
|
|
||||||
|
self.assertContains(response, "Ajax United")
|
||||||
|
self.assertContains(response, "#123456")
|
||||||
|
|
||||||
|
def test_allows_same_origin_framing(self):
|
||||||
|
# The site-wide default (SecurityMiddleware, no X_FRAME_OPTIONS override
|
||||||
|
# in settings.py) is DENY -- this response overrides that specifically,
|
||||||
|
# since EmailPreviewListView's own iframe needs to load it.
|
||||||
|
response = self.client.get(reverse("management:email_preview_render", args=["dues_invoice"]), HTTP_HOST="ajax-united.rosterchief.app")
|
||||||
|
|
||||||
|
self.assertEqual(response["X-Frame-Options"], "SAMEORIGIN")
|
||||||
|
|
||||||
|
def test_an_unknown_key_404s(self):
|
||||||
|
response = self.client.get(reverse("management:email_preview_render", args=["not-a-real-key"]), HTTP_HOST="ajax-united.rosterchief.app")
|
||||||
|
|
||||||
|
self.assertEqual(response.status_code, 404)
|
||||||
|
|
||||||
|
def test_non_admin_gets_403(self):
|
||||||
|
coach_user = User.objects.create_user(email="coach-email-render@example.com", password="pw-secret-123")
|
||||||
|
coach_member = Member.objects.create(user=coach_user, first_name="Cara", last_name="Coach")
|
||||||
|
team = Team.objects.create(club=self.club, name="U19", short_name="U19")
|
||||||
|
position = Position.objects.create(club=self.club, name="Coach15", short_name="C15", staff_position=True)
|
||||||
|
StaffAssignment.objects.create(team=team, member=coach_member, season=self.season, position=position)
|
||||||
|
self.client.force_login(coach_user)
|
||||||
|
|
||||||
|
response = self.client.get(reverse("management:email_preview_render", args=["dues_invoice"]), HTTP_HOST="ajax-united.rosterchief.app")
|
||||||
|
|
||||||
|
self.assertEqual(response.status_code, 403)
|
||||||
|
|
||||||
|
|
||||||
class BillingEndingBannerTests(ManagementTestBase):
|
class BillingEndingBannerTests(ManagementTestBase):
|
||||||
"""The club dashboard's "billing is about to stop" warning -- see
|
"""The club dashboard's "billing is about to stop" warning -- see
|
||||||
management.views.HomeView and management/templates/management/home.html.
|
management.views.HomeView and management/templates/management/home.html.
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ urlpatterns = [
|
|||||||
# Settings (admin only)
|
# Settings (admin only)
|
||||||
path("settings/", views.ClubSettingsView.as_view(), name="club_settings"),
|
path("settings/", views.ClubSettingsView.as_view(), name="club_settings"),
|
||||||
path("settings/email-previews/", views.EmailPreviewListView.as_view(), name="email_preview_list"),
|
path("settings/email-previews/", views.EmailPreviewListView.as_view(), name="email_preview_list"),
|
||||||
|
path("settings/email-previews/<str:key>/render/", views.EmailPreviewRenderView.as_view(), name="email_preview_render"),
|
||||||
path("settings/onboarding-requirements/", views.OnboardingRequirementListView.as_view(), name="onboarding_requirement_list"),
|
path("settings/onboarding-requirements/", views.OnboardingRequirementListView.as_view(), name="onboarding_requirement_list"),
|
||||||
path("settings/onboarding-requirements/new/", views.OnboardingRequirementCreateView.as_view(), name="onboarding_requirement_create"),
|
path("settings/onboarding-requirements/new/", views.OnboardingRequirementCreateView.as_view(), name="onboarding_requirement_create"),
|
||||||
path("settings/onboarding-requirements/<uuid:pk>/edit/", views.OnboardingRequirementUpdateView.as_view(), name="onboarding_requirement_update"),
|
path("settings/onboarding-requirements/<uuid:pk>/edit/", views.OnboardingRequirementUpdateView.as_view(), name="onboarding_requirement_update"),
|
||||||
|
|||||||
@@ -7,9 +7,11 @@ from django.http import FileResponse, Http404, HttpResponse, JsonResponse
|
|||||||
from django.shortcuts import get_object_or_404, redirect, render
|
from django.shortcuts import get_object_or_404, redirect, render
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
from django.utils import timezone
|
from django.utils import timezone
|
||||||
|
from django.utils.decorators import method_decorator
|
||||||
from django.utils.http import url_has_allowed_host_and_scheme
|
from django.utils.http import url_has_allowed_host_and_scheme
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
from django.utils.translation import ngettext
|
from django.utils.translation import ngettext
|
||||||
|
from django.views.decorators.clickjacking import xframe_options_sameorigin
|
||||||
from django.views.generic import CreateView, DetailView, FormView, ListView, TemplateView, UpdateView, View
|
from django.views.generic import CreateView, DetailView, FormView, ListView, TemplateView, UpdateView, View
|
||||||
|
|
||||||
from billing.models import Due
|
from billing.models import Due
|
||||||
@@ -3326,16 +3328,47 @@ class EmailPreviewListView(ClubAdminRequiredMixin, TemplateView):
|
|||||||
this app can send, rendered against sample data for this club so an admin
|
this app can send, rendered against sample data for this club so an admin
|
||||||
can see exactly what a member/parent would receive without sending
|
can see exactly what a member/parent would receive without sending
|
||||||
anything. Same access level as Club identity: this is a branding/comms
|
anything. Same access level as Club identity: this is a branding/comms
|
||||||
concern, not day-to-day people/roster work."""
|
concern, not day-to-day people/roster work.
|
||||||
|
|
||||||
|
The HTML render itself is NOT embedded directly here -- see
|
||||||
|
EmailPreviewRenderView, which each card's iframe points its `src` at.
|
||||||
|
An email's markup is full of its own double-quoted style="..." attributes;
|
||||||
|
dropping that whole document into a srcdoc="..." attribute here relies on
|
||||||
|
Django's autoescaping to get every one of those quotes right, and depends
|
||||||
|
on browser-specific handling of an escaped, inherited-CSP srcdoc document
|
||||||
|
that turned out not to render reliably. A same-origin sub-request sidesteps
|
||||||
|
all of that -- ordinary HTML delivered as an ordinary response."""
|
||||||
|
|
||||||
template_name = "management/email_previews.html"
|
template_name = "management/email_previews.html"
|
||||||
|
|
||||||
def get_context_data(self, **kwargs):
|
def get_context_data(self, **kwargs):
|
||||||
club = self.request.club
|
club = self.request.club
|
||||||
previews = [{"key": preview.key, "label": preview.label, "description": preview.description, **render_preview(preview, club=club, request=self.request)} for preview in EMAIL_PREVIEWS]
|
previews = []
|
||||||
|
for preview in EMAIL_PREVIEWS:
|
||||||
|
rendered = render_preview(preview, club=club, request=self.request)
|
||||||
|
previews.append({"key": preview.key, "label": preview.label, "description": preview.description, "subject": rendered["subject"], "text": rendered["text"]})
|
||||||
return super().get_context_data(previews=previews, **kwargs)
|
return super().get_context_data(previews=previews, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
@method_decorator(xframe_options_sameorigin, name="get")
|
||||||
|
class EmailPreviewRenderView(ClubAdminRequiredMixin, View):
|
||||||
|
"""The actual HTML document for one preview, served at its own URL so
|
||||||
|
EmailPreviewListView's iframe can `src` it directly rather than smuggling
|
||||||
|
it through a srcdoc="..." attribute -- see that view's docstring for why.
|
||||||
|
xframe_options_sameorigin overrides the site-wide X-Frame-Options: DENY
|
||||||
|
(settings.py has no X_FRAME_OPTIONS override, so SecurityMiddleware's
|
||||||
|
default applies everywhere else): this response only ever needs to be
|
||||||
|
framed by the page that links to it, on the same origin."""
|
||||||
|
|
||||||
|
def get(self, request, key):
|
||||||
|
preview = next((preview for preview in EMAIL_PREVIEWS if preview.key == key), None)
|
||||||
|
if preview is None:
|
||||||
|
raise Http404
|
||||||
|
|
||||||
|
rendered = render_preview(preview, club=request.club, request=request)
|
||||||
|
return HttpResponse(rendered["html"], content_type="text/html; charset=utf-8")
|
||||||
|
|
||||||
|
|
||||||
class OnboardingRequirementListView(MemberAdminRequiredMixin, ListView):
|
class OnboardingRequirementListView(MemberAdminRequiredMixin, ListView):
|
||||||
"""What a club requires from every member after they sign up or renew (a
|
"""What a club requires from every member after they sign up or renew (a
|
||||||
photo, a medical certificate, ...) -- see club/models.py's OnboardingRequirement
|
photo, a medical certificate, ...) -- see club/models.py's OnboardingRequirement
|
||||||
|
|||||||
Reference in New Issue
Block a user