From d88bbbc945833cb82fc167e3046e6fbbb689a313 Mon Sep 17 00:00:00 2001 From: Wil Clouser Date: Mon, 29 Mar 2010 08:09:52 -0700 Subject: [PATCH] Add Registration, Confirmation, and Confirmation resend --- apps/users/forms.py | 54 +++++++++- apps/users/models.py | 15 +++ apps/users/templates/email/confirm.ltxt | 15 +++ apps/users/templates/login.html | 12 ++- apps/users/templates/users/register.html | 101 +++++++++++++++++++ apps/users/tests/test_forms.py | 115 +++++++++++++++++++++ apps/users/tests/test_models.py | 11 +++ apps/users/tests/test_views.py | 73 +++++++------- apps/users/urls.py | 8 +- apps/users/views.py | 121 ++++++++++++++++++++++- 10 files changed, 478 insertions(+), 47 deletions(-) create mode 100644 apps/users/templates/email/confirm.ltxt create mode 100644 apps/users/templates/users/register.html diff --git a/apps/users/forms.py b/apps/users/forms.py index 15b9b90056..0ee45044a4 100644 --- a/apps/users/forms.py +++ b/apps/users/forms.py @@ -1,7 +1,7 @@ import logging from django import forms -from django.forms.util import ErrorList from django.contrib.auth import forms as auth_forms +from django.forms.util import ErrorList from l10n import ugettext as _ @@ -139,3 +139,55 @@ class UserEditForm(forms.ModelForm): log.debug('User (%s) updated their profile', amouser) amouser.save() + + +class UserRegisterForm(forms.ModelForm): + """For registering users. We're not building off + d.contrib.auth.forms.UserCreationForm because it doesn't do a lot of the + details here, so we'd have to rewrite most of it anyway.""" + password = forms.CharField(max_length=255, required=False, + widget=forms.PasswordInput(render_value=False)) + password2 = forms.CharField(max_length=255, required=False, + widget=forms.PasswordInput(render_value=False)) + + class Meta: + model = models.UserProfile + + def clean_nickname(self): + """We're breaking the rules and allowing null=True and blank=True on a + CharField because I want to enforce uniqueness in the db. In order to + let save() work, I override '' here.""" + n = self.cleaned_data['nickname'] + if n == '': + n = None + return n + + def clean(self): + super(UserRegisterForm, self).clean() + + data = self.cleaned_data + + # Passwords + p1 = data.get("password") + p2 = data.get("password2") + + if p1 != p2: + msg = _("The passwords did not match.") + self._errors["password2"] = ErrorList([msg]) + #del data["password"] + del data["password2"] + + # Names + if not ("nickname" in self._errors or + "firstname" in self._errors or + "lastname" in self._errors): + fname = data.get("firstname") + lname = data.get("lastname") + nname = data.get("nickname") + if not (fname or lname or nname): + msg = _("A first name, last name or nickname is required.") + self._errors["firstname"] = ErrorList([msg]) + self._errors["lastname"] = ErrorList([msg]) + self._errors["nickname"] = ErrorList([msg]) + + return data diff --git a/apps/users/models.py b/apps/users/models.py index 95cc868ac3..061d28d7f7 100644 --- a/apps/users/models.py +++ b/apps/users/models.py @@ -8,12 +8,15 @@ import time from django.conf import settings from django.contrib.auth.models import User as DjangoUser +from django.core.mail import send_mail from django.db import models +from django.template import Context, loader import amo import amo.models from amo.urlresolvers import reverse +from l10n import ugettext as _ from translations.fields import PurifiedField log = logging.getLogger('z.users') @@ -156,6 +159,18 @@ class UserProfile(amo.models.ModelBase): def set_password(self, raw_password, algorithm='sha512'): self.password = create_password(algorithm, raw_password) + def email_confirmation_code(self): + log.debug("Sending account confirmation code for user (%s)", self) + + url = "%s%s" % (settings.SITE_URL, + reverse('users.confirm', + args=[self.id, self.confirmationcode])) + domain = settings.DOMAIN + t = loader.get_template('email/confirm.ltxt') + c = {'domain': domain, 'url': url, } + send_mail(_("Please confirm your email address"), + t.render(Context(c)), None, [self.email]) + def create_django_user(self): """Make a django.contrib.auth.User for this UserProfile.""" # Reusing the id will make our life easier, because we can use the diff --git a/apps/users/templates/email/confirm.ltxt b/apps/users/templates/email/confirm.ltxt new file mode 100644 index 0000000000..018b3e86f1 --- /dev/null +++ b/apps/users/templates/email/confirm.ltxt @@ -0,0 +1,15 @@ +{% load i18n %} +{# L10n: This is an email. Whitespace matters! #} +{% blocktrans %} +Welcome to {{ domain }}. + +Before you can use your new account you must activate it - this ensures the e-mail address you used is valid and belongs to you. To activate your account, click the link below or copy and paste the whole thing into your browser's location bar: + +{{ url }} + +Once you've successfully activated your account, you can throw away this e-mail. + +Thanks! + +The {{ domain }} staff +{% endblocktrans %} diff --git a/apps/users/templates/login.html b/apps/users/templates/login.html index 7b7e80686d..49b1a07997 100644 --- a/apps/users/templates/login.html +++ b/apps/users/templates/login.html @@ -5,6 +5,15 @@ {% block content %}
+ {% if messages %} + {% for message in messages %} + {% if message %} +
+

{{ message|safe }}

+
+ {% endif %} + {% endfor %} + {% endif %} {% if form.non_field_errors() %}
    @@ -52,8 +61,7 @@

    {{ _('Login Problems?') }}

    diff --git a/apps/users/templates/users/register.html b/apps/users/templates/users/register.html new file mode 100644 index 0000000000..e7f6f092e3 --- /dev/null +++ b/apps/users/templates/users/register.html @@ -0,0 +1,101 @@ +{% extends "base.html" %} + +{% block title %}{{ page_title(_('New User Registration')) }}{% endblock %} + +{% block content %} + +
    +
    + {% if messages %} + {% for message in messages %} +
    +

    {{ message }}

    +
    + {% endfor %} + {% endif %} + + {% if form %} +
    + {{ csrf() }} +
    + {{ _('Register') }} +
      +
    • + + {{ form.email|safe }} + {{ form.email.errors|safe }} +
      • +
    • + + {{ form.emailhidden.errors|safe }} +
      • +
    • + + {{ form.password|safe }} + {{ form.password.errors|safe }} +
      • +
    • + + {{ form.password2|safe }} + {{ form.password2.errors|safe }} +
      • +
    • + + {{ form.firstname|safe }} + {{ form.firstname.errors|safe }} +
      • +
    • + + {{ form.lastname|safe }} + {{ form.lastname.errors|safe }} +
      • +
    • + + {{ form.nickname|safe }} + {{ form.nickname.errors|safe }} +
      • +
    • + + {{ form.homepage|safe }} + {{ form.homepage.errors|safe }} +
      • +
    +
    +
    + +
    +
    + {% endif %} +
    +
    +
    +

    {{ _('Why register?') }}

    + {% trans legal="http://www.mozilla.com/about/legal.html", + privacy="http://www.mozilla.com/privacy-policy.html" %} +

    Registration on AMO is not required if you + simply want to download and install public add-ons.

    +

    You only need to register if:

    +
      +
    • You want to submit reviews for add-ons
      • +
    • You want to keep track of your favorite add-on collections or create one yourself
      • +
    • You are an add-on developer and want to upload your add-on for hosting on AMO
      • +
    +

    Upon successful registration, you will be sent a confirmation + email to the address you provided. Please follow the instructions + there to confirm your account.

    If you like, you can read our + Legal Notices and + Privacy Policy.

    + {% endtrans %} +
    +
    {# .primary #} + +{% endblock content %} diff --git a/apps/users/tests/test_forms.py b/apps/users/tests/test_forms.py index 393d450624..fdf94ad719 100644 --- a/apps/users/tests/test_forms.py +++ b/apps/users/tests/test_forms.py @@ -5,6 +5,7 @@ from django.core import mail from django.test.client import Client from django.utils.http import int_to_base36 +from manage import settings from nose.tools import eq_ @@ -158,3 +159,117 @@ class TestUserEditForm(UserFormBase): 'newpassword2': 'new', } r = self.client.post('/en-US/firefox/users/edit', data) self.assertContains(r, "Profile Updated") + + +class TestUserLoginForm(UserFormBase): + + def _get_login_url(self): + return "/en-US/firefox/users/login" + + def test_credential_fail(self): + url = self._get_login_url() + r = self.client.post(url, {'username': '', 'password': ''}) + self.assertFormError(r, 'form', 'username', "This field is required.") + self.assertFormError(r, 'form', 'password', "This field is required.") + + r = self.client.post(url, {'username': 'jbalogh@mozilla.com', + 'password': 'wrongpassword'}) + self.assertFormError(r, 'form', '', ("Please enter a correct username " + "and password. Note that both " + "fields are case-sensitive.")) + + def test_credential_success(self): + url = self._get_login_url() + r = self.client.post(url, {'username': 'jbalogh@mozilla.com', + 'password': 'foo'}, follow=True) + self.assertContains(r, "Welcome, Jeff") + self.assertTrue(self.client.session.get_expire_at_browser_close()) + + r = self.client.post(url, {'username': 'jbalogh@mozilla.com', + 'password': 'foo', + 'rememberme': 1}, follow=True) + self.assertContains(r, "Welcome, Jeff") + # Subtract 100 to give some breathing room + age = settings.SESSION_COOKIE_AGE - 100 + assert self.client.session.get_expiry_age() > age + + def test_unconfirmed_account(self): + url = self._get_login_url() + self.user_profile.confirmationcode = 'blah' + self.user_profile.save() + r = self.client.post(url, {'username': 'jbalogh@mozilla.com', + 'password': 'foo'}, follow=True) + self.assertNotContains(r, "Welcome, Jeff") + self.assertContains(r, "A link to activate your user account") + self.assertContains(r, "If you did not receive the confirmation") + + def test_disabled_account(self): + url = self._get_login_url() + self.user_profile.deleted = True + self.user_profile.save() + r = self.client.post(url, {'username': 'jbalogh@mozilla.com', + 'password': 'foo'}, follow=True) + self.assertNotContains(r, "Welcome, Jeff") + self.assertContains(r, "Wrong email address or password!") + + +class TestUserRegisterForm(UserFormBase): + + def test_no_info(self): + data = {'email': '', + 'password': '', + 'password2': '', + 'firstname': '', + 'lastname': '', + 'nickname': '', } + r = self.client.post('/en-US/firefox/users/register', data) + self.assertFormError(r, 'form', 'email', + 'This field is required.') + msg = "A first name, last name or nickname is required." + self.assertFormError(r, 'form', 'nickname', msg) + self.assertFormError(r, 'form', 'firstname', msg) + self.assertFormError(r, 'form', 'lastname', msg) + + def test_register_existing_account(self): + data = {'email': 'jbalogh@mozilla.com', + 'password': 'xxx', + 'password2': 'xxx', + 'firstname': 'xxx', } + r = self.client.post('/en-US/firefox/users/register', data) + self.assertFormError(r, 'form', 'email', + 'User profile with this Email already exists.') + eq_(len(mail.outbox), 0) + + def test_set_unmatched_passwords(self): + data = {'email': 'john.connor@sky.net', + 'password': 'new1', + 'password2': 'new2', } + r = self.client.post('/en-US/firefox/users/register', data) + self.assertFormError(r, 'form', 'password2', + 'The passwords did not match.') + eq_(len(mail.outbox), 0) + + def test_already_logged_in(self): + self.client.login(username='jbalogh@mozilla.com', password='foo') + r = self.client.get('/users/register', follow=True) + self.assertContains(r, "You are already logged in") + self.assertNotContains(r, '') + + def test_success(self): + data = {'email': 'john.connor@sky.net', + 'password': 'carebears', + 'password2': 'carebears', + 'firstname': 'John', + 'lastname': 'Connor', + 'nickname': 'BigJC', + 'homepage': ''} + r = self.client.post('/en-US/firefox/users/register', data) + self.assertContains(r, "Congratulations!") + + u = User.objects.get(email='john.connor@sky.net').get_profile() + + assert u.confirmationcode + eq_(len(mail.outbox), 1) + assert mail.outbox[0].subject.find('Please confirm your email') == 0 + assert mail.outbox[0].body.find('%s/confirm/%s' % + (u.id, u.confirmationcode)) > 0 diff --git a/apps/users/tests/test_models.py b/apps/users/tests/test_models.py index 9ddb463f5a..d67ca067cc 100644 --- a/apps/users/tests/test_models.py +++ b/apps/users/tests/test_models.py @@ -3,6 +3,7 @@ import hashlib from django import test from django.contrib.auth.models import User +from django.core import mail from nose.tools import eq_ @@ -27,6 +28,16 @@ class TestUserProfile(test.TestCase): u = UserProfile(nickname='Terminator', pk=1) eq_(u.display_name, 'Terminator') + def test_email_confirmation_code(self): + u = User.objects.get(id='4043307').get_profile() + u.confirmationcode = 'blah' + u.email_confirmation_code() + + eq_(len(mail.outbox), 1) + assert mail.outbox[0].subject.find('Please confirm your email') == 0 + assert mail.outbox[0].body.find('%s/confirm/%s' % + (u.id, u.confirmationcode)) > 0 + def test_welcome_name(self): u1 = UserProfile(lastname='Connor') u2 = UserProfile(firstname='Sarah', nickname='sc', lastname='Connor') diff --git a/apps/users/tests/test_views.py b/apps/users/tests/test_views.py index 30ae0c3080..a77b9606ef 100644 --- a/apps/users/tests/test_views.py +++ b/apps/users/tests/test_views.py @@ -6,8 +6,6 @@ from django.test.client import Client from nose.tools import eq_ -from manage import settings - from users.utils import EmailResetCode @@ -48,7 +46,8 @@ class TestEmailChange(UserViewBase): def setUp(self): super(TestEmailChange, self).setUp() - self.token, self.hash = EmailResetCode.create(self.user.id, 'nobody@mozilla.org') + self.token, self.hash = EmailResetCode.create(self.user.id, + 'nobody@mozilla.org') def test_fail(self): # Completely invalid user, valid code @@ -79,37 +78,7 @@ class TestEmailChange(UserViewBase): class TestLogin(UserViewBase): - def _get_login_url(self): - return "/en-US/firefox/users/login" - - def test_credential_fail(self): - url = self._get_login_url() - r = self.client.post(url, {'username': '', 'password': ''}) - self.assertFormError(r, 'form', 'username', "This field is required.") - self.assertFormError(r, 'form', 'password', "This field is required.") - - r = self.client.post(url, {'username': 'jbalogh@mozilla.com', - 'password': 'wrongpassword'}) - self.assertFormError(r, 'form', '', ("Please enter a correct username " - "and password. Note that both " - "fields are case-sensitive.")) - - def test_credential_success(self): - url = self._get_login_url() - r = self.client.post(url, {'username': 'jbalogh@mozilla.com', - 'password': 'foo'}, follow=True) - self.assertContains(r, "Welcome, Jeff") - self.assertTrue(self.client.session.get_expire_at_browser_close()) - - r = self.client.post(url, {'username': 'jbalogh@mozilla.com', - 'password': 'foo', - 'rememberme': 1}, follow=True) - self.assertContains(r, "Welcome, Jeff") - # Subtract 100 to give some breathing room - age = settings.SESSION_COOKIE_AGE - 100 - assert self.client.session.get_expiry_age() > age - - def test_test_client_login(self): + def test_client_login(self): """This is just here to make sure Test Client's login() works with our custom code.""" assert not self.client.login(username='jbalogh@mozilla.com', @@ -130,5 +99,37 @@ class TestLogout(UserViewBase): self.assertContains(r, "Log in") -class TestProfile(UserViewBase): - pass +class TestRegistration(UserViewBase): + + def test_confirm(self): + # User doesn't have a confirmation code + url = reverse('users.confirm', args=[self.user.id, 'code']) + r = self.client.get(url, follow=True) + self.assertContains(r, '') + + self.user_profile.confirmationcode = "code" + self.user_profile.save() + + # URL has the wrong confirmation code + url = reverse('users.confirm', args=[self.user.id, 'blah']) + r = self.client.get(url, follow=True) + eq_(r.status_code, 400) + + # URL has the right confirmation code + url = reverse('users.confirm', args=[self.user.id, 'code']) + r = self.client.get(url, follow=True) + self.assertContains(r, 'Successfully verified!') + + def test_confirm_resend(self): + # User doesn't have a confirmation code + url = reverse('users.confirm.resend', args=[self.user.id]) + r = self.client.get(url, follow=True) + self.assertContains(r, '') + + self.user_profile.confirmationcode = "code" + self.user_profile.save() + + # URL has the wrong confirmation code + url = reverse('users.confirm.resend', args=[self.user.id]) + r = self.client.get(url, follow=True) + self.assertContains(r, 'An email has been sent to your address') diff --git a/apps/users/urls.py b/apps/users/urls.py index 0021b11840..ade42f3224 100644 --- a/apps/users/urls.py +++ b/apps/users/urls.py @@ -7,6 +7,8 @@ from . import views # These will all start with /user// detail_patterns = patterns('', url('^$', views.profile, name='users.profile'), + url('^confirm/resend$', views.confirm_resend, name='users.confirm.resend'), + url('^confirm/(?P[-\w]+)$', views.confirm, name='users.confirm'), url(r'^emailchange/(?P[-\w]+={0,3})/(?P[\w]+)$', views.emailchange, name="users.emailchange"), ) @@ -17,9 +19,9 @@ urlpatterns = patterns('', url('^users/delete$', views.delete, name='users.delete'), url('^users/edit$', views.edit, name='users.edit'), - - url(r'^users/login', views.login, name='users.login'), - url(r'^users/logout', views.logout, name='users.logout'), + url('^users/login', views.login, name='users.login'), + url('^users/logout', views.logout, name='users.logout'), + url('^users/register$', views.register, name='users.register'), # Password reset stuff url(r'^users/pwreset/?$', auth_views.password_reset, diff --git a/apps/users/views.py b/apps/users/views.py index b28121099b..181dfc21a5 100644 --- a/apps/users/views.py +++ b/apps/users/views.py @@ -1,4 +1,6 @@ import logging +import string +from random import Random from django import http from django.core.mail import send_mail from django.core.urlresolvers import reverse @@ -25,6 +27,45 @@ from users.utils import EmailResetCode log = logging.getLogger('z.users') +def confirm(request, user_id, token): + user = get_object_or_404(UserProfile, id=user_id) + + if not user.confirmationcode: + return http.HttpResponseRedirect(reverse('users.login')) + + if user.confirmationcode != token: + log.info("Account confirmation failed for user (%s)", user) + return http.HttpResponse(status=400) + + user.confirmationcode = '' + user.save() + messages.success(request, _('Successfully verified!')) + log.info("Account confirmed for user (%s)", user) + + return http.HttpResponseRedirect(reverse('users.login')) + + +def confirm_resend(request, user_id): + user = get_object_or_404(UserProfile, id=user_id) + + if not user.confirmationcode: + return http.HttpResponseRedirect(reverse('users.login')) + + # Potential for flood here if someone requests a confirmationcode and then + # re-requests confirmations. We may need to track requests in the future. + log.info("Account confirm re-requested for user (%s)", user) + + user.email_confirmation_code() + + msg = _(('An email has been sent to your address {0} to confirm ' + 'your account. Before you can log in, you have to activate ' + 'your account by clicking on the link provided in this ' + 'email.').format(user.email)) + messages.info(request, msg) + + return http.HttpResponseRedirect(reverse('users.login')) + + @login_required def delete(request): amouser = request.user.get_profile() @@ -124,15 +165,42 @@ def login(request): logout(request) r = auth.views.login(request, template_name='login.html', authentication_form=forms.AuthenticationForm) - form = forms.AuthenticationForm(data=request.POST) - form.is_valid() # clean the data if isinstance(r, HttpResponseRedirect): - # Succsesful log in + # Succsesful log in according to django. Now we do our checks. I do + # the checks here instead of the form's clean() because I want to use + # the messages framework and it's not available in the request there user = request.user.get_profile() - if form.cleaned_data['rememberme']: + + if user.deleted: + logout(request) + log.warning('Attempt to log in with deleted account (%s)' % user) + messages.error(request, _('Wrong email address or password!')) + return jingo.render(request, 'login.html', + {'form': forms.AuthenticationForm()}) + + if user.confirmationcode: + logout(request) + log.info('Attempt to log in with unconfirmed account (%s)' % user) + msg1 = _(('A link to activate your user account was sent by email ' + 'to your address {0}. You have to click it before you ' + 'can log in.').format(user.email)) + url = "%s%s" % (settings.SITE_URL, + reverse('users.confirm.resend', args=[user.id])) + msg2 = _(('If you did not receive the confirmation email, make ' + 'sure your email service did not mark it as "junk ' + 'mail" or "spam". If you need to, you can have us ' + 'resend the confirmation message ' + 'to your email address mentioned above.') % url) + messages.error(request, msg1) + messages.info(request, msg2) + return jingo.render(request, 'login.html', + {'form': forms.AuthenticationForm()}) + + rememberme = request.POST.get('rememberme', None) + if rememberme: request.session.set_expiry(settings.SESSION_COOKIE_AGE) - log.debug(('User (%s) logged in successfully with' + log.debug(('User (%s) logged in successfully with ' '"remember me" set') % user) else: log.debug("User (%s) logged in successfully" % user) @@ -180,3 +248,46 @@ def profile(request, user_id): return jingo.render(request, 'users/profile.html', {'profile': user, 'own_coll': own_coll, 'fav_coll': fav_coll}) + + +def register(request): + if request.user.is_authenticated(): + messages.info(request, _("You are already logged in to an account.")) + form = None + elif request.method == 'POST': + form = forms.UserRegisterForm(request.POST) + if form.is_valid(): + data = request.POST + u = UserProfile() + u.email = data.get('email') + u.emailhidden = data.get('emailhidden', False) + u.firstname = data.get('firstname', None) + u.lastname = data.get('lastname', None) + u.nickname = data.get('nickname', None) + u.homepage = data.get('homepage', None) + u.deleted = False # This defaults to true...? + + u.set_password(data.get('password')) + u.confirmationcode = ''.join(Random().sample( + string.letters + string.digits, 60)) + + u.save() + u.create_django_user() + log.info("Registered new account for user (%s)", u) + + u.email_confirmation_code() + + messages.success(request, _(('Congratulations! Your user account ' + 'was successfully created.'))) + msg = _(('An email has been sent to your address {0} to confirm ' + 'your account. Before you can log in, you have to ' + 'activate your account by clicking on the link provided ' + ' in this email.').format(u.email)) + messages.info(request, msg) + form = None + else: + messages.error(request, _(('There are errors in this form. Please ' + 'correct them and resubmit.'))) + else: + form = forms.UserRegisterForm() + return jingo.render(request, 'users/register.html', {'form': form, })