Skip to content

Commit 2fc3204

Browse files
committed
Merge branch 'master' into chris/FAL-4330-history-log
2 parents 8ba850e + 7694a68 commit 2fc3204

60 files changed

Lines changed: 4929 additions & 1445 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ def test_home_page_libraries_response(self):
329329
'can_edit': True,
330330
'is_migrated': True,
331331
'migrated_to_title': 'Test Library',
332-
'migrated_to_key': 'lib:name0:test-key',
332+
'migrated_to_key': str(self.lib_key_v2),
333333
'migrated_to_collection_key': 'test-collection',
334334
'migrated_to_collection_title': 'Test Collection',
335335
},
@@ -364,7 +364,7 @@ def test_home_page_libraries_response(self):
364364
'can_edit': True,
365365
'is_migrated': True,
366366
'migrated_to_title': 'Test Library',
367-
'migrated_to_key': 'lib:name0:test-key',
367+
'migrated_to_key': str(self.lib_key_v2),
368368
'migrated_to_collection_key': 'test-collection',
369369
'migrated_to_collection_title': 'Test Collection',
370370
}

cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"""
66
import ddt
77
from opaque_keys.edx.keys import UsageKey
8+
from openedx_content.api import signals as content_signals
89
from openedx_events.content_authoring.signals import (
910
LIBRARY_BLOCK_DELETED,
1011
XBLOCK_CREATED,
@@ -405,6 +406,7 @@ class ClipboardPasteFromV2LibraryTestCase(OpenEdxEventsTestMixin, ImmediateOnCom
405406
Test Clipboard Paste functionality with a "new" (as of Sumac) library
406407
"""
407408
ENABLED_OPENEDX_EVENTS = [
409+
content_signals.ENTITIES_DRAFT_CHANGED.event_type, # Required for library events to work
408410
LIBRARY_BLOCK_DELETED.event_type,
409411
XBLOCK_CREATED.event_type,
410412
XBLOCK_DELETED.event_type,
@@ -491,7 +493,8 @@ def test_paste_from_library_read_only_tags(self):
491493
assert object_tag.is_copied
492494

493495
# If we delete the upstream library block...
494-
library_api.delete_library_block(self.lib_block_key)
496+
with self.captureOnCommitCallbacks(execute=True): # make event handlers fire now, within TestCase transaction
497+
library_api.delete_library_block(self.lib_block_key)
495498

496499
# ...the copied tags remain, but should no longer be marked as "copied"
497500
object_tags = tagging_api.get_object_tags(new_block_key)

cms/djangoapps/modulestore_migrator/tasks.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -895,13 +895,9 @@ def _migrate_container(
895895
).publishable_entity_version
896896

897897
# Publish the container
898-
# Call post publish events synchronously to avoid
899-
# an error when calling `wait_for_post_publish_events`
900-
# inside a celery task.
901898
libraries_api.publish_container_changes(
902899
container.container_key,
903900
context.created_by,
904-
call_post_publish_events_sync=True,
905901
)
906902
context.used_container_slugs.add(container.container_key.container_id)
907903
return container_publishable_entity_version, None

cms/envs/common.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,6 @@
256256

257257
MARKETING_EMAILS_OPT_IN = False
258258

259-
############################# MICROFRONTENDS ###################################
260-
COURSE_AUTHORING_MICROFRONTEND_URL = None
261-
262259
############################# SET PATH INFORMATION #############################
263260
PROJECT_ROOT = path(__file__).abspath().dirname().dirname() # /edx-platform/cms
264261
CMS_ROOT = REPO_ROOT / "cms" # noqa: F405

cms/envs/help_tokens.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ video = course_author:references/course_development/guide_to_video.html
2727
certificates = course_author:concepts/open_edx_platform/about_certificates.html
2828
content_highlights = course_author:how-tos/course_development/manage_course_highlight_emails.html
2929
social_sharing = course_author:how-tos/course_development/social_sharing.html
30+
sync_library_updates = course_author:how-tos/course_development/sync_a_library_update_to_your_course.html
3031

3132
# below are the language directory names for the different locales
3233
[locales]

common/djangoapps/third_party_auth/tasks.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616
from common.djangoapps.third_party_auth.models import SAMLConfiguration, SAMLProviderConfig
1717
from common.djangoapps.third_party_auth.utils import (
1818
MetadataParseError,
19+
SAMLMetadataURLError,
1920
create_or_update_bulk_saml_provider_data,
2021
parse_metadata_xml,
22+
validate_saml_metadata_url,
2123
)
2224

2325
log = logging.getLogger(__name__)
@@ -74,10 +76,9 @@ def fetch_saml_metadata():
7476
failure_messages = [] # We return the length of this array for num_failed
7577
for url, entity_ids in url_map.items():
7678
try:
79+
validate_saml_metadata_url(url)
7780
log.info("Fetching %s", url)
78-
if not url.lower().startswith('https'):
79-
log.warning("This SAML metadata URL is not secure! It should use HTTPS. (%s)", url)
80-
response = requests.get(url, verify=True) # May raise HTTPError or SSLError or ConnectionError
81+
response = requests.get(url, verify=True, timeout=30) # May raise HTTPError or SSLError or ConnectionError
8182
response.raise_for_status() # May raise an HTTPError
8283

8384
try:
@@ -96,7 +97,13 @@ def fetch_saml_metadata():
9697
num_updated += 1
9798
else:
9899
log.info(f"→ Updated existing SAMLProviderData. Nothing has changed for entityID {entity_id}")
99-
except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException, MetadataParseError) as error:
100+
except (
101+
exceptions.SSLError,
102+
exceptions.HTTPError,
103+
exceptions.RequestException,
104+
MetadataParseError,
105+
SAMLMetadataURLError,
106+
) as error:
100107
# Catch and process exception in case of errors during fetching and processing saml metadata.
101108
# Here is a description of each exception.
102109
# SSLError is raised in case of errors caused by SSL (e.g. SSL cer verification failure etc.)

