Skip to content

Commit 6a95d24

Browse files
committed
🛂(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: suitenumerique/people#456
1 parent e816f0a commit 6a95d24

File tree

3 files changed

+65
-2
lines changed

3 files changed

+65
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ and this project adheres to
2323

2424
## Fixed
2525

26+
- 🛂(backend) do not duplicate user when disabled
2627
- 🐛(frontend) invalidate queries after removing user #336
2728
- 🐛(backend) Fix dysfunctional permissions on document create #329
2829
- 🐛(backend) fix nginx docker container #340

src/backend/core/authentication/backends.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ def get_or_create_user(self, access_token, id_token, payload):
8484
user = self.get_existing_user(sub, email)
8585

8686
if user:
87+
if not user.is_active:
88+
raise SuspiciousOperation(_("User account is disabled"))
8789
self.update_user_if_needed(user, claims)
8890
elif self.get_settings("OIDC_CREATE_USER", True):
8991
user = User.objects.create(sub=sub, password="!", **claims) # noqa: S106
@@ -101,11 +103,11 @@ def compute_full_name(self, user_info):
101103
def get_existing_user(self, sub, email):
102104
"""Fetch existing user by sub or email."""
103105
try:
104-
return User.objects.get(sub=sub, is_active=True)
106+
return User.objects.get(sub=sub)
105107
except User.DoesNotExist:
106108
if email and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION:
107109
try:
108-
return User.objects.get(email=email, is_active=True)
110+
return User.objects.get(email=email)
109111
except User.DoesNotExist:
110112
pass
111113
return None

src/backend/core/tests/authentication/test_backends.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,3 +305,63 @@ def test_authentication_get_userinfo_invalid_response():
305305
match="Invalid response format or token verification failed",
306306
):
307307
oidc_backend.get_userinfo("fake_access_token", None, None)
308+
309+
310+
def test_authentication_getter_existing_disabled_user_via_sub(
311+
django_assert_num_queries, monkeypatch
312+
):
313+
"""
314+
If an existing user matches the sub but is disabled,
315+
an error should be raised and a user should not be created.
316+
"""
317+
318+
klass = OIDCAuthenticationBackend()
319+
db_user = UserFactory(is_active=False)
320+
321+
def get_userinfo_mocked(*args):
322+
return {
323+
"sub": db_user.sub,
324+
"email": db_user.email,
325+
"first_name": "John",
326+
"last_name": "Doe",
327+
}
328+
329+
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
330+
331+
with (
332+
django_assert_num_queries(1),
333+
pytest.raises(SuspiciousOperation, match="User account is disabled"),
334+
):
335+
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
336+
337+
assert models.User.objects.count() == 1
338+
339+
340+
def test_authentication_getter_existing_disabled_user_via_email(
341+
django_assert_num_queries, monkeypatch
342+
):
343+
"""
344+
If an existing user does not matches the sub but matches the email and is disabled,
345+
an error should be raised and a user should not be created.
346+
"""
347+
348+
klass = OIDCAuthenticationBackend()
349+
db_user = UserFactory(is_active=False)
350+
351+
def get_userinfo_mocked(*args):
352+
return {
353+
"sub": "random",
354+
"email": db_user.email,
355+
"first_name": "John",
356+
"last_name": "Doe",
357+
}
358+
359+
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
360+
361+
with (
362+
django_assert_num_queries(2),
363+
pytest.raises(SuspiciousOperation, match="User account is disabled"),
364+
):
365+
klass.get_or_create_user(access_token="test-token", id_token=None, payload=None)
366+
367+
assert models.User.objects.count() == 1

0 commit comments

Comments
 (0)