From cfd78455df0f2e215253a513f99b408752a02766 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Wed, 27 Oct 2021 21:05:34 +0500 Subject: [PATCH] fix: update discussion API permissions (#29127) --- .../discussion/django_comment_client/utils.py | 6 +- .../djangoapps/discussions/permissions.py | 58 +++++++++++++++++++ openedx/core/djangoapps/discussions/views.py | 57 +----------------- 3 files changed, 64 insertions(+), 57 deletions(-) create mode 100644 openedx/core/djangoapps/discussions/permissions.py diff --git a/lms/djangoapps/discussion/django_comment_client/utils.py b/lms/djangoapps/discussion/django_comment_client/utils.py index ee5c6cc129..49e81a8562 100644 --- a/lms/djangoapps/discussion/django_comment_client/utils.py +++ b/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 diff --git a/openedx/core/djangoapps/discussions/permissions.py b/openedx/core/djangoapps/discussions/permissions.py new file mode 100644 index 0000000000..7028defc3e --- /dev/null +++ b/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)) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index c3d69cdd9e..0f5da0fa74 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/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: