Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a middleware for referrer policy #7346

Merged
merged 3 commits into from Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions dockerfiles/nginx/proxito.conf
Expand Up @@ -64,6 +64,8 @@ server {
add_header X-RTD-Redirect $rtd_redirect always;
set $cache_tag $upstream_http_cache_tag;
add_header Cache-Tag $cache_tag always;
set $referrer_policy $upstream_http_referrer_policy;
add_header Referrer-Policy $referrer_policy always;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a corresponding ops change as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does although the critical piece is getting the stricter policy for the dashboard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a PR for the corresponding ops change.

}

# Serve 404 pages here
Expand Down
44 changes: 44 additions & 0 deletions readthedocs/core/middleware.py
Expand Up @@ -6,6 +6,7 @@
from django.contrib.sessions.backends.base import UpdateError
from django.contrib.sessions.middleware import SessionMiddleware
from django.core.exceptions import SuspiciousOperation
from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
from django.http import Http404, HttpResponseBadRequest
from django.urls.base import set_urlconf
Expand Down Expand Up @@ -156,3 +157,46 @@ def process_response(self, request, response):
samesite=settings.SESSION_COOKIE_SAMESITE,
)
return 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
2 changes: 2 additions & 0 deletions readthedocs/settings/base.py
Expand Up @@ -76,6 +76,7 @@ class CommunityBaseSettings(Settings):
# https://docs.djangoproject.com/en/1.11/ref/middleware/#django.middleware.security.SecurityMiddleware
SECURE_BROWSER_XSS_FILTER = True
SECURE_CONTENT_TYPE_NOSNIFF = True
SECURE_REFERRER_POLICY = "strict-origin-when-cross-origin"
X_FRAME_OPTIONS = 'DENY'

# Content Security Policy
Expand Down Expand Up @@ -193,6 +194,7 @@ def USE_PROMOS(self): # noqa
'dj_pagination.middleware.PaginationMiddleware',
'corsheaders.middleware.CorsMiddleware',
'csp.middleware.CSPMiddleware',
'readthedocs.core.middleware.ReferrerPolicyMiddleware',
)

AUTHENTICATION_BACKENDS = (
Expand Down
1 change: 1 addition & 0 deletions readthedocs/settings/proxito/base.py
Expand Up @@ -11,6 +11,7 @@ class CommunityProxitoSettingsMixin:

ROOT_URLCONF = 'readthedocs.proxito.urls'
USE_SUBDOMAIN = True
SECURE_REFERRER_POLICY = "no-referrer-when-downgrade"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I think strict-origin-when-cross-origin would be fine for docs sites as we don't really use the additional data as far as I'm aware but at least we have an easy upgrade path this way.


@property
def DATABASES(self):
Expand Down