🛂(backend) match email if no existing user matches the sub
Some OIDC identity providers may provide a random value in the "sub" field instead of an identifying ID. In this case, it may be a good idea to fallback to matching the user on its email field.
This commit is contained in:
committed by
Sabrina Demagny
parent
f243a2423f
commit
0e48bc0f90
@@ -16,6 +16,10 @@ and this project adheres to
|
|||||||
|
|
||||||
- ✨(backend) manage roles on domain admin view
|
- ✨(backend) manage roles on domain admin view
|
||||||
|
|
||||||
|
### Changed
|
||||||
|
|
||||||
|
- 🛂(backend) match email if no existing user matches the sub
|
||||||
|
|
||||||
## [1.2.1] - 2024-10-03
|
## [1.2.1] - 2024-10-03
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Authentication Backends for the People core app."""
|
"""Authentication Backends for the People core app."""
|
||||||
|
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
|
from django.contrib.auth import get_user_model
|
||||||
from django.core.exceptions import SuspiciousOperation
|
from django.core.exceptions import SuspiciousOperation
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
|
|
||||||
@@ -9,6 +10,8 @@ from mozilla_django_oidc.auth import (
|
|||||||
OIDCAuthenticationBackend as MozillaOIDCAuthenticationBackend,
|
OIDCAuthenticationBackend as MozillaOIDCAuthenticationBackend,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
User = get_user_model()
|
||||||
|
|
||||||
|
|
||||||
class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
||||||
"""Custom OpenID Connect (OIDC) Authentication Backend.
|
"""Custom OpenID Connect (OIDC) Authentication Backend.
|
||||||
@@ -48,7 +51,7 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
return userinfo
|
return userinfo
|
||||||
|
|
||||||
def get_or_create_user(self, access_token, id_token, payload):
|
def get_or_create_user(self, access_token, id_token, payload):
|
||||||
"""Return a User based on userinfo. Get or create a new user if no user matches the Sub.
|
"""Return a User based on userinfo. Create a new user if no match is found.
|
||||||
|
|
||||||
Parameters:
|
Parameters:
|
||||||
- access_token (str): The access token.
|
- access_token (str): The access token.
|
||||||
@@ -64,30 +67,27 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
|
|
||||||
user_info = self.get_userinfo(access_token, id_token, payload)
|
user_info = self.get_userinfo(access_token, id_token, payload)
|
||||||
|
|
||||||
# Compute user name from OIDC name fields as defined in settings
|
# Get user's full name from OIDC fields defined in settings
|
||||||
names_list = [
|
full_name = self.compute_full_name(user_info)
|
||||||
user_info[field]
|
email = user_info.get("email")
|
||||||
for field in settings.USER_OIDC_FIELDS_TO_NAME
|
|
||||||
if user_info.get(field)
|
claims = {
|
||||||
]
|
"email": email,
|
||||||
user_info["name"] = " ".join(names_list) or None
|
"name": full_name,
|
||||||
|
}
|
||||||
|
|
||||||
sub = user_info.get("sub")
|
sub = user_info.get("sub")
|
||||||
if sub is None:
|
if not sub:
|
||||||
raise SuspiciousOperation(
|
raise SuspiciousOperation(
|
||||||
_("User info contained no recognizable user identification")
|
_("User info contained no recognizable user identification")
|
||||||
)
|
)
|
||||||
|
|
||||||
try:
|
# if sub is absent, try matching on email
|
||||||
user = self.UserModel.objects.get(sub=sub, is_active=True)
|
user = self.get_existing_user(sub, email)
|
||||||
except self.UserModel.DoesNotExist:
|
if user:
|
||||||
if self.get_settings("OIDC_CREATE_USER", True):
|
self.update_user_if_needed(user, claims)
|
||||||
user = self.create_user(user_info)
|
elif self.get_settings("OIDC_CREATE_USER", True):
|
||||||
else:
|
user = User.objects.create(sub=sub, password="!", **claims) # noqa: S106
|
||||||
email = user_info.get("email")
|
|
||||||
name = user_info.get("name")
|
|
||||||
if email and email != user.email or name and name != user.name:
|
|
||||||
self.UserModel.objects.filter(sub=sub).update(email=email, name=name)
|
|
||||||
|
|
||||||
return user
|
return user
|
||||||
|
|
||||||
@@ -105,3 +105,32 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
email=claims.get("email"),
|
email=claims.get("email"),
|
||||||
name=claims.get("name"),
|
name=claims.get("name"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def compute_full_name(self, user_info):
|
||||||
|
"""Compute user's full name based on OIDC fields in settings."""
|
||||||
|
name_fields = settings.USER_OIDC_FIELDS_TO_NAME
|
||||||
|
full_name = " ".join(
|
||||||
|
user_info[field] for field in name_fields if user_info.get(field)
|
||||||
|
)
|
||||||
|
return full_name or None
|
||||||
|
|
||||||
|
def get_existing_user(self, sub, email):
|
||||||
|
"""Fetch existing user by sub or email."""
|
||||||
|
try:
|
||||||
|
return User.objects.get(sub=sub, is_active=True)
|
||||||
|
except User.DoesNotExist:
|
||||||
|
if email and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION:
|
||||||
|
try:
|
||||||
|
return User.objects.get(email=email, is_active=True)
|
||||||
|
except User.DoesNotExist:
|
||||||
|
pass
|
||||||
|
return None
|
||||||
|
|
||||||
|
def update_user_if_needed(self, user, claims):
|
||||||
|
"""Update user claims if they have changed."""
|
||||||
|
has_changed = any(
|
||||||
|
value and value != getattr(user, key) for key, value in claims.items()
|
||||||
|
)
|
||||||
|
if has_changed:
|
||||||
|
updated_claims = {key: value for key, value in claims.items() if value}
|
||||||
|
self.UserModel.objects.filter(sub=user.sub).update(**updated_claims)
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
"""Unit tests for the Authentication Backends."""
|
"""Unit tests for the Authentication Backends."""
|
||||||
|
|
||||||
|
from django.contrib.auth import get_user_model
|
||||||
from django.core.exceptions import SuspiciousOperation
|
from django.core.exceptions import SuspiciousOperation
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -8,6 +9,7 @@ from core import factories, models
|
|||||||
from core.authentication.backends import OIDCAuthenticationBackend
|
from core.authentication.backends import OIDCAuthenticationBackend
|
||||||
|
|
||||||
pytestmark = pytest.mark.django_db
|
pytestmark = pytest.mark.django_db
|
||||||
|
User = get_user_model()
|
||||||
|
|
||||||
|
|
||||||
def test_authentication_getter_existing_user_no_email(
|
def test_authentication_getter_existing_user_no_email(
|
||||||
@@ -101,6 +103,59 @@ def test_authentication_getter_existing_user_change_fields(
|
|||||||
assert user.name == f"{first_name:s} {last_name:s}"
|
assert user.name == f"{first_name:s} {last_name:s}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_authentication_getter_existing_user_via_email(
|
||||||
|
django_assert_num_queries, monkeypatch
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
If an existing user doesn't match the sub but matches the email,
|
||||||
|
the user should be returned.
|
||||||
|
"""
|
||||||
|
|
||||||
|
klass = OIDCAuthenticationBackend()
|
||||||
|
db_user = factories.UserFactory()
|
||||||
|
|
||||||
|
def get_userinfo_mocked(*args):
|
||||||
|
return {"sub": "123", "email": db_user.email}
|
||||||
|
|
||||||
|
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||||
|
|
||||||
|
with django_assert_num_queries(2):
|
||||||
|
user = klass.get_or_create_user(
|
||||||
|
access_token="test-token", id_token=None, payload=None
|
||||||
|
)
|
||||||
|
|
||||||
|
assert user == db_user
|
||||||
|
|
||||||
|
|
||||||
|
def test_authentication_getter_existing_user_no_fallback_to_email(
|
||||||
|
settings, monkeypatch
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
When the "OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION" setting is set to False,
|
||||||
|
the system should not match users by email, even if the email matches.
|
||||||
|
"""
|
||||||
|
|
||||||
|
klass = OIDCAuthenticationBackend()
|
||||||
|
db_user = factories.UserFactory()
|
||||||
|
|
||||||
|
# Set the setting to False
|
||||||
|
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = False
|
||||||
|
|
||||||
|
def get_userinfo_mocked(*args):
|
||||||
|
return {"sub": "123", "email": db_user.email}
|
||||||
|
|
||||||
|
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||||
|
|
||||||
|
user = klass.get_or_create_user(
|
||||||
|
access_token="test-token", id_token=None, payload=None
|
||||||
|
)
|
||||||
|
|
||||||
|
# Since the sub doesn't match, it should create a new user
|
||||||
|
assert models.User.objects.count() == 2
|
||||||
|
assert user != db_user
|
||||||
|
assert user.sub == "123"
|
||||||
|
|
||||||
|
|
||||||
def test_authentication_getter_new_user_no_email(monkeypatch):
|
def test_authentication_getter_new_user_no_email(monkeypatch):
|
||||||
"""
|
"""
|
||||||
If no user matches the user's info sub, a user should be created.
|
If no user matches the user's info sub, a user should be created.
|
||||||
|
|||||||
@@ -413,6 +413,11 @@ class Base(Configuration):
|
|||||||
)
|
)
|
||||||
|
|
||||||
OIDC_TIMEOUT = values.Value(None, environ_name="OIDC_TIMEOUT", environ_prefix=None)
|
OIDC_TIMEOUT = values.Value(None, environ_name="OIDC_TIMEOUT", environ_prefix=None)
|
||||||
|
OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = values.BooleanValue(
|
||||||
|
default=True,
|
||||||
|
environ_name="OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION",
|
||||||
|
environ_prefix=None,
|
||||||
|
)
|
||||||
|
|
||||||
# MAILBOX-PROVISIONING API
|
# MAILBOX-PROVISIONING API
|
||||||
WEBMAIL_URL = values.Value(
|
WEBMAIL_URL = values.Value(
|
||||||
|
|||||||
Reference in New Issue
Block a user