fix: enforce GlobalRestrictedCountry on course access, not just registration

[asadali145]

Description

GlobalRestrictedCountry (added in #36202 / #36398) only blocks account
registration and profile-country changes today — it has no effect on course
access. The only mechanism that enforces course access is RestrictedCourse +
CountryAccessRule, which needs a row per course, so there was no way to
block a country from every course at once.

This PR wires GlobalRestrictedCountry into
embargo.api.check_course_access() so a listed country blocks every course,
with or without a RestrictedCourse entry. Per-course CountryAccessRule
checks still apply on top where configured, staff still bypass every check,
and a per-course disable_access_check override can never bypass a global
block (only a per-course one).

It also surfaces a related enforcement gap discovered while testing this
change: embargo enforcement previously ran only inside EmbargoMiddleware,
which recognizes legacy /course///courses/ URLs. The Learning MFE's
course_home_api endpoints (outline, dates, progress, navigation,
course_metadata) don't match that URL pattern, so a learner in an embargoed
country could browse a course through the MFE with no indication they were
blocked, regardless of RestrictedCourse/GlobalRestrictedCountry
configuration.

This PR closes that gap for the course_metadata endpoint specifically —
the one endpoint the Learning MFE already reads a course_access
(hasAccess/errorCode) flag from to redirect denied learners. A new
check_embargo_access() helper (lms/djangoapps/courseware/access_utils.py)
is called directly from CourseHomeMetadataView
(lms/djangoapps/course_home_api/course_metadata/views.py), so an embargoed
learner's course_access.errorCode comes back as "embargo".

Known, deliberate limitation (not fixed here): the check is not wired
into the shared check_course_access() in
lms/djangoapps/courseware/courses.py that outline/dates/progress/
navigation route through — an earlier version of this PR did that, but it
meant those endpoints started hard-403ing, which crashed the Learning MFE
(it wasn't written to expect a 403 there) and would have required frontend
data-layer changes to fix cleanly. Scoping the check down to course_metadata
only avoids that: those four endpoints still serve real course content to an
embargoed learner today, even though course_metadata correctly reports the
block. A companion Learning MFE PR
(openedx/frontend-app-learning#2055) makes the MFE redirect away from every
tab (including outline, which previously rendered anyway) once it sees
errorCode: "embargo" — but that's a UI-level mitigation, not a data-layer
block. Extending enforcement to those endpoints is left as follow-up work if/
when it's needed.

Behavior changes reviewers should know about

disable_access_check no longer waives a global block. The per-course
RestrictedCourse.disable_access_check escape hatch can override a
CountryAccessRule block, but not a GlobalRestrictedCountry one. This is
reachable whenever a course has a RestrictedCourse row with
disable_access_check=True and the learner is in a globally-restricted
country — that learner was previously let through and is now blocked. It is a
deliberate reduction in per-course operator control: a platform-wide legal
restriction should not be waivable per course.

Staff still bypass embargo blocks. Global and course staff/authors keep
access even from a restricted country. That is long-standing platform
behavior (has_course_author_access in embargo.api), not introduced here —
global blocks simply inherit it. If OFAC compliance requires course staff to
be blocked too, that is a follow-up policy decision rather than part of this
PR.

Error-code precedence in course_metadata. The embargo check runs only
once access is otherwise granted, so a more specific denial keeps its own
error code — an unenrolled learner in a restricted country reports
enrollment_required, not embargo. They are still blocked from enrolling
by the existing enrollment-access-point check, so no gap is introduced, and
already-denied requests skip the embargo check's country lookups.

Default blocked-message URL now keeps its access point.
RestrictedCourse's message-URL fallback previously hardcoded the
courseware access point. A course blocked only by GlobalRestrictedCountry
has no RestrictedCourse row, so it always lands on that fallback — which
meant a learner blocked while enrolling was shown courseware messaging. The
fallback now uses the caller's access point. This changes a pre-existing
code path (covered by the updated
test_message_url_path_no_restrictions_for_course), though it was previously
unreachable in blocked flows: before this PR, only a course with a
RestrictedCourse row could ever be blocked, and such a course always found
its configured message key.

Impact: Operators can now block a country platform-wide via
GlobalRestrictedCountry (Django admin) instead of a row per course.
Learners in a globally-restricted country are blocked from enrollment, the
legacy courseware pages, and (via course_metadata) get a correctly-flagged
Learning MFE that redirects them away. Real course content served by the
Learning MFE's other BFF endpoints is not yet blocked at the data layer (see
above). No change for Course Authors/Developers, and no behavior change for
existing deployments (both tables are empty by default everywhere).

Supporting information

Motivating use case: mitodl/hq#13170 (OFAC embargo requirement).

Companion PR: openedx/frontend-app-learning#2055 (Learning MFE reacts to
errorCode: "embargo" by redirecting every tab to the legacy blocked-message
page).

Testing instructions

1. Enable FEATURES['EMBARGO'] (or confirm settings.EMBARGO is already
   True — it defaults True under lms.envs.tutor.development, False
   in general envs/common.py).
2. Add a country to GlobalRestrictedCountry in Django Admin (no
   RestrictedCourse row needed), or via shell:

   from openedx.core.djangoapps.embargo.models import Country, GlobalRestrictedCountry
   country, _ = Country.objects.get_or_create(country='IR')
   GlobalRestrictedCountry.objects.get_or_create(country=country)

3. Set a test learner's profile country to the same code (easiest way to
   test without IP-header spoofing tooling):

   from django.contrib.auth import get_user_model
   from django.core.cache import cache
   user = get_user_model().objects.get(username='<test-learner>')
   user.profile.country = 'IR'
   user.profile.save()
   cache.clear()

