diff --git a/edx_django_utils/security/clickjacking/README.rst b/edx_django_utils/security/clickjacking/README.rst new file mode 100644 index 00000000..81f5f54d --- /dev/null +++ b/edx_django_utils/security/clickjacking/README.rst @@ -0,0 +1,28 @@ +Security Utils +############## + +Common security utilities. + +Deprecated - Clickjacking middleware +************************************ + +Consider this deprecated as new projects should be using Content Security Policy headers instead. +However, this won't be removed until no openedx project depends on it anymore, specifically, edx-platform. +Please do not add this middleware to a project unless you have to use the ``X-Frame-Options`` header, +and do not add it outside the openedx github organization, as it may be removed in the future. + +Clickjacking middleware +*********************** + +The preferred way to prevent clickjacking is to use the Content Security Policy headers. +In some legacy code where those headers are not used, it is required to instead use the older +header ``X-Frame-Options`` set either to ``DENY`` or to ``SAMEORIGIN``. +In any case, this middleware allows you both to set the ``X-Frame-Options`` header to any recognized value - +``DENY``, ``SAMEORIGIN``, ``ALLOW`` per django setting - but defaults to ``DENY``. +It also allows you to override the header for specific urls defined via regex. + +- Add the middleware ``'edx_django_utils.security.clickjacking.middleware.EdxXFrameOptionsMiddleware'`` near the end of your ``MIDDLEWARE`` list. +- This will add an `X-Frame-Options` header to all responses. +- Add ``X_FRAME_OPTIONS = value`` to your django settings file with "value" being ``DENY``, ``SAMEORIGIN``, or ``ALLOW``. +- Optionally, add ``X_FRAME_OPTIONS_OVERRIDES = [[regex, value]]`` where ``[[regex, value]]`` is a list of + pairs consisting of a regex that matches urls to override and a value that's one of ``DENY``, ``SAMEORIGIN``, and ``ALLOW``. diff --git a/edx_django_utils/security/clickjacking/__init__.py b/edx_django_utils/security/clickjacking/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/edx_django_utils/security/clickjacking/middleware.py b/edx_django_utils/security/clickjacking/middleware.py new file mode 100644 index 00000000..b79c5a2e --- /dev/null +++ b/edx_django_utils/security/clickjacking/middleware.py @@ -0,0 +1,59 @@ +""" +Middleware to add correct x-frame-options headers. + +The headers get set to the platform default which we assume is `DENY`. +However, there's a number of paths that are set to `SAMEORIGIN` which +we identify via regexes stored in a django setting in the application calling this. +""" + +import re + +from django.conf import settings +from django.http.request import HttpRequest +from django.http.response import HttpResponse +from django.middleware.clickjacking import XFrameOptionsMiddleware + +PERMISSIBLE_VALUES = ['DENY', 'SAMEORIGIN', 'ALLOW'] + + +class InvalidHeaderValueError(ValueError): + """ A custom error that is thrown when we try to set an invalid value for a header """ + pass + + +def _validate_header_value(value): + if value not in PERMISSIBLE_VALUES: + raise InvalidHeaderValueError( + f'Invalid value "{value}" for header "X-Frame-Options"' + ) + + +class EdxXFrameOptionsMiddleware(XFrameOptionsMiddleware): + """ + A class extending the django XFrameOptionsMiddleware with the ability to override + the header for URLs specified in a `X_FRAME_OPTIONS_OVERRIDES` django setting. + You can set this via `X_FRAME_OPTIONS_OVERRIDES = [[regex, value]]` in your django application + where you specify a list of pairs of regex and value, where regex matches urls and value is + one of `DENY`, `SAMEORIGIN`, `ALLOW`. The latter is not advisable unless you have a content security + policy in place. + """ + def process_response(self, request: HttpRequest, response: HttpResponse) -> HttpResponse: + """ + Process the response and set the x-frame-options header to the value specified + """ + response = super().process_response(request, response) + headers = response.headers + request_path = request.path + frame_options = getattr(settings, 'X_FRAME_OPTIONS', 'DENY') + _validate_header_value(frame_options) + + headers['X-Frame-Options'] = frame_options + overrides = getattr(settings, 'X_FRAME_OPTIONS_OVERRIDES', []) + for override in overrides: + print('override', override) + regex, value = override + _validate_header_value(value) + if re.search(regex, request_path): + headers['X-Frame-Options'] = value + response.headers = headers + return response diff --git a/edx_django_utils/security/clickjacking/tests/test_clickjacking_middleware.py b/edx_django_utils/security/clickjacking/tests/test_clickjacking_middleware.py new file mode 100644 index 00000000..4e33d0d7 --- /dev/null +++ b/edx_django_utils/security/clickjacking/tests/test_clickjacking_middleware.py @@ -0,0 +1,90 @@ +""" +Tests for Content-Security-Policy middleware. +""" + +from unittest import TestCase +from unittest.mock import MagicMock, patch + +import ddt + +from edx_django_utils.security.clickjacking.middleware import EdxXFrameOptionsMiddleware + + +@ddt.ddt +class TestEdxXFrameOptionsMiddleware(TestCase): + """Test the actual middleware.""" + + @patch('edx_django_utils.security.clickjacking.middleware._validate_header_value') + @patch('edx_django_utils.security.clickjacking.middleware.settings') + def test_x_frame_setting_must_apply_on_no_override(self, settings, validate_header): + """ + If the setting `X_FRAME_OPTIONS` is set but no overrides are specified, + the `X-Frame-Options` header should be set to that setting. + """ + settings.X_FRAME_OPTIONS = 'SAMEORIGIN' + validate_header.return_value = True + + request = MagicMock() + response = MagicMock() + response.headers = {} + middleware = EdxXFrameOptionsMiddleware(get_response=lambda _: response) + + middleware.process_response(request, response) + + assert response.headers['X-Frame-Options'] == 'SAMEORIGIN' + validate_header.assert_called_once_with('SAMEORIGIN') + + @patch('edx_django_utils.security.clickjacking.middleware._validate_header_value') + @patch('edx_django_utils.security.clickjacking.middleware.settings') + def test_on_override_with_valid_regex_is_sameorigin(self, settings, validate_header): + """ + If the URL matches one of the overrides, the header should be set to + the correct override setting as specified in the `X_FRAME_OPTIONS_OVERRIDES` list. + """ + settings.X_FRAME_OPTIONS = 'DENY' + settings.X_FRAME_OPTIONS_OVERRIDES = [['.*/media/scorm/.*', 'SAMEORIGIN']] + validate_header.return_value = True + + request = MagicMock() + response = MagicMock() + response.headers = {} + request.path = 'http://localhost:18010/media/scorm/hello/world' + middleware = EdxXFrameOptionsMiddleware(get_response=lambda _: response) + + middleware.process_response(request, response) + + assert response.headers['X-Frame-Options'] == 'SAMEORIGIN' + + @patch('edx_django_utils.security.clickjacking.middleware._validate_header_value') + @patch('edx_django_utils.security.clickjacking.middleware.settings') + def test_on_override_for_non_matching_urls_is_deny(self, settings, validate_header): + """ + If the URL does not match any of the overrides, the header should be set to + the `X_FRAME_OPTIONS` setting. + """ + settings.X_FRAME_OPTIONS = 'DENY' + settings.X_FRAME_OPTIONS_OVERRIDES = [['.*/media/scorm/.*', 'SAMEORIGIN']] + validate_header.return_value = True + + request = MagicMock() + response = MagicMock() + response.headers = {} + request.path = 'http://localhost:18010/notmedia/scorm/hello/world' + middleware = EdxXFrameOptionsMiddleware(get_response=lambda _: response) + + middleware.process_response(request, response) + + assert response.headers['X-Frame-Options'] == 'DENY' + + def test_x_frame_defaults_to_deny(self): + """ + The default value of the `X-Frame-Options` header should be `DENY`. + """ + request = MagicMock() + response = MagicMock() + response.headers = {} + middleware = EdxXFrameOptionsMiddleware(get_response=lambda _: response) + + middleware.process_response(request, response) + + assert response.headers['X-Frame-Options'] == 'DENY'