Skip to content

Commit

Permalink
Merge pull request #33275 from open-craft/0x29a/bb7834/inherited-role…
Browse files Browse the repository at this point in the history
…s-studio-fix

Fix studio for users with derived roles and some other related changes [BB-7834]
  • Loading branch information
Agrendalath authored Oct 16, 2023
2 parents 755fa7f + 9e56a18 commit 6b082c7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 8 deletions.
24 changes: 20 additions & 4 deletions common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import logging
from abc import ABCMeta, abstractmethod
from collections import defaultdict
from contextlib import contextmanager

from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from opaque_keys.edx.django.models import CourseKeyField
Expand Down Expand Up @@ -44,6 +45,21 @@ def register_access_role(cls):
return cls


@contextmanager
def strict_role_checking():
"""
Context manager that temporarily disables role inheritance.
You may want to use it to check if a user has a base role. For example, if a user has `CourseLimitedStaffRole`,
by enclosing `has_role` call with this context manager, you can check it has the `CourseStaffRole` too. This is
useful when derived roles have less permissions than their base roles, but users can have both roles at the same.
"""
OLD_ACCESS_ROLES_INHERITANCE = ACCESS_ROLES_INHERITANCE.copy()
ACCESS_ROLES_INHERITANCE.clear()
yield
ACCESS_ROLES_INHERITANCE.update(OLD_ACCESS_ROLES_INHERITANCE)


class BulkRoleCache: # lint-amnesty, pylint: disable=missing-class-docstring
CACHE_NAMESPACE = "student.roles.BulkRoleCache"
CACHE_KEY = 'roles_by_user'
Expand Down Expand Up @@ -78,7 +94,7 @@ def __init__(self, user):
)

@staticmethod
def _get_roles(role):
def get_roles(role):
"""
Return the roles that should have the same permissions as the specified role.
"""
Expand All @@ -90,7 +106,7 @@ def has_role(self, role, course_id, org):
or a role that inherits from the specified role, course_id and org.
"""
return any(
access_role.role in self._get_roles(role) and
access_role.role in self.get_roles(role) and
access_role.course_id == course_id and
access_role.org == org
for access_role in self._roles
Expand Down Expand Up @@ -463,11 +479,11 @@ def remove_courses(self, *course_keys):

def courses_with_role(self):
"""
Return a django QuerySet for all of the courses with this user x role. You can access
Return a django QuerySet for all of the courses with this user x (or derived from x) role. You can access
any of these properties on each result record:
* user (will be self.user--thus uninteresting)
* org
* course_id
* role (will be self.role--thus uninteresting)
"""
return CourseAccessRole.objects.filter(role=self.role, user=self.user)
return CourseAccessRole.objects.filter(role__in=RoleCache.get_roles(self.role), user=self.user)
8 changes: 6 additions & 2 deletions lms/djangoapps/courseware/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.enrollments.api import is_enrollment_valid_for_proctoring
from common.djangoapps.student.models import CourseAccessRole
from common.djangoapps.student.roles import CourseRole, OrgRole
from common.djangoapps.student.roles import CourseRole, OrgRole, strict_role_checking
from xmodule.course_block import CourseBlock # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.error_block import ErrorBlock # lint-amnesty, pylint: disable=wrong-import-order

Expand Down Expand Up @@ -47,10 +47,14 @@ class HasAccessRule(Rule):
"""
A rule that calls `has_access` to determine whether it passes
"""
def __init__(self, action):
def __init__(self, action, strict=False):
self.action = action
self.strict = strict

def check(self, user, instance=None):
if self.strict:
with strict_role_checking():
return has_access(user, self.action, instance)
return has_access(user, self.action, instance)

def query(self, user):
Expand Down
4 changes: 4 additions & 0 deletions lms/djangoapps/instructor/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
RESCORE_EXAMS = 'instructor.rescore_exams'
VIEW_REGISTRATION = 'instructor.view_registration'
VIEW_DASHBOARD = 'instructor.dashboard'
VIEW_ENROLLMENTS = 'instructor.view_enrollments'
VIEW_FORUM_MEMBERS = 'instructor.view_forum_members'


perms[ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM] = HasAccessRule('staff')
Expand Down Expand Up @@ -60,3 +62,5 @@
'instructor',
'data_researcher'
) | HasAccessRule('staff') | HasAccessRule('instructor')
perms[VIEW_ENROLLMENTS] = HasAccessRule('staff')
perms[VIEW_FORUM_MEMBERS] = HasAccessRule('staff')
4 changes: 2 additions & 2 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,7 @@ def get_anon_ids(request, course_id):
@require_POST
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_course_permission(permissions.CAN_ENROLL)
@require_course_permission(permissions.VIEW_ENROLLMENTS)
@require_post_params(
unique_student_identifier="email or username of student for whom to get enrollment status"
)
Expand Down Expand Up @@ -2611,7 +2611,7 @@ def problem_grade_report(request, course_id):
@require_POST
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_course_permission(permissions.CAN_ENROLL)
@require_course_permission(permissions.VIEW_FORUM_MEMBERS)
@require_post_params('rolename')
def list_forum_members(request, course_id):
"""
Expand Down

0 comments on commit 6b082c7

Please sign in to comment.