fix: update discussion API permissions (#29127)

lms/djangoapps/discussion/django_comment_client/utils.py

@@ -90,10 +90,10 @@ def has_discussion_privileges(user, course_id):
     Returns:
       bool
     """
-    # get_role_ids returns a dictionary of only admin, moderator and community TAs.
     roles = get_role_ids(course_id)
-    for role in roles:
-        if user.id in roles[role]:
+
+    for user_ids in roles.values():
+        if user.id in user_ids:
             return True
     return False
 

openedx/core/djangoapps/discussions/permissions.py

@@ -0,0 +1,58 @@
+"""
+API library for Django REST Framework permissions-oriented workflows
+"""
+from rest_framework.exceptions import PermissionDenied
+from rest_framework.permissions import BasePermission
+
+from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff, CourseInstructorRole
+from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
+from openedx.core.lib.api.view_utils import validate_course_key
+
+DEFAULT_MESSAGE = "You're not authorized to perform this operation."
+PERMISSION_MESSAGES = {
+    "change_provider": "Must be global staff to change discussion provider after the course has started.",
+}
+
+
+class IsStaffOrCourseTeam(BasePermission):
+    """
+    Check if user is global or course staff
+
+    Permission that checks to see if the user is global staff, course
+    staff, course admin, or has discussion privileges. If none of those conditions are
+    met, HTTP403 is returned.
+    """
+
+    def has_permission(self, request, view):
+        course_key_string = view.kwargs.get('course_key_string')
+        course_key = validate_course_key(course_key_string)
+
+        if GlobalStaff().has_user(request.user):
+            return True
+
+        return (
+            CourseInstructorRole(course_key).has_user(request.user) or
+            CourseStaffRole(course_key).has_user(request.user) or
+            has_discussion_privileges(request.user, course_key)
+        )
+
+
+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),
+    }
+
+
+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))

openedx/core/djangoapps/discussions/views.py

@@ -1,70 +1,19 @@
 """
-Handle view-logic for the djangoapp
+Handle view-logic for the discussions app.
 """
 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, 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
+from .permissions import check_course_permissions, IsStaffOrCourseTeam
 from .serializers import DiscussionsConfigurationSerializer
 
 
-class IsStaff(BasePermission):
-    """
-    Check if user is global or course staff
-
-    We create our own copy of this because other versions of this check
-    allow access to additional user roles.
-    """
-
-    def has_permission(self, request, view):
-        """
-        Check if user has global or course staff permission
-        """
-        user = request.user
-        if user.is_staff:
-            return True
-        course_key_string = view.kwargs.get('course_key_string')
-        course_key = validate_course_key(course_key_string)
-        return CourseStaffRole(
-            course_key,
-        ).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
@@ -74,7 +23,7 @@ class DiscussionsConfigurationView(APIView):
         BearerAuthenticationAllowInactiveUser,
         SessionAuthenticationAllowInactiveUser
     )
-    permission_classes = (IsStaff,)
+    permission_classes = (IsStaffOrCourseTeam,)
 
     # pylint: disable=redefined-builtin
     def get(self, request, course_key_string: str, **_kwargs) -> Response: