Skip to content

Commit

Permalink
Merge "SIO-1531 strange characters still allowed in logins (eg. @kruk)"
Browse files Browse the repository at this point in the history
  • Loading branch information
ordyh authored and Gerrit Code Review committed Jun 10, 2014
2 parents 7894afb + 2c7076d commit 154c54e
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 4 deletions.
4 changes: 4 additions & 0 deletions oioioi/base/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from oioioi.base.utils import ObjectWithMixins, ClassInitMeta
from oioioi.base.utils.redirect import safe_redirect
from oioioi.base.menu import MenuRegistry, side_pane_menus_registry
from oioioi.base.forms import OioioiUserChangeForm, OioioiUserCreationForm

TabularInline = admin.TabularInline
StackedInline = admin.StackedInline
Expand Down Expand Up @@ -142,6 +143,9 @@ def login(self, request, extra_context=None):


class OioioiUserAdmin(UserAdmin, ObjectWithMixins):
form = OioioiUserChangeForm
add_form = OioioiUserCreationForm

__metaclass__ = ModelAdminMeta

# This is handled by AdminSite._reinit_model_admins
Expand Down
39 changes: 37 additions & 2 deletions oioioi/base/forms.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
import re

from django import forms
from django.contrib.auth.forms import UserCreationForm, UserChangeForm
from django.contrib.auth.models import User
from django.core.validators import RegexValidator
from django.utils.translation import ugettext_lazy as _
from registration.forms import RegistrationForm

from oioioi.base.utils.validators import ValidationError
from oioioi.base.utils.user import USERNAME_REGEX


def adjust_username_field(form):
help_text = \
_("This value may contain only letters, numbers and underscore.")
form.fields['username'].error_messages['invalid'] = _("Invalid username")
form.fields['username'].help_text = help_text
form.fields['username'].validators += \
[RegexValidator(regex=USERNAME_REGEX)]


class RegistrationFormWithNames(RegistrationForm):
def __init__(self, *args, **kwargs):
super(RegistrationFormWithNames, self).__init__(*args, **kwargs)
adjust_username_field(self)

