Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync opencraft-release/palm.1 with Upstream 20231120-1700438846 #607

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@
}

DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
DEFAULT_HASHING_ALGORITHM = 'sha1'
DEFAULT_HASHING_ALGORITHM = 'sha256'

#################### Python sandbox ############################################

Expand Down
2 changes: 1 addition & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1671,7 +1671,7 @@ def _make_mako_template_dirs(settings):


DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
DEFAULT_HASHING_ALGORITHM = 'sha1'
DEFAULT_HASHING_ALGORITHM = 'sha256'

#################### Python sandbox ############################################

Expand Down
34 changes: 28 additions & 6 deletions openedx/core/djangoapps/cache_toolbox/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.utils.crypto import constant_time_compare
from django.utils.deprecation import MiddlewareMixin
from edx_django_utils.monitoring import set_custom_attribute

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

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

def process_request(self, request):
set_custom_attribute('DEFAULT_HASHING_ALGORITHM', settings.DEFAULT_HASHING_ALGORITHM)
try:
# Try and construct a User instance from data stored in the cache
session_user_id = SafeSessionMiddleware.get_user_id_from_session(request)
Expand Down Expand Up @@ -141,9 +143,29 @@ def _verify_session_auth(self, request):
auto_auth_enabled = settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING', False)
if not auto_auth_enabled and hasattr(request.user, 'get_session_auth_hash'):
session_hash = request.session.get(HASH_SESSION_KEY)
if not (session_hash and constant_time_compare(session_hash, request.user.get_session_auth_hash())):
# The session hash has changed due to a password
# change. Log the user out.
request.session.flush()
request.user = AnonymousUser()
_mark_cookie_for_deletion(request)
session_hash_verified = session_hash and constant_time_compare(
session_hash, request.user.get_session_auth_hash())

# session hash is verified from the default algo, so skip legacy check
if session_hash_verified:
set_custom_attribute('session_hash_verified', "default")
return

if (
session_hash and
hasattr(request.user, '_legacy_get_session_auth_hash') and
constant_time_compare(
session_hash,
request.user._legacy_get_session_auth_hash() # pylint: disable=protected-access
)
):
# session hash is verified from legacy hashing algorithm.
set_custom_attribute('session_hash_verified', "fallback")
return

# The session hash has changed due to a password
# change. Log the user out.
request.session.flush()
request.user = AnonymousUser()
_mark_cookie_for_deletion(request)
set_custom_attribute('failed_session_verification', True)
107 changes: 95 additions & 12 deletions openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
"""Tests for cached authentication middleware."""
from unittest.mock import patch
from unittest.mock import call, patch

import django
from django.conf import settings
from django.contrib.auth.models import User, AnonymousUser # lint-amnesty, pylint: disable=imported-auth-user
from django.urls import reverse
from django.test import TestCase
from django.contrib.auth import SESSION_KEY
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.http import HttpResponse, SimpleCookie
from django.test import TestCase
from django.urls import reverse

from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.cache_toolbox.middleware import CacheBackedAuthenticationMiddleware
from openedx.core.djangoapps.safe_sessions.middleware import SafeCookieData, SafeSessionMiddleware
from openedx.core.djangolib.testing.utils import skip_unless_cms, skip_unless_lms, get_mock_request
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangolib.testing.utils import get_mock_request, skip_unless_cms, skip_unless_lms


