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 093e93dcf302051b2a5c61dd067a320984dcf254 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 | 23 +- .../core/djangoapps/content/search/models.py | 193 +++++++- .../content/search/tests/test_models.py | 455 +++++++++++++++++- .../content/search/tests/test_views.py | 55 +++ 4 files changed, 718 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 6d6ce6148cd2..6281fba9ca6c 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -38,7 +38,12 @@ 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, + authz_has_platform_access, + 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 +1074,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, omit_orgs=list(orgs))) + return list(orgs) def _get_meili_access_filter(request: Request) -> dict: @@ -1084,6 +1096,11 @@ def _get_meili_access_filter(request: Request) -> dict: if GlobalStaff().has_user(request.user): return {} + # An authz platform-wide grant (course-v1:*) is the authz analogue of global + # staff -- it means "all courses across the platform", so no filters required. + if authz_has_platform_access(request.user): + return {} + # Everyone else is limited to their org staff roles... user_orgs = _get_user_orgs(request)[:MAX_ORGS_IN_FILTER] diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index d726f1ead057..ec7ba17b098c 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -2,14 +2,34 @@ from __future__ import annotations -from django.db import models +import logging + +from django.contrib.auth import get_user_model +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, + PlatformCourseOverviewGlobData, +) 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 common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + authz_get_all_course_assignments_for_user, +) +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__) + +User = get_user_model() class SearchAccess(models.Model): # noqa: DJ008 @@ -46,14 +66,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, 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 +95,165 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> ) +@request_cached(arg_map_function=lambda user: user.username) +def _get_cached_authz_course_assignments(user: User): + """ + Returns the user's course-relevant authz role assignments, cached per request. + + Delegates to ``authz_get_all_course_assignments_for_user``, the shared + course-role helper in ``student.roles`` (which also backs the legacy-compat + ``RoleCache`` path), so the set of scope types search cares about -- + ``CourseOverviewData`` (per-course), ``OrgCourseOverviewGlobData`` (org-wide), + and ``PlatformCourseOverviewGlobData`` (platform-wide) -- is defined in one + place rather than re-derived here. That single fetch is shared by all three + search helpers below (per-course keys, org keys, platform access), so each + filters the same materialized set by scope type without a second enforcer + round-trip within one request. + + Keyed on ``user.username`` via ``arg_map_function`` (a ``User`` object's + default repr would otherwise be an unstable cache key), so the helpers share + one 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 authz_get_all_course_assignments_for_user(user) + + +def _get_authz_course_keys(user: User, 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 shared per-request assignment set (see + ``_get_cached_authz_course_assignments``) 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_course_assignments(user) + 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", + user.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(user: User, 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 shared per-request assignment set (see + ``_get_cached_authz_course_assignments``) 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_course_assignments(user) + 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", + user.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 + + +def authz_has_platform_access(user: User) -> bool: + """ + Returns whether the user holds a platform-wide (``course-v1:*``) authz course + role grant that is active under the authz course authoring flag. + + A platform-level grant surfaces as a ``PlatformCourseOverviewGlobData`` scope + and means "all courses across the whole platform" -- the authz analogue of + legacy global staff. Such a user should see everything in search, so the + caller short-circuits the per-course/per-org filter entirely rather than + expanding the grant to one org entry per registered org (which + ``student.roles`` does for the legacy-compat path, but which would blow + ``MAX_ORGS_IN_FILTER`` here). The flag is resolved against the global switch + via ``is_enabled()`` because a platform-wide grant is tied to neither a course + nor an org against which a per-scope override could be evaluated. + + Filters the shared per-request assignment set (see + ``_get_cached_authz_course_assignments``). Fails open on a database error + (logged, returns ``False``) so search degrades to the per-scope + legacy/authz filter rather than 500-ing; any other exception propagates. + """ + try: + assignments = _get_cached_authz_course_assignments(user) + except DatabaseError as exc: + log.warning( + "Could not load authz role assignments for user %r; " + "falling back to per-scope search access. Error: %s", + user.username, + exc, + ) + return False + + if not core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(): + return False + + return any( + isinstance(assignment.scope, PlatformCourseOverviewGlobData) + for assignment in assignments + ) + + 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..1950822b0361 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -1,26 +1,95 @@ """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, + authz_has_platform_access, + 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() + authz_has_platform_access = lambda username: False + +try: + # Real-policy authz test helpers. Guarded for the same reason as above (the + # openedx_authz engine is only wired up where content.search is installed). + from openedx_authz.api.users import assign_role_to_user_in_scope + from openedx_authz.constants.roles import COURSE_EDITOR + from openedx_authz.engine.enforcer import AuthzEnforcer + + from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin + _HAS_AUTHZ_TEST_SUPPORT = True +except (RuntimeError, ImportError): + # Placeholder base when authz test support is unavailable (e.g. under lms, + # where content.search is not installed). Must be an empty *mixin* class, not + # ``object``: the real class lists it BEFORE SharedModuleStoreTestCase, and a + # bare ``object`` in that position is an inconsistent MRO (object must resolve + # last). The class is never instantiated -- the pytest.mark.skipif below skips + # the whole class on this path -- but it must still linearize cleanly so the + # module imports and pylint's MRO check passes. + class CourseAuthoringAuthzTestMixin: # pylint: disable=too-few-public-methods + """No-op placeholder used when openedx_authz test support is unavailable.""" + + assign_role_to_user_in_scope = None + COURSE_EDITOR = None + AuthzEnforcer = None + _HAS_AUTHZ_TEST_SUPPORT = False + + +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 +124,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 +321,380 @@ 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.authz_get_all_course_assignments_for_user' + + 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, 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.authz_get_all_course_assignments_for_user' + + def setUp(self): + super().setUp() + self.authz_org_user = UserFactory.create( + username='authz_org_editor', + email='authz_org_editor@example.com', + is_staff=False, + password='authz_org_editor_pass', + ) + + @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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, omit_orgs=[]) + + +@skip_unless_cms +@pytest.mark.skipif(not _HAS_AUTHZ_TEST_SUPPORT, reason="openedx_authz test support unavailable") +class StudioSearchAuthzRealPolicyTest(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase): + """ + End-to-end authz coverage that seeds REAL role assignments into the enforcer + (via ``assign_role_to_user_in_scope``) instead of mocking + ``get_user_role_assignments``. + + The mocked tests above prove the helper logic in isolation, but they stub the + assignment set -- so they never exercise the real + ``RoleAssignmentData.scope.external_key`` -> ``CourseKey`` contract, nor that + a real per-course / org-glob / platform-glob grant lands in the right scope + class. These tests close that gap by driving the actual openedx-authz storage, + which is the concrete contract search depends on. + + Deliberately self-contained rather than reusing ``StudioSearchTestMixin``: + ``CourseAuthoringAuthzTestMixin`` patches ``AUTHZ_COURSE_AUTHORING_FLAG`` on + for the whole class, which routes the mixin's legacy org-role seeding through + the authz compatibility layer -- an unrelated path this test does not need. + So we build only the minimal course fixture the authz helpers read. The flag + being on for the class is exactly what these happy-path grants want, so no + per-method ``@override_waffle_flag`` is required. + """ + + def setUp(self): + super().setUp() + # Synthetic RequestFactory requests never cross RequestCacheMiddleware, + # so clear the per-request authz cache between tests explicitly. + RequestCache.clear_all_namespaces() + + self.course_keys = [] + for num in range(2): + course_location = self.store.make_course_key('org1', f'AuthzCourse{num}', 'Run') + CourseFactory.create( + org=course_location.org, + number=course_location.course, + run=course_location.run, + ) + course = CourseOverviewFactory.create(id=course_location, org=course_location.org) + SearchAccess.objects.create(context_key=course.id) + self.course_keys.append(course_location) + + 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) + ) + + def _assign_scope(self, user, scope_external_key, role=None): + """Seed a real authz grant for ``user`` at ``scope_external_key`` and reload policy.""" + assign_role_to_user_in_scope( + user.username, + (role or COURSE_EDITOR).external_key, + scope_external_key, + ) + AuthzEnforcer.get_enforcer().load_policy() + + def test_real_per_course_grant_surfaces_course(self): + """ + A real per-course authz grant (``course-v1:Org+C+R``) resolves through + the actual ``scope.external_key`` -> ``CourseKey`` path and yields that + course's access_id -- no legacy role involved. + """ + for course_key in self.course_keys: + self._assign_scope(self.authorized_user, str(course_key)) + + request = RequestFactory().get('/course') + request.user = self.authorized_user + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(self.course_keys) + + def test_real_org_glob_grant_surfaces_org(self): + """A real org-wide grant (``course-v1:Org+*``) resolves to that org short_name.""" + # The org-glob path resolves the flag via ``is_enabled_for_org`` (org + # override, else global switch). The mixin patches ``is_enabled`` on but + # not ``is_enabled_for_org``, so seed a real per-org override to turn the + # org on -- exercising the actual org-tier resolution end to end. + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.name, + org='org1', + override_choice=WaffleFlagOrgOverrideModel.ALL_CHOICES.on, + enabled=True, + ) + self._assign_scope(self.authorized_user, 'course-v1:org1+*') + + orgs = get_authz_org_keys(self.authorized_user, omit_orgs=[]) + + assert orgs == {'org1'} + + def test_real_platform_glob_grant_grants_all_access(self): + """ + A real platform-wide grant (``course-v1:*``) is recognized as "see + everything" -- the authz analogue of global staff (openedx-authz#417 + follow-up: a ``course-v1:*`` user should see all orgs and all courses). + """ + self._assign_scope(self.authorized_user, 'course-v1:*') + + assert authz_has_platform_access(self.authorized_user) is True + + def test_no_grant_has_no_platform_access(self): + """A user with no platform-wide grant is not treated as see-everything.""" + assert authz_has_platform_access(self.unauthorized_user) is False diff --git a/openedx/core/djangoapps/content/search/tests/test_views.py b/openedx/core/djangoapps/content/search/tests/test_views.py index 3d056b2b4327..9c92d1fc2fa3 100644 --- a/openedx/core/djangoapps/content/search/tests/test_views.py +++ b/openedx/core/djangoapps/content/search/tests/test_views.py @@ -205,6 +205,61 @@ 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.authz_has_platform_access') + @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') + def test_studio_search_authz_platform_glob_access(self, mock_search_client, mock_platform_access): + """ + A user with a platform-wide (``course-v1:*``) authz course grant is the + authz analogue of global staff and can search any document, so the + access filter is empty -- no ``org``/``access_id`` restriction clause + (openedx/openedx-authz#417). The student has no legacy access, so the + empty filter here comes purely from the platform-access short-circuit. + """ + mock_platform_access.return_value = True + + 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 + mock_platform_access.assert_called_once() + mock_generate_tenant_token.assert_called_once_with( + api_key_uid=MOCK_API_KEY_UID, + search_rules={ + "studio_content": {} + }, + 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):