diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 464258f9c5b..833972dbb4e 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -68,7 +68,7 @@ COURSE_ENROLLMENT_CREATED, COURSE_UNENROLLMENT_COMPLETED, ) -from openedx_filters.learning.enrollment import PreEnrollmentFilter +from openedx_filters.learning.filters import CourseEnrollmentStarted import openedx.core.djangoapps.django_comment_common.comment_client as cc from common.djangoapps.course_modes.models import CourseMode, get_cosmetic_verified_display_price from common.djangoapps.student.emails import send_proctoring_requirements_email @@ -1620,10 +1620,10 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal Also emits relevant events for analytics purposes. """ try: - user, course_key, mode = PreEnrollmentFilter.run( + user, course_key, mode = CourseEnrollmentStarted.run_filter( user=user, course_key=course_key, mode=mode, ) - except PreEnrollmentFilter.PreventEnrollment as exc: + except CourseEnrollmentStarted.PreventEnrollment as exc: raise EnrollmentNotAllowed(str(exc)) from exc if mode is None: diff --git a/common/djangoapps/student/tests/test_filters.py b/common/djangoapps/student/tests/test_filters.py index edb2865a57a..b1e5ace7e23 100644 --- a/common/djangoapps/student/tests/test_filters.py +++ b/common/djangoapps/student/tests/test_filters.py @@ -2,27 +2,25 @@ Test that various filters are fired for models in the student app. """ from django.test import override_settings -from openedx_filters.learning.enrollment import PreEnrollmentFilter from openedx_filters import PipelineStep +from openedx_filters.learning.filters import CourseEnrollmentStarted +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.models import CourseEnrollment, EnrollmentNotAllowed from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory - from openedx.core.djangolib.testing.utils import skip_unless_lms -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - class TestEnrollmentPipelineStep(PipelineStep): """ Utility function used when getting steps for pipeline. """ - def run(self, user, course_key, mode): # pylint: disable=unused-argument, arguments-differ + def run_filter(self, user, course_key, mode): # pylint: disable=arguments-differ, unused-argument """Pipeline steps that changes mode to honor.""" if mode == "no-id-professional": - raise PreEnrollmentFilter.PreventEnrollment() + raise CourseEnrollmentStarted.PreventEnrollment() return {"mode": "honor"} @@ -33,7 +31,7 @@ class EnrollmentFiltersTest(ModuleStoreTestCase): This class guarantees that the following filters are triggered during the user's enrollment: - - PreEnrollmentFilter + - CourseEnrollmentStarted """ def setUp(self): # pylint: disable=arguments-differ @@ -62,7 +60,7 @@ def test_enrollment_filter_executed(self): enrollment process. Expected result: - - PreEnrollmentFilter is triggered and executes TestEnrollmentPipelineStep. + - CourseEnrollmentStarted is triggered and executes TestEnrollmentPipelineStep. - The arguments that the receiver gets are the arguments used by the filter with the enrollment mode changed. """ @@ -85,8 +83,22 @@ def test_enrollment_filter_prevent_enroll(self): Test prevent the user's enrollment through a pipeline step. Expected result: - - PreEnrollmentFilter is triggered and executes TestEnrollmentPipelineStep. + - CourseEnrollmentStarted is triggered and executes TestEnrollmentPipelineStep. - The user can't enroll. """ with self.assertRaises(EnrollmentNotAllowed): CourseEnrollment.enroll(self.user, self.course.id, mode='no-id-professional') + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_enrollment_without_filter_configuration(self): + """ + Test usual enrollment process, without filter's intervention. + + Expected result: + - CourseEnrollmentStarted does not have any effect on the enrollment process. + - The enrollment process ends successfully. + """ + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='audit') + + self.assertEqual('audit', enrollment.mode) + self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 1e6af668ff1..75db221904a 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -30,6 +30,8 @@ from openedx_events.learning.data import UserData, UserPersonalData from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED +from openedx_filters.learning.filters import StudentLoginRequested + from common.djangoapps.edxmako.shortcuts import render_to_response from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -515,6 +517,16 @@ def login_user(request): possibly_authenticated_user = user + try: + possibly_authenticated_user = StudentLoginRequested.run_filter(user=possibly_authenticated_user) + except StudentLoginRequested.PreventLogin as exc: + raise AuthFailedError( + str(exc), + redirect_url=exc.redirect_to, # pylint: disable=no-member + error_code=exc.error_code, # pylint: disable=no-member + context=exc.context, # pylint: disable=no-member + ) from exc + if not is_user_third_party_authenticated: possibly_authenticated_user = _authenticate_first_party(request, user, third_party_auth_requested) if possibly_authenticated_user and password_policy_compliance.should_enforce_compliance_on_login(): diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 2a132341b27..7f46eee8094 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -25,6 +25,7 @@ from edx_toggles.toggles import LegacyWaffleFlag, LegacyWaffleFlagNamespace from openedx_events.learning.data import UserData, UserPersonalData from openedx_events.learning.signals import STUDENT_REGISTRATION_COMPLETED +from openedx_filters.learning.filters import StudentRegistrationRequested from pytz import UTC from ratelimit.decorators import ratelimit from requests import HTTPError @@ -536,6 +537,14 @@ def post(self, request): data = request.POST.copy() self._handle_terms_of_service(data) + try: + data = StudentRegistrationRequested.run_filter(form_data=data) + except StudentRegistrationRequested.PreventRegistration as exc: + errors = { + "error_message": [{"user_message": str(exc)}], + } + return self._create_response(request, errors, status_code=exc.status_code) + response = self._handle_duplicate_email_username(request, data) if response: return response diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_filters.py b/openedx/core/djangoapps/user_authn/views/tests/test_filters.py new file mode 100644 index 00000000000..dbca9183002 --- /dev/null +++ b/openedx/core/djangoapps/user_authn/views/tests/test_filters.py @@ -0,0 +1,274 @@ +""" +Test that various filters are fired for the vies in the user_authn app. +""" +from django.contrib.auth import get_user_model +from django.test import override_settings +from django.urls import reverse +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import StudentLoginRequested, StudentRegistrationRequested +from rest_framework import status + +from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory +from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase +from openedx.core.djangolib.testing.utils import skip_unless_lms + +User = get_user_model() + + +class TestRegisterPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, form_data): # pylint: disable=arguments-differ + """Pipeline steps that changes the user's username.""" + username = f"{form_data.get('username')}-OpenEdx" + form_data["username"] = username + return { + "form_data": form_data, + } + + +class TestAnotherRegisterPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, form_data): # pylint: disable=arguments-differ + """Pipeline steps that changes the user's username.""" + username = f"{form_data.get('username')}-Test" + form_data["username"] = username + return { + "form_data": form_data, + } + + +class TestStopRegisterPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, form_data): # pylint: disable=arguments-differ + """Pipeline steps that stops the user's registration process.""" + raise StudentRegistrationRequested.PreventRegistration("You can't register on this site.", status_code=403) + + +class TestLoginPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user): # pylint: disable=arguments-differ + """Pipeline steps that adds a field to the user's profile.""" + user.profile.set_meta({"logged_in": True}) + user.profile.save() + return { + "user": user + } + + +class TestAnotherLoginPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user): # pylint: disable=arguments-differ + """Pipeline steps that adds a field to the user's profile.""" + new_meta = user.profile.get_meta() + new_meta.update({"another_logged_in": True}) + user.profile.set_meta(new_meta) + user.profile.save() + return { + "user": user + } + + +class TestStopLoginPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user): # pylint: disable=arguments-differ + """Pipeline steps that stops the user's login.""" + raise StudentLoginRequested.PreventLogin("You can't login on this site.") + + +@skip_unless_lms +class RegistrationFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the user registration process. + + This class guarantees that the following filters are triggered during the user's registration: + + - StudentRegistrationRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.url = reverse("user_api_registration") + self.user_info = { + "email": "user@example.com", + "name": "Test User", + "username": "test", + "password": "password", + "honor_code": "true", + } + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.student.registration.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestRegisterPipelineStep", + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestAnotherRegisterPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_register_filter_executed(self): + """ + Test whether the student register filter is triggered before the user's + registration process. + + Expected result: + - StudentRegistrationRequested is triggered and executes TestRegisterPipelineStep. + - The user's username is updated. + """ + self.client.post(self.url, self.user_info) + + user = User.objects.filter(username=f"{self.user_info.get('username')}-OpenEdx-Test") + self.assertTrue(user) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.student.registration.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestRegisterPipelineStep", + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestStopRegisterPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_register_filter_prevent_registration(self): + """ + Test prevent the user's registration through a pipeline step. + + Expected result: + - StudentRegistrationRequested is triggered and executes TestStopRegisterPipelineStep. + - The user's registration stops. + """ + response = self.client.post(self.url, self.user_info) + + self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_register_without_filter_configuration(self): + """ + Test usual registration process, without filter's intervention. + + Expected result: + - StudentRegistrationRequested does not have any effect on the registration process. + - The registration process ends successfully. + """ + self.client.post(self.url, self.user_info) + + user = User.objects.filter(username=f"{self.user_info.get('username')}") + self.assertTrue(user) + + +@skip_unless_lms +class LoginFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the user login process. + + This class guarantees that the following filters are triggered during the user's login: + + - StudentLoginRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.user = UserFactory.create( + username="test", + email="test@example.com", + password="password", + ) + self.user_profile = UserProfileFactory.create(user=self.user, name="Test Example") + self.url = reverse('login_api') + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.student.login.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestLoginPipelineStep", + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestAnotherLoginPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_login_filter_executed(self): + """ + Test whether the student login filter is triggered before the user's + login process. + + Expected result: + - StudentLoginRequested is triggered and executes TestLoginPipelineStep. + - The user's profile is updated. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + self.client.post(self.url, data) + + user = User.objects.get(username=self.user.username) + self.assertDictEqual({"logged_in": True, "another_logged_in": True}, user.profile.get_meta()) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.student.login.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestLoginPipelineStep", + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestStopLoginPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_login_filter_prevent_login(self): + """ + Test prevent the user's login through a pipeline step. + + Expected result: + - StudentLoginRequested is triggered and executes TestStopLoginPipelineStep. + - Test prevent the user's login through a pipeline step. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + response = self.client.post(self.url, data) + + self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_login_without_filter_configuration(self): + """ + Test usual login process, without filter's intervention. + + Expected result: + - StudentLoginRequested does not have any effect on the login process. + - The login process ends successfully. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + response = self.client.post(self.url, data) + + self.assertEqual(status.HTTP_200_OK, response.status_code) diff --git a/requirements/edunext/base.txt b/requirements/edunext/base.txt index 50cf3fe8f87..58101e0a95a 100644 --- a/requirements/edunext/base.txt +++ b/requirements/edunext/base.txt @@ -60,7 +60,7 @@ markupsafe==1.1.1 # via -c requirements/edunext/../edx/base.txt, jinja2, newrelic==6.2.0.156 # via -c requirements/edunext/../edx/base.txt, edx-django-utils oauthlib==3.0.1 # via -c requirements/edunext/../edx/base.txt, django-oauth-toolkit, requests-oauthlib, social-auth-core openedx-events==0.6.0 # via -r requirements/edunext/base.in, eox-hooks -openedx-filters==0.3.1 # via -r requirements/edunext/base.in +openedx-filters==0.4.3 # via -r requirements/edunext/base.in openedx-scorm-xblock==12.0.0 # via -r requirements/edunext/base.in git+https://github.com/oppia/xblock.git@3b5c17c5832b4f8ef132c6bbf48da8a86df43b3d#egg=oppia-xblock # via -r requirements/edunext/github.in packaging==20.9 # via -c requirements/edunext/../edx/base.txt, drf-yasg diff --git a/requirements/edunext/constraints.txt b/requirements/edunext/constraints.txt index 6ce36657341..eb326bcc044 100644 --- a/requirements/edunext/constraints.txt +++ b/requirements/edunext/constraints.txt @@ -8,5 +8,5 @@ # pin when possible. Writing an issue against the offending project and # linking to it here is good. -openedx-filters==0.3.1 # Pin working openedx-filters release until libraries are stable +openedx-filters==0.4.3 # Pin working openedx-filters release until libraries are stable openedx-events==0.6.0 # Pin working openedx-events release until libraries are stable