Fix incremental assessment state - #464
Closed
santhosh-apphelix-2u wants to merge 692 commits into
Closed
Conversation
…tion feat: add forum telemetry for comment create update and delete
…umentation feat: add forum telemetry for comment vote and unvote
…port (#207) * feat: add SES routing for account activation emails with fallback support * refactor: simplify SES routing and fallback logic per review feedback
NOTE: This is a cherrypick of `30f8ae30793cc6f67ba0ab2786e8742a458929ea` security patch from the release/ulmo branch (GHSA-4xv3-5j4x-q8g4). `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, which enforces HTTPS and blocks loopback, link-local (including cloud metadata endpoints like 169.254.169.254), and reserved IP addresses. RFC 1918 private ranges are blocked by default and can be opted out via `SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True` for deployments where the SAML IdP lives on the same private network. Calls the validator in `fetch_saml_metadata()` before `requests.get()`, also adds a 30s request timeout and removes the previous non-enforcing HTTP warning. Addresses the platform-side fetch path described in: GHSA-328g-7h4g-r2m9 Note: the primary exploit path (`sync_provider_data` endpoint) now lives in edx-enterprise following the migration documented in docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst and will need a corresponding fix there. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous two commits are patches cherry-picked from upstream, but here in the 2U fork of platform it generates weird lint errors we do not understand: ``` openedx/envs/common.py:1588:0: E7670: setting annotation (SAML_METADATA_URL_ALLOW_PRIVATE_IPS) cannot have a boolean value (setting-boolean-default-value) ``` I thought maybe it was related to us running an outdated version of edx-lint (5.6.0 vs 6.0.0 in upstream platform) but upgrading did not fix the errors.
Upstream patch "fix: block SSRF in SAML metadata URL fetching" introduced a long line. How they didn't catch this upstream, I don't know.
fix: block SSRF in SAML metadata URL fetching
April 24 security fixes
…ation feat: Add discussion moderation and restore telemetry
feat: add telemetry for bulk discussion user post workflows
feat: ENT-11771 Option for disabling email during TPA
fix: ENT-11771 SAML login - Allow email editing if emtpy
This PR fixes an issue where the “Load more comments” button was not rendering in discussion threads, preventing users from viewing additional comments beyond the initial set.
This PR fixes an issue where the Pinned post icon” is missing in the My Posts tab of frontend-app-discussion causing the confusion for the users with respect to All post tab.
feat: grant enterprise_openedx_operator admin manage perms Commit generated by workflow `edx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/release-ulmo`
…onfig This refactoring moves all third-party authentication settings from runtime configuration during AppConfig.ready() to static definitions in lms/envs/common.py. ## Problem The third_party_auth app used an `apply_settings()` function called during Django's app initialization to modify Django settings. This pattern caused issues: 1. Settings were modified after Django initialization when they should be finalized, making debugging difficult 2. Operators couldn't override these settings in their YAML config files because apply_settings() would overwrite their values ## Why the ENABLE_THIRD_PARTY_AUTH conditional was removed The settings are now defined unconditionally because: 1. These settings are inert when social auth backends aren't configured in AUTHENTICATION_BACKENDS - they have no effect 2. ENABLE_THIRD_PARTY_AUTH still controls what matters: registration of authentication backends and exposure of auth URLs 3. This follows standard Django patterns where middleware and settings exist but are no-ops when their feature isn't active ## Enterprise pipeline integration The enterprise pipeline step (handle_enterprise_logistration) is included statically in SOCIAL_AUTH_PIPELINE rather than being inserted dynamically. This step handles its own runtime checks and returns early if enterprise is not configured, making it safe to include always. This avoids complexity with Derived() functions that would need to import Django models at settings load time (before apps are ready). ## Operator impact Operators can now properly override any of these settings in their YAML configuration files, including SOCIAL_AUTH_PIPELINE for custom flows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ise pipeline The settings tests were failing when run with CMS settings because the third_party_auth settings (SOCIAL_AUTH_PIPELINE, ExceptionMiddleware, etc.) are now defined only in lms/envs/common.py. Investigation confirmed CMS does not need these settings: - CMS has no social auth URL endpoints (third_party_auth URLs only included in LMS when ENABLE_THIRD_PARTY_AUTH is true) - CMS uses EdxDjangoStrategy for OAuth2 SSO with LMS, not ConfigurationModelStrategy for third-party identity providers - CMS authentication backends are EdXOAuth2 (LMS SSO) and LtiAuthenticationBackend, not social auth backends - The third_party_auth app is only in cms/envs/test.py INSTALLED_APPS to avoid import errors from indirect dependencies (like enterprise) Changes: - Added @skip_unless_lms decorator to SettingsUnitTest class - Added hasattr guard for SOCIAL_AUTH_PIPELINE in apps.py to prevent AttributeError when running under CMS (which doesn't have this setting) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Don't add a new reference to the old FEATURES dictionary and drop an unncessary test. Co-authored-by: Taylor Payne <taylor.payne2@wgu.edu> Co-authored-by: Feanil Patel <feanil@axim.org>
- Remove enterprise pipeline functions and insert_enterprise_pipeline_elements; enterprise pipeline steps are now injected by the enterprise plugin. - Add SAMLAccountDisconnected signal in SAMLAuth.disconnect() to replace the unlink_enterprise_user_from_idp import (moved to the enterprise plugin). - Added an ADR to help explain the migration. ENT-11566 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes an issue where users cannot delete their own posts created using the “Post anonymously” option in Discussions.
…child handling" This reverts commit cf11446.
feat: incrementally load large assessment xblocks
fix: free retired learner's email on retirement completion
feat: add exec ed course uuid to learner home serializer
A retired learner's original email is permanently blocked from re-registration, because is_email_retired() still finds a User row whose email matches the retired-hash of that address. This adds release_retired_learner_email(user), which replaces a fully-retired user's hashed email with a placeholder, clearing that match so the original address becomes reusable. The UserRetirementStatus row and retirement history are left untouched. Exposed via a new release_retired_user_email management command for Operator/Support use. This is manual only, for now.
… [LP-1205] (#461) * chore(video): remove audio description waffle flag and related gating The Audio Description feature is fully rolled out on edx.org, so the contentstore.enable_audio_description CourseWaffleFlag no longer serves a purpose. Remove the flag, its toggle helper, and every place it was consulted: - studio_audio_description XBlock handler no longer returns 404 when the flag is off - Studio editor context always exposes the AD file name and handler URL - LMS player metadata no longer carries audioDescriptionEnabled, and 09_video_audio_description.js no longer gates rendering/binding on it - course_waffle_flags serializer, view docstring and tests drop enable_audio_description - Handler tests drop the flag-off case and the override_waffle_flag decorators Behaviour is unchanged for every environment where the flag was on. Prerequisite for the upstream contribution under LP-911, following the LP-1108 precedent for the transcript editor. * fix(video): drop stale audioDescriptionEnabled metadata expectations The waffle flag removal took audioDescriptionEnabled out of the player metadata, but five expected-metadata dicts in test_video_mongo.py still asserted it. Remove those entries. The separate snake_case audio_description_enabled template-context key means "this video has an AD file" and is intentionally left in place. LP-1205
|
Thank you for your pull request! Congratulations on completing the Open edX tutorial! A team member will be by to take a look shortly. |
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
20 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.
Summary
Fixes CR-8239, where incrementally loaded assessment questions could appear unanswered even though the learner’s attempt state existed in
StudentModule.When the learner submitted the question again, the backend detected the existing attempt and returned:
Root cause
Lazy children were obtained through
parent.get_children()and could already be bound to the parent’s shallow field-data cache. When the endpoint later supplied the correct batch cache,bind_for_student()skipped rebinding because the user ID was unchanged. As a result, the question rendered with default state instead of the learner’s persisted answer and attempt count.Changes