Mercurial > public > sg101
changeset 905:be233ba7ca31
Reworked registration process.
Previous one proved too challenging for some humans.
Hopefully made it simpler but still unusual to confuse bots.
Increased test coverage also.
| author | Brian Neal <bgneal@gmail.com> |
|---|---|
| date | Sun, 08 Mar 2015 11:06:07 -0500 |
| parents | d4479ebbd118 |
| children | d75da42ba11d 344f7914d421 |
| files | accounts/forms.py accounts/static/js/accounts_code.js accounts/tests/test_views.py accounts/urls.py accounts/views.py sg101/templates/accounts/register.html sg101/templates/accounts/register1.html sg101/templates/accounts/register2.html |
| diffstat | 8 files changed, 274 insertions(+), 155 deletions(-) [+] |
line wrap: on
line diff
--- a/accounts/forms.py Sun Mar 08 11:01:00 2015 -0500 +++ b/accounts/forms.py Sun Mar 08 11:06:07 2015 -0500 @@ -1,6 +1,7 @@ """forms for the accounts application""" import logging +import random from django import forms from django.contrib.auth.models import User @@ -18,6 +19,30 @@ logger = logging.getLogger('auth') +CODE_WORD_CHOICES = [ + 'reverb', + 'sg101', + 'guitar', + 'showman', + 'surf', + 'surfer', + 'tank', + 'splash', + 'gremmie', + 'hodad', + 'stomp', + 'belairs', + 'dickdale', + 'chantays', + 'astronauts', + 'fender', +] + +def generate_registration_code(): + """Generates a simple, random registration code for the user to enter.""" + return '{}-{}'.format(random.choice(CODE_WORD_CHOICES), random.randint(100, 999)) + + class RegisterForm(forms.Form): """Form used to register with the website""" username = forms.RegexField( @@ -56,28 +81,6 @@ required=False, widget=forms.TextInput(attrs={'class': 'text'})) - SPAM_CHOICES = [(1, 'cat'), (2, '328'), (4, 'green'), (8, 'ocean')] - question4 = forms.ChoiceField(label="Pick the number", choices=SPAM_CHOICES) - question5 = forms.IntegerField(label="What number did you pick, above?", - widget=forms.TextInput(attrs={'class': 'text'})) - question6 = forms.ChoiceField(label="Pick the color", choices=SPAM_CHOICES) - - USER_QUALITIES = [ - (1, "I am the lowest form of life, a spammer"), - (2, "I'm a fan of surf music or I want to learn more"), - (3, "Someone is paying me to post links to this site"), - (4, "I am not going to spam this site"), - (5, "I understand my account may be removed if I post spam"), - (6, "I am going to spam the crap out of this site"), - (7, "Surf music is one of my interests, not spam"), - ] - question7 = forms.MultipleChoiceField( - label="Check all that apply", - choices=USER_QUALITIES, - required=False, - widget=forms.CheckboxSelectMultiple) - - def __init__(self, *args, **kwargs): self.ip = kwargs.pop('ip', '?') super(RegisterForm, self).__init__(*args, **kwargs) @@ -150,60 +153,78 @@ self._validation_error('Oops, that should be blank', answer) return answer - def clean_question4(self): - answer = self.cleaned_data.get('question4') - if answer != u'2': - self._validation_error('Please pick the number', answer) - return answer + def save(self, request): + request.session['reg_info'] = { + 'username': self.cleaned_data['username'], + 'email': self.cleaned_data['email'], + 'password': self.cleaned_data['password1'], + 'code': generate_registration_code(), + } - def clean_question5(self): - answer = self.cleaned_data.get('question5') - if answer != 328: - self._validation_error('Please enter the correct number', answer) - return answer + def _validation_error(self, msg, param=None): + logger.error('Accounts/registration [%s]: %s (%s)', self.ip, msg, param) + raise forms.ValidationError(msg) - def clean_question6(self): - answer = self.cleaned_data.get('question6') - if answer != u'4': - self._validation_error('Please pick the color', answer) - return answer - def clean_question7(self): - answer = self.cleaned_data.get('question7') - answer.sort() - if answer != [u'2', u'4', u'5', u'7']: - self._validation_error("Please double-check your checkboxes", answer) - return answer +class RegisterCodeForm(forms.Form): + """Form for processing anti-spam code.""" + code = forms.CharField( + label="Registration code", + widget=forms.TextInput(attrs={'class': 'text'})) + + def __init__(self, *args, **kwargs): + self.session = kwargs.pop('session', None) + self.ip = kwargs.pop('ip', '?') + super(RegisterCodeForm, self).__init__(*args, **kwargs) + + def clean_code(self): + reg_info = self.session.get('reg_info') + saved_code = reg_info.get('code') if reg_info else None + if not saved_code: + self._validation_error("Registration code error") + user_code = self.cleaned_data['code'] + if user_code: + user_code = user_code.strip() + + if user_code != saved_code: + self._validation_error("The registration code does not match") def save(self): - pending_user = PendingUser.objects.create_pending_user(self.cleaned_data['username'], - self.cleaned_data['email'], - self.cleaned_data['password1']) + """Process successful registration.""" + reg_info = self.session['reg_info'] + username = reg_info['username'] + email = reg_info['email'] + password = reg_info['password'] + + pending_user = PendingUser.objects.create_pending_user( + username, + email, + password) # Send the confirmation email - site = Site.objects.get_current() admin_email = settings.DEFAULT_FROM_EMAIL - activation_link = 'http://%s%s' % (site.domain, reverse('accounts.views.register_confirm', - kwargs = {'username' : pending_user.username, 'key' : pending_user.key})) + activation_link = 'http://%s%s' % ( + site.domain, + reverse('accounts-register_confirm', + kwargs={'username': pending_user.username, + 'key': pending_user.key})) - msg = render_to_string('accounts/registration_email.txt', - { - 'site_name' : site.name, - 'site_domain' : site.domain, - 'user_email' : pending_user.email, - 'activation_link' : activation_link, - 'username' : pending_user.username, - 'admin_email' : admin_email, + msg = render_to_string('accounts/registration_email.txt', { + 'site_name' : site.name, + 'site_domain' : site.domain, + 'user_email' : pending_user.email, + 'activation_link' : activation_link, + 'username' : pending_user.username, + 'admin_email' : admin_email, }) subject = 'Registration Confirmation for ' + site.name - send_mail(subject, msg, admin_email, [self.cleaned_data['email']]) + send_mail(subject, msg, admin_email, [email]) logger.info('Accounts/registration conf. email sent to %s for user %s; IP = %s', - self.cleaned_data['email'], pending_user.username, self.ip) - - return pending_user + email, pending_user.username, self.ip) + del self.session['reg_info'] def _validation_error(self, msg, param=None): logger.error('Accounts/registration [%s]: %s (%s)', self.ip, msg, param)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/accounts/static/js/accounts_code.js Sun Mar 08 11:06:07 2015 -0500 @@ -0,0 +1,5 @@ +$(document).ready(function() { + $.get('/accounts/register/code/', function(data) { + $('#reg-code-block').text(data.code); + }); +});
--- a/accounts/tests/test_views.py Sun Mar 08 11:01:00 2015 -0500 +++ b/accounts/tests/test_views.py Sun Mar 08 11:06:07 2015 -0500 @@ -46,10 +46,6 @@ 'question1': '101', 'question2': '', 'question3': '', - 'question4': u'2', - 'question5': u'328', - 'question6': u'4', - 'question7': [u'2', u'4', u'5', u'7'], } def test_get_view(self): @@ -75,6 +71,7 @@ 'agree_privacy': 'on', 'question1': '101', 'question2': '', + 'question3': '', }) self.assertEqual(response.status_code, 200) @@ -177,89 +174,6 @@ self.post_vals) self.assertEqual(response.status_code, 200) - def test_question4(self): - """ - Ensure our security question 4 works - - """ - self.post_vals['question4'] = u'1' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question4'] = u'4' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question4'] = u'8' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - def test_question5(self): - """ - Ensure our security question 5 works - - """ - self.post_vals['question5'] = u'1' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question5'] = u'X' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question5'] = u'2983' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - def test_question6(self): - """ - Ensure our security question 6 works - - """ - self.post_vals['question6'] = u'1' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question6'] = u'2' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question6'] = u'8' - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - def test_question7(self): - """Test security question 7""" - - self.post_vals['question7'] = [] - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question7'] = [u'1'] - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question7'] = [u'6', u'2', u'4', u'5', u'7'] - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - - self.post_vals['question7'] = [u'4', u'3', u'7'] - response = self.client.post(reverse('accounts-register'), - self.post_vals) - self.assertEqual(response.status_code, 200) - def test_success(self): """ Ensure we can successfully register. @@ -267,7 +181,59 @@ """ response = self.client.post(reverse('accounts-register'), self.post_vals) - self.assertEqual(response.status_code, 302) + self.assertRedirects(response, reverse('accounts-register1')) + + # No pending user should exist yet + try: + pending = PendingUser.objects.get(username='a_new_user') + except PendingUser.DoesNotExist: + pass + else: + self.fail("PendingUser was created early") + + # Should have created a reg_info dict in the session + reg_info = self.client.session.get('reg_info') + self.assertEqual(reg_info, { + 'username': self.post_vals['username'], + 'email': self.post_vals['email'], + 'password': self.post_vals['password1'], + 'code': reg_info['code'], + }) + code = reg_info['code'] + match = re.match(r'\w+-\d{3}', code) + self.assertIsNotNone(match) + + # Get the next page + response = self.client.get(reverse('accounts-register2')) + self.assertEqual(response.status_code, 200) + + # No pending user should exist yet + try: + pending = PendingUser.objects.get(username='a_new_user') + except PendingUser.DoesNotExist: + pass + else: + self.fail("PendingUser was created early") + + # Try bad code + response = self.client.post(reverse('accounts-register2'), + {'code': code + code }) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "The registration code does not match") + + # No pending user should exist yet + try: + pending = PendingUser.objects.get(username='a_new_user') + except PendingUser.DoesNotExist: + pass + else: + self.fail("PendingUser was created early") + + # Try good code + response = self.client.post(reverse('accounts-register2'), + {'code': code }) + self.assertRedirects(response, reverse('accounts-register_thanks')) + self.assertIsNone(self.client.session.get('reg_info')) try: pending = PendingUser.objects.get(username='a_new_user') @@ -279,6 +245,44 @@ datetime.timedelta(minutes=1)) self.assertTrue(check_password('my_password', pending.password)) + self.assertEqual(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertTrue(msg.subject.startswith('Registration Confirmation')) + self.assertTrue(len(msg.to) == 1 and msg.to[0] == pending.email) + msg_text = msg.message().as_string() + + activation_link = 'http://example.com%s' % ( + reverse('accounts-register_confirm', + kwargs={'username': pending.username, + 'key': pending.key})) + self.assertTrue(activation_link in msg_text) + + # Vist confirm link + response = self.client.get(reverse('accounts-register_confirm', + kwargs={'username': pending.username, + 'key': pending.key})) + self.assertEqual(response.status_code, 200) + + try: + pending = PendingUser.objects.get(username='a_new_user') + except PendingUser.DoesNotExist: + pass + else: + self.fail("PendingUser was not deleted upon confirmation") + + user = User.objects.get(username=pending.username) + self.assertEqual(user.email, pending.email) + now = datetime.datetime.now() + delta = datetime.timedelta(seconds=10) + self.assertTrue(now - user.last_login < delta) + self.assertTrue(now - user.date_joined < delta) + self.assertEqual(user.password, pending.password) + self.assertEqual(user.first_name, '') + self.assertEqual(user.last_name, '') + self.assertFalse(user.is_staff) + self.assertTrue(user.is_active) + self.assertFalse(user.is_superuser) + class ForgotUsernameTest(TestCase):
--- a/accounts/urls.py Sun Mar 08 11:01:00 2015 -0500 +++ b/accounts/urls.py Sun Mar 08 11:06:07 2015 -0500 @@ -5,8 +5,13 @@ urlpatterns = patterns('accounts.views', url(r'^register/$', 'register', name='accounts-register'), - (r'^register/thanks/$', 'register_thanks'), - (r'^register/confirm/(?P<username>[\w.@+-]{1,30})/(?P<key>[a-zA-Z0-9]{20})/$', 'register_confirm'), + url(r'^register/1/$', 'register1', name='accounts-register1'), + url(r'^register/2/$', 'register2', name='accounts-register2'), + url(r'^register/code/$', 'get_code', name='accounts-code'), + url(r'^register/thanks/$', 'register_thanks', name='accounts-register_thanks'), + url(r'^register/confirm/(?P<username>[\w.@+-]{1,30})/(?P<key>[a-zA-Z0-9]{20})/$', + 'register_confirm', + name='accounts-register_confirm'), ) urlpatterns += patterns('',
--- a/accounts/views.py Sun Mar 08 11:01:00 2015 -0500 +++ b/accounts/views.py Sun Mar 08 11:06:07 2015 -0500 @@ -2,13 +2,17 @@ Views for the accounts application. """ +import json import logging +from django.http import HttpResponse from django.shortcuts import render, redirect from django.conf import settings from accounts.models import PendingUser -from accounts.forms import RegisterForm, ForgotUsernameForm +from accounts.forms import RegisterForm +from accounts.forms import RegisterCodeForm +from accounts.forms import ForgotUsernameForm from accounts import create_new_user from antispam.decorators import log_auth_failures @@ -25,8 +29,8 @@ if request.method == 'POST': form = RegisterForm(request.POST, ip=request.META.get('REMOTE_ADDR', '?')) if form.is_valid(): - form.save() - return redirect('accounts.views.register_thanks') + form.save(request) + return redirect('accounts-register1') else: form = RegisterForm() @@ -34,6 +38,45 @@ ####################################################################### +def register1(request): + """Displays the registration code.""" + if request.user.is_authenticated(): + return redirect(settings.LOGIN_REDIRECT_URL) + + return render(request, 'accounts/register1.html') + +####################################################################### + +@log_auth_failures('Register') +def register2(request): + """Processes the registration code and creates the user.""" + if request.user.is_authenticated(): + return redirect(settings.LOGIN_REDIRECT_URL) + + if request.method == 'POST': + form = RegisterCodeForm(request.POST, + session=request.session, + ip=request.META.get('REMOTE_ADDR', '?')) + if form.is_valid(): + form.save() + return redirect('accounts-register_thanks') + else: + form = RegisterCodeForm() + + return render(request, 'accounts/register2.html', {'form': form}) + +####################################################################### + +def get_code(request): + code = {'code': 'FAIL-123'} + reg_info = request.session.get('reg_info') + if reg_info: + code['code'] = reg_info.get('code', code['code']) + + return HttpResponse(json.dumps(code), content_type='application/json') + +####################################################################### + def register_thanks(request): if request.user.is_authenticated(): return redirect(settings.LOGIN_REDIRECT_URL)
--- a/sg101/templates/accounts/register.html Sun Mar 08 11:01:00 2015 -0500 +++ b/sg101/templates/accounts/register.html Sun Mar 08 11:06:07 2015 -0500 @@ -24,7 +24,7 @@ <form action="." method="post">{% csrf_token %} <table> {{ form.as_table }} - <tr><td> </td><td><input type="submit" value="Register" /></td></tr> + <tr><td> </td><td><input type="submit" value="Next step..." /></td></tr> </table> </form> {% endblock %}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/sg101/templates/accounts/register1.html Sun Mar 08 11:06:07 2015 -0500 @@ -0,0 +1,27 @@ +{% extends 'base.html' %} +{% block custom_js %} +<script type="text/javascript" src="{{ STATIC_URL }}js/accounts_code.js"></script> +{% endblock %} +{% block title %}New User Registration Step 2{% endblock %} +{% block content %} +<h2>New User Registration Step 2</h2> +<p> +Thank you! You are almost finished with your registration! Just one more step. +</p> +<p> +Please remember or write down the following code as we will ask you to type +it on the next page. +</p> +<p> + <strong class="large"> + <span id="reg-code-block">Please wait for the registration code to appear...</span> + </strong> +</p> +<p> +Got the code memorized or written down? Go to the next page and enter it. +</p> + +<form action="{% url 'accounts-register2' %}" method="get"> +<button>Continue...</button> +</form> +{% endblock %}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/sg101/templates/accounts/register2.html Sun Mar 08 11:06:07 2015 -0500 @@ -0,0 +1,14 @@ +{% extends 'base.html' %} +{% block title %}New User Registration Final Step{% endblock %} +{% block content %} +<h2>New User Registration Final Step</h2> +<p> +Please enter the registration code you received in the previous step. +</p> +<form action="." method="post">{% csrf_token %} +<table> + {{ form.as_table }} + <tr><td> </td><td><input type="submit" value="Complete Registration" /></td></tr> +</table> +</form> +{% endblock %}