Fix underscore in usernames handling (#1625)

* WIP update tests to use username with underscore * Rebuild js * Update validators * Rename OtherUser to other_user in tests * Format code with black * Fix username validator
rafalp · May 28, 2023 · 88fbff9 · 88fbff9
1 parent 9dcfc8f
commit 88fbff9
Show file tree

Hide file tree

Showing 39 changed files with 165 additions and 103 deletions.
diff --git a/frontend/src/utils/validators.js b/frontend/src/utils/validators.js
@@ -1,10 +1,11 @@
 const EMAIL =
   /^(([^<>()[\]\.,;:\s@\"]+(\.[^<>()[\]\.,;:\s@\"]+)*)|(\".+\"))@(([^<>()[\]\.,;:\s@\"]+\.)+[^<>()[\]\.,;:\s@\"]{2,})$/i
-const USERNAME = new RegExp("^[0-9a-z]+$", "i")
+const USERNAME = new RegExp("^[0-9a-z_]+$", "i")
+const USERNAME_ALPHANUMERIC = new RegExp("[0-9a-z]", "i")
 
 export function required(message) {
   return function (value) {
-    if (value === false || value === null || $.trim(value).length === 0) {
+    if (value === false || value === null || value.trim().length === 0) {
       return message || gettext("This field is required.")
     }
   }
@@ -31,7 +32,7 @@ export function email(message) {
 export function minLength(limitValue, message) {
   return function (value) {
     var returnMessage = ""
-    var length = $.trim(value).length
+    var length = value.trim().length
 
     if (length < limitValue) {
       if (message) {
@@ -58,7 +59,7 @@ export function minLength(limitValue, message) {
 export function maxLength(limitValue, message) {
   return function (value) {
     var returnMessage = ""
-    var length = $.trim(value).length
+    var length = value.trim().length
 
     if (length > limitValue) {
       if (message) {
@@ -106,9 +107,15 @@ export function usernameMaxLength(lengthMax) {
 
 export function usernameContent() {
   return function (value) {
-    if (!USERNAME.test($.trim(value))) {
+    const valueTrimmed = value.trim()
+    if (!USERNAME.test(valueTrimmed)) {
       return gettext(
-        "Username can only contain latin alphabet letters and digits."
+        "Username can only contain Latin alphabet letters, digits, and an underscore sign."
+      )
+    }
+    if (!USERNAME_ALPHANUMERIC.test(valueTrimmed)) {
+      return gettext(
+        "Username can must contain Latin alphabet letters or digits."
       )
     }
   }

diff --git a/misago/conftest.py b/misago/conftest.py
@@ -73,7 +73,7 @@ def user_acl(user, cache_versions):
 
 @pytest.fixture
 def other_user(db, user_password):
-    return create_test_user("OtherUser", "otheruser@example.com", user_password)
+    return create_test_user("Other_User", "otheruser@example.com", user_password)
 
 
 @pytest.fixture
@@ -83,7 +83,7 @@ def other_user_acl(other_user, cache_versions):
 
 @pytest.fixture
 def staffuser(db, user_password):
-    user = create_test_superuser("Staffuser", "staffuser@example.com", user_password)
+    user = create_test_superuser("Staff_User", "staffuser@example.com", user_password)
     user.is_superuser = False
     user.save()
     return user
@@ -97,7 +97,7 @@ def staffuser_acl(staffuser, cache_versions):
 @pytest.fixture
 def other_staffuser(db, user_password):
     user = create_test_superuser(
-        "OtherStaffuser", "otherstaffuser@example.com", user_password
+        "Other_Staff_User", "otherstaffuser@example.com", user_password
     )
 
     user.is_superuser = False
@@ -107,7 +107,7 @@ def other_staffuser(db, user_password):
 
 @pytest.fixture
 def superuser(db, user_password):
-    return create_test_superuser("Superuser", "superuser@example.com", user_password)
+    return create_test_superuser("Super_User", "superuser@example.com", user_password)
 
 
 @pytest.fixture
@@ -118,14 +118,14 @@ def superuser_acl(superuser, cache_versions):
 @pytest.fixture
 def other_superuser(db, user_password):
     return create_test_superuser(
-        "OtherSuperuser", "othersuperuser@example.com", user_password
+        "OtherSuperUser", "othersuperuser@example.com", user_password
     )
 
 
 @pytest.fixture
 def inactive_user(db, user_password):
     return create_test_user(
-        "InactiveUser", "inactiveuser@example.com", user_password, is_active=False
+        "Inactive_User", "inactiveuser@example.com", user_password, is_active=False
     )
 
 

diff --git a/misago/markup/mentions.py b/misago/markup/mentions.py
@@ -3,22 +3,23 @@
 
 from django.contrib.auth import get_user_model
 
+from ..users.utils import slugify_username
 from .htmlparser import (
     ElementNode,
     RootNode,
     TextNode,
 )
 
 EXCLUDE_ELEMENTS = ("pre", "code", "a")
-USERNAME_RE = re.compile(r"@[0-9a-z]+", re.IGNORECASE)
+USERNAME_RE = re.compile(r"@[0-9a-z_]+", re.IGNORECASE)
 MENTIONS_LIMIT = 32
 
 
 def add_mentions(result, root_node):
     if "@" not in result["parsed_text"]:
         return
 
-    mentions = set()
+    mentions = set()  # usernames slugs
     nodes = []
 
     find_mentions(root_node, mentions, nodes)
@@ -62,7 +63,7 @@ def find_mentions_in_str(text: str):
     if not matches:
         return None
 
-    return set([match.lower()[1:] for match in matches])
+    return set([slugify_username(match[1:]) for match in matches])
 
 
 def get_users_data(mentions):
@@ -102,7 +103,7 @@ def add_mentions_to_text(text: str, users_data):
             return nodes
 
         start, end = match.span()
-        user_slug = text[start + 1 : end].lower()
+        user_slug = text[start + 1 : end].lower().replace("_", "-")
 
         # Append text between 0 and start to nodes
         if start > 0:

diff --git a/misago/markup/tests/test_mentions.py b/misago/markup/tests/test_mentions.py
@@ -26,13 +26,13 @@ def test_util_adds_mention_to_parsig_result(user):
 
 
 def test_mentions_arent_added_for_nonexisting_user(user):
-    parsing_result = {"parsed_text": f"<p>Hello, @OtherUser!</p>", "mentions": []}
+    parsing_result = {"parsed_text": f"<p>Hello, @Aerith!</p>", "mentions": []}
     root_node = parse_html_string(parsing_result["parsed_text"])
 
     add_mentions(parsing_result, root_node)
 
     parsing_result["parsed_text"] = print_html_string(root_node)
-    assert parsing_result["parsed_text"] == "<p>Hello, @OtherUser!</p>"
+    assert parsing_result["parsed_text"] == "<p>Hello, @Aerith!</p>"
 
 
 def test_util_replaces_multiple_mentions_with_link_to_user_profiles_in_parsed_text(

diff --git a/misago/oauth2/tests/test_user_data_validation.py b/misago/oauth2/tests/test_user_data_validation.py
@@ -73,7 +73,10 @@ def test_error_was_raised_for_user_data_with_without_name(db, dynamic_settings):
             )
 
     assert excinfo.value.error_list == [
-        "Username can only contain latin alphabet letters and digits."
+        (
+            "Username can only contain Latin alphabet letters, digits, "
+            "and an underscore sign."
+        )
     ]
 
 
@@ -97,7 +100,10 @@ def test_error_was_raised_for_user_data_with_invalid_name(db, dynamic_settings):
             )
 
     assert excinfo.value.error_list == [
-        "Username can only contain latin alphabet letters and digits."
+        (
+            "Username can only contain Latin alphabet letters, digits, "
+            "and an underscore sign."
+        )
     ]
 
 

diff --git a/misago/static/misago/js/misago.js b/misago/static/misago/js/misago.js
diff --git a/misago/static/misago/js/misago.js.map b/misago/static/misago/js/misago.js.map
diff --git a/misago/threads/api/postingendpoint/participants.py b/misago/threads/api/postingendpoint/participants.py
@@ -7,6 +7,7 @@
 from . import PostingEndpoint, PostingMiddleware
 from ....acl import useracl
 from ....categories import PRIVATE_THREADS_ROOT_NAME
+from ....users.utils import slugify_username
 from ...participants import add_participants, set_owner
 from ...permissions import allow_message_user
 
@@ -44,7 +45,7 @@ def validate_to(self, usernames):
     def clean_usernames(self, usernames):
         clean_usernames = []
         for name in usernames:
-            clean_name = name.strip().lower()
+            clean_name = slugify_username(name)
 
             if clean_name == self.context["user"].slug:
                 raise serializers.ValidationError(

diff --git a/misago/threads/api/threadendpoints/patch.py b/misago/threads/api/threadendpoints/patch.py
@@ -14,6 +14,7 @@
 from ....conf import settings
 from ....core.apipatch import ApiPatch
 from ....core.shortcuts import get_int_or_404
+from ....users.utils import slugify_username
 from ...moderation import threads as moderation
 from ...participants import (
     add_participant,
@@ -270,16 +271,16 @@ def patch_unmark_best_answer(request, thread, value):
 thread_patch_dispatcher.remove("best-answer", patch_unmark_best_answer)
 
 
-def patch_add_participant(request, thread, value):
+def patch_add_participant(request, thread, value: str):
     allow_add_participants(request.user_acl, thread)
 
     try:
-        username = str(value).strip().lower()
-        if not username:
+        user_slug = slugify_username(value)
+        if not user_slug:
             raise PermissionDenied(_("You have to enter new participant's username."))
-        participant = User.objects.get(slug=username)
+        participant = User.objects.get(slug=user_slug)
     except User.DoesNotExist:
-        raise PermissionDenied(_("No user with such name exists."))
+        raise PermissionDenied(_("No user with this name exists."))
 
     if participant in [p.user for p in thread.participants_list]:
         raise PermissionDenied(_("This user is already thread participant."))

diff --git a/misago/threads/test.py b/misago/threads/test.py
@@ -39,7 +39,7 @@ def patch_acl(_, user_acl):
 
 def patch_other_user_category_acl(acl_patch=None):
     def patch_acl(user, user_acl):
-        if user.slug != "otheruser":
+        if user.slug != "other_user":
             return
 
         category = Category.objects.get(slug="first-category")
@@ -82,7 +82,7 @@ def patch_acl(_, user_acl):
 
 
 def other_user_cant_use_private_threads(user, user_acl):
-    if user.slug == "otheruser":
+    if user.slug == "other-user":
         user_acl.update({"can_use_private_threads": False})
 
 

diff --git a/misago/threads/tests/test_anonymize_data.py b/misago/threads/tests/test_anonymize_data.py
@@ -40,7 +40,7 @@ def get_request(self, user=None):
 
     def test_anonymize_changed_owner_event(self):
         """changed owner event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request()
 
         set_owner(self.thread, self.user)
@@ -64,7 +64,7 @@ def test_anonymize_changed_owner_event(self):
     @patch("misago.threads.participants.notify_on_new_private_thread")
     def test_anonymize_added_participant_event(self, notify_on_new_private_thread_mock):
         """added participant event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request()
 
         set_owner(self.thread, self.user)
@@ -88,7 +88,7 @@ def test_anonymize_added_participant_event(self, notify_on_new_private_thread_mo
     @patch("misago.threads.participants.notify_on_new_private_thread")
     def test_anonymize_owner_left_event(self, notify_on_new_private_thread_mock):
         """owner left event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request(user)
 
         set_owner(self.thread, user)
@@ -115,7 +115,7 @@ def test_anonymize_owner_left_event(self, notify_on_new_private_thread_mock):
     @patch("misago.threads.participants.notify_on_new_private_thread")
     def test_anonymize_removed_owner_event(self, notify_on_new_private_thread_mock):
         """removed owner event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request()
 
         set_owner(self.thread, user)
@@ -141,7 +141,7 @@ def test_anonymize_removed_owner_event(self, notify_on_new_private_thread_mock):
 
     def test_anonymize_participant_left_event(self):
         """participant left event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request(user)
 
         set_owner(self.thread, self.user)
@@ -170,7 +170,7 @@ def test_anonymize_removed_participant_event(
         self, notify_on_new_private_thread_mock
     ):
         """removed participant event is anonymized by user.anonymize_data"""
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         request = self.get_request()
 
         set_owner(self.thread, self.user)
@@ -214,7 +214,7 @@ def test_anonymize_user_likes(self):
         post = test.reply_thread(thread)
         post.acl = {"can_like": True}
 
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
 
         patch_is_liked(self.get_request(self.user), post, 1)
         patch_is_liked(self.get_request(user), post, 1)
@@ -248,7 +248,7 @@ def test_anonymize_user_posts(self):
         category = Category.objects.get(slug="first-category")
         thread = test.post_thread(category)
 
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
         post = test.reply_thread(thread, poster=user)
         user.anonymize_data(anonymous_username="Deleted")
 

diff --git a/misago/threads/tests/test_delete_user_likes.py b/misago/threads/tests/test_delete_user_likes.py
@@ -26,7 +26,7 @@ def test_anonymize_user_likes(self):
         post = test.reply_thread(thread)
         post.acl = {"can_like": True}
 
-        user = create_test_user("OtherUser", "otheruser@example.com")
+        user = create_test_user("Other_User", "otheruser@example.com")
 
         patch_is_liked(self.get_request(self.user), post, 1)
         patch_is_liked(self.get_request(user), post, 1)

diff --git a/misago/threads/tests/test_participants.py b/misago/threads/tests/test_participants.py
@@ -50,7 +50,7 @@ def test_has_participants(self):
         """has_participants returns true if thread has participants"""
         users = [
             create_test_user("User", "user@example.com"),
-            create_test_user("OtherUser", "otheruser@example.com"),
+            create_test_user("Other_User", "otheruser@example.com"),
         ]
 
         self.assertFalse(has_participants(self.thread))
@@ -67,7 +67,7 @@ def test_make_threads_participants_aware(self):
         annotations on list of threads
         """
         user = create_test_user("User", "user@example.com")
-        other_user = create_test_user("OtherUser", "otheruser@example.com")
+        other_user = create_test_user("Other_User", "otheruser@example.com")
 
         self.assertFalse(hasattr(self.thread, "participants_list"))
         self.assertFalse(hasattr(self.thread, "participant"))
@@ -92,7 +92,7 @@ def test_make_thread_participants_aware(self):
         annotations on thread model
         """
         user = create_test_user("User", "user@example.com")
-        other_user = create_test_user("OtherUser", "otheruser@example.com")
+        other_user = create_test_user("Other_User", "otheruser@example.com")
 
         self.assertFalse(hasattr(self.thread, "participants_list"))
         self.assertFalse(hasattr(self.thread, "participant"))
@@ -133,7 +133,7 @@ def test_set_users_unread_private_threads_sync(self):
         """
         users = [
             create_test_user("User", "user@example.com"),
-            create_test_user("OtherUser", "otheruser@example.com"),
+            create_test_user("Other_User", "otheruser@example.com"),
         ]
 
         set_users_unread_private_threads_sync(users=users)
@@ -148,7 +148,7 @@ def test_set_participants_unread_private_threads_sync(self):
         """
         users = [
             create_test_user("User", "user@example.com"),
-            create_test_user("OtherUser", "otheruser@example.com"),
+            create_test_user("Other_User", "otheruser@example.com"),
         ]
 
         participants = [ThreadParticipant(user=u) for u in users]
@@ -165,7 +165,7 @@ def test_set_participants_users_unread_private_threads_sync(self):
         """
         users = [create_test_user("User", "user@example.com")]
         participants = [ThreadParticipant(user=u) for u in users]
-        users.append(create_test_user("OtherUser", "otheruser@example.com"))
+        users.append(create_test_user("Other_User", "otheruser@example.com"))
 
         set_users_unread_private_threads_sync(users=users, participants=participants)
         for user in users:
@@ -176,7 +176,7 @@ def test_set_users_unread_private_threads_sync_exclude_user(self):
         """exclude_user kwarg works"""
         users = [
             create_test_user("User", "user@example.com"),
-            create_test_user("OtherUser", "otheruser@example.com"),
+            create_test_user("Other_User", "otheruser@example.com"),
         ]
 
         set_users_unread_private_threads_sync(users=users, exclude_user=users[0])