From 6a3c30aa71b83e385e6f5e698187b33c7cbff1e3 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 9 Apr 2024 15:06:25 +0930 Subject: [PATCH] fix: get_access_ids_for_request only returns individual access instead of all course keys that the user can read. Org- and GlobalStaff access checks will handle the rest. --- .../core/djangoapps/content/search/models.py | 17 ++++++++++------- .../content/search/tests/test_models.py | 10 ++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index b46299a9f362..c312ec6e166b 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -2,14 +2,13 @@ from __future__ import annotations -from typing import List - from django.db import models from django.utils.translation import gettext_lazy as _ from opaque_keys.edx.django.models import LearningContextKeyField from rest_framework.request import Request -from cms.djangoapps.contentstore.views.course import get_courses_accessible_to_user +from common.djangoapps.student.role_helpers import get_course_roles +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user @@ -35,14 +34,18 @@ class SearchAccess(models.Model): ) -def get_access_ids_for_request(request: Request) -> List[int]: +def get_access_ids_for_request(request: Request) -> list[int]: """ - Returns the SearchAccess.id values that the user has read access to. + Returns a list of SearchAccess.id values for courses and content libraries that the requesting user has been + individually grated access to. """ - courses, _ = get_courses_accessible_to_user(request) + course_roles = get_course_roles(request.user) course_clause = models.Q(context_key__in=[ - course.id for course in courses + role.course_id + for role in course_roles + if role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] ]) + libraries = get_libraries_for_user(user=request.user) library_clause = models.Q(context_key__in=[ lib.library_key for lib in libraries diff --git a/openedx/core/djangoapps/content/search/tests/test_models.py b/openedx/core/djangoapps/content/search/tests/test_models.py index a447b412b603..0e8048373689 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -16,7 +16,7 @@ from openedx.core.djangoapps.content.search.models import SearchAccess, get_access_ids_for_request except RuntimeError: SearchAccess = {} - get_access_ids_for_request = lambda x: x + get_access_ids_for_request = lambda request: [] @skip_unless_cms @@ -32,7 +32,6 @@ def setUp(self): super().setUp() self.global_staff = UserFactory(password=self.TEST_PASSWORD) GlobalStaff().add_users(self.global_staff) - self.staff_user_keys = [] self.course_staff = UserFactory(password=self.TEST_PASSWORD) self.course_instructor = UserFactory(password=self.TEST_PASSWORD) @@ -47,13 +46,11 @@ def setUp(self): CourseStaffRole(course_location).add_users(self.course_staff) CourseInstructorRole(course_location).add_users(self.course_instructor) self.course_user_keys.append(course_location) - self.staff_user_keys.append(course_location) # Create a few courses that only global_staff can access for num in range(3): course_location = self.store.make_course_key('Org', 'StaffCourse' + str(num), 'Run') self._create_course(course_location) - self.staff_user_keys.append(course_location) # Create orgs to test library access self.org1, _ = Organization.objects.get_or_create( @@ -146,7 +143,9 @@ def test_course_instructor_get_access_ids_for_request(self): self._check_access_ids(access_ids, self.course_user_keys) def test_staff_get_access_ids_for_request(self): - """Global staff can see all courses and libraries""" + """ + Global staff can see all courses and libraries, but they only have individual access granted for libraries. + """ request = RequestFactory().get('/course') request.user = self.global_staff @@ -156,7 +155,6 @@ def test_staff_get_access_ids_for_request(self): def test_delete_removes_access_ids_for_request(self): """Removing courses and library should remove their associated access_ids.""" remaining_keys = self.staff_user_keys - remaining_keys.remove(self.last_course.id) remaining_keys.remove(self.last_library.key) self.last_course.delete() library_api.delete_library(self.last_library.key)