diff --git a/common/djangoapps/student/signals/receivers.py b/common/djangoapps/student/signals/receivers.py index 6d0cf4f061ca..079647c1451e 100644 --- a/common/djangoapps/student/signals/receivers.py +++ b/common/djangoapps/student/signals/receivers.py @@ -23,6 +23,7 @@ ) from common.djangoapps.student.models_api import confirm_name_change from common.djangoapps.student.signals import USER_EMAIL_CHANGED +from openedx.core.djangoapps.safe_sessions.middleware import EmailChangeMiddleware from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed logger = logging.getLogger(__name__) @@ -105,8 +106,12 @@ def listen_for_verified_name_approved(sender, user_id, profile_name, **kwargs): @receiver(USER_EMAIL_CHANGED) -def _listen_for_user_email_changed(sender, user, **kwargs): - """ If user has changed their email, update that in email Braze. """ +def _listen_for_user_email_changed(sender, user, request, **kwargs): + """ If user has changed their email, update that in session and Braze profile. """ + + # Store the user's email for session consistency (used by EmailChangeMiddleware) + EmailChangeMiddleware.register_email_change(request, user.email) + email = user.email user_id = user.id attributes = [{'email': email, 'external_id': user_id}] diff --git a/common/djangoapps/student/tests/test_receivers.py b/common/djangoapps/student/tests/test_receivers.py index 8ce869a73198..e987120a9d67 100644 --- a/common/djangoapps/student/tests/test_receivers.py +++ b/common/djangoapps/student/tests/test_receivers.py @@ -9,7 +9,7 @@ from common.djangoapps.student.signals.signals import USER_EMAIL_CHANGED from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory, UserProfileFactory from lms.djangoapps.courseware.toggles import COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES -from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.core.djangolib.testing.utils import skip_unless_lms, get_mock_request from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order @@ -75,10 +75,18 @@ def test_listen_for_verified_name_approved(self): @patch('common.djangoapps.student.signals.receivers.get_braze_client') def test_listen_for_user_email_changed(self, mock_get_braze_client): """ - Ensure that USER_EMAIL_CHANGED signal triggers correct calls to get_braze_client. + Ensure that USER_EMAIL_CHANGED signal triggers correct calls to + get_braze_client and update email in session. """ user = UserFactory(email='email@test.com', username='jdoe') + request = get_mock_request(user=user) + request.session = self.client.session - USER_EMAIL_CHANGED.send(sender=None, user=user) + # simulating email change + user.email = 'new_email@test.com' + user.save() + + USER_EMAIL_CHANGED.send(sender=None, user=user, request=request) assert mock_get_braze_client.called + assert request.session.get('email', None) == user.email diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index cb3df1627fd9..d3e1c54180d3 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -910,7 +910,7 @@ def confirm_email_change(request, key): response = render_to_response("email_change_successful.html", address_context) - USER_EMAIL_CHANGED.send(sender=None, user=user) + USER_EMAIL_CHANGED.send(sender=None, user=user, request=request) return response diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index 9e62eab6d837..fd31e3456961 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -309,7 +309,7 @@ def test_no_certificate(self): def test_query_counts(self): # Test student with no certificates student_no_cert = UserFactory.create(password=self.user_password) - with self.assertNumQueries(17, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(21, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -319,7 +319,7 @@ def test_query_counts(self): assert len(resp.data) == 0 # Test student with 1 certificate - with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -359,7 +359,7 @@ def test_query_counts(self): download_url='www.google.com', grade="0.88", ) - with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index c39f808478ef..4065c54a983b 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -434,7 +434,7 @@ def test_too_many_courses(self): self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [50, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] + query_counts = [53, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) self.clear_caches() diff --git a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py index 74b66fa5f068..4b3524eb1ee2 100644 --- a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py @@ -86,6 +86,7 @@ def test_get_masqueraded_user(self): assert self.client.get(self.url).data['username'] == self.user.username def test_get_unknown_course(self): + self.client.logout() url = reverse('course-home:course-metadata', args=['course-v1:unknown+course+2T2020']) # Django TestCase wraps every test in a transaction, so we must specifically wrap this when we expect an error with transaction.atomic(): diff --git a/lms/envs/common.py b/lms/envs/common.py index 94d298cddbd1..b567b20a37f1 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2238,6 +2238,9 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring #'django.contrib.auth.middleware.AuthenticationMiddleware', 'openedx.core.djangoapps.cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + # Middleware to flush user's session in other browsers when their email is changed. + 'openedx.core.djangoapps.safe_sessions.middleware.EmailChangeMiddleware', + 'common.djangoapps.student.middleware.UserStandingMiddleware', 'openedx.core.djangoapps.contentserver.middleware.StaticContentServer', @@ -5041,6 +5044,20 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-838 ENABLE_DYNAMIC_REGISTRATION_FIELDS = False +############## Settings for EmailChangeMiddleware ############### + +# .. toggle_name: ENFORCE_SESSION_EMAIL_MATCH +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When enabled, this setting invalidates sessions in other browsers +# upon email change, while preserving the session validity in the browser where the +# email change occurs. This toggle is just being used for rollout. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2023-12-07 +# .. toggle_target_removal_date: 2024-04-01 +# .. toggle_tickets: https://2u-internal.atlassian.net/browse/VAN-1797 +ENFORCE_SESSION_EMAIL_MATCH = False + LEARNER_HOME_MFE_REDIRECT_PERCENTAGE = 0 ############### Settings for the ace_common plugin ################# diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index 5f4449d93c42..f3948217efd9 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -95,7 +95,7 @@ from edx_django_utils.monitoring import set_custom_attribute from edx_toggles.toggles import SettingToggle -from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies +from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies, set_logged_in_cookies from openedx.core.lib.mobile_utils import is_request_from_mobile_app # .. toggle_name: LOG_REQUEST_USER_CHANGES @@ -768,6 +768,92 @@ def _get_encrypted_request_headers(request): return encrypt_for_log(str(request.headers), getattr(settings, 'SAFE_SESSIONS_DEBUG_PUBLIC_KEY', None)) +class EmailChangeMiddleware(MiddlewareMixin): + """ + Middleware responsible for performing the following + jobs on detecting an email change + 1) It will update the session's email and update the JWT cookie + to match the new email. + 2) It will invalidate any future session on other browsers where + the user's email does not match its session email. + + This middleware ensures that the sessions in other browsers + are invalidated when a user changes their email in one browser. + The active session in which the email change is made will remain valid. + + The user's email is stored in their session and JWT cookies during login + and gets updated when the user changes their email. + This middleware checks for any mismatch between the stored email + and the current user's email in each request, and if found, + it invalidates/flushes the session and mark cookies for deletion in request + which are then deleted in the process_response of SafeSessionMiddleware. + """ + + def process_request(self, request): + """ + Invalidate the user session if there's a mismatch + between the email in the user's session and request.user.email. + """ + if request.user.is_authenticated: + user_session_email = request.session.get('email', None) + are_emails_mismatched = user_session_email is not None and request.user.email != user_session_email + EmailChangeMiddleware._set_session_email_match_custom_attributes(are_emails_mismatched) + if settings.ENFORCE_SESSION_EMAIL_MATCH and are_emails_mismatched: + # Flush the session and mark cookies for deletion. + log.info( + f'EmailChangeMiddleware invalidating session for user: {request.user.id} due to email mismatch.' + ) + request.session.flush() + request.user = AnonymousUser() + _mark_cookie_for_deletion(request) + + def process_response(self, request, response): + """ + 1. Update the logged-in cookies if the email change was requested + 2. Store user's email in session if not already + """ + if request.user.is_authenticated: + if request.session.get('email', None) is None: + # .. custom_attribute_name: session_with_no_email_found + # .. custom_attribute_description: Indicates that user's email was not + # yet stored in the user's session. + set_custom_attribute('session_with_no_email_found', True) + request.session['email'] = request.user.email + + if request_cache.get_cached_response('email_change_requested').is_found: + # Update the JWT cookies with new user email + response = set_logged_in_cookies(request, response, request.user) + + return response + + @staticmethod + def register_email_change(request, email): + """ + Stores the fact that an email change happened. + + 1. Sets the email in session for later comparison. + 2. Sets a request level variable to mark that the user email change was requested. + """ + request.session['email'] = email + request_cache.set('email_change_requested', True) + + @staticmethod + def _set_session_email_match_custom_attributes(are_emails_mismatched): + """ + Sets custom attributes of session_email_match + """ + # .. custom_attribute_name: session_email_match + # .. custom_attribute_description: Indicates whether there is a match between the + # email in the user's session and the current user's email in the request. + set_custom_attribute('session_email_mismatch', are_emails_mismatched) + + # .. custom_attribute_name: is_enforce_session_email_match_enabled + # .. custom_attribute_description: Indicates whether session email match was enforced. + # When enforced/enabled, it invalidates sessions in other browsers upon email change, + # while preserving the session validity in the browser where the email change occurs. + set_custom_attribute('is_enforce_session_email_match_enabled', settings.ENFORCE_SESSION_EMAIL_MATCH) + + def obscure_token(value: Union[str, None]) -> Union[str, None]: """ Return a short string that can be used to detect other occurrences diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py index 079cdcdd0c71..0ffb1aeb8e59 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py @@ -1,22 +1,29 @@ """ Unit tests for SafeSessionMiddleware """ - +import uuid from unittest.mock import call, patch, MagicMock import ddt from crum import set_current_request from django.conf import settings from django.contrib.auth import SESSION_KEY -from django.contrib.auth.models import AnonymousUser +from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user from django.http import HttpResponse, HttpResponseRedirect, SimpleCookie from django.test import TestCase from django.test.utils import override_settings - -from openedx.core.djangolib.testing.utils import get_mock_request, CacheIsolationTestCase +from django.urls import reverse +from edx_django_utils.cache import RequestCache +from edx_rest_framework_extensions.auth.jwt import cookies as jwt_cookies + +from common.djangoapps.student.models import PendingEmailChange +from openedx.core.djangolib.testing.utils import get_mock_request, CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client +from openedx.core.djangoapps.user_authn.cookies import ALL_LOGGED_IN_COOKIE_NAMES from common.djangoapps.student.tests.factories import UserFactory from ..middleware import ( + EmailChangeMiddleware, SafeCookieData, SafeSessionMiddleware, mark_user_change_as_expected, @@ -615,3 +622,748 @@ def test_user_change_with_no_ids(self): request.user = object() assert len(request.debug_user_changes) == 2 assert "Changing request user but user has no id." in request.debug_user_changes[1] + + +@skip_unless_lms +class TestEmailChangeMiddleware(TestSafeSessionsLogMixin, TestCase): + """ + Test class for EmailChangeMiddleware + """ + + def setUp(self): + super().setUp() + self.EMAIL = 'test@example.com' + self.PASSWORD = 'Password1234' + self.user = UserFactory.create(email=self.EMAIL, password=self.PASSWORD) + self.addCleanup(set_current_request, None) + self.request = get_mock_request(self.user) + self.request.session = {} + self.client.response = HttpResponse() + self.client.response.cookies = SimpleCookie() + self.addCleanup(RequestCache.clear_all_namespaces) + + self.login_url = reverse("user_api_login_session", kwargs={'api_version': 'v2'}) + self.register_url = reverse("user_api_registration_v2") + self.dashboard_url = reverse('dashboard') + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=False) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + def test_process_request_user_not_authenticated_with_toggle_disabled(self, mock_mark_cookie_for_deletion): + """ + Calls EmailChangeMiddleware.process_request when no user is authenticated + and ENFORCE_SESSION_EMAIL_MATCH toggle is disabled. + Verifies that session and cookies are not affected. + """ + # Unauthenticated User + self.request.user = AnonymousUser() + + # Call process_request without authenticating a user + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Assert that session and cookies are not affected + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + def test_process_request_user_not_authenticated_with_toggle_enabled(self, mock_mark_cookie_for_deletion): + """ + Calls EmailChangeMiddleware.process_request when no user is authenticated + and ENFORCE_SESSION_EMAIL_MATCH toggle is enabled. + Verifies that session and cookies are not affected. + """ + # Unauthenticated User + self.request.user = AnonymousUser() + + # Call process_request without authenticating a user + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Assert that session and cookies are not affected + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute") + def test_process_request_emails_match_with_toggle_enabled( + self, mock_set_custom_attribute, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when user is authenticated, + ENFORCE_SESSION_EMAIL_MATCH is enabled and user session and request email also match. + Verifies that session and cookies are not affected. + """ + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Registering email change (store user's email in session for later comparison by + # process_request function of middleware) + EmailChangeMiddleware.register_email_change(request=self.request, email=self.user.email) + + # Ensure email is set in the session + self.assertEqual(self.request.session.get('email'), self.user.email) + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # No email change occurred in any browser + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Verify that session_email_mismatch and is_enforce_session_email_match_enabled + # custom attributes are set + mock_set_custom_attribute.assert_has_calls([call('session_email_mismatch', False)]) + mock_set_custom_attribute.assert_has_calls([call('is_enforce_session_email_match_enabled', True)]) + + # Assert that the session and cookies are not affected + self.assertEqual(self.request.session.get('email'), self.user.email) + self.assertEqual(len(self.client.response.cookies), 1) + self.assertEqual(self.client.response.cookies[settings.SESSION_COOKIE_NAME].value, 'authenticated') + + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=False) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute") + def test_process_request_emails_match_with_toggle_disabled( + self, mock_set_custom_attribute, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when user is authenticated, + ENFORCE_SESSION_EMAIL_MATCH is disabled and user session and request email match. + Verifies that session and cookies are not affected. + """ + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Registering email change (store user's email in session for later comparison by + # process_request function of middleware) + EmailChangeMiddleware.register_email_change(request=self.request, email=self.user.email) + + # Ensure email is set in the session + self.assertEqual(self.request.session.get('email'), self.user.email) + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # No email change occurred in any browser + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Verify that session_email_mismatch and is_enforce_session_email_match_enabled + # custom attributes are set + mock_set_custom_attribute.assert_has_calls([call('session_email_mismatch', False)]) + mock_set_custom_attribute.assert_has_calls([call('is_enforce_session_email_match_enabled', False)]) + + # Assert that the session and cookies are not affected + self.assertEqual(self.request.session.get('email'), self.user.email) + self.assertEqual(len(self.client.response.cookies), 1) + self.assertEqual(self.client.response.cookies[settings.SESSION_COOKIE_NAME].value, 'authenticated') + + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute") + def test_process_request_emails_mismatch_with_toggle_enabled( + self, mock_set_custom_attribute, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when user is authenticated, + ENFORCE_SESSION_EMAIL_MATCH is enabled and user session and request + email mismatch. (Email was changed in some other browser) + Verifies that session is flushed and cookies are marked for deletion. + """ + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Registering email change (store user's email in session for later comparison by + # process_request function of middleware) + EmailChangeMiddleware.register_email_change(request=self.request, email=self.user.email) + + # Ensure email is set in the session + self.assertEqual(self.request.session.get('email'), self.user.email) + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # simulating email changed in some other browser + self.user.email = 'new_email@test.com' + self.user.save() + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Verify that session_email_mismatch and is_enforce_session_email_match_enabled + # custom attributes are set + mock_set_custom_attribute.assert_has_calls([call('session_email_mismatch', True)]) + mock_set_custom_attribute.assert_has_calls([call('is_enforce_session_email_match_enabled', True)]) + + # Assert that the session is flushed and cookies marked for deletion + mock_mark_cookie_for_deletion.assert_called() + assert self.request.session.get(SESSION_KEY) is None + assert self.request.user == AnonymousUser() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=False) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute") + def test_process_request_emails_mismatch_with_toggle_disabled( + self, mock_set_custom_attribute, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when user is authenticated, + ENFORCE_SESSION_EMAIL_MATCH is disabled and user session and request + email mismatch. (Email was changed in some other browser) + Verifies that session and cookies are not affected. + """ + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Registering email change (store user's email in session for later comparison by + # process_request function of middleware) + EmailChangeMiddleware.register_email_change(request=self.request, email=self.user.email) + + # Ensure email is set in the session + self.assertEqual(self.request.session.get('email'), self.user.email) + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # simulating email changed in some other browser + self.user.email = 'new_email@test.com' + self.user.save() + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Verify that session_email_mismatch and is_enforce_session_email_match_enabled + # custom attributes are set + mock_set_custom_attribute.assert_has_calls([call('session_email_mismatch', True)]) + mock_set_custom_attribute.assert_has_calls([call('is_enforce_session_email_match_enabled', False)]) + + # Assert that the session and cookies are not affected + self.assertNotEqual(self.request.session.get('email'), self.user.email) + self.assertEqual(len(self.client.response.cookies), 1) + self.assertEqual(self.client.response.cookies[settings.SESSION_COOKIE_NAME].value, 'authenticated') + + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + def test_process_request_no_email_change_history_with_toggle_enabled( + self, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when there is no previous + history of an email change and ENFORCE_SESSION_EMAIL_MATCH is enabled + Verifies that existing sessions are not affected. + Test that sessions predating this code are not affected. + """ + # Log in the user (Simulating user logged-in before this code and email was not set in session) + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Ensure there is no email in the session denoting no previous history of email change + self.assertEqual(self.request.session.get('email'), None) + + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # simulating email changed in some other browser + self.user.email = 'new_email@test.com' + self.user.save() + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Assert that the session and cookies are not affected + self.assertEqual(len(self.client.response.cookies), 1) + self.assertEqual(self.client.response.cookies[settings.SESSION_COOKIE_NAME].value, 'authenticated') + + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=False) + @patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion') + def test_process_request_no_email_change_history_with_toggle_disabled( + self, mock_mark_cookie_for_deletion + ): + """ + Calls EmailChangeMiddleware.process_request when there is no previous + history of an email change and ENFORCE_SESSION_EMAIL_MATCH is disabled + Verifies that existing sessions are not affected. + Test that sessions predating this code are not affected. + """ + # Log in the user (Simulating user logged-in before this code and email was not set in session) + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Ensure there is no email in the session denoting no previous history of email change + self.assertEqual(self.request.session.get('email'), None) + + # Ensure session cookie exist + self.assertEqual(len(self.client.response.cookies), 1) + + # simulating email changed in some other browser + self.user.email = 'new_email@test.com' + self.user.save() + + # Call process_request + EmailChangeMiddleware(get_response=lambda request: None).process_request(self.request) + + # Assert that the session and cookies are not affected + self.assertEqual(len(self.client.response.cookies), 1) + self.assertEqual(self.client.response.cookies[settings.SESSION_COOKIE_NAME].value, 'authenticated') + + # Assert that _mark_cookie_for_deletion not called + mock_mark_cookie_for_deletion.assert_not_called() + + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_logged_in_cookies") + def test_process_response_user_not_authenticated(self, mock_set_logged_in_cookies): + """ + Calls EmailChangeMiddleware.process_response when user is not authenticated. + Verify that the logged-in cookies are not updated + """ + # return value of mock + mock_set_logged_in_cookies.return_value = self.client.response + + # Unauthenticated User + self.request.user = AnonymousUser() + + # Call process_response without authenticating a user + response = EmailChangeMiddleware(get_response=lambda request: None).process_response( + self.request, self.client.response + ) + + assert response.status_code == 200 + # Assert that cookies are not updated + # Assert that mock_set_logged_in_cookies not called + mock_set_logged_in_cookies.assert_not_called() + + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_logged_in_cookies") + def test_process_response_user_authenticated_but_email_change_not_requested(self, mock_set_logged_in_cookies): + """ + Calls EmailChangeMiddleware.process_response when user is authenticated but email + change was not requested. + Verify that the logged-in cookies are not updated + """ + # return value of mock + mock_set_logged_in_cookies.return_value = self.client.response + + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # No call to register_email_change to indicate email was not changed + + # Call process_response + response = EmailChangeMiddleware(get_response=lambda request: None).process_response( + self.request, self.client.response + ) + + assert response.status_code == 200 + # Assert that cookies are not updated + # Assert that mock_set_logged_in_cookies not called + mock_set_logged_in_cookies.assert_not_called() + + @patch("openedx.core.djangoapps.safe_sessions.middleware.set_logged_in_cookies") + def test_process_response_user_authenticated_and_email_change_requested(self, mock_set_logged_in_cookies): + """ + Calls EmailChangeMiddleware.process_response when user is authenticated and email + change was requested. + Verify that the logged-in cookies are updated + """ + # return value of mock + mock_set_logged_in_cookies.return_value = self.client.response + + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Registering email change (setting a variable `email_change_requested` to indicate email was changed) + # so that process_response can update cookies + EmailChangeMiddleware.register_email_change(request=self.request, email=self.user.email) + + # Call process_response + response = EmailChangeMiddleware(get_response=lambda request: None).process_response( + self.request, self.client.response + ) + + assert response.status_code == 200 + # Assert that cookies are updated + # Assert that mock_set_logged_in_cookies is called + mock_set_logged_in_cookies.assert_called() + + def test_process_response_no_email_in_session(self): + """ + Calls EmailChangeMiddleware.process_response when user is authenticated and + user's email was not stored in user's session. + Verify that the user's email is stored in session + """ + # Log in the user + self.client.login(email=self.user.email, password=self.PASSWORD) + self.request.session = self.client.session + self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie + + # Ensure there is no email in the session + self.assertEqual(self.request.session.get('email'), None) + + # Call process_response + response = EmailChangeMiddleware(get_response=lambda request: None).process_response( + self.request, self.client.response + ) + + assert response.status_code == 200 + # Verify that email is set in the session + self.assertEqual(self.request.session.get('email'), self.user.email) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + def test_user_remain_authenticated_on_email_change_in_other_browser_with_toggle_disabled(self): + """ + Integration Test: test that a user remains authenticated upon email change + in other browser when ENFORCE_SESSION_EMAIL_MATCH toggle is disabled + Verify that the session and cookies are not affected in current browser and + user remains authenticated + """ + setup_login_oauth_client() + + # Login the user with 'test@example.com` email and test password in current browser + response = self.client.post(self.login_url, { + "email_or_username": self.EMAIL, + "password": self.PASSWORD, + }) + # Verify that the user is logged in successfully in current browser + assert response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email changed in some other browser (Email is changed in DB) + self.user.email = 'new_email@test.com' + self.user.save() + + # Verify that the user remains authenticated in current browser and can access the dashboard + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + def test_cookies_are_updated_with_new_email_on_email_change_with_toggle_enabled(self): + """ + Integration Test: test that cookies are updated with new email upon email change + in current browser regardless of toggle setting + Verify that the cookies are updated in current browser and + user remains authenticated + """ + setup_login_oauth_client() + + # Login the user with 'test@example.com` email and test password in current browser + login_response = self.client.post(self.login_url, { + "email_or_username": self.EMAIL, + "password": self.PASSWORD, + }) + # Verify that the user is logged in successfully in current browser + assert login_response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(login_response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email change in current browser + activation_key = uuid.uuid4().hex + PendingEmailChange.objects.update_or_create( + user=self.user, + defaults={ + 'new_email': 'new_email@test.com', + 'activation_key': activation_key, + } + ) + email_change_response = self.client.get( + reverse('confirm_email_change', kwargs={'key': activation_key}), + ) + + # Verify that email change is successful + assert email_change_response.status_code == 200 + self._assert_logged_in_cookies_present(email_change_response) + + # Verify that jwt cookies are updated with new email and + # not equal to old logged-in cookies in current browser + self.assertNotEqual( + login_response.cookies[jwt_cookies.jwt_cookie_header_payload_name()].value, + email_change_response.cookies[jwt_cookies.jwt_cookie_header_payload_name()].value + ) + self.assertNotEqual( + login_response.cookies[jwt_cookies.jwt_cookie_signature_name()].value, + email_change_response.cookies[jwt_cookies.jwt_cookie_signature_name()].value + ) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=False) + def test_cookies_are_updated_with_new_email_on_email_change_with_toggle_disabled(self): + """ + Integration Test: test that cookies are updated with new email upon email change + in current browser regardless of toggle setting + Verify that the cookies are updated in current browser and + user remains authenticated + """ + setup_login_oauth_client() + + # Login the user with 'test@example.com` email and test password in current browser + login_response = self.client.post(self.login_url, { + "email_or_username": self.EMAIL, + "password": self.PASSWORD, + }) + # Verify that the user is logged in successfully in current browser + assert login_response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(login_response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email change in current browser + activation_key = uuid.uuid4().hex + PendingEmailChange.objects.update_or_create( + user=self.user, + defaults={ + 'new_email': 'new_email@test.com', + 'activation_key': activation_key, + } + ) + email_change_response = self.client.get( + reverse('confirm_email_change', kwargs={'key': activation_key}), + ) + + # Verify that email change is successful + assert email_change_response.status_code == 200 + self._assert_logged_in_cookies_present(email_change_response) + + # Verify that jwt cookies are updated with new email and + # not equal to old logged-in cookies in current browser + self.assertNotEqual( + login_response.cookies[jwt_cookies.jwt_cookie_header_payload_name()].value, + email_change_response.cookies[jwt_cookies.jwt_cookie_header_payload_name()].value + ) + self.assertNotEqual( + login_response.cookies[jwt_cookies.jwt_cookie_signature_name()].value, + email_change_response.cookies[jwt_cookies.jwt_cookie_signature_name()].value + ) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + def test_logged_in_user_unauthenticated_on_email_change_in_other_browser(self): + """ + Integration Test: Test that a user logged-in in one browser gets unauthenticated + when the email is changed in some other browser and the request and session emails mismatch. + Verify that the session is invalidated and cookies are deleted in current browser + and user gets unauthenticated. + """ + setup_login_oauth_client() + + # Login the user with 'test@example.com` email and test password in current browser + response = self.client.post(self.login_url, { + "email_or_username": self.EMAIL, + "password": self.PASSWORD, + }) + # Verify that the user is logged in successfully in current browser + assert response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email changed in some other browser (Email is changed in DB) + self.user.email = 'new_email@test.com' + self.user.save() + + # Verify that the user gets unauthenticated in current browser and cannot access the dashboard + response = self.client.get(self.dashboard_url) + assert response.status_code == 302 + self._assert_logged_in_cookies_not_present(response) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + def test_logged_in_user_remains_authenticated_on_email_change_in_same_browser(self): + """ + Integration Test: test that a user logged-in in some browser remains authenticated + when the email is changed in same browser. + Verify that the session and cookies are updated in current browser and + user remains authenticated + """ + setup_login_oauth_client() + + # Login the user with 'test@example.com` email and test password in current browser + response = self.client.post(self.login_url, { + "email_or_username": self.EMAIL, + "password": self.PASSWORD, + }) + # Verify that the user is logged in successfully in current browser + assert response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email change in current browser + activation_key = uuid.uuid4().hex + PendingEmailChange.objects.update_or_create( + user=self.user, + defaults={ + 'new_email': 'new_email@test.com', + 'activation_key': activation_key, + } + ) + email_change_response = self.client.get( + reverse('confirm_email_change', kwargs={'key': activation_key}), + ) + + # Verify that email change is successful and all logged-in + # cookies are set in current browser + assert email_change_response.status_code == 200 + self._assert_logged_in_cookies_present(email_change_response) + + # Verify that the user remains authenticated in current browser and can access the dashboard + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + def test_registered_user_unauthenticated_on_email_change_in_other_browser(self): + """ + Integration Test: Test that a user registered in one browser gets unauthenticated + when the email is changed in some other browser and the request and session emails mismatch. + Verify that the session is invalidated and cookies are deleted in current browser + and user gets unauthenticated + """ + setup_login_oauth_client() + + # Register the user with 'john_doe@example.com` email and test password in current browser + response = self.client.post(self.register_url, { + "email": 'john_doe@example.com', + "name": 'John Doe', + "username": 'john_doe', + "password": 'password', + "honor_code": "true", + }) + # Verify that the user is logged in successfully in current browser + assert response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # simulating email changed in some other browser (Email is changed in DB) + registered_user = User.objects.get(email='john_doe@example.com') + registered_user.email = 'new_email@test.com' + registered_user.save() + + # Verify that the user get unauthenticated in current browser and cannot access the dashboard + response = self.client.get(self.dashboard_url) + assert response.status_code == 302 + self._assert_logged_in_cookies_not_present(response) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + @override_settings(ENFORCE_SESSION_EMAIL_MATCH=True) + def test_registered_user_remain_authenticated_on_email_change_in_same_browser(self): + """ + Integration Test: test that a user registered in one browser remains + authenticated in current browser when the email is changed in same browser. + Verify that the session and cookies updated and user remains + authenticated in current browser + """ + setup_login_oauth_client() + + # Register the user with 'john_doe@example.com` email and test password in current browser + response = self.client.post(self.register_url, { + "email": 'john_doe@example.com', + "name": 'John Doe', + "username": 'john_doe', + "password": 'password', + "honor_code": "true", + }) + # Verify that the user is logged in successfully in current browser + assert response.status_code == 200 + # Verify that the logged-in cookies are set in current browser + self._assert_logged_in_cookies_present(response) + + # Verify that the authenticated user can access the dashboard in current browser + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + # getting newly created user + registered_user = User.objects.get(email='john_doe@example.com') + + # simulating email change in current browser + activation_key = uuid.uuid4().hex + PendingEmailChange.objects.update_or_create( + user=registered_user, + defaults={ + 'new_email': 'new_email@test.com', + 'activation_key': activation_key, + } + ) + email_change_response = self.client.get( + reverse('confirm_email_change', kwargs={'key': activation_key}), + ) + + # Verify that email change is successful and all logged-in + # cookies are updated with new email in current browser + assert email_change_response.status_code == 200 + self._assert_logged_in_cookies_present(email_change_response) + + # Verify that the user remains authenticated in current browser and can access the dashboard + response = self.client.get(self.dashboard_url) + assert response.status_code == 200 + + def _assert_logged_in_cookies_present(self, response): + """ + Helper function to verify that all logged-in cookies are available + and have valid values (not empty strings) + """ + all_cookies = ALL_LOGGED_IN_COOKIE_NAMES + (settings.SESSION_COOKIE_NAME,) + + for cookie in all_cookies: + # Check if the cookie is present in response.cookies.keys() + self.assertIn(cookie, response.cookies.keys()) + + # Assert that the value is not an empty string + self.assertNotEqual(response.cookies[cookie].value, "") + + def _assert_logged_in_cookies_not_present(self, response): + """ + Helper function to verify that all logged-in cookies are cleared + and have empty values + """ + all_cookies = ALL_LOGGED_IN_COOKIE_NAMES + (settings.SESSION_COOKIE_NAME,) + + for cookie in all_cookies: + # Check if the cookie is present in response.cookies.keys() + self.assertIn(cookie, response.cookies.keys()) + + # Assert that the value is not an empty string + self.assertEqual(response.cookies[cookie].value, "") diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 546b8afacc2b..0496f45ae2fc 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -232,7 +232,7 @@ def test_get_username(self): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(16) + self._verify_get_own_username(19) def test_get_username_inactive(self): """ @@ -242,7 +242,7 @@ def test_get_username_inactive(self): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(16) + self._verify_get_own_username(19) def test_get_username_not_logged_in(self): """ @@ -358,7 +358,7 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe """ ENABLED_CACHES = ['default'] - TOTAL_QUERY_COUNT = 24 + TOTAL_QUERY_COUNT = 27 FULL_RESPONSE_FIELD_COUNT = 29 def setUp(self): @@ -811,7 +811,7 @@ def verify_get_own_information(queries): assert data['time_zone'] is None self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(self._get_num_queries(22)) + verify_get_own_information(self._get_num_queries(25)) # Now make sure that the user can get the same information, even if not active self.user.is_active = False @@ -831,7 +831,7 @@ def test_get_account_empty_string(self): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(self._get_num_queries(22), table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(self._get_num_queries(25), table_ignorelist=WAFFLE_TABLES): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "state", "bio",): assert response.data[empty_field] is None diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 7b764e6372b7..f5dec2e8cb1e 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -463,7 +463,7 @@ edx-django-utils==5.10.1 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==10.0.0 +edx-drf-extensions==10.1.0 # via # -r requirements/edx/kernel.in # edx-completion diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 5f70b4e9bc36..1d89deb5d412 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -742,7 +742,7 @@ edx-django-utils==5.10.1 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==10.0.0 +edx-drf-extensions==10.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index ae3ab27a7df3..da22aa05fe94 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -541,7 +541,7 @@ edx-django-utils==5.10.1 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==10.0.0 +edx-drf-extensions==10.1.0 # via # -r requirements/edx/base.txt # edx-completion diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 27bf1eec7378..2211f8798b61 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -567,7 +567,7 @@ edx-django-utils==5.10.1 # openedx-blockstore # ora2 # super-csv -edx-drf-extensions==10.0.0 +edx-drf-extensions==10.1.0 # via # -r requirements/edx/base.txt # edx-completion