From 64eba521f8d9e2e1542e84110f72ceb082fe3050 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Thu, 5 Oct 2023 15:56:55 +0500 Subject: [PATCH] Revert "[django42] sha1 is removed in django42 version. " (#33213) * Revert "[django42] sha1 is removed in django42 version. (#33129)" --- .../djangoapps/safe_sessions/middleware.py | 10 ++---- .../tests/test_safe_cookie_data.py | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index a9ade2ac3b95..5f4449d93c42 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -267,10 +267,7 @@ def sign(self, user_id): SHA256(version '|' session_id '|' user_id '|'). """ data_to_sign = self._compute_digest(user_id) - - self.signature = signing.TimestampSigner( - salt=self.key_salt, algorithm=settings.DEFAULT_HASHING_ALGORITHM - ).sign_object(data_to_sign, serializer=signing.JSONSerializer, compress=False) + self.signature = signing.dumps(data_to_sign, salt=self.key_salt) def verify(self, user_id): """ @@ -279,10 +276,7 @@ def verify(self, user_id): (not expired) and bound to the given user. """ try: - unsigned_data = signing.TimestampSigner( - salt=self.key_salt, algorithm=settings.DEFAULT_HASHING_ALGORITHM - ).unsign_object(self.signature, serializer=signing.JSONSerializer, max_age=settings.SESSION_COOKIE_AGE) - + unsigned_data = signing.loads(self.signature, salt=self.key_salt, max_age=settings.SESSION_COOKIE_AGE) if unsigned_data == self._compute_digest(user_id): return True log.error("SafeCookieData '%r' is not bound to user '%s'.", str(self), user_id) diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py index b8a23c567d24..47efa626ec99 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py @@ -10,6 +10,7 @@ import pytest import ddt +import django from django.test import TestCase from ..middleware import SafeCookieData, SafeCookieError @@ -204,6 +205,7 @@ def test_digest_incorrect_field_value(self, field_name, incorrect_field_value): #---- Test roundtrip with pinned values ----# + @pytest.mark.skipif(django.VERSION[0] >= 4, reason="For django32 default algorithm is sha1. No need for django42.") def test_pinned_values(self): """ Compute a cookie with all inputs held constant and assert that the @@ -236,3 +238,37 @@ def test_pinned_values(self): ":1m6Hve" ":OMhY2FL2pudJjSSXChtI-zR8QVA" ) + + @pytest.mark.skipif(django.VERSION[0] < 4, reason="For django42 default algorithm is sha256. No need for django32.") + def test_pinned_values_django42(self): + """ + Compute a cookie with all inputs held constant and assert that the + exact output never changes. This protects against unintentional + changes to the algorithm. + """ + user_id = '8523' + session_id = 'SSdtIGEgc2Vzc2lvbiE' + a_random_string = 'HvGnjXf1b3jU' + timestamp = 1626895850 + + module = 'openedx.core.djangoapps.safe_sessions.middleware' + with patch(f"{module}.signing.time.time", return_value=timestamp): + with patch(f"{module}.get_random_string", return_value=a_random_string): + safe_cookie_data = SafeCookieData.create(session_id, user_id) + serialized_value = str(safe_cookie_data) + + # **IMPORTANT**: If a change to the algorithm causes this test + # to start failing, you will either need to allow both the old + # and new format or all users will become logged out upon + # deploy of the changes. + # + # Also assumes SECRET_KEY is '85920908f28904ed733fe576320db18cabd7b6cd' + # (set in lms or cms.envs.test) + assert serialized_value == ( + "1" + "|SSdtIGEgc2Vzc2lvbiE" + "|HvGnjXf1b3jU" + "|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi" + ":1m6Hve" + ":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY" + )