Skip to content

Commit

Permalink
feat: Roles 15 - permission checks back end changes part 1 (#33347)
Browse files Browse the repository at this point in the history
* test: add test cases for permission list check functions

* test: update tests

* feat: add helper functions to check lists of permissions

* style: improve code style

* feat: add course roles checks in the contentstore app

* feat: add course roles checks in the student app

* feat: add course roles checks in the lms discussion app

* feat: add course roles checks in the lms instructor app

* feat: add course roles checks in the Learning Sequences package

* style: fix code style

* fix: course_permission_check calls

* feat: add validation for AnonymousUser in course permission check helper functions

* fix: disable some pylint warnings

* test: update number of querys asserted in has_course_author_access

* feat: add helper functions to check course or organization permissions

* test: update course_roles tests

* feat: replace course or organization helper functions in auth

* docs: update course_roles docstrings
  • Loading branch information
julianpalmerio authored and hsinkoff committed Oct 27, 2023
1 parent 675006e commit 29e7975
Show file tree
Hide file tree
Showing 10 changed files with 497 additions and 19 deletions.
14 changes: 12 additions & 2 deletions cms/djangoapps/contentstore/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole
from common.djangoapps.util.json_request import JsonResponse, expect_json
from openedx.core.djangoapps.course_roles.helpers import course_permission_check

from ..toggles import use_new_course_team_page
from ..utils import get_course_team_url, get_course_team
Expand Down Expand Up @@ -118,6 +119,7 @@ def _course_team_user(request, course_key, email):
else:
role_hierarchy = (CourseInstructorRole, CourseStaffRole)

MANAGE_ALL_USERS_PERMISSION = "manage_all_users"
if request.method == "GET":
# just return info about the user
msg = {
Expand All @@ -127,7 +129,11 @@ def _course_team_user(request, course_key, email):
}
# what's the highest role that this user has? (How should this report global staff?)
for role in role_hierarchy:
if role(course_key).has_user(user):
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check call
# below will never be true.
# Remove the .has_user call when course_roles Django app are implemented.
if (role(course_key).has_user(user) or
course_permission_check(user, MANAGE_ALL_USERS_PERMISSION, course_key)):
msg["role"] = role.ROLE
break
return JsonResponse(msg)
Expand Down Expand Up @@ -167,7 +173,11 @@ def _course_team_user(request, course_key, email):
role_added = True
else:
return permissions_error_response
elif role.has_user(user, check_user_activation=False): # pylint: disable=no-value-for-parameter
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check call
# below will never return true.
# Remove the .has_user() call below when course_roles Django app are implemented.
elif (role.has_user(user, check_user_activation=False) or # pylint: disable=no-value-for-parameter
course_permission_check(user, MANAGE_ALL_USERS_PERMISSION, course_key)):
# Remove the user from this old role:
old_roles.add(role)

Expand Down
10 changes: 8 additions & 2 deletions common/djangoapps/student/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
GlobalStaff,
REGISTERED_ACCESS_ROLES as _REGISTERED_ACCESS_ROLES,
)
from openedx.core.djangoapps.course_roles.helpers import course_permission_check
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers


Expand Down Expand Up @@ -130,9 +131,14 @@ def is_user_staff_or_instructor_in_course(user, course_key):
"""
if not isinstance(course_key, CourseKey):
course_key = CourseKey.from_string(course_key)

MANAGE_STUDENTS_PERMISSION = "manage_students"
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check call
# below will never be true.
# Remove the CourseStaffRole and the CourseInstructorRole .has_user() calls below when course
# roles are implemented.
return (
GlobalStaff().has_user(user) or
CourseStaffRole(course_key).has_user(user) or
CourseInstructorRole(course_key).has_user(user)
CourseInstructorRole(course_key).has_user(user) or
course_permission_check(user, MANAGE_STUDENTS_PERMISSION, course_key)
)
59 changes: 54 additions & 5 deletions common/djangoapps/student/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
OrgLibraryUserRole,
OrgStaffRole
)
from openedx.core.djangoapps.course_roles.helpers import (
course_permissions_list_check,
course_or_organization_permission_check,
course_or_organization_permission_list_check,
organization_permissions_list_check
)

# Studio permissions:
STUDIO_EDIT_ROLES = 8
Expand Down Expand Up @@ -79,6 +85,20 @@ def get_user_permissions(user, course_key, org=None):
Can also set course_key=None and pass in an org to get the user's
permissions for that organization as a whole.
"""
COURSE_INSTRUCTOR_ROLE_PERMISSIONS = [
"edit_content",
"manage_course_settings",
"manage_adv_settings",
"view_course_settings",
"manage_all_users"
]
STAFF_ROLE_PERMISSIONS = [
"edit_content",
"manage_course_settings",
"manage_adv_settings",
"view_course_settings",
"manage_users_except_admin_and_staff"
]
if org is None:
org = course_key.org
course_key = course_key.for_branch(None)
Expand All @@ -89,9 +109,23 @@ def get_user_permissions(user, course_key, org=None):
return STUDIO_NO_PERMISSIONS
all_perms = STUDIO_EDIT_ROLES | STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
# global staff, org instructors, and course instructors have all permissions:
if GlobalStaff().has_user(user) or OrgInstructorRole(org=org).has_user(user):
# TODO: course roles: If the course roles feature flag is disabled the organization_permissions_list_check call
# below will never return true.
# Remove the OrgInstructorRole .has_user call when course_roles Django app are implemented.
if (
GlobalStaff().has_user(user)
or OrgInstructorRole(org=org).has_user(user)
or organization_permissions_list_check(user, COURSE_INSTRUCTOR_ROLE_PERMISSIONS, org)
):
return all_perms
if course_key and user_has_role(user, CourseInstructorRole(course_key)):

# TODO: course roles: If the course roles feature flag is disabled the course_permissions_list_check call
# below will never return true.
# Remove the user_has_role call when course_roles Django app are implemented.
if course_key and (
user_has_role(user, CourseInstructorRole(course_key))
or course_permissions_list_check(user, COURSE_INSTRUCTOR_ROLE_PERMISSIONS, course_key)
):
return all_perms
# HACK: Limited Staff should not have studio read access. However, since many LMS views depend on the
# `has_course_author_access` check and `course_author_access_required` decorator, we have to allow write access
Expand All @@ -101,14 +135,30 @@ def get_user_permissions(user, course_key, org=None):
# The permissions matrix from the RBAC project (https://github.com/openedx/platform-roadmap/issues/246) shows that
# the LMS and Studio permissions will be separated as a part of this project. Once this is done (and this code is
# not removed during its implementation), we can replace the Limited Staff permissions with more granular ones.

# Limited Course Staff does not have access to Studio.
# TODO: course roles: Remove this validation when course roles app are implemented
if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)):
return STUDIO_EDIT_CONTENT
# Staff have all permissions except EDIT_ROLES:
if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
# TODO: course roles: If the course roles feature flag is disabled the
# course_or_organization_permissions_list_check call below will never return true.
# Remove the OrgStaffRole has_user call and the user_has_role call when course_roles Django app are implemented.
if (OrgStaffRole(org=org).has_user(user) or
(course_key and user_has_role(user, CourseStaffRole(course_key)))) or (
course_or_organization_permission_list_check(user, STAFF_ROLE_PERMISSIONS, course_key, org)
):
return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
# Otherwise, for libraries, users can view only:
LIBRARY_USER_ROLE_PERMISSION = "view_library"
if course_key and isinstance(course_key, LibraryLocator):
if OrgLibraryUserRole(org=org).has_user(user) or user_has_role(user, LibraryUserRole(course_key)):
# TODO: course roles: If the course roles feature flag is disabled the course_or_organization_permission_check
# call below will never return true.
# Remove the OrgLibraryUserRole has_user call and the user_has_role call
# when course_roles Django app are implemented.
if (OrgLibraryUserRole(org=org).has_user(user) or user_has_role(user, LibraryUserRole(course_key))) or (
course_or_organization_permission_check(user, LIBRARY_USER_ROLE_PERMISSION, course_key, org)
):
return STUDIO_VIEW_USERS | STUDIO_VIEW_CONTENT
return STUDIO_NO_PERMISSIONS

Expand Down Expand Up @@ -230,7 +280,6 @@ def _check_caller_authority(caller, role):
# superuser
if GlobalStaff().has_user(caller):
return

if isinstance(role, (GlobalStaff, CourseCreatorRole, OrgContentCreatorRole)): # lint-amnesty, pylint: disable=no-else-raise
raise PermissionDenied
elif isinstance(role, CourseRole): # instructors can change the roles w/in their course
Expand Down
11 changes: 9 additions & 2 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE, ENABLE_LEARNERS_TAB_IN_DISCUSSIONS_MFE
from lms.djangoapps.discussion.toggles_utils import reported_content_email_notification_enabled
from lms.djangoapps.discussion.views import is_privileged_user
from openedx.core.djangoapps.course_roles.helpers import course_permission_check
from openedx.core.djangoapps.discussions.models import (
DiscussionsConfiguration,
DiscussionTopicLink,
Expand Down Expand Up @@ -334,6 +335,7 @@ def _format_datetime(dt):
course.get_discussion_blackout_datetimes()
)

MODEREATE_DISCUSSION_FORUM_PERMISSION = 'moderate_discussion_forum'
return {
"id": str(course_key),
"is_posting_enabled": is_posting_enabled,
Expand All @@ -359,8 +361,13 @@ 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: course roles: If the course roles feature flag is disabled the calls to course_permission_check
# below will never be true.
# Remove .has_user() calls below when course_roles Django app are implemented.
"is_course_staff": (CourseStaffRole(course_key).has_user(request.user) or
course_permission_check(request.user, MODEREATE_DISCUSSION_FORUM_PERMISSION, course_key)),
"is_course_admin": (CourseInstructorRole(course_key).has_user(request.user) or
course_permission_check(request.user, MODEREATE_DISCUSSION_FORUM_PERMISSION, 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
8 changes: 7 additions & 1 deletion 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.helpers import course_permission_check
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)
MODEREATE_DISCUSSION_FORUM_PERMISSION = 'moderate_discussion_forum'
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check
# below will never return true.
# Remove .has_user() calls when implementing course_roles Django app.
return ((CourseInstructorRole(course_key).has_user(user) or CourseStaffRole(course_key).has_user(user)) or
course_permission_check(user, MODEREATE_DISCUSSION_FORUM_PERMISSION, course_key))


def is_privileged_user(course_key: CourseKey, user: User):
Expand Down
8 changes: 7 additions & 1 deletion lms/djangoapps/instructor/views/instructor_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
from lms.djangoapps.grades.api import is_writable_gradebook_enabled
from lms.djangoapps.instructor.constants import INSTRUCTOR_DASHBOARD_PLUGIN_VIEW_NAME
from openedx.core.djangoapps.course_groups.cohorts import DEFAULT_COHORT_NAME, get_course_cohorts, is_course_cohorted
from openedx.core.djangoapps.course_roles.helpers import course_permission_check
from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled
from openedx.core.djangoapps.discussions.utils import available_division_schemes
from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings
Expand Down Expand Up @@ -186,10 +187,15 @@ def instructor_dashboard_2(request, course_id): # lint-amnesty, pylint: disable
# Gate access to Special Exam tab depending if either timed exams or proctored exams
# are enabled in the course

MANAGE_STUDENTS_PERMISSION = "manage_students"
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check call
# below will never return true.
# Remove .has_user() calls below when course_roles Django app are implemented.
user_has_access = any([
request.user.is_staff,
CourseStaffRole(course_key).has_user(request.user),
CourseInstructorRole(course_key).has_user(request.user)
CourseInstructorRole(course_key).has_user(request.user),
course_permission_check(request.user, MANAGE_STUDENTS_PERMISSION, course_key)
])
course_has_special_exams = course.enable_proctored_exams or course.enable_timed_exams
can_see_special_exams = course_has_special_exams and user_has_access and settings.FEATURES.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from edx_when.api import get_dates_for_course
from opaque_keys.edx.keys import UsageKey, CourseKey # lint-amnesty, pylint: disable=unused-import
from openedx.core import types
from openedx.core.djangoapps.course_roles.helpers import course_permission_check

from common.djangoapps.student.auth import user_has_role
from common.djangoapps.student.roles import CourseBetaTesterRole
Expand Down Expand Up @@ -60,7 +61,15 @@ def load_data(self, full_course_outline):
course_usage_key = self.course_key.make_usage_key('course', 'course')
self._course_start = self.keys_to_schedule_fields[course_usage_key].get('start')
self._course_end = self.keys_to_schedule_fields[course_usage_key].get('end')
self._is_beta_tester = user_has_role(self.user, CourseBetaTesterRole(self.course_key))
VIEW_ALL_PUBLISHED_CONTENT_PERMISSION = "view_all_published_content"
VIEW_ONLY_LIVE_PUBLISHED_CONTENT_PERMISSION = "view_only_live_published_content"
# TODO: course roles: If the course roles feature flag is disabled the course_permission_check calls
# below will never return true.
# Remove the user_has_role call when course_roles Django app are implemented.
self._is_beta_tester = user_has_role(self.user, CourseBetaTesterRole(self.course_key)) or (
course_permission_check(self.user, VIEW_ALL_PUBLISHED_CONTENT_PERMISSION, self.course_key)
or course_permission_check(self.user, VIEW_ONLY_LIVE_PUBLISHED_CONTENT_PERMISSION, self.course_key)
)

def inaccessible_sequences(self, full_course_outline):
"""
Expand Down
50 changes: 50 additions & 0 deletions openedx/core/djangoapps/course_roles/helpers.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,80 @@
"""
Helpers for the course roles app.
"""
from django.contrib.auth.models import AnonymousUser

from openedx.core.djangoapps.course_roles.models import CourseRolesUserRole
from openedx.core.lib.cache_utils import request_cached
from xmodule.modulestore.django import modulestore


@request_cached()
def course_permission_check(user, permission_name, course_id):
"""
Check if a user has a permission in a course.
"""
if isinstance(user, AnonymousUser):
return False
return CourseRolesUserRole.objects.filter(
user=user,
role__permissions__name=permission_name,
course=course_id,
).exists()


@request_cached()
def course_permissions_list_check(user, permission_names, course_id):
"""
Check if a user has all of the given permissions in a course.
"""
return all(course_permission_check(user, permission_name, course_id) for permission_name in permission_names)


@request_cached()
def organization_permission_check(user, permission_name, organization_name):
"""
Check if a user has a permission in an organization.
"""
if isinstance(user, AnonymousUser):
return False
return CourseRolesUserRole.objects.filter(
user=user,
role__permissions__name=permission_name,
course__isnull=True,
org__name=organization_name,
).exists()


@request_cached()
def organization_permissions_list_check(user, permission_names, organization_name):
"""
Check if a user has all of the given permissions in an organization.
"""
return all(
organization_permission_check(user, permission_name, organization_name) for permission_name in permission_names
)


@request_cached()
def course_or_organization_permission_check(user, permission_name, course_id, organization_name=None):
"""
Check if a user has a permission in an organization or a course.
"""
if isinstance(user, AnonymousUser):
return False
if organization_name is None:
organization_name = modulestore().get_course(course_id).org
return (course_permission_check(user, permission_name, course_id) or
organization_permission_check(user, permission_name, organization_name)
)


@request_cached()
def course_or_organization_permission_list_check(user, permission_names, course_id, organization_name=None):
"""
Check if a user has all of the given permissions in an organization or a course.
"""
return all(
course_or_organization_permission_check(user, permission_name, course_id, organization_name)
for permission_name in permission_names
)
Loading

0 comments on commit 29e7975

Please sign in to comment.