Skip to content

Commit

Permalink
Merge pull request #68 from onaio/update-username-regex-and-more
Browse files Browse the repository at this point in the history
Update username regex and more
  • Loading branch information
FrankApiyo authored Sep 19, 2024
2 parents 25aaea5 + 80c2708 commit 7577b15
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 27 deletions.
18 changes: 9 additions & 9 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
},
Expand Down
4 changes: 2 additions & 2 deletions oidc/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "-.",
Expand Down
22 changes: 14 additions & 8 deletions oidc/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
82 changes: 80 additions & 2 deletions tests/test_viewsets.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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",
},
},
}
Expand Down Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7577b15

Please sign in to comment.