From f22a129b23800427d3c96d1763d5d9b65ac612c2 Mon Sep 17 00:00:00 2001 From: Abdurrahman Asad <51022010+A-ASAD@users.noreply.github.com> Date: Wed, 11 May 2022 09:41:48 +0500 Subject: [PATCH] fix: discussion moderators can't create posts for specific cohorts (#30352) fix: discussion moderators can't create posts for specific cohorts --- lms/djangoapps/discussion/rest_api/forms.py | 2 +- .../discussion/rest_api/permissions.py | 25 ++++++++++ .../discussion/rest_api/tests/test_views.py | 46 +++++++++++++++++++ lms/djangoapps/discussion/rest_api/views.py | 4 +- .../djangoapps/course_groups/permissions.py | 33 +++++++++++++ .../core/djangoapps/course_groups/views.py | 5 +- 6 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 openedx/core/djangoapps/course_groups/permissions.py diff --git a/lms/djangoapps/discussion/rest_api/forms.py b/lms/djangoapps/discussion/rest_api/forms.py index b4f7008453..4683f241b4 100644 --- a/lms/djangoapps/discussion/rest_api/forms.py +++ b/lms/djangoapps/discussion/rest_api/forms.py @@ -179,7 +179,7 @@ class CourseDiscussionSettingsForm(Form): course_id = self.cleaned_data['course_id'] try: course_key = CourseKey.from_string(course_id) - self.cleaned_data['course'] = get_course_with_access(self.request_user, 'staff', course_key) + self.cleaned_data['course'] = get_course_with_access(self.request_user, 'load', course_key) self.cleaned_data['course_key'] = course_key return course_id except InvalidKeyError: diff --git a/lms/djangoapps/discussion/rest_api/permissions.py b/lms/djangoapps/discussion/rest_api/permissions.py index eedf4572d9..c97fbc9b7d 100644 --- a/lms/djangoapps/discussion/rest_api/permissions.py +++ b/lms/djangoapps/discussion/rest_api/permissions.py @@ -13,10 +13,14 @@ from common.djangoapps.student.roles import ( GlobalStaff, ) from lms.djangoapps.discussion.django_comment_client.utils import ( + get_user_role_names, has_discussion_privileges, ) from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread +from openedx.core.djangoapps.django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR +) def _is_author(cc_content, context): @@ -156,3 +160,24 @@ class IsStaffOrCourseTeamOrEnrolled(permissions.BasePermission): CourseEnrollment.is_enrolled(request.user, course_key) or has_discussion_privileges(request.user, course_key) ) + + +class IsStaffOrAdmin(permissions.BasePermission): + """ + Permission that checks if the user is staff or an admin. + """ + + def has_permission(self, request, view): + """Returns true if the user is admin or staff and request method is GET.""" + course_key = CourseKey.from_string(view.kwargs.get('course_id')) + user_roles = get_user_role_names(request.user, course_key) + is_user_staff = bool(user_roles & { + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + }) + return ( + GlobalStaff().has_user(request.user) or + request.user.is_staff or + is_user_staff and request.method == "GET" + ) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 529ae16c89..904ba52257 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -2282,6 +2282,12 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor """Log the client in as the staff.""" self.client.login(username=self.user.username, password=self.password) + def _login_as_discussion_staff(self): + user = UserFactory(username='abc', password='abc') + role = Role.objects.create(name='Administrator', course_id=self.course.id) + role.users.set([user]) + self.client.login(username=user.username, password='abc') + def _create_divided_discussions(self): """Create some divided discussions for testing.""" divided_inline_discussions = ['Topic A', ] @@ -2373,6 +2379,46 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor ) assert response.status_code == 404 + def test_patch_request_by_discussion_staff(self): + """Test the response when patch request is sent by a user with discussions staff role.""" + self._login_as_discussion_staff() + response = self.patch_request( + {'always_divide_inline_discussions': True} + ) + assert response.status_code == 403 + + def test_get_request_by_discussion_staff(self): + """Test the response when get request is sent by a user with discussions staff role.""" + self._login_as_discussion_staff() + divided_inline_discussions, divided_course_wide_discussions = self._create_divided_discussions() + response = self.client.get(self.path) + assert response.status_code == 200 + expected_response = self._get_expected_response() + expected_response['divided_course_wide_discussions'] = [ + topic_name_to_id(self.course, name) for name in divided_course_wide_discussions + ] + expected_response['divided_inline_discussions'] = [ + topic_name_to_id(self.course, name) for name in divided_inline_discussions + ] + content = json.loads(response.content.decode('utf-8')) + assert content == expected_response + + def test_get_request_by_non_staff_user(self): + """Test the response when get request is sent by a regular user with no staff role.""" + user = UserFactory(username='abc', password='abc') + self.client.login(username=user.username, password='abc') + response = self.client.get(self.path) + assert response.status_code == 403 + + def test_patch_request_by_non_staff_user(self): + """Test the response when patch request is sent by a regular user with no staff role.""" + user = UserFactory(username='abc', password='abc') + self.client.login(username=user.username, password='abc') + response = self.patch_request( + {'always_divide_inline_discussions': True} + ) + assert response.status_code == 403 + def test_get_settings(self): """Test the current discussion settings against the expected response.""" divided_inline_discussions, divided_course_wide_discussions = self._create_divided_discussions() diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 28fad72d52..ff2599f852 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -63,7 +63,7 @@ from ..rest_api.forms import ( UserCommentListGetForm, UserOrdering, ) -from ..rest_api.permissions import IsStaffOrCourseTeamOrEnrolled +from ..rest_api.permissions import IsStaffOrAdmin, IsStaffOrCourseTeamOrEnrolled from ..rest_api.serializers import ( CourseMetadataSerailizer, DiscussionRolesListSerializer, @@ -1096,7 +1096,7 @@ class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView): SessionAuthenticationAllowInactiveUser, ) parser_classes = (JSONParser, MergePatchParser,) - permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + permission_classes = (permissions.IsAuthenticated, IsStaffOrAdmin) def _get_request_kwargs(self, course_id): return dict(course_id=course_id) diff --git a/openedx/core/djangoapps/course_groups/permissions.py b/openedx/core/djangoapps/course_groups/permissions.py new file mode 100644 index 0000000000..a7c1b6bc59 --- /dev/null +++ b/openedx/core/djangoapps/course_groups/permissions.py @@ -0,0 +1,33 @@ +""" +Permissions for cohorts API +""" + +from opaque_keys.edx.keys import CourseKey +from rest_framework import permissions + +from openedx.core.djangoapps.django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR +) +from common.djangoapps.student.roles import GlobalStaff +from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names + + +class IsStaffOrAdmin(permissions.BasePermission): + """ + Permission that checks if the user is staff or an admin. + """ + + def has_permission(self, request, view): + """Returns true if the user is admin or staff and request method is GET.""" + course_key = CourseKey.from_string(view.kwargs.get('course_key_string')) + user_roles = get_user_role_names(request.user, course_key) + is_user_staff = bool(user_roles & { + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + }) + return ( + GlobalStaff().has_user(request.user) or + request.user.is_staff or + is_user_staff and request.method == "GET" + ) diff --git a/openedx/core/djangoapps/course_groups/views.py b/openedx/core/djangoapps/course_groups/views.py index 4e9b7a66cf..b03744bb98 100644 --- a/openedx/core/djangoapps/course_groups/views.py +++ b/openedx/core/djangoapps/course_groups/views.py @@ -27,6 +27,7 @@ from rest_framework.serializers import Serializer from lms.djangoapps.courseware.courses import get_course, get_course_with_access from common.djangoapps.edxmako.shortcuts import render_to_response from openedx.core.djangoapps.course_groups.models import CohortMembership +from openedx.core.djangoapps.course_groups.permissions import IsStaffOrAdmin from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from common.djangoapps.student.auth import has_course_author_access @@ -431,7 +432,7 @@ class APIPermissions(GenericAPIView): BearerAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser, ) - permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + permission_classes = (permissions.IsAuthenticated, IsStaffOrAdmin) serializer_class = Serializer @@ -505,7 +506,7 @@ class CohortHandler(DeveloperErrorViewMixin, APIPermissions): """ Endpoint to get either one or all cohorts. """ - course_key, course = _get_course_with_access(request, course_key_string) + course_key, course = _get_course_with_access(request, course_key_string, 'load') if not cohort_id: all_cohorts = cohorts.get_course_cohorts(course) paginator = NamespacedPageNumberPagination()