Skip to content

Commit

Permalink
Merge branch 'master' into dkaplan1/APER-1322_certs-with-invalidation…
Browse files Browse the repository at this point in the history
…-records-arent-in-the-unavailable-status
  • Loading branch information
deborahgu authored Jan 29, 2024
2 parents 1c049a3 + c456781 commit 2538531
Show file tree
Hide file tree
Showing 25 changed files with 1,191 additions and 325 deletions.
2 changes: 1 addition & 1 deletion cms/static/js/views/export.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ define([
* @return {JSON} the data of the previous export
*/
storedExport: function(contentHomeUrl) {
var storedData = JSON.parse($.cookie(COOKIE_NAME));
var storedData = JSON.parse($.cookie(COOKIE_NAME) || null);
if (storedData) {
successUnixDate = storedData.date;
}
Expand Down
9 changes: 7 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 EmailChangeMiddleware
from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -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}]
Expand Down
14 changes: 11 additions & 3 deletions common/djangoapps/student/tests/test_receivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 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()

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
6 changes: 3 additions & 3 deletions lms/djangoapps/certificates/apis/v0/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/course_api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
"""
Discussion notifications sender util.
"""
import logging

from django.conf import settings
from lms.djangoapps.discussion.django_comment_client.permissions import get_team
from openedx_events.learning.data import UserNotificationData, CourseNotificationData
Expand All @@ -22,9 +20,6 @@
)


log = logging.getLogger(__name__)


class DiscussionNotificationSender:
"""
Class to send notifications to users who are subscribed to the thread.
Expand Down Expand Up @@ -262,8 +257,6 @@ def send_new_thread_created_notification(self):
'username': self.creator.username,
'post_title': self.thread.title
}

log.info(f"Temp: Audience filter for course-wide notification is {audience_filters}")
self._send_course_wide_notification(notification_type, audience_filters, context)


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

Expand Down Expand Up @@ -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 #################
Expand Down
6 changes: 0 additions & 6 deletions openedx/core/djangoapps/notifications/audience_filters.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""
Audience based filters for notifications
"""
import logging

from abc import abstractmethod

Expand All @@ -22,9 +21,6 @@
)


log = logging.getLogger(__name__)


class NotificationAudienceFilterBase:
"""
Base class for notification audience filters
Expand Down Expand Up @@ -84,12 +80,10 @@ def filter(self, course_roles):

if 'staff' in course_roles:
staff_users = CourseStaffRole(course_key).users_with_role().values_list('id', flat=True)
log.info(f'Temp: Course wide notification, staff users calculated are {staff_users}')
user_ids.extend(staff_users)

if 'instructor' in course_roles:
instructor_users = CourseInstructorRole(course_key).users_with_role().values_list('id', flat=True)
log.info(f'Temp: Course wide notification, instructor users calculated are {instructor_users}')
user_ids.extend(instructor_users)

return user_ids
Expand Down
4 changes: 0 additions & 4 deletions openedx/core/djangoapps/notifications/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,10 @@ def calculate_course_wide_notification_audience(course_key, audience_filters):
if filter_class:
filter_instance = filter_class(course_key)
filtered_users = filter_instance.filter(filter_values)
log.info(f'Temp: Course-wide notification filtered users are '
f'{filtered_users} for filter type {filter_type}')
audience_user_ids.extend(filtered_users)
else:
raise ValueError(f"Invalid audience filter type: {filter_type}")

log.info(f'Temp: Course-wide notification after audience filter is applied, users: {list(set(audience_user_ids))}')
return list(set(audience_user_ids))


Expand Down Expand Up @@ -131,5 +128,4 @@ def generate_course_notifications(signal, sender, course_notification_data, meta
'content_url': course_notification_data.get('content_url'),
}

log.info(f"Temp: Course-wide notification, user_ids to sent notifications to {notification_data.get('user_ids')}")
send_notifications.delay(**notification_data)
88 changes: 87 additions & 1 deletion openedx/core/djangoapps/safe_sessions/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 2538531

Please sign in to comment.