Skip to content

Commit

Permalink
Rename CORS_ORIGIN_ALLOW_ALL to CORS_ALLOW_ALL_ORIGINS (adamchainz#563)
Browse files Browse the repository at this point in the history
Following adamchainz#562, perform this rename whilst we're at it, to make the names more standardized and comprehensible.
  • Loading branch information
adamchainz authored Aug 25, 2020
1 parent 125d47b commit e4a6e0a
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 28 deletions.
6 changes: 4 additions & 2 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ History
=======

* Following Django’s example in
`Ticket #31670 <https://code.djangoproject.com/ticket/31670>`__, the
following settings have been renamed:
`Ticket #31670 <https://code.djangoproject.com/ticket/31670>`__ for replacing
the term “whitelist”, plus an aim to make the setting names more
comprehensible, the following settings have been renamed:

* ``CORS_ORIGIN_WHITELIST`` -> ``CORS_ALLOWED_ORIGINS``
* ``CORS_ORIGIN_WHITELIST_REGEX`` -> ``CORS_ALLOWED_ORIGIN_REGEXES``
* ``CORS_ORIGIN_ALLOW_ALL`` -> ``CORS_ALLOW_ALL_ORIGINS``

The old names will continue to work as aliases, with the new ones taking
precedence.
Expand Down
14 changes: 9 additions & 5 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ least one of three following settings:

* ``CORS_ALLOWED_ORIGINS``
* ``CORS_ALLOWED_ORIGIN_REGEXES``
* ``CORS_ORIGIN_ALLOW_ALL``
* ``CORS_ALLOW_ALL_ORIGINS``

``CORS_ALLOWED_ORIGINS``
~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -144,7 +144,7 @@ Example:
]
Previously this setting was called ``CORS_ORIGIN_WHITELIST``, which still works
as an alias, with the new name takeing precedence.
as an alias, with the new name taking precedence.

``CORS_ALLOWED_ORIGIN_REGEXES``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand All @@ -163,10 +163,11 @@ Example:
]
Previously this setting was called ``CORS_ORIGIN_REGEX_WHITELIST``, which still
works as an alias, with the new name takeing precedence.
works as an alias, with the new name taking precedence.

``CORS_ALLOW_ALL_ORIGINS``
~~~~~~~~~~~~~~~~~~~~~~~~~~

``CORS_ORIGIN_ALLOW_ALL``
~~~~~~~~~~~~~~~~~~~~~~~~~
If ``True``, all origins will be allowed. Other settings restricting allowed
origins will be ignored. Defaults to ``False``.

Expand All @@ -175,6 +176,9 @@ cross-origin requests to yours. Generally you'll want to restrict the list of
allowed origins with ``CORS_ALLOWED_ORIGINS`` or
``CORS_ALLOWED_ORIGIN_REGEXES``.

Previously this setting was called ``CORS_ORIGIN_ALLOW_ALL``, which still
works as an alias, with the new name taking precedence.

--------------

The following are optional settings, for which the defaults probably suffice.
Expand Down
8 changes: 6 additions & 2 deletions src/corsheaders/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,14 @@ def check_settings(app_configs, **kwargs):
)
)

if not isinstance(conf.CORS_ORIGIN_ALLOW_ALL, bool):
if not isinstance(conf.CORS_ALLOW_ALL_ORIGINS, bool):
if hasattr(settings, "CORS_ALLOW_ALL_ORIGINS"):
allow_all_alias = "CORS_ALLOW_ALL_ORIGINS"
else:
allow_all_alias = "CORS_ORIGIN_ALLOW_ALL"
errors.append(
checks.Error(
"CORS_ORIGIN_ALLOW_ALL should be a bool.", id="corsheaders.E005"
"{} should be a bool.".format(allow_all_alias), id="corsheaders.E005",
)
)

Expand Down
8 changes: 6 additions & 2 deletions src/corsheaders/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ def CORS_PREFLIGHT_MAX_AGE(self):
return getattr(settings, "CORS_PREFLIGHT_MAX_AGE", 86400)

