From a6cb38b5b45440683af853e11e3e6eee743d50d2 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Fri, 11 Sep 2026 10:31:39 -0600 Subject: [PATCH 1/2] feat: add org-tier resolution to CourseWaffleFlag CourseWaffleFlag could only resolve a flag for a course key (is_enabled), which walks course override -> org override -> global switch. There was no public way to ask whether a flag is enabled for an entire org, independent of any single course. Add a public is_enabled_for_org(org) that resolves the flag at the org tier only: an org override (force-on / force-off) takes precedence, otherwise the global switch. Per-course overrides are not consulted -- a setting made for one course cannot answer whether the flag is on for the whole org. Extract the org-override read that was inline in _get_course_override_value into a shared _get_org_override_value(org) helper so both entry points resolve org overrides identically and share one cached_flags() entry per org. --- .../core/djangoapps/waffle_utils/__init__.py | 61 +++++++++++++++---- .../waffle_utils/tests/test_init.py | 36 +++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 95fa360d5e84..52df524a1234 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -62,7 +62,7 @@ def _get_course_override_value(self, course_key): course_key (CourseKey): The course to check for override before checking waffle. """ # Import is placed here to avoid model import at project startup. - from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel + from .models import WaffleFlagCourseOverrideModel course_cache_key = f"{self.name}.cwaffle.{str(course_key)}" course_override = self.cached_flags().get(course_cache_key) @@ -80,20 +80,34 @@ def _get_course_override_value(self, course_key): # Since no course-specific override was found, fall back to checking at the org-level. if course_key: - org = course_key.org - org_cache_key = f"{self.name}.owaffle.{org}" - org_override = self.cached_flags().get(org_cache_key) + return self._get_org_override_value(course_key.org) - if org_override is None: - org_override = WaffleFlagOrgOverrideModel.override_value( - self.name, org - ) - self.cached_flags()[org_cache_key] = org_override + return None + + def _get_org_override_value(self, org): + """ + Check whether the flag was overridden for an entire org. + + Returns True/False if the flag was forced on or off for the provided org. + Returns None if the flag was not overridden at the org level. + + Arguments: + org (str): The org short_name to check for an override. + """ + # Import is placed here to avoid model import at project startup. + from .models import WaffleFlagOrgOverrideModel + + org_cache_key = f"{self.name}.owaffle.{org}" + org_override = self.cached_flags().get(org_cache_key) + + if org_override is None: + org_override = WaffleFlagOrgOverrideModel.override_value(self.name, org) + self.cached_flags()[org_cache_key] = org_override - if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.on: - return True - if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.off: - return False + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.on: + return True + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.off: + return False return None @@ -120,3 +134,24 @@ def is_enabled(self, course_key=None): # pylint: disable=arguments-differ # act like a normal waffle flag. We currently don't support library-specific overrides. assert isinstance(course_key, LearningContextKey), "expected a course key or other learning context key" return super().is_enabled() + + def is_enabled_for_org(self, org): + """ + Returns whether the flag is enabled for an entire org. + + Resolves the flag at the org tier: an org override (force-on / force-off) + takes precedence, otherwise falls back to the global waffle switch. Unlike + :meth:`is_enabled`, this takes an org short_name rather than a course key, + for grants that are not tied to a single course (e.g. an org-wide role). + + A result here does not guarantee the flag's state for any specific course + in the org -- that course may have its own override. Use :meth:`is_enabled` + to check a specific course. + + Arguments: + org (str): The org short_name to check. + """ + org_override = self._get_org_override_value(org) + if org_override is not None: + return org_override + return super().is_enabled() diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index 57f31767373a..00ad1d4c7782 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -225,3 +225,39 @@ def test_without_request_and_everyone_active_waffle(self): test_course_flag = CourseWaffleFlag(self.NAMESPACED_FLAG_NAME, __name__) with override_waffle_flag(self.TEST_COURSE_FLAG, active=True): assert test_course_flag.is_enabled(self.TEST_COURSE_KEY) is True + + @ddt.data( + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + ) + @ddt.unpack + def test_is_enabled_for_org(self, waffle_enabled, org_override_choice, is_enabled): + """ + Tests is_enabled_for_org: an org override (on/off) takes precedence, otherwise + the base waffle switch decides. Takes an org short_name, not a course key. + + on = active (enabled) + off = inactive (disabled) + unset = mirror the base waffle flag's activity + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + org=self.TEST_ORG, + override_choice=org_override_choice, + note='', + enabled=True + ) + with override_waffle_flag(self.TEST_COURSE_FLAG, active=waffle_enabled): + assert self.TEST_COURSE_FLAG.is_enabled_for_org(self.TEST_ORG) == is_enabled + + def test_is_enabled_for_org_no_override_uses_global_switch(self): + """ + With no org override at all, is_enabled_for_org falls back to the global switch. + """ + with override_waffle_flag(self.TEST_COURSE_FLAG, active=True): + assert self.TEST_COURSE_FLAG.is_enabled_for_org("SomeUnoverriddenOrg") is True + assert self.TEST_COURSE_FLAG.is_enabled_for_org("SomeUnoverriddenOrg") is False From aef74efe7f3b9ecf90b0ce5fcc69304501b5bd54 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Fri, 11 Sep 2026 10:31:55 -0600 Subject: [PATCH 2/2] feat: include authz course + org role grants in Meilisearch access filter get_access_ids_for_request built the Meilisearch tenant-token filter from legacy CourseStaffRole/CourseInstructorRole only, so users holding an authz-only course role (course_editor / course_auditor with no legacy twin) were excluded from the search access filter. Their courses returned zero results in the global Studio search modal and the Library Updates "Review Content Updates" tab. This is openedx-authz#417. Add _get_authz_course_keys to union the user's per-course authz role assignments (CourseOverviewData scope) into the access_id filter clause, gated per-course on AUTHZ_COURSE_AUTHORING_FLAG so global-off deployments with per-course/org overrides still resolve correctly. Also handle org-wide (glob) authz grants, e.g. course-v1:Org+*, which surface as an OrgCourseOverviewGlobData scope rather than a per-course scope and so were missed by both filter clauses -- the same #417 gap for the org case. Add get_authz_org_keys to resolve org-glob grants to org short_names and union them into _get_user_orgs, landing them in the org IN [...] clause (one entry per org, mirroring legacy org staff roles and avoiding per-course access_id fan-out against the JWT size cap). Each org is gated via CourseWaffleFlag.is_enabled_for_org, since a per-course override cannot gate an org-wide grant. A single per-request-cached fetch (_get_cached_authz_assignments) backs both helpers so the enforcer is queried once per request rather than twice. The authz lookups fail open on a DatabaseError (logged, empty set) so search degrades to legacy access rather than 500-ing; any other error propagates. Scope parsing is narrowed to InvalidKeyError so a non-course authz scope (e.g. a library) is skipped rather than mishandled. Closes: openedx/openedx-authz#417 --- openedx/core/djangoapps/content/search/api.py | 17 +- .../core/djangoapps/content/search/models.py | 135 +++++++- .../content/search/tests/test_models.py | 314 +++++++++++++++++- .../content/search/tests/test_views.py | 29 ++ 4 files changed, 488 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 6d6ce6148cd2..dbd71fa6a947 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -38,7 +38,11 @@ INDEX_SEARCHABLE_ATTRIBUTES, INDEX_SORTABLE_ATTRIBUTES, ) -from openedx.core.djangoapps.content.search.models import IncrementalIndexCompleted, get_access_ids_for_request +from openedx.core.djangoapps.content.search.models import ( + IncrementalIndexCompleted, + get_access_ids_for_request, + get_authz_org_keys, +) from openedx.core.djangoapps.content_libraries import api as lib_api from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -1069,11 +1073,18 @@ def _get_user_orgs(request: Request) -> list[str]: Get the org.short_names for the organizations that the requesting user has OrgStaffRole or OrgInstructorRole. Note: org-level roles have course_id=None to distinguish them from course-level roles. + + Also includes orgs where the user holds an org-wide (glob) authz course role, so that + authz-only users granted at the org level (e.g. ``course-v1:Org+*``) are covered by the + ``org IN [...]`` search filter clause rather than being dropped. """ course_roles = get_course_roles(request.user) - return list( - set(role.org for role in course_roles if role.course_id is None and role.role in ["staff", "instructor"]) + orgs = set( + role.org for role in course_roles if role.course_id is None and role.role in ["staff", "instructor"] ) + # Union in org-level authz grants (flag-gated per org inside the helper). + orgs.update(get_authz_org_keys(request.user.username, omit_orgs=list(orgs))) + return list(orgs) def _get_meili_access_filter(request: Request) -> dict: diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index d726f1ead057..ac9edcbb22bd 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -2,14 +2,24 @@ from __future__ import annotations -from django.db import models +import logging + +from django.db import DatabaseError, models from django.utils.translation import gettext_lazy as _ +from opaque_keys import InvalidKeyError from opaque_keys.edx.django.models import LearningContextKeyField +from opaque_keys.edx.keys import CourseKey +from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData +from openedx_authz.api.users import get_user_role_assignments from rest_framework.request import Request from common.djangoapps.student.role_helpers import get_course_roles from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole +from openedx.core import toggles as core_toggles from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user +from openedx.core.lib.cache_utils import request_cached + +log = logging.getLogger(__name__) class SearchAccess(models.Model): # noqa: DJ008 @@ -46,14 +56,20 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> omit_orgs = omit_orgs or [] course_roles = get_course_roles(request.user) - course_clause = models.Q(context_key__in=[ + course_keys = set( role.course_id for role in course_roles if ( role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] and role.org not in omit_orgs ) - ]) + ) + + # When authz is enabled, also include courses where the user has an authz role assignment. + # This ensures authz-only users (editor/auditor without legacy roles) can search their courses. + course_keys.update(_get_authz_course_keys(request.user.username, omit_orgs)) + + course_clause = models.Q(context_key__in=list(course_keys)) libraries = get_libraries_for_user(user=request.user) library_clause = models.Q(context_key__in=[ @@ -69,6 +85,119 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> ) +@request_cached() +def _get_cached_authz_assignments(username: str): + """ + Returns the user's full authz role-assignment set, cached per request. + + ``get_user_role_assignments_per_scope_type`` fetches the user's entire + assignment set from the enforcer regardless of the scope types requested, + then filters in Python. Both search helpers below need a different slice of + that same set (per-course vs org-glob scopes), so caching the single + underlying whole-set fetch lets them each filter by scope type without + paying for a second enforcer round-trip within one request. + + Keyed on ``username`` only (the sole argument), so the two helpers share the + cache entry. Not wrapped in a try/except here: callers own the fail-open + decision, and ``request_cached`` never caches a raised exception, so a + transient ``DatabaseError`` on the first call is re-attempted on the second. + """ + return get_user_role_assignments(user_external_key=username) + + +def _get_authz_course_keys(username: str, omit_orgs: list[str]) -> set[str]: + """ + Returns serialized course keys from the user's authz role assignments where + the authz course authoring flag is enabled. + + Filters the per-request-cached whole assignment set to the per-course + (``CourseOverviewData``) scopes, then to only courses where the flag is + active (supporting both global enablement and per-course overrides). + + Keys are returned as strings (not ``CourseKey`` objects) to match the legacy + ``get_course_roles`` branch, whose ``course_id`` is already a string. This + keeps the unioned ``course_keys`` set type-homogeneous so a course held via + both a legacy and an authz role de-duplicates to a single entry. + + Fails open: if the authz lookup hits a database error, it is logged and an + empty set is returned so that search degrades to legacy-role access rather + than returning a 500. Any other (unexpected) exception propagates. + """ + try: + assignments = _get_cached_authz_assignments(username) + except DatabaseError as exc: + log.warning( + "Could not load authz role assignments for user %r; " + "falling back to legacy course roles for search access. Error: %s", + username, + exc, + ) + return set() + + course_keys = set() + for assignment in assignments: + if not isinstance(assignment.scope, CourseOverviewData): + continue + try: + course_key = CourseKey.from_string(assignment.scope.external_key) + except InvalidKeyError: + # A non-course scope (e.g. a library) can legitimately appear here; skip it. + continue + if course_key.org not in omit_orgs and core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(course_key): + # Store the serialized form to match the legacy branch's string + # course_id, so the unioned set de-duplicates across both paths. + course_keys.add(str(course_key)) + return course_keys + + +def get_authz_org_keys(username: str, omit_orgs: list[str]) -> set[str]: + """ + Returns org short_names from the user's org-level (glob) authz course role + assignments where the authz course authoring flag is enabled for that org. + + An authz role can be granted at an org-wide scope (e.g. ``course-v1:Org+*``), + which surfaces as an ``OrgCourseOverviewGlobData`` scope rather than a + per-course ``CourseOverviewData`` scope. Such a grant means "all courses in + this org", so it belongs in the Meilisearch ``org IN [...]`` clause -- one + entry per org rather than fanning out to every course id (which would burn + ``MAX_ACCESS_IDS_IN_FILTER`` slots) and mirrors how legacy org staff roles + are handled. + + Filters the per-request-cached whole assignment set to the org-glob scopes, + so this and ``_get_authz_course_keys`` share a single enforcer fetch within + one request. The flag is resolved at the org tier via + ``CourseWaffleFlag.is_enabled_for_org`` (org override takes precedence, else + the global switch), since an org-wide grant is not tied to a single course. + + Fails open on a database error (logged, returns an empty set) so search + degrades to legacy access rather than 500-ing; any other exception + propagates. Orgs already covered by ``omit_orgs`` are skipped. + """ + try: + assignments = _get_cached_authz_assignments(username) + except DatabaseError as exc: + log.warning( + "Could not load authz org role assignments for user %r; " + "falling back to legacy roles for search access. Error: %s", + username, + exc, + ) + return set() + + org_keys = set() + for assignment in assignments: + if not isinstance(assignment.scope, OrgCourseOverviewGlobData): + continue + org = assignment.scope.org + if ( + org + and org not in omit_orgs + and core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled_for_org(org) + ): + org_keys.add(org) + return org_keys + + class IncrementalIndexCompleted(models.Model): # noqa: DJ008 """ Stores the contex keys of aleady indexed courses and libraries for incremental indexing. diff --git a/openedx/core/djangoapps/content/search/tests/test_models.py b/openedx/core/djangoapps/content/search/tests/test_models.py index ef42a1c04879..0498528daa58 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -1,26 +1,68 @@ """Content search model tests""" from __future__ import annotations +from unittest import mock + import ddt +import pytest +from django.db import OperationalError from django.test import RequestFactory from django.utils.crypto import get_random_string +from edx_django_utils.cache import RequestCache +from edx_toggles.toggles.testutils import override_waffle_flag from organizations.models import Organization from common.djangoapps.student.auth import update_org_role from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, OrgInstructorRole, OrgStaffRole from common.djangoapps.student.tests.factories import UserFactory +from openedx.core import toggles as core_toggles from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.waffle_utils.models import WaffleFlagOrgOverrideModel from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory try: # This import errors in the lms because content.search is not an installed app there. - from openedx.core.djangoapps.content.search.models import SearchAccess, get_access_ids_for_request + from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData + + from openedx.core.djangoapps.content.search.models import ( + SearchAccess, + _get_authz_course_keys, + get_access_ids_for_request, + get_authz_org_keys, + ) except RuntimeError: SearchAccess = {} + CourseOverviewData = OrgCourseOverviewGlobData = None + _get_authz_course_keys = lambda username, omit_orgs: set() get_access_ids_for_request = lambda request: [] + get_authz_org_keys = lambda username, omit_orgs: set() + + +def _fake_authz_assignment(course_key): + """ + Build a stand-in for openedx_authz's RoleAssignmentData whose scope is a + real ``CourseOverviewData`` (so it passes the ``isinstance`` scope-type + filter in ``_get_authz_course_keys``) exposing the ``external_key`` the + helper reads. + + We stub the assignment rather than provision real authz role rows because + the model helper only cares about the scope's type and external key; the + shape of the authz storage is exercised by openedx-authz's own test suite. + """ + return mock.Mock(scope=CourseOverviewData(external_key=str(course_key))) + + +def _fake_authz_org_glob_assignment(org): + """ + Build a stand-in for openedx_authz's RoleAssignmentData whose scope is a + real org-wide (glob) ``OrgCourseOverviewGlobData`` (so it passes the + ``isinstance`` scope-type filter in ``get_authz_org_keys``), exposing the + ``org`` property the helper reads (e.g. ``course-v1:Org+*`` -> ``'Org'``). + """ + return mock.Mock(scope=OrgCourseOverviewGlobData(external_key=f'course-v1:{org}+*')) class StudioSearchTestMixin: @@ -55,6 +97,13 @@ def setUp(self): """ super().setUp() + # The authz assignment fetch is memoized per request via + # @request_cached. These tests exercise the helpers with synthetic + # RequestFactory requests that never cross RequestCacheMiddleware (which + # normally clears the cache at request boundaries), so clear it here to + # keep each test's mocked fetch from leaking into the next. + RequestCache.clear_all_namespaces() + self.course_user_keys = [] self.staff_user_keys = [] @@ -245,3 +294,266 @@ def test_no_access_ids_for_request(self): request.user = self.student access_ids = get_access_ids_for_request(request) assert not access_ids + + +@ddt.ddt +@skip_unless_cms +class StudioSearchAuthzAccessTest(StudioSearchTestMixin, SharedModuleStoreTestCase): + """ + Tests that ``get_access_ids_for_request`` includes courses granted through + openedx-authz role assignments, not just legacy CourseStaffRole / + CourseInstructorRole (openedx/openedx-authz#417). + """ + + AUTHZ_PATH = 'openedx.core.djangoapps.content.search.models.get_user_role_assignments' + + def _create_course(self, course_location): + """Create a SearchAccess row per course so access_ids can resolve.""" + course = super()._create_course(course_location) + SearchAccess.objects.create(context_key=course.id) + return course + + def _create_library(self, org, num): + """Create a SearchAccess row per library so access_ids can resolve.""" + library = super()._create_library(org, num) + SearchAccess.objects.create(context_key=library.key) + return library + + def _authz_only_user(self): + """A user with no legacy course role — access can only come from authz.""" + return UserFactory.create( + username='authz_editor', + email='authz_editor@example.com', + is_staff=False, + password='authz_editor_pass', + ) + + def _course_access_ids(self, course_keys): + """Resolve the SearchAccess ids for the given course keys.""" + return set( + SearchAccess.objects.filter(context_key__in=course_keys).values_list('id', flat=True) + ) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_only_user_sees_authz_courses(self): + """ + A user with an authz role assignment but no legacy role gets the + matching course access_ids when the flag is enabled. + """ + user = self._authz_only_user() + granted = self.course_user_keys[:2] # first two are CourseKeys (libraries come later) + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(granted) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_courses_respect_omit_orgs(self): + """Authz-granted courses in an omitted org are excluded.""" + user = self._authz_only_user() + granted = self.course_user_keys[:2] + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request, omit_orgs=['Org']) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_authz_courses_excluded_when_flag_off(self): + """With the flag disabled, authz assignments contribute no access_ids.""" + user = self._authz_only_user() + granted = self.course_user_keys[:2] + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_db_failure_is_swallowed(self): + """ + A database failure in the authz query must not break search; the + request falls back to legacy roles only (here: none, so no access_ids). + """ + user = self._authz_only_user() + request = RequestFactory().get('/course') + request.user = user + + with mock.patch(self.AUTHZ_PATH, side_effect=OperationalError('authz db down')): + access_ids = get_access_ids_for_request(request) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_unexpected_error_propagates(self): + """ + Only known operational (DatabaseError) failures are swallowed. An + unexpected error is NOT masked — it propagates so real bugs surface. + """ + user = self._authz_only_user() + request = RequestFactory().get('/course') + request.user = user + + with mock.patch(self.AUTHZ_PATH, side_effect=RuntimeError('unexpected')): + with pytest.raises(RuntimeError): + get_access_ids_for_request(request) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_ignores_unparseable_scope(self): + """A scope external_key that is not a course key is skipped, not fatal.""" + user = self._authz_only_user() + granted = self.course_user_keys[:1] + request = RequestFactory().get('/course') + request.user = user + assignments = [ + _fake_authz_assignment(granted[0]), + _fake_authz_assignment('not-a-course-key'), + ] + + with mock.patch(self.AUTHZ_PATH, return_value=assignments): + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(granted) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_union_with_legacy_roles_no_duplicates(self): + """ + When a course is granted through BOTH a legacy role and authz, its + access_id appears exactly once. + """ + request = RequestFactory().get('/course') + request.user = self.course_staff # already has legacy CourseStaffRole on course_user_keys + legacy_courses = self.course_user_keys[:2] + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in legacy_courses], + ): + access_ids = get_access_ids_for_request(request) + + assert len(access_ids) == len(set(access_ids)) + assert self._course_access_ids(legacy_courses).issubset(set(access_ids)) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_course_keys_are_strings_matching_legacy(self): + """ + _get_authz_course_keys returns serialized (str) keys, matching the + legacy get_course_roles branch (whose course_id is a str). This keeps + the unioned course_keys set type-homogeneous: a course held via both a + legacy role and an authz role collapses to ONE set member rather than + two (a str and a CourseKey object hash/compare unequal). The downstream + SQL ``IN`` coerces both forms, so the query works either way -- this + guards the set-union de-dup invariant itself, at the source. + """ + user = self._authz_only_user() + granted = self.course_user_keys[:1] + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + keys = _get_authz_course_keys(user.username, omit_orgs=[]) + + assert keys == {str(key) for key in granted} + assert all(isinstance(key, str) for key in keys) + + +@ddt.ddt +@skip_unless_cms +class StudioSearchAuthzOrgAccessTest(StudioSearchTestMixin, SharedModuleStoreTestCase): + """ + Tests that ``get_authz_org_keys`` surfaces org-wide (glob) authz course + role assignments -- e.g. a ``course_editor`` granted at ``course-v1:Org+*`` + -- so that org-level authz-only users are covered by the ``org IN [...]`` + search filter clause (openedx/openedx-authz#417). + + An org glob returns an ``OrgCourseOverviewGlobData`` scope, which the + per-course path (``get_access_ids_for_request`` -> ``CourseOverviewData``) + deliberately skips; it must be resolved to an org short_name here instead. + """ + + AUTHZ_PATH = 'openedx.core.djangoapps.content.search.models.get_user_role_assignments' + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_grant_returns_org(self): + """An org-wide authz grant yields that org's short_name when the flag is on.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == {'org1'} + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_respects_omit_orgs(self): + """An org already covered by the legacy org clause is not duplicated.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=['org1']) + + assert orgs == set() + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_org_glob_excluded_when_flag_off(self): + """With the flag globally off (and no org override), org globs contribute nothing.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == set() + + def test_org_glob_enabled_by_org_override_when_flag_off_globally(self): + """ + A per-org waffle override forces the org on even when the global flag is + off -- the same resolution CourseWaffleFlag applies at the org tier. + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.name, + org='org1', + override_choice=WaffleFlagOrgOverrideModel.ALL_CHOICES.on, + enabled=True, + ) + with override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False): + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == {'org1'} + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_db_failure_is_swallowed(self): + """A DatabaseError in the authz org query fails open (empty set, no raise).""" + with mock.patch(self.AUTHZ_PATH, side_effect=OperationalError('authz db down')): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == set() + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_unexpected_error_propagates(self): + """An unexpected error is not masked -- it propagates so real bugs surface.""" + with mock.patch(self.AUTHZ_PATH, side_effect=RuntimeError('unexpected')): + with pytest.raises(RuntimeError): + get_authz_org_keys('authz_org_editor', omit_orgs=[]) diff --git a/openedx/core/djangoapps/content/search/tests/test_views.py b/openedx/core/djangoapps/content/search/tests/test_views.py index 3d056b2b4327..3a726363728d 100644 --- a/openedx/core/djangoapps/content/search/tests/test_views.py +++ b/openedx/core/djangoapps/content/search/tests/test_views.py @@ -205,6 +205,35 @@ def test_studio_search_org_access(self, username, mock_search_client): expires_at=ANY, ) + @mock_meilisearch(enabled=True) + @patch('openedx.core.djangoapps.content.search.api.get_authz_org_keys') + @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') + def test_studio_search_authz_org_glob_access(self, mock_search_client, mock_authz_orgs): + """ + A user with only an org-wide (glob) authz course grant -- and no legacy + role -- is covered by the org clause (openedx/openedx-authz#417). The + student has no legacy access, so 'org1' here comes purely from the authz + org union wired into ``_get_user_orgs``. + """ + mock_authz_orgs.return_value = {'org1'} + + self.client.login(username='student', password='student_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 + # The authz org union is fed into _get_user_orgs, which is passed as omit_orgs + # to the access_ids query, so org1's courses are covered by the org clause. + mock_authz_orgs.assert_called_once() + mock_generate_tenant_token.assert_called_once_with( + api_key_uid=MOCK_API_KEY_UID, + search_rules={ + "studio_content": { + "filter": "org IN ['org1'] OR access_id IN []", + } + }, + expires_at=ANY, + ) + @mock_meilisearch(enabled=True) @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') def test_studio_search_omit_orgs(self, mock_search_client):