diff --git a/CHANGELOG.md b/CHANGELOG.md index d418a60a96..7fdd240f45 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 be85957d2a..db115f82df 100644 --- a/src/backend/core/authentication/backends.py +++ b/src/backend/core/authentication/backends.py @@ -84,6 +84,8 @@ def get_or_create_user(self, access_token, id_token, payload): 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 @@ def compute_full_name(self, user_info): 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 668504ed93..8bf8b9f2d8 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