@property
def CORS_ORIGIN_ALLOW_ALL(self):
return getattr(settings, "CORS_ORIGIN_ALLOW_ALL", False)
def CORS_ALLOW_ALL_ORIGINS(self):
return getattr(
settings,
"CORS_ALLOW_ALL_ORIGINS",
getattr(settings, "CORS_ORIGIN_ALLOW_ALL", False),
)

@property
def CORS_ALLOWED_ORIGINS(self):
Expand Down
9 changes: 5 additions & 4 deletions src/corsheaders/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ def _https_referer_replace(self, request):
):

url = urlparse(origin)
if not conf.CORS_ORIGIN_ALLOW_ALL and not self.origin_found_in_white_lists(
origin, url
if (
not conf.CORS_ALLOW_ALL_ORIGINS
and not self.origin_found_in_white_lists(origin, url)
):
return

Expand Down Expand Up @@ -121,13 +122,13 @@ def process_response(self, request, response):
response[ACCESS_CONTROL_ALLOW_CREDENTIALS] = "true"

if (
not conf.CORS_ORIGIN_ALLOW_ALL
not conf.CORS_ALLOW_ALL_ORIGINS
and not self.origin_found_in_white_lists(origin, url)
and not self.check_signal(request)
):
return response

if conf.CORS_ORIGIN_ALLOW_ALL and not conf.CORS_ALLOW_CREDENTIALS:
if conf.CORS_ALLOW_ALL_ORIGINS and not conf.CORS_ALLOW_CREDENTIALS:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = "*"
else:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = origin
Expand Down
12 changes: 9 additions & 3 deletions tests/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def test_defaults_pass(self):
def test_defaults_pass_check(self):
call_command("check")

@override_settings(CORS_ORIGIN_ALLOW_ALL=object)
@override_settings(CORS_ALLOW_ALL_ORIGINS=object)
def test_checks_are_bound(self):
with pytest.raises(base.SystemCheckError):
call_command("check")
Expand Down Expand Up @@ -56,9 +56,15 @@ def test_cors_preflight_max_age_non_integer(self):
def test_cors_preflight_max_age_negative(self):
self.check_error_codes(["corsheaders.E004"])

@override_settings(CORS_ALLOW_ALL_ORIGINS=object)
def test_cors_allow_all_origins_non_bool(self):
errors = self.check_error_codes(["corsheaders.E005"])
assert errors[0].msg.startswith("CORS_ALLOW_ALL_ORIGINS should be")

@override_settings(CORS_ORIGIN_ALLOW_ALL=object)
def test_cors_origin_allow_all_non_bool(self):
self.check_error_codes(["corsheaders.E005"])
def test_cors_allow_all_origins_old_name(self):
errors = self.check_error_codes(["corsheaders.E005"])
assert errors[0].msg.startswith("CORS_ORIGIN_ALLOW_ALL should be")

@override_settings(CORS_ALLOWED_ORIGINS=object)
def test_cors_allowed_origins_non_sequence(self):
Expand Down
10 changes: 10 additions & 0 deletions tests/test_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ class ConfTests(SimpleTestCase):
def test_can_override(self):
assert conf.CORS_ALLOW_HEADERS == ["foo"]

@override_settings(CORS_ORIGIN_ALLOW_ALL=True)
def test_cors_allow_all_origins_old_alias(self):
assert conf.CORS_ALLOW_ALL_ORIGINS is True

@override_settings(
CORS_ALLOW_ALL_ORIGINS=False, CORS_ORIGIN_ALLOW_ALL=True,
)
def test_cors_allow_all_origins_new_setting_takes_precedence(self):
assert conf.CORS_ALLOW_ALL_ORIGINS is False

@override_settings(CORS_ORIGIN_WHITELIST=["example.com"])
def test_cors_allowed_origins_old_alias(self):
assert conf.CORS_ALLOWED_ORIGINS == ["example.com"]
Expand Down
20 changes: 10 additions & 10 deletions tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,24 +64,24 @@ def test_file_in_allowed_origins(self):
assert resp[ACCESS_CONTROL_ALLOW_ORIGIN] == "file://"

@override_settings(
CORS_ORIGIN_ALLOW_ALL=True,
CORS_ALLOW_ALL_ORIGINS=True,
CORS_EXPOSE_HEADERS=["accept", "origin", "content-type"],
)
def test_get_expose_headers(self):
resp = self.client.get("/", HTTP_ORIGIN="http://example.com")
assert resp[ACCESS_CONTROL_EXPOSE_HEADERS] == "accept, origin, content-type"

@override_settings(CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_ALL_ORIGINS=True)
def test_get_dont_expose_headers(self):
resp = self.client.get("/", HTTP_ORIGIN="http://example.com")
assert ACCESS_CONTROL_EXPOSE_HEADERS not in resp

@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ALLOW_ALL_ORIGINS=True)
def test_get_allow_credentials(self):
resp = self.client.get("/", HTTP_ORIGIN="http://example.com")
assert resp[ACCESS_CONTROL_ALLOW_CREDENTIALS] == "true"

