From be55ebfdbd4725e0acf967618fe7a965c71b5b98 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Fri, 8 Mar 2024 09:11:36 +0300 Subject: [PATCH 1/5] Add Username Hyphen Validation on Django Admin Signed-off-by: Kipchirchir Sigei --- onadata/apps/main/admin.py | 61 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 onadata/apps/main/admin.py diff --git a/onadata/apps/main/admin.py b/onadata/apps/main/admin.py new file mode 100644 index 0000000000..4cee275952 --- /dev/null +++ b/onadata/apps/main/admin.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +"""API Django admin amendments.""" +# pylint: disable=imported-auth-user +from django.contrib import admin +from django.contrib.auth.admin import UserAdmin +from django.contrib.auth.forms import UserCreationForm, UserChangeForm +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError + + +class BaseCustomUserForm: + """ + Base form class for custom user forms. + Contains common logic for validating the username. + """ + + def clean_username(self): + """ + Clean the username field to ensure it does not contain hyphens. + Raises: + ValidationError: If the username contains hyphens. + Returns: + str: The cleaned username. + """ + username = self.cleaned_data["username"] + if "-" in username: + raise ValidationError("Usernames cannot contain hyphens.") + return username + + +class CustomUserCreationForm(BaseCustomUserForm, UserCreationForm): + """ + Custom form for user creation. + Inherits from BaseCustomUserForm and UserCreationForm. + """ + + class Meta(UserCreationForm.Meta): + model = User + + +class CustomUserChangeForm(BaseCustomUserForm, UserChangeForm): + """ + Custom form for user change. + Inherits from BaseCustomUserForm and UserChangeForm. + """ + + class Meta(UserChangeForm.Meta): + model = User + + +class CustomUserAdmin(UserAdmin): + """ + Custom User admin panel configuration. + """ + + add_form = CustomUserCreationForm + form = CustomUserChangeForm + + +admin.site.unregister(User) +admin.site.register(User, CustomUserAdmin) From 7d8bee7461e5f78e9e54e9e58fd20ca9bec62bd8 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Fri, 8 Mar 2024 10:43:20 +0300 Subject: [PATCH 2/5] Add tests Signed-off-by: Kipchirchir Sigei --- .../apps/main/tests/test_user_validation.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 onadata/apps/main/tests/test_user_validation.py diff --git a/onadata/apps/main/tests/test_user_validation.py b/onadata/apps/main/tests/test_user_validation.py new file mode 100644 index 0000000000..7b2066974e --- /dev/null +++ b/onadata/apps/main/tests/test_user_validation.py @@ -0,0 +1,27 @@ +from onadata.apps.main.admin import CustomUserChangeForm, CustomUserCreationForm +from onadata.apps.main.tests.test_base import TestBase + + +class TestUserValidation(TestBase): + def test_custom_user_creation_form_invalid_username(self): + # Try to create a user with a hyphenated username + form_data = { + "username": "john-doe", + "password1": "testpassword", + "password2": "testpassword", + } + form = CustomUserCreationForm(data=form_data) + self.assertFalse(form.is_valid()) + self.assertIn("username", form.errors) + errors = form.errors.get("username")[0] + self.assertEqual(str(errors), "Usernames cannot contain hyphens.") + + def test_custom_user_change_form_invalid_username(self): + # Try to change a user's username to one with a hyphen + user = self._create_user("bob-user", "bob") + form_data = {"username": "bob-user-1"} + form = CustomUserChangeForm(data=form_data, instance=user) + self.assertFalse(form.is_valid()) + self.assertIn("username", form.errors) + errors = form.errors.get("username")[0] + self.assertEqual(str(errors), "Usernames cannot contain hyphens.") From 4af0b2fa7b71b7805d31dcf84245a2a845984287 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Thu, 14 Mar 2024 09:16:44 +0300 Subject: [PATCH 3/5] Update username regex in routes Signed-off-by: Kipchirchir Sigei --- onadata/apps/main/urls.py | 84 +++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index 57ecdbfa9e..ff9c4dfac9 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -197,7 +197,7 @@ ), # briefcase api urls re_path( - r"^(?P\w+)/view/submissionList$", + r"^(?P[^/]+)/view/submissionList$", BriefcaseViewset.as_view({"get": "list", "head": "list"}), name="view-submission-list", ), @@ -212,7 +212,7 @@ name="view-submission-list", ), re_path( - r"^(?P\w+)/view/downloadSubmission$", + r"^(?P[^/]+)/view/downloadSubmission$", BriefcaseViewset.as_view({"get": "retrieve", "head": "retrieve"}), name="view-download-submission", ), @@ -227,117 +227,117 @@ name="view-download-submission", ), re_path( - r"^(?P\w+)/formUpload$", + r"^(?P[^/]+)/formUpload$", BriefcaseViewset.as_view({"post": "create", "head": "create"}), name="form-upload", ), re_path( - r"^(?P\w+)/upload$", + r"^(?P[^/]+)/upload$", BriefcaseViewset.as_view({"post": "create", "head": "create"}), name="upload", ), # exporting stuff re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.csv$", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.csv$", viewer_views.data_export, name="csv_export", kwargs={"export_type": "csv"}, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.xls", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.xls", viewer_views.data_export, name="xlsx_export", kwargs={"export_type": "xlsx"}, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.csv.zip", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.csv.zip", viewer_views.data_export, name="csv_zip_export", kwargs={"export_type": "csv_zip"}, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.sav.zip", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.sav.zip", viewer_views.data_export, name="sav_zip_export", kwargs={"export_type": "sav_zip"}, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.kml$", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.kml$", viewer_views.kml_export, name="kml-export", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/data\.zip", + r"^(?P[^/]+)/forms/(?P[^/]+)/data\.zip", viewer_views.zip_export, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/gdocs$", + r"^(?P[^/]+)/forms/(?P[^/]+)/gdocs$", viewer_views.google_xlsx_export, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/map_embed", + r"^(?P[^/]+)/forms/(?P[^/]+)/map_embed", viewer_views.map_embed_view, ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/map", + r"^(?P[^/]+)/forms/(?P[^/]+)/map", viewer_views.map_view, name="map-view", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/instance", + r"^(?P[^/]+)/forms/(?P[^/]+)/instance", viewer_views.instance, name="submission-instance", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/enter-data", + r"^(?P[^/]+)/forms/(?P[^/]+)/enter-data", logger_views.enter_data, name="enter_data", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/add-submission-with", # noqa + r"^(?P[^/]+)/forms/(?P[^/]+)/add-submission-with", # noqa viewer_views.add_submission_with, name="add_submission_with", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/thank_you_submission", # noqa + r"^(?P[^/]+)/forms/(?P[^/]+)/thank_you_submission", # noqa viewer_views.thank_you_submission, name="thank_you_submission", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/edit-data/(?P\d+)$", # noqa + r"^(?P[^/]+)/forms/(?P[^/]+)/edit-data/(?P\d+)$", # noqa logger_views.edit_data, name="edit_data", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/view-data", + r"^(?P[^/]+)/forms/(?P[^/]+)/view-data", viewer_views.data_view, name="data-view", ), re_path( - r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)/new$", # noqa + r"^(?P[^/]+)/exports/(?P[^/]+)/(?P\w+)/new$", # noqa viewer_views.create_export, name="new-export", ), re_path( - r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" # noqa + r"^(?P[^/]+)/exports/(?P[^/]+)/(?P\w+)" # noqa "/delete$", viewer_views.delete_export, name="delete-export", ), re_path( - r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" # noqa + r"^(?P[^/]+)/exports/(?P[^/]+)/(?P\w+)" # noqa "/progress$", viewer_views.export_progress, name="export-progress", ), re_path( - r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" # noqa + r"^(?P[^/]+)/exports/(?P[^/]+)/(?P\w+)" # noqa "/$", viewer_views.export_list, name="export-list", ), re_path( - r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" # noqa + r"^(?P[^/]+)/exports/(?P[^/]+)/(?P\w+)" # noqa "/(?P[^/]+)$", viewer_views.export_download, name="export-download", @@ -366,7 +366,7 @@ name="form-list", ), re_path( - r"^(?P\w+)/formList$", + r"^(?P[^/]+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), name="form-list", ), @@ -391,22 +391,22 @@ name="form-list", ), re_path( - r"^(?P\w+)/(?P\d+)/formList$", + r"^(?P[^/]+)/(?P\d+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), name="form-list", ), re_path( - r"^preview/(?P\w+)/(?P\d+)/formList$", + r"^preview/(?P[^/]+)/(?P\d+)/formList$", PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}), name="form-list", ), re_path( - r"^preview/(?P\w+)/formList$", + r"^preview/(?P[^/]+)/formList$", PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}), name="form-list", ), re_path( - r"^(?P\w+)/xformsManifest/(?P[\d+^/]+)$", + r"^(?P[^/]+)/xformsManifest/(?P[\d+^/]+)$", XFormListViewSet.as_view({"get": "manifest", "head": "manifest"}), name="manifest-url", ), @@ -416,13 +416,13 @@ name="manifest-url", ), re_path( - r"^(?P\w+)/xformsMedia/(?P[\d+^/]+)/(?P[\d+^/.]+)$", # noqa + r"^(?P[^/]+)/xformsMedia/(?P[\d+^/]+)/(?P[\d+^/.]+)$", # noqa XFormListViewSet.as_view({"get": "media", "head": "media"}), name="xform-media", ), re_path( # pylint: disable=line-too-long - r"^(?P\w+)/xformsMedia/(?P[\d+^/]+)/(?P[\d+^/.]+)\.(?P([a-z]|[0-9])*)$", # noqa + r"^(?P[^/]+)/xformsMedia/(?P[\d+^/]+)/(?P[\d+^/.]+)\.(?P([a-z]|[0-9])*)$", # noqa XFormListViewSet.as_view({"get": "media", "head": "media"}), name="xform-media", ), @@ -438,7 +438,7 @@ name="xform-media", ), re_path( - r"^(?P\w+)/submission$", + r"^(?P[^/]+)/submission$", XFormSubmissionViewSet.as_view({"post": "create", "head": "create"}), name="submissions", ), @@ -458,41 +458,41 @@ name="submissions", ), re_path( - r"^(?P\w+)/(?P\d+)/submission$", + r"^(?P[^/]+)/(?P\d+)/submission$", XFormSubmissionViewSet.as_view({"post": "create", "head": "create"}), name="submissions", ), - re_path(r"^(?P\w+)/bulk-submission$", logger_views.bulksubmission), + re_path(r"^(?P[^/]+)/bulk-submission$", logger_views.bulksubmission), re_path( - r"^(?P\w+)/bulk-submission-form$", logger_views.bulksubmission_form + r"^(?P[^/]+)/bulk-submission-form$", logger_views.bulksubmission_form ), re_path( - r"^(?P\w+)/forms/(?P[\d+^/]+)/form\.xml$", + r"^(?P[^/]+)/forms/(?P[\d+^/]+)/form\.xml$", XFormListViewSet.as_view({"get": "retrieve", "head": "retrieve"}), name="download_xform", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/form\.xml$", + r"^(?P[^/]+)/forms/(?P[^/]+)/form\.xml$", logger_views.download_xform, name="download_xform", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/form\.xls$", + r"^(?P[^/]+)/forms/(?P[^/]+)/form\.xls$", logger_views.download_xlsform, name="download_xlsform", ), re_path( - r"^(?P\w+)/forms/(?P[^/]+)/form\.json", + r"^(?P[^/]+)/forms/(?P[^/]+)/form\.json", logger_views.download_jsonform, name="download_jsonform", ), re_path( - r"^(?P\w+)/delete/(?P[^/]+)/$", + r"^(?P[^/]+)/delete/(?P[^/]+)/$", logger_views.delete_xform, name="delete-xform", ), re_path( - r"^(?P\w+)/(?P[^/]+)/toggle_downloadable/$", + r"^(?P[^/]+)/(?P[^/]+)/toggle_downloadable/$", logger_views.toggle_downloadable, name="toggle-downloadable", ), @@ -530,7 +530,7 @@ ), # Stats tables re_path( - r"^(?P\w+)/forms/(?P[^/]+)/tables", + r"^(?P[^/]+)/forms/(?P[^/]+)/tables", viewer_views.stats_tables, name="stats-tables", ), From d2897494d2dc04d17474a0b126fe5e25870a5cf3 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Thu, 14 Mar 2024 16:24:42 +0300 Subject: [PATCH 4/5] Cleanup Signed-off-by: Kipchirchir Sigei --- onadata/apps/main/admin.py | 61 ------------------- .../apps/main/tests/test_user_validation.py | 27 -------- 2 files changed, 88 deletions(-) delete mode 100644 onadata/apps/main/admin.py delete mode 100644 onadata/apps/main/tests/test_user_validation.py diff --git a/onadata/apps/main/admin.py b/onadata/apps/main/admin.py deleted file mode 100644 index 4cee275952..0000000000 --- a/onadata/apps/main/admin.py +++ /dev/null @@ -1,61 +0,0 @@ -# -*- coding: utf-8 -*- -"""API Django admin amendments.""" -# pylint: disable=imported-auth-user -from django.contrib import admin -from django.contrib.auth.admin import UserAdmin -from django.contrib.auth.forms import UserCreationForm, UserChangeForm -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError - - -class BaseCustomUserForm: - """ - Base form class for custom user forms. - Contains common logic for validating the username. - """ - - def clean_username(self): - """ - Clean the username field to ensure it does not contain hyphens. - Raises: - ValidationError: If the username contains hyphens. - Returns: - str: The cleaned username. - """ - username = self.cleaned_data["username"] - if "-" in username: - raise ValidationError("Usernames cannot contain hyphens.") - return username - - -class CustomUserCreationForm(BaseCustomUserForm, UserCreationForm): - """ - Custom form for user creation. - Inherits from BaseCustomUserForm and UserCreationForm. - """ - - class Meta(UserCreationForm.Meta): - model = User - - -class CustomUserChangeForm(BaseCustomUserForm, UserChangeForm): - """ - Custom form for user change. - Inherits from BaseCustomUserForm and UserChangeForm. - """ - - class Meta(UserChangeForm.Meta): - model = User - - -class CustomUserAdmin(UserAdmin): - """ - Custom User admin panel configuration. - """ - - add_form = CustomUserCreationForm - form = CustomUserChangeForm - - -admin.site.unregister(User) -admin.site.register(User, CustomUserAdmin) diff --git a/onadata/apps/main/tests/test_user_validation.py b/onadata/apps/main/tests/test_user_validation.py deleted file mode 100644 index 7b2066974e..0000000000 --- a/onadata/apps/main/tests/test_user_validation.py +++ /dev/null @@ -1,27 +0,0 @@ -from onadata.apps.main.admin import CustomUserChangeForm, CustomUserCreationForm -from onadata.apps.main.tests.test_base import TestBase - - -class TestUserValidation(TestBase): - def test_custom_user_creation_form_invalid_username(self): - # Try to create a user with a hyphenated username - form_data = { - "username": "john-doe", - "password1": "testpassword", - "password2": "testpassword", - } - form = CustomUserCreationForm(data=form_data) - self.assertFalse(form.is_valid()) - self.assertIn("username", form.errors) - errors = form.errors.get("username")[0] - self.assertEqual(str(errors), "Usernames cannot contain hyphens.") - - def test_custom_user_change_form_invalid_username(self): - # Try to change a user's username to one with a hyphen - user = self._create_user("bob-user", "bob") - form_data = {"username": "bob-user-1"} - form = CustomUserChangeForm(data=form_data, instance=user) - self.assertFalse(form.is_valid()) - self.assertIn("username", form.errors) - errors = form.errors.get("username")[0] - self.assertEqual(str(errors), "Usernames cannot contain hyphens.") From 03faa78ad8e6213824cda7be20366780dbf45bf9 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Tue, 26 Mar 2024 09:17:00 +0300 Subject: [PATCH 5/5] Add tests Signed-off-by: Kipchirchir Sigei --- .../apps/main/tests/test_username_patterns.py | 68 +++++++++++++++++++ onadata/apps/main/urls.py | 16 ++--- 2 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 onadata/apps/main/tests/test_username_patterns.py diff --git a/onadata/apps/main/tests/test_username_patterns.py b/onadata/apps/main/tests/test_username_patterns.py new file mode 100644 index 0000000000..1c1d37e714 --- /dev/null +++ b/onadata/apps/main/tests/test_username_patterns.py @@ -0,0 +1,68 @@ +# import re +# from django.test import RequestFactory, TestCase +# from django.urls import resolve, reverse + +# from django_digest.test import Client as DigestClient +# from django_digest.test import DigestAuth + +# from onadata.apps.main.tests.test_base import TestBase +# from onadata.libs.permissions import DataEntryRole + +# class TestUsernamePatterns(TestBase): +# def setUp(self): +# super().setUp() + +# def test_submission_list_view(self): +# # Define test data +# usernames = ["test_user", "user-with-hyphen", "user.with.dot"] +# id_string = "sample_id_string" + +# # Define the URL pattern name +# url_name = "form-list-username" + +# # Iterate over usernames +# for username in usernames: +# # Generate the URL with parameters +# url = reverse(url_name, kwargs={'username': username}) + +# # Check if the URL matches the expected pattern and resolves to the correct view +# resolved = resolve(url) +# import ipdb; ipdb.set_trace() + +# # Check if the resolved view has the correct resolver_match attribute +# self.assertEqual(resolved.url_name, "form-list-username") +# self.assertEqual(resolved.kwargs, {'username': username}) + + +import re +from django.test import SimpleTestCase +from django.urls import resolve + +class TestRoutePatternMatching(SimpleTestCase): + def test_route_pattern_matching(self): + # Define test data + urls = [ + "/test_user/view/submissionList", + "/forms/example_xform_pk/view/submissionList", + "/projects/123/view/submissionList" + ] + + # Define the expected regular expressions for route patterns + patterns = [ + r"^(?P[\w.@-]+)/view/submissionList$", + r"^forms/(?P\w+)/view/submissionList$", + r"^projects/(?P\d+)/view/submissionList$" + ] + + # Loop through each URL and corresponding pattern + for url, pattern in zip(urls, patterns): + # Resolve the URL and get the route pattern + resolved = resolve(url) + route_pattern = resolved.route + import ipdb; ipdb.set_trace() + + # Test if the route pattern matches the expected regular expression + with self.subTest(url=url, pattern=pattern): + self.assertIsNotNone(re.match(pattern, url), f"The route pattern '{route_pattern}' does not match the expected regular expression '{pattern}'") + + diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index ff9c4dfac9..c184b3c8c8 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -368,42 +368,42 @@ re_path( r"^(?P[^/]+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-username", ), re_path( r"^projects/(?P\d+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-projects", ), re_path( r"^enketo/(?P\w+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-enketo", ), re_path( r"^forms/(?P\w+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-forms", ), re_path( r"^enketo-preview/(?P\w+)/formList$", PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-enketo-preview", ), re_path( r"^(?P[^/]+)/(?P\d+)/formList$", XFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-username-xform", ), re_path( r"^preview/(?P[^/]+)/(?P\d+)/formList$", PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-preview-xform", ), re_path( r"^preview/(?P[^/]+)/formList$", PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}), - name="form-list", + name="form-list-preview-username", ), re_path( r"^(?P[^/]+)/xformsManifest/(?P[\d+^/]+)$",