Skip to content

Commit

Permalink
Drop unnecessary CSP directives for gold view (#11605)
Browse files Browse the repository at this point in the history
* Drop unnecessary CSP directives for gold view

This does not seem needed, as there is no inline script src in
`subscription_detail.html`. It seems like maybe we wrote this for
`subscription_form.html`, which was old.

This conditional was breaking the view for me locally, as we don't have
any CSP directives for `script-src` locally, we only have these in
production. Because of this, there are no other `script-src` exceptions.

* Revert test functionality
  • Loading branch information
agjohnson authored Sep 18, 2024
1 parent c5004d3 commit 3179613
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 22 deletions.
22 changes: 12 additions & 10 deletions readthedocs/gold/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re

from django.contrib.auth.models import User
from django.test import TestCase, override_settings
from django.test import TestCase
from django.urls import reverse
from django_dynamic_fixture import get

Expand All @@ -11,17 +11,19 @@ def setUp(self):
self.user = get(User)

def test_csp_headers(self):
"""
Test CSP headers aren't altered.
This view originally altered the CSP directives based on whether we were
using the new dashboard. We weren't using inline scripts in this view
however, so this was reverted. The tests remain for now, but aren't
super useful and will break when we change `script-src` in base settings.
"""
self.client.force_login(self.user)
csp_header = "Content-Security-Policy"
script_src_regex = re.compile(r".*\s+script-src [^;]*'unsafe-inline'")
url = reverse("gold_detail")

with override_settings(RTD_EXT_THEME_ENABLED=False):
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertIsNone(script_src_regex.match(resp[csp_header]))

with override_settings(RTD_EXT_THEME_ENABLED=True):
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertTrue(script_src_regex.match(resp[csp_header]))
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertIsNone(script_src_regex.match(resp[csp_header]))
12 changes: 0 additions & 12 deletions readthedocs/gold/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,6 @@ class GoldSubscription(
form_class = GoldSubscriptionForm
template_name = "gold/subscription_detail.html"

def dispatch(self, request, *args, **kwargs):
response = super().dispatch(request, *args, **kwargs)
# Allow inline scripts for the gold view.
# We are using inline javascript to initialize Stripe Checkout.
# Allowing inline scripts defeats the purpose of using CSP,
# but we are limiting it to this view.
# TODO: use the `@csp_update` decorator once we are running
# ext-theme by default.
if settings.RTD_EXT_THEME_ENABLED:
response._csp_update = {"script-src": "'unsafe-inline'"}
return response

def get(self, *args, **kwargs):
subscribed = self.request.GET.get("subscribed", None)
if subscribed == "true":
Expand Down

0 comments on commit 3179613

Please sign in to comment.