🛂(backend) fallback to email matching when OIDC sub is not found
When OIDC providers return random values in the "sub" field instead of stable identifiers, implement email-based user matching as fallback. Note: Current implementation needs improvement. Tests forthcoming. Original: @sampaccoud (ff7914f) on Impress
This commit is contained in:
committed by
aleb_the_flash
parent
04d76acce5
commit
5ef6359b7c
@@ -1,5 +1,6 @@
|
||||
"""Authentication Backends for the Meet core app."""
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
@@ -71,22 +72,31 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
||||
_("User info contained no recognizable user identification")
|
||||
)
|
||||
|
||||
try:
|
||||
user = User.objects.get(sub=sub)
|
||||
except User.DoesNotExist:
|
||||
if self.get_settings("OIDC_CREATE_USER", True):
|
||||
user = User.objects.create(
|
||||
sub=sub,
|
||||
email=user_info.get("email"),
|
||||
password="!", # noqa: S106
|
||||
)
|
||||
else:
|
||||
user = None
|
||||
email = user_info.get("email")
|
||||
user = self.get_existing_user(sub, email)
|
||||
|
||||
if not user:
|
||||
if not user and self.get_settings("OIDC_CREATE_USER", True):
|
||||
user = User.objects.create(
|
||||
sub=sub,
|
||||
email=email,
|
||||
password="!", # noqa: S106
|
||||
)
|
||||
elif not user:
|
||||
return None
|
||||
|
||||
if not user.is_active:
|
||||
raise SuspiciousOperation(_("User account is disabled"))
|
||||
|
||||
return user
|
||||
|
||||
def get_existing_user(self, sub, email):
|
||||
"""Fetch existing user by sub or email."""
|
||||
try:
|
||||
return User.objects.get(sub=sub)
|
||||
except User.DoesNotExist:
|
||||
if email and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION:
|
||||
try:
|
||||
return User.objects.get(email=email)
|
||||
except User.DoesNotExist:
|
||||
pass
|
||||
return None
|
||||
|
||||
@@ -11,32 +11,35 @@ from core.factories import UserFactory
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
def test_authentication_getter_existing_user_no_email(
|
||||
django_assert_num_queries, monkeypatch
|
||||
):
|
||||
def test_authentication_getter_existing_user(monkeypatch):
|
||||
"""
|
||||
If an existing user matches the user's info sub, the user should be returned.
|
||||
If an existing user matches, the user should be returned.
|
||||
"""
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
db_user = UserFactory()
|
||||
db_user = UserFactory(email="foo@mail.com")
|
||||
|
||||
def get_userinfo_mocked(*args):
|
||||
return {"sub": db_user.sub}
|
||||
return {"sub": db_user.sub, "email": "some@mail.com"}
|
||||
|
||||
def get_existing_user(*args):
|
||||
return db_user
|
||||
|
||||
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||
monkeypatch.setattr(
|
||||
OIDCAuthenticationBackend, "get_existing_user", get_existing_user
|
||||
)
|
||||
|
||||
with django_assert_num_queries(1):
|
||||
user = klass.get_or_create_user(
|
||||
access_token="test-token", id_token=None, payload=None
|
||||
)
|
||||
user = klass.get_or_create_user(
|
||||
access_token="test-token", id_token=None, payload=None
|
||||
)
|
||||
|
||||
assert user == db_user
|
||||
|
||||
|
||||
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, a user should be created.
|
||||
User's info doesn't contain an email, created user's email should be empty.
|
||||
"""
|
||||
klass = OIDCAuthenticationBackend()
|
||||
@@ -58,7 +61,7 @@ def test_authentication_getter_new_user_no_email(monkeypatch):
|
||||
|
||||
def test_authentication_getter_new_user_with_email(monkeypatch):
|
||||
"""
|
||||
If no user matches the user's info sub, a user should be created.
|
||||
If no user matches, a user should be created.
|
||||
User's email and name should be set on the identity.
|
||||
The "email" field on the User model should not be set as it is reserved for staff users.
|
||||
"""
|
||||
@@ -143,3 +146,59 @@ def test_authentication_get_inactive_user(monkeypatch):
|
||||
),
|
||||
):
|
||||
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
|
||||
|
||||
|
||||
def test_finds_user_by_sub(django_assert_num_queries):
|
||||
"""Should return user when found by sub, and email is matching."""
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
db_user = UserFactory(email="foo@mail.com")
|
||||
|
||||
with django_assert_num_queries(1):
|
||||
user = klass.get_existing_user(db_user.sub, db_user.email)
|
||||
|
||||
assert user == db_user
|
||||
|
||||
|
||||
def test_finds_user_when_email_fallback_disabled(django_assert_num_queries, settings):
|
||||
"""Should not return a user when not found by sub, and email fallback is disabled."""
|
||||
|
||||
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = False
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
db_user = UserFactory(email="foo@mail.com")
|
||||
|
||||
with django_assert_num_queries(1):
|
||||
user = klass.get_existing_user("wrong-sub", db_user.email)
|
||||
|
||||
assert user is None
|
||||
|
||||
|
||||
def test_finds_user_when_email_is_none(django_assert_num_queries, settings):
|
||||
"""Should not return a user when not found by sub, and email is empty."""
|
||||
|
||||
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = True
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
UserFactory(email="foo@mail.com")
|
||||
|
||||
empty_email = ""
|
||||
|
||||
with django_assert_num_queries(1):
|
||||
user = klass.get_existing_user("wrong-sub", empty_email)
|
||||
|
||||
assert user is None
|
||||
|
||||
|
||||
def test_finds_user_by_email(django_assert_num_queries, settings):
|
||||
"""Should return user when found by email, and sub is not matching."""
|
||||
|
||||
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = True
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
db_user = UserFactory(email="foo@mail.com")
|
||||
|
||||
with django_assert_num_queries(2):
|
||||
user = klass.get_existing_user("wrong-sub", db_user.email)
|
||||
|
||||
assert user == db_user
|
||||
|
||||
@@ -327,6 +327,10 @@ class Base(Configuration):
|
||||
default=True,
|
||||
environ_name="OIDC_CREATE_USER",
|
||||
)
|
||||
OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = values.BooleanValue(
|
||||
default=False,
|
||||
environ_name="OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION",
|
||||
)
|
||||
OIDC_RP_SIGN_ALGO = values.Value(
|
||||
"RS256", environ_name="OIDC_RP_SIGN_ALGO", environ_prefix=None
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user