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

Adding fall back method for sha1 in case default algo is sha256 #33345

Merged
merged 4 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -1085,8 +1085,8 @@
}

DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
# This will be overridden through CMS config
DEFAULT_HASHING_ALGORITHM = 'sha1'

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

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


DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
# This will be overridden through LMS config
DEFAULT_HASHING_ALGORITHM = 'sha1'

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

CODE_JAIL = {
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")
robrap marked this conversation as resolved.
Show resolved Hide resolved
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)
67 changes: 56 additions & 11 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
"""Tests for cached authentication middleware for django32."""
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,10 +37,33 @@ 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)
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)

def _test_custom_attribute_after_changing_hash(self, test_url, mock_set_custom_attribute):
"""verify that set_custom_attribute is called with expected values"""
response = self.client.get(test_url)
mock_set_custom_attribute.assert_has_calls([
robrap marked this conversation as resolved.
Show resolved Hide resolved
call('DEFAULT_HASHING_ALGORITHM', settings.DEFAULT_HASHING_ALGORITHM),
call('session_hash_verified', "default"),
])
if django.VERSION < (4, 0):
# Reset for testing with change algo, do only for Django 3.2
mock_set_custom_attribute.reset_mock()
with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'):
response = self.client.get(test_url)
mock_set_custom_attribute.assert_has_calls([
call('DEFAULT_HASHING_ALGORITHM', "sha256"),
call('session_hash_verified', "fallback"),
])

@skip_unless_lms
def test_session_change_lms(self):
"""Test session verification with LMS-specific URLs."""
Expand All @@ -53,6 +77,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,8 +113,15 @@ 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(get_response=lambda request: None).process_request(self.request)
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
)
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 @@ -225,6 +225,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
Loading