Merge pull request #412 from maykinmedia/issue/997-openidconnect-impr…

…ovements

[#997] Issue/OpenIdConnect improvements

src/open_inwoner/accounts/backends.py

@@ -44,7 +44,7 @@ def create_user(self, claims):
         if "email" in claims:
             email = claims["email"]
 
-        existing_user = self.UserModel.objects.filter(email=email).first()
+        existing_user = self.UserModel.objects.filter(email__iexact=email).first()
         if existing_user:
             logger.debug("Updating OIDC user: %s with email %s", unique_id, email)
             existing_user.oidc_id = unique_id

src/open_inwoner/accounts/tests/test_oidc_views.py

@@ -1,12 +1,12 @@
 from unittest.mock import patch
 
 from django.contrib.auth import get_user_model
-from django.db import IntegrityError
 from django.test import TestCase
 from django.urls import reverse
 
 from mozilla_django_oidc_db.models import OpenIDConnectConfig
 
+from ..choices import LoginTypeChoices
 from .factories import UserFactory
 
 User = get_user_model()
@@ -21,23 +21,21 @@ class OIDCFlowTests(TestCase):
         "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo",
         return_value=OpenIDConnectConfig(enabled=True),
     )
-    def test_duplicate_email_unique_constraint_violated(
+    def test_existing_email_updates_user(
         self,
         mock_get_solo,
         mock_get_token,
         mock_verify_token,
         mock_store_tokens,
         mock_get_userinfo,
     ):
-        """
-        Assert that duplicate email addresses result in usable user feedback.
-        """
         # set up a user with a colliding email address
+        # sub is the oidc_id field in our db
         mock_get_userinfo.return_value = {
-            "email": "collision@example.com",
+            "email": "existing_user@example.com",
             "sub": "some_username",
         }
-        user = UserFactory.create(email="collision@example.com")
+        user = UserFactory.create(email="existing_user@example.com")
         self.assertEqual(user.oidc_id, "")
         session = self.client.session
         session["oidc_states"] = {"mock": {"nonce": "nonce"}}
@@ -49,14 +47,18 @@ def test_duplicate_email_unique_constraint_violated(
             callback_url, {"code": "mock", "state": "mock"}
         )
 
+        user.refresh_from_db()
+
         self.assertRedirects(
             callback_response, reverse("root"), fetch_redirect_response=False
         )
         self.assertTrue(User.objects.filter(oidc_id="some_username").exists())
-        user.refresh_from_db()
         self.assertEqual(user.oidc_id, "some_username")
+
         db_user = User.objects.filter(oidc_id="some_username").first()
+
         self.assertEqual(db_user.id, user.id)
+        self.assertEqual(db_user.login_type, LoginTypeChoices.oidc)
 
     @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo")
     @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens")
@@ -66,28 +68,76 @@ def test_duplicate_email_unique_constraint_violated(
         "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo",
         return_value=OpenIDConnectConfig(enabled=True),
     )
-    def test_happy_flow(
+    def test_existing_case_sensitive_email_updates_user(
         self,
         mock_get_solo,
         mock_get_token,
         mock_verify_token,
         mock_store_tokens,
         mock_get_userinfo,
     ):
-        """
-        Assert that duplicate email addresses result in usable user feedback.
-        """
         # set up a user with a colliding email address
+        # sub is the oidc_id field in our db
         mock_get_userinfo.return_value = {
-            "email": "[email protected]",
+            "email": "[email protected]",
+            "sub": "some_username",
+        }
+        user = UserFactory.create(
+            email="[email protected]", login_type=LoginTypeChoices.default
+        )
+        self.assertEqual(user.oidc_id, "")
+        session = self.client.session
+        session["oidc_states"] = {"mock": {"nonce": "nonce"}}
+        session.save()
+        callback_url = reverse("oidc_authentication_callback")
+
+        # enter the login flow
+        callback_response = self.client.get(
+            callback_url, {"code": "mock", "state": "mock"}
+        )
+
+        user.refresh_from_db()
+
+        self.assertRedirects(
+            callback_response, reverse("root"), fetch_redirect_response=False
+        )
+        self.assertTrue(User.objects.filter(oidc_id="some_username").exists())
+        self.assertEqual(user.oidc_id, "some_username")
+
+        db_user = User.objects.filter(oidc_id="some_username").first()
+
+        self.assertEqual(db_user.id, user.id)
+        self.assertEqual(db_user.login_type, LoginTypeChoices.oidc)
+
+    @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo")
+    @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens")
+    @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.verify_token")
+    @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token")
+    @patch(
+        "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo",
+        return_value=OpenIDConnectConfig(enabled=True),
+    )
+    def test_new_user_is_created_when_new_email(
+        self,
+        mock_get_solo,
+        mock_get_token,
+        mock_verify_token,
+        mock_store_tokens,
+        mock_get_userinfo,
+    ):
+        # set up a user with a non existing email address
+        mock_get_userinfo.return_value = {
+            "email": "[email protected]",
             "sub": "some_username",
         }
-        UserFactory.create(email="collision@example.com")
+        UserFactory.create(email="existing_user@example.com")
         session = self.client.session
         session["oidc_states"] = {"mock": {"nonce": "nonce"}}
         session.save()
         callback_url = reverse("oidc_authentication_callback")
 
+        self.assertFalse(User.objects.filter(email="[email protected]").exists())
+
         # enter the login flow
         callback_response = self.client.get(
             callback_url, {"code": "mock", "state": "mock"}
@@ -96,7 +146,11 @@ def test_happy_flow(
         self.assertRedirects(
             callback_response, reverse("root"), fetch_redirect_response=False
         )
-        self.assertTrue(User.objects.filter(email="[email protected]").exists())
+        new_user = User.objects.filter(email="[email protected]")
+
+        self.assertTrue(new_user.exists())
+        self.assertEqual(new_user.get().oidc_id, "some_username")
+        self.assertEqual(new_user.get().login_type, LoginTypeChoices.oidc)
 
     def test_error_page_direct_access_forbidden(self):
         error_url = reverse("admin-oidc-error")