- {% filter force_escape %} - {% blocktrans with start_link="" end_link="" trimmed %} - Markdown syntax is allowed. See the {{ start_link }}cheatsheet{{ end_link }} for help. + {% blocktrans trimmed asvar tmsg %} + Markdown syntax is allowed. See the {start_link}cheatsheet{end_link} for help. {% endblocktrans %} - {% endfilter %} + {% interpolate_html tmsg start_link=''|safe end_link=''|safe %}
diff --git a/openedx/core/djangoapps/cache_toolbox/middleware.py b/openedx/core/djangoapps/cache_toolbox/middleware.py index 4ba2162fe35f..9d1ea2bf0690 100644 --- a/openedx/core/djangoapps/cache_toolbox/middleware.py +++ b/openedx/core/djangoapps/cache_toolbox/middleware.py @@ -95,6 +95,7 @@ from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user from django.utils.crypto import constant_time_compare from django.utils.deprecation import MiddlewareMixin +from edx_django_utils.monitoring import set_custom_attribute from openedx.core.djangoapps.safe_sessions.middleware import SafeSessionMiddleware, _mark_cookie_for_deletion @@ -112,6 +113,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def process_request(self, request): + set_custom_attribute('DEFAULT_HASHING_ALGORITHM', settings.DEFAULT_HASHING_ALGORITHM) try: # Try and construct a User instance from data stored in the cache session_user_id = SafeSessionMiddleware.get_user_id_from_session(request) @@ -141,9 +143,29 @@ def _verify_session_auth(self, request): auto_auth_enabled = settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING', False) if not auto_auth_enabled and hasattr(request.user, 'get_session_auth_hash'): session_hash = request.session.get(HASH_SESSION_KEY) - if not (session_hash and constant_time_compare(session_hash, request.user.get_session_auth_hash())): - # The session hash has changed due to a password - # change. Log the user out. - request.session.flush() - request.user = AnonymousUser() - _mark_cookie_for_deletion(request) + session_hash_verified = session_hash and constant_time_compare( + session_hash, request.user.get_session_auth_hash()) + + # session hash is verified from the default algo, so skip legacy check + if session_hash_verified: + set_custom_attribute('session_hash_verified', "default") + return + + if ( + session_hash and + hasattr(request.user, '_legacy_get_session_auth_hash') and + constant_time_compare( + session_hash, + request.user._legacy_get_session_auth_hash() # pylint: disable=protected-access + ) + ): + # session hash is verified from legacy hashing algorithm. + set_custom_attribute('session_hash_verified', "fallback") + return + + # The session hash has changed due to a password + # change. Log the user out. + request.session.flush() + request.user = AnonymousUser() + _mark_cookie_for_deletion(request) + set_custom_attribute('failed_session_verification', True) diff --git a/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py b/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py index 21547b69ca1c..a55090b1207b 100644 --- a/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py +++ b/openedx/core/djangoapps/cache_toolbox/tests/test_middleware.py @@ -1,17 +1,18 @@ """Tests for cached authentication middleware.""" -from unittest.mock import patch +from unittest.mock import call, patch +import django from django.conf import settings -from django.contrib.auth.models import User, AnonymousUser # lint-amnesty, pylint: disable=imported-auth-user -from django.urls import reverse -from django.test import TestCase from django.contrib.auth import SESSION_KEY +from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user from django.http import HttpResponse, SimpleCookie +from django.test import TestCase +from django.urls import reverse +from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.cache_toolbox.middleware import CacheBackedAuthenticationMiddleware from openedx.core.djangoapps.safe_sessions.middleware import SafeCookieData, SafeSessionMiddleware -from openedx.core.djangolib.testing.utils import skip_unless_cms, skip_unless_lms, get_mock_request -from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangolib.testing.utils import get_mock_request, skip_unless_cms, skip_unless_lms class CachedAuthMiddlewareTestCase(TestCase): @@ -36,9 +37,68 @@ def _test_change_session_hash(self, test_url, redirect_url, target_status_code=2 """ response = self.client.get(test_url) assert response.status_code == 200 - with patch.object(User, 'get_session_auth_hash', return_value='abc123'): - response = self.client.get(test_url) - self.assertRedirects(response, redirect_url, target_status_code=target_status_code) + + with patch( + "openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute" + ) as mock_set_custom_attribute: + with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True): + # Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not + # Remove once we reach Django 4 + if hasattr(User, '_legacy_get_session_auth_hash'): + with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'): + response = self.client.get(test_url) + else: + response = self.client.get(test_url) + + self.assertRedirects(response, redirect_url, target_status_code=target_status_code) + mock_set_custom_attribute.assert_any_call('failed_session_verification', True) + + def _test_custom_attribute_after_changing_hash(self, test_url, mock_set_custom_attribute): + """verify that set_custom_attribute is called with expected values""" + password = 'test-password' + + # Test DEFAULT_HASHING_ALGORITHM of 'sha1' for both login and client get + with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'): + self.client.login(username=self.user.username, password=password) + self.client.get(test_url) + # For Django 3.2, the setting 'sha1' applies and is the "default". + # For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default". + mock_set_custom_attribute.assert_has_calls([ + call('DEFAULT_HASHING_ALGORITHM', 'sha1'), + call('session_hash_verified', "default"), + ]) + mock_set_custom_attribute.reset_mock() + + # Test DEFAULT_HASHING_ALGORITHM of 'sha1' for login and switch to 'sha256' for client get. + with self.settings(DEFAULT_HASHING_ALGORITHM='sha1'): + self.client.login(username=self.user.username, password=password) + with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'): + self.client.get(test_url) + if django.VERSION < (4, 0): + # For Django 3.2, the setting 'sha1' applies to login, and uses 'she256' for client get, + # and should "fallback" to 'sha1". + mock_set_custom_attribute.assert_has_calls([ + call('DEFAULT_HASHING_ALGORITHM', 'sha256'), + call('session_hash_verified', "fallback"), + ]) + else: + # For Django 4, the setting no longer applies, and again 'sha256' will be used for both as the "default". + mock_set_custom_attribute.assert_has_calls([ + call('DEFAULT_HASHING_ALGORITHM', 'sha256'), + call('session_hash_verified', "default"), + ]) + mock_set_custom_attribute.reset_mock() + + # Test DEFAULT_HASHING_ALGORITHM of 'sha256' for both login and client get + with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'): + self.client.login(username=self.user.username, password=password) + self.client.get(test_url) + # For Django 3.2, the setting 'sha256' applies and is the "default". + # For Django 4, the setting no longer applies, and 'sha256' will be used for both as the "default". + mock_set_custom_attribute.assert_has_calls([ + call('DEFAULT_HASHING_ALGORITHM', 'sha256'), + call('session_hash_verified', "default"), + ]) @skip_unless_lms def test_session_change_lms(self): @@ -53,6 +113,20 @@ def test_session_change_cms(self): # Studio login redirects to LMS login self._test_change_session_hash(home_url, settings.LOGIN_URL + '?next=' + home_url, target_status_code=302) + @skip_unless_lms + @patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute") + def test_custom_attribute_after_changing_hash_lms(self, mock_set_custom_attribute): + """Test set_custom_attribute is called with expected values in LMS""" + test_url = reverse('dashboard') + self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute) + + @skip_unless_cms + @patch("openedx.core.djangoapps.cache_toolbox.middleware.set_custom_attribute") + def test_custom_attribute_after_changing_hash_cms(self, mock_set_custom_attribute): + """Test set_custom_attribute is called with expected values in CMS""" + test_url = reverse('home') + self._test_custom_attribute_after_changing_hash(test_url, mock_set_custom_attribute) + def test_user_logout_on_session_hash_change(self): """ Verify that if a user's session auth hash and the request's hash @@ -75,9 +149,18 @@ def test_user_logout_on_session_hash_change(self): assert self.client.response.cookies.get(settings.SESSION_COOKIE_NAME).value == session_id assert self.client.response.cookies.get('edx-jwt-cookie-header-payload').value == 'test-jwt-payload' - with patch.object(User, 'get_session_auth_hash', return_value='abc123'): - CacheBackedAuthenticationMiddleware().process_request(self.request) - SafeSessionMiddleware().process_response(self.request, self.client.response) + with patch.object(User, 'get_session_auth_hash', return_value='abc123', autospec=True): + # Django 3.2 has _legacy_get_session_auth_hash, and Django 4 does not + # Remove once we reach Django 4 + if hasattr(User, '_legacy_get_session_auth_hash'): + with patch.object(User, '_legacy_get_session_auth_hash', return_value='abc123'): + CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request) + + else: + CacheBackedAuthenticationMiddleware(get_response=lambda request: None).process_request(self.request) + SafeSessionMiddleware(get_response=lambda request: None).process_response( + self.request, self.client.response + ) # asserts that user, session, and JWT cookies do not exist assert self.request.session.get(SESSION_KEY) is None diff --git a/openedx/core/djangoapps/models/tests/test_course_details.py b/openedx/core/djangoapps/models/tests/test_course_details.py index 98e3c4b15df0..86301740c7f1 100644 --- a/openedx/core/djangoapps/models/tests/test_course_details.py +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -4,6 +4,7 @@ import datetime +from django.test import override_settings import pytest import ddt from pytz import UTC @@ -29,27 +30,37 @@ class CourseDetailsTestCase(ModuleStoreTestCase): def setUp(self): super().setUp() - self.course = CourseFactory.create() - - def test_virgin_fetch(self): - details = CourseDetails.fetch(self.course.id) - assert details.org == self.course.location.org, 'Org not copied into' - assert details.course_id == self.course.location.course, 'Course_id not copied into' - assert details.run == self.course.location.run, 'Course run not copied into' - assert details.course_image_name == self.course.course_image - assert details.start_date.tzinfo is not None - assert details.end_date is None, ('end date somehow initialized ' + str(details.end_date)) - assert details.enrollment_start is None,\ - ('enrollment_start date somehow initialized ' + str(details.enrollment_start)) - assert details.enrollment_end is None,\ - ('enrollment_end date somehow initialized ' + str(details.enrollment_end)) - assert details.certificate_available_date is None,\ - ('certificate_available_date date somehow initialized ' + str(details.certificate_available_date)) - assert details.syllabus is None, ('syllabus somehow initialized' + str(details.syllabus)) - assert details.intro_video is None, ('intro_video somehow initialized' + str(details.intro_video)) - assert details.effort is None, ('effort somehow initialized' + str(details.effort)) - assert details.language is None, ('language somehow initialized' + str(details.language)) - assert not details.self_paced + self.course = CourseFactory.create(default_enrollment_start=True) + + @ddt.data(True, False) + def test_virgin_fetch(self, should_have_default_enroll_start): + features = settings.FEATURES.copy() + features['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] = should_have_default_enroll_start + + with override_settings(FEATURES=features): + course = CourseFactory.create(default_enrollment_start=should_have_default_enroll_start) + details = CourseDetails.fetch(course.id) + wrong_enrollment_start_msg = ( + 'enrollment_start not copied into' + if should_have_default_enroll_start + else f'enrollment_start date somehow initialized {str(details.enrollment_start)}' + ) + assert details.org == course.location.org, 'Org not copied into' + assert details.course_id == course.location.course, 'Course_id not copied into' + assert details.run == course.location.run, 'Course run not copied into' + assert details.course_image_name == course.course_image + assert details.start_date.tzinfo is not None + assert details.end_date is None, ('end date somehow initialized ' + str(details.end_date)) + assert details.enrollment_start == course.enrollment_start, wrong_enrollment_start_msg + assert details.enrollment_end is None,\ + ('enrollment_end date somehow initialized ' + str(details.enrollment_end)) + assert details.certificate_available_date is None,\ + ('certificate_available_date date somehow initialized ' + str(details.certificate_available_date)) + assert details.syllabus is None, ('syllabus somehow initialized' + str(details.syllabus)) + assert details.intro_video is None, ('intro_video somehow initialized' + str(details.intro_video)) + assert details.effort is None, ('effort somehow initialized' + str(details.effort)) + assert details.language is None, ('language somehow initialized' + str(details.language)) + assert not details.self_paced def test_update_and_fetch(self): jsondetails = CourseDetails.fetch(self.course.id) diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py index f23773b842a7..babc3160f81e 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_middleware.py @@ -223,6 +223,16 @@ def test_update_cookie_data_at_step_3(self): assert safe_cookie_data.session_id == 'some_session_id' assert safe_cookie_data.verify(self.user.id) + def test_update_cookie_data_at_step_3_with_sha256(self): + """ first encode cookie with default algo sha1 and then check with sha256""" + self.assert_response(set_request_user=True, set_session_cookie=True) + serialized_cookie_data = self.client.response.cookies[settings.SESSION_COOKIE_NAME].value + safe_cookie_data = SafeCookieData.parse(serialized_cookie_data) + assert safe_cookie_data.version == SafeCookieData.CURRENT_VERSION + assert safe_cookie_data.session_id == 'some_session_id' + with self.settings(DEFAULT_HASHING_ALGORITHM='sha256'): + assert safe_cookie_data.verify(self.user.id) + def test_cant_update_cookie_at_step_3_error(self): self.client.response.cookies[settings.SESSION_COOKIE_NAME] = None with self.assert_invalid_session_id(): diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py index b8a23c567d24..bbad0e85851d 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py @@ -234,5 +234,5 @@ def test_pinned_values(self): "|HvGnjXf1b3jU" "|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi" ":1m6Hve" - ":OMhY2FL2pudJjSSXChtI-zR8QVA" + ":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY" ) diff --git a/openedx/core/djangoapps/user_authn/api/tests/test_data.py b/openedx/core/djangoapps/user_authn/api/tests/test_data.py new file mode 100644 index 000000000000..05d0f7cf6b8e --- /dev/null +++ b/openedx/core/djangoapps/user_authn/api/tests/test_data.py @@ -0,0 +1,141 @@ +""" Mocked data for testing """ + +mfe_context_data_keys = { + 'contextData', + 'registrationFields', + 'optionalFields' +} + +mock_mfe_context_data = { + 'context_data': { + 'currentProvider': 'edX', + 'platformName': 'edX', + 'providers': [ + { + 'id': 'oa2-facebook', + 'name': 'Facebook', + 'iconClass': 'fa-facebook', + 'iconImage': None, + 'skipHintedLogin': False, + 'skipRegistrationForm': False, + 'loginUrl': 'https://facebook.com/login', + 'registerUrl': 'https://facebook.com/register' + }, + { + 'id': 'oa2-google-oauth2', + 'name': 'Google', + 'iconClass': 'fa-google-plus', + 'iconImage': None, + 'skipHintedLogin': False, + 'skipRegistrationForm': False, + 'loginUrl': 'https://google.com/login', + 'registerUrl': 'https://google.com/register' + } + ], + 'secondaryProviders': [], + 'finishAuthUrl': 'https://edx.com/auth/finish', + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'pipeline_user_details': { + 'username': 'test123', + 'email': 'test123@edx.com', + 'fullname': 'Test Test', + 'first_name': 'Test', + 'last_name': 'Test' + } + }, + 'registration_fields': {}, + 'optional_fields': { + 'extended_profile': [] + } +} + +mock_default_mfe_context_data = { + 'context_data': { + 'currentProvider': None, + 'platformName': 'édX', + 'providers': [], + 'secondaryProviders': [], + 'finishAuthUrl': None, + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'pipeline_user_details': {} + }, + 'registration_fields': {}, + 'optional_fields': { + 'extended_profile': [] + } +} + +expected_mfe_context_data = { + 'contextData': { + 'currentProvider': 'edX', + 'platformName': 'edX', + 'providers': [ + { + 'id': 'oa2-facebook', + 'name': 'Facebook', + 'iconClass': 'fa-facebook', + 'iconImage': None, + 'skipHintedLogin': False, + 'skipRegistrationForm': False, + 'loginUrl': 'https://facebook.com/login', + 'registerUrl': 'https://facebook.com/register' + }, + { + 'id': 'oa2-google-oauth2', + 'name': 'Google', + 'iconClass': 'fa-google-plus', + 'iconImage': None, + 'skipHintedLogin': False, + 'skipRegistrationForm': False, + 'loginUrl': 'https://google.com/login', + 'registerUrl': 'https://google.com/register' + } + ], + 'secondaryProviders': [], + 'finishAuthUrl': 'https://edx.com/auth/finish', + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'pipelineUserDetails': { + 'username': 'test123', + 'email': 'test123@edx.com', + 'name': 'Test Test', + 'firstName': 'Test', + 'lastName': 'Test' + } + }, + 'registrationFields': {}, + 'optionalFields': { + 'extended_profile': [] + } +} + +default_expected_mfe_context_data = { + 'contextData': { + 'currentProvider': None, + 'platformName': 'édX', + 'providers': [], + 'secondaryProviders': [], + 'finishAuthUrl': None, + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'pipelineUserDetails': {} + }, + 'registrationFields': {}, + 'optionalFields': { + 'extended_profile': [] + } +} diff --git a/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py b/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py new file mode 100644 index 000000000000..348292b172f0 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py @@ -0,0 +1,44 @@ +"""Tests for serializers for the MFE Context""" + +from django.test import TestCase + +from openedx.core.djangoapps.user_authn.api.tests.test_data import ( + mock_mfe_context_data, + expected_mfe_context_data, + mock_default_mfe_context_data, + default_expected_mfe_context_data, +) +from openedx.core.djangoapps.user_authn.serializers import MFEContextSerializer + + +class TestMFEContextSerializer(TestCase): + """ + High-level unit tests for MFEContextSerializer + """ + + def test_mfe_context_serializer(self): + """ + Test MFEContextSerializer with mock data that serializes data correctly + """ + + output_data = MFEContextSerializer( + mock_mfe_context_data + ).data + + self.assertDictEqual( + output_data, + expected_mfe_context_data + ) + + def test_mfe_context_serializer_default_response(self): + """ + Test MFEContextSerializer with default data + """ + serialized_data = MFEContextSerializer( + mock_default_mfe_context_data + ).data + + self.assertDictEqual( + serialized_data, + default_expected_mfe_context_data + ) diff --git a/openedx/core/djangoapps/user_authn/api/tests/test_views.py b/openedx/core/djangoapps/user_authn/api/tests/test_views.py index af079f6d7ca2..d8ab2c37c1a1 100644 --- a/openedx/core/djangoapps/user_authn/api/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/api/tests/test_views.py @@ -16,9 +16,10 @@ from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.third_party_auth import pipeline from common.djangoapps.third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline -from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration from openedx.core.djangoapps.geoinfo.api import country_code_from_ip +from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase +from openedx.core.djangoapps.user_authn.api.tests.test_data import mfe_context_data_keys from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -42,6 +43,7 @@ def setUp(self): # pylint: disable=arguments-differ hostname = socket.gethostname() ip_address = socket.gethostbyname(hostname) self.country_code = country_code_from_ip(ip_address) + self.pipeline_user_details = {} # Several third party auth providers are created for these tests: self.configure_google_provider(enabled=True, visible=True) @@ -93,8 +95,20 @@ def get_context(self, params=None, current_provider=None, backend_name=None, add """ Returns the MFE context """ + + if add_user_details: + self.pipeline_user_details.update( + { + 'username': None, + 'email': 'test@test.com', + 'name': None, + 'firstName': None, + 'lastName': None + } + ) + return { - 'context_data': { + 'contextData': { 'currentProvider': current_provider, 'platformName': settings.PLATFORM_NAME, 'providers': self.get_provider_data(params) if params else [], @@ -102,12 +116,13 @@ def get_context(self, params=None, current_provider=None, backend_name=None, add 'finishAuthUrl': pipeline.get_complete_url(backend_name) if backend_name else None, 'errorMessage': None, 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, 'syncLearnerProfileData': False, - 'pipeline_user_details': {'email': 'test@test.com'} if add_user_details else {}, - 'countryCode': self.country_code + 'countryCode': self.country_code, + 'pipelineUserDetails': self.pipeline_user_details, }, - 'registration_fields': {}, - 'optional_fields': { + 'registrationFields': {}, + 'optionalFields': { 'extended_profile': [], }, } @@ -182,7 +197,7 @@ def test_tpa_hint(self): }) response = self.client.get(self.url, self.query_params) - assert response.data['context_data']['providers'] == provider_data + assert response.data['contextData']['providers'] == provider_data def test_user_country_code(self): """ @@ -191,7 +206,7 @@ def test_user_country_code(self): response = self.client.get(self.url, self.query_params) assert response.status_code == 200 - assert response.data['context_data']['countryCode'] == self.country_code + assert response.data['contextData']['countryCode'] == self.country_code @override_settings( ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, @@ -205,7 +220,7 @@ def test_required_fields_not_configured(self): self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) assert response.status_code == status.HTTP_200_OK - assert response.data['registration_fields']['fields'] == {} + assert response.data['registrationFields']['fields'] == {} @with_site_configuration( configuration={ @@ -223,8 +238,9 @@ def test_required_field_order(self): """ self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) + assert response.status_code == status.HTTP_200_OK - assert list(response.data['registration_fields']['fields'].keys()) == ['first_name', 'last_name', 'state'] + assert list(response.data['registrationFields']['fields'].keys()) == ['first_name', 'last_name', 'state'] @override_settings( ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, @@ -248,7 +264,7 @@ def test_optional_field_has_no_description(self): self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) assert response.status_code == status.HTTP_200_OK - assert response.data['optional_fields']['fields'] == expected_response + assert response.data['optionalFields']['fields'] == expected_response @with_site_configuration( configuration={ @@ -282,8 +298,9 @@ def test_configurable_select_option_fields(self): } self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) + assert response.status_code == status.HTTP_200_OK - assert response.data['optional_fields']['fields'] == expected_response + assert response.data['optionalFields']['fields'] == expected_response @with_site_configuration( configuration={ @@ -302,7 +319,7 @@ def test_optional_field_order(self): self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) assert response.status_code == status.HTTP_200_OK - assert list(response.data['optional_fields']['fields'].keys()) == ['specialty', 'goals'] + assert list(response.data['optionalFields']['fields'].keys()) == ['specialty', 'goals'] @with_site_configuration( configuration={ @@ -322,7 +339,7 @@ def test_field_not_available_in_extended_profile_config(self): self.query_params.update({'is_register_page': True}) response = self.client.get(self.url, self.query_params) assert response.status_code == status.HTTP_200_OK - assert list(response.data['registration_fields']['fields'].keys()) == ['specialty'] + assert list(response.data['registrationFields']['fields'].keys()) == ['specialty'] @override_settings( ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, @@ -333,9 +350,34 @@ def test_response_structure(self): Test that API return valid response dictionary with both required and optional fields """ response = self.client.get(self.url, self.query_params) - assert response.data == self.get_context() + def test_mfe_context_api_serialized_response(self): + """ + Test MFE Context API serialized response + """ + response = self.client.get(self.url, self.query_params) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + params = { + 'next': self.query_params['next'] + } + + self.assertEqual( + response.data, + self.get_context(params) + ) + + def test_mfe_context_api_response_keys(self): + """ + Test MFE Context API response keys + """ + response = self.client.get(self.url, self.query_params) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + response_keys = set(response.data.keys()) + self.assertSetEqual(response_keys, mfe_context_data_keys) + @skip_unless_lms class SendAccountActivationEmail(UserAPITestCase): diff --git a/openedx/core/djangoapps/user_authn/api/views.py b/openedx/core/djangoapps/user_authn/api/views.py index 7e84a6747301..2fbc014f52fb 100644 --- a/openedx/core/djangoapps/user_authn/api/views.py +++ b/openedx/core/djangoapps/user_authn/api/views.py @@ -15,6 +15,7 @@ from common.djangoapps.student.views import compose_and_send_activation_email from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_authn.api.helper import RegistrationFieldsContext +from openedx.core.djangoapps.user_authn.serializers import MFEContextSerializer from openedx.core.djangoapps.user_authn.views.utils import get_mfe_context @@ -65,6 +66,7 @@ def get(self, request, **kwargs): # lint-amnesty, pylint: disable=unused-argume context['registration_fields'].update({ 'fields': registration_fields, }) + optional_fields = RegistrationFieldsContext('optional').get_fields() if optional_fields: context['optional_fields'].update({ @@ -74,7 +76,9 @@ def get(self, request, **kwargs): # lint-amnesty, pylint: disable=unused-argume return Response( status=status.HTTP_200_OK, - data=context + data=MFEContextSerializer( + context + ).data ) diff --git a/openedx/core/djangoapps/user_authn/serializers.py b/openedx/core/djangoapps/user_authn/serializers.py new file mode 100644 index 000000000000..9bbed367e6e2 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/serializers.py @@ -0,0 +1,82 @@ +""" +MFE Context API Serializers +""" + +from rest_framework import serializers + + +class ProvidersSerializer(serializers.Serializer): + """ + Providers Serializers + """ + + id = serializers.CharField(allow_null=True) + name = serializers.CharField(allow_null=True) + iconClass = serializers.CharField(allow_null=True) + iconImage = serializers.CharField(allow_null=True) + skipHintedLogin = serializers.BooleanField(default=False) + skipRegistrationForm = serializers.BooleanField(default=False) + loginUrl = serializers.CharField(allow_null=True) + registerUrl = serializers.CharField(allow_null=True) + + +class PipelineUserDetailsSerializer(serializers.Serializer): + """ + Pipeline User Details Serializers + """ + + username = serializers.CharField(allow_null=True) + email = serializers.CharField(allow_null=True) + name = serializers.CharField(source='fullname', allow_null=True) + firstName = serializers.CharField(source='first_name', allow_null=True) + lastName = serializers.CharField(source='last_name', allow_null=True) + + +class ContextDataSerializer(serializers.Serializer): + """ + Context Data Serializers + """ + + currentProvider = serializers.CharField(allow_null=True) + platformName = serializers.CharField(allow_null=True) + providers = serializers.ListField( + child=ProvidersSerializer(), + allow_null=True + ) + secondaryProviders = serializers.ListField( + child=ProvidersSerializer(), + allow_null=True + ) + finishAuthUrl = serializers.CharField(allow_null=True) + errorMessage = serializers.CharField(allow_null=True) + registerFormSubmitButtonText = serializers.CharField(allow_null=True) + autoSubmitRegForm = serializers.BooleanField(default=False) + syncLearnerProfileData = serializers.BooleanField(default=False) + countryCode = serializers.CharField(allow_null=True) + pipelineUserDetails = serializers.SerializerMethodField() + + def get_pipelineUserDetails(self, obj): + if obj.get('pipeline_user_details'): + return PipelineUserDetailsSerializer(obj.get('pipeline_user_details')).data + return {} + + +class MFEContextSerializer(serializers.Serializer): + """ + Serializer class to convert the keys of MFE Context Response dict object to camelCase format. + """ + + contextData = ContextDataSerializer( + source='context_data', + default={} + ) + registrationFields = serializers.DictField( + source='registration_fields', + default={} + ) + optionalFields = serializers.DictField( + source='optional_fields', + default={ + 'extended_profile': [] + } + ) diff --git a/openedx/core/djangoapps/user_authn/tasks.py b/openedx/core/djangoapps/user_authn/tasks.py index 27ddfe466e3b..9c2c4ef1098c 100644 --- a/openedx/core/djangoapps/user_authn/tasks.py +++ b/openedx/core/djangoapps/user_authn/tasks.py @@ -45,7 +45,7 @@ def check_pwned_password_and_send_track_event(user_id, password, internal_user=F @shared_task(bind=True, default_retry_delay=30, max_retries=2) @set_code_owner_attribute -def send_activation_email(self, msg_string, from_address=None): +def send_activation_email(self, msg_string, from_address=None, site_id=None): """ Sending an activation email to the user. """ @@ -62,7 +62,7 @@ def send_activation_email(self, msg_string, from_address=None): dest_addr = msg.recipient.email_address - site = Site.objects.get_current() + site = Site.objects.get(id=site_id) if site_id else Site.objects.get_current() user = User.objects.get(id=msg.recipient.lms_user_id) try: diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index c494f8e015c2..939f2cca7ece 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -200,10 +200,9 @@ def create_account_with_params(request, params): # pylint: disable=too-many-sta set_custom_attribute('register_user_tpa', pipeline.running(request)) extended_profile_fields = configuration_helpers.get_value('extended_profile_fields', []) # Can't have terms of service for certain SHIB users, like at Stanford - registration_fields = getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) tos_required = ( - registration_fields.get('terms_of_service') != 'hidden' or - registration_fields.get('honor_code') != 'hidden' + extra_fields.get('terms_of_service') != 'hidden' or + extra_fields.get('honor_code') != 'hidden' ) form = AccountCreationForm( diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 3a2b9aee8a9e..6efe7984212b 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -140,7 +140,12 @@ openedx-blockstore path piexif # Exif image metadata manipulation, used in the profile_images app Pillow # Image manipulation library; used for course assets, profile images, invoice PDFs, etc. -py2neo # Driver for converting Python modulestore structures to Neo4j's schema (for Coursegraph). + +# Driver for converting Python modulestore structures to Neo4j's schema (for Coursegraph). +# Using the fork because official package has been removed from PyPI/GitHub +# Follow up issue to remove this fork: https://github.com/openedx/edx-platform/issues/33456 +https://github.com/overhangio/py2neo/releases/download/2021.2.3/py2neo-2021.2.3.tar.gz + pycountry pycryptodomex pygments # Used to support colors in paver command output diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 0a2b02dd3ad1..27737d29504a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -177,7 +177,7 @@ defusedxml==0.7.1 # social-auth-core deprecated==1.2.13 # via jwcrypto -django==3.2.20 +django==3.2.21 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.in @@ -827,7 +827,7 @@ psutil==5.9.5 # via # -r requirements/edx/paver.txt # edx-django-utils -py2neo==2021.2.3 +py2neo @ https://github.com/overhangio/py2neo/releases/download/2021.2.3/py2neo-2021.2.3.tar.gz # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index b419cdf16d8a..53d53ed50a76 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -273,7 +273,7 @@ distlib==0.3.6 # via # -r requirements/edx/testing.txt # virtualenv -django==3.2.20 +django==3.2.21 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/testing.txt @@ -1127,7 +1127,7 @@ py==1.11.0 # via # -r requirements/edx/testing.txt # tox -py2neo==2021.2.3 +py2neo @ https://github.com/overhangio/py2neo/releases/download/2021.2.3/py2neo-2021.2.3.tar.gz # via # -c requirements/edx/../constraints.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 39b22737bb2f..662efff18eea 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -256,7 +256,7 @@ dill==0.3.6 # via pylint distlib==0.3.6 # via virtualenv -django==3.2.20 +django==3.2.21 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt @@ -1065,7 +1065,7 @@ psutil==5.9.5 # pytest-xdist py==1.11.0 # via tox -py2neo==2021.2.3 +py2neo @ https://github.com/overhangio/py2neo/releases/download/2021.2.3/py2neo-2021.2.3.tar.gz # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/xmodule/course_block.py b/xmodule/course_block.py index 27b5cabd4464..d97f4cbc48aa 100644 --- a/xmodule/course_block.py +++ b/xmodule/course_block.py @@ -11,6 +11,7 @@ import requests from django.conf import settings from django.core.validators import validate_email +from edx_toggles.toggles import SettingDictToggle from lazy import lazy from lxml import etree from path import Path as path @@ -54,6 +55,22 @@ COURSE_VISIBILITY_PUBLIC_OUTLINE = 'public_outline' COURSE_VISIBILITY_PUBLIC = 'public' +# .. toggle_name: FEATURES['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] +# .. toggle_implementation: SettingDictToggle +# .. toggle_default: False +# .. toggle_description: The default behavior, when this is disabled, is that a newly created course has no +# enrollment_start date set. When the feature is enabled - the newly created courses will have the +# enrollment_start_date set to DEFAULT_START_DATE. This is intended to be a permanent option. +# This toggle affects the course listing pages (platform's index page, /courses page) when course search is +# performed using the `lms.djangoapp.branding.get_visible_courses` method and the +# COURSE_CATALOG_VISIBILITY_PERMISSION setting is set to 'see_exists'. Switching the toggle to True will prevent +# the newly created (empty) course from appearing in the course listing. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2023-06-22 +CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE = SettingDictToggle( + "FEATURES", "CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE", default=False, module_name=__name__ +) + class StringOrDate(Date): # lint-amnesty, pylint: disable=missing-class-docstring def from_json(self, value): # lint-amnesty, pylint: disable=arguments-differ @@ -311,7 +328,11 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring ) wiki_slug = String(help=_("Slug that points to the wiki for this course"), scope=Scope.content) - enrollment_start = Date(help=_("Date that enrollment for this class is opened"), scope=Scope.settings) + enrollment_start = Date( + help=_("Date that enrollment for this class is opened"), + default=DEFAULT_START_DATE if CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE.is_enabled() else None, + scope=Scope.settings + ) enrollment_end = Date(help=_("Date that enrollment for this class is closed"), scope=Scope.settings) start = Date( help=_("Start time when this block is visible"), @@ -729,7 +750,7 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring invitation_only = Boolean( display_name=_("Invitation Only"), help=_("Whether to restrict enrollment to invitation by the course staff."), - default=False, + default=True, scope=Scope.settings ) diff --git a/xmodule/modulestore/tests/factories.py b/xmodule/modulestore/tests/factories.py index b740b10e053f..9e0a2aa5cfd1 100644 --- a/xmodule/modulestore/tests/factories.py +++ b/xmodule/modulestore/tests/factories.py @@ -123,6 +123,11 @@ def _create(cls, target_class, **kwargs): # lint-amnesty, pylint: disable=argum run = kwargs.pop('run', name) user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test) emit_signals = kwargs.pop('emit_signals', False) + # By default course has enrollment_start in the future which means course is closed for enrollment. + # We're setting the 'enrollment_start' field to None to reduce number of arguments needed to setup course. + # Use the 'default_enrollment_start=True' kwarg to skip this and use the default enrollment_start date. + if not kwargs.get('enrollment_start', kwargs.pop('default_enrollment_start', False)): + kwargs['enrollment_start'] = None # Pass the metadata just as field=value pairs kwargs.update(kwargs.pop('metadata', {})) diff --git a/xmodule/tests/test_course_block.py b/xmodule/tests/test_course_block.py index 183a4f9c069e..86a98e5ffd57 100644 --- a/xmodule/tests/test_course_block.py +++ b/xmodule/tests/test_course_block.py @@ -4,20 +4,22 @@ import itertools import unittest from datetime import datetime, timedelta +import sys from unittest.mock import Mock, patch -import pytest import ddt from dateutil import parser from django.conf import settings from django.test import override_settings from fs.memoryfs import MemoryFS from opaque_keys.edx.keys import CourseKey +import pytest from pytz import utc from xblock.runtime import DictKeyValueStore, KvsFieldData from openedx.core.lib.teams_config import TeamsConfig, DEFAULT_COURSE_RUN_MAX_TEAM_SIZE import xmodule.course_block +from xmodule.course_metadata_utils import DEFAULT_START_DATE from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.xml import ImportSystem, XMLModuleStore from xmodule.modulestore.exceptions import InvalidProctoringProvider @@ -32,10 +34,22 @@ _NEXT_WEEK = _TODAY + timedelta(days=7) -class CourseFieldsTestCase(unittest.TestCase): +@ddt.ddt() +class CourseFieldsTestCase(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring def test_default_start_date(self): - assert xmodule.course_block.CourseFields.start.default == datetime(2030, 1, 1, tzinfo=utc) + assert xmodule.course_block.CourseFields.start.default == DEFAULT_START_DATE + + @ddt.data(True, False) + def test_default_enrollment_start_date(self, should_have_default_enroll_start): + features = settings.FEATURES.copy() + features['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] = should_have_default_enroll_start + with override_settings(FEATURES=features): + # reimport, so settings override could take effect + del sys.modules['xmodule.course_block'] + import xmodule.course_block # lint-amnesty, pylint: disable=redefined-outer-name, reimported + expected = DEFAULT_START_DATE if should_have_default_enroll_start else None + assert xmodule.course_block.CourseFields.enrollment_start.default == expected class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring