Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat!: Rename CookieNameChange middleware settings (#28584)

Browse Source 
CookieNameChange allowed the use of the expand-contract pattern for
cookies, but the suggested procedure assumed an instantaneous change on
the server side. Without that, there would be a brief window of time where
servers that had received the newer config would be writing cookies that
the servers with old config would not be able to understand.

However, the mechanism can be made seamless by using it *twice* in
succession, with the first usage in a "no-op" configuration. This allows
all the servers to become aware of the new name without using it. The
second change flips the configuration and changes the official name of the
cookie, and during that window both sets of servers are able to understand
both sets of names, even though they're sending a mix of names. This can
then be followed by the usual cleanup.
This commit is contained in:
Tim McCormack
2021-08-31 16:50:54 +00:00
committed by GitHub
parent a17443f1f7
commit f25b395eca
2 changed files with 65 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
openedx/core/djangoapps/cookie_metadata/middleware.py
View File

@@ -13,18 +13,49 @@ class CookieNameChange:
Changes the names of a cookie in request.COOKIES
For this middleware to run:
- set COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE = True
- set COOKIE_NAME_CHANGE_ACTIVATE = True
- COOKIE_NAME_CHANGE_EXPAND_INFO is a dict and has following info:
"old_name": Previous name of cookie
"new_name": New name of cookie
- "current": Cookie name that will be used by relying code
- "alternate": Other cookie name, to be renamed to current name if present
Actions taken by middleware:
- will delete any cookie with "old_name"
- if create a new cookie with "new_name" and set its value to value of "old_name" cookie
+ it will not modify cookie with "new_name" if it already exists in request.COOKIES
Actions taken by middleware, during request phase:
- Delete alternate-name cookie from request.COOKIES
- Preserve any cookie with current name, or create one with value of
cookie with alternate name (if alt cookie was present)
To perform a seamless name change for a cookie, follow this
expand-contract procedure:
1. Baseline configuration::
SOME_COOKIE_NAME: old
2. Enable servers to understand both names by renaming the *new* name
to the *old* (current) name, which should have no immediate effect::
COOKIE_NAME_CHANGE_ACTIVATE: True
COOKIE_NAME_CHANGE_EXPAND_INFO:
current: old
alternate: new
SOME_COOKIE_NAME: old
3. Change the current name, both in the main cookie setting and in the
name-change dictionary, now that all servers are capable of reading
either name::
COOKIE_NAME_CHANGE_ACTIVATE: True
COOKIE_NAME_CHANGE_EXPAND_INFO:
current: new
alternate: old
SOME_COOKIE_NAME: new
4. After some time period to allow old cookies to age out, remove the
transition settings::
SOME_COOKIE_NAME: new
"""
# .. toggle_name: COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE
# .. toggle_name: COOKIE_NAME_CHANGE_ACTIVATE
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: Used to enable CookieNameChange middleware which changes a cookie name in request.COOKIES
@@ -33,26 +64,26 @@ class CookieNameChange:
# .. toggle_creation_date: 2021-08-04
# .. toggle_target_removal_date: 2021-10-01
# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1872
if getattr(settings, "COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE", False):
old_cookie_in_request = False
if getattr(settings, "COOKIE_NAME_CHANGE_ACTIVATE", False):
alt_cookie_in_request = False
expand_settings = getattr(settings, "COOKIE_NAME_CHANGE_EXPAND_INFO", None)
if (
expand_settings is not None
and isinstance(expand_settings, dict)
and "new_name" in expand_settings
and "old_name" in expand_settings
and "current" in expand_settings
and "alternate" in expand_settings
):
if expand_settings["old_name"] in request.COOKIES:
old_cookie_in_request = True
old_cookie_value = request.COOKIES[expand_settings["old_name"]]
del request.COOKIES[expand_settings["old_name"]]
if expand_settings["alternate"] in request.COOKIES:
alt_cookie_in_request = True
alt_cookie_value = request.COOKIES[expand_settings["alternate"]]
del request.COOKIES[expand_settings["alternate"]]
if (
expand_settings["new_name"] not in request.COOKIES
and old_cookie_in_request
expand_settings["current"] not in request.COOKIES
and alt_cookie_in_request
):
request.COOKIES[expand_settings["new_name"]] = old_cookie_value
request.COOKIES[expand_settings["current"]] = alt_cookie_value
response = self.get_response(request)
return response

30
openedx/core/djangoapps/cookie_metadata/tests/test_cookie_name_change.py
View File

@@ -32,8 +32,8 @@ class TestCookieNameChange(TestCase):
}
self.expand_settings = {
"old_name": self.old_key,
"new_name": "b",
"alternate": self.old_key,
"current": "b",
}
def test_cookie_swap(self):
@@ -44,26 +44,26 @@ class TestCookieNameChange(TestCase):
self.mock_request.COOKIES = self.old_dict.copy()
with self.settings(
COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE=True
COOKIE_NAME_CHANGE_ACTIVATE=True
), self.settings(COOKIE_NAME_CHANGE_EXPAND_INFO=self.expand_settings):
self.cookie_name_change_middleware(self.mock_request)
assert self.expand_settings["old_name"] not in self.mock_request.COOKIES.keys()
assert self.expand_settings["new_name"] in self.mock_request.COOKIES.keys()
assert self.mock_request.COOKIES[self.expand_settings["new_name"]] == self.old_value
assert self.expand_settings["alternate"] not in self.mock_request.COOKIES.keys()
assert self.expand_settings["current"] in self.mock_request.COOKIES.keys()
assert self.mock_request.COOKIES[self.expand_settings["current"]] == self.old_value
test_dict = self.extra_cookies.copy()
test_dict[self.expand_settings['new_name']] = self.old_value
test_dict[self.expand_settings['current']] = self.old_value
assert self.mock_request.COOKIES == test_dict
# make sure response function is called once
self.mock_response.assert_called_once()
def test_cookie_no_swap(self):
"""Make sure self.cookie_name_change_middleware does not change cookie if new_name cookie is already present"""
"""Make sure self.cookie_name_change_middleware does not change cookie if current cookie is already present"""
new_value = "." * 13
no_change_cookies = {
self.expand_settings['new_name']: new_value,
self.expand_settings['current']: new_value,
"_c_": "." * 13,
"a.b": "." * 10,
}
@@ -73,13 +73,13 @@ class TestCookieNameChange(TestCase):
self.mock_request.COOKIES = self.old_dict.copy()
with self.settings(
COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE=True
COOKIE_NAME_CHANGE_ACTIVATE=True
), self.settings(COOKIE_NAME_CHANGE_EXPAND_INFO=self.expand_settings):
self.cookie_name_change_middleware(self.mock_request)
assert self.expand_settings["old_name"] not in self.mock_request.COOKIES.keys()
assert self.expand_settings["new_name"] in self.mock_request.COOKIES.keys()
assert self.mock_request.COOKIES[self.expand_settings["new_name"]] == new_value
assert self.expand_settings["alternate"] not in self.mock_request.COOKIES.keys()
assert self.expand_settings["current"] in self.mock_request.COOKIES.keys()
assert self.mock_request.COOKIES[self.expand_settings["current"]] == new_value
assert self.mock_request.COOKIES == no_change_cookies
# make sure response function is called once
@@ -90,7 +90,7 @@ class TestCookieNameChange(TestCase):
new_value = "." * 13
no_change_cookies = {
self.expand_settings['new_name']: new_value,
self.expand_settings['current']: new_value,
"_c_": "." * 13,
"a.b": "." * 10,
}
@@ -99,7 +99,7 @@ class TestCookieNameChange(TestCase):
self.mock_request.COOKIES = self.old_dict.copy()
with self.settings(
COOKIE_NAME_CHANGE_ACTIVATE_EXPAND_PHASE=False
COOKIE_NAME_CHANGE_ACTIVATE=False
), self.settings(COOKIE_NAME_CHANGE_EXPAND_INFO=self.expand_settings):
self.cookie_name_change_middleware(self.mock_request)