4. Confirm enrollment and the legacy courseware page
   (/courses/<course_id>/course/) are blocked as before (unchanged by this
   PR) — the legacy page redirects to /embargo/blocked-message/courseware/default/.
5. Confirm GET /api/course_home/course_metadata/<course_id> now returns
   course_access: {"has_access": false, "error_code": "embargo", ...} for
   that learner (status is still 200 — course_metadata always reports
   access in-band, same as enrollment_required/other denial reasons).
6. Confirm staff/course-author accounts bypass the block (still has_access: true).
7. Optional, if also running the companion Learning MFE branch: log into the
   MFE as the test learner and confirm every course tab (outline included)
   redirects to the legacy blocked-message page instead of rendering.
8. pytest openedx/core/djangoapps/embargo/ lms/djangoapps/courseware/tests/test_access.py lms/djangoapps/course_home_api/
9. Clean up: remove the GlobalRestrictedCountry row and reset the test
   learner's profile.country.

Deadline

None.

Other information

No migration (reuses the existing GlobalRestrictedCountry table), no
dependency changes, no public API change (check_course_access()'s
signature/return contract is unchanged). See "Known, deliberate limitation"
above for the accepted scope gap (outline/dates/progress/navigation
endpoints still serve real content to an embargoed learner via the Learning
MFE) and its companion frontend PR.

[openedx-webhooks]

Thanks for the pull request, @asadali145!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[Copilot]

🔵 Needs a closer look

It changes core access-control enforcement paths (including caching and request-derived GeoIP behavior), so it warrants a final human review despite only minor issues found.

Pull request overview

This PR extends the Embargo subsystem so GlobalRestrictedCountry is enforced for course access (not just registration/profile country changes), and closes a key enforcement gap for the Learning MFE by ensuring the course_metadata BFF endpoint reports embargo denial via course_access.error_code = "embargo".

Changes:

• Wire GlobalRestrictedCountry into embargo.api.check_course_access() while preserving staff bypass and ensuring per-course disable_access_check cannot override global blocks.
• Add a courseware-layer helper to surface embargo denial as an AccessError and apply it in the course_metadata endpoint’s access computation.
• Expand test coverage for global restriction behavior, staff bypass, redirect behavior, and query/caching expectations.

File summaries

File	Description
openedx/core/djangoapps/embargo/api.py	Implements global-country enforcement in course access checks and exposes a richer internal result for redirect logic.
openedx/core/djangoapps/embargo/tests/test_api.py	Updates/expands tests to cover global restriction, staff bypass, redirect behavior, and revised query counts.
lms/djangoapps/courseware/access_utils.py	Adds check_embargo_access() helper for use by LMS access flows / BFF endpoints.
lms/djangoapps/courseware/access_response.py	Introduces EmbargoAccessError with error_code = "embargo" for in-band API signaling.
lms/djangoapps/course_home_api/course_metadata/views.py	Applies embargo access check to ensure metadata reports embargo denial ahead of other denial reasons.
lms/djangoapps/course_home_api/course_metadata/tests/test_views.py	Adds coverage verifying embargo denial and staff bypass behavior for course metadata endpoint.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Country-based access control is security-sensitive, and the acknowledged data-layer gap across other course-home endpoints requires final human risk acceptance.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[pdpinch]

@asadali145 I'm concerned this PR is doing too much at once -- adding the global block and also changing the API response. What would you think about separating them out?

Since the API change is intended to facilitate a UX change, I think it should be gated on some product review.

[asadali145]

I'm concerned this PR is doing too much at once -- adding the global block and also changing the API response. What would you think about separating them out?

@pdpinch Even if we add a global block, learners would still be able to access courses in learning MFE because there is no gating there, so I think it's best to add both changes in a single PR.

