Skip to content

Commit

Permalink
feat: Add permission checks to role checks - discussion and course_li…
Browse files Browse the repository at this point in the history
…ve (#33964)

* feat: add permission checks to role checks - discussion and course_live
  • Loading branch information
hsinkoff committed Jan 22, 2024
1 parent 2e92919 commit 7807f3e
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 71 deletions.
8 changes: 8 additions & 0 deletions lms/djangoapps/discussion/django_comment_client/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
)
from lms.djangoapps.discussion.django_comment_client.settings import MAX_COMMENT_DEPTH
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.djangoapps.discussions.utils import (
get_accessible_discussion_xblocks,
get_accessible_discussion_xblocks_by_course_id,
Expand Down Expand Up @@ -126,11 +127,18 @@ def has_discussion_privileges(user, course_id):
Returns:
bool
"""
# TODO: remove user_ids check once course_roles is fully impelented and data is migrated
roles = get_role_ids(course_id)

for user_ids in roles.values():
if user.id in user_ids:
return True
if user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_id
) and user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.perm_name, course_id
):
return True
return False


Expand Down
23 changes: 20 additions & 3 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE
from lms.djangoapps.discussion.views import is_privileged_user
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.djangoapps.discussions.models import (
DiscussionsConfiguration,
DiscussionTopicLink,
Expand Down Expand Up @@ -359,8 +360,15 @@ def _format_datetime(dt):
}),
"is_group_ta": bool(user_roles & {FORUM_ROLE_GROUP_MODERATOR}),
"is_user_admin": request.user.is_staff,
"is_course_staff": CourseStaffRole(course_key).has_user(request.user),
"is_course_admin": CourseInstructorRole(course_key).has_user(request.user),
# TODO: remove role checks once course_roles is fully impelented and data is migrated
"is_course_staff": CourseStaffRole(course_key).has_user(request.user) or
request.user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key
),
"is_course_admin": CourseInstructorRole(course_key).has_user(request.user) or
request.user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key
),
"provider": course_config.provider_type,
"enable_in_context": course_config.enable_in_context,
"group_at_subsection": course_config.plugin_configuration.get("group_at_subsection", False),
Expand Down Expand Up @@ -976,7 +984,16 @@ def get_thread_list(
]

if request.GET.get("group_id", None):
if Role.user_has_role_for_course(request.user, course_key, allowed_roles):
# TODO: remove role check once course_roles is fully impelented and data is migrated
if (
Role.user_has_role_for_course(request.user, course_key, allowed_roles) or
request.user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key
) or
request.user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.perm_name, course_key
)
):
try:
group_id = int(request.GET.get("group_id", None))
except ValueError:
Expand Down
7 changes: 7 additions & 0 deletions lms/djangoapps/discussion/rest_api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
CourseStaffRole,
GlobalStaff,
)
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from lms.djangoapps.discussion.django_comment_client.utils import (
get_user_role_names,
has_discussion_privileges,
Expand Down Expand Up @@ -155,10 +156,16 @@ class IsStaffOrCourseTeamOrEnrolled(permissions.BasePermission):
def has_permission(self, request, view):
"""Returns true if the user is enrolled or is staff."""
course_key = CourseKey.from_string(view.kwargs.get('course_id'))
# TODO: remove role check once course_roles is fully impelented and data is migrated
return (
GlobalStaff().has_user(request.user) or
CourseStaffRole(course_key).has_user(request.user) or
CourseInstructorRole(course_key).has_user(request.user) or
(request.user.has_perms([
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name,
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.perm_name
], course_key
)) or
CourseEnrollment.is_enrolled(request.user, course_key) or
has_discussion_privileges(request.user, course_key)
)
Expand Down
22 changes: 19 additions & 3 deletions lms/djangoapps/discussion/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE
from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context
from lms.djangoapps.teams import api as team_api
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.djangoapps.discussions.utils import (
available_division_schemes,
get_discussion_categories_ids,
Expand Down Expand Up @@ -756,7 +757,12 @@ def is_course_staff(course_key: CourseKey, user: User):
"""
Check if user has course instructor or course staff role.
"""
return CourseInstructorRole(course_key).has_user(user) or CourseStaffRole(course_key).has_user(user)
# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
CourseInstructorRole(course_key).has_user(user) or
CourseStaffRole(course_key).has_user(user) or
user.has_perm(CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key)
)


def is_privileged_user(course_key: CourseKey, user: User):
Expand All @@ -772,7 +778,13 @@ def is_privileged_user(course_key: CourseKey, user: User):
FORUM_ROLE_ADMINISTRATOR,
]
has_course_role = Role.user_has_role_for_course(user, course_key, forum_roles)
return GlobalStaff().has_user(user) or has_course_role
# TODO: remove role checks once course_roles is fully impelented and data is migrated
has_moderate_discussion_permissions = user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key
) or user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.perm_name, course_key
)
return GlobalStaff().has_user(user) or has_course_role or has_moderate_discussion_permissions


class DiscussionBoardFragmentView(EdxFragmentView):
Expand Down Expand Up @@ -804,7 +816,11 @@ def render_to_fragment( # lint-amnesty, pylint: disable=arguments-differ
course_key = CourseKey.from_string(course_id)
# Force using the legacy view if a user profile is requested or the URL contains a specific topic or thread
force_legacy_view = (profile_page_context or thread_id or discussion_id)
is_educator_or_staff = is_course_staff(course_key, request.user) or GlobalStaff().has_user(request.user)
# TODO: remove role checks once course_roles is fully impelented and data is migrated
is_educator_or_staff = (
is_course_staff(course_key, request.user) or GlobalStaff().has_user(request.user) or
request.user.has_perm(CourseRolesPermission.MANAGE_DISCUSSION_MODERATORS.perm_name, course_key)
)
try:
base_context = _create_base_discussion_view_context(request, course_key)
# Note:
Expand Down
21 changes: 18 additions & 3 deletions openedx/core/djangoapps/course_live/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.lib.api.view_utils import validate_course_key


Expand All @@ -22,10 +23,11 @@ def has_permission(self, request, view):

if GlobalStaff().has_user(request.user):
return True

# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
CourseInstructorRole(course_key).has_user(request.user) or
CourseStaffRole(course_key).has_user(request.user)
CourseStaffRole(course_key).has_user(request.user) or
request.user.has_perm(CourseRolesPermission.MANAGE_CONTENT.perm_name, course_key)
)


Expand All @@ -41,8 +43,21 @@ def has_permission(self, request, view):
if GlobalStaff().has_user(request.user):
return True

user_has_permissions = (
request.user.has_perm(
CourseRolesPermission.VIEW_ALL_CONTENT.perm_name, course_key
) or
request.user.has_perm(
CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT.perm_name, course_key
) or
request.user.has_perm(
CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT.perm_name, course_key
)
)
# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
CourseInstructorRole(course_key).has_user(request.user) or
CourseStaffRole(course_key).has_user(request.user) or
CourseEnrollment.is_enrolled(request.user, course_key)
CourseEnrollment.is_enrolled(request.user, course_key) or
user_has_permissions
)
19 changes: 18 additions & 1 deletion openedx/core/djangoapps/course_live/tab.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from lms.djangoapps.courseware.tabs import EnrolledTab
from openedx.core.djangoapps.course_live.models import CourseLiveConfiguration
from openedx.core.djangoapps.course_live.providers import HasGlobalCredentials, ProviderManager
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.lib.cache_utils import request_cached
from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url
from openedx.features.lti_course_tab.tab import LtiCourseLaunchMixin
Expand All @@ -33,7 +34,23 @@ def user_is_staff_or_instructor(user: AbstractBaseUser, course: CourseBlock) ->
"""
Check if the user is a staff or instructor for the course.
"""
return CourseStaffRole(course.id).has_user(user) or CourseInstructorRole(course.id).has_user(user)
user_has_permissions = (
user.has_perm(
CourseRolesPermission.VIEW_ALL_CONTENT.perm_name, course.id
) or
user.has_perm(
CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT.perm_name, course.id
) or
user.has_perm(
CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT.perm_name, course.id
)
)
# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
CourseStaffRole(course.id).has_user(user) or
CourseInstructorRole(course.id).has_user(user) or
user_has_permissions
)


class CourseLiveTab(LtiCourseLaunchMixin, TabFragmentViewMixin, EnrolledTab):
Expand Down
80 changes: 20 additions & 60 deletions openedx/core/djangoapps/course_roles/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
from bridgekeeper import perms
from bridgekeeper.rules import is_staff

from lms.djangoapps.courseware.rules import HasRolesRule
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.djangoapps.course_roles.rules import HasPermissionRule, HasForumsRolesRule
from openedx.core.djangoapps.course_roles.rules import HasPermissionRule


# DO NOT USE FOR AUTHORIZATION
Expand All @@ -17,89 +16,50 @@
perms['course_roles.is_staff'] = (
is_staff
)
perms[CourseRolesPermission.MANAGE_CONTENT.perm_name] = (
HasRolesRule('staff', 'instructor')
| HasPermissionRule(CourseRolesPermission.MANAGE_CONTENT)
)
perms[CourseRolesPermission.MANAGE_CONTENT.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_CONTENT)
perms[CourseRolesPermission.MANAGE_COURSE_SETTINGS.perm_name] = (
HasRolesRule('staff', 'instructor')
| HasPermissionRule(CourseRolesPermission.MANAGE_COURSE_SETTINGS)
HasPermissionRule(CourseRolesPermission.MANAGE_COURSE_SETTINGS)
)
perms[CourseRolesPermission.MANAGE_ADVANCED_SETTINGS.perm_name] = (
HasRolesRule('staff', 'instructor')
| HasPermissionRule(CourseRolesPermission.MANAGE_ADVANCED_SETTINGS)
)
perms[CourseRolesPermission.VIEW_ALL_CONTENT.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.VIEW_ALL_CONTENT)
HasPermissionRule(CourseRolesPermission.MANAGE_ADVANCED_SETTINGS)
)
perms[CourseRolesPermission.VIEW_ALL_CONTENT.perm_name] = HasPermissionRule(CourseRolesPermission.VIEW_ALL_CONTENT)
perms[CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT.perm_name] = (
HasRolesRule('beta_testers', 'ccx_coach')
| HasForumsRolesRule('administrator')
| HasPermissionRule(CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT)
HasPermissionRule(CourseRolesPermission.VIEW_LIVE_PUBLISHED_CONTENT)
)
perms[CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT.perm_name] = (
HasPermissionRule(CourseRolesPermission.VIEW_ALL_PUBLISHED_CONTENT)
)
perms[CourseRolesPermission.ACCESS_INSTRUCTOR_DASHBOARD.perm_name] = (
HasRolesRule('staff', 'instructor', 'ccx_coach', 'data_researcher', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.ACCESS_INSTRUCTOR_DASHBOARD)
HasPermissionRule(CourseRolesPermission.ACCESS_INSTRUCTOR_DASHBOARD)
)
perms[CourseRolesPermission.ACCESS_DATA_DOWNLOADS.perm_name] = (
HasRolesRule('data_researcher')
| HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS)
)
perms[CourseRolesPermission.MANAGE_GRADES.perm_name] = (
HasRolesRule('staff', 'instructor', 'ccx_coach', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.MANAGE_GRADES)
)
perms[CourseRolesPermission.VIEW_GRADEBOOK.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.VIEW_GRADEBOOK)
)
perms[CourseRolesPermission.MANAGE_ALL_USERS.perm_name] = (
HasRolesRule('instructor')
| HasPermissionRule(CourseRolesPermission.MANAGE_ALL_USERS)
HasPermissionRule(CourseRolesPermission.ACCESS_DATA_DOWNLOADS)
)
perms[CourseRolesPermission.MANAGE_GRADES.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_GRADES)
perms[CourseRolesPermission.VIEW_GRADEBOOK.perm_name] = HasPermissionRule(CourseRolesPermission.VIEW_GRADEBOOK)
perms[CourseRolesPermission.MANAGE_ALL_USERS.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_ALL_USERS)
perms[CourseRolesPermission.MANAGE_USERS_EXCEPT_ADMIN_AND_STAFF.perm_name] = (
HasRolesRule('staff', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.MANAGE_USERS_EXCEPT_ADMIN_AND_STAFF)
HasPermissionRule(CourseRolesPermission.MANAGE_USERS_EXCEPT_ADMIN_AND_STAFF)
)
perms[CourseRolesPermission.MANAGE_DISCUSSION_MODERATORS.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasForumsRolesRule('administrator')
| HasPermissionRule(CourseRolesPermission.MANAGE_DISCUSSION_MODERATORS)
)
perms[CourseRolesPermission.MANAGE_COHORTS.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.MANAGE_COHORTS)
)
perms[CourseRolesPermission.MANAGE_STUDENTS.perm_name] = (
HasRolesRule('staff', 'instructor', 'ccx_coach', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.MANAGE_STUDENTS)
HasPermissionRule(CourseRolesPermission.MANAGE_DISCUSSION_MODERATORS)
)
perms[CourseRolesPermission.MANAGE_COHORTS.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_COHORTS)
perms[CourseRolesPermission.MANAGE_STUDENTS.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_STUDENTS)
perms[CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasForumsRolesRule('administrator', 'moderator', 'community_ta')
| HasPermissionRule(CourseRolesPermission.MODERATE_DISCUSSION_FORUMS)
HasPermissionRule(CourseRolesPermission.MODERATE_DISCUSSION_FORUMS)
)
perms[CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasForumsRolesRule('administrator', 'moderator', 'group_moderator', 'community_ta')
| HasPermissionRule(CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT)
HasPermissionRule(CourseRolesPermission.MODERATE_DISCUSSION_FORUMS_FOR_A_COHORT)
)
perms[CourseRolesPermission.MANAGE_CERTIFICATES.perm_name] = (
HasPermissionRule(CourseRolesPermission.MANAGE_CERTIFICATES)
)
perms[CourseRolesPermission.MANAGE_LIBRARIES.perm_name] = (
HasRolesRule('library_user')
| HasPermissionRule(CourseRolesPermission.MANAGE_LIBRARIES)
)
perms[CourseRolesPermission.MANAGE_LIBRARIES.perm_name] = HasPermissionRule(CourseRolesPermission.MANAGE_LIBRARIES)
perms[CourseRolesPermission.GENERAL_MASQUERADING.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.GENERAL_MASQUERADING)
HasPermissionRule(CourseRolesPermission.GENERAL_MASQUERADING)
)
perms[CourseRolesPermission.SPECIFIC_MASQUERADING.perm_name] = (
HasRolesRule('staff', 'instructor', 'limited_staff')
| HasPermissionRule(CourseRolesPermission.SPECIFIC_MASQUERADING)
HasPermissionRule(CourseRolesPermission.SPECIFIC_MASQUERADING)
)
6 changes: 5 additions & 1 deletion openedx/core/djangoapps/discussions/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff, CourseInstructorRole
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
from openedx.core.djangoapps.course_roles.data import CourseRolesPermission
from openedx.core.lib.api.view_utils import validate_course_key

DEFAULT_MESSAGE = "You're not authorized to perform this operation."
Expand All @@ -29,10 +30,13 @@ def has_permission(self, request, view):

if GlobalStaff().has_user(request.user):
return True

# TODO: remove role checks once course_roles is fully impelented and data is migrated
return (
CourseInstructorRole(course_key).has_user(request.user) or
CourseStaffRole(course_key).has_user(request.user) or
request.user.has_perm(
CourseRolesPermission.MODERATE_DISCUSSION_FORUMS.perm_name, course_key
) or
has_discussion_privileges(request.user, course_key)
)

Expand Down

0 comments on commit 7807f3e

Please sign in to comment.