diff --git a/spirit/comment/flag/admin/templates/spirit/comment/flag/admin/detail.html b/spirit/comment/flag/admin/templates/spirit/comment/flag/admin/detail.html index 85dd49e26..46490f63e 100644 --- a/spirit/comment/flag/admin/templates/spirit/comment/flag/admin/detail.html +++ b/spirit/comment/flag/admin/templates/spirit/comment/flag/admin/detail.html @@ -16,7 +16,7 @@

{% trans "Flag" %}

{% if flag.moderator %}
{% trans "Moderated by" %}:
-
{{ flag.moderator.username }}
+
{{ flag.moderator.st.nickname }}
{% endif %} @@ -41,7 +41,7 @@

{% trans "Comment flagged" %}

- {{ flag.comment.user.username }} + {{ flag.comment.user.st.nickname }}
@@ -77,7 +77,7 @@

{% trans "Reporters" %}

- {{ f.user.username }} + {{ f.user.st.nickname }}
diff --git a/spirit/comment/flag/forms.py b/spirit/comment/flag/forms.py index fc8c85f2e..2dc48c8cc 100644 --- a/spirit/comment/flag/forms.py +++ b/spirit/comment/flag/forms.py @@ -4,7 +4,7 @@ from django import forms from django.utils.translation import ugettext_lazy as _ -from django.db import IntegrityError +from django.db import IntegrityError, transaction from django.utils import timezone from .models import Flag, CommentFlag @@ -39,8 +39,10 @@ def save(self, commit=True): self.instance.comment = self.comment try: - CommentFlag.objects.update_or_create(comment=self.comment, - defaults={'date': timezone.now(), }) + with transaction.atomic(): + CommentFlag.objects.update_or_create( + comment=self.comment, + defaults={'date': timezone.now(), }) except IntegrityError: pass diff --git a/spirit/comment/flag/templates/spirit/comment/flag/create.html b/spirit/comment/flag/templates/spirit/comment/flag/create.html index dede67be1..7ca4f8e8d 100644 --- a/spirit/comment/flag/templates/spirit/comment/flag/create.html +++ b/spirit/comment/flag/templates/spirit/comment/flag/create.html @@ -7,7 +7,7 @@ {% block content %}

- {% blocktrans trimmed with username=comment.user.username %} + {% blocktrans trimmed with username=comment.user.st.nickname %} Reporting {{ username }}'s comment {% endblocktrans %}

diff --git a/spirit/comment/history/templates/spirit/comment/history/detail.html b/spirit/comment/history/templates/spirit/comment/history/detail.html index 867134759..2ad869d4d 100644 --- a/spirit/comment/history/templates/spirit/comment/history/detail.html +++ b/spirit/comment/history/templates/spirit/comment/history/detail.html @@ -24,7 +24,7 @@

{% trans "Comment history" %}

