Update username regex and more

.github/workflows/ci.yml

@@ -13,15 +13,15 @@ jobs:
       fail-fast: false
       matrix:
         include:
-          - python_version: 3.8
-            toxenv: "py38-django32"
-          - python_version: 3.9
-            toxenv: "py39-django32"
-          - python_version: 3.8
-            toxenv: "py38-django40"
-          - python_version: 3.9
-            toxenv: "py39-django40"
-          - python_version: 3.9
+          - python_version: "3.10"
+            toxenv: "py310-django32"
+          - python_version: 3.11
+            toxenv: "py311-django32"
+          - python_version: "3.10"
+            toxenv: "py310-django40"
+          - python_version: 3.11
+            toxenv: "py311-django40"
+          - python_version: "3.10"
             toxenv: "lint"
     steps:
       - name: Checkout code

README.md

@@ -55,7 +55,7 @@ OPENID_CONNECT_VIEWSET_CONFIG = {
     # that's used to validate all field inputs retrieved for the particular key
     "FIELD_VALIDATION_REGEX": {
         "username": {
-            "regex": "(?!^\d+$)^.+$",
+            "regex": "^(?!\d+$)[a-zA-Z0-9]{3,}$",
             "help_text": "Username should only contain alpha numeric characters",
         }
     },

oidc/settings.py

@@ -18,8 +18,8 @@
     "JWT_ALGORITHM": "HS256",
     "FIELD_VALIDATION_REGEX": {
         "username": {
-            "regex": "(?!^\d+$)^.+$",  # noqa
-            "help_text": "Username should only contain alpha numeric characters",
+            "regex": r"^(?!\d+$)[a-zA-Z0-9_]{3,}$",  # noqa
+            "help_text": "Username should only contain alpha numeric characters and should be at least 3 characters",
         }
     },
     "REPLACE_USERNAME_CHARACTERS": "-.",

oidc/viewsets.py

@@ -227,9 +227,15 @@ def _clean_user_data(self, user_data) -> Tuple[dict, Optional[list]]:
             user_data["first_name"] = user_data["last_name"]
             missing_fields.remove("first_name")
 
-        # use email as username if username is missing
+        # use email as username if username is missing or username is invalid
         if self.use_email_as_username:
-            if "username" in missing_fields and "email" in user_data:
+            username_regex = re.compile(
+                self.field_validation_regex["username"].get("regex")
+            )
+            if (
+                "username" in missing_fields
+                or not username_regex.search(user_data["username"])
+            ) and "email" in user_data:
                 username = user_data["email"].split("@")[0]
                 if (
                     self.user_model.objects.filter(username__iexact=username).count()
@@ -246,12 +252,12 @@ def _clean_user_data(self, user_data) -> Tuple[dict, Optional[list]]:
                             )
 
                     # Validate retrieved username matches regex
-                    if "username" in self.field_validation_regex:
-                        regex = re.compile(
-                            self.field_validation_regex["username"].get("regex")
-                        )
-                        if regex.search(username):
-                            user_data["username"] = username
+                    if (
+                        "username" in self.field_validation_regex
+                        and username_regex.search(username)
+                    ):
+                        user_data["username"] = username
+                        if "username" in missing_fields:
                             missing_fields.remove("username")
 
         return user_data, missing_fields

tests/test_viewsets.py

@@ -1,6 +1,7 @@
 """
 Tests for the OpenID Client
 """
+
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -44,10 +45,11 @@
     "SSO_COOKIE_DATA": "email",
     "JWT_ALGORITHM": "HS256",
     "JWT_SECRET_KEY": "abc",
+    "REPLACE_USERNAME_CHARACTERS": "-.",
     "FIELD_VALIDATION_REGEX": {
         "username": {
-            "regex": "^[\w\d]*$",
-            "help_text": "Username should only contain word characters & numbers i.e datatester23",
+            "regex": r"^(?!\d+$)[a-zA-Z0-9_]{3,}$",
+            "help_text": "Username should only contain word characters & numbers and should have 3 or more characters",
         },
     },
 }
@@ -83,6 +85,82 @@ def test_returns_data_entry_template_on_missing_username_claim(self):
             self.assertEqual(response.status_code, 200)
             self.assertEqual(response.template_name, "oidc/oidc_user_data_entry.html")
 
