LP-592: Block certificate access during proctoring review - #471
Closed
santhosh-apphelix-2u wants to merge 697 commits into
Closed
santhosh-apphelix-2u wants to merge 697 commits into
santhosh-apphelix-2u wants to merge 697 commits into
Conversation
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.
feat: remove enterprise imports from third_party_auth
Commit generated by workflow `edx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/release-ulmo`
…ef2e feat: Upgrade Python dependency edx-enterprise
…268) * fix: update anonymous author labeling for moderator and global staff roles
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
Rework activate_account() so the post-activation destination is no longer sensitive to enterprise membership, and consistently redirects unauthenticated learners to the login page in a way that preserves the `next` URL. Furthermore, this commit expands the cases where the activation CTA cookie is cleared, so that it's *always* cleared after successful activation. Prior to this commit, there were three user stories which seemed wrong and had possibly misleading behavior: 1. Already-logged-in, non-enterprise learners trying to click an "activate account" link containing a `next` URL would get dropped into the learner dashboard despite a `next` URL being specified. 2. Logged-out learners on a platform with the AuthN MFE disabled (still a supported configuration) trying to click an "activate account" link containing a `next` URL would get dropped into the learner dashboard despite a `next` URL being specified. 3. Logged-out learners on a platform with the AuthN MFE enabled may see an activation CTA (reminder message to activate) even after successful activation and login. In user stories 1-2, the `next` URL has been carefully supplied by the client, but subsequently ignored. If the intended behavior is to terminate on the dashboard, clients should NOT specify a `next` query parameter on the activate_account link. In user story 3, the stray activation CTA may seem nagging and annoying since activation was already attempted and succeeded. ENT-11816
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
* feat: add Course Optimizer extended-report proxy views
Studio proxies both kicking off a Course Optimizer extended-analysis run
(CourseAnalysisReportView, generating the course export server-side via the
existing create_export_tarball) and polling its status
(CourseAnalysisReportStatusView) to the xpert-ai-workflows backend, keyed by
a shared secret -- the browser only ever talks to Studio.
* chore(devstack): point Course Optimizer proxy at local xpert-ai-workflows
Reached via Docker's host-gateway alias, since it runs on the host machine
rather than in the devstack docker network.
* feat: add enable_course_optimizer_extended_report waffle flag
Gates the Course Optimizer extended-report section in the authoring MFE,
matching enable_course_optimizer's existing CourseWaffleFlag pattern.
Without this, the frontend's default for this flag would apply
unconditionally everywhere, since it's absent from every course waffle
flags API response until a real flag exists to answer for it.
* refactor: move timeout to settings
* fix: return 404 for invalid/nonexistent course in analysis report status
CourseAnalysisReportStatusView.get parsed course_id without
@verify_course_exists(), so an invalid or unknown course key produced a
500 instead of the documented 404.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: gate course analysis report endpoints behind waffle flag
CourseAnalysisReportView.post and CourseAnalysisReportStatusView.get
triggered/proxied analysis runs regardless of the
enable_course_optimizer_extended_report flag state, so the backend
call happened even when the flag was off. Also fixes a NameError in
the status view from a leftover reference to an undefined
_COURSE_ANALYSIS_REPORT_REQUEST_TIMEOUT_SECONDS constant.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor: update waffle flag name
* refactor: move Course Optimizer extended-analysis views to v1 API
Splits the new xpert-ai-workflows-driven extended report endpoints
(CourseAnalysisReportView, CourseAnalysisReportStatusView) out to v1,
nested under a course_optimizer/analysis/{course_id} prefix, while
leaving the original link-checking/rerun-link-update endpoints on v0.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: guard against malformed JSON from xpert-ai-workflows and allow devstack overrides
Addresses PR review feedback: response.json() on the extended-report
proxy views now returns 502 instead of an unhandled 500 when the
backend returns a non-JSON body, and the devstack URL/API key default
to ENV_TOKENS overrides like other devstack secrets.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
feat: preserve the activation next URL for all learners
feat: hydrate learner state for paginated assessment children
|
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
Implement end-to-end certificate access blocking for learners whose required proctored exams are incomplete, under review, or in a blocking status.
Changes
eligibleas derivednot_attempted.Scope
Certificate generation, certificate requesting, and provider-specific proctoring integrations are unchanged.