chore: sync common Ulmo branch with upstream#840
Merged
Conversation
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>
Fix violations that were introduced when we merged fixes for GHSA-328g-7h4g-r2m9 (cherry picked from commit 6fda1f1)
Agrendalath
force-pushed
the
agrendalath/sync-ulmo-with-upstream
branch
from
513b74f to
93a62a8
Compare
xitij2000
approved these changes
May 4, 2026
Member
xitij2000
left a comment
xitij2000
left a comment
There was a problem hiding this comment.
Looks good! Checked the code and there doesn't seem to be anything that should break an existing ulmo instance.
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
4 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.
This is a clean cherry-pick of upstream Ulmo commits, with pylint fixes cherry-picked from the
masterbranch.Note: do not use anything other than "Create a merge commit" for this PR. And do not use the "Update with*" button. We want to preserve original commit IDs.