Skip to content

Commit

Permalink
Remove unused middlewares (#11722)
Browse files Browse the repository at this point in the history
- We no longer hit APIs from our main domain from docs pages,
  so there is no need to not se cookies.
- SECURE_REFERRER_POLICY is a built-in Django setting
  now https://docs.djangoproject.com/en/4.2/ref/middleware/#referrer-policy.

Closes #11692
  • Loading branch information
stsewd authored Oct 29, 2024
1 parent 727da6e commit f4aea02
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 143 deletions.
87 changes: 0 additions & 87 deletions readthedocs/core/middleware.py
Original file line number Diff line number Diff line change
@@ -1,96 +1,9 @@
import structlog
from django.conf import settings
from django.contrib.sessions.backends.base import SessionBase
from django.contrib.sessions.middleware import SessionMiddleware
from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed
from django.http import HttpResponse

log = structlog.get_logger(__name__)


class ReadTheDocsSessionMiddleware(SessionMiddleware):

"""
An overridden session middleware with a few changes.
- Doesn't create a session on logged out doc views.
"""

# Don't set a session cookie on these URLs unless the cookie is already set
IGNORE_URLS = [
"/api/v2/footer_html",
"/sustainability/view",
"/sustainability/click",
]

def process_request(self, request):
for url in self.IGNORE_URLS:
if (
request.path_info.startswith(url)
and settings.SESSION_COOKIE_NAME not in request.COOKIES
):
# Hack request.session otherwise the Authentication middleware complains.
request.session = SessionBase() # create an empty session
return

super().process_request(request)

def process_response(self, request, response):
for url in self.IGNORE_URLS:
if (
request.path_info.startswith(url)
and settings.SESSION_COOKIE_NAME not in request.COOKIES
):
return response

return super().process_response(request, response)


class ReferrerPolicyMiddleware:

"""
A middleware implementing the Referrer-Policy header.
The value of the header will be read from the SECURE_REFERRER_POLICY setting.
Important:
In Django 3.x, this feature is built-in to the SecurityMiddleware.
After upgrading to Django3, this middleware should be removed.
https://docs.djangoproject.com/en/3.1/ref/middleware/#referrer-policy
Based heavily on: https://github.com/ubernostrum/django-referrer-policy
"""

VALID_REFERRER_POLICIES = [
"no-referrer",
"no-referrer-when-downgrade",
"origin",
"origin-when-cross-origin",
"same-origin",
"strict-origin",
"strict-origin-when-cross-origin",
"unsafe-url",
]

def __init__(self, get_response):
self.get_response = get_response

if not settings.SECURE_REFERRER_POLICY:
log.warning(
"SECURE_REFERRER_POLICY not set - not setting the referrer policy"
)
raise MiddlewareNotUsed()
if settings.SECURE_REFERRER_POLICY not in self.VALID_REFERRER_POLICIES:
raise ImproperlyConfigured(
"settings.SECURE_REFERRER_POLICY has an illegal value."
)

def __call__(self, request):
response = self.get_response(request)
response["Referrer-Policy"] = settings.SECURE_REFERRER_POLICY
return response


class NullCharactersMiddleware:

"""
Expand Down
17 changes: 0 additions & 17 deletions readthedocs/rtd_tests/tests/test_footer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import pytest
from django.contrib.auth.models import User
from django.contrib.sessions.backends.base import SessionBase
from django.http import HttpResponse
from django.test import TestCase, override_settings
from django.urls import reverse
from django_dynamic_fixture import get
Expand All @@ -12,7 +10,6 @@
from readthedocs.api.v2.views.footer_views import get_version_compare_data
from readthedocs.builds.constants import BRANCH, EXTERNAL, LATEST, TAG
from readthedocs.builds.models import Version
from readthedocs.core.middleware import ReadTheDocsSessionMiddleware
from readthedocs.organizations.models import Organization
from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND, PRIVATE, PUBLIC
from readthedocs.projects.models import Project
Expand Down Expand Up @@ -143,20 +140,6 @@ def test_epub_not_mentioned_in_footer_when_doesnt_exists(self):
response = self.render()
self.assertNotIn("epub", response.data["html"])

def test_no_session_logged_out(self):
mid = ReadTheDocsSessionMiddleware(lambda request: HttpResponse())

# Null session here
request = self.factory.get("/api/v2/footer_html/")
mid.process_request(request)
self.assertIsInstance(request.session, SessionBase)
self.assertEqual(list(request.session.keys()), [])

# Proper session here
home_request = self.factory.get("/")
mid.process_request(home_request)
self.assertEqual(home_request.session.TEST_COOKIE_NAME, "testcookie")

def test_show_version_warning(self):
self.pip.show_version_warning = True
self.pip.save()
Expand Down
38 changes: 1 addition & 37 deletions readthedocs/rtd_tests/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,12 @@
ACCESS_CONTROL_ALLOW_CREDENTIALS,
ACCESS_CONTROL_ALLOW_ORIGIN,
)
from django.conf import settings
from django.http import HttpResponse
from django.test import TestCase, override_settings
from django.test.client import RequestFactory
from django_dynamic_fixture import get

from readthedocs.builds.constants import LATEST
from readthedocs.core.middleware import (
NullCharactersMiddleware,
ReadTheDocsSessionMiddleware,
)
from readthedocs.core.middleware import NullCharactersMiddleware
from readthedocs.projects.constants import PRIVATE, PUBLIC
from readthedocs.projects.models import Domain, Project, ProjectRelationship
from readthedocs.rtd_tests.utils import create_user
Expand Down Expand Up @@ -243,37 +238,6 @@ def test_apiv2_endpoint_not_allowed(self):
self.assertNotIn(ACCESS_CONTROL_ALLOW_CREDENTIALS, resp.headers)


class TestSessionMiddleware(TestCase):
def setUp(self):
self.factory = RequestFactory()
self.middleware = ReadTheDocsSessionMiddleware(lambda request: HttpResponse())

self.user = create_user(username="owner", password="test")

@override_settings(SESSION_COOKIE_SAMESITE="None")
def test_main_cookie_samesite_none(self):
request = self.factory.get("/")
response = HttpResponse()
self.middleware.process_request(request)
request.session["test"] = "value"
response = self.middleware.process_response(request, response)

self.assertEqual(
response.cookies[settings.SESSION_COOKIE_NAME]["samesite"], "None"
)

def test_main_cookie_samesite_lax(self):
request = self.factory.get("/")
response = HttpResponse()
self.middleware.process_request(request)
request.session["test"] = "value"
response = self.middleware.process_response(request, response)

self.assertEqual(
response.cookies[settings.SESSION_COOKIE_NAME]["samesite"], "Lax"
)


class TestNullCharactersMiddleware(TestCase):
def setUp(self):
self.factory = RequestFactory()
Expand Down
3 changes: 1 addition & 2 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def USE_PROMOS(self): # noqa
def MIDDLEWARE(self):
middlewares = [
"readthedocs.core.middleware.NullCharactersMiddleware",
"readthedocs.core.middleware.ReadTheDocsSessionMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
"django.middleware.locale.LocaleMiddleware",
"corsheaders.middleware.CorsMiddleware",
"django.middleware.common.CommonMiddleware",
Expand All @@ -334,7 +334,6 @@ def MIDDLEWARE(self):
"allauth.account.middleware.AccountMiddleware",
"dj_pagination.middleware.PaginationMiddleware",
"csp.middleware.CSPMiddleware",
"readthedocs.core.middleware.ReferrerPolicyMiddleware",
"simple_history.middleware.HistoryRequestMiddleware",
"readthedocs.core.logs.ReadTheDocsRequestMiddleware",
"django_structlog.middlewares.CeleryMiddleware",
Expand Down

0 comments on commit f4aea02

Please sign in to comment.