From 32c9eaf2d70cfae4f52f8e51b0ac4cd1523c5915 Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Sat, 16 Dec 2023 21:42:12 +0100 Subject: [PATCH 1/5] feat: DEFAULT_AUTO_FIELD --- ChangeLog.rst | 4 ++++ allauth/account/apps.py | 4 +++- allauth/app_settings.py | 4 ++++ allauth/mfa/apps.py | 6 +++++- allauth/socialaccount/apps.py | 4 +++- allauth/usersessions/apps.py | 6 +++++- docs/common/configuration.rst | 8 ++++++++ docs/common/index.rst | 1 + 8 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 docs/common/configuration.rst diff --git a/ChangeLog.rst b/ChangeLog.rst index 2ed3e589df..7bc97a1e13 100644 --- a/ChangeLog.rst +++ b/ChangeLog.rst @@ -10,6 +10,10 @@ Note worthy changes change/set via the newly introduced ``get_password_change_redirect_url()`` adapter method. +- You can now configure the primary key of all models by configuring + ``ALLAUTH_DEFAULT_AUTO_FIELD``, for example to: + ``"hashid_field.HashidAutoField"``. + Backwards incompatible changes ------------------------------ diff --git a/allauth/account/apps.py b/allauth/account/apps.py index eabed36771..934c2c946d 100644 --- a/allauth/account/apps.py +++ b/allauth/account/apps.py @@ -3,11 +3,13 @@ from django.core.exceptions import ImproperlyConfigured from django.utils.translation import gettext_lazy as _ +from allauth import app_settings + class AccountConfig(AppConfig): name = "allauth.account" verbose_name = _("Accounts") - default_auto_field = "django.db.models.AutoField" + default_auto_field = app_settings.DEFAULT_AUTO_FIELD or "django.db.models.AutoField" def ready(self): required_mw = "allauth.account.middleware.AccountMiddleware" diff --git a/allauth/app_settings.py b/allauth/app_settings.py index fc8903fcff..27dfdbc92a 100644 --- a/allauth/app_settings.py +++ b/allauth/app_settings.py @@ -26,6 +26,10 @@ def MFA_ENABLED(self): def USERSESSIONS_ENABLED(self): return apps.is_installed("allauth.usersessions") + @property + def DEFAULT_AUTO_FIELD(self): + return self._setting("DEFAULT_AUTO_FIELD", None) + _app_settings = AppSettings("ALLAUTH_") diff --git a/allauth/mfa/apps.py b/allauth/mfa/apps.py index b8c01bb25d..49b71ad00d 100644 --- a/allauth/mfa/apps.py +++ b/allauth/mfa/apps.py @@ -1,11 +1,15 @@ from django.apps import AppConfig from django.utils.translation import gettext_lazy as _ +from allauth import app_settings + class MFAConfig(AppConfig): name = "allauth.mfa" verbose_name = _("MFA") - default_auto_field = "django.db.models.BigAutoField" + default_auto_field = ( + app_settings.DEFAULT_AUTO_FIELD or "django.db.models.BigAutoField" + ) def ready(self): from allauth.account import signals as account_signals diff --git a/allauth/socialaccount/apps.py b/allauth/socialaccount/apps.py index 6ef853b8ad..26ee42fc3a 100644 --- a/allauth/socialaccount/apps.py +++ b/allauth/socialaccount/apps.py @@ -1,8 +1,10 @@ from django.apps import AppConfig from django.utils.translation import gettext_lazy as _ +from allauth import app_settings + class SocialAccountConfig(AppConfig): name = "allauth.socialaccount" verbose_name = _("Social Accounts") - default_auto_field = "django.db.models.AutoField" + default_auto_field = app_settings.DEFAULT_AUTO_FIELD or "django.db.models.AutoField" diff --git a/allauth/usersessions/apps.py b/allauth/usersessions/apps.py index 7865873b07..7c51c0a003 100644 --- a/allauth/usersessions/apps.py +++ b/allauth/usersessions/apps.py @@ -1,11 +1,15 @@ from django.apps import AppConfig from django.utils.translation import gettext_lazy as _ +from allauth import app_settings + class UserSessionsConfig(AppConfig): name = "allauth.usersessions" verbose_name = _("User Sessions") - default_auto_field = "django.db.models.BigAutoField" + default_auto_field = ( + app_settings.DEFAULT_AUTO_FIELD or "django.db.models.BigAutoField" + ) def ready(self): from allauth.account.signals import user_logged_in diff --git a/docs/common/configuration.rst b/docs/common/configuration.rst new file mode 100644 index 0000000000..545e41b836 --- /dev/null +++ b/docs/common/configuration.rst @@ -0,0 +1,8 @@ +Configuration +============= + +Available settings: + +``ALLAUTH_DEFAULT_AUTO_FIELD`` + Can be set to configure the primary key of all models. For + example: ``"hashid_field.HashidAutoField"``. diff --git a/docs/common/index.rst b/docs/common/index.rst index c3401665c4..15475908c3 100644 --- a/docs/common/index.rst +++ b/docs/common/index.rst @@ -4,6 +4,7 @@ Common Functionality .. toctree:: :maxdepth: 1 + configuration email templates messages From 7885f38e86842eaa7235d021c4741659c5c998c1 Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Tue, 19 Dec 2023 20:06:40 +0100 Subject: [PATCH 2/5] fix(socialaccount): Ensure uid not empty --- .../socialaccount/providers/base/provider.py | 2 ++ .../providers/sharefile/tests.py | 25 ++++++++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/allauth/socialaccount/providers/base/provider.py b/allauth/socialaccount/providers/base/provider.py index c2d4ef4b2a..ef2e48a356 100644 --- a/allauth/socialaccount/providers/base/provider.py +++ b/allauth/socialaccount/providers/base/provider.py @@ -68,6 +68,8 @@ def sociallogin_from_response(self, request, response): raise ImproperlyConfigured( f"SOCIALACCOUNT_UID_MAX_LENGTH too small (<{len(uid)})" ) + if not uid: + raise ValueError("uid must be a non-empty string") extra_data = self.extract_extra_data(response) common_fields = self.extract_common_fields(response) diff --git a/allauth/socialaccount/providers/sharefile/tests.py b/allauth/socialaccount/providers/sharefile/tests.py index 0d37f28c4c..d231c2c6da 100644 --- a/allauth/socialaccount/providers/sharefile/tests.py +++ b/allauth/socialaccount/providers/sharefile/tests.py @@ -11,18 +11,15 @@ def get_mocked_response(self): return MockedResponse( 200, """ - {"access_token": "12345678abcdef", - "refresh_token": "12345678abcdef", - "token_type": "bearer", - "expires_in": 28800, - "appcp": "sharefile.com", - "apicp": "sharefile.com", - "subdomain": "example", - "access_files_folders": true, - "modify_files_folders": true, - "admin_users": true, - "admin_accounts": true, - "change_my_settings": true, - "web_app_login": true} - """, +{ + "Id": "123", + "Email":"user.one@domain.com", + "FirstName":"Name", + "LastName":"Last Name", + "Company":"Company", + "DefaultZone": + { + "Id":"zoneid" + } +} """, ) From 6251d5e082542297b3670d424edfaca97a6fe9d8 Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Tue, 19 Dec 2023 21:50:21 +0100 Subject: [PATCH 3/5] fix(saml): Attribute extraction could result in blank values --- .../socialaccount/providers/saml/provider.py | 6 ++--- allauth/socialaccount/providers/saml/tests.py | 27 ++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/allauth/socialaccount/providers/saml/provider.py b/allauth/socialaccount/providers/saml/provider.py index 6dcbf537f5..4e6226867b 100644 --- a/allauth/socialaccount/providers/saml/provider.py +++ b/allauth/socialaccount/providers/saml/provider.py @@ -55,7 +55,7 @@ def extract_uid(self, data): The `uid` is not unique across different SAML IdP's. Therefore, we're using a fully qualified ID: @. """ - return self._extract(data)["uid"] + return self._extract(data).get("uid") def extract_common_fields(self, data): ret = self._extract(data) @@ -74,8 +74,8 @@ def _extract(self, data): if isinstance(provider_keys, str): provider_keys = [provider_keys] for provider_key in provider_keys: - attribute_list = raw_attributes.get(provider_key, [""]) - if len(attribute_list) > 0: + attribute_list = raw_attributes.get(provider_key, None) + if attribute_list is not None and len(attribute_list) > 0: attributes[key] = attribute_list[0] break email_verified = attributes.get("email_verified") diff --git a/allauth/socialaccount/providers/saml/tests.py b/allauth/socialaccount/providers/saml/tests.py index 1e062b1beb..d67ff6b5a3 100644 --- a/allauth/socialaccount/providers/saml/tests.py +++ b/allauth/socialaccount/providers/saml/tests.py @@ -1,4 +1,4 @@ -from unittest.mock import patch +from unittest.mock import Mock, patch from urllib.parse import parse_qs, urlparse from django.urls import reverse @@ -7,6 +7,7 @@ import pytest from allauth.account.models import EmailAddress +from allauth.socialaccount.adapter import get_adapter from allauth.socialaccount.models import SocialAccount from allauth.socialaccount.providers.saml.utils import build_saml_config @@ -166,3 +167,27 @@ def test_build_saml_config(rf, provider_config): assert config["idp"]["x509cert"] == "cert" assert config["idp"]["singleSignOnService"] == {"url": "https://idp.org/sso/"} assert config["idp"]["singleLogoutService"] == {"url": "https://idp.saml.org/slo/"} + + +@pytest.mark.parametrize( + "data, result", + [ + ({"urn:oasis:names:tc:SAML:attribute:subject-id": ["123"]}, {"uid": "123"}), + ({"http://schemas.auth0.com/clientID": ["123"]}, {"uid": "123"}), + ], +) +def test_extract_attributes(db, data, result, settings): + settings.SOCIALACCOUNT_PROVIDERS = { + "saml": { + "APPS": [ + { + "client_id": "org", + "provider_id": "urn:dev-123.us.auth0.com", + } + ] + } + } + provider = get_adapter().get_provider(request=None, provider="saml") + onelogin_data = Mock() + onelogin_data.get_attributes.return_value = data + assert provider._extract(onelogin_data) == result From 53f5901cad06c12146b6a821ef3b17b4e4a343cc Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Wed, 20 Dec 2023 21:40:07 +0100 Subject: [PATCH 4/5] feat(saml): Default to NameID, email verifcation/authn per app --- ChangeLog.rst | 4 ++ allauth/socialaccount/adapter.py | 44 +++++++++++++++++++ allauth/socialaccount/models.py | 8 ++-- .../socialaccount/providers/base/provider.py | 8 ++-- .../socialaccount/providers/saml/provider.py | 40 ++++++++++++++--- allauth/socialaccount/providers/saml/tests.py | 17 +++++-- 6 files changed, 103 insertions(+), 18 deletions(-) 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 From 49824851e997063b06912de74fe562cd81b1f4c1 Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Fri, 22 Dec 2023 22:25:44 +0100 Subject: [PATCH 5/5] docs(example): pip install extras --- example/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/README.rst b/example/README.rst index 558fa9c344..998e8c35e1 100644 --- a/example/README.rst +++ b/example/README.rst @@ -11,7 +11,7 @@ django-allauth example application in this directory: $ cd django-allauth/example $ virtualenv venv $ . venv/bin/activate - $ pip install .. + $ pip install ..[mfa,saml] Now we need to create the database tables and an admin user. Run the following and when prompted to create a superuser choose yes and