From d0ac31994ed1fc456215e322e249db64d66a7393 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Fri, 26 Nov 2021 16:47:56 -0400 Subject: [PATCH] feat: add first batch of Open edX Filters * Add PreEnrollmentFilter * Add PreRegisterFilter * Add PreLoginFilter For more info: https://github.com/openedx/edx-platform/pull/29449 Some events that were already on the platform were also added: * Add COURSE_ENROLLMENT_CHANGED: sent after the enrollment update * Add COURSE_ENROLLMENT_CREATED event after the user's enrollment creation * Add COURSE_UNENROLLMENT_COMPLETED: sent after the user's unenrollment For more info: https://github.com/openedx/edx-platform/pull/28266 https://github.com/openedx/edx-platform/pull/28640 --- common/djangoapps/student/models.py | 23 ++ .../djangoapps/student/tests/test_filters.py | 104 +++++++ .../core/djangoapps/user_authn/views/login.py | 12 + .../djangoapps/user_authn/views/register.py | 11 + .../user_authn/views/tests/test_filters.py | 274 ++++++++++++++++++ requirements/edx/base.in | 2 + requirements/edx/base.txt | 2 + requirements/edx/development.txt | 2 + requirements/edx/testing.txt | 2 + 9 files changed, 432 insertions(+) create mode 100644 common/djangoapps/student/tests/test_filters.py create mode 100644 openedx/core/djangoapps/user_authn/views/tests/test_filters.py diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 2fd50fd7247..931339db075 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -55,6 +55,18 @@ from slumber.exceptions import HttpClientError, HttpServerError from user_util import user_util +from openedx_events.learning.data import ( + CourseData, + CourseEnrollmentData, + UserData, + UserPersonalData, +) +from openedx_events.learning.signals import ( + COURSE_ENROLLMENT_CHANGED, + COURSE_ENROLLMENT_CREATED, + COURSE_UNENROLLMENT_COMPLETED, +) +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 @@ -1100,6 +1112,10 @@ class AlreadyEnrolledError(CourseEnrollmentException): pass +class EnrollmentNotAllowed(CourseEnrollmentException): + pass + + class CourseEnrollmentManager(models.Manager): """ Custom manager for CourseEnrollment with Table-level filter methods. @@ -1555,6 +1571,13 @@ 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 = CourseEnrollmentStarted.run_filter( + user=user, course_key=course_key, mode=mode, + ) + except CourseEnrollmentStarted.PreventEnrollment as exc: + raise EnrollmentNotAllowed(str(exc)) from exc + if mode is None: mode = _default_course_mode(str(course_key)) # All the server-side checks for whether a user is allowed to enroll. diff --git a/common/djangoapps/student/tests/test_filters.py b/common/djangoapps/student/tests/test_filters.py new file mode 100644 index 00000000000..2f8420d01ce --- /dev/null +++ b/common/djangoapps/student/tests/test_filters.py @@ -0,0 +1,104 @@ +""" +Test that various filters are fired for models in the student app. +""" +from django.test import override_settings +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from openedx_filters.learning.filters import CourseEnrollmentStarted +from openedx_filters import PipelineStep + +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 + + +class TestEnrollmentPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode): # pylint: disable=arguments-differ + """Pipeline steps that changes mode to honor.""" + if mode == "no-id-professional": + raise CourseEnrollmentStarted.PreventEnrollment() + return {"mode": "honor"} + + +@skip_unless_lms +class EnrollmentFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the enrollment process through the enroll method. + + This class guarantees that the following filters are triggered during the user's enrollment: + + - CourseEnrollmentStarted + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course = CourseFactory.create() + self.user = UserFactory.create( + username="test", + email="test@example.com", + password="password", + ) + self.user_profile = UserProfileFactory.create(user=self.user, name="Test Example") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.enrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestEnrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_enrollment_filter_executed(self): + """ + Test whether the student enrollment filter is triggered before the user's + enrollment process. + + Expected result: + - CourseEnrollmentStarted is triggered and executes TestEnrollmentPipelineStep. + - The arguments that the receiver gets are the arguments used by the filter + with the enrollment mode changed. + """ + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='audit') + + self.assertEqual('honor', enrollment.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.enrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestEnrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_enrollment_filter_prevent_enroll(self): + """ + Test prevent the user's enrollment through a pipeline step. + + Expected result: + - 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 9d47e2bc7ee..32f60ee2f2a 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -28,6 +28,11 @@ from ratelimit.decorators import ratelimit from rest_framework.views import APIView +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 import third_party_auth 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 @@ -500,6 +505,13 @@ 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, error_code=exc.error_code, context=exc.context, + ) 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 c97f973af8d..7d7bbd44b0e 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -23,6 +23,9 @@ from django.views.decorators.debug import sensitive_post_parameters from edx_django_utils.monitoring import set_custom_attribute 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 @@ -522,6 +525,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/edx/base.in b/requirements/edx/base.in index 510fa261803..4d1c048b2e1 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -116,6 +116,8 @@ newrelic # New Relic agent for performance monitoring nodeenv # Utility for managing Node.js environments; we use this for deployments and testing oauthlib # OAuth specification support for authenticating via LTI or other Open edX services openedx-calc # Library supporting mathematical calculations for Open edX +openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) +openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) ora2 piexif # Exif image metadata manipulation, used in the profile_images app Pillow # Image manipulation library; used for course assets, profile images, invoice PDFs, etc. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index b201ea1d7c9..5510616b21e 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -166,6 +166,8 @@ nodeenv==1.5.0 # via -r requirements/edx/base.in numpy==1.20.2 # via chem, openedx-calc, scipy oauthlib==3.0.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.in, django-oauth-toolkit, lti-consumer-xblock, requests-oauthlib, social-auth-core openedx-calc==2.0.1 # via -r requirements/edx/base.in +openedx-events==0.7.1 # via -r requirements/edx/base.in +openedx-filters==0.4.3 # via -r requirements/edx/base.in ora2==3.4.1 # via -r requirements/edx/base.in packaging==20.9 # via bleach, drf-yasg path.py==12.5.0 # via edx-enterprise, edx-i18n-tools, ora2, staff-graded-xblock, xmodule diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 505b69f4fec..6016c44bf07 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -199,6 +199,8 @@ nodeenv==1.5.0 # via -r requirements/edx/testing.txt numpy==1.20.2 # via -r requirements/edx/testing.txt, chem, openedx-calc, scipy oauthlib==3.0.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.txt, django-oauth-toolkit, lti-consumer-xblock, requests-oauthlib, social-auth-core openedx-calc==2.0.1 # via -r requirements/edx/testing.txt +openedx-events==0.7.1 # via -r requirements/edx/testing.txt +openedx-filters==0.4.3 # via -r requirements/edx/testing.txt ora2==3.4.1 # via -r requirements/edx/testing.txt packaging==20.9 # via -r requirements/edx/testing.txt, bleach, drf-yasg, pytest, sphinx, tox path.py==12.5.0 # via -r requirements/edx/testing.txt, edx-enterprise, edx-i18n-tools, ora2, staff-graded-xblock, xmodule diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index ef5ae5af09d..7f7e739c5b7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -191,6 +191,8 @@ nodeenv==1.5.0 # via -r requirements/edx/base.txt numpy==1.20.2 # via -r requirements/edx/base.txt, chem, openedx-calc, scipy oauthlib==3.0.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.txt, django-oauth-toolkit, lti-consumer-xblock, requests-oauthlib, social-auth-core openedx-calc==2.0.1 # via -r requirements/edx/base.txt +openedx-events==0.7.1 # via -r requirements/edx/base.txt +openedx-filters==0.4.3 # via -r requirements/edx/base.txt ora2==3.4.1 # via -r requirements/edx/base.txt packaging==20.9 # via -r requirements/edx/base.txt, bleach, drf-yasg, pytest, tox path.py==12.5.0 # via -r requirements/edx/base.txt, edx-enterprise, edx-i18n-tools, ora2, staff-graded-xblock, xmodule