class CachedAuthMiddlewareTestCase(TestCase):
Expand All @@ -36,9 +37,68 @@ def _test_change_session_hash(self, test_url, redirect_url, target_status_code=2
"""
response = self.client.get(test_url)
assert response.status_code == 200
with patch.object(User, 'get_session_auth_hash', return_value='abc123'):
response = self.client.get(test_url)
self.assertRedirects(response, redirect_url, target_status_code=target_status_code)

with patch(
"openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute"
) as mock_set_custom_attribute:
with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True):
# Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not
# Remove once we reach Django 4
if hasattr(User, '_legacy_get_session_auth_hash'):
with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'):
response = self.client.get(test_url)
else:
response = self.client.get(test_url)

self.assertRedirects(response, redirect_url, target_status_code=target_status_code)
mock_set_custom_attribute.assert_any_call('failed_session_verification', True)

def _test_custom_attribute_after_changing_hash(self, test_url, mock_set_custom_attribute):
"""verify that set_custom_attribute is called with expected values"""
password = 'test-password'

# Test DEFAULT_HASHING_ALGORITHM of 'sha1' for both login and client get
with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'):
self.client.login(username=self.user.username, password=password)
self.client.get(test_url)
# For Django 3.2, the setting 'sha1' applies and is the "default".
# For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default".
mock_set_custom_attribute.assert_has_calls([
call('DEFAULT_HASHING_ALGORITHM', 'sha1'),
call('session_hash_verified', "default"),
])
mock_set_custom_attribute.reset_mock()

# Test DEFAULT_HASHING_ALGORITHM of 'sha1' for login and switch to 'sha256' for client get.
with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'):
self.client.login(username=self.user.username, password=password)
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
self.client.get(test_url)
if django.VERSION < (4, 0):
# For Django 3.2, the setting 'sha1' applies to login, and uses 'she256' for client get,
# and should "fallback" to 'sha1".
mock_set_custom_attribute.assert_has_calls([
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
call('session_hash_verified', "fallback"),
])
else:
# For Django 4, the setting no longer applies, and again 'sha256' will be used for both as the "default".
mock_set_custom_attribute.assert_has_calls([
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
call('session_hash_verified', "default"),
])
mock_set_custom_attribute.reset_mock()

# Test DEFAULT_HASHING_ALGORITHM of 'sha256' for both login and client get
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
self.client.login(username=self.user.username, password=password)
self.client.get(test_url)
# For Django 3.2, the setting 'sha256' applies and is the "default".
# For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default".
mock_set_custom_attribute.assert_has_calls([
call('DEFAULT_HASHING_ALGORITHM', 'sha256'),
call('session_hash_verified', "default"),
])

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

@skip_unless_lms
@patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute")
def test_custom_attribute_after_changing_hash_lms(self, mock_set_custom_attribute):
"""Test set_custom_attribute is called with expected values in LMS"""
test_url = reverse('dashboard')
self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute)

@skip_unless_cms
@patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute")
def test_custom_attribute_after_changing_hash_cms(self, mock_set_custom_attribute):
"""Test set_custom_attribute is called with expected values in CMS"""
test_url = reverse('home')
self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute)

def test_user_logout_on_session_hash_change(self):
"""
Verify that if a user's session auth hash and the request's hash
Expand All @@ -75,9 +149,18 @@ def test_user_logout_on_session_hash_change(self):
assert self.client.response.cookies.get(settings.SESSION_COOKIE_NAME).value == session_id
assert self.client.response.cookies.get('edx-jwt-cookie-header-payload').value == 'test-jwt-payload'

with patch.object(User, 'get_session_auth_hash', return_value='abc123'):
CacheBackedAuthenticationMiddleware().process_request(self.request)
SafeSessionMiddleware().process_response(self.request, self.client.response)
with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True):
# Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not
# Remove once we reach Django 4
if hasattr(User, '_legacy_get_session_auth_hash'):
with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'):
CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request)

else:
CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request)
SafeSessionMiddleware(get_response=lambda request: None).process_response(
self.request, self.client.response
)

# asserts that user, session, and JWT cookies do not exist
assert self.request.session.get(SESSION_KEY) is None
Expand Down
10 changes: 10 additions & 0 deletions openedx/core/djangoapps/safe_sessions/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,16 @@ def test_update_cookie_data_at_step_3(self):
assert safe_cookie_data.session_id == 'some_session_id'
assert safe_cookie_data.verify(self.user.id)

def test_update_cookie_data_at_step_3_with_sha256(self):
""" first encode cookie with default algo sha1 and then check with sha256"""
self.assert_response(set_request_user=True, set_session_cookie=True)
serialized_cookie_data = self.client.response.cookies[settings.SESSION_COOKIE_NAME].value
safe_cookie_data = SafeCookieData.parse(serialized_cookie_data)
assert safe_cookie_data.version == SafeCookieData.CURRENT_VERSION
assert safe_cookie_data.session_id == 'some_session_id'
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
assert safe_cookie_data.verify(self.user.id)

def test_cant_update_cookie_at_step_3_error(self):
self.client.response.cookies[settings.SESSION_COOKIE_NAME] = None
with self.assert_invalid_session_id():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,5 +234,5 @@ def test_pinned_values(self):
"|HvGnjXf1b3jU"
"|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi"
":1m6Hve"
":OMhY2FL2pudJjSSXChtI-zR8QVA"
":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY"
)
Loading