From d635c484ae8e7b1324bb143a21a13dd274d0b6ff Mon Sep 17 00:00:00 2001
From: Quentin BEY <quentin.bey@mail.numerique.gouv.fr>
Date: Fri, 11 Oct 2024 12:57:13 +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 https://github.com/numerique-gouv/people/issues/455
---
 CHANGELOG.md                                  |  4 ++
 src/backend/core/authentication/backends.py   |  7 ++-
 .../tests/authentication/test_backends.py     | 60 +++++++++++++++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b522b9d..6fad2ea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,10 @@ and this project adheres to
 
 - 🛂(backend) match email if no existing user matches the sub
 
+### Fixed
+
+- 🛂(backend) do not duplicate user when disabled
+
 ## [1.2.1] - 2024-10-03
 
 ### Fixed
diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py
index 3dfdafa..ecbfdb7 100644
--- a/src/backend/core/authentication/backends.py
+++ b/src/backend/core/authentication/backends.py
@@ -84,7 +84,10 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
 
         # if sub is absent, try matching on email
         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
@@ -117,11 +120,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 bc19c95..5be246c 100644
--- a/src/backend/core/tests/authentication/test_backends.py
+++ b/src/backend/core/tests/authentication/test_backends.py
@@ -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)
 
     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