- {{ c.comment_fk.user.username }} + {{ c.comment_fk.user.st.nickname }}
{% endspaceless %} diff --git a/spirit/core/tests/tests_markdown.py b/spirit/core/tests/tests_markdown.py index 4cafcfcf4..0aef1cc29 100644 --- a/spirit/core/tests/tests_markdown.py +++ b/spirit/core/tests/tests_markdown.py @@ -69,13 +69,14 @@ def test_markdown_mentions(self): """ comment = "@nitely, @esteban,@áéíóú, @fakeone" comment_md = Markdown().render(comment) - self.assertEqual(comment_md, '

@nitely, ' - '@esteban,' - '@áéíóú, ' - '@fakeone

' % - (self.user.st.get_absolute_url(), - self.user2.st.get_absolute_url(), - self.user3.st.get_absolute_url())) + self.assertEqual(comment_md, + '

@nitely, ' + '@esteban,' + '@áéíóú, ' + '@fakeone

' % + (self.user.st.get_absolute_url(), + self.user2.st.get_absolute_url(), + self.user3.st.get_absolute_url())) @override_settings(ST_MENTIONS_PER_COMMENT=2) def test_markdown_mentions_limit(self): @@ -94,8 +95,29 @@ def test_markdown_mentions_dict(self): md = Markdown() md.render(comment) # mentions get dynamically added on MentionifyExtension - self.assertDictEqual(md.get_mentions(), {'nitely': self.user, - 'esteban': self.user2}) + self.assertDictEqual(md.get_mentions(), { + 'nitely': self.user, + 'esteban': self.user2}) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_markdown_mentions_dict_ci(self): + """ + markdown mentions dict case-insensitive + """ + comment = "@NiTely, @EsTebaN, @nitEly, @NiteLy" + md = Markdown() + md.render(comment) + self.assertDictEqual(md.get_mentions(), { + 'nitely': self.user, + 'esteban': self.user2}) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_markdown_mentions_dict_ci_off(self): + comment = "@NiTely, @esteban, @nitEly, @NiteLy" + md = Markdown() + md.render(comment) + self.assertDictEqual(md.get_mentions(), { + 'esteban': self.user2}) def test_markdown_emoji(self): """ @@ -115,7 +137,9 @@ def test_markdown_quote(self): """ comment = "text\nnew line" quote = quotify(comment, self.user) - self.assertListEqual(quote.splitlines(), ("> @%s said:\n> text\n> new line\n\n" % self.user.username).splitlines()) + self.assertListEqual( + quote.splitlines(), + ("> @%s said:\n> text\n> new line\n\n" % self.user.st.nickname).splitlines()) @override_settings(LANGUAGE_CODE='en') def test_markdown_quote_header_language(self): @@ -127,7 +151,9 @@ def test_markdown_quote_header_language(self): quote = quotify(comment, self.user) with translation.override('es'): - self.assertListEqual(quote.splitlines(), ("> @%s said:\n> \n\n" % self.user.username).splitlines()) + self.assertListEqual( + quote.splitlines(), + ("> @%s said:\n> \n\n" % self.user.st.nickname).splitlines()) @override_settings(LANGUAGE_CODE='en') def test_markdown_quote_no_polls(self): @@ -145,7 +171,9 @@ def test_markdown_quote_no_polls(self): "2. opt 2\n" \ "[/poll]" quote = quotify(comment, self.user) - self.assertListEqual(quote.splitlines(), ("> @%s said:\n> foo\n> \n> bar\n\n" % self.user.username).splitlines()) + self.assertListEqual( + quote.splitlines(), + ("> @%s said:\n> foo\n> \n> bar\n\n" % self.user.username).splitlines()) def test_markdown_image(self): """ diff --git a/spirit/core/tests/utils.py b/spirit/core/tests/utils.py index 6abe0223e..aed3dfed8 100644 --- a/spirit/core/tests/utils.py +++ b/spirit/core/tests/utils.py @@ -75,7 +75,9 @@ def create_comment(**kwargs): def login(test_case_instance, user=None, password=None): user = user or test_case_instance.user password = password or "bar" - login_successful = test_case_instance.client.login(username=user.username, password=password) + login_successful = test_case_instance.client.login( + username=user.username, + password=password) test_case_instance.assertTrue(login_successful) diff --git a/spirit/core/utils/markdown/inline.py b/spirit/core/utils/markdown/inline.py index ef281f5a4..816851cc8 100644 --- a/spirit/core/utils/markdown/inline.py +++ b/spirit/core/utils/markdown/inline.py @@ -65,25 +65,33 @@ def output_emoji(self, m): def output_mention(self, m): username = m.group('username') + if settings.ST_CASE_INSENSITIVE_USERNAMES: + username = username.lower() + # Already mentioned? if username in self.mentions: user = self.mentions[username] - return self.renderer.mention(username, user.st.get_absolute_url()) + return self.renderer.mention( + user.st.nickname, + user.st.get_absolute_url()) # Mentions limiter + # We increase this before doing the query to avoid abuses + # i.e adding 1K invalid usernames won't make 1K queries if self._mention_count >= settings.ST_MENTIONS_PER_COMMENT: return m.group(0) - - # We increase this before doing the query to avoid abuses self._mention_count += 1 # New mention try: - user = User.objects\ - .select_related('st')\ - .get(username=username) + user = ( + User.objects + .select_related('st') + .get(username=username)) except User.DoesNotExist: return m.group(0) self.mentions[username] = user - return self.renderer.mention(username, user.st.get_absolute_url()) + return self.renderer.mention( + user.st.nickname, + user.st.get_absolute_url()) diff --git a/spirit/core/utils/widgets.py b/spirit/core/utils/widgets.py index 95cf72565..ebdfa8f38 100644 --- a/spirit/core/utils/widgets.py +++ b/spirit/core/utils/widgets.py @@ -11,9 +11,6 @@ class MultipleInput(forms.TextInput): TextInput widget for input multiple *raw* choices """ - def __init__(self, *args, **kwargs): - super(MultipleInput, self).__init__(*args, **kwargs) - def render(self, name, value, *args, **kwargs): if value: value = ','.join(force_text(v) for v in value) @@ -28,3 +25,21 @@ def value_from_datadict(self, data, files, name, *args, **kwargs): if value: return [v.strip() for v in value.split(',')] + + +class CIMultipleInput(MultipleInput): + """Case-Insensitive ``MultipleInput`` widget""" + + def value_from_datadict(self, *args, **kwargs): + value = super(CIMultipleInput, self).value_from_datadict(*args, **kwargs) + if value: + return [v.lower() for v in value] + + +class CITextInput(forms.TextInput): + """Case-Insensitive ``TextInput`` widget""" + + def value_from_datadict(self, *args, **kwargs): + value = super(CITextInput, self).value_from_datadict(*args, **kwargs) + if value: + return value.lower() diff --git a/spirit/topic/notification/managers.py b/spirit/topic/notification/managers.py index 4b7c09012..f86a55389 100644 --- a/spirit/topic/notification/managers.py +++ b/spirit/topic/notification/managers.py @@ -9,26 +9,32 @@ class TopicNotificationQuerySet(models.QuerySet): def unremoved(self): - return self.filter(Q(topic__category__parent=None) | Q(topic__category__parent__is_removed=False), - topic__category__is_removed=False, - topic__is_removed=False) + return self.filter( + Q(topic__category__parent=None) | + Q(topic__category__parent__is_removed=False), + topic__category__is_removed=False, + topic__is_removed=False) def unread(self): return self.filter(is_read=False) def _access(self, user): - return self.filter(Q(topic__category__is_private=False) | Q(topic__topics_private__user=user), - user=user) + return self.filter( + Q(topic__category__is_private=False) | + Q(topic__topics_private__user=user), + user=user) def for_access(self, user): - return self.unremoved()\ - ._access(user=user)\ - .exclude(action=0) # Undefined action + return ( + self.unremoved() + ._access(user=user) + .exclude(action=0)) # Undefined action def read(self, user): # returns updated rows count (int) - return self.filter(user=user)\ - .update(is_read=True) + return ( + self.filter(user=user) + .update(is_read=True)) def with_related_data(self): return self.select_related('comment__user__st', 'topic') diff --git a/spirit/topic/notification/templates/spirit/topic/notification/_render_list.html b/spirit/topic/notification/templates/spirit/topic/notification/_render_list.html index b9d73e4cc..db0ebd941 100644 --- a/spirit/topic/notification/templates/spirit/topic/notification/_render_list.html +++ b/spirit/topic/notification/templates/spirit/topic/notification/_render_list.html @@ -6,11 +6,11 @@ {% url "spirit:comment:find" pk=n.comment.pk as url_topic %} {% if n.is_comment %} - {% blocktrans trimmed with username=n.comment.user.username topic_title=n.topic.title %} + {% blocktrans trimmed with username=n.comment.user.st.nickname topic_title=n.topic.title %} {{ username }} has commented on {{ topic_title }} {% endblocktrans %} {% elif n.is_mention %} - {% blocktrans trimmed with username=n.comment.user.username topic_title=n.topic.title %} + {% blocktrans trimmed with username=n.comment.user.st.nickname topic_title=n.topic.title %} {{ username }} has mention you on {{ topic_title }} {% endblocktrans %} {% else %} diff --git a/spirit/topic/notification/views.py b/spirit/topic/notification/views.py index bead7cf96..49efbf9db 100644 --- a/spirit/topic/notification/views.py +++ b/spirit/topic/notification/views.py @@ -65,7 +65,7 @@ def index_ajax(request): notifications = [ { - 'user': escape(n.comment.user.username), + 'user': escape(n.comment.user.st.nickname), 'action': n.action, 'title': escape(n.topic.title), 'url': n.get_absolute_url(), diff --git a/spirit/topic/private/forms.py b/spirit/topic/private/forms.py index 0719827ac..ded613656 100644 --- a/spirit/topic/private/forms.py +++ b/spirit/topic/private/forms.py @@ -9,7 +9,10 @@ from ...core.conf import settings from ...core import utils -from ...core.utils.widgets import MultipleInput +from ...core.utils.widgets import ( + MultipleInput, + CIMultipleInput, + CITextInput) from ...topic.models import Topic from ...category.models import Category from .models import TopicPrivate @@ -61,19 +64,27 @@ def save(self, commit=True): return super(TopicForPrivateForm, self).save(commit) +def cx_multiple_input(*args, **kwargs): + if settings.ST_CASE_INSENSITIVE_USERNAMES: + return CIMultipleInput(*args, **kwargs) + return MultipleInput(*args, **kwargs) + + class TopicPrivateManyForm(forms.Form): # Only good for create users = forms.ModelMultipleChoiceField( label=_("Invite users"), queryset=User.objects.all(), - to_field_name=User.USERNAME_FIELD, - widget=MultipleInput(attrs={'placeholder': _("user1, user2, ...")})) + to_field_name=User.USERNAME_FIELD) def __init__(self, user=None, topic=None, *args, **kwargs): super(TopicPrivateManyForm, self).__init__(*args, **kwargs) self.user = user self.topic = topic + # Make it dynamic for testing + self.fields['users'].widget = cx_multiple_input( + attrs={'placeholder': _("user1, user2, ...")}) def clean_users(self): users = set(self.cleaned_data['users']) @@ -91,21 +102,31 @@ def get_users(self): def save_m2m(self): users = self.cleaned_data['users'] # Since the topic was just created this should not raise an exception - return TopicPrivate.objects.bulk_create([TopicPrivate(user=user, topic=self.topic) - for user in users]) + return TopicPrivate.objects.bulk_create( + [TopicPrivate(user=user, topic=self.topic) + for user in users]) + + +def cx_text_input(*args, **kwargs): + if settings.ST_CASE_INSENSITIVE_USERNAMES: + return CITextInput(*args, **kwargs) + return forms.TextInput(*args, **kwargs) class TopicPrivateInviteForm(forms.ModelForm): # Only good for create - user = forms.ModelChoiceField(queryset=User.objects.all(), - to_field_name=User.USERNAME_FIELD, - widget=forms.TextInput(attrs={'placeholder': _("username"), }), - label=_("Invite user")) + user = forms.ModelChoiceField( + queryset=User.objects.all(), + to_field_name=User.USERNAME_FIELD, + label=_("Invite user")) def __init__(self, topic=None, *args, **kwargs): super(TopicPrivateInviteForm, self).__init__(*args, **kwargs) self.topic = topic + # Make it dynamic for testing + self.fields['user'].widget = cx_text_input( + attrs={'placeholder': _("username")}) class Meta: model = TopicPrivate @@ -114,13 +135,14 @@ class Meta: def clean_user(self): user = self.cleaned_data['user'] - private = TopicPrivate.objects.filter(user=user, - topic=self.topic) + private = TopicPrivate.objects.filter( + user=user, topic=self.topic) if private.exists(): # Do this since some of the unique_together fields are excluded. - raise forms.ValidationError(_("%(username)s is already a participant") % - {'username': getattr(user, user.USERNAME_FIELD), }) + raise forms.ValidationError( + _("%(username)s is already a participant") % + {'username': user.st.nickname}) return user @@ -148,13 +170,14 @@ class Meta: def clean(self): cleaned_data = super(TopicPrivateJoinForm, self).clean() - private = TopicPrivate.objects.filter(user=self.user, - topic=self.topic) + private = TopicPrivate.objects.filter( + user=self.user, topic=self.topic) if private.exists(): # Do this since some of the unique_together fields are excluded. - raise forms.ValidationError(_("%(username)s is already a participant") % - {'username': getattr(self.user, self.user.USERNAME_FIELD), }) + raise forms.ValidationError( + _("%(username)s is already a participant") % + {'username': self.user.st.nickname}) return cleaned_data diff --git a/spirit/topic/private/templates/spirit/topic/private/delete.html b/spirit/topic/private/templates/spirit/topic/private/delete.html index e9893b898..9cdc447c7 100644 --- a/spirit/topic/private/templates/spirit/topic/private/delete.html +++ b/spirit/topic/private/templates/spirit/topic/private/delete.html @@ -7,7 +7,7 @@ {% block content %}

