Skip to content

Commit 188798c

Browse files
authored
Cookies: set samesite: Lax by default (#8304)
The django's default is `Lax`. We also are only allowing cross site requests from public versions only. Even if the domain is known, if the version is private, we don't set the CORS header.
1 parent d4fe8ce commit 188798c

File tree

6 files changed

+51
-43
lines changed

6 files changed

+51
-43
lines changed

docs/server-side-search.rst

+12-6
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,15 @@ and then click on :guilabel:`Search Analytics`.
7272
API
7373
---
7474

75-
Search is exposed through our API that's proxied from the domain where your docs are being served.
76-
This is ``https://docs.readthedocs.io/_/api/v2/search`` for the ``docs`` project, for example.
75+
If you are using :doc:`/commercial/index` you will need to replace
76+
https://readthedocs.org/ with https://readthedocs.com/ in all the URLs used in the following examples.
77+
Check :ref:`server-side-search:authentication and authorization` if you are using private versions.
7778

7879
.. warning::
7980

8081
This API isn't stable yet, some small things may change in the future.
8182

82-
.. http:get:: /_/api/v2/search/
83+
.. http:get:: /api/v2/search/
8384
8485
Return a list of search results for a project,
8586
including results from its :doc:`/subprojects`.
@@ -120,12 +121,12 @@ This is ``https://docs.readthedocs.io/_/api/v2/search`` for the ``docs`` project
120121

121122
.. code-tab:: bash
122123

123-
$ curl "https://docs.readthedocs.io/_/api/v2/search/?project=docs&version=latest&q=server%20side%20search"
124+
$ curl "https://readthedocs.org/api/v2/search/?project=docs&version=latest&q=server%20side%20search"
124125

125126
.. code-tab:: python
126127

127128
import requests
128-
URL = 'https://docs.readthedocs.io/_/api/v2/search/'
129+
URL = 'https://readthedocs.org/api/v2/search/'
129130
params = {
130131
'q': 'server side search',
131132
'project': 'docs',
@@ -140,7 +141,7 @@ This is ``https://docs.readthedocs.io/_/api/v2/search`` for the ``docs`` project
140141

141142
{
142143
"count": 41,
143-
"next": "https://docs.readthedocs.io/api/v2/search/?page=2&project=read-the-docs&q=server+side+search&version=latest",
144+
"next": "https://readthedocs.org/api/v2/search/?page=2&project=read-the-docs&q=server+side+search&version=latest",
144145
"previous": null,
145146
"results": [
146147
{
@@ -194,3 +195,8 @@ If you are using :ref:`private versions <versions:privacy levels>`,
194195
users will only be allowed to search projects they have permissions over.
195196
Authentication and authorization is done using the current session,
196197
or any of the valid :doc:`sharing methods </commercial/sharing>`.
198+
199+
To be able to use the user's current session you need to use the API from the domain where your docs are being served
200+
(``<you-docs-domain>/_/api/v2/search/``).
201+
This is ``https://docs.readthedocs-hosted.com/_/api/v2/search/``
202+
for the ``https://docs.readthedocs-hosted.com/`` project, for example.

readthedocs/core/middleware.py

+6-12
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,16 @@
22
import time
33

44
from django.conf import settings
5-
from django.contrib.sessions.backends.base import SessionBase
6-
from django.contrib.sessions.backends.base import UpdateError
5+
from django.contrib.sessions.backends.base import SessionBase, UpdateError
76
from django.contrib.sessions.middleware import SessionMiddleware
8-
from django.core.exceptions import SuspiciousOperation
9-
from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed
10-
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
11-
from django.http import Http404, HttpResponseBadRequest
12-
from django.urls.base import set_urlconf
7+
from django.core.exceptions import (
8+
ImproperlyConfigured,
9+
MiddlewareNotUsed,
10+
SuspiciousOperation,
11+
)
1312
from django.utils.cache import patch_vary_headers
14-
from django.utils.deprecation import MiddlewareMixin
1513
from django.utils.http import http_date
1614
from django.utils.translation import ugettext_lazy as _
17-
from django.shortcuts import render
18-
19-
from readthedocs.projects.models import Domain, Project
20-
2115

2216
log = logging.getLogger(__name__)
2317

readthedocs/core/signals.py

+3-21
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
"""Signal handling for core app."""
22

33
import logging
4-
from urllib.parse import urlparse
54

65
from corsheaders import signals
76
from django.conf import settings
8-
from django.db.models import Count, Q
7+
from django.db.models import Count
98
from django.db.models.signals import pre_delete
109
from django.dispatch import Signal, receiver
1110
from rest_framework.permissions import SAFE_METHODS
1211
from simple_history.signals import pre_create_historical_record
1312

1413
from readthedocs.builds.models import Version
1514
from readthedocs.core.unresolver import unresolve
16-
from readthedocs.projects.models import Domain, Project
15+
from readthedocs.projects.models import Project
1716

1817
log = logging.getLogger(__name__)
1918

@@ -50,8 +49,7 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen
5049
* It's a safe HTTP method
5150
* The origin is in ALLOWED_URLS
5251
* The URL is owned by the project that they are requesting data from
53-
* The version is public or the domain is linked to the project
54-
(except for the embed API).
52+
* The version is public
5553
5654
.. note::
5755
@@ -63,8 +61,6 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen
6361
if 'HTTP_ORIGIN' not in request.META or request.method not in SAFE_METHODS:
6462
return False
6563

66-
host = urlparse(request.META['HTTP_ORIGIN']).netloc.split(':')[0]
67-
6864
# Always allow the sustainability API,
6965
# it's used only on .org to check for ad-free users.
7066
if _has_donate_app() and request.path_info.startswith('/api/v2/sustainability'):
@@ -104,20 +100,6 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen
104100
if is_public:
105101
return True
106102

107-
# Don't check for known domains for the embed api.
108-
# It gives a lot of information,
109-
# we should use a list of trusted domains from the user.
110-
if valid_url == '/api/v2/embed':
111-
return False
112-
113-
# Or allow if they have a registered domain
114-
# linked to that project.
115-
domain = Domain.objects.filter(
116-
Q(domain__iexact=host),
117-
Q(project=project) | Q(project__subprojects__child=project),
118-
)
119-
if domain.exists():
120-
return True
121103
return False
122104

123105

readthedocs/rtd_tests/tests/test_middleware.py

+14-2
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def test_allow_linked_domain_from_public_version(self):
7272
resp = self.middleware.process_response(request, {})
7373
self.assertIn('Access-Control-Allow-Origin', resp)
7474

75-
def test_allow_linked_domain_from_private_version(self):
75+
def test_dont_allow_linked_domain_from_private_version(self):
7676
self.version.privacy_level = PRIVATE
7777
self.version.save()
7878
request = self.factory.get(
@@ -81,7 +81,7 @@ def test_allow_linked_domain_from_private_version(self):
8181
HTTP_ORIGIN='http://my.valid.domain',
8282
)
8383
resp = self.middleware.process_response(request, {})
84-
self.assertIn('Access-Control-Allow-Origin', resp)
84+
self.assertNotIn('Access-Control-Allow-Origin', resp)
8585

8686
def test_allowed_api_public_version_from_another_domain(self):
8787
request = self.factory.get(
@@ -228,6 +228,7 @@ def setUp(self):
228228

229229
self.user = create_user(username='owner', password='test')
230230

231+
@override_settings(SESSION_COOKIE_SAMESITE=None)
231232
def test_fallback_cookie(self):
232233
request = self.factory.get('/')
233234
response = HttpResponse()
@@ -238,6 +239,7 @@ def test_fallback_cookie(self):
238239
self.assertTrue(settings.SESSION_COOKIE_NAME in response.cookies)
239240
self.assertTrue(self.middleware.cookie_name_fallback in response.cookies)
240241

242+
@override_settings(SESSION_COOKIE_SAMESITE=None)
241243
def test_main_cookie_samesite_none(self):
242244
request = self.factory.get('/')
243245
response = HttpResponse()
@@ -247,3 +249,13 @@ def test_main_cookie_samesite_none(self):
247249

248250
self.assertEqual(response.cookies[settings.SESSION_COOKIE_NAME]['samesite'], 'None')
249251
self.assertEqual(response.cookies[self.middleware.cookie_name_fallback]['samesite'], '')
252+
253+
def test_main_cookie_samesite_lax(self):
254+
request = self.factory.get('/')
255+
response = HttpResponse()
256+
self.middleware.process_request(request)
257+
request.session['test'] = 'value'
258+
response = self.middleware.process_response(request, response)
259+
260+
self.assertEqual(response.cookies[settings.SESSION_COOKIE_NAME]['samesite'], 'Lax')
261+
self.assertTrue(self.test_main_cookie_samesite_none not in response.cookies)

readthedocs/settings/base.py

+10-2
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,16 @@ class CommunityBaseSettings(Settings):
7373
SESSION_COOKIE_HTTPONLY = True
7474
SESSION_COOKIE_AGE = 30 * 24 * 60 * 60 # 30 days
7575
SESSION_SAVE_EVERY_REQUEST = True
76-
# This cookie is used in cross-origin API requests from *.readthedocs.io to readthedocs.org
77-
SESSION_COOKIE_SAMESITE = None
76+
77+
@property
78+
def SESSION_COOKIE_SAMESITE(self):
79+
"""
80+
Cookie used in cross-origin API requests from *.rtd.io to rtd.org/api/v2/sustainability/.
81+
"""
82+
if self.USE_PROMOS:
83+
return None
84+
# This is django's default.
85+
return 'Lax'
7886

7987
# CSRF
8088
CSRF_COOKIE_HTTPONLY = True

readthedocs/settings/proxito/base.py

+6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ class CommunityProxitoSettingsMixin:
1313
USE_SUBDOMAIN = True
1414
SECURE_REFERRER_POLICY = "no-referrer-when-downgrade"
1515

16+
# Always set to Lax for proxito cookies.
17+
# Even if the donate app is present.
18+
# Since we don't want to allow cookies from cross origin requests.
19+
# This is django's default.
20+
SESSION_COOKIE_SAMESITE = 'Lax'
21+
1622
@property
1723
def DATABASES(self):
1824
# This keeps connections to the DB alive,

0 commit comments

Comments
 (0)