Skip to content

Commit

Permalink
feat: logout other sessions on email change
Browse files Browse the repository at this point in the history
  • Loading branch information
syedsajjadkazmii committed Dec 7, 2023
1 parent 129940b commit a86486b
Show file tree
Hide file tree
Showing 10 changed files with 264 additions and 29 deletions.
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def activate_user(self, email):


@ddt
@override_settings(ENFORCE_SESSION_EMAIL_MATCH=False)
class AuthTestCase(ContentStoreTestCase):
"""Check that various permissions-related things work"""

Expand Down
10 changes: 8 additions & 2 deletions common/djangoapps/student/signals/receivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 EmailChangeSessionInvalidationMiddleware
from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -105,8 +106,13 @@ 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. """

if settings.ENFORCE_SESSION_EMAIL_MATCH:
# Store the user's email for session consistency (used by EmailChangeSessionInvalidationMiddleware)
EmailChangeSessionInvalidationMiddleware.register_email_change(request, user.email)

email = user.email
user_id = user.id
attributes = [{'email': email, 'external_id': user_id}]
Expand Down
25 changes: 21 additions & 4 deletions common/djangoapps/student/tests/test_receivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

from unittest import skipUnless
from unittest.mock import patch
from django.test.utils import override_settings

from edx_toggles.toggles.testutils import override_waffle_flag

from common.djangoapps.student.models import CourseEnrollmentCelebration, PendingNameChange, UserProfile
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

Expand Down Expand Up @@ -75,10 +76,26 @@ 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 if ENFORCE_SESSION_EMAIL_MATCH
is enabled.
"""
user = UserFactory(email='[email protected]', 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 = '[email protected]'
user.save()

assert mock_get_braze_client.called
with override_settings(ENFORCE_SESSION_EMAIL_MATCH=False):
USER_EMAIL_CHANGED.send(sender=None, user=user, request=request)

assert mock_get_braze_client.called
assert request.session.get('email', None) is None

with override_settings(ENFORCE_SESSION_EMAIL_MATCH=True):
USER_EMAIL_CHANGED.send(sender=None, user=user, request=request)

assert mock_get_braze_client.called
assert request.session.get('email', None) == user.email
2 changes: 1 addition & 1 deletion common/djangoapps/student/views/management.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
16 changes: 16 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2224,6 +2224,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.EmailChangeSessionInvalidationMiddleware',

'common.djangoapps.student.middleware.UserStandingMiddleware',
'openedx.core.djangoapps.contentserver.middleware.StaticContentServer',

Expand Down Expand Up @@ -5024,6 +5027,19 @@ 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 EmailChangeSessionInvalidationMiddleware ###############

# .. 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.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2023-12-07
# .. toggle_target_removal_date: None
# .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-1694
ENFORCE_SESSION_EMAIL_MATCH = False

LEARNER_HOME_MFE_REDIRECT_PERCENTAGE = 0

############### Settings for the ace_common plugin #################
Expand Down
2 changes: 2 additions & 0 deletions openedx/core/djangoapps/lang_pref/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from django.contrib.sessions.middleware import SessionMiddleware
from django.http import HttpResponse
from django.test.client import Client, RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
from django.utils.translation.trans_real import parse_accept_lang_header

Expand All @@ -30,6 +31,7 @@


@ddt.ddt
@override_settings(ENFORCE_SESSION_EMAIL_MATCH=False)
class TestUserPreferenceMiddleware(CacheIsolationTestCase):
"""
Tests to make sure user preferences are getting properly set in the middleware.
Expand Down
53 changes: 53 additions & 0 deletions openedx/core/djangoapps/safe_sessions/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,59 @@ def _get_encrypted_request_headers(request):
return encrypt_for_log(str(request.headers), getattr(settings, 'SAFE_SESSIONS_DEBUG_PUBLIC_KEY', None))


class EmailChangeSessionInvalidationMiddleware(MiddlewareMixin):
"""
Middleware responsible for invalidating user sessions
by detecting a mismatch between the user's email in
the session and request.user.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 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 settings.ENFORCE_SESSION_EMAIL_MATCH and request.user.is_authenticated:
user_session_email = request.session.get('email', None)

if user_session_email is not None and request.user.email != user_session_email:
# Flush the session and mark cookies for deletion.
request.session.flush()
request.user = AnonymousUser()
_mark_cookie_for_deletion(request)
EmailChangeSessionInvalidationMiddleware._set_session_email_match_custom_attribute(False)
else:
EmailChangeSessionInvalidationMiddleware._set_session_email_match_custom_attribute(True)

@staticmethod
def register_email_change(request, email):
"""
Sets email in session for comparison
"""
request.session['email'] = email

@staticmethod
def _set_session_email_match_custom_attribute(value):
"""
Sets custom attribute 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_match', value)


def obscure_token(value: Union[str, None]) -> Union[str, None]:
"""
Return a short string that can be used to detect other occurrences
Expand Down
124 changes: 124 additions & 0 deletions openedx/core/djangoapps/safe_sessions/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from common.djangoapps.student.tests.factories import UserFactory

from ..middleware import (
EmailChangeSessionInvalidationMiddleware,
SafeCookieData,
SafeSessionMiddleware,
mark_user_change_as_expected,
Expand Down Expand Up @@ -615,3 +616,126 @@ 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]


class TestEmailChangeSessionInvalidationMiddleware(TestSafeSessionsLogMixin, TestCase):
"""
Test class for EmailChangeSessionInvalidationMiddleware
"""

def setUp(self):
super().setUp()
self.TEST_PASSWORD = 'Password1234'
self.user = UserFactory.create(email='[email protected]', password=self.TEST_PASSWORD)
self.addCleanup(set_current_request, None)
self.request = get_mock_request(self.user)
self.client.response = HttpResponse()
self.client.response.cookies = SimpleCookie()

@override_settings(ENFORCE_SESSION_EMAIL_MATCH=False)
@patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion')
def test_process_request_settings_disabled(self, mock_mark_cookie_for_deletion):
"""
Calls EmailChangeSessionInvalidationMiddleware.process_request when no user is authenticated.
Verifies that session and cookies are not affected.
"""
# Log in the user
self.client.login(email=self.user.email, password=self.TEST_PASSWORD)
self.request.session = self.client.session

# Ensure no email is stored in the session
self.assertIsNone(self.request.session.get('email'))

# Call process_request without authenticating a user
EmailChangeSessionInvalidationMiddleware(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_not_authenticated(self, mock_mark_cookie_for_deletion):
"""
Calls EmailChangeSessionInvalidationMiddleware.process_request when no user is authenticated.
Verifies that session and cookies are not affected.
"""
# Unauthenticated User
self.request.user = AnonymousUser()

# Call process_request without authenticating a user
EmailChangeSessionInvalidationMiddleware(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)
def test_process_request_authenticated_no_session_email(self):
"""
Calls EmailChangeSessionInvalidationMiddleware.process_request when a user is authenticated
but there is no email in the session. Verifies that session and cookies are not affected.
"""
# Log in the user
self.client.login(email=self.user.email, password=self.TEST_PASSWORD)
self.request.session = self.client.session

# Ensure no email is stored in the session
self.assertIsNone(self.request.session.get('email'))

# Call process_request
EmailChangeSessionInvalidationMiddleware(get_response=lambda request: None).process_request(self.request)

# Assert that the session and cookies are not affected
self.assertIsNone(self.request.session.get('email'))
self.assertEqual(len(self.client.response.cookies), 0)

@override_settings(ENFORCE_SESSION_EMAIL_MATCH=True)
def test_process_request_authenticated_matching_session_email(self):
"""
Calls EmailChangeSessionInvalidationMiddleware.process_request when a user is authenticated
and the session email matches request.user.email. Verifies that session and cookies are not affected.
"""
# Log in the user
self.client.login(email=self.user.email, password=self.TEST_PASSWORD)
self.request.session = self.client.session
self.request.session['email'] = self.user.email # Set the session email to match request.user.email
self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie

# Ensure email matches 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)

# Call process_request
EmailChangeSessionInvalidationMiddleware(get_response=lambda request: None).process_request(self.request)

# 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')

@override_settings(ENFORCE_SESSION_EMAIL_MATCH=True)
@patch('openedx.core.djangoapps.safe_sessions.middleware._mark_cookie_for_deletion')
def test_process_request_email_mismatch(self, mock_mark_cookie_for_deletion):
"""
Calls EmailChangeSessionInvalidationMiddleware.process_request with
a mismatch between session email and user email. Verifies that session
is flushed and cookies are marked for deletion.
"""
# Log in the user
self.client.login(email=self.user.email, password=self.TEST_PASSWORD)
self.request.session = self.client.session
self.request.session['email'] = self.user.email # Set the session email to match request.user.email
self.client.response.set_cookie(settings.SESSION_COOKIE_NAME, 'authenticated') # Add some logged-in cookie

# Modify session email to create a mismatch (Email changed)
self.request.session['email'] = '[email protected]'

# Call process_request
EmailChangeSessionInvalidationMiddleware(get_response=lambda request: None).process_request(self.request)

# 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()
9 changes: 8 additions & 1 deletion openedx/core/djangoapps/user_authn/views/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@
from common.djangoapps.util.json_request import JsonResponse
from common.djangoapps.util.password_policy_validators import normalize_password
from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance
from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected
from openedx.core.djangoapps.safe_sessions.middleware import (
mark_user_change_as_expected,
EmailChangeSessionInvalidationMiddleware
)
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.user_api import accounts
from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY
Expand Down Expand Up @@ -315,6 +318,10 @@ def _handle_successful_authentication_and_login(user, request):
request.session.set_expiry(604800 * 4)
log.debug("Setting user session expiry to 4 weeks")

if settings.ENFORCE_SESSION_EMAIL_MATCH:
# Store the user's email for session consistency (used by EmailChangeSessionInvalidationMiddleware)
EmailChangeSessionInvalidationMiddleware.register_email_change(request, user.email)

# .. event_implemented_name: SESSION_LOGIN_COMPLETED
SESSION_LOGIN_COMPLETED.send_event(
user=UserData(
Expand Down
Loading

0 comments on commit a86486b

Please sign in to comment.