Skip to content

Commit c5bd3f3

Browse files
AgrendalathAgrendalath
authored
Merge pull request #607 from open-craft/sync-open-release/palm.master-20231120-1700438846
Sync opencraft-release/palm.1 with Upstream 20231120-1700438846
2 parents 4df908a + cf6e266 commit c5bd3f3

File tree

6 files changed

+136
-21
lines changed

6 files changed

+136
-21
lines changed

‎cms/envs/common.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@
11011101
}
11021102

11031103
DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
1104-
DEFAULT_HASHING_ALGORITHM = 'sha1'
1104+
DEFAULT_HASHING_ALGORITHM = 'sha256'
11051105

11061106
#################### Python sandbox ############################################
11071107

‎lms/envs/common.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1714,7 +1714,7 @@ def _make_mako_template_dirs(settings):
17141714

17151715

17161716
DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
1717-
DEFAULT_HASHING_ALGORITHM = 'sha1'
1717+
DEFAULT_HASHING_ALGORITHM = 'sha256'
17181718

17191719
#################### Python sandbox ############################################
17201720

‎openedx/core/djangoapps/cache_toolbox/middleware.py‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
9696
from django.utils.crypto import constant_time_compare
9797
from django.utils.deprecation import MiddlewareMixin
98+
from edx_django_utils.monitoring import set_custom_attribute
9899

99100
from openedx.core.djangoapps.safe_sessions.middleware import SafeSessionMiddleware, _mark_cookie_for_deletion
100101

@@ -112,6 +113,7 @@ def __init__(self, *args, **kwargs):
112113
super().__init__(*args, **kwargs)
113114

114115
def process_request(self, request):
116+
set_custom_attribute('DEFAULT_HASHING_ALGORITHM', settings.DEFAULT_HASHING_ALGORITHM)
115117
try:
116118
# Try and construct a User instance from data stored in the cache
117119
session_user_id = SafeSessionMiddleware.get_user_id_from_session(request)
@@ -141,9 +143,29 @@ def _verify_session_auth(self, request):
141143
auto_auth_enabled = settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING', False)
142144
if not auto_auth_enabled and hasattr(request.user, 'get_session_auth_hash'):
143145
session_hash = request.session.get(HASH_SESSION_KEY)
144-
if not (session_hash and constant_time_compare(session_hash, request.user.get_session_auth_hash())):
145-
# The session hash has changed due to a password
146-
# change. Log the user out.
147-
request.session.flush()
148-
request.user = AnonymousUser()
149-
_mark_cookie_for_deletion(request)
146+
session_hash_verified = session_hash and constant_time_compare(
147+
session_hash, request.user.get_session_auth_hash())
148+
149+
# session hash is verified from the default algo, so skip legacy check
150+
if session_hash_verified:
151+
set_custom_attribute('session_hash_verified', "default")
152+
return
153+
154+
if (
155+
session_hash and
156+
hasattr(request.user, '_legacy_get_session_auth_hash') and
157+
constant_time_compare(
158+
session_hash,
159+
request.user._legacy_get_session_auth_hash() # pylint: disable=protected-access
160+
)
161+
):
162+
# session hash is verified from legacy hashing algorithm.
163+
set_custom_attribute('session_hash_verified', "fallback")
164+
return
165+
166+
# The session hash has changed due to a password
167+
# change. Log the user out.
168+
request.session.flush()
169+
request.user = AnonymousUser()
170+
_mark_cookie_for_deletion(request)
171+
set_custom_attribute('failed_session_verification', True)

‎openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py‎

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
"""Tests for cached authentication middleware."""
2-
from unittest.mock import patch
2+
from unittest.mock import call, patch
33

4+
import django
45
from django.conf import settings
5-
from django.contrib.auth.models import User, AnonymousUser # lint-amnesty, pylint: disable=imported-auth-user
6-
from django.urls import reverse
7-
from django.test import TestCase
86
from django.contrib.auth import SESSION_KEY
7+
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
98
from django.http import HttpResponse, SimpleCookie
9+
from django.test import TestCase
10+
from django.urls import reverse
1011

12+
from common.djangoapps.student.tests.factories import UserFactory
1113
from openedx.core.djangoapps.cache_toolbox.middleware import CacheBackedAuthenticationMiddleware
1214
from openedx.core.djangoapps.safe_sessions.middleware import SafeCookieData, SafeSessionMiddleware
13-
from openedx.core.djangolib.testing.utils import skip_unless_cms, skip_unless_lms, get_mock_request
14-
from common.djangoapps.student.tests.factories import UserFactory
15+
from openedx.core.djangolib.testing.utils import get_mock_request, skip_unless_cms, skip_unless_lms
1516

1617

