chore: sync common Ulmo branch with upstream

.readthedocs.yaml

@@ -10,6 +10,10 @@ sphinx:
 
 python:
   install:
+    # Need to install this to set the correct version of setuptools for now
+    # because it is needed by fs
+    # See https://github.com/openedx/openedx-platform/issues/38068 for details.
+  - requirements: "requirements/pip.txt"
   - requirements: "requirements/edx/doc.txt"
   - method: pip
     path: .

common/djangoapps/third_party_auth/samlproviderdata/views.py

@@ -15,6 +15,7 @@
 from rest_framework.response import Response
 
 from common.djangoapps.third_party_auth.utils import (
+    SAMLMetadataURLError,
     convert_saml_slug_provider_id,
     create_or_update_bulk_saml_provider_data,
     fetch_metadata_xml,
@@ -121,7 +122,7 @@ def sync_provider_data(self, request):
             # part 1: fetch information from remote metadata based on metadataUrl in samlproviderconfig
             try:
                 xml = fetch_metadata_xml(metadata_url)
-            except (SSLError, MissingSchema, HTTPError) as ex:
+            except (SSLError, MissingSchema, HTTPError, SAMLMetadataURLError) as ex:
                 msg = f'Could not verify provider metadata url. Exc type: {type(ex).__name__}'
                 log.warning(msg)
                 return Response(msg, status.HTTP_406_NOT_ACCEPTABLE)

common/djangoapps/third_party_auth/tasks.py

@@ -16,8 +16,10 @@
 from common.djangoapps.third_party_auth.models import SAMLConfiguration, SAMLProviderConfig
 from common.djangoapps.third_party_auth.utils import (
     MetadataParseError,
+    SAMLMetadataURLError,
     create_or_update_bulk_saml_provider_data,
     parse_metadata_xml,
+    validate_saml_metadata_url,
 )
 
 log = logging.getLogger(__name__)
@@ -74,10 +76,9 @@ def fetch_saml_metadata():
     failure_messages = []  # We return the length of this array for num_failed
     for url, entity_ids in url_map.items():
         try:
+            validate_saml_metadata_url(url)
             log.info("Fetching %s", url)
-            if not url.lower().startswith('https'):
-                log.warning("This SAML metadata URL is not secure! It should use HTTPS. (%s)", url)
-            response = requests.get(url, verify=True)  # May raise HTTPError or SSLError or ConnectionError
+            response = requests.get(url, verify=True, timeout=30)  # May raise HTTPError or SSLError or ConnectionError
             response.raise_for_status()  # May raise an HTTPError
 
             try:
@@ -96,13 +97,20 @@ def fetch_saml_metadata():
                     num_updated += 1
                 else:
                     log.info(f"→ Updated existing SAMLProviderData. Nothing has changed for entityID {entity_id}")
-        except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException, MetadataParseError) as error:
+        except (
+            exceptions.SSLError,
+            exceptions.HTTPError,
+            exceptions.RequestException,
+            MetadataParseError,
+            SAMLMetadataURLError,
+        ) as error:
             # Catch and process exception in case of errors during fetching and processing saml metadata.
             # Here is a description of each exception.
             # SSLError is raised in case of errors caused by SSL (e.g. SSL cer verification failure etc.)
             # HTTPError is raised in case of unexpected status code (e.g. 500 error etc.)
             # RequestException is the base exception for any request related error that "requests" lib raises.
             # MetadataParseError is raised if there is error in the fetched meta data (e.g. missing @entityID etc.)
+            # SAMLMetadataURLError is raised if the URL fails security validation.
 
             log.exception(str(error))
             failure_messages.append(

common/djangoapps/third_party_auth/tests/test_utils.py

@@ -6,18 +6,22 @@
 from unittest.mock import MagicMock
 
 import ddt
+import pytest
+from django.test import override_settings
 from lxml import etree
 
 from common.djangoapps.student.tests.factories import UserFactory
 from common.djangoapps.third_party_auth.tests.testutil import TestCase
 from common.djangoapps.third_party_auth.utils import (
+    SAMLMetadataURLError,
+    convert_saml_slug_provider_id,
     get_associated_user_by_email_response,
     get_user_from_email,
     is_enterprise_customer_user,
     is_oauth_provider,
     parse_metadata_xml,
     user_exists,
-    convert_saml_slug_provider_id,
+    validate_saml_metadata_url,
 )
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from openedx.features.enterprise_support.tests.factories import (
@@ -216,3 +220,63 @@ def test_parse_metadata_with_use_attribute_missing(self):
         public_keys, sso_url, _ = parse_metadata_xml(xml, entity_id)
         assert public_keys == ['abc+hkIuUktxkg=']
         assert sso_url == 'https://idp/SSOService.php'
+
+
+@ddt.ddt
+class TestValidateSAMLMetadataURL(TestCase):
+    """
+    Tests for validate_saml_metadata_url — the SSRF-prevention validator.
+    """
+
+    @ddt.data(
+        'https://idp.example.com/metadata',
+        'https://1.1.1.1/metadata',
+    )
+    def test_valid_urls_pass(self, url):
+        # Should not raise
+        validate_saml_metadata_url(url)
+
+    @ddt.data(
+        ('http://idp.example.com/metadata', 'must use HTTPS'),
+        ('ftp://idp.example.com/metadata', 'must use HTTPS'),
+        ('https://', 'no hostname'),
+    )
+    @ddt.unpack
+    def test_invalid_scheme_or_missing_hostname(self, url, match):
+        with pytest.raises(SAMLMetadataURLError, match=match):
+            validate_saml_metadata_url(url)
+
+    @ddt.data(
+        # Loopback
+        ('https://127.0.0.1/metadata', False),
+        ('https://127.0.0.1/metadata', True),
+        # Link-local (includes cloud metadata endpoints like 169.254.169.254)
+        ('https://169.254.169.254/metadata', False),
+        ('https://169.254.169.254/metadata', True),
+    )
+    @ddt.unpack
+    def test_always_blocked_regardless_of_setting(self, url, allow_private):
+        with override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=allow_private):
+            with pytest.raises(SAMLMetadataURLError, match='blocked address'):
+                validate_saml_metadata_url(url)
+
+    @ddt.data(
+        'https://10.0.0.1/metadata',
+        'https://172.16.0.1/metadata',
+        'https://192.168.1.1/metadata',
+        'https://[fc00::1]/metadata',
+    )
+    @override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=False)
+    def test_private_ip_blocked_by_default(self, url):
+        with pytest.raises(SAMLMetadataURLError, match='private address'):
+            validate_saml_metadata_url(url)
+
+    @ddt.data(
+        'https://10.0.0.1/metadata',
+        'https://172.16.0.1/metadata',
+        'https://192.168.1.1/metadata',
+    )
+    @override_settings(SAML_METADATA_URL_ALLOW_PRIVATE_IPS=True)
+    def test_private_ip_allowed_with_setting(self, url):
+        # Should not raise when private IPs are explicitly allowed
+        validate_saml_metadata_url(url)