[AhtishamShahid]

It can be replaced with access_checks filter setp

class EmbargoAccessCheck(PipelineStep):
    def run_filter(self, user, course_key):
        request = crum.get_current_request()
        if not embargo_api.check_course_access(course_key, user=user,
                                               ip_addresses=ip.get_all_client_ips(request)):
            raise CoursewareAccessChecksRequested.PreventCoursewareAccess(
                "Embargo", error_code="embargo",
                user_message=_("Access to this course is blocked from your current location"),
            )

This will require an update in env, i.e

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.courseware.access_checks.requested.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx.core.djangoapps.embargo.filters.EnforceCountryEmbargo",
        ],
    },
}

and you can revert changes in lms/djangoapps/courseware/access_response.py and │ lms/.../course_metadata/views.py after this, as they will no longer be required.

[asadali145]

Thanks for the pointer — I looked into it properly. The mechanism does fit: CoursewareAccessChecksRequested fires at exactly the call site I patched, and error_code: "embargo" would still reach the MFE, so access_response.py and the view edit could both go away.

What gave me pause is whether a core, settings.EMBARGO-gated feature should route through the extension framework. docs/concepts/extension_points.rst describes filters as being for plugin developers extending the platform, and there isn't a single non-test PipelineStep in edx-platform today — the platform defines the trigger points and plugins supply the steps. Going the filter route here would also mean embargo enforcement is off unless an operator sets OPEN_EDX_FILTERS_CONFIG, and since that dict gets replaced wholesale by env overrides, an OFAC-style control could be dropped silently. Would you agree that's a bit risky for this particular feature?

One thing I did take from your suggestion though: the filter chain runs after the enrollment check, which is better precedence than what I had. I've reordered the view so the embargo check only runs once access is otherwise granted, so a more specific denial keeps its own error code.

[AhtishamShahid]

Review comments from claude :

1. GeoIP cost, site-wide. geoinfo/api.py's country_code_from_ip opens and closes the MaxMind reader on every call, uncached. Before this change that only ran for courses with a RestrictedCourse row; after it, a single GlobalRestrictedCountry row makes EmbargoMiddleware do a file-open + lookup per IP in the forwarded chain on every /courses/… request, platform-wide. The 21 → 22 test change measures the SQL and misses this entirely. Cheap mitigation: check the (already cached) profile country before walking the IP chain, and/or cache the reader. Probably its own PR, but worth naming before this ships.
2. Staff bypass on a global block. _deny_unless_staff grants access via has_course_author_access, so any course author or global staff sitting in the embargoed country keeps full access. If the driver is genuinely OFAC (mitodl/hq#13170), OFAC doesn't exempt course staff — that feels like it wants a legal answer rather than inheriting the per-course default. Worth noting PriorityAccessFiltersError is documented as non-bypassable by staff, which is another point in the filter's favour.
3. disable_access_check semantics. Making the override unable to waive a global block is defensible, but it's a silent reduction in operator control and currently only lives in a docstring. Worth calling out in the description as a behavior change — it's reachable whenever a course has RestrictedCourse + disable_access_check=True and the learner is in a globally-restricted country.
4. Error-code clobbering. if not embargo_access: load_access = embargo_access discards a more specific denial — an unenrolled learner in an embargoed country reports embargo instead of enrollment_required. Probably intended, but it also means the embargo check runs even when access was already denied.
5. Wrong message page for global enrollment blocks. A global block on a course with no RestrictedCourse row falls through RestrictedCourse._get_message_url_path_from_db (models.py:325) to courseware/default, even at the enrollment access point — so the enrollment-specific messaging is unreachable for global blocks.

[asadali145]

Went through all five — thanks for running these. Items 1, 4 and 5 are addressed in the follow-up commit:

• 1: the check now tests the (cached) profile country against the global list before walking the IP chain, so a globally blocked learner triggers no IP lookups at all.
• 4: the embargo check only runs once access is otherwise granted, so enrollment_required is no longer masked by embargo. The learner is still blocked from enrolling by the existing enrollment-access-point check, so nothing slips through.
• 5: the default blocked-message URL now keeps the caller's access point, so a learner blocked while enrolling gets the enrollment message instead of the courseware one.

On 2, staff bypass is long-standing embargo behavior rather than something this PR introduces — global blocks just inherit it. I've noted it in the description as an open question, since whether OFAC exempts course staff feels like it needs a legal answer rather than a code one. Happy to drop the bypass for global blocks if that's the call.

3 was already covered in the description; I've expanded it into an explicit behavior-change callout, along with the other three above.

[asadali145]

@pdpinch, do you have any thoughts on 2? The current embargo implementation allows access to course staff/ global staff even if they are sitting in an embargoed country, and I haven't changed that.