Skip to content

Commit

Permalink
feat: move progress related apis permissions to standard permission f…
Browse files Browse the repository at this point in the history
…ile so can be override later
  • Loading branch information
tehreem-sadat committed Jan 10, 2025
1 parent 0630c65 commit e4c6a85
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 6 deletions.
8 changes: 4 additions & 4 deletions lms/djangoapps/course_home_api/course_metadata/views.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 lms.djangoapps.course_api.api import course_detail
from lms.djangoapps.course_goals.models import UserActivity
from lms.djangoapps.course_home_api import permissions
from lms.djangoapps.course_home_api.course_metadata.serializers import CourseHomeMetadataSerializer
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs
Expand Down Expand Up @@ -81,25 +82,24 @@ def get(self, request, *args, **kwargs):
course_key = CourseKey.from_string(course_key_string)
original_user_is_global_staff = self.request.user.is_staff
original_user_is_staff = has_access(request.user, 'staff', course_key).has_access

user_can_masquarade = request.user.has_perm(permissions.CAN_MASQUARADE_LEARNER_PROGRESS, course_key)
course = course_detail(request, request.user.username, course_key)

# We must compute course load access *before* setting up masquerading,
# else course staff (who are not enrolled) will not be able view
# their course from the perspective of a learner.
load_access = check_course_access(
course,
request.user,
'load',
check_if_enrolled=True,
check_if_enrolled=(not user_can_masquarade),
check_if_authenticated=True,
apply_enterprise_checks=True,
)

_, request.user = setup_masquerade(
request,
course_key,
staff_access=original_user_is_staff,
staff_access=user_can_masquarade,
reset_masquerade_data=True,
)

Expand Down
10 changes: 10 additions & 0 deletions lms/djangoapps/course_home_api/permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
"""
Permissions for the course home apis and associated actions
"""
from bridgekeeper import perms
from lms.djangoapps.courseware.rules import HasAccessRule


CAN_MASQUARADE_LEARNER_PROGRESS = 'course_home_api.can_masquarade_progress'

perms[CAN_MASQUARADE_LEARNER_PROGRESS] = HasAccessRule('staff')
6 changes: 4 additions & 2 deletions lms/djangoapps/course_home_api/progress/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from xmodule.modulestore.django import modulestore
from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.course_home_api import permissions
from lms.djangoapps.course_home_api.progress.serializers import ProgressTabSerializer
from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active
from lms.djangoapps.courseware.access import has_access, has_ccx_coach_role
Expand Down Expand Up @@ -188,8 +189,9 @@ def get(self, request, *args, **kwargs):
monitoring_utils.set_custom_attribute('user_id', request.user.id)
monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff)
is_staff = bool(has_access(request.user, 'staff', course_key))
can_masquarade = request.user.has_perm(permissions.CAN_MASQUARADE_LEARNER_PROGRESS, course_key)

student = self._get_student_user(request, course_key, student_id, is_staff)
student = self._get_student_user(request, course_key, student_id, can_masquarade)
username = get_enterprise_learner_generic_name(request) or student.username

course = get_course_or_403(student, 'load', course_key, check_if_enrolled=False)
Expand All @@ -198,7 +200,7 @@ def get(self, request, *args, **kwargs):
enrollment = CourseEnrollment.get_enrollment(student, course_key)
enrollment_mode = getattr(enrollment, 'mode', None)

if not (enrollment and enrollment.is_active) and not is_staff:
if not (enrollment and enrollment.is_active) and not can_masquarade:
return Response('User not enrolled.', status=401)

# The block structure is used for both the course_grade and has_scheduled content fields
Expand Down
70 changes: 70 additions & 0 deletions lms/djangoapps/course_home_api/tests/test_permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
"""
Tests for permissions defined in courseware.rules
"""
import ddt

from common.djangoapps.student.roles import OrgStaffRole, CourseStaffRole
from common.djangoapps.student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
from lms.djangoapps.course_home_api.permissions import CAN_MASQUARADE_LEARNER_PROGRESS


@ddt.ddt
class PermissionTests(ModuleStoreTestCase):
"""
Tests for permissions defined in courseware.rules
"""
def setUp(self):
super().setUp()
self.user = UserFactory()
self.course = CourseFactory(org='org')
self.another_course = CourseFactory(org='org')

def tearDown(self):
super().tearDown()
self.user.delete()

@ddt.data(
(
True, None, None, True,
"Global staff users should have masquerade access",
),
(
False, None, None, False,
"Non-staff users shouldn't have masquerade access",
),
(
False, 'another_org', None, False,
"User with staff access on another org shouldn't have masquerade access",
),
(
False, 'org', None, True,
"User with org-wide staff access should have masquerade access",
),
(
False, None, 'another_course', False,
"User with staff access on another course shouldn't have masquerade access",
),
(
False, None, 'course', True,
"User with staff access on the course should have masquerade access",
),
)
@ddt.unpack
def test_can_masquerade_return_value(self, is_staff, org_role, course_role, expected_permission, description):
"""
Test that only authorized users have masquerade access
"""
self.user.is_staff = is_staff
self.user.save()
assert self.user.is_staff == is_staff

if org_role:
OrgStaffRole(org_role).add_users(self.user)

if course_role:
CourseStaffRole(getattr(self, course_role).id).add_users(self.user)

has_perm = self.user.has_perm(CAN_MASQUARADE_LEARNER_PROGRESS, self.course.id)
assert has_perm == expected_permission, description

0 comments on commit e4c6a85

Please sign in to comment.