diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb index 3bd749e071..963f72622f 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/course_home_api/dates/views.py b/lms/djangoapps/course_home_api/dates/views.py index 3ca59b6bdc..6c95f82349 100644 --- a/lms/djangoapps/course_home_api/dates/views.py +++ b/lms/djangoapps/course_home_api/dates/views.py @@ -13,9 +13,10 @@ from rest_framework.response import Response from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_home_api.dates.serializers import DatesTabSerializer +from lms.djangoapps.course_home_api.utils import get_course_or_403 from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs -from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_with_access +from lms.djangoapps.courseware.courses import get_course_date_blocks from lms.djangoapps.courseware.date_summary import TodaysDate from lms.djangoapps.courseware.masquerade import setup_masquerade from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser @@ -59,6 +60,7 @@ class DatesTabView(RetrieveAPIView): * 200 on success with above fields. * 401 if the user is not authenticated. + * 403 if the user does not have access to the course. * 404 if the course is not available or cannot be seen. """ @@ -79,7 +81,7 @@ class DatesTabView(RetrieveAPIView): monitoring_utils.set_custom_attribute('user_id', request.user.id) monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) + course = get_course_or_403(request.user, 'load', course_key, check_if_enrolled=False) is_staff = bool(has_access(request.user, 'staff', course_key)) _, request.user = setup_masquerade( diff --git a/lms/djangoapps/course_home_api/outline/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py index 20b205e304..29b46d30e9 100644 --- a/lms/djangoapps/course_home_api/outline/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -109,6 +109,16 @@ class ResumeCourseSerializer(serializers.Serializer): url = serializers.URLField() +class OutlineTabCourseAccessRedirectSerializer(serializers.Serializer): + """ + Serializer for a Course Access Redirect response from the outline tab + """ + url = serializers.URLField() + error_code = serializers.CharField(source='access_error.error_code') + developer_message = serializers.CharField(source='access_error.developer_message') + user_message = serializers.CharField(source='access_error.user_message') + + class OutlineTabSerializer(DatesBannerSerializer, VerifiedModeSerializer): """ Serializer for the Outline Tab diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index 253779005e..7650c2ae53 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -27,10 +27,13 @@ from lms.djangoapps.course_goals.api import ( get_course_goal, ) from lms.djangoapps.course_goals.models import CourseGoal -from lms.djangoapps.course_home_api.outline.serializers import OutlineTabSerializer +from lms.djangoapps.course_home_api.outline.serializers import ( + OutlineTabSerializer, +) +from lms.djangoapps.course_home_api.utils import get_course_or_403 from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs -from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_info_section, get_course_with_access +from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_info_section from lms.djangoapps.courseware.date_summary import TodaysDate from lms.djangoapps.courseware.masquerade import is_masquerading, setup_masquerade from lms.djangoapps.courseware.views.views import get_cert_data @@ -76,7 +79,9 @@ class OutlineTabView(RetrieveAPIView): **Response Values** - Body consists of the following fields: + Body consists of two possible shapes. + + For a good 200 response, the response will include: access_expiration: An object detailing when access to this course will expire expiration_date: (str) When the access expires, in ISO 8601 notation @@ -143,9 +148,19 @@ class OutlineTabView(RetrieveAPIView): welcome_message_html: (str) Raw HTML for the course updates banner user_has_passing_grade: (bool) Whether the user currently is passing the course + If the learner does not have access to the course for a specific reason and should be redirected this + view will return a 403 and the following data: + + url: (str) The URL to which the user should be redirected + error_code: (str) A system identifier for the reason the user is being redirected + developer_message: (str) A message explaining why the user is being redirected, + intended for developer debugging. + user_message: (str) A message explaining why the user is being redirected, intended to be shown to the user. + **Returns** - * 200 on success with above fields. + * 200 on success. + * 403 if the user does not currently have access to the course and should be redirected. * 404 if the course is not available or cannot be seen. """ @@ -167,7 +182,7 @@ class OutlineTabView(RetrieveAPIView): monitoring_utils.set_custom_attribute('user_id', request.user.id) monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) + course = get_course_or_403(request.user, 'load', course_key, check_if_enrolled=False) masquerade_object, request.user = setup_masquerade( request, @@ -365,7 +380,7 @@ def dismiss_welcome_message(request): # pylint: disable=missing-function-docstr try: course_key = CourseKey.from_string(course_id) - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) + course = get_course_or_403(request.user, 'load', course_key, check_if_enrolled=True) dismiss_current_update_for_user(request, course) return Response({'message': _('Welcome message successfully dismissed.')}) except Exception: diff --git a/lms/djangoapps/course_home_api/progress/views.py b/lms/djangoapps/course_home_api/progress/views.py index fd96ff6c08..8c21335dca 100644 --- a/lms/djangoapps/course_home_api/progress/views.py +++ b/lms/djangoapps/course_home_api/progress/views.py @@ -21,8 +21,9 @@ from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.course_blocks.transformers import start_date from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException +from lms.djangoapps.course_home_api.utils import get_course_or_403 from lms.djangoapps.courseware.courses import ( - get_course_blocks_completion_summary, get_course_with_access, get_studio_url, + get_course_blocks_completion_summary, get_studio_url, ) from lms.djangoapps.courseware.masquerade import setup_masquerade from lms.djangoapps.courseware.views.views import credit_course_requirements, get_cert_data @@ -128,6 +129,7 @@ class ProgressTabView(RetrieveAPIView): * 200 on success with above fields. * 401 if the user is not authenticated or not enrolled. + * 403 if the user does not have access to the course. * 404 if the course is not available or cannot be seen. """ @@ -190,7 +192,7 @@ class ProgressTabView(RetrieveAPIView): student = self._get_student_user(request, course_key, student_id, is_staff) username = get_enterprise_learner_generic_name(request) or student.username - course = get_course_with_access(student, 'load', course_key, check_if_enrolled=False) + course = get_course_or_403(student, 'load', course_key, check_if_enrolled=False) course_overview = CourseOverview.get_from_id(course_key) enrollment = CourseEnrollment.get_enrollment(student, course_key) diff --git a/lms/djangoapps/course_home_api/tests/test_utils.py b/lms/djangoapps/course_home_api/tests/test_utils.py new file mode 100644 index 0000000000..1d665a6b37 --- /dev/null +++ b/lms/djangoapps/course_home_api/tests/test_utils.py @@ -0,0 +1,53 @@ +""" Tests for course home api utils """ + +from contextlib import contextmanager +from rest_framework.exceptions import PermissionDenied +from unittest import mock + +from lms.djangoapps.course_home_api.utils import get_course_or_403 +from lms.djangoapps.courseware.access_response import AccessError +from lms.djangoapps.courseware.exceptions import CourseAccessRedirect +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class GetCourseOr403Test(ModuleStoreTestCase): + """ Tests for get_course_or_403 """ + + @contextmanager + def mock_get_course(self, *args, **kwargs): + """ Mock base_get_course_with_access helper """ + with mock.patch( + 'lms.djangoapps.course_home_api.utils.base_get_course_with_access', + *args, + **kwargs + ) as mock_get: + yield mock_get + + def test_no_exception(self): + """ If no exception is raised we should return the return of get_course_with_access """ + expected_return = mock.Mock() + with self.mock_get_course(return_value=expected_return): + assert get_course_or_403() == expected_return + + def test_redirect(self): + """ Test for behavior when get_course_with_access raises a redirect error """ + expected_url = "www.testError.access/redirect.php?work=yes" + mock_access_error = AccessError('code', 'dev_msg', 'usr_msg') + mock_course_access_redirect = CourseAccessRedirect(expected_url, mock_access_error) + + with self.mock_get_course(side_effect=mock_course_access_redirect): + try: + get_course_or_403() + self.fail('Call to get_course_or_403 should raise exception') + except PermissionDenied as e: + assert str(e.detail) == mock_access_error.user_message + assert e.detail.code == mock_access_error.error_code + + def test_other_exception(self): + """ Any other exception should not be caught """ + class MyException(Exception): + pass + + with self.mock_get_course(side_effect=MyException()): + with self.assertRaises(MyException): + get_course_or_403() diff --git a/lms/djangoapps/course_home_api/utils.py b/lms/djangoapps/course_home_api/utils.py new file mode 100644 index 0000000000..c215061db0 --- /dev/null +++ b/lms/djangoapps/course_home_api/utils.py @@ -0,0 +1,24 @@ +""" Utilities for views in the course home api""" +from rest_framework.exceptions import PermissionDenied + +from lms.djangoapps.courseware.courses import get_course_with_access as base_get_course_with_access +from lms.djangoapps.courseware.exceptions import CourseAccessRedirect + + +def get_course_or_403(*args, **kwargs): + """ + When we make requests to the various Learner Home API endpoints, we do not want to return the actual redirects, + Instead we should return an error code. The redirect info is returned from the course metadata endpoint and the + URL can be followed by whatever client is calling. + + Raises: + - 404 if course is not found + - 403 if the requesting user does not have access to the course + """ + try: + return base_get_course_with_access(*args, **kwargs) + except CourseAccessRedirect as e: + raise PermissionDenied( + detail=e.access_error.user_message, + code=e.access_error.error_code + ) from e 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 0000000000..05d0f7cf6b --- /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 index 54e9a9c96c..348292b172 100644 --- a/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py +++ b/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py @@ -2,6 +2,12 @@ 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 @@ -10,128 +16,29 @@ class TestMFEContextSerializer(TestCase): High-level unit tests for MFEContextSerializer """ - @staticmethod - def get_mock_mfe_context_data(): - """ - Helper function to generate mock data for the MFE Context API view. - """ - - mock_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': [] - } - } - - return mock_context_data - - @staticmethod - def get_expected_data(): - """ - Helper function to generate expected data for the MFE Context API view serializer. - """ - - expected_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': [] - } - } - - return expected_data - def test_mfe_context_serializer(self): """ Test MFEContextSerializer with mock data that serializes data correctly """ - mfe_context_data = self.get_mock_mfe_context_data() - expected_data = self.get_expected_data() output_data = MFEContextSerializer( - mfe_context_data + mock_mfe_context_data ).data self.assertDictEqual( output_data, - expected_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 75111a792f..d8ab2c37c1 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.models import Registration 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 @@ -351,6 +352,32 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase): 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/xmodule/split_test_block.py b/xmodule/split_test_block.py index c45848800f..7feceebfee 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -409,7 +409,7 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method ) raise else: - self.runtime.publish('xblock.split_test.child_render', {'child_id': child_id}) + self.runtime.publish(self, 'xblock.split_test.child_render', {'child_id': child_id}) return Response() def get_icon_class(self):