Require scheme in origins (#397)

This "upgrades" the change in #388 to *require* scheme, rather than going through a deprecation period. It simplifies the code greatly.

I added checks as well to ensure that `CORS_ORIGIN_WHITELIST` is well-formed, so that users upgrading will see some obvious errors.

HISTORY.rst

@@ -6,15 +6,34 @@ Pending
 
 .. Insert new release notes below this line
 
-* Origin is now scheme-aware. Deprecation warning has been added when origin
-  without scheme is included.
+* ``CORS_ORIGIN_WHITELIST`` now requires URI schemes, and optionally ports.
+  This is part of the CORS specification
+  (`Section 3.2 <https://tools.ietf.org/html/rfc6454#section-3.2>`_) that was
+  not implemented in this library, except from with the
+  ``CORS_ORIGIN_REGEX_WHITELIST`` setting. It fixes a security issue where the
+  CORS middleware would allow requests between schemes, for example from
+  insecure ``http://`` Origins to a secure ``https://`` site.
+
+  You will need to update your whitelist to include schemes, for example from
+  this:
+
+  .. code-block:: python
+
+      CORS_ORIGIN_WHITELIST = ['example.com']
+
+  ...to this:
+
+  .. code-block:: python
+
+      CORS_ORIGIN_WHITELIST = ['https://example.com']
+
 * Removed the ``CORS_MODEL`` setting, and associated class. It seems very few,
   or no users were using it, since there were no bug reports since its move to
   abstract in version 2.0.0 (2017-01-07). If you *are* using this
   functionality, you can continue by changing your model to not inherit from
   the abstract one, and add a signal handler for ``check_request_enabled`` that
-  reads from your model. Note you'll need to handle the move to scheme-aware
-  values for Origin.
+  reads from your model. Note you'll need to handle the move to include schemes
+  for Origins.
 
 2.5.3 (2019-04-28)
 ------------------

README.rst

@@ -95,40 +95,44 @@ Defaults to ``False``.
 ``CORS_ORIGIN_WHITELIST``
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-A list of origin hostnames that are authorized to make cross-site HTTP
-requests. The value ``'null'`` can also appear in this list, and will match the
-``Origin: null`` header that is used in `"privacy-sensitive contexts"
-<https://tools.ietf.org/html/rfc6454#section-6>`_, such as when the client is
-running from a ``file://`` domain. Defaults to ``[]``. Proper origin should consist of
-scheme, host and port (which could be given implicitly, e.g. for http it is assumed that the port is
-80). Skipping scheme is allowed only for backward compatibility, deprecation warning will be raised
-if this is discovered.
+A list of origins that are authorized to make cross-site HTTP requests.
+Defaults to ``[]``.
+
+An Origin is defined by
+`the CORS RFC Section 3.2 <https://tools.ietf.org/html/rfc6454#section-3.2>`_
+as a URI scheme + hostname + port, or the special value `'null'`.
+Default ports (HTTPS = 443, HTTP = 80) are optional here.
+The special value `null` is sent by the browser in
+`"privacy-sensitive contexts" <https://tools.ietf.org/html/rfc6454#section-6>`_,
+such as when the client is running from a ``file://`` domain.
 
 Example:
 
 .. code-block:: python
 
-    CORS_ORIGIN_WHITELIST = (
-        'https://google.com',
-        'http://hostname.example.com',
-        'http://localhost:8000',
-        'http://127.0.0.1:9000'
+    CORS_ORIGIN_WHITELIST = [
+        "https://example.com",
+        "https://sub.example.com",
+        "http://localhost:8080",
+        "http://127.0.0.1:9000"
     )
 
 
 ``CORS_ORIGIN_REGEX_WHITELIST``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-A list of regexes that match origin regex list of origin hostnames that are
-authorized to make cross-site HTTP requests. Defaults to ``[]``. Useful when
-``CORS_ORIGIN_WHITELIST`` is impractical, such as when you have a large
-number of subdomains.
+A list of strings representing regexes that match Origins that are authorized
+to make cross-site HTTP requests. Defaults to ``[]``. Useful when
+``CORS_ORIGIN_WHITELIST`` is impractical, such as when you have a large number
+of subdomains.
 
 Example:
 
 .. code-block:: python
 
-    CORS_ORIGIN_REGEX_WHITELIST = (r'^(https?://)?(\w+\.)?google\.com$', )
+    CORS_ORIGIN_REGEX_WHITELIST = [
+        r"^https://\w+\.example\.com$",
+    ]
 
 --------------
 

corsheaders/checks.py

@@ -6,6 +6,7 @@
 from django.conf import settings
 from django.core import checks
 from django.utils import six
+from django.utils.six.moves.urllib.parse import urlparse
 
 from .conf import conf
 
@@ -68,6 +69,22 @@ def check_settings(app_configs, **kwargs):
                 id="corsheaders.E006"
             )
         )