+    @override_settings(OPENID_CONNECT_VIEWSET_CONFIG=OPENID_CONNECT_VIEWSET_CONFIG)
+    def test_user_created_successfully_when_email_has_a_valid_username(self):
+        """
+        Test that the user is created ok when
+        username is not present in decoded token but email has a valid username
+        """
+        view = UserModelOpenIDConnectViewset.as_view({"post": "callback"})
+        with patch(
+            "oidc.viewsets.OpenIDClient.verify_and_decode_id_token"
+        ) as mock_func:
+            mock_func.return_value = {
+                "family_name": "bob",
+                "given_name": "just bob",
+                "username": "[email protected]",
+                "email": "[email protected]",
+            }
+
+            data = {"id_token": "sadsdaio3209lkasdlkas0d.sdojdsiad.iosdadia"}
+            request = self.factory.post("/", data=data)
+            response = view(request, auth_server="default")
+            self.assertEqual(response.status_code, 302)
+            user = User.objects.get(username="boby")
+            self.assertEqual(user.email, "[email protected]")
+
+    @override_settings(OPENID_CONNECT_VIEWSET_CONFIG=OPENID_CONNECT_VIEWSET_CONFIG)
+    def test_returns_data_entry_template_on_invalid_username(self):
+        """
+        Test that users are redirected to the data entry
+        page when username is not present in decoded token and
+        provided email also does not provide a valid username
+        """
+        view = UserModelOpenIDConnectViewset.as_view({"post": "callback"})
+        with patch(
+            "oidc.viewsets.OpenIDClient.verify_and_decode_id_token"
+        ) as mock_func:
+            mock_func.return_value = {
+                "family_name": "bob",
+                "given_name": "just bob",
+                "email": "[email protected]",
+            }
+
+            data = {"id_token": "sadsdaio3209lkasdlkas0d.sdojdsiad.iosdadia"}
+            request = self.factory.post("/", data=data)
+            response = view(request, auth_server="default")
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.template_name, "oidc/oidc_user_data_entry.html")
+
+    @override_settings(OPENID_CONNECT_VIEWSET_CONFIG=OPENID_CONNECT_VIEWSET_CONFIG)
+    def test_returns_data_entry_template_on_invalid_username_and_bad_email(self):
+        """
+        Test that users are redirected to the data entry
+        page when username provided in decoded token is invalid and
+        provided email also does not provide a valid username
+        """
+        view = UserModelOpenIDConnectViewset.as_view({"post": "callback"})
+        with patch(
+            "oidc.viewsets.OpenIDClient.verify_and_decode_id_token"
+        ) as mock_func:
+            mock_func.return_value = {
+                "family_name": "bob",
+                "given_name": "just bob",
+                "username": "[email protected]",
+                "email": "[email protected]",
+            }
+
+            data = {"id_token": "sadsdaio3209lkasdlkas0d.sdojdsiad.iosdadia"}
+            request = self.factory.post("/", data=data)
+            response = view(request, auth_server="default")
+            self.assertEqual(response.status_code, 400)
+            self.assertTrue(
+                response.rendered_content.startswith(
+                    b'{"error":"Username should only contain word characters & numbers and should have 3 or more characters"'
+                )
+            )
+            self.assertEqual(response.template_name, "oidc/oidc_user_data_entry.html")
+
     def test_unrecoverable_error_on_missing_claim(self):
         """
         Test that an error is returned when a required claim field other than the

tox.ini

@@ -1,28 +1,30 @@
 [tox]
 envlist =
-    py{38,39}-django{32,40}
+    py{310,311,312}-django{32,40}
     lint
 
 [testenv:lint]
 deps =
     pipenv
     flake8
     black
-basepython = python3.9
+basepython = python3.10
 commands =
     pipenv sync --dev
     flake8 {toxinidir}/oidc
-    black -v {toxinidir}/oidc --check -t py38 -t py39
+    black -v {toxinidir}/oidc --check -t py310 -t py311
     isort -c -v {toxinidir}/oidc
 
 [testenv]
 deps =
     pipenv
 basepython =
-    py38: python3.8
-    py39: python3.9
+    py310: python3.10
+    py311: python3.11
+    py312: python3.12
 commands =
     django32: pip install Django>=3.2.13,<4
     django40: pip install Django>=4,<5
     pipenv sync --dev
     python manage.py test {toxinidir}/tests
+