@override_settings(CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_ALL_ORIGINS=True)
def test_get_dont_allow_credentials(self):
resp = self.client.get("/", HTTP_ORIGIN="http://example.com")
assert ACCESS_CONTROL_ALLOW_CREDENTIALS not in resp
Expand All @@ -90,7 +90,7 @@ def test_get_dont_allow_credentials(self):
CORS_ALLOW_HEADERS=["content-type", "origin"],
CORS_ALLOW_METHODS=["GET", "OPTIONS"],
CORS_PREFLIGHT_MAX_AGE=1002,
CORS_ORIGIN_ALLOW_ALL=True,
CORS_ALLOW_ALL_ORIGINS=True,
)
def test_options_allowed_origin(self):
resp = self.client.options("/", HTTP_ORIGIN="http://example.com")
Expand All @@ -102,7 +102,7 @@ def test_options_allowed_origin(self):
CORS_ALLOW_HEADERS=["content-type", "origin"],
CORS_ALLOW_METHODS=["GET", "OPTIONS"],
CORS_PREFLIGHT_MAX_AGE=0,
CORS_ORIGIN_ALLOW_ALL=True,
CORS_ALLOW_ALL_ORIGINS=True,
)
def test_options_no_max_age(self):
resp = self.client.options("/", HTTP_ORIGIN="http://example.com")
Expand Down Expand Up @@ -149,14 +149,14 @@ def test_options_no_header(self):
resp = self.client.options("/")
assert resp.status_code == 404

@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ALLOW_ALL_ORIGINS=True)
def test_allow_all_origins_get(self):
resp = self.client.get("/", HTTP_ORIGIN="http://example.com")
assert resp.status_code == 200
assert resp[ACCESS_CONTROL_ALLOW_ORIGIN] == "http://example.com"
assert resp["Vary"] == "Origin"

@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ALLOW_ALL_ORIGINS=True)
def test_allow_all_origins_options(self):
resp = self.client.options(
"/",
Expand All @@ -167,7 +167,7 @@ def test_allow_all_origins_options(self):
assert resp[ACCESS_CONTROL_ALLOW_ORIGIN] == "http://example.com"
assert resp["Vary"] == "Origin"

@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ALLOW_ALL_ORIGINS=True)
def test_non_200_headers_still_set(self):
"""
It's not clear whether the header should still be set for non-HTTP200
Expand All @@ -181,7 +181,7 @@ def test_non_200_headers_still_set(self):
assert resp.status_code == 401
assert resp[ACCESS_CONTROL_ALLOW_ORIGIN] == "http://example.com"

@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ORIGIN_ALLOW_ALL=True)
@override_settings(CORS_ALLOW_CREDENTIALS=True, CORS_ALLOW_ALL_ORIGINS=True)
def test_auth_view_options(self):
"""
Ensure HTTP200 and header still set, for preflight requests to views requiring
Expand Down

0 comments on commit e4a6e0a

Please sign in to comment.