Skip to content

Commit

Permalink
Use literal None in session cookie samesite setting (#11424)
Browse files Browse the repository at this point in the history
  • Loading branch information
stsewd authored Jun 19, 2024
1 parent 8b6fc60 commit 299ba15
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 9 deletions.
11 changes: 6 additions & 5 deletions readthedocs/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ def process_request(self, request):
request.session = SessionBase() # create an empty session
return

if settings.SESSION_COOKIE_SAMESITE:
super().process_request(request)
else:
if settings.SESSION_COOKIE_SAMESITE == "None":
if settings.SESSION_COOKIE_NAME in request.COOKIES:
session_key = request.COOKIES.get(settings.SESSION_COOKIE_NAME)
else:
session_key = request.COOKIES.get(self.cookie_name_fallback)

request.session = self.SessionStore(session_key)
else:
super().process_request(request)

def process_response(self, request, response):
for url in self.IGNORE_URLS:
Expand Down Expand Up @@ -139,7 +139,7 @@ def process_response(self, request, response):
)

# NOTE: This was added to support the fallback cookie
if not settings.SESSION_COOKIE_SAMESITE:
if settings.SESSION_COOKIE_SAMESITE == "None":
# Forcibly set the session cookie to SameSite=None
# This isn't supported in Django<3.1
# https://github.com/django/django/pull/11894
Expand All @@ -157,7 +157,8 @@ def process_response(self, request, response):
path=settings.SESSION_COOKIE_PATH,
secure=settings.SESSION_COOKIE_SECURE or None,
httponly=settings.SESSION_COOKIE_HTTPONLY or None,
samesite=settings.SESSION_COOKIE_SAMESITE,
# Use browser default in case SameSite=None is rejected.
samesite=False,
)
return response

Expand Down
4 changes: 2 additions & 2 deletions readthedocs/rtd_tests/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def setUp(self):

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

@override_settings(SESSION_COOKIE_SAMESITE=None)
@override_settings(SESSION_COOKIE_SAMESITE="None")
def test_fallback_cookie(self):
request = self.factory.get("/")
response = HttpResponse()
Expand All @@ -261,7 +261,7 @@ def test_fallback_cookie(self):
self.assertTrue(settings.SESSION_COOKIE_NAME in response.cookies)
self.assertTrue(self.middleware.cookie_name_fallback in response.cookies)

@override_settings(SESSION_COOKIE_SAMESITE=None)
@override_settings(SESSION_COOKIE_SAMESITE="None")
def test_main_cookie_samesite_none(self):
request = self.factory.get("/")
response = HttpResponse()
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def SESSION_COOKIE_SAMESITE(self):
Cookie used in cross-origin API requests from *.rtd.io to rtd.org/api/v2/sustainability/.
"""
if self.USE_PROMOS:
return None
return "None"
# This is django's default.
return "Lax"

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/settings/proxito/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class CommunityProxitoSettingsMixin:

# Allow cookies from cross-site requests on subdomains for now.
# As 'Lax' breaks when the page is embedded in an iframe.
SESSION_COOKIE_SAMESITE = None
SESSION_COOKIE_SAMESITE = "None"

@property
def DATABASES(self):
Expand Down

0 comments on commit 299ba15

Please sign in to comment.