From 811687d9f4271793bb786d9e2e31eca769b7c599 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 10 Apr 2024 09:49:13 +0930 Subject: [PATCH] feat: omit access_ids from the search filter that are covered by the user's org roles --- .../core/djangoapps/content/search/models.py | 12 ++++++-- .../content/search/tests/test_models.py | 10 +++++++ .../content/search/tests/test_views.py | 29 +++++++++++++++++++ .../core/djangoapps/content/search/views.py | 2 +- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index c312ec6e166b..711c493ff895 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -34,21 +34,29 @@ class SearchAccess(models.Model): ) -def get_access_ids_for_request(request: Request) -> list[int]: +def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> list[int]: """ Returns a list of SearchAccess.id values for courses and content libraries that the requesting user has been individually grated access to. + + Omits any courses/libraries with orgs in the `omit_orgs` list. """ + omit_orgs = omit_orgs or [] + course_roles = get_course_roles(request.user) course_clause = models.Q(context_key__in=[ role.course_id for role in course_roles - if role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] + if ( + role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] + and role.org not in omit_orgs + ) ]) libraries = get_libraries_for_user(user=request.user) library_clause = models.Q(context_key__in=[ lib.library_key for lib in libraries + if lib.library_key.org not in omit_orgs ]) # Sort by descending access ID to simulate prioritizing the "most recently created context keys". diff --git a/openedx/core/djangoapps/content/search/tests/test_models.py b/openedx/core/djangoapps/content/search/tests/test_models.py index a662660ff58d..ef42a1c04879 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -216,6 +216,16 @@ def test_staff_get_access_ids_for_request(self): access_ids = get_access_ids_for_request(request) self._check_access_ids(access_ids, self.staff_user_keys) + def test_get_access_ids_for_request_omit_orgs(self): + """ + Omit the org1 library keys from the returned list. + """ + request = RequestFactory().get('/course') + request.user = self.global_staff + + access_ids = get_access_ids_for_request(request, omit_orgs=['org1']) + self._check_access_ids(access_ids, self.staff_user_keys[-2:]) + 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 diff --git a/openedx/core/djangoapps/content/search/tests/test_views.py b/openedx/core/djangoapps/content/search/tests/test_views.py index 4b23898a082f..853f5b5149a6 100644 --- a/openedx/core/djangoapps/content/search/tests/test_views.py +++ b/openedx/core/djangoapps/content/search/tests/test_views.py @@ -8,6 +8,8 @@ from django.test import override_settings from rest_framework.test import APIClient +from common.djangoapps.student.auth import update_org_role +from common.djangoapps.student.roles import OrgStaffRole from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -193,6 +195,33 @@ def test_studio_search_org_access(self, username, mock_search_client): expires_at=mock.ANY, ) + @mock_meilisearch(enabled=True) + @mock.patch('openedx.core.djangoapps.content.search.views.meilisearch.Client') + def test_studio_search_omit_orgs(self, mock_search_client): + """ + Grant org access to our staff user to ensure that org's access_ids are omitted from the search filter. + """ + update_org_role(caller=self.global_staff, role=OrgStaffRole, user=self.course_staff, orgs=['org1']) + self.client.login(username='course_staff', password='course_staff_pass') + mock_generate_tenant_token = self._mock_generate_tenant_token(mock_search_client) + result = self.client.get(STUDIO_SEARCH_ENDPOINT_URL) + assert result.status_code == 200 + + expected_access_ids = list(SearchAccess.objects.filter( + context_key__in=[key for key in self.course_user_keys if key.org != 'org1'], + ).only('id').values_list('id', flat=True)) + expected_access_ids.sort(reverse=True) + + mock_generate_tenant_token.assert_called_once_with( + api_key_uid=MOCK_API_KEY_UID, + search_rules={ + "studio_content": { + "filter": f"org IN ['org1'] OR access_id IN {expected_access_ids}", + } + }, + expires_at=mock.ANY, + ) + @mock_meilisearch(enabled=True) @mock.patch('openedx.core.djangoapps.content.search.views._get_user_orgs') @mock.patch('openedx.core.djangoapps.content.search.views.get_access_ids_for_request') diff --git a/openedx/core/djangoapps/content/search/views.py b/openedx/core/djangoapps/content/search/views.py index 3a4020d60a8e..5b29a8db1b94 100644 --- a/openedx/core/djangoapps/content/search/views.py +++ b/openedx/core/djangoapps/content/search/views.py @@ -48,7 +48,7 @@ def _get_meili_access_filter(request: Request) -> dict: user_orgs = _get_user_orgs(request)[:MAX_ORGS_IN_FILTER] # ...or the N most recent courses and libraries they can access. - access_ids = get_access_ids_for_request(request)[:MAX_ACCESS_IDS_IN_FILTER] + access_ids = get_access_ids_for_request(request, omit_orgs=user_orgs)[:MAX_ACCESS_IDS_IN_FILTER] return { "filter": f"org IN {user_orgs} OR access_id IN {access_ids}", }