Skip to content

Commit

Permalink
feat(saml): Default to NameID, email verifcation/authn per app
Browse files Browse the repository at this point in the history
  • Loading branch information
pennersr committed Dec 20, 2023
1 parent 6251d5e commit 53f5901
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 18 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
*******************
Expand Down
44 changes: 44 additions & 0 deletions allauth/socialaccount/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
8 changes: 3 additions & 5 deletions allauth/socialaccount/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
8 changes: 4 additions & 4 deletions allauth/socialaccount/providers/base/provider.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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):
Expand Down
40 changes: 35 additions & 5 deletions allauth/socialaccount/providers/saml/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -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 <saml:NameID> 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: <uid>@<entity_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)
Expand All @@ -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


Expand Down
17 changes: 13 additions & 4 deletions allauth/socialaccount/providers/saml/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]"},
"123",
),
({}, {"email": "[email protected]"}, "[email protected]"),
],
)
def test_extract_attributes(db, data, result, settings):
def test_extract_attributes(db, data, result, uid, settings):
settings.SOCIALACCOUNT_PROVIDERS = {
"saml": {
"APPS": [
Expand All @@ -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 = "[email protected]"
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

0 comments on commit 53f5901

Please sign in to comment.