common/djangoapps/third_party_auth/tests/test_utils.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,23 @@
66
from unittest.mock import MagicMock
77

88
import ddt
9+
import pytest
10+
from django.test import override_settings
911
from lxml import etree
1012

1113
from common.djangoapps.student.tests.factories import UserFactory
1214
from common.djangoapps.third_party_auth.models import SAMLProviderData
1315
from common.djangoapps.third_party_auth.tests.testutil import TestCase
1416
from common.djangoapps.third_party_auth.utils import (
17+
SAMLMetadataURLError,
1518
create_or_update_bulk_saml_provider_data,
1619
get_associated_user_by_email_response,
1720
get_user_from_email,
1821
is_enterprise_customer_user,
1922
is_oauth_provider,
2023
parse_metadata_xml,
2124
user_exists,
25+
validate_saml_metadata_url,
2226
)
2327
from openedx.core.djangolib.testing.utils import skip_unless_lms
2428
from openedx.features.enterprise_support.tests.factories import (
@@ -272,3 +276,60 @@ def test_multiple_keys(self):
272276
entity_id='http://entity1',
273277
).count()
274278
assert count == 2
279+
280+
281+
@ddt.ddt
282+
@skip_unless_lms
283+
class TestValidateSAMLMetadataURL(TestCase):
284+
"""Tests for validate_saml_metadata_url."""
285+
286+
@ddt.data(
287+
'https://idp.example.com/metadata',
288+
'https://1.1.1.1/metadata',
289+
)
290+
def test_valid_urls_pass(self, url):
291+
validate_saml_metadata_url(url) # should not raise
292+
293+
@ddt.data(
294+
('http://idp.example.com/metadata', 'must use HTTPS'),
295+
('ftp://idp.example.com/metadata', 'must use HTTPS'),
296+
('https://', 'no hostname'),
297+
)
298+
@ddt.unpack
299+
def test_invalid_scheme_or_missing_hostname(self, url, expected_fragment):
300+
with pytest.raises(SAMLMetadataURLError, match=expected_fragment):
301+
validate_saml_metadata_url(url)
302+
303+
@ddt.data(
304+
'https://127.0.0.1/metadata', # IPv4 loopback
305+
'https://[::1]/metadata', # IPv6 loopback
306+
'https://169.254.169.254/latest', # AWS metadata endpoint
307+
'https://169.254.0.1/metadata', # other link-local
308+
'https://[fe80::1]/metadata', # IPv6 link-local
309+
'https://240.0.0.1/metadata', # reserved (Class E)
310+
)
311+
def test_always_blocked_regardless_of_setting(self, url):
312+
for allow_private in (False, True):
313+
with override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=allow_private):
314+
with pytest.raises(SAMLMetadataURLError):
315+
validate_saml_metadata_url(url)
316+
317+
@ddt.data(
318+
'https://10.0.0.1/metadata',
319+
'https://172.16.0.1/metadata',
320+
'https://192.168.1.1/metadata',
321+
'https://[fc00::1]/metadata', # IPv6 unique local
322+
)
323+
def test_private_ip_blocked_by_default(self, url):
324+
with pytest.raises(SAMLMetadataURLError):
325+
validate_saml_metadata_url(url)
326+
327+
@ddt.data(
328+
'https://10.0.0.1/metadata',
329+
'https://172.16.0.1/metadata',
330+
'https://192.168.1.1/metadata',
331+
'https://[fc00::1]/metadata', # IPv6 unique local
332+
)
333+
@override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=True)
334+
def test_private_ip_allowed_when_setting_enabled(self, url):
335+
validate_saml_metadata_url(url) # should not raise

