diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index c7d3685353..9aa56b83ec 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -338,6 +338,7 @@ class ModuleStoreTestUsersMixin(): Create a test user for a course. """ if user_type is CourseUserType.ANONYMOUS: + self.client.logout() return AnonymousUser() is_enrolled = user_type is CourseUserType.ENROLLED diff --git a/openedx/core/djangoapps/discussions/tests/test_views.py b/openedx/core/djangoapps/discussions/tests/test_views.py index 38e6732643..580a418f47 100644 --- a/openedx/core/djangoapps/discussions/tests/test_views.py +++ b/openedx/core/djangoapps/discussions/tests/test_views.py @@ -2,6 +2,7 @@ Test app view logic """ # pylint: disable=test-inherits-tests +from datetime import datetime, timedelta, timezone import unittest import ddt @@ -422,3 +423,54 @@ class DataTest(AuthorizedApiTest): assert data['provider_type'] == 'legacy' assert not data['plugin_configuration']['allow_anonymous'] assert not data['lti_configuration'] + + @ddt.data(*[ + user_type.name for user_type in CourseUserType + if user_type not in { # pylint: disable=undefined-variable + CourseUserType.ANONYMOUS, + CourseUserType.GLOBAL_STAFF + } + ]) + def test_unable_to_change_provider_for_running_course(self, user_type): + """ + Ensure that certain users cannot change provider for a running course. + """ + self.course.start = datetime.now(timezone.utc) - timedelta(days=5) + self.course = self.update_course(self.course, self.user.id) + + # use the global staff user to do the initial config + # so we're sure to not get permissions errors + response = self._post({ + 'enabled': True, + 'provider_type': 'legacy', + }) + assert response.status_code == status.HTTP_200_OK + + self.create_user_for_course(self.course, CourseUserType[user_type]) + + response = self._post({ + 'enabled': True, + 'provider_type': 'piazza', + }) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_global_staff_can_change_provider_for_running_course(self): + """ + Ensure that global staff can change provider for a running course. + """ + self.course.start = datetime.now(timezone.utc) - timedelta(days=5) + self.course = self.update_course(self.course, self.user.id) + + # use the global staff user to do the initial config + # so we're sure to not get permissions errors + response = self._post({ + 'enabled': True, + 'provider_type': 'legacy', + }) + assert response.status_code == status.HTTP_200_OK + + response = self._post({ + 'enabled': True, + 'provider_type': 'piazza', + }) + assert response.status_code == status.HTTP_200_OK diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index bf1c0a28fc..c3d69cdd9e 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -4,10 +4,12 @@ Handle view-logic for the djangoapp from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from rest_framework.permissions import BasePermission +from rest_framework.exceptions import PermissionDenied from rest_framework.response import Response from rest_framework.views import APIView -from common.djangoapps.student.roles import CourseStaffRole +from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import validate_course_key from .models import DiscussionsConfiguration @@ -36,6 +38,33 @@ class IsStaff(BasePermission): ).has_user(request.user) +def user_permissions_for_course(course, user): + """ + Return the user's permissions over the discussion configuration of the course. + """ + return { + "change_provider": not course.has_started() or GlobalStaff().has_user(user), + } + +PERMISSION_MESSAGES = { + "change_provider": "Must be global staff to change discussion provider after the course has started.", +} + +DEFAULT_MESSAGE = "You're not authorized to perform this operation." + + +def check_course_permissions(course, user, permission): + """ + Check the user has permissions for the operation over the course configuration. + + Raises PermissionDenied if the user does not have permission + """ + permissions = user_permissions_for_course(course, user) + granted = permissions.get(permission) + if not granted: + raise PermissionDenied(PERMISSION_MESSAGES.get(permission, DEFAULT_MESSAGE)) + + class DiscussionsConfigurationView(APIView): """ Handle configuration-related view-logic @@ -54,7 +83,12 @@ class DiscussionsConfigurationView(APIView): """ course_key = validate_course_key(course_key_string) configuration = DiscussionsConfiguration.get(course_key) - serializer = DiscussionsConfigurationSerializer(configuration) + serializer = DiscussionsConfigurationSerializer( + configuration, + context={ + 'user_id': request.user.id, + } + ) return Response(serializer.data) def post(self, request, course_key_string: str, **_kwargs) -> Response: @@ -63,6 +97,7 @@ class DiscussionsConfigurationView(APIView): """ course_key = validate_course_key(course_key_string) configuration = DiscussionsConfiguration.get(course_key) + course = CourseOverview.get_from_id(course_key) serializer = DiscussionsConfigurationSerializer( configuration, context={ @@ -72,5 +107,8 @@ class DiscussionsConfigurationView(APIView): partial=True, ) if serializer.is_valid(raise_exception=True): + if serializer.validated_data['provider_type'] != configuration.provider_type: + check_course_permissions(course, request.user, 'change_provider') + serializer.save() return Response(serializer.data)