diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb index 252af4d68d..746f83c056 100644 Binary files a/common/static/data/geoip/GeoLite2-Country.mmdb and b/common/static/data/geoip/GeoLite2-Country.mmdb differ diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 88ff4562b0..83be9b6448 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -38,14 +38,6 @@ class ActivateLoginTest(LoginEnrollmentTestCase): """ self.logout() - def test_request_attr_on_logout(self): - """ - Test request object after logging out to see whether it - has 'is_from_log_out' attribute set to true. - """ - response = self.client.get(reverse('logout')) - assert getattr(response.wsgi_request, 'is_from_logout', False) - class PageLoaderTestCase(LoginEnrollmentTestCase): """ diff --git a/lms/envs/common.py b/lms/envs/common.py index fd3ed02fa6..0bb174d0ec 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -991,6 +991,9 @@ COURSE_MESSAGE_ALERT_DURATION_IN_DAYS = 14 MARKETING_EMAILS_OPT_IN = False +# VAN-754 - Year of birth field put behind a flag to make it available for OpenedX. +COLLECT_YEAR_OF_BIRTH = True + ############################# SET PATH INFORMATION ############################# PROJECT_ROOT = path(__file__).abspath().dirname().dirname() # /edx-platform/lms REPO_ROOT = PROJECT_ROOT.dirname() diff --git a/lms/envs/production.py b/lms/envs/production.py index 63bc68d343..ca0fb39819 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -157,6 +157,12 @@ SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True) DCS_SESSION_COOKIE_SAMESITE = ENV_TOKENS.get('DCS_SESSION_COOKIE_SAMESITE', DCS_SESSION_COOKIE_SAMESITE) DCS_SESSION_COOKIE_SAMESITE_FORCE_ALL = ENV_TOKENS.get('DCS_SESSION_COOKIE_SAMESITE_FORCE_ALL', DCS_SESSION_COOKIE_SAMESITE_FORCE_ALL) # lint-amnesty, pylint: disable=line-too-long +# As django-cookies-samesite package is set to be removed from base requirements when we upgrade to Django 3.2, +# we should follow the settings name provided by Django. +# https://docs.djangoproject.com/en/3.2/ref/settings/#session-cookie-samesite +if django.VERSION >= (3, 1): + SESSION_COOKIE_SAMESITE = DCS_SESSION_COOKIE_SAMESITE + AWS_SES_REGION_NAME = ENV_TOKENS.get('AWS_SES_REGION_NAME', 'us-east-1') AWS_SES_REGION_ENDPOINT = ENV_TOKENS.get('AWS_SES_REGION_ENDPOINT', 'email.us-east-1.amazonaws.com') diff --git a/openedx/core/djangoapps/discussions/models.py b/openedx/core/djangoapps/discussions/models.py index 38e7394607..efdd616148 100644 --- a/openedx/core/djangoapps/discussions/models.py +++ b/openedx/core/djangoapps/discussions/models.py @@ -33,40 +33,43 @@ ProviderExternalLinks = namedtuple( ['learn_more', 'configuration', 'general', 'accessibility', 'contact_email'] ) +ProviderFeature = namedtuple('ProviderFeature', ['id', 'feature_support_type']) + class Features(Enum): """ Features to be used/mapped in discussion providers """ - ANONYMOUS_POSTING = 'anonymous-posting' - # Todo: https://openedx.atlassian.net/browse/TNL-8546 - # This will be added back in once we add LTI v1.3 support for discussion - # configuration in the future. https://openedx.atlassian.net/browse/TNL-8365 - # AUTOMATIC_LEARNER_ENROLLMENT = 'automatic-learner-enrollment' + # Basic Supported Features + PRIMARY_DISCUSSION_APP_EXPERIENCE = ProviderFeature('primary-discussion-app-experience', 'basic') + LTI_BASIC_CONFIGURATION = ProviderFeature('lti-basic-configuration', 'basic') + # DISCUSSION_PAGE = ProviderFeature('discussion-page', 'basic') - BLACKOUT_DISCUSSION_DATES = 'blackout-discussion-dates' - COMMUNITY_TA_SUPPORT = 'community-ta-support' - COURSE_COHORT_SUPPORT = 'course-cohort-support' - DISCUSSION_PAGE = 'discussion-page' - INTERNATIONALIZATION_SUPPORT = 'internationalization-support' - PRIMARY_DISCUSSION_APP_EXPERIENCE = 'primary-discussion-app-experience' - QUESTION_DISCUSSION_SUPPORT = 'question-discussion-support' - REPORT_FLAG_CONTENT_TO_MODERATORS = 'report/flag-content-to-moderators' - RESEARCH_DATA_EVENTS = 'research-data-events' - WCAG_2_0_SUPPORT = 'wcag-2.0-support' - WCAG_2_1 = 'wcag-2.1' - ADVANCED_IN_CONTEXT_DISCUSSION = 'advanced-in-context-discussion' - DIRECT_MESSAGES_FROM_INSTRUCTORS = 'direct-messages-from-instructors' - DISCUSSION_CONTENT_PROMPTS = 'discussion-content-prompts' - EMAIL_NOTIFICATIONS = 'email-notifications' - EMBEDDED_COURSE_SECTIONS = 'embedded-course-sections' - GRADED_DISCUSSIONS = 'graded-discussions' - IN_PLATFORM_NOTIFICATIONS = 'in-platform-notifications' - LTI_ADVANCED_SHARING_MODE = 'lti-advanced-sharing-mode' - LTI_BASIC_CONFIGURATION = 'lti-basic-configuration' - SIMPLIFIED_IN_CONTEXT_DISCUSSION = 'simplified-in-context-discussion' - USER_MENTIONS = 'user-mentions' + # Partially Supported Features + QUESTION_DISCUSSION_SUPPORT = ProviderFeature('question-discussion-support', 'partial') + COMMUNITY_TA_SUPPORT = ProviderFeature('community-ta-support', 'partial') + REPORT_FLAG_CONTENT_TO_MODERATORS = ProviderFeature('report/flag-content-to-moderators', 'partial') + LTI_ADVANCED_SHARING_MODE = ProviderFeature('lti-advanced-sharing-mode', 'partial') + AUTOMATIC_LEARNER_ENROLLMENT = ProviderFeature('automatic-learner-enrollment', 'partial') + ANONYMOUS_POSTING = ProviderFeature('anonymous-posting', 'partial') + INTERNATIONALIZATION_SUPPORT = ProviderFeature('internationalization-support', 'partial') + EMAIL_NOTIFICATIONS = ProviderFeature('email-notifications', 'partial') + WCAG_2_0_SUPPORT = ProviderFeature('wcag-2.0-support', 'partial') + BLACKOUT_DISCUSSION_DATES = ProviderFeature('blackout-discussion-dates', 'partial') + # WCAG_2_1 = ProviderFeature('wcag-2.1', 'partial') + # EMBEDDED_COURSE_SECTIONS = ProviderFeature('embedded-course-sections', 'basic') + + # Fully Supported Features + COURSE_COHORT_SUPPORT = ProviderFeature('course-cohort-support', 'full') + RESEARCH_DATA_EVENTS = ProviderFeature('research-data-events', 'full') + + # Commonly Requested Features + IN_PLATFORM_NOTIFICATIONS = ProviderFeature('in-platform-notifications', 'common') + DISCUSSION_CONTENT_PROMPTS = ProviderFeature('discussion-content-prompts', 'common') + GRADED_DISCUSSIONS = ProviderFeature('graded-discussions', 'common') + DIRECT_MESSAGES_FROM_INSTRUCTORS = ProviderFeature('direct-messages-from-instructors', 'common') + USER_MENTIONS = ProviderFeature('user-mentions', 'common') def pii_sharing_required_message(provider_name): @@ -91,18 +94,18 @@ def pii_sharing_required_message(provider_name): AVAILABLE_PROVIDER_MAP = { 'legacy': { 'features': [ - Features.ANONYMOUS_POSTING.value, - Features.BLACKOUT_DISCUSSION_DATES.value, - Features.COMMUNITY_TA_SUPPORT.value, - Features.COURSE_COHORT_SUPPORT.value, - Features.DISCUSSION_PAGE.value, - Features.INTERNATIONALIZATION_SUPPORT.value, - Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, - Features.QUESTION_DISCUSSION_SUPPORT.value, - Features.RESEARCH_DATA_EVENTS.value, - Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value, - Features.WCAG_2_0_SUPPORT.value, - Features.WCAG_2_1.value, + Features.LTI_BASIC_CONFIGURATION.value.id, + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value.id, + Features.QUESTION_DISCUSSION_SUPPORT.value.id, + Features.COMMUNITY_TA_SUPPORT.value.id, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value.id, + Features.AUTOMATIC_LEARNER_ENROLLMENT.value.id, + Features.ANONYMOUS_POSTING.value.id, + Features.INTERNATIONALIZATION_SUPPORT.value.id, + Features.WCAG_2_0_SUPPORT.value.id, + Features.BLACKOUT_DISCUSSION_DATES.value.id, + Features.COURSE_COHORT_SUPPORT.value.id, + Features.RESEARCH_DATA_EVENTS.value.id, ], 'external_links': ProviderExternalLinks( learn_more='', @@ -114,20 +117,71 @@ AVAILABLE_PROVIDER_MAP = { 'messages': [], 'has_full_support': True }, + 'ed-discuss': { + 'features': [ + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value.id, + Features.LTI_BASIC_CONFIGURATION.value.id, + Features.QUESTION_DISCUSSION_SUPPORT.value.id, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value.id, + Features.LTI_ADVANCED_SHARING_MODE.value.id, + Features.AUTOMATIC_LEARNER_ENROLLMENT.value.id, + Features.ANONYMOUS_POSTING.value.id, + Features.INTERNATIONALIZATION_SUPPORT.value.id, + Features.EMAIL_NOTIFICATIONS.value.id, + Features.WCAG_2_0_SUPPORT.value.id, + Features.BLACKOUT_DISCUSSION_DATES.value.id, + Features.IN_PLATFORM_NOTIFICATIONS.value.id, + Features.USER_MENTIONS.value.id, + ], + 'external_links': ProviderExternalLinks( + learn_more='', + configuration='', + general='https://edstem.org/us/', + accessibility='', + contact_email='', + )._asdict(), + 'messages': [], + 'has_full_support': False + }, + 'inscribe': { + 'features': [ + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value.id, + Features.LTI_BASIC_CONFIGURATION.value.id, + Features.QUESTION_DISCUSSION_SUPPORT.value.id, + Features.COMMUNITY_TA_SUPPORT.value.id, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value.id, + Features.LTI_ADVANCED_SHARING_MODE.value.id, + Features.AUTOMATIC_LEARNER_ENROLLMENT.value.id, + Features.ANONYMOUS_POSTING.value.id, + Features.INTERNATIONALIZATION_SUPPORT.value.id, + Features.EMAIL_NOTIFICATIONS.value.id, + Features.WCAG_2_0_SUPPORT.value.id, + Features.RESEARCH_DATA_EVENTS.value.id, + Features.IN_PLATFORM_NOTIFICATIONS.value.id, + Features.DISCUSSION_CONTENT_PROMPTS.value.id, + ], + 'external_links': ProviderExternalLinks( + learn_more='', + configuration='', + general='https://www.inscribeapp.com/', + accessibility='', + contact_email='', + )._asdict(), + 'messages': [pii_sharing_required_message('InScribe')], + 'has_full_support': False + }, 'piazza': { 'features': [ - Features.ANONYMOUS_POSTING.value, - Features.BLACKOUT_DISCUSSION_DATES.value, - Features.COMMUNITY_TA_SUPPORT.value, - Features.DIRECT_MESSAGES_FROM_INSTRUCTORS.value, - Features.DISCUSSION_CONTENT_PROMPTS.value, - Features.DISCUSSION_PAGE.value, - Features.EMAIL_NOTIFICATIONS.value, - Features.LTI_BASIC_CONFIGURATION.value, - Features.QUESTION_DISCUSSION_SUPPORT.value, - Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value, - Features.USER_MENTIONS.value, - Features.WCAG_2_0_SUPPORT.value, + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value.id, + Features.LTI_BASIC_CONFIGURATION.value.id, + Features.QUESTION_DISCUSSION_SUPPORT.value.id, + Features.COMMUNITY_TA_SUPPORT.value.id, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value.id, + Features.LTI_ADVANCED_SHARING_MODE.value.id, + Features.ANONYMOUS_POSTING.value.id, + Features.EMAIL_NOTIFICATIONS.value.id, + Features.WCAG_2_0_SUPPORT.value.id, + Features.BLACKOUT_DISCUSSION_DATES.value.id, ], 'external_links': ProviderExternalLinks( learn_more='https://piazza.com/product/overview', @@ -141,19 +195,18 @@ AVAILABLE_PROVIDER_MAP = { }, 'yellowdig': { 'features': [ - Features.ANONYMOUS_POSTING.value, - Features.COMMUNITY_TA_SUPPORT.value, - Features.DIRECT_MESSAGES_FROM_INSTRUCTORS.value, - Features.EMAIL_NOTIFICATIONS.value, - Features.GRADED_DISCUSSIONS.value, - Features.IN_PLATFORM_NOTIFICATIONS.value, - Features.LTI_BASIC_CONFIGURATION.value, - Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, - Features.QUESTION_DISCUSSION_SUPPORT.value, - Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value, - Features.RESEARCH_DATA_EVENTS.value, - Features.USER_MENTIONS.value, - Features.WCAG_2_0_SUPPORT.value, + Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value.id, + Features.LTI_BASIC_CONFIGURATION.value.id, + Features.QUESTION_DISCUSSION_SUPPORT.value.id, + Features.COMMUNITY_TA_SUPPORT.value.id, + Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value.id, + Features.EMAIL_NOTIFICATIONS.value.id, + Features.WCAG_2_0_SUPPORT.value.id, + Features.RESEARCH_DATA_EVENTS.value.id, + Features.IN_PLATFORM_NOTIFICATIONS.value.id, + Features.GRADED_DISCUSSIONS.value.id, + Features.DIRECT_MESSAGES_FROM_INSTRUCTORS.value.id, + Features.USER_MENTIONS.value.id, ], 'external_links': ProviderExternalLinks( learn_more='https://www.youtube.com/watch?v=ZACief-qMwY', @@ -166,59 +219,6 @@ AVAILABLE_PROVIDER_MAP = { 'has_full_support': False, 'admin_only_config': True, }, - 'inscribe': { - 'features': [ - Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, - Features.LTI_BASIC_CONFIGURATION.value, - ], - 'external_links': ProviderExternalLinks( - learn_more='', - configuration='', - general='https://www.inscribeapp.com/', - accessibility='', - contact_email='', - )._asdict(), - 'messages': [pii_sharing_required_message('InScribe')], - 'has_full_support': False - }, - 'discourse': { - 'features': [ - Features.LTI_ADVANCED_SHARING_MODE.value, - Features.LTI_BASIC_CONFIGURATION.value, - Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, - ], - 'external_links': ProviderExternalLinks( - learn_more='', - configuration='', - general='http://discourse.org/', - accessibility='', - contact_email='', - )._asdict(), - 'messages': [pii_sharing_required_message('Discourse')], - 'has_full_support': False - }, - 'ed-discuss': { - 'features': [ - Features.ANONYMOUS_POSTING.value, - Features.COMMUNITY_TA_SUPPORT.value, - Features.EMAIL_NOTIFICATIONS.value, - Features.INTERNATIONALIZATION_SUPPORT.value, - Features.LTI_BASIC_CONFIGURATION.value, - Features.PRIMARY_DISCUSSION_APP_EXPERIENCE.value, - Features.QUESTION_DISCUSSION_SUPPORT.value, - Features.REPORT_FLAG_CONTENT_TO_MODERATORS.value, - Features.WCAG_2_0_SUPPORT.value, - ], - 'external_links': ProviderExternalLinks( - learn_more='', - configuration='', - general='https://edstem.org/us/', - accessibility='', - contact_email='', - )._asdict(), - 'messages': [], - 'has_full_support': False - } } diff --git a/openedx/core/djangoapps/discussions/serializers.py b/openedx/core/djangoapps/discussions/serializers.py index dd0751c7f4..cd9c350077 100644 --- a/openedx/core/djangoapps/discussions/serializers.py +++ b/openedx/core/djangoapps/discussions/serializers.py @@ -203,7 +203,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer): course_key = instance.context_key payload = super().to_representation(instance) lti_configuration_data = {} - supports_lti = instance.supports(Features.LTI_BASIC_CONFIGURATION.value) + supports_lti = instance.supports(Features.LTI_BASIC_CONFIGURATION.value.id) if supports_lti: lti_configuration = LtiSerializer(instance.lti_configuration, context={ 'pii_sharing_allowed': get_lti_pii_sharing_state_for_course(course_key), @@ -219,7 +219,10 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer): ) if legacy_settings.is_valid(raise_exception=True): plugin_configuration = legacy_settings.data - features_list = [feature.value for feature in Features] + features_list = [ + {'id': feature.value.id, 'feature_support_type': feature.value.feature_support_type} + for feature in Features + ] payload.update({ 'features': features_list, 'lti_configuration': lti_configuration_data, @@ -256,7 +259,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer): Update LtiConfiguration """ lti_configuration_data = validated_data.get('lti_configuration') - supports_lti = instance.supports(Features.LTI_BASIC_CONFIGURATION.value) + supports_lti = instance.supports(Features.LTI_BASIC_CONFIGURATION.value.id) if not supports_lti: instance.lti_configuration = None elif lti_configuration_data: diff --git a/openedx/core/djangoapps/discussions/tests/test_views.py b/openedx/core/djangoapps/discussions/tests/test_views.py index 12d0813590..f6c6805461 100644 --- a/openedx/core/djangoapps/discussions/tests/test_views.py +++ b/openedx/core/djangoapps/discussions/tests/test_views.py @@ -150,7 +150,12 @@ class DataTest(AuthorizedApiTest): name for name, spec in data['providers']['available'].items() if "messages" not in spec ], "Found available providers without messages field" - assert data['lti_configuration'] == {} + assert data['lti_configuration'] == { + 'lti_1p1_client_key': '', + 'lti_1p1_client_secret': '', + 'lti_1p1_launch_url': '', + 'version': None + } assert data['plugin_configuration'] == { 'allow_anonymous': True, 'allow_anonymous_to_peers': False, @@ -480,7 +485,6 @@ class DataTest(AuthorizedApiTest): assert data['enabled'] assert data['provider_type'] == 'legacy' assert not data['plugin_configuration']['allow_anonymous'] - assert not data['lti_configuration'] @ddt.data(*[ user_type.name for user_type in CourseUserType diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index 9031598036..ad2bd2cb8a 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -3,6 +3,17 @@ This module defines SafeSessionMiddleware that makes use of a SafeCookieData that cryptographically binds the user to the session id in the cookie. +The primary goal is to avoid and detect situations where a session is +corrupted and the client becomes logged in as the wrong user. This +could happen via cache corruption (which we've seen before) or a +request handling bug. It's unlikely to happen again, but would be a +critical issue, so we've built in some checks to make sure the user on +the session doesn't change over the course of the session or between +the request and response phases. + +The secondary goal is to improve upon Django's session handling by +including cryptographically enforced expiration. + The implementation is inspired in part by the proposal in the paper but deviates in a number of ways; mostly it just uses the technique @@ -66,9 +77,8 @@ Custom Attributes: import inspect from base64 import b64encode -from contextlib import contextmanager from hashlib import sha256 -from logging import ERROR, getLogger +from logging import getLogger from django.conf import settings from django.contrib.auth import SESSION_KEY @@ -278,7 +288,8 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): Step 4. Once the session is retrieved, verify that the user bound in the safe_cookie_data matches the user attached to the - server's session information. + server's session information. Otherwise, reject the request + (bypass the view and return an error or redirect). Step 5. If all is successful, the now verified user_id is stored separately in the request object so it is available for another @@ -313,6 +324,8 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): if LOG_REQUEST_USER_CHANGES: log_request_user_changes(request) else: + # Return an error or redirect, and don't continue to + # the underlying view. return self._on_user_authentication_failed(request) def process_response(self, request, response): @@ -345,13 +358,12 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): if not _is_cookie_marked_for_deletion(request) and _is_cookie_present(response): try: user_id_in_session = self.get_user_id_from_session(request) - with controlled_logging(request, log): - self._verify_user(request, user_id_in_session) # Step 2 + self._verify_user(request, response, user_id_in_session) # Step 2 - # Use the user_id marked in the session instead of the - # one in the request in case the user is not set in the - # request, for example during Anonymous API access. - self.update_with_safe_session_cookie(response.cookies, user_id_in_session) # Step 3 + # Use the user_id marked in the session instead of the + # one in the request in case the user is not set in the + # request, for example during Anonymous API access. + self.update_with_safe_session_cookie(response.cookies, user_id_in_session) # Step 3 except SafeCookieError: _mark_cookie_for_deletion(request) @@ -378,12 +390,23 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): return redirect_to_login(request.path) @staticmethod - def _verify_user(request, userid_in_session): + def _verify_user(request, response, userid_in_session): """ Logs an error if the user marked at the time of process_request does not match either the current user in the request or the given userid_in_session. """ + # It's expected that a small number of views may change the + # user over the course of the request. We have exemptions for + # the user changing to/from None, but the login view can + # sometimes change the user from one value to another between + # the request and response phases, specifically when the login + # page is used during an active session. + # + # The relevant views set a flag to indicate the exemption. + if getattr(response, 'safe_sessions_expected_user_change', None): + return + if hasattr(request, 'safe_cookie_verified_user_id'): if hasattr(request.user, 'real_user'): # If a view overrode the request.user with a masqueraded user, this will @@ -431,6 +454,7 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin): except KeyError: return None + # TODO move to test code, maybe rename, get rid of old Django compat stuff @staticmethod def set_user_id_in_session(request, user): """ @@ -573,28 +597,13 @@ def log_request_user_changes(request): request.__class__ = SafeSessionRequestWrapper -def _is_from_logout(request): +def mark_user_change_as_expected(response, new_user_id): """ - Returns whether the request has come from logout action to see if - 'is_from_logout' attribute is present. - """ - return getattr(request, 'is_from_logout', False) + Indicate to the safe-sessions middleware that it is expected that + the user is changing between the request and response phase of + the current request. - -@contextmanager -def controlled_logging(request, logger): + The new_user_id may be None or an LMS user ID, and may be the same + as the previous user ID. """ - Control the logging by changing logger's level if - the request is from logout. - """ - default_level = None - from_logout = _is_from_logout(request) - if from_logout: - default_level = logger.getEffectiveLevel() - logger.setLevel(ERROR) - - try: - yield - finally: - if from_logout: - logger.setLevel(default_level) + response.safe_sessions_expected_user_change = {'new_user_id': new_user_id} diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py index 5686086152..ce49f5bd95 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py @@ -17,7 +17,7 @@ from django.test.utils import override_settings from openedx.core.djangolib.testing.utils import get_mock_request from common.djangoapps.student.tests.factories import UserFactory -from ..middleware import SafeCookieData, SafeSessionMiddleware, log_request_user_changes +from ..middleware import SafeCookieData, SafeSessionMiddleware, log_request_user_changes, mark_user_change_as_expected from .test_utils import TestSafeSessionsLogMixin @@ -263,9 +263,9 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, TestCase): settings.SESSION_COOKIE_NAME ] - def verify_success(self): + def set_up_for_success(self): """ - Verifies success path. + Set up request for success path -- everything up until process_response(). """ self.client.login(username=self.user.username, password='test') self.request.user = self.user @@ -281,6 +281,12 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, TestCase): assert self.request.safe_cookie_verified_user_id == self.user.id self.cookies_from_request_to_response() + def verify_success(self): + """ + Verifies success path. + """ + self.set_up_for_success() + with self.assert_not_logged(): response = SafeSessionMiddleware().process_response(self.request, self.client.response) assert response.status_code == 200 @@ -322,6 +328,41 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, TestCase): self.request.META = {'HTTP_USER_AGENT': 'open edX Mobile App Version 2.1'} self.verify_error(401) + def test_warn_on_user_change(self): + """ + Verifies that warnings are emitted and custom attributes set if + the user changes unexpectedly between request and response. + """ + self.set_up_for_success() + + # But then user changes unexpectedly + self.request.user = UserFactory.create() + + with self.assert_logged_for_request_user_mismatch(self.user.id, self.request.user.id, 'warning', '/'): + with patch('openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute') as mock_attr: + response = SafeSessionMiddleware().process_response(self.request, self.client.response) + assert response.status_code == 200 + mock_attr.assert_called_with("safe_sessions.user_mismatch", "request-response-mismatch") + + def test_no_warn_on_expected_user_change(self): + """ + Verifies that no warnings is emitted when the user change is expected. + This might happen on a login, for example. + """ + self.set_up_for_success() + + # User changes... + new_user = UserFactory.create() + self.request.user = new_user + # ...but so does session, and view sets a flag to say it's OK. + mark_user_change_as_expected(self.client.response, new_user.id) + + with self.assert_no_warning_logged(): + with patch('openedx.core.djangoapps.safe_sessions.middleware.set_custom_attribute') as mock_attr: + response = SafeSessionMiddleware().process_response(self.request, self.client.response) + assert response.status_code == 200 + mock_attr.assert_not_called() + @ddt.ddt class TestLogRequestUserChanges(TestCase): diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index e536be42ab..c72b28154d 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -41,6 +41,7 @@ from common.djangoapps.track import segment 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.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY from openedx.core.djangoapps.user_authn.cookies import get_response_with_refreshed_jwt_cookies, set_logged_in_cookies @@ -593,6 +594,7 @@ def login_user(request, api_version='v1'): set_custom_attribute('login_user_auth_failed_error', False) set_custom_attribute('login_user_response_status', response.status_code) set_custom_attribute('login_user_redirect_url', redirect_url) + mark_user_change_as_expected(response, user.id) return response except AuthFailedError as error: response_content = error.get_response() diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index bbbbdae7ce..ef8e3bf59a 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -11,6 +11,7 @@ from django.utils.http import urlencode from django.views.generic import TemplateView from oauth2_provider.models import Application +from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect from common.djangoapps.third_party_auth import pipeline as tpa_pipeline @@ -69,7 +70,6 @@ class LogoutView(TemplateView): def dispatch(self, request, *args, **kwargs): # We do not log here, because we have a handler registered to perform logging on successful logouts. - request.is_from_logout = True # Get third party auth provider's logout url self.tpa_logout_url = tpa_pipeline.get_idp_logout_url_from_running_pipeline(request) @@ -81,6 +81,7 @@ class LogoutView(TemplateView): # Clear the cookie used by the edx.org marketing site delete_logged_in_cookies(response) + mark_user_change_as_expected(response, None) return response def _build_logout_url(self, url): diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 91c1d9328b..bfaf36ce55 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -385,7 +385,6 @@ def _track_user_registration(user, profile, params, third_party_provider, regist # VAN-738 - added below properties to experiment marketing emails opt in/out events on Braze. if params.get('marketing_emails_opt_in') and settings.MARKETING_EMAILS_OPT_IN: properties['marketing_emails_opt_in'] = params.get('marketing_emails_opt_in') == 'true' - properties['is_active'] = params.get('marketing_emails_opt_in') == 'true' # DENG-803: For segment events forwarded along to Hubspot, duplicate the `properties` section of # the event payload into the `traits` section so that they can be received. This is a temporary diff --git a/requirements/constraints.txt b/requirements/constraints.txt index cd21cb80d7..ad097db621 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -30,7 +30,7 @@ django-storages<1.9 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==3.30.6 +edx-enterprise==3.30.10 # Newer versions need a more recent version of python-dateutil freezegun==0.3.12 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 5289827b12..c15f6e695a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -433,7 +433,7 @@ edx-drf-extensions==8.0.0 # edx-rbac # edx-when # edxval -edx-enterprise==3.30.6 +edx-enterprise==3.30.10 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.in @@ -462,7 +462,7 @@ edx-opaque-keys[django]==2.2.2 # xmodule edx-organizations==6.10.0 # via -r requirements/edx/base.in -edx-proctoring==4.1.1 +edx-proctoring==4.1.2 # via # -r requirements/edx/base.in # edx-proctoring-proctortrack diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index a5f3acdcd9..9b762139bf 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -535,7 +535,7 @@ edx-drf-extensions==8.0.0 # edx-rbac # edx-when # edxval -edx-enterprise==3.30.6 +edx-enterprise==3.30.10 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/testing.txt @@ -568,7 +568,7 @@ edx-opaque-keys[django]==2.2.2 # xmodule edx-organizations==6.10.0 # via -r requirements/edx/testing.txt -edx-proctoring==4.1.1 +edx-proctoring==4.1.2 # via # -r requirements/edx/testing.txt # edx-proctoring-proctortrack diff --git a/requirements/edx/pip-tools.txt b/requirements/edx/pip-tools.txt index 78155d000a..d5a92ca1b8 100644 --- a/requirements/edx/pip-tools.txt +++ b/requirements/edx/pip-tools.txt @@ -10,7 +10,7 @@ click==7.1.2 # pip-tools pep517==0.11.0 # via pip-tools -pip-tools==6.3.0 +pip-tools==6.4.0 # via -r requirements/edx/pip-tools.in tomli==1.2.1 # via pep517 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 9efb6f5faf..ece2103ce4 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -515,7 +515,7 @@ edx-drf-extensions==8.0.0 # edx-rbac # edx-when # edxval -edx-enterprise==3.30.6 +edx-enterprise==3.30.10 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt @@ -549,7 +549,7 @@ edx-opaque-keys[django]==2.2.2 # xmodule edx-organizations==6.10.0 # via -r requirements/edx/base.txt -edx-proctoring==4.1.1 +edx-proctoring==4.1.2 # via # -r requirements/edx/base.txt # edx-proctoring-proctortrack diff --git a/scripts/jenkins-common.sh b/scripts/jenkins-common.sh index d4048817d9..4d5bbc1512 100644 --- a/scripts/jenkins-common.sh +++ b/scripts/jenkins-common.sh @@ -48,6 +48,7 @@ source $VENV_PATH/edx-venv/bin/activate # Hack to fix up egg-link files given that the virtualenv is not relocatable sed -i "s|^/home/jenkins/shallow-clone|`pwd`|" -- \ $VENV_PATH/edx-venv/lib/python*/site-packages/*.egg-link +pip install pip==21.3 pip install -qr requirements/edx/pip-tools.txt pip-sync -q requirements/edx/testing.txt requirements/edx/django.txt diff --git a/themes/edx.org/lms/templates/certificates/_about-edx.html b/themes/edx.org/lms/templates/certificates/_about-edx.html index 99d8621454..46cf3c7483 100644 --- a/themes/edx.org/lms/templates/certificates/_about-edx.html +++ b/themes/edx.org/lms/templates/certificates/_about-edx.html @@ -1,6 +1,6 @@ <%page expression_filter="h"/> -<%! -from django.utils.translation import ugettext as _ +<%! +from django.utils.translation import ugettext as _ from openedx.core.djangolib.markup import HTML, Text %>
@@ -8,7 +8,7 @@ from openedx.core.djangolib.markup import HTML, Text