common/djangoapps/third_party_auth/utils.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
"""
44

55
import datetime
6+
import ipaddress
7+
from urllib.parse import urlparse
68
from zoneinfo import ZoneInfo
79

810
import dateutil.parser
11+
from django.conf import settings
912
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
1013
from django.utils.timezone import now
1114
from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser
@@ -27,6 +30,67 @@ class MetadataParseError(Exception):
2730
pass # lint-amnesty, pylint: disable=unnecessary-pass
2831

2932

33+
class SAMLMetadataURLError(Exception):
34+
""" A SAML metadata URL failed security validation """
35+
pass # lint-amnesty, pylint: disable=unnecessary-pass
36+
37+
38+
def validate_saml_metadata_url(url):
39+
"""
40+
Validate that a SAML metadata URL is safe to fetch.
41+
42+
Enforces HTTPS and blocks requests to loopback, link-local, and reserved IP
43+
addresses. Link-local specifically covers cloud instance metadata endpoints
44+
(169.254.0.0/16, e.g. the AWS metadata service at 169.254.169.254).
45+
Reserved addresses (e.g. 240.0.0.0/4) are IETF-assigned ranges that are
46+
never routable on real networks.
47+
48+
Private IP ranges (RFC 1918: 10.x, 172.16.x, 192.168.x) are also blocked by
49+
default, since most Open edX deployments fetch SAML metadata from public IdPs.
50+
Operators running in a private network where the SAML IdP has a private IP can
51+
opt out by setting SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True in Django settings.
52+
53+
Limitation: IP address checks only apply to literal IPs in the URL. Hostname-
54+
based URLs are not validated against the IP blocklists. Operators are encouraged
55+
to complement this with network-level egress filtering that blocks outbound
56+
connections from the Open edX server to link-local (169.254.0.0/16) and RFC
57+
1918 private address ranges.
58+
59+
Raises SAMLMetadataURLError if the URL fails validation.
60+
"""
61+
parsed = urlparse(url)
62+
63+
if parsed.scheme != 'https':
64+
raise SAMLMetadataURLError(
65+
f"SAML metadata URL must use HTTPS, got scheme: {parsed.scheme!r}"
66+
)
67+
68+
hostname = parsed.hostname
69+
if not hostname:
70+
raise SAMLMetadataURLError("SAML metadata URL has no hostname")
71+
72+
try:
73+
addr = ipaddress.ip_address(hostname)
74+
except ValueError:
75+
# hostname is a domain name, not a numeric IP literal — pass through.
76+
return
77+
78+
# Loopback, link-local, and reserved ranges are never legitimate SAML IdP
79+
# addresses regardless of deployment topology.
80+
if addr.is_loopback or addr.is_link_local or addr.is_reserved:
81+
raise SAMLMetadataURLError(
82+
f"SAML metadata URL hostname is a forbidden IP address: {addr}"
83+
)
84+
85+
# Private ranges are blocked by default but can be allowed via Django settings
86+
# for deployments where the SAML IdP lives on the same private network.
87+
if addr.is_private and not settings.SAML_METADATA_URL_ALLOW_PRIVATE_IPS:
88+
raise SAMLMetadataURLError(
89+
f"SAML metadata URL hostname is a private IP address: {addr}. "
90+
"Set SAML_METADATA_URL_ALLOW_PRIVATE_IPS = True in Django settings to allow this."
91+
)
92+
93+
3094
def parse_metadata_xml(xml, entity_id):
3195
"""
3296
Given an XML document containing SAML 2.0 metadata, parse it and return a tuple of

lms/djangoapps/certificates/data.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,12 @@ class CertificateStatuses:
5757
requesting = 'requesting'
5858

5959
readable_statuses = {
60-
downloadable: "already received",
61-
notpassing: "didn't receive",
62-
error: "error states",
63-
audit_passing: "audit passing states",
64-
audit_notpassing: "audit not passing states",
60+
downloadable: "Received",
61+
notpassing: "Not Received",
62+
unavailable: "Invalidated",
63+
error: "Error State",
64+
audit_passing: "Audit - Passing",
65+
audit_notpassing: "Audit - Not Passing",
6566
}
6667

6768
PASSED_STATUSES = (downloadable, generating)

lms/djangoapps/certificates/models.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ def get_certificate_generation_candidates(self):
588588
if not task_input.strip():
589589
# if task input is empty, it means certificates were generated for all learners
590590
# Translators: This string represents task was executed for all learners.
591-
return _("All learners")
591+
return _("All Learners")
592592

593593
task_input_json = json.loads(task_input)
594594

@@ -607,9 +607,9 @@ def get_certificate_generation_candidates(self):
607607
# for backwards compatibility.
608608
if 'student_set' in task_input_json or 'students' in task_input_json:
609609
# Translators: This string represents task was executed for students having exceptions.
610-
return _("For exceptions")
610+
return _("Granted Exceptions")
611611
else:
612-
return _("All learners")
612+
return _("All Learners")
613613

614614
class Meta:
615615
app_label = "certificates"

0 commit comments

Comments
 (0)