Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix underscore in usernames handling #1625

Merged
merged 6 commits into from
May 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions frontend/src/utils/validators.js
Original file line number Diff line number Diff line change
@@ -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.")
}
}
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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."
)
}
}
Expand Down
12 changes: 6 additions & 6 deletions misago/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def user_acl(user, cache_versions):

@pytest.fixture
def other_user(db, user_password):
return create_test_user("OtherUser", "[email protected]", user_password)
return create_test_user("Other_User", "[email protected]", user_password)


@pytest.fixture
Expand All @@ -83,7 +83,7 @@ def other_user_acl(other_user, cache_versions):

@pytest.fixture
def staffuser(db, user_password):
user = create_test_superuser("Staffuser", "[email protected]", user_password)
user = create_test_superuser("Staff_User", "[email protected]", user_password)
user.is_superuser = False
user.save()
return user
Expand All @@ -97,7 +97,7 @@ def staffuser_acl(staffuser, cache_versions):
@pytest.fixture
def other_staffuser(db, user_password):
user = create_test_superuser(
"OtherStaffuser", "[email protected]", user_password
"Other_Staff_User", "[email protected]", user_password
)

user.is_superuser = False
Expand All @@ -107,7 +107,7 @@ def other_staffuser(db, user_password):

@pytest.fixture
def superuser(db, user_password):
return create_test_superuser("Superuser", "[email protected]", user_password)
return create_test_superuser("Super_User", "[email protected]", user_password)


@pytest.fixture
Expand All @@ -118,14 +118,14 @@ def superuser_acl(superuser, cache_versions):
@pytest.fixture
def other_superuser(db, user_password):
return create_test_superuser(
"OtherSuperuser", "[email protected]", user_password
"OtherSuperUser", "[email protected]", user_password
)


@pytest.fixture
def inactive_user(db, user_password):
return create_test_user(
"InactiveUser", "[email protected]", user_password, is_active=False
"Inactive_User", "[email protected]", user_password, is_active=False
)


Expand Down
9 changes: 5 additions & 4 deletions misago/markup/mentions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions misago/markup/tests/test_mentions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 8 additions & 2 deletions misago/oauth2/tests/test_user_data_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
]


Expand All @@ -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."
)
]


Expand Down
2 changes: 1 addition & 1 deletion misago/static/misago/js/misago.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion misago/static/misago/js/misago.js.map

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion misago/threads/api/postingendpoint/participants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down
11 changes: 6 additions & 5 deletions misago/threads/api/threadendpoints/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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."))
Expand Down
4 changes: 2 additions & 2 deletions misago/threads/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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})


Expand Down
16 changes: 8 additions & 8 deletions misago/threads/tests/test_anonymize_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request()

set_owner(self.thread, self.user)
Expand All @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request()

set_owner(self.thread, self.user)
Expand All @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request(user)

set_owner(self.thread, user)
Expand All @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request()

set_owner(self.thread, user)
Expand All @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request(user)

set_owner(self.thread, self.user)
Expand Down Expand Up @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
request = self.get_request()

set_owner(self.thread, self.user)
Expand Down Expand Up @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")

patch_is_liked(self.get_request(self.user), post, 1)
patch_is_liked(self.get_request(user), post, 1)
Expand Down Expand Up @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")
post = test.reply_thread(thread, poster=user)
user.anonymize_data(anonymous_username="Deleted")

Expand Down
2 changes: 1 addition & 1 deletion misago/threads/tests/test_delete_user_likes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]")
user = create_test_user("Other_User", "[email protected]")

patch_is_liked(self.get_request(self.user), post, 1)
patch_is_liked(self.get_request(user), post, 1)
Expand Down
14 changes: 7 additions & 7 deletions misago/threads/tests/test_participants.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_has_participants(self):
"""has_participants returns true if thread has participants"""
users = [
create_test_user("User", "[email protected]"),
create_test_user("OtherUser", "[email protected]"),
create_test_user("Other_User", "[email protected]"),
]

self.assertFalse(has_participants(self.thread))
Expand All @@ -67,7 +67,7 @@ def test_make_threads_participants_aware(self):
annotations on list of threads
"""
user = create_test_user("User", "[email protected]")
other_user = create_test_user("OtherUser", "[email protected]")
other_user = create_test_user("Other_User", "[email protected]")

self.assertFalse(hasattr(self.thread, "participants_list"))
self.assertFalse(hasattr(self.thread, "participant"))
Expand All @@ -92,7 +92,7 @@ def test_make_thread_participants_aware(self):
annotations on thread model
"""
user = create_test_user("User", "[email protected]")
other_user = create_test_user("OtherUser", "[email protected]")
other_user = create_test_user("Other_User", "[email protected]")

self.assertFalse(hasattr(self.thread, "participants_list"))
self.assertFalse(hasattr(self.thread, "participant"))
Expand Down Expand Up @@ -133,7 +133,7 @@ def test_set_users_unread_private_threads_sync(self):
"""
users = [
create_test_user("User", "[email protected]"),
create_test_user("OtherUser", "[email protected]"),
create_test_user("Other_User", "[email protected]"),
]

set_users_unread_private_threads_sync(users=users)
Expand All @@ -148,7 +148,7 @@ def test_set_participants_unread_private_threads_sync(self):
"""
users = [
create_test_user("User", "[email protected]"),
create_test_user("OtherUser", "[email protected]"),
create_test_user("Other_User", "[email protected]"),
]

participants = [ThreadParticipant(user=u) for u in users]
Expand All @@ -165,7 +165,7 @@ def test_set_participants_users_unread_private_threads_sync(self):
"""
users = [create_test_user("User", "[email protected]")]
participants = [ThreadParticipant(user=u) for u in users]
users.append(create_test_user("OtherUser", "[email protected]"))
users.append(create_test_user("Other_User", "[email protected]"))

set_users_unread_private_threads_sync(users=users, participants=participants)
for user in users:
Expand All @@ -176,7 +176,7 @@ def test_set_users_unread_private_threads_sync_exclude_user(self):
"""exclude_user kwarg works"""
users = [
create_test_user("User", "[email protected]"),
create_test_user("OtherUser", "[email protected]"),
create_test_user("Other_User", "[email protected]"),
]

set_users_unread_private_threads_sync(users=users, exclude_user=users[0])
Expand Down
Loading