- {% blocktrans trimmed with username=topic_private.user.username topic_title=topic_private.topic.title %} + {% blocktrans trimmed with username=topic_private.user.st.nickname topic_title=topic_private.topic.title %} Removing {{ username }}'s from {{ topic_title }} {% endblocktrans %}

diff --git a/spirit/topic/private/templates/spirit/topic/private/detail.html b/spirit/topic/private/templates/spirit/topic/private/detail.html index ad5fbb89c..655de8b53 100644 --- a/spirit/topic/private/templates/spirit/topic/private/detail.html +++ b/spirit/topic/private/templates/spirit/topic/private/detail.html @@ -24,7 +24,7 @@

{{ topic.title }}

{% spaceless %} {% for tp in topic.topics_private.all %}
  • - {{ tp.user.username }} + {{ tp.user.st.nickname }} {% ifequal user.pk topic.user.pk %} diff --git a/spirit/topic/private/tests.py b/spirit/topic/private/tests.py index 3d8bba146..9793c83c2 100644 --- a/spirit/topic/private/tests.py +++ b/spirit/topic/private/tests.py @@ -530,6 +530,78 @@ def test_private_create_many(self): privates = TopicPrivate.objects.all() self.assertEqual([p.user.pk for p in privates], [self.user2.pk, self.user.pk]) + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_private_invite_case_insensitive(self): + user1 = utils.create_user(username='UnIQuEfOo') + self.assertNotEqual( + user1.username, user1.username.upper()) + form_data = {'user': user1.username.upper()} + private = utils.create_private_topic(user=self.user) + form = TopicPrivateInviteForm(private.topic, data=form_data) + self.assertEqual(form.is_valid(), True) + + # regular username should still work + form_data = {'user': user1.username} + private = utils.create_private_topic(user=self.user) + form = TopicPrivateInviteForm(private.topic, data=form_data) + self.assertEqual(form.is_valid(), True) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_private_invite_case_insensitive_off(self): + user1 = utils.create_user(username='UnIQuEfOo') + self.assertNotEqual( + user1.username, user1.username.upper()) + form_data = {'user': user1.username.upper()} + private = utils.create_private_topic(user=self.user) + form = TopicPrivateInviteForm(private.topic, data=form_data) + self.assertEqual(form.is_valid(), False) + + # regular username should still work + form_data = {'user': user1.username} + private = utils.create_private_topic(user=self.user) + form = TopicPrivateInviteForm(private.topic, data=form_data) + self.assertEqual(form.is_valid(), True) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_private_create_many_case_insensitive(self): + """ + create many private topics accesses + """ + user1 = utils.create_user(username='UnIQuEfOo') + user2 = utils.create_user(username='uniquebar') + self.assertNotEqual( + user1.username, user1.username.upper()) + self.assertEqual( + user2.username, 'uniquebar') + users = '%s, %s' % ( + user1.username.upper(), 'uniquebar') + form_data = {'users': users} + form = TopicPrivateManyForm(self.user, data=form_data) + self.assertEqual(form.is_valid(), True) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_private_create_many_case_insensitive_off(self): + """ + create many private topics accesses + """ + user1 = utils.create_user(username='UnIQuEfOo') + user2 = utils.create_user(username='uniquebar') + self.assertNotEqual( + user1.username, user1.username.upper()) + self.assertEqual( + user2.username, 'uniquebar') + users = '%s, %s' % ( + user1.username.upper(), 'uniquebar') + form_data = {'users': users} + form = TopicPrivateManyForm(self.user, data=form_data) + self.assertEqual(form.is_valid(), False) + + users = 'UnIQuEfOo, uniquebar' + form_data = {'users': users} + form = TopicPrivateManyForm(self.user, data=form_data) + print(form.errors) + self.assertEqual(form.is_valid(), True) + def test_private_create(self): """ create single private topic access diff --git a/spirit/topic/private/views.py b/spirit/topic/private/views.py index 8907fda36..b07c18ec7 100644 --- a/spirit/topic/private/views.py +++ b/spirit/topic/private/views.py @@ -64,7 +64,7 @@ def publish(request, user_id=None): if user_id: # todo: move to form user_to = get_object_or_404(User, pk=user_id) - initial = {'users': [user_to.username]} + initial = {'users': [user_to.st.nickname]} tpform = TopicPrivateManyForm(initial=initial) @@ -149,8 +149,7 @@ def join_in(request, topic_id): Topic, pk=topic_id, user=request.user, - category_id=settings.ST_TOPIC_PRIVATE_CATEGORY_PK - ) + category_id=settings.ST_TOPIC_PRIVATE_CATEGORY_PK) if request.method == 'POST': form = TopicPrivateJoinForm(topic=topic, user=request.user, data=request.POST) diff --git a/spirit/user/admin/templates/spirit/user/admin/_list.html b/spirit/user/admin/templates/spirit/user/admin/_list.html index 738df9a41..c17c899dc 100644 --- a/spirit/user/admin/templates/spirit/user/admin/_list.html +++ b/spirit/user/admin/templates/spirit/user/admin/_list.html @@ -6,7 +6,7 @@ {% for u in users %}
    - {{ u.username }} + {{ u.st.nickname }} {% trans "edit" %} diff --git a/spirit/user/auth/backends.py b/spirit/user/auth/backends.py index 3cca09516..b064d9d63 100644 --- a/spirit/user/auth/backends.py +++ b/spirit/user/auth/backends.py @@ -10,41 +10,44 @@ User = get_user_model() -class EmailAuthBackend(ModelBackend): +class _SpiritBackend(ModelBackend): + + def get_user(self, user_id): + try: + return ( + User._default_manager + .select_related('st') + .get(pk=user_id)) + except User.DoesNotExist: + pass + + +class EmailAuthBackend(_SpiritBackend): def authenticate(self, request, username=None, password=None, **kwargs): # TODO: authenticate when multiple users are returned + if username is None: + username = kwargs.get(User.USERNAME_FIELD) if settings.ST_CASE_INSENSITIVE_EMAILS: username = username.lower() try: user = User._default_manager.get(email=username) - - if user.check_password(password): - return user except (User.DoesNotExist, User.MultipleObjectsReturned): - pass - - def get_user(self, user_id): - # This is called if the user - # get authenticated with email - try: - return ( - User._default_manager - .select_related('st') - .get(pk=user_id)) - except User.DoesNotExist: - pass + User().set_password(password) + else: + if (user.check_password(password) and + self.user_can_authenticate(user)): + return user -class UsernameAuthBackend(ModelBackend): +class UsernameAuthBackend(_SpiritBackend): # TODO: test! - def get_user(self, user_id): - try: - return ( - User._default_manager - .select_related('st') - .get(pk=user_id)) - except User.DoesNotExist: - pass + def authenticate(self, request, username=None, password=None, **kwargs): + if username is None: + username = kwargs.get(User.USERNAME_FIELD) + if settings.ST_CASE_INSENSITIVE_USERNAMES: + username = username.lower() + return super(UsernameAuthBackend, self).authenticate( + request, username=username, password=password, **kwargs) diff --git a/spirit/user/auth/forms.py b/spirit/user/auth/forms.py index 26bf5e7c2..fc4dc9707 100644 --- a/spirit/user/auth/forms.py +++ b/spirit/user/auth/forms.py @@ -19,11 +19,14 @@ class RegistrationForm(CleanEmailMixin, forms.ModelForm): label=_("Email confirmation"), widget=forms.EmailInput, max_length=254, - help_text=_("Enter the same email as above, for verification.") - ) + help_text=_("Enter the same email as above, for verification.")) # todo: add password validator for Django 1.9 - password = forms.CharField(label=_("Password"), widget=forms.PasswordInput) - honeypot = forms.CharField(label=_("Leave blank"), required=False) + password = forms.CharField( + label=_("Password"), + widget=forms.PasswordInput) + honeypot = forms.CharField( + label=_("Leave blank"), + required=False) class Meta: model = User @@ -38,21 +41,26 @@ def clean_honeypot(self): value = self.cleaned_data["honeypot"] if value: - raise forms.ValidationError(_("Do not fill this field.")) + raise forms.ValidationError( + _("Do not fill this field.")) return value def clean_username(self): username = self.cleaned_data["username"] - is_taken = User.objects\ - .filter(username=username)\ - .exists() + if settings.ST_CASE_INSENSITIVE_USERNAMES: + username = username.lower() + is_taken = ( + User.objects + .filter(username=username) + .exists()) if is_taken: - raise forms.ValidationError(_("The username is taken.")) + raise forms.ValidationError( + _("The username is taken.")) - return username + return self.cleaned_data["username"] def clean_email2(self): email = self.cleaned_data.get("email") @@ -63,8 +71,7 @@ def clean_email2(self): if email and email != email2: raise forms.ValidationError( - _("The two email fields didn't match.") - ) + _("The two email fields didn't match.")) return email2 @@ -76,7 +83,9 @@ def save(self, commit=True): class LoginForm(AuthenticationForm): - username = forms.CharField(label=_("Username or Email"), max_length=254) + username = forms.CharField( + label=_("Username or Email"), + max_length=254) def __init__(self, *args, **kwargs): super(LoginForm, self).__init__(*args, **kwargs) @@ -94,26 +103,29 @@ def _validate_username(self): if not username: return - is_found = User.objects\ - .filter(username=username)\ - .exists() + if settings.ST_CASE_INSENSITIVE_USERNAMES: + username = username.lower() + is_found = ( + User.objects + .filter(username=username) + .exists()) if is_found: return if settings.ST_CASE_INSENSITIVE_EMAILS: username = username.lower() - is_found_email = User.objects\ - .filter(email=username)\ - .exists() - + is_found_email = ( + User.objects + .filter(email=username) + .exists()) if is_found_email: return raise forms.ValidationError( - _("No account matches %(username)s.") % {'username': username} - ) + _("No account matches %(username)s.") % { + 'username': username}) def clean(self): self._validate_username() @@ -122,7 +134,10 @@ def clean(self): class ResendActivationForm(forms.Form): - email = forms.CharField(label=_("Email"), widget=forms.EmailInput, max_length=254) + email = forms.CharField( + label=_("Email"), + widget=forms.EmailInput, + max_length=254) def clean_email(self): email = self.cleaned_data["email"] @@ -130,20 +145,24 @@ def clean_email(self): if settings.ST_CASE_INSENSITIVE_EMAILS: email = email.lower() - is_existent = User.objects\ - .filter(email=email)\ - .exists() - + is_existent = ( + User.objects + .filter(email=email) + .exists()) if not is_existent: - raise forms.ValidationError(_("The provided email does not exists.")) - - self.user = User.objects\ - .filter(email=email, st__is_verified=False)\ - .order_by('-pk')\ - .first() - + raise forms.ValidationError( + _("The provided email does not exists.")) + + self.user = ( + User.objects + .filter( + email=email, + st__is_verified=False) + .order_by('-pk') + .first()) if not self.user: - raise forms.ValidationError(_("This account is verified, try logging-in.")) + raise forms.ValidationError( + _("This account is verified, try logging-in.")) return email diff --git a/spirit/user/auth/tests/tests.py b/spirit/user/auth/tests/tests.py index 4582f55d3..b273bf65f 100644 --- a/spirit/user/auth/tests/tests.py +++ b/spirit/user/auth/tests/tests.py @@ -12,7 +12,7 @@ from ....core.tests import utils from ..forms import RegistrationForm, ResendActivationForm, LoginForm -from ..backends import EmailAuthBackend +from ..backends import EmailAuthBackend, UsernameAuthBackend from ...utils.tokens import UserActivationTokenGenerator from ...models import UserProfile from .urls import CustomRegisterForm @@ -57,6 +57,58 @@ def test_login_redirect(self): response = self.client.get(reverse('spirit:user:auth:login') + '?next=/fakepath/') self.assertRedirects(response, '/fakepath/', status_code=302, target_status_code=404) + @override_settings(ST_CASE_INSENSITIVE_EMAILS=True) + def test_login_email_case_insensitive(self): + """ + try to login by email + """ + self.assertNotEqual( + self.user.email, self.user.email.upper()) + form_data = { + 'username': self.user.email.upper(), + 'password': "bar"} + response = self.client.post( + reverse('spirit:user:auth:login'), form_data) + expected_url = reverse('spirit:user:update') + self.assertRedirects(response, expected_url, status_code=302) + + @override_settings(ST_CASE_INSENSITIVE_EMAILS=False) + def test_login_email_case_insensitive_off(self): + """ + try to login by email + """ + self.assertNotEqual( + self.user.email, self.user.email.upper()) + form_data = { + 'username': self.user.email.upper(), + 'password': "bar"} + response = self.client.post( + reverse('spirit:user:auth:login'), form_data) + self.assertEqual(response.status_code, 200) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_login_username_case_insensitive(self): + self.assertNotEqual( + self.user.username, self.user.username.upper()) + form_data = { + 'username': self.user.username.upper(), + 'password': "bar"} + response = self.client.post( + reverse('spirit:user:auth:login'), form_data) + expected_url = reverse('spirit:user:update') + self.assertRedirects(response, expected_url, status_code=302) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_login_username_case_insensitive_off(self): + self.assertNotEqual( + self.user.username, self.user.username.upper()) + form_data = { + 'username': self.user.username.upper(), + 'password': "bar"} + response = self.client.post( + reverse('spirit:user:auth:login'), form_data) + self.assertEqual(response.status_code, 200) + def test_register(self): """ register @@ -78,6 +130,38 @@ def test_register(self): response = self.client.get(reverse('spirit:user:auth:register')) self.assertRedirects(response, reverse('spirit:user:update'), status_code=302) + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_register_username_case_insensitive(self): + form_data = { + 'username': 'UnIqUeFoO', + 'email': 'some@some.com', + 'email2': 'some@some.com', + 'password': 'pass'} + response = self.client.post( + reverse('spirit:user:auth:register'), form_data) + expected_url = reverse('spirit:user:auth:login') + self.assertRedirects(response, expected_url, status_code=302) + self.assertTrue( + User.objects.filter(username='uniquefoo').exists()) + self.assertFalse( + User.objects.filter(username='UnIqUeFoO').exists()) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_register_username_case_insensitive_off(self): + form_data = { + 'username': 'UnIqUeFoO', + 'email': 'some@some.com', + 'email2': 'some@some.com', + 'password': 'pass'} + response = self.client.post( + reverse('spirit:user:auth:register'), form_data) + expected_url = reverse('spirit:user:auth:login') + self.assertRedirects(response, expected_url, status_code=302) + self.assertFalse( + User.objects.filter(username='uniquefoo').exists()) + self.assertTrue( + User.objects.filter(username='UnIqUeFoO').exists()) + def test_register_email_sent(self): """ register and send activation email @@ -545,7 +629,9 @@ class UserBackendTest(TestCase): def setUp(self): utils.cache_clear() - self.user = utils.create_user(email="foobar@bar.com", password="bar") + self.user = utils.create_user( + email="foobar@bar.com", + password="bar") def test_email_auth_backend(self): user = EmailAuthBackend().authenticate( @@ -573,3 +659,27 @@ def test_email_auth_backend_case_sensitive(self): user = EmailAuthBackend().authenticate( request=None, username="FooBar@bAr.COM", password="bar") self.assertIsNone(user) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_username_auth_backend_case_sensitive(self): + usr = utils.create_user( + username="FooBar", + password="bar") + user = UsernameAuthBackend().authenticate( + request=None, username="FooBar", password="bar") + self.assertEqual(user.pk, usr.pk) + user = UsernameAuthBackend().authenticate( + request=None, username="foobar", password="bar") + self.assertEqual(user.pk, usr.pk) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_username_auth_backend_case_sensitive_off(self): + usr = utils.create_user( + username="FooBar", + password="bar") + user = UsernameAuthBackend().authenticate( + request=None, username="FooBar", password="bar") + self.assertEqual(user.pk, usr.pk) + user = UsernameAuthBackend().authenticate( + request=None, username="foobar", password="bar") + self.assertIsNone(user) diff --git a/spirit/user/migrations/0010_userprofile_nickname.py b/spirit/user/migrations/0010_userprofile_nickname.py new file mode 100644 index 000000000..52538f53a --- /dev/null +++ b/spirit/user/migrations/0010_userprofile_nickname.py @@ -0,0 +1,19 @@ +# Generated by Django 2.1.3 on 2018-11-24 23:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('spirit_user', '0009_auto_20161114_1850'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='nickname', + field=models.CharField(default='', max_length=255, verbose_name='nickname'), + preserve_default=False, + ), + ] diff --git a/spirit/user/migrations/0011_auto_20181124_2320.py b/spirit/user/migrations/0011_auto_20181124_2320.py new file mode 100644 index 000000000..732f0a169 --- /dev/null +++ b/spirit/user/migrations/0011_auto_20181124_2320.py @@ -0,0 +1,65 @@ +# Generated by Django 2.1.3 on 2018-11-24 23:20 + +from django.db import migrations, IntegrityError, transaction + +# This can be ran multiple times, I got your back ;) +def populate_nickname(apps, schema_editor): + from django.db.models import Subquery, OuterRef + from ...core.conf import settings + User = apps.get_model(settings.AUTH_USER_MODEL) + UserProfile = apps.get_model("spirit_user", "UserProfile") + first_user = UserProfile.objects.first() + if first_user and not first_user.nickname: + UserProfile.objects.all().update( + nickname=Subquery( + User.objects + .filter(pk=OuterRef('user_id')) + .values('username')[:1])) + +def make_usernames_lower(apps, schema_editor): + from django.db.models.functions import Lower + from django.db.models import Count + from ...core.conf import settings + User = apps.get_model(settings.AUTH_USER_MODEL) + + if settings.ST_CASE_INSENSITIVE_USERNAMES: + try: + with transaction.atomic(): + User.objects.all().update( + username=Lower('username')) + except IntegrityError: + # Show all duplicated usernames + users_lower = ( + User.objects + .annotate(username_lower=Lower('username')) + .values('username_lower') + .annotate(cnt=Count(Lower('username'))) + .values('username_lower', 'cnt') + .filter(cnt__gt=1) + .order_by('username_lower', 'cnt')) + users = [ + u.username + for u in ( + User.objects + .annotate(username_lower=Lower('username')) + .filter(username_lower__in=[ + u['username_lower'] for u in users_lower]))] + raise IntegrityError( + 'There are two or more users with ' + 'similar name but different casing, for example: ' + 'someUser and SomeUser, either remove one of them ' + 'or set the `ST_CASE_INSENSITIVE_USERNAMES` setting ' + 'to False. Then run the upgrade/migration again. ' + 'Any change was reverted. Duplicate users are {}'.format(users)) + + +class Migration(migrations.Migration): + + dependencies = [ + ('spirit_user', '0010_userprofile_nickname'), + ] + + operations = [ + migrations.RunPython(populate_nickname), + migrations.RunPython(make_usernames_lower) + ] diff --git a/spirit/user/models.py b/spirit/user/models.py index fb3647312..10b01d074 100644 --- a/spirit/user/models.py +++ b/spirit/user/models.py @@ -20,6 +20,7 @@ class UserProfile(models.Model): on_delete=models.CASCADE) slug = AutoSlugField(populate_from="user.username", db_index=False, blank=True) + nickname = models.CharField(_("nickname"), max_length=255) location = models.CharField(_("location"), max_length=75, blank=True) last_seen = models.DateTimeField(_("last seen"), auto_now=True) last_ip = models.GenericIPAddressField(_("last ip"), blank=True, null=True) diff --git a/spirit/user/signals.py b/spirit/user/signals.py index 5f032eaff..d4a04c112 100644 --- a/spirit/user/signals.py +++ b/spirit/user/signals.py @@ -5,6 +5,7 @@ from django.db.models.signals import post_save from django.contrib.auth import get_user_model +from ..core.conf import settings from .models import UserProfile User = get_user_model() @@ -12,10 +13,24 @@ def update_or_create_user_profile(sender, instance, created, **kwargs): user = instance - if created: - UserProfile.objects.create(user=user) + UserProfile.objects.create(user=user, nickname=user.username) else: user.st.save() -post_save.connect(update_or_create_user_profile, sender=User, dispatch_uid=__name__) \ No newline at end of file + +def lower_username(sender, instance, created, **kwargs): + user = instance + if created and settings.ST_CASE_INSENSITIVE_USERNAMES: + (User.objects + .filter(pk=user.pk) + .update(username=user.username.lower())) + user.username = user.username.lower() + + +def post_save_receivers(*args, **kwargs): + update_or_create_user_profile(*args, **kwargs) + lower_username(*args, **kwargs) + + +post_save.connect(post_save_receivers, sender=User, dispatch_uid=__name__) diff --git a/spirit/user/templates/spirit/user/_profile.html b/spirit/user/templates/spirit/user/_profile.html index a0dfac939..d424a19f0 100644 --- a/spirit/user/templates/spirit/user/_profile.html +++ b/spirit/user/templates/spirit/user/_profile.html @@ -3,7 +3,7 @@
    -
    {{ p_user.username }}
    +
    {{ p_user.st.nickname }}
    diff --git a/spirit/user/templates/spirit/user/_render_comments_list.html b/spirit/user/templates/spirit/user/_render_comments_list.html index 7d533e9cc..67d2f9ce5 100644 --- a/spirit/user/templates/spirit/user/_render_comments_list.html +++ b/spirit/user/templates/spirit/user/_render_comments_list.html @@ -15,7 +15,7 @@
    - {{ c.user.username }} + {{ c.user.st.nickname }}
    {% spaceless %} diff --git a/spirit/user/templates/spirit/user/profile_comments.html b/spirit/user/templates/spirit/user/profile_comments.html index d4abcacd3..17306626e 100644 --- a/spirit/user/templates/spirit/user/profile_comments.html +++ b/spirit/user/templates/spirit/user/profile_comments.html @@ -2,7 +2,7 @@ {% load spirit_tags i18n tz %} -{% block title %}{{ p_user.username }} {% trans "comments" %}{% endblock %} +{% block title %}{{ p_user.st.nickname }} {% trans "comments" %}{% endblock %} {% block content %} diff --git a/spirit/user/templates/spirit/user/profile_likes.html b/spirit/user/templates/spirit/user/profile_likes.html index 2e987971d..29ce4e12d 100644 --- a/spirit/user/templates/spirit/user/profile_likes.html +++ b/spirit/user/templates/spirit/user/profile_likes.html @@ -2,7 +2,7 @@ {% load spirit_tags i18n tz %} -{% block title %}{{ p_user.username }} {% trans "likes" %}{% endblock %} +{% block title %}{{ p_user.st.nickname }} {% trans "likes" %}{% endblock %} {% block content %} diff --git a/spirit/user/templates/spirit/user/profile_topics.html b/spirit/user/templates/spirit/user/profile_topics.html index 64987b1b6..d82162c64 100644 --- a/spirit/user/templates/spirit/user/profile_topics.html +++ b/spirit/user/templates/spirit/user/profile_topics.html @@ -2,7 +2,7 @@ {% load spirit_tags i18n %} -{% block title %}{{ p_user.username }} {% trans "topics" %}{% endblock %} +{% block title %}{{ p_user.st.nickname }} {% trans "topics" %}{% endblock %} {% block content %} diff --git a/spirit/user/tests.py b/spirit/user/tests.py index e08b192f1..47096bc45 100644 --- a/spirit/user/tests.py +++ b/spirit/user/tests.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals import datetime +import importlib from django.test import TestCase, RequestFactory from django.urls import reverse @@ -11,6 +12,8 @@ from django.utils import timezone from django.test.utils import override_settings from django.contrib.auth.models import AnonymousUser +from django.apps import apps +from django.db import IntegrityError from djconfig.utils import override_djconfig @@ -27,6 +30,9 @@ from . import middleware from .models import UserProfile +data_migration_11 = importlib.import_module( + 'spirit.user.migrations.0011_auto_20181124_2320') + User = get_user_model() @@ -63,6 +69,66 @@ def test_user_views_denied_to_non_logged_users(self): response = self.client.get(reverse('spirit:user:email-change-confirm', kwargs={'token': "foo"})) self.assertEqual(response.status_code, 302) + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_profile_creation_on_user_create_case_insensitive(self): + user = utils.create_user(username='UnIqUeFoO') + self.assertTrue(user.username, 'uniquefoo') + self.assertTrue( + User.objects.filter(username='uniquefoo').exists()) + self.assertTrue( + UserProfile.objects.filter( + nickname='UnIqUeFoO', + user_id=user.pk + ).exists()) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_profile_creation_on_user_create_case_insensitive_off(self): + user = utils.create_user(username='UnIqUeFoO') + self.assertTrue(user.username, 'UnIqUeFoO') + self.assertTrue( + User.objects.filter(username='UnIqUeFoO').exists()) + self.assertTrue( + UserProfile.objects.filter( + nickname='UnIqUeFoO', + user_id=user.pk + ).exists()) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=True) + def test_profile_creation_on_register_case_insensitive_user(self): + form_data = { + 'username': 'UnIqUeFoO', + 'email': 'some@some.com', + 'email2': 'some@some.com', + 'password': 'pass'} + response = self.client.post( + reverse('spirit:user:auth:register'), form_data) + expected_url = reverse('spirit:user:auth:login') + self.assertRedirects(response, expected_url, status_code=302) + self.assertTrue( + UserProfile.objects.filter( + nickname='UnIqUeFoO', + user__username='uniquefoo' + ).exists()) + self.assertFalse( + UserProfile.objects.filter(nickname='uniquefoo').exists()) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_profile_creation_on_register_case_insensitive_user_off(self): + form_data = { + 'username': 'UnIqUeFoO', + 'email': 'some@some.com', + 'email2': 'some@some.com', + 'password': 'pass'} + response = self.client.post( + reverse('spirit:user:auth:register'), form_data) + expected_url = reverse('spirit:user:auth:login') + self.assertRedirects(response, expected_url, status_code=302) + self.assertTrue( + UserProfile.objects.filter( + nickname='UnIqUeFoO', + user__username='UnIqUeFoO' + ).exists()) + def test_profile_topics(self): """ profile user's topics @@ -1147,3 +1213,110 @@ def test_active_user_anonym_user(self): self.assertFalse(req.user.is_authenticated) self.assertIsNone( middleware.ActiveUserMiddleware().process_request(req)) + + +class UserMigrationsTest(TestCase): + + def setUp(self): + utils.cache_clear() + + def test_migration_11(self): + # create users with the CI feature off + # to replicate pre feature database state + with override_settings(ST_CASE_INSENSITIVE_USERNAMES=False): + utils.create_user(username='FOO') + utils.create_user(username='BaR') + utils.create_user(username='baz') + # default all nicknames to empty + self.assertEqual( + UserProfile.objects.all().update(nickname=''), 3) + data_migration_11.populate_nickname(apps, None) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO', 'BaR', 'baz']) + + self.assertEqual( + [u.username for u in User.objects.all()], + ['FOO', 'BaR', 'baz']) + data_migration_11.make_usernames_lower(apps, None) + self.assertEqual( + [u.username for u in User.objects.all()], + ['foo', 'bar', 'baz']) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO', 'BaR', 'baz']) + + @override_settings(ST_CASE_INSENSITIVE_USERNAMES=False) + def test_migration_11_no_ci_usernames(self): + utils.create_user(username='FOO') + utils.create_user(username='foo') + utils.create_user(username='BaR') + utils.create_user(username='bar') + utils.create_user(username='baz') + + self.assertEqual( + UserProfile.objects.all().update(nickname=''), 5) + data_migration_11.populate_nickname(apps, None) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO', 'foo', 'BaR', 'bar', 'baz']) + + self.assertEqual( + [u.username for u in User.objects.all()], + ['FOO', 'foo', 'BaR', 'bar', 'baz']) + data_migration_11.make_usernames_lower(apps, None) + self.assertEqual( + [u.username for u in User.objects.all()], + ['FOO', 'foo', 'BaR', 'bar', 'baz']) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO', 'foo', 'BaR', 'bar', 'baz']) + + def test_migration_11_make_usernames_lower_integrity_err(self): + with override_settings(ST_CASE_INSENSITIVE_USERNAMES=False): + utils.create_user(username='FOO') + utils.create_user(username='fOo') + utils.create_user(username='Foo') + utils.create_user(username='bar') + utils.create_user(username='bAr') + utils.create_user(username='baz') + + self.assertEqual( + [u.username for u in User.objects.all()], + ['FOO', 'fOo', 'Foo', 'bar', 'bAr', 'baz']) + + # transaction is already handled + with self.assertRaises(IntegrityError) as cm: + data_migration_11.make_usernames_lower(apps, None) + self.maxDiff = None + self.assertEqual( + str(cm.exception), + "There are two or more users with similar name but " + "different casing, for example: someUser and SomeUser, " + "either remove one of them or set the " + "`ST_CASE_INSENSITIVE_USERNAMES` setting to False. " + "Then run the upgrade/migration again. Any change was reverted. " + "Duplicate users are ['FOO', 'fOo', 'Foo', 'bar', 'bAr']") + + def test_migration_11_idempotency(self): + """Should be idempotent""" + with override_settings(ST_CASE_INSENSITIVE_USERNAMES=False): + utils.create_user(username='FOO') + self.assertEqual( + UserProfile.objects.all().update(nickname=''), 1) + data_migration_11.populate_nickname(apps, None) + data_migration_11.make_usernames_lower(apps, None) + self.assertEqual( + [u.username for u in User.objects.all()], + ['foo']) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO']) + data_migration_11.populate_nickname(apps, None) + data_migration_11.make_usernames_lower(apps, None) + self.assertEqual( + [u.username for u in User.objects.all()], + ['foo']) + self.assertEqual( + [u.nickname for u in UserProfile.objects.all()], + ['FOO'])