🛂(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 #324 Work initially contributed by @qbey on: https://github.com/numerique-gouv/people/pull/456
This commit is contained in:
committed by
Samuel Paccoud
parent
e816f0afc8
commit
6a95d24441
@@ -23,6 +23,7 @@ and this project adheres to
|
|||||||
|
|
||||||
## Fixed
|
## Fixed
|
||||||
|
|
||||||
|
- 🛂(backend) do not duplicate user when disabled
|
||||||
- 🐛(frontend) invalidate queries after removing user #336
|
- 🐛(frontend) invalidate queries after removing user #336
|
||||||
- 🐛(backend) Fix dysfunctional permissions on document create #329
|
- 🐛(backend) Fix dysfunctional permissions on document create #329
|
||||||
- 🐛(backend) fix nginx docker container #340
|
- 🐛(backend) fix nginx docker container #340
|
||||||
|
|||||||
@@ -84,6 +84,8 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
|||||||
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
|
||||||
@@ -101,11 +103,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
|
||||||
|
|||||||
@@ -305,3 +305,63 @@ def test_authentication_get_userinfo_invalid_response():
|
|||||||
match="Invalid response format or token verification failed",
|
match="Invalid response format or token verification failed",
|
||||||
):
|
):
|
||||||
oidc_backend.get_userinfo("fake_access_token", None, None)
|
oidc_backend.get_userinfo("fake_access_token", None, None)
|
||||||
|
|
||||||
|
|
||||||
|
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 = UserFactory(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 matches the email and is disabled,
|
||||||
|
an error should be raised and a user should not be created.
|
||||||
|
"""
|
||||||
|
|
||||||
|
klass = OIDCAuthenticationBackend()
|
||||||
|
db_user = UserFactory(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