+    else:
+        for origin in conf.CORS_ORIGIN_WHITELIST:
+            parsed = urlparse(origin)
+            if parsed.scheme == '' or parsed.netloc == '':
+                errors.append(checks.Error(
+                    "Origin {} in CORS_ORIGIN_WHITELIST is missing scheme or netloc".format(repr(origin)),
+                    id="corsheaders.E013"
+                ))
+            else:
+                # Only do this check in this case because if the scheme is not provided, netloc ends up in path
+                for part in ('path', 'params', 'query', 'fragment'):
+                    if getattr(parsed, part) != '':
+                        errors.append(checks.Error(
+                            "Origin {} in CORS_ORIGIN_WHITELIST should not have {}".format(repr(origin), part),
+                            id="corsheaders.E014"
+                        ))
 
     if not is_sequence(conf.CORS_ORIGIN_REGEX_WHITELIST, six.string_types + (re_type,)):
         errors.append(

corsheaders/middleware.py

@@ -1,7 +1,6 @@
 from __future__ import absolute_import
 
 import re
-import warnings
 
 from django import http
 from django.utils.cache import patch_vary_headers
@@ -143,11 +142,9 @@ def process_response(self, request, response):
         return response
 
     def origin_found_in_white_lists(self, origin, url):
-        whitelisted_origins = self._get_parsed_whitelisted_origins(conf.CORS_ORIGIN_WHITELIST)
-        self._check_for_origins_without_scheme(whitelisted_origins)
         return (
-            self._url_in_whitelist(url, whitelisted_origins)
-            or (origin == 'null' and origin in conf.CORS_ORIGIN_WHITELIST)
+            (origin == 'null' and origin in conf.CORS_ORIGIN_WHITELIST)
+            or self._url_in_whitelist(url)
             or self.regex_domain_match(origin)
         )
 
@@ -172,31 +169,9 @@ def check_signal(self, request):
             function, return_value in signal_responses
         )
 
-    def _get_parsed_whitelisted_origins(self, origins):
-        whitelisted_origins = []
-        for origin in origins:
-            # Note that when port is defined explicitly, it's part of netloc/path
-            parsed_origin = urlparse(origin)
-            whitelisted_origins.append(
-                {
-                    'scheme': parsed_origin.scheme,
-                    'host': parsed_origin.netloc or parsed_origin.path
-                }
-            )
-        return whitelisted_origins
-
-    def _check_for_origins_without_scheme(self, origins):
-        if any((origin['scheme'] == '' and origin['host'] != 'null' for origin in origins)):
-            warnings.warn('Passing origins without scheme will be deprecated.', DeprecationWarning)
-
-    def _url_in_whitelist(self, url, origins_whitelist):
-        possible_matching_origins = [
-            origin for origin in origins_whitelist if origin['host'] == url.netloc
-        ]
-        if not possible_matching_origins:
-            return False
-        else:
-            for origin in possible_matching_origins:
-                if origin['scheme'] == '' or origin['scheme'] == url.scheme:
-                    return True
-            return False
+    def _url_in_whitelist(self, url):
+        origins = [urlparse(o) for o in conf.CORS_ORIGIN_WHITELIST]
+        return any(
+            origin.scheme == url.scheme and origin.netloc == url.netloc
+            for origin in origins
+        )

setup.cfg

@@ -2,7 +2,7 @@
 universal = 1
 
 [flake8]
-ignore = X99999, W503
+ignore = X99999, W503, C901
 max-complexity = 12
 max-line-length = 120
 

tests/test_checks.py

@@ -70,6 +70,18 @@ def test_cors_origin_whitelist_non_sequence(self):
     def test_cors_origin_whitelist_non_string(self):
         self.check_error_codes(['corsheaders.E006'])
 
+    @override_settings(CORS_ORIGIN_WHITELIST=['example.com'])
+    def test_cors_origin_whitelist_no_scheme(self):
+        self.check_error_codes(['corsheaders.E013'])
+
+    @override_settings(CORS_ORIGIN_WHITELIST=['https://'])
+    def test_cors_origin_whitelist_no_netloc(self):
+        self.check_error_codes(['corsheaders.E013'])
+
+    @override_settings(CORS_ORIGIN_WHITELIST=['https://example.com/foobar'])
+    def test_cors_origin_whitelist_path(self):
+        self.check_error_codes(['corsheaders.E014'])
+
     @override_settings(CORS_ORIGIN_REGEX_WHITELIST=object)
     def test_cors_origin_regex_whitelist_non_sequence(self):
         self.check_error_codes(['corsheaders.E007'])

tests/test_middleware.py

@@ -1,7 +1,5 @@
 from __future__ import absolute_import
 
-import warnings
-
 from django.http import HttpResponse
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -41,15 +39,6 @@ def test_get_not_in_whitelist_due_to_wrong_scheme(self):
         resp = self.client.get('/', HTTP_ORIGIN='http://example.org')
         assert ACCESS_CONTROL_ALLOW_ORIGIN not in resp
 
-    @override_settings(CORS_ORIGIN_WHITELIST=['example.org'])
-    def test_get_without_scheme_in_whitelist_raises_warning(self):
-        with warnings.catch_warnings(record=True) as warn:
-            resp = self.client.get('/', HTTP_ORIGIN='http://example.org')
-            assert ACCESS_CONTROL_ALLOW_ORIGIN in resp
-            assert len(warn) == 1
-            assert issubclass(warn[-1].category, DeprecationWarning)
-            assert 'Passing origins without scheme will be deprecated.' in str(warn[-1].message)
-
     @override_settings(CORS_ORIGIN_WHITELIST=['http://example.com', 'http://example.org'])
     def test_get_in_whitelist(self):
         resp = self.client.get('/', HTTP_ORIGIN='http://example.org')
@@ -116,7 +105,7 @@ def test_options_no_max_age(self):
     @override_settings(
         CORS_ALLOW_METHODS=['OPTIONS'],
         CORS_ALLOW_CREDENTIALS=True,
-        CORS_ORIGIN_WHITELIST=('http://localhost:9000',),
+        CORS_ORIGIN_WHITELIST=['http://localhost:9000'],
     )
     def test_options_whitelist_with_port(self):
         resp = self.client.options('/', HTTP_ORIGIN='http://localhost:9000')