1718
class CachedAuthMiddlewareTestCase(TestCase):
@@ -36,9 +37,68 @@ def _test_change_session_hash(self, test_url, redirect_url, target_status_code=2
3637
"""
3738
response = self.client.get(test_url)
3839
assert response.status_code == 200
39-
with patch.object(User, 'get_session_auth_hash', return_value='abc123'):
40-
response = self.client.get(test_url)
41-
self.assertRedirects(response, redirect_url, target_status_code=target_status_code)
40+
41+
with patch(
42+
"openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute"
43+
) as mock_set_custom_attribute:
44+
with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True):
45+
# Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not
46+
# Remove once we reach Django 4
47+
if hasattr(User, '_legacy_get_session_auth_hash'):
48+
with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'):
49+
response = self.client.get(test_url)
50+
else:
51+
response = self.client.get(test_url)
52+
53+
self.assertRedirects(response, redirect_url, target_status_code=target_status_code)
54+
mock_set_custom_attribute.assert_any_call('failed_session_verification', True)
55+
56+
def _test_custom_attribute_after_changing_hash(self, test_url, mock_set_custom_attribute):
57+
"""verify that set_custom_attribute is called with expected values"""
58+
password = 'test-password'
59+
60+
# Test DEFAULT_HASHING_ALGORITHM of 'sha1' for both login and client get
61+
with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'):
62+
self.client.login(username=self.user.username, password=password)
63+
self.client.get(test_url)
64+
# For Django 3.2, the setting 'sha1' applies and is the "default".
65+
# For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default".
66+
mock_set_custom_attribute.assert_has_calls([
67+
call('DEFAULT_HASHING_ALGORITHM', 'sha1'),
68+
call('session_hash_verified', "default"),
69+
])
70+
mock_set_custom_attribute.reset_mock()
71+
72+
# Test DEFAULT_HASHING_ALGORITHM of 'sha1' for login and switch to 'sha256' for client get.
73+
with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'):
74+
self.client.login(username=self.user.username, password=password)
75+
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
76+
self.client.get(test_url)
77+
if django.VERSION < (4, 0):
78+
# For Django 3.2, the setting 'sha1' applies to login, and uses 'she256' for client get,
79+
# and should "fallback" to 'sha1".
80+
mock_set_custom_attribute.assert_has_calls([
81+
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
82+
call('session_hash_verified', "fallback"),
83+
])
84+
else:
85+
# For Django 4, the setting no longer applies, and again 'sha256' will be used for both as the "default".
86+
mock_set_custom_attribute.assert_has_calls([
87+
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
88+
call('session_hash_verified', "default"),
89+
])
90+
mock_set_custom_attribute.reset_mock()
91+
92+
# Test DEFAULT_HASHING_ALGORITHM of 'sha256' for both login and client get
93+
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
94+
self.client.login(username=self.user.username, password=password)
95+
self.client.get(test_url)
96+
# For Django 3.2, the setting 'sha256' applies and is the "default".
97+
# For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default".
98+
mock_set_custom_attribute.assert_has_calls([
99+
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
100+
call('session_hash_verified', "default"),
101+
])
42102

43103
@skip_unless_lms
44104
def test_session_change_lms(self):
@@ -53,6 +113,20 @@ def test_session_change_cms(self):
53113
# Studio login redirects to LMS login
54114
self._test_change_session_hash(home_url, settings.LOGIN_URL + '?next=' + home_url, target_status_code=302)
55115

116+
@skip_unless_lms
117+
@patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute")
118+
def test_custom_attribute_after_changing_hash_lms(self, mock_set_custom_attribute):
119+
"""Test set_custom_attribute is called with expected values in LMS"""
120+
test_url = reverse('dashboard')
121+
self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute)
122+
123+
@skip_unless_cms
124+
@patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute")
125+
def test_custom_attribute_after_changing_hash_cms(self, mock_set_custom_attribute):
126+
"""Test set_custom_attribute is called with expected values in CMS"""
127+
test_url = reverse('home')
128+
self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute)
129+
56130
def test_user_logout_on_session_hash_change(self):
57131
"""
58132
Verify that if a user's session auth hash and the request's hash
@@ -75,9 +149,18 @@ def test_user_logout_on_session_hash_change(self):
75149
assert self.client.response.cookies.get(settings.SESSION_COOKIE_NAME).value == session_id
76150
assert self.client.response.cookies.get('edx-jwt-cookie-header-payload').value == 'test-jwt-payload'
77151

78-
with patch.object(User, 'get_session_auth_hash', return_value='abc123'):
79-
CacheBackedAuthenticationMiddleware().process_request(self.request)
80-
SafeSessionMiddleware().process_response(self.request, self.client.response)
152+
with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True):
153+
# Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not
154+
# Remove once we reach Django 4
155+
if hasattr(User, '_legacy_get_session_auth_hash'):
156+
with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'):
157+
CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request)
158+
159+
else:
160+
CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request)
161+
SafeSessionMiddleware(get_response=lambda request: None).process_response(
162+
self.request, self.client.response
163+
)
81164

82165
# asserts that user, session, and JWT cookies do not exist
83166
assert self.request.session.get(SESSION_KEY) is None

‎openedx/core/djangoapps/safe_sessions/tests/test_middleware.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,16 @@ def test_update_cookie_data_at_step_3(self):
223223
assert safe_cookie_data.session_id == 'some_session_id'
224224
assert safe_cookie_data.verify(self.user.id)
225225

226+
def test_update_cookie_data_at_step_3_with_sha256(self):
227+
""" first encode cookie with default algo sha1 and then check with sha256"""
228+
self.assert_response(set_request_user=True, set_session_cookie=True)
229+
serialized_cookie_data = self.client.response.cookies[settings.SESSION_COOKIE_NAME].value
230+
safe_cookie_data = SafeCookieData.parse(serialized_cookie_data)
231+
assert safe_cookie_data.version == SafeCookieData.CURRENT_VERSION
232+
assert safe_cookie_data.session_id == 'some_session_id'
233+
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
234+
assert safe_cookie_data.verify(self.user.id)
235+
226236
def test_cant_update_cookie_at_step_3_error(self):
227237
self.client.response.cookies[settings.SESSION_COOKIE_NAME] = None
228238
with self.assert_invalid_session_id():

‎openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,5 +234,5 @@ def test_pinned_values(self):
234234
"|HvGnjXf1b3jU"
235235
"|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi"
236236
":1m6Hve"
237-
":OMhY2FL2pudJjSSXChtI-zR8QVA"
237+
":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY"
238238
)

0 commit comments

Comments
 (0)