diff --git a/ChangeLog.rst b/ChangeLog.rst index 7bc97a1e13..abc2b838d9 100644 --- a/ChangeLog.rst +++ b/ChangeLog.rst @@ -24,6 +24,10 @@ Backwards incompatible changes ``foo`` uses ``/accounts/oidc/foo/login/`` as its login URL. Set it to empty (``""``) to keep the previous URL structure (``/accounts/foo/login/``). +- The SAML default attribute mapping for ``uid`` has been changed to only + include ``urn:oasis:names:tc:SAML:attribute:subject-id``. If the SAML response + does not contain that, it will fallback to use ``NameID``. + 0.59.0 (2023-12-13) ******************* diff --git a/allauth/socialaccount/adapter.py b/allauth/socialaccount/adapter.py index f3d6e849b7..a00c6ecc37 100644 --- a/allauth/socialaccount/adapter.py +++ b/allauth/socialaccount/adapter.py @@ -312,6 +312,50 @@ def get_requests_session(self): ) return session + def is_email_verified(self, provider, email): + """ + Returns ``True`` iff the given email encountered during a social + login for the given provider is to be assumed verified. + + This can be configured with a ``"verified_email"`` key in the provider + app settings, or a ``"VERIFIED_EMAIL"`` in the global provider settings + (``SOCIALACCOUNT_PROVIDERS``). Both can be set to ``False`` or + ``True``, or, a list of domains to match email addresses against. + """ + verified_email = None + if provider.app: + verified_email = provider.app.settings.get("verified_email") + if verified_email is None: + settings = provider.get_settings() + verified_email = settings.get("VERIFIED_EMAIL", False) + if isinstance(verified_email, bool): + pass + elif isinstance(verified_email, list): + email_domain = email.partition("@")[2].lower() + verified_domains = [d.lower() for d in verified_email] + verified_email = email_domain in verified_domains + else: + raise ImproperlyConfigured("verified_email wrongly configured") + return verified_email + + def can_authenticate_by_email(self, login, email): + """ + Returns ``True`` iff authentication by email is active for this login/email. + + This can be configured with a ``"email_authentication"`` key in the provider + app settings, or a ``"VERIFIED_EMAIL"`` in the global provider settings + (``SOCIALACCOUNT_PROVIDERS``). + """ + ret = None + provider = login.account.get_provider() + if provider.app: + ret = provider.app.settings.get("email_authentication") + if ret is None: + ret = app_settings.EMAIL_AUTHENTICATION or provider.get_settings().get( + "EMAIL_AUTHENTICATION", False + ) + return ret + def get_adapter(request=None): return import_attribute(app_settings.ADAPTER)(request) diff --git a/allauth/socialaccount/models.py b/allauth/socialaccount/models.py index ef72ad6eb8..92aecdcfa0 100644 --- a/allauth/socialaccount/models.py +++ b/allauth/socialaccount/models.py @@ -279,11 +279,7 @@ def lookup(self): points, if any. """ if not self._lookup_by_socialaccount(): - provider_id = self.account.get_provider().id - if app_settings.EMAIL_AUTHENTICATION or app_settings.PROVIDERS.get( - provider_id, {} - ).get("EMAIL_AUTHENTICATION", False): - self._lookup_by_email() + self._lookup_by_email() def _lookup_by_socialaccount(self): assert not self.is_existing @@ -324,6 +320,8 @@ def _lookup_by_socialaccount(self): def _lookup_by_email(self): emails = [e.email for e in self.email_addresses if e.verified] for email in emails: + if not get_adapter().can_authenticate_by_email(self, email): + continue users = filter_users_by_email(email, prefer_verified=True) if users: self.user = users[0] diff --git a/allauth/socialaccount/providers/base/provider.py b/allauth/socialaccount/providers/base/provider.py index ef2e48a356..f0437755a2 100644 --- a/allauth/socialaccount/providers/base/provider.py +++ b/allauth/socialaccount/providers/base/provider.py @@ -1,6 +1,7 @@ from django.core.exceptions import ImproperlyConfigured from allauth.socialaccount import app_settings +from allauth.socialaccount.adapter import get_adapter class ProviderException(Exception): @@ -135,10 +136,9 @@ def cleanup_email_addresses(self, email, addresses, email_verified=False): EmailAddress(email=email, verified=bool(email_verified), primary=True) ) # Force verified emails - settings = self.get_settings() - verified_email = settings.get("VERIFIED_EMAIL", False) - if verified_email: - for address in addresses: + adapter = get_adapter() + for address in addresses: + if adapter.is_email_verified(self, address.email): address.verified = True def extract_email_addresses(self, data): diff --git a/allauth/socialaccount/providers/saml/provider.py b/allauth/socialaccount/providers/saml/provider.py index 4e6226867b..f00af00408 100644 --- a/allauth/socialaccount/providers/saml/provider.py +++ b/allauth/socialaccount/providers/saml/provider.py @@ -15,7 +15,6 @@ class SAMLProvider(Provider): account_class = SAMLAccount default_attribute_mapping = { "uid": [ - "http://schemas.auth0.com/clientID", "urn:oasis:names:tc:SAML:attribute:subject-id", ], "email": [ @@ -51,11 +50,33 @@ def extract_extra_data(self, data): return data.get_attributes() def extract_uid(self, data): + """http://docs.oasis-open.org/security/saml-subject-id-attr/v1.0/csprd01/saml-subject-id-attr-v1.0-csprd01.html + + Quotes: + + "While the Attributes defined in this profile have as a goal the + explicit replacement of the element as a means of subject + identification, it is certainly possible to compose them with existing + NameID usage provided the same subject is being identified. This can + also serve as a migration strategy for existing applications." + + + "SAML does not define an identifier that meets all of these + requirements well. It does standardize a kind of NameID termed + “persistent” that meets some of them in the particular case of so-called + “pairwise” identification, where an identifier varies by relying + party. It has seen minimal adoption outside of a few contexts, and fails + at the “compact” and “simple to handle” criteria above, on top of the + disadvantages inherent with all NameID usage." + + Overall, our strategy is to prefer a uid resulting from explicit + attribute mappings, and only if there is no such uid fallback to the + NameID. """ - The `uid` is not unique across different SAML IdP's. Therefore, - we're using a fully qualified ID: @. - """ - return self._extract(data).get("uid") + uid = self._extract(data).get("uid") + if uid is None: + uid = data.get_nameid() + return uid def extract_common_fields(self, data): ret = self._extract(data) @@ -82,6 +103,15 @@ def _extract(self, data): if email_verified: email_verified = email_verified.lower() in ["true", "1", "t", "y", "yes"] attributes["email_verified"] = email_verified + + # If we did not find an email, check if the NameID contains the email. + if ( + not attributes.get("email") + and data.get_nameid_format() + == "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" + ): + attributes["email"] = data.get_nameid() + return attributes diff --git a/allauth/socialaccount/providers/saml/tests.py b/allauth/socialaccount/providers/saml/tests.py index d67ff6b5a3..2c0a66fdb8 100644 --- a/allauth/socialaccount/providers/saml/tests.py +++ b/allauth/socialaccount/providers/saml/tests.py @@ -170,13 +170,17 @@ def test_build_saml_config(rf, provider_config): @pytest.mark.parametrize( - "data, result", + "data, result, uid", [ - ({"urn:oasis:names:tc:SAML:attribute:subject-id": ["123"]}, {"uid": "123"}), - ({"http://schemas.auth0.com/clientID": ["123"]}, {"uid": "123"}), + ( + {"urn:oasis:names:tc:SAML:attribute:subject-id": ["123"]}, + {"uid": "123", "email": "nameid@saml.org"}, + "123", + ), + ({}, {"email": "nameid@saml.org"}, "nameid@saml.org"), ], ) -def test_extract_attributes(db, data, result, settings): +def test_extract_attributes(db, data, result, uid, settings): settings.SOCIALACCOUNT_PROVIDERS = { "saml": { "APPS": [ @@ -190,4 +194,9 @@ def test_extract_attributes(db, data, result, settings): provider = get_adapter().get_provider(request=None, provider="saml") onelogin_data = Mock() onelogin_data.get_attributes.return_value = data + onelogin_data.get_nameid.return_value = "nameid@saml.org" + onelogin_data.get_nameid_format.return_value = ( + "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" + ) assert provider._extract(onelogin_data) == result + assert provider.extract_uid(onelogin_data) == uid