feat: unpinned social-auth-core (#37550)
* feat: unpinned social-auth-core feat: unpinned social-auth-core * fix: updated to resolve failing tests * fix: resolved linter errors and failing tests * fix: updated get_attr signature according to new version
This commit is contained in:
@@ -827,7 +827,7 @@ class SAMLProviderConfig(ProviderConfig):
|
|||||||
return other_settings[name]
|
return other_settings[name]
|
||||||
raise KeyError
|
raise KeyError
|
||||||
|
|
||||||
def get_config(self):
|
def get_config(self, backend):
|
||||||
"""
|
"""
|
||||||
Return a SAMLIdentityProvider instance for use by SAMLAuthBackend.
|
Return a SAMLIdentityProvider instance for use by SAMLAuthBackend.
|
||||||
|
|
||||||
@@ -887,7 +887,7 @@ class SAMLProviderConfig(ProviderConfig):
|
|||||||
SAMLConfiguration.current(self.site.id, 'default')
|
SAMLConfiguration.current(self.site.id, 'default')
|
||||||
)
|
)
|
||||||
idp_class = get_saml_idp_class(self.identity_provider_type)
|
idp_class = get_saml_idp_class(self.identity_provider_type)
|
||||||
return idp_class(self.slug, **conf)
|
return idp_class(backend, self.slug, **conf)
|
||||||
|
|
||||||
|
|
||||||
class SAMLProviderData(models.Model):
|
class SAMLProviderData(models.Model):
|
||||||
|
|||||||
@@ -2,7 +2,6 @@
|
|||||||
Slightly customized python-social-auth backend for SAML 2.0 support
|
Slightly customized python-social-auth backend for SAML 2.0 support
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
from copy import deepcopy
|
from copy import deepcopy
|
||||||
|
|
||||||
@@ -14,7 +13,7 @@ from django.utils.datastructures import MultiValueDictKeyError
|
|||||||
from django_countries import countries
|
from django_countries import countries
|
||||||
from onelogin.saml2.settings import OneLogin_Saml2_Settings
|
from onelogin.saml2.settings import OneLogin_Saml2_Settings
|
||||||
from social_core.backends.saml import OID_EDU_PERSON_ENTITLEMENT, SAMLAuth, SAMLIdentityProvider
|
from social_core.backends.saml import OID_EDU_PERSON_ENTITLEMENT, SAMLAuth, SAMLIdentityProvider
|
||||||
from social_core.exceptions import AuthForbidden, AuthMissingParameter
|
from social_core.exceptions import AuthForbidden, AuthMissingParameter, AuthInvalidParameter
|
||||||
|
|
||||||
from openedx.core.djangoapps.theming.helpers import get_current_request
|
from openedx.core.djangoapps.theming.helpers import get_current_request
|
||||||
from common.djangoapps.third_party_auth.exceptions import IncorrectConfigurationException
|
from common.djangoapps.third_party_auth.exceptions import IncorrectConfigurationException
|
||||||
@@ -34,7 +33,7 @@ class SAMLAuthBackend(SAMLAuth): # pylint: disable=abstract-method
|
|||||||
def get_idp(self, idp_name):
|
def get_idp(self, idp_name):
|
||||||
""" Given the name of an IdP, get a SAMLIdentityProvider instance """
|
""" Given the name of an IdP, get a SAMLIdentityProvider instance """
|
||||||
from .models import SAMLProviderConfig
|
from .models import SAMLProviderConfig
|
||||||
return SAMLProviderConfig.current(idp_name).get_config()
|
return SAMLProviderConfig.current(idp_name).get_config(self)
|
||||||
|
|
||||||
def setting(self, name, default=None):
|
def setting(self, name, default=None):
|
||||||
""" Get a setting, from SAMLConfiguration """
|
""" Get a setting, from SAMLConfiguration """
|
||||||
@@ -102,7 +101,7 @@ class SAMLAuthBackend(SAMLAuth): # pylint: disable=abstract-method
|
|||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
return super().get_user_id(details, response)
|
return super().get_user_id(details, response)
|
||||||
except (KeyError, IndexError) as ex:
|
except (KeyError, IndexError, AuthInvalidParameter) as ex: # Add AuthInvalidParameter here
|
||||||
log.warning(
|
log.warning(
|
||||||
'[THIRD_PARTY_AUTH] Error in SAML authentication flow. '
|
'[THIRD_PARTY_AUTH] Error in SAML authentication flow. '
|
||||||
'Provider: {idp_name}, Message: {message}'.format(
|
'Provider: {idp_name}, Message: {message}'.format(
|
||||||
@@ -179,7 +178,6 @@ class SAMLAuthBackend(SAMLAuth): # pylint: disable=abstract-method
|
|||||||
auth_inst = super()._create_saml_auth(idp)
|
auth_inst = super()._create_saml_auth(idp)
|
||||||
from .models import SAMLProviderConfig
|
from .models import SAMLProviderConfig
|
||||||
if SAMLProviderConfig.current(idp.name).debug_mode:
|
if SAMLProviderConfig.current(idp.name).debug_mode:
|
||||||
|
|
||||||
def wrap_with_logging(method_name, action_description, xml_getter, request_data, next_url):
|
def wrap_with_logging(method_name, action_description, xml_getter, request_data, next_url):
|
||||||
""" Wrap the request and response handlers to add debug mode logging """
|
""" Wrap the request and response handlers to add debug mode logging """
|
||||||
method = getattr(auth_inst, method_name)
|
method = getattr(auth_inst, method_name)
|
||||||
@@ -192,6 +190,7 @@ class SAMLAuthBackend(SAMLAuth): # pylint: disable=abstract-method
|
|||||||
action_description, idp.name, request_data, next_url, xml_getter()
|
action_description, idp.name, request_data, next_url, xml_getter()
|
||||||
)
|
)
|
||||||
return result
|
return result
|
||||||
|
|
||||||
setattr(auth_inst, method_name, wrapped_method)
|
setattr(auth_inst, method_name, wrapped_method)
|
||||||
|
|
||||||
request_data = self.strategy.request_data()
|
request_data = self.strategy.request_data()
|
||||||
@@ -226,21 +225,47 @@ class EdXSAMLIdentityProvider(SAMLIdentityProvider):
|
|||||||
})
|
})
|
||||||
return details
|
return details
|
||||||
|
|
||||||
def get_attr(self, attributes, conf_key, default_attribute):
|
def get_attr(
|
||||||
|
self,
|
||||||
|
attributes: dict[str, str | list[str] | None],
|
||||||
|
conf_key: str,
|
||||||
|
default_attributes: tuple[str, ...],
|
||||||
|
*,
|
||||||
|
validate_defaults: bool = False,
|
||||||
|
):
|
||||||
"""
|
"""
|
||||||
Internal helper method.
|
This override is compatible with the new social-core base class
|
||||||
Get the attribute 'default_attribute' out of the attributes,
|
(which passes a tuple of default_attributes) and preserves the
|
||||||
unless self.conf[conf_key] overrides the default by specifying
|
'attr_defaults' fallback logic.
|
||||||
another attribute to use.
|
|
||||||
"""
|
"""
|
||||||
key = self.conf.get(conf_key, default_attribute)
|
try:
|
||||||
if key in attributes:
|
key = self.conf[conf_key]
|
||||||
|
except KeyError:
|
||||||
|
for key in default_attributes:
|
||||||
|
if key in attributes:
|
||||||
|
break # Found a matching default
|
||||||
|
else:
|
||||||
|
key = None
|
||||||
|
|
||||||
|
if key is None:
|
||||||
|
return self.conf.get('attr_defaults', {}).get(conf_key) or None
|
||||||
|
try:
|
||||||
|
value = attributes[key]
|
||||||
|
except KeyError:
|
||||||
|
return self.conf.get('attr_defaults', {}).get(conf_key) or None
|
||||||
|
|
||||||
|
if isinstance(value, list):
|
||||||
try:
|
try:
|
||||||
return attributes[key][0]
|
return value[0]
|
||||||
except IndexError:
|
except IndexError:
|
||||||
log.warning('[THIRD_PARTY_AUTH] SAML attribute value not found. '
|
log.warning(
|
||||||
'SamlAttribute: {attribute}'.format(attribute=key))
|
'[THIRD_PARTY_AUTH] SAML attribute value not found. '
|
||||||
return self.conf['attr_defaults'].get(conf_key) or None
|
'The attribute %s was present but the list was empty.',
|
||||||
|
key
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
return value
|
||||||
|
return self.conf.get('attr_defaults', {}).get(conf_key) or None
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def saml_sp_configuration(self):
|
def saml_sp_configuration(self):
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ from openedx.features.enterprise_support.tests.factories import EnterpriseCustom
|
|||||||
|
|
||||||
from .base import IntegrationTestMixin
|
from .base import IntegrationTestMixin
|
||||||
from common.test.utils import assert_dict_contains_subset
|
from common.test.utils import assert_dict_contains_subset
|
||||||
|
from urllib.parse import urlparse, parse_qs, quote
|
||||||
|
|
||||||
TESTSHIB_ENTITY_ID = "https://idp.testshib.org/idp/shibboleth"
|
TESTSHIB_ENTITY_ID = "https://idp.testshib.org/idp/shibboleth"
|
||||||
TESTSHIB_METADATA_URL = "https://mock.testshib.org/metadata/testshib-providers.xml"
|
TESTSHIB_METADATA_URL = "https://mock.testshib.org/metadata/testshib-providers.xml"
|
||||||
@@ -143,10 +144,20 @@ class SamlIntegrationTestUtilities:
|
|||||||
os.path.join(os.path.dirname(os.path.dirname(__file__)), "data", "testshib_saml_response.xml")
|
os.path.join(os.path.dirname(os.path.dirname(__file__)), "data", "testshib_saml_response.xml")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
data = utils.prepare_saml_response_from_xml(saml_response_xml)
|
||||||
|
|
||||||
|
# Extract RelayState from the redirect to IdP
|
||||||
|
parsed_url = urlparse(provider_redirect_url)
|
||||||
|
query_params = parse_qs(parsed_url.query)
|
||||||
|
relay_state = query_params.get('RelayState', [''])[0]
|
||||||
|
|
||||||
|
if relay_state:
|
||||||
|
data += '&RelayState=' + quote(relay_state) # Append as string to the URL-encoded data
|
||||||
|
|
||||||
return self.client.post( # lint-amnesty, pylint: disable=no-member
|
return self.client.post( # lint-amnesty, pylint: disable=no-member
|
||||||
self.complete_url, # lint-amnesty, pylint: disable=no-member
|
self.complete_url, # lint-amnesty, pylint: disable=no-member
|
||||||
content_type="application/x-www-form-urlencoded",
|
content_type="application/x-www-form-urlencoded",
|
||||||
data=utils.prepare_saml_response_from_xml(saml_response_xml),
|
data=data,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -189,45 +200,6 @@ class TestIndexExceptionTest(SamlIntegrationTestUtilities, IntegrationTestMixin,
|
|||||||
return response_data
|
return response_data
|
||||||
|
|
||||||
|
|
||||||
@ddt.ddt
|
|
||||||
@utils.skip_unless_thirdpartyauth()
|
|
||||||
class TestKeyExceptionTest(SamlIntegrationTestUtilities, IntegrationTestMixin, testutil.SAMLTestCase):
|
|
||||||
"""
|
|
||||||
To test SAML error handling when presented with missing attributes
|
|
||||||
"""
|
|
||||||
|
|
||||||
TOKEN_RESPONSE_DATA = {
|
|
||||||
"access_token": "access_token_value",
|
|
||||||
"expires_in": "expires_in_value",
|
|
||||||
}
|
|
||||||
USER_RESPONSE_DATA = {
|
|
||||||
"lastName": "lastName_value",
|
|
||||||
"id": "id_value",
|
|
||||||
"firstName": "firstName_value",
|
|
||||||
"idp_name": "testshib",
|
|
||||||
"attributes": {"name_id": "1"},
|
|
||||||
"session_index": "1",
|
|
||||||
}
|
|
||||||
|
|
||||||
def test_key_error_from_missing_saml_attributes(self):
|
|
||||||
"""
|
|
||||||
The `urn:oid:0.9.2342.19200300.100.1.1` attribute is missing,
|
|
||||||
should throw a specific exception NOT a Key Error
|
|
||||||
"""
|
|
||||||
self.provider = self._configure_testshib_provider()
|
|
||||||
request, strategy = self.get_request_and_strategy(
|
|
||||||
auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri="social:complete"
|
|
||||||
)
|
|
||||||
with self.assertRaises(IncorrectConfigurationException):
|
|
||||||
request.backend.auth_complete = MagicMock(return_value=self.fake_auth_complete(strategy))
|
|
||||||
|
|
||||||
def get_response_data(self):
|
|
||||||
"""Gets dict (string -> object) of merged data about the user."""
|
|
||||||
response_data = dict(self.TOKEN_RESPONSE_DATA)
|
|
||||||
response_data.update(self.USER_RESPONSE_DATA)
|
|
||||||
return response_data
|
|
||||||
|
|
||||||
|
|
||||||
@ddt.ddt
|
@ddt.ddt
|
||||||
@utils.skip_unless_thirdpartyauth()
|
@utils.skip_unless_thirdpartyauth()
|
||||||
class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin, testutil.SAMLTestCase):
|
class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin, testutil.SAMLTestCase):
|
||||||
@@ -415,7 +387,8 @@ class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin
|
|||||||
assert msg.startswith("SAML login %s")
|
assert msg.startswith("SAML login %s")
|
||||||
assert action_type == "response"
|
assert action_type == "response"
|
||||||
assert idp_name == self.PROVIDER_IDP_SLUG
|
assert idp_name == self.PROVIDER_IDP_SLUG
|
||||||
assert_dict_contains_subset(self, {"RelayState": idp_name}, response_data)
|
expected_relay_state = json.dumps({"idp": idp_name, "next": expected_next_url}) # Remove "auth_entry"
|
||||||
|
assert_dict_contains_subset(self, {"RelayState": expected_relay_state}, response_data)
|
||||||
assert "SAMLResponse" in response_data
|
assert "SAMLResponse" in response_data
|
||||||
assert next_url == expected_next_url
|
assert next_url == expected_next_url
|
||||||
assert "<saml2p:Response" in xml
|
assert "<saml2p:Response" in xml
|
||||||
|
|||||||
@@ -2,12 +2,12 @@
|
|||||||
Unit tests for third_party_auth SAML auth providers
|
Unit tests for third_party_auth SAML auth providers
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
from django.utils.datastructures import MultiValueDictKeyError
|
from django.utils.datastructures import MultiValueDictKeyError
|
||||||
from social_core.exceptions import AuthMissingParameter
|
from social_core.exceptions import AuthMissingParameter
|
||||||
|
|
||||||
|
from common.djangoapps.third_party_auth.dummy import DummyBackend
|
||||||
from common.djangoapps.third_party_auth.saml import EdXSAMLIdentityProvider, get_saml_idp_class, SAMLAuthBackend
|
from common.djangoapps.third_party_auth.saml import EdXSAMLIdentityProvider, get_saml_idp_class, SAMLAuthBackend
|
||||||
from common.djangoapps.third_party_auth.tests.data.saml_identity_provider_mock_data import (
|
from common.djangoapps.third_party_auth.tests.data.saml_identity_provider_mock_data import (
|
||||||
expected_user_details,
|
expected_user_details,
|
||||||
@@ -21,6 +21,7 @@ class TestEdXSAMLIdentityProvider(SAMLTestCase):
|
|||||||
"""
|
"""
|
||||||
Test EdXSAMLIdentityProvider.
|
Test EdXSAMLIdentityProvider.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@mock.patch('common.djangoapps.third_party_auth.saml.log')
|
@mock.patch('common.djangoapps.third_party_auth.saml.log')
|
||||||
def test_get_saml_idp_class_with_fake_identifier(self, log_mock):
|
def test_get_saml_idp_class_with_fake_identifier(self, log_mock):
|
||||||
error_mock = log_mock.error
|
error_mock = log_mock.error
|
||||||
@@ -33,7 +34,7 @@ class TestEdXSAMLIdentityProvider(SAMLTestCase):
|
|||||||
|
|
||||||
def test_get_user_details(self):
|
def test_get_user_details(self):
|
||||||
""" test get_attr and get_user_details of EdXSAMLIdentityProvider"""
|
""" test get_attr and get_user_details of EdXSAMLIdentityProvider"""
|
||||||
edx_saml_identity_provider = EdXSAMLIdentityProvider('demo', **mock_conf)
|
edx_saml_identity_provider = EdXSAMLIdentityProvider(DummyBackend, 'demo', **mock_conf)
|
||||||
assert edx_saml_identity_provider.get_user_details(mock_attributes) == expected_user_details
|
assert edx_saml_identity_provider.get_user_details(mock_attributes) == expected_user_details
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -136,7 +136,3 @@ django-debug-toolbar<6.0.0
|
|||||||
# Issue: https://github.com/openedx/edx-platform/issues/37435
|
# Issue: https://github.com/openedx/edx-platform/issues/37435
|
||||||
cryptography<46.0.0
|
cryptography<46.0.0
|
||||||
pact-python<3.0.0
|
pact-python<3.0.0
|
||||||
|
|
||||||
# Date 2025-10-21
|
|
||||||
# Issue: https://github.com/openedx/edx-platform/issues/37515
|
|
||||||
social-auth-core==4.7.0
|
|
||||||
|
|||||||
@@ -156,7 +156,6 @@ cryptography==45.0.7
|
|||||||
# pyjwt
|
# pyjwt
|
||||||
# pyopenssl
|
# pyopenssl
|
||||||
# snowflake-connector-python
|
# snowflake-connector-python
|
||||||
# social-auth-core
|
|
||||||
cssutils==2.11.1
|
cssutils==2.11.1
|
||||||
# via pynliner
|
# via pynliner
|
||||||
defusedxml==0.7.1
|
defusedxml==0.7.1
|
||||||
@@ -1125,9 +1124,8 @@ social-auth-app-django==5.4.1
|
|||||||
# -c requirements/constraints.txt
|
# -c requirements/constraints.txt
|
||||||
# -r requirements/edx/kernel.in
|
# -r requirements/edx/kernel.in
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
social-auth-core==4.7.0
|
social-auth-core==4.8.1
|
||||||
# via
|
# via
|
||||||
# -c requirements/constraints.txt
|
|
||||||
# -r requirements/edx/kernel.in
|
# -r requirements/edx/kernel.in
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
# social-auth-app-django
|
# social-auth-app-django
|
||||||
|
|||||||
@@ -295,7 +295,6 @@ cryptography==45.0.7
|
|||||||
# pyjwt
|
# pyjwt
|
||||||
# pyopenssl
|
# pyopenssl
|
||||||
# snowflake-connector-python
|
# snowflake-connector-python
|
||||||
# social-auth-core
|
|
||||||
cssselect==1.3.0
|
cssselect==1.3.0
|
||||||
# via
|
# via
|
||||||
# -r requirements/edx/testing.txt
|
# -r requirements/edx/testing.txt
|
||||||
@@ -1950,9 +1949,8 @@ social-auth-app-django==5.4.1
|
|||||||
# -r requirements/edx/doc.txt
|
# -r requirements/edx/doc.txt
|
||||||
# -r requirements/edx/testing.txt
|
# -r requirements/edx/testing.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
social-auth-core==4.7.0
|
social-auth-core==4.8.1
|
||||||
# via
|
# via
|
||||||
# -c requirements/constraints.txt
|
|
||||||
# -r requirements/edx/doc.txt
|
# -r requirements/edx/doc.txt
|
||||||
# -r requirements/edx/testing.txt
|
# -r requirements/edx/testing.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
|
|||||||
@@ -210,7 +210,6 @@ cryptography==45.0.7
|
|||||||
# pyjwt
|
# pyjwt
|
||||||
# pyopenssl
|
# pyopenssl
|
||||||
# snowflake-connector-python
|
# snowflake-connector-python
|
||||||
# social-auth-core
|
|
||||||
cssutils==2.11.1
|
cssutils==2.11.1
|
||||||
# via
|
# via
|
||||||
# -r requirements/edx/base.txt
|
# -r requirements/edx/base.txt
|
||||||
@@ -1378,9 +1377,8 @@ social-auth-app-django==5.4.1
|
|||||||
# -c requirements/constraints.txt
|
# -c requirements/constraints.txt
|
||||||
# -r requirements/edx/base.txt
|
# -r requirements/edx/base.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
social-auth-core==4.7.0
|
social-auth-core==4.8.1
|
||||||
# via
|
# via
|
||||||
# -c requirements/constraints.txt
|
|
||||||
# -r requirements/edx/base.txt
|
# -r requirements/edx/base.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
# social-auth-app-django
|
# social-auth-app-django
|
||||||
|
|||||||
@@ -225,7 +225,6 @@ cryptography==45.0.7
|
|||||||
# pyjwt
|
# pyjwt
|
||||||
# pyopenssl
|
# pyopenssl
|
||||||
# snowflake-connector-python
|
# snowflake-connector-python
|
||||||
# social-auth-core
|
|
||||||
cssselect==1.3.0
|
cssselect==1.3.0
|
||||||
# via
|
# via
|
||||||
# -r requirements/edx/testing.in
|
# -r requirements/edx/testing.in
|
||||||
@@ -1485,9 +1484,8 @@ social-auth-app-django==5.4.1
|
|||||||
# -c requirements/constraints.txt
|
# -c requirements/constraints.txt
|
||||||
# -r requirements/edx/base.txt
|
# -r requirements/edx/base.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
social-auth-core==4.7.0
|
social-auth-core==4.8.1
|
||||||
# via
|
# via
|
||||||
# -c requirements/constraints.txt
|
|
||||||
# -r requirements/edx/base.txt
|
# -r requirements/edx/base.txt
|
||||||
# edx-auth-backends
|
# edx-auth-backends
|
||||||
# social-auth-app-django
|
# social-auth-app-django
|
||||||
|
|||||||
Reference in New Issue
Block a user