Skip to content

Commit

Permalink
feat: omit access_ids from the search filter that are covered by the …
Browse files Browse the repository at this point in the history
…user's org roles
  • Loading branch information
pomegranited committed Apr 10, 2024
1 parent ef24a88 commit 811687d
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
12 changes: 10 additions & 2 deletions openedx/core/djangoapps/content/search/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down
10 changes: 10 additions & 0 deletions openedx/core/djangoapps/content/search/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions openedx/core/djangoapps/content/search/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/content/search/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
}
Expand Down

0 comments on commit 811687d

Please sign in to comment.