🛂(backend) do not duplicate user when disabled
When a user is disabled and tries to login, we don't want the user to be duplicated, the user should not be able to login. Fixes https://github.com/numerique-gouv/people/issues/455
This commit is contained in:
@@ -20,6 +20,10 @@ and this project adheres to
|
|||||||
|
|
||||||
- 🛂(backend) match email if no existing user matches the sub
|
- 🛂(backend) match email if no existing user matches the sub
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- 🛂(backend) do not duplicate user when disabled
|
||||||
|
|
||||||
## [1.2.1] - 2024-10-03
|
## [1.2.1] - 2024-10-03
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -84,7 +84,10 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
|
|
||||||
# if sub is absent, try matching on email
|
# if sub is absent, try matching on email
|
||||||
user = self.get_existing_user(sub, email)
|
user = self.get_existing_user(sub, email)
|
||||||
|
|
||||||
if user:
|
if user:
|
||||||
|
if not user.is_active:
|
||||||
|
raise SuspiciousOperation(_("User account is disabled"))
|
||||||
self.update_user_if_needed(user, claims)
|
self.update_user_if_needed(user, claims)
|
||||||
elif self.get_settings("OIDC_CREATE_USER", True):
|
elif self.get_settings("OIDC_CREATE_USER", True):
|
||||||
user = User.objects.create(sub=sub, password="!", **claims) # noqa: S106
|
user = User.objects.create(sub=sub, password="!", **claims) # noqa: S106
|
||||||
@@ -117,11 +120,11 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
def get_existing_user(self, sub, email):
|
def get_existing_user(self, sub, email):
|
||||||
"""Fetch existing user by sub or email."""
|
"""Fetch existing user by sub or email."""
|
||||||
try:
|
try:
|
||||||
return User.objects.get(sub=sub, is_active=True)
|
return User.objects.get(sub=sub)
|
||||||
except User.DoesNotExist:
|
except User.DoesNotExist:
|
||||||
if email and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION:
|
if email and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION:
|
||||||
try:
|
try:
|
||||||
return User.objects.get(email=email, is_active=True)
|
return User.objects.get(email=email)
|
||||||
except User.DoesNotExist:
|
except User.DoesNotExist:
|
||||||
pass
|
pass
|
||||||
return None
|
return None
|
||||||
|
|||||||
@@ -224,3 +224,63 @@ def test_models_oidc_user_getter_invalid_token(django_assert_num_queries, monkey
|
|||||||
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
|
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
|
||||||
|
|
||||||
assert models.User.objects.exists() is False
|
assert models.User.objects.exists() is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_authentication_getter_existing_disabled_user_via_sub(
|
||||||
|
django_assert_num_queries, monkeypatch
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
If an existing user matches the sub but is disabled,
|
||||||
|
an error should be raised and a user should not be created.
|
||||||
|
"""
|
||||||
|
|
||||||
|
klass = OIDCAuthenticationBackend()
|
||||||
|
db_user = factories.UserFactory(name="John Doe", is_active=False)
|
||||||
|
|
||||||
|
def get_userinfo_mocked(*args):
|
||||||
|
return {
|
||||||
|
"sub": db_user.sub,
|
||||||
|
"email": db_user.email,
|
||||||
|
"first_name": "John",
|
||||||
|
"last_name": "Doe",
|
||||||
|
}
|
||||||
|
|
||||||
|
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||||
|
|
||||||
|
with (
|
||||||
|
django_assert_num_queries(1),
|
||||||
|
pytest.raises(SuspiciousOperation, match="User account is disabled"),
|
||||||
|
):
|
||||||
|
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
|
||||||
|
|
||||||
|
assert models.User.objects.count() == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_authentication_getter_existing_disabled_user_via_email(
|
||||||
|
django_assert_num_queries, monkeypatch
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
If an existing user does not matches the sub but match the email and is disabled,
|
||||||
|
an error should be raised and a user should not be created.
|
||||||
|
"""
|
||||||
|
|
||||||
|
klass = OIDCAuthenticationBackend()
|
||||||
|
db_user = factories.UserFactory(name="John Doe", is_active=False)
|
||||||
|
|
||||||
|
def get_userinfo_mocked(*args):
|
||||||
|
return {
|
||||||
|
"sub": "random",
|
||||||
|
"email": db_user.email,
|
||||||
|
"first_name": "John",
|
||||||
|
"last_name": "Doe",
|
||||||
|
}
|
||||||
|
|
||||||
|
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||||
|
|
||||||
|
with (
|
||||||
|
django_assert_num_queries(2),
|
||||||
|
pytest.raises(SuspiciousOperation, match="User account is disabled"),
|
||||||
|
):
|
||||||
|
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
|
||||||
|
|
||||||
|
assert models.User.objects.count() == 1
|
||||||
|
|||||||
Reference in New Issue
Block a user