Repository navigation
Conversation
chore(logging): add additional logs for LTI launch flow
This update is required for the Catalog MFE to function, as the new `unstable/v0/course_list_search/` endpoint was released in `4.4.0`.
django security fixes Commit generated by workflow `openedx/openedx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master`
…penedx#37991) (openedx#37992) We hope this will fix an error where loremipsum is using pkg_resources, which setuptools dropped support for as of v82 (released yesterday).
…i-logging chore(logging): add additional logs for LTI launch flow (backport)
The activation_key field was exposed in /api/user/v1/accounts/{username},
allowing an attacker to bypass email verification by combining two behaviors:
1. OAuth2 password grant issues tokens to inactive users (intentional)
2. activation_key returned in API response (the vulnerability)
An attacker could register, get an OAuth2 token, read the activation_key
from the API, then GET /activate/{key} to activate without email access.
Fix: remove activation_key from UserReadOnlySerializer.to_representation()
and from ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] (which controls
the field whitelist in _filter_fields — listed fields default to None even
if absent from the serializer data dict).
Reported by Daniel Baillo via the Open edX security working group.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* build: Fix the docs build. Our use of `fs` means we have a runtime dependency on setuptools. The newest version is not compatible with the current version of `fs` so we've added a constraint. But the docs build didn't previously install our specified version of setuptools. This is to fix that. Co-authored-by: Sarina Canelake <sarina@axim.org>
The view_survey endpoint accepted a redirect_url GET parameter and passed
it directly to HttpResponseRedirect() with no validation. If a non-existent
survey name was requested, this produced an immediate 302 to an
attacker-controlled URL. If a valid survey was requested, the same URL was
embedded in a hidden _redirect_url form field; after submission, submit_answers
echoed it back in JSON and client-side JS used it as location.href — a second
unvalidated redirect path.
Fix both by ignoring user-supplied redirect URLs entirely:
- view_survey no longer reads redirect_url from GET params
- submit_answers always redirects to reverse('dashboard') rather than
reading _redirect_url from the POST body
Note: view_student_survey retains its redirect_url parameter because it is
also called from the courseware view (courseware/views/views.py), which passes
a server-controlled course_home_url. That call path is unaffected.
Fixes: GHSA-2843-x998-f8r2
BREAKING CHANGE: The redirect_url GET parameter on /survey/<name>/ is no longer
honored. Requests that previously redirected to a caller-specified URL after
survey completion will now always redirect to the dashboard.
Backports: 42e4e0d Likely fixes: openedx#38027 Co-authored-by: María Fernanda Magallanes <35668326+MaferMazu@users.noreply.github.com>
`clean_thread_html_body()` was missing `<style>` from its tag denylist, allowing arbitrary CSS to survive sanitization and be rendered via the `|safe` filter in email templates. This enabled CSS-based email tracking (IP disclosure via background-image/import), content spoofing, and phishing via pseudo-elements. Uses `decompose()` rather than `unwrap()` so the CSS text content is also removed, not just the tag wrapper. Ref: GHSA-4xv3-5j4x-q8g4 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds `validate_saml_metadata_url()` to `third_party_auth/utils.py` which enforces HTTPS and blocks requests to loopback, link-local, and reserved addresses unconditionally, and RFC 1918 private ranges by default (configurable via `SAML_METADATA_URL_ALLOW_PRIVATE_IPS`). The validator is called in three places where user-supplied metadata URLs are fetched: `fetch_metadata_xml()` in utils.py, the inline fetch in `tasks.py::fetch_saml_metadata`, and `sync_provider_data` in the SAMLProviderDataViewSet. A 30-second timeout is also added to all `requests.get()` calls that were previously unbounded. Fixes GHSA-328g-7h4g-r2m9. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This reverts commit 3a8fdad.
The set_course_mode_price view had no authorization check beyond @login_required, meaning any authenticated user could POST to it and rewrite the honor-mode price for any course — a privilege escalation vulnerability. Ideally this endpoint would be removed: it has no known callers in the UI (no templates or JS reference it), no tests, and targets the legacy 'honor' mode. However, it is publicly routed and external consumers may depend on it, so removal requires going through the DEPR process before we can act. In the meantime, this commit closes the security hole regardless of how active the endpoint is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This reverts commit 84f4afb. This is to prevent conflicts when we bring in the upstream version. It should be reapplied.
validate_timestamp_and_nonce previously returned True unconditionally, allowing any captured LTI launch request to be replayed indefinitely. Now rejects requests whose oauth_timestamp falls outside a ±5-minute window, then atomically records the nonce in the Django cache via cache.add() (OEP-0022 key generation via get_cache_key). A replay returns False immediately because cache.add() only writes when the key is absent. TieredCache is intentionally not used here: it has no atomic add primitive, so a separate get-then-set would leave a race window that defeats the protection. See the updated docstring for details. Documents the requirement for a shared cache backend (Redis or Memcached) in multi-node deployments in both the app and repo READMEs. Fixes GHSA-6gm5-c49g-p3h9 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add missing class docstring to TimestampAndNonceValidatorTest (C0115) - Move time.time patch from class decorator into setUp/addCleanup to eliminate unused mock parameters in every test method (PT019) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The setting annotation used setting_name/setting_default which triggers E7670 (setting-boolean-default-value) for boolean defaults. This was fixed on master using toggle_name/toggle_default annotations (which allow boolean values) but was not backported to ulmo. Fixing here alongside the other style issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps the `openedx-forum` pin from 0.3.8 to 0.4.1 on `release/ulmo` to pull in the fix for the "pinned"-NULL sort bug (openedx/forum#270, commit 78b36e4) where discussion threads were not ordering correctly when users selected "recent first" — old threads with NULL `pinned` values floated above newer threads. Reported in: https://discuss.openedx.org/t/discuss-forum-messages-order-not-organized-as-expected-in-teak/18665 The 0.3.8 -> 0.4.1 delta brings in: * 0.3.9 — five small bug fixes (Mongo content_type, timestamps, MySQL read_states lookup, Forum V2 author field, build). * 0.4.0 — MySQL backend query optimizations (N+1 fixes, prefetch/select_related); backwards compatible. * 0.4.1 — the `pinned` NULL sort fix (commit 78b36e4) plus the optional Typesense search backend (additive, off by default). Caps openedx-forum at <=0.4.1 in constraints.txt to avoid: * 0.4.2 — drops Python 3.11 support. * 0.4.3 — removes the MongoDB backend (Ulmo deployments may rely on it). The constraint should be removed on master / post-Ulmo release lines. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint fetched
every client-supplied files[].url server-side with
requests.get(url, allow_redirects=True) and returned the bytes inside the
ZIP response. Because the URLs were never validated, an authenticated user
with studio read access could point them at internal services or cloud
metadata endpoints and exfiltrate the responses (GHSA-fpf9-9rpr-jvrx).
By design these URLs are always a subset of the course's own VAL
encoded_videos[].url values (the same data the video listing hands the
frontend). Restrict fetches to that allowlist: build the set of legitimate
URLs for the course and reject any request containing a URL outside it
before any HTTP request is made. This eliminates the SSRF rather than
merely narrowing it.
Adds VideoDownloadViewTest (the endpoint previously had no test coverage)
covering the allowed-URL success path, rejection of disallowed URLs without
any outbound request, mixed allowed/disallowed batches, and the non-staff
permission gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Change the `i18n` service declaration from `wants` to `needs`, since the runtime must provide it for the block to function correctly. - Update the `public_view` webpack JS reference from `VideoBlockMain` to `VideoBlockDisplay`, as all VideoBlock JS files are bundled into `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything or no longer exists in the repository.
…kMain-to-VideoBlockDisplay-ulmo.3 chore: optimize/correct the VideoBlock code (openedx#38012)
The fork's #215 (8fb361b) capped the video language selection menu height. Its CSS part merged into VideoBlockDisplay.css, but its JS part was on xmodule/js/src/video/09_video_caption.js, which upstream ulmo moved to xmodule/assets/video/public/js/. Apply it to the live file. Co-Authored-By: Claude <noreply@anthropic.com>
Upstream removed studio-frontend (28ab2ce), and this branch no longer depends on it, but the merge kept the studio-frontend source in `pull_translations`. Remove it; `$(ATLAS_EXTRA_SOURCES)` stays as the continuation line. Co-Authored-By: Claude <noreply@anthropic.com>
Adds docs/plans/, a home for temporary in-flight plans, with a README setting the lifecycle rule (a plan is deleted when the work it describes is done), and two LP-1148 documents: - LP-1148-ulmo4-upgrade.md: context, decisions, current state of the upgrade branch robrap/lp1148-ulmo4-continue, and next steps. - LP-1148-ulmo4-merge-audit.md: the merge audit of that branch, with the scripts used, so it can be re-run after re-syncing. Plans are Markdown, which the Sphinx build ignores; the README is :orphan: so it needs no toctree entry. Co-Authored-By: Claude <noreply@anthropic.com>
|
Thank you for your pull request! Congratulations on completing the Open edX tutorial! A team member will be by to take a look shortly. |
The plan and merge audit now live in the upgrade PR (#505) rather than a separate docs PR. Record that, mark the draft-PR step done, add the final merge and clean-up steps, and allow plans to ship with the work they describe in the docs/plans README. Also fix a broken section link. Co-Authored-By: Claude <noreply@anthropic.com>
Commit generated by workflow `edx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/release-ulmo`
Co-Authored-By: Claude <noreply@anthropic.com>
…elist openedx-learning 0.30.2 adds this model, so pii_check coverage fell to 99.8% and failed Quality checks. release-ulmo lists it only under the old openedx_content label. Co-Authored-By: Claude <noreply@anthropic.com>
Author
|
[inform] This PR is unrelated to the Open edX tutorial. Please disregard. |
The upstream-added owners (@farhan @irtazaakram @salman2013) are Open edX community accounts, not owners in this fork. Restore the fork's line. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Brings in #503 (log user ID instead of PII when SQUELCH_PII_IN_LOGS is enabled). Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…1148 plan Record what running the edx-internal upgrade playbook against this branch found: stage/prod migration state to check (ulmo.1 was deployed and rolled back in May), the nine breaking commits in the range, the contentstore 0014 rollback caveat, verified pre-flight items, and a Feed-to-playbook list that must be resolved before these docs are deleted. Co-Authored-By: Claude <noreply@anthropic.com>
…endix Co-Authored-By: Claude <noreply@anthropic.com>
… plan Co-Authored-By: Claude <noreply@anthropic.com>
vgulati-apphelix
pushed a commit
that referenced
this pull request
Oct 7, 2026
check-for-tutorial-prs.yml posts an Open edX community welcome comment on any PR touching lms/templates/dashboard.html. It does not apply to this fork and adds noise (e.g. on the ulmo.4+ upgrade PR #505). Co-authored-by: Claude <noreply@anthropic.com>
The plan, merge audit and docs/plans README now live only in #514 (branch robrap/lp1148-ulmo1-batch1). This PR is superseded and kept as a reference branch. Co-Authored-By: Claude <noreply@anthropic.com>
robrap
changed the title
Author
|
Closing as superseded by #514 (batch 1 = ulmo.1, with the plan docs). This branch stays as a reference only; do not merge or deploy it. |
robrap
added a commit
that referenced
this pull request
Oct 8, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUPERSEDED. Reference only; do not merge or deploy this branch. It is kept as a second opinion for conflict resolutions, ports and the audit, and will be closed.
The upgrade is now landed in batches. The plan, decisions and next steps live in #514 (
docs/plans/LP-1148-ulmo-upgrade.md). Batch 1 is ulmo.1 only, the only point that has run in prod; later batches merge the upstream ulmo tags straight intorelease-ulmo.Why this branch is not the path: it is built on
edx/edx/ulmo.3(never landed, not proven) with a single ulmo.4+ merge on top. See #514 for the reasoning.The docs that used to be in this PR were moved to #514. The last version of them on this branch is at
be17bd60caif needed for reference.Ticket: LP-1148 (epic LP-1309).
🤖 Generated with Claude Code