From 6a95d24441cb4dc4b578b2086108be4c51abc0e6 Mon Sep 17 00:00:00 2001
From: Samuel Paccoud - DINUM <samuel.paccoud@mail.numerique.gouv.fr>
Date: Wed, 16 Oct 2024 18:19:54 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82(backend)=20do=20not=20duplicate=20?=
 =?UTF-8?q?user=20when=20disabled?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 CHANGELOG.md                                  |  1 +
 src/backend/core/authentication/backends.py   |  6 +-
 .../tests/authentication/test_backends.py     | 60 +++++++++++++++++++
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d418a60a..7fdd240f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ and this project adheres to
 
 ## Fixed
 
+- 🛂(backend) do not duplicate user when disabled
 - 🐛(frontend) invalidate queries after removing user #336
 - 🐛(backend) Fix dysfunctional permissions on document create #329
 - 🐛(backend) fix nginx docker container #340
diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py
index be85957d..db115f82 100644
--- a/src/backend/core/authentication/backends.py
+++ b/src/backend/core/authentication/backends.py
@@ -84,6 +84,8 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
         user = self.get_existing_user(sub, email)
 
         if user:
+            if not user.is_active:
+                raise SuspiciousOperation(_("User account is disabled"))
             self.update_user_if_needed(user, claims)
         elif self.get_settings("OIDC_CREATE_USER", True):
             user = User.objects.create(sub=sub, password="!", **claims)  # noqa: S106
@@ -101,11 +103,11 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
     def get_existing_user(self, sub, email):
         """Fetch existing user by sub or email."""
         try:
-            return User.objects.get(sub=sub, is_active=True)
+            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, is_active=True)
+                    return User.objects.get(email=email)
                 except User.DoesNotExist:
                     pass
         return None
diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py
index 668504ed..8bf8b9f2 100644
--- a/src/backend/core/tests/authentication/test_backends.py
+++ b/src/backend/core/tests/authentication/test_backends.py
@@ -305,3 +305,63 @@ def test_authentication_get_userinfo_invalid_response():
         match="Invalid response format or token verification failed",
     ):
         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