self.fields.insert(1, 'first_name',
forms.CharField(label=_("First name")))
self.fields.insert(2, 'last_name',
Expand All @@ -19,17 +37,34 @@ class Meta(object):
fields = ['username', 'first_name', 'last_name', 'email']

def __init__(self, *args, **kwargs):
self.allow_login_change = kwargs.pop('allow_login_change', False)
super(UserForm, self).__init__(*args, **kwargs)
self.fields['username'].widget.attrs.update(readonly=True)
adjust_username_field(self)
if not self.allow_login_change:
self.fields['username'].widget.attrs.update(readonly=True)

def clean_username(self):
instance = getattr(self, 'instance', None)
if instance:
if instance and not self.allow_login_change:
if self.cleaned_data['username'] != instance.username:
raise ValidationError(_("You cannot change your username."))
return instance.username
else:
return self.cleaned_data['username']


class OioioiUserCreationForm(UserCreationForm):
def __init__(self, *args, **kwargs):
super(OioioiUserCreationForm, self).__init__(*args, **kwargs)
adjust_username_field(self)


class OioioiUserChangeForm(UserChangeForm):
def __init__(self, *args, **kwargs):
super(OioioiUserChangeForm, self).__init__(*args, **kwargs)
adjust_username_field(self)


# http://stackoverflow.com/questions/3657709/how-to-force-save-an-empty-unchanged-django-admin-inline
class AlwaysChangedModelForm(forms.ModelForm):
def has_changed(self):
Expand Down
26 changes: 26 additions & 0 deletions oioioi/base/middleware.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
# pylint: disable=W0703
# Catching too general exception Exception

from django.contrib import messages
from django.contrib.auth import BACKEND_SESSION_KEY
from django.core.exceptions import ImproperlyConfigured
from django.core.urlresolvers import reverse
from django.utils import timezone
from django.http import HttpResponseNotAllowed
from django.template import RequestContext
from django.template.loader import render_to_string
from django.utils.translation import ugettext_lazy as _
from django.utils.safestring import mark_safe

from oioioi.base.views import has_valid_username
from oioioi.su.utils import is_under_su


Expand Down Expand Up @@ -81,3 +86,24 @@ def process_exception(self, request, exception):

except Exception:
pass


class CheckLoginMiddleware(object):
def process_request(self, request):
if has_valid_username(request.user):
return
storage = messages.get_messages(request)
check_login_message = \
_("Your login - %s - contains not allowed characters. "
"Please click <a href='%s'>here</a> to change it. "
"It will take only a while.") \
% (request.user.username, reverse('edit_profile'))

# https://docs.djangoproject.com/en/dev/ref/contrib/messages/#expiration-of-messages
all_messages = [s.message for s in storage]
storage.used = False

if check_login_message in all_messages:
return
messages.add_message(request, messages.INFO,
mark_safe(check_login_message))
86 changes: 86 additions & 0 deletions oioioi/base/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -920,3 +920,89 @@ def example_view(request):
request = self.factory()
res = example_view(request)
self.assertTrue(isinstance(res, HttpResponseRedirect))


class TestLoginChange(TestCase):
fixtures = ['test_users', 'test_contest']

def setUp(self):
self.invalid_logins = ['#', 'p@n', ' user', 'user\xc4\x99']
self.valid_logins = ['test_user', 'user', 'uSeR', 'U__4']
self.user = User.objects.get(username='test_user')
self.client.login(username=self.user.username)

def test_message(self):
url_index = reverse('index')

for login in self.invalid_logins:
self.user.username = login
self.user.save()

response = self.client.get(url_index, follow=True)
self.assertIn('contains not allowed characters', response.content)

for login in self.valid_logins:
self.user.username = login
self.user.save()

response = self.client.get(url_index, follow=True)
self.assertNotIn('contains not allowed characters',
response.content)

def test_login_change(self):
url_index = reverse('index')
url_edit_profile = reverse('edit_profile')

for login in self.invalid_logins:
self.user.username = login
self.user.save()

response = self.client.get(url_edit_profile)
self.assertIn('<input id="id_username" maxlength="30" name='
'"username" type="text" value="%s" />' % (login,),
response.content)

self.client.post(url_edit_profile, {'username': 'valid_user'},
follow=True)
self.assertEqual(self.user.username, login)

response = self.client.post(url_index, follow=True)
self.assertNotIn('contains not allowed characters',
response.content)

response = self.client.get(url_edit_profile)
self.assertIn('<input id="id_username" maxlength="30" name='
'"username" readonly="True" type="text"'
' value="valid_user" />', response.content)

for login in self.valid_logins:
self.user.username = login
self.user.save()

response = self.client.get(url_edit_profile)
self.assertIn('<input id="id_username" maxlength="30" name='
'"username" readonly="True" type="text" value="%s" />'
% (login,), response.content)

response = self.client.post(url_edit_profile,
{'username': 'valid_user'}, follow=True)
self.assertEqual(self.user.username, login)
self.assertIn('You cannot change your username.', response.content)

response = self.client.get(url_index, follow=True)
self.assertNotIn('contains not allowed characters',
response.content)

response = self.client.get(url_edit_profile)
self.assertNotIn('valid_user', response.content)

def test_failed_login_change(self):
url_edit_profile = reverse('edit_profile')

self.user.username = self.invalid_logins[0]
self.user.save()

for login in self.invalid_logins:
self.client.post(url_edit_profile, {'username': login},
follow=True)
self.assertEqual(self.user.username, self.invalid_logins[0])
8 changes: 8 additions & 0 deletions oioioi/base/utils/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import re

USERNAME_REGEX = r'^[a-zA-Z0-9_]+$'


def has_valid_username(user):
return user is None or user.is_anonymous() or \
re.match(USERNAME_REGEX, user.username) is not None
9 changes: 7 additions & 2 deletions oioioi/base/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# pylint: disable=W0703
# Catching too general exception Exception
import re

from django.contrib.auth import REDIRECT_FIELD_NAME
from django.core.exceptions import PermissionDenied
from django.shortcuts import render_to_response, redirect, render
Expand All @@ -15,6 +17,7 @@
login as auth_login
from oioioi.base.permissions import enforce_condition, not_anonymous
from oioioi.base.utils.redirect import safe_redirect
from oioioi.base.utils.user import has_valid_username
from oioioi.contests.views import default_contest_view
from oioioi.base.forms import UserForm
from oioioi.base.menu import account_menu_registry
Expand Down Expand Up @@ -88,12 +91,14 @@ def handler403(request):
@enforce_condition(not_anonymous)
def edit_profile_view(request):
if request.method == 'POST':
form = UserForm(request.POST, instance=request.user)
form = UserForm(request.POST, instance=request.user,
allow_login_change=not has_valid_username(request.user))
if form.is_valid():
form.save()
return redirect(index_view)
else:
form = UserForm(instance=request.user)
form = UserForm(instance=request.user, allow_login_change=not
has_valid_username(request.user))
return TemplateResponse(request, 'registration/registration_form.html',
{'form': form})

Expand Down
1 change: 1 addition & 0 deletions oioioi/default_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
'linaro_django_pagination.middleware.PaginationMiddleware',
'oioioi.contests.middleware.CurrentContestMiddleware',
'oioioi.base.middleware.HttpResponseNotAllowedMiddleware',
'oioioi.base.middleware.CheckLoginMiddleware',
# Uncomment the next line for simple clickjacking protection:
# 'django.middleware.clickjacking.XFrameOptionsMiddleware',
)
Expand Down

0 comments on commit 154c54e

Please sign in to comment.