common/djangoapps/third_party_auth/utils.py

@@ -3,12 +3,15 @@
 """
 
 import datetime
+import ipaddress
 import logging
+from urllib.parse import urlparse
 from uuid import UUID
 
 import dateutil.parser
 import pytz
 import requests
+from django.conf import settings
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from django.utils.timezone import now
 from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser
@@ -36,16 +39,60 @@ class MetadataParseError(Exception):
     pass  # lint-amnesty, pylint: disable=unnecessary-pass
 
 
+class SAMLMetadataURLError(Exception):
+    """ The SAML metadata URL failed security validation """
+    pass  # lint-amnesty, pylint: disable=unnecessary-pass
+
+
+def validate_saml_metadata_url(url):
+    """
+    Validate that a SAML metadata URL is safe to fetch.
+
+    Enforces HTTPS and blocks requests to loopback, link-local, and reserved
+    IP addresses. RFC 1918 private ranges are blocked by default but can be
+    allowed via SAML_METADATA_URL_ALLOW_PRIVATE_IPS for deployments where the
+    IdP lives on the same private network as the Open edX server.
+
+    Note: validation is IP-based and only applies when the URL contains a
+    literal IP address. Hostname-based URLs are not resolved here — operators
+    should enforce network-level egress filtering (e.g. firewall rules or a
+    dedicated egress proxy) as a complementary control to guard against
+    DNS-based bypasses.
+
+    Raises SAMLMetadataURLError if the URL fails any check.
+    """
+    parsed = urlparse(url)
+    if parsed.scheme != 'https':
+        raise SAMLMetadataURLError(f"SAML metadata URL must use HTTPS, got: {parsed.scheme!r}")
+    if not parsed.hostname:
+        raise SAMLMetadataURLError("SAML metadata URL has no hostname")
+
+    try:
+        addr = ipaddress.ip_address(parsed.hostname)
+    except ValueError:
+        # Not a literal IP — hostname-based, allow it through
+        return
+
+    if addr.is_loopback or addr.is_link_local or addr.is_reserved:
+        raise SAMLMetadataURLError(f"SAML metadata URL resolves to a blocked address: {addr}")
+
+    allow_private = getattr(settings, 'SAML_METADATA_URL_ALLOW_PRIVATE_IPS', False)
+    if not allow_private and addr.is_private:
+        raise SAMLMetadataURLError(
+            f"SAML metadata URL resolves to a private address: {addr}. "
+            "Set SAML_METADATA_URL_ALLOW_PRIVATE_IPS=True to allow this."
+        )
+
+
 def fetch_metadata_xml(url):
     """
     Fetches IDP metadata from provider url
     Returns: xml document
     """
+    validate_saml_metadata_url(url)
     try:
         log.info("Fetching %s", url)
-        if not url.lower().startswith('https'):
-            log.warning("This SAML metadata URL is not secure! It should use HTTPS. (%s)", url)
-        response = requests.get(url, verify=True)  # May raise HTTPError or SSLError or ConnectionError
+        response = requests.get(url, verify=True, timeout=30)  # May raise HTTPError or SSLError or ConnectionError
         response.raise_for_status()  # May raise an HTTPError
 
         try:
@@ -55,13 +102,15 @@ def fetch_metadata_xml(url):
             raise
         # TODO: Can use OneLogin_Saml2_Utils to validate signed XML if anyone is using that
         return xml
-    except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException, MetadataParseError) as error:
+    except (exceptions.SSLError, exceptions.HTTPError, exceptions.RequestException,
+            MetadataParseError, SAMLMetadataURLError) as error:
         # Catch and process exception in case of errors during fetching and processing saml metadata.
         # Here is a description of each exception.
         # SSLError is raised in case of errors caused by SSL (e.g. SSL cer verification failure etc.)
         # HTTPError is raised in case of unexpected status code (e.g. 500 error etc.)
         # RequestException is the base exception for any request related error that "requests" lib raises.
         # MetadataParseError is raised if there is error in the fetched meta data (e.g. missing @entityID etc.)
+        # SAMLMetadataURLError is raised if the URL fails security validation.
         log.exception(str(error), exc_info=error)
         raise error
     except etree.XMLSyntaxError as error: