Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roles 15 - permission checks back end changes part 1 #33347

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ddcccf1
test: add test cases for permission list check functions
julianpalmerio Sep 19, 2023
6c79a3d
test: update tests
julianpalmerio Sep 19, 2023
8341962
feat: add helper functions to check lists of permissions
julianpalmerio Sep 19, 2023
ee11185
style: improve code style
julianpalmerio Sep 19, 2023
883d15b
feat: add course roles checks in the contentstore app
julianpalmerio Sep 26, 2023
4f887d3
feat: add course roles checks in the student app
julianpalmerio Sep 26, 2023
6e5de16
feat: add course roles checks in the lms discussion app
julianpalmerio Sep 26, 2023
56ed879
feat: add course roles checks in the lms instructor app
julianpalmerio Sep 26, 2023
64c4146
feat: add course roles checks in the Learning Sequences package
julianpalmerio Sep 26, 2023
e511114
style: fix code style
julianpalmerio Sep 26, 2023
19c0039
fix: course_permission_check calls
julianpalmerio Sep 26, 2023
570d12d
feat: add validation for AnonymousUser in course permission check hel…
julianpalmerio Sep 28, 2023
d977e37
fix: disable some pylint warnings
julianpalmerio Sep 28, 2023
782c056
test: update number of querys asserted in has_course_author_access
julianpalmerio Sep 28, 2023
e64f1f6
feat: add helper functions to check course or organization permissions
julianpalmerio Sep 28, 2023
b2d6932
test: update course_roles tests
julianpalmerio Sep 28, 2023
3a6d2eb
feat: replace course or organization helper functions in auth
julianpalmerio Sep 28, 2023
2233cb0
docs: update course_roles docstrings
julianpalmerio Sep 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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 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)
)
58 changes: 53 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,19 +109,48 @@ 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 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 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

# Limited Course Staff does not have access to Studio.
# TODO: course roles: Remove this validation when course roles are implemented
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding clarity by explaining which type of course roles you mean (student_courseaccessrole, django_comment_client_role, or course_roles_roles)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to refer to the course_roles Django App . What you think about changing the comment to: "# TODO: course roles: Remove this validation when implementing course_roles django app."?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is a positive change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)):
return STUDIO_NO_PERMISSIONS
# 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 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 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 @@ -223,7 +272,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 @@ -333,6 +334,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 @@ -358,8 +360,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 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.
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 @@ -50,6 +50,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 @@ -185,10 +186,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 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 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
Loading