From 56e2243f3be7e859666ce0e4e1a8b8b94444f8d4 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 12 Aug 2010 12:15:48 +0800 Subject: [PATCH] Use generic email confirmation object Rather than having a UserPerson-specific confirmation, add an EmailConfirmation object to allow multiple types of confirmations (eg, opt-out requests in future). To do this, we use a view (patchwork.views.confirm) that will call the type-specific view with the confirmation object. Also, add tests to check that the User/Person linkage system works. Signed-off-by: Jeremy Kerr --- apps/patchwork/models.py | 29 +++--- apps/patchwork/tests/__init__.py | 1 + apps/patchwork/tests/confirm.py | 67 +++++++++++++ apps/patchwork/tests/user.py | 121 ++++++++++++++++++++++++ apps/patchwork/urls.py | 3 +- apps/patchwork/views/base.py | 24 ++++- apps/patchwork/views/user.py | 24 +++-- lib/sql/grant-all.mysql.sql | 2 +- lib/sql/grant-all.postgres.sql | 4 +- lib/sql/migration/008-confirmations.sql | 11 +++ templates/patchwork/confirm-error.html | 19 ++++ templates/patchwork/user-link.mail | 2 +- 12 files changed, 274 insertions(+), 33 deletions(-) create mode 100644 apps/patchwork/tests/confirm.py create mode 100644 apps/patchwork/tests/user.py create mode 100644 lib/sql/migration/008-confirmations.sql create mode 100644 templates/patchwork/confirm-error.html diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index 6c8fc71..ee6748f 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -373,34 +373,29 @@ class BundlePatch(models.Model): unique_together = [('bundle', 'patch')] ordering = ['order'] -class UserPersonConfirmation(models.Model): - user = models.ForeignKey(User) +class EmailConfirmation(models.Model): + validity = datetime.timedelta(days = 30) + type = models.CharField(max_length = 20, choices = [ + ('userperson', 'User-Person association'), + ]) email = models.CharField(max_length = 200) + user = models.ForeignKey(User, null = True) key = HashField() - date = models.DateTimeField(default=datetime.datetime.now) + date = models.DateTimeField(default = datetime.datetime.now) active = models.BooleanField(default = True) - def confirm(self): - if not self.active: - return - person = None - try: - person = Person.objects.get(email__iexact = self.email) - except Exception: - pass - if not person: - person = Person(email = self.email) - - person.link_to_user(self.user) - person.save() + def deactivate(self): self.active = False self.save() + def is_valid(self): + return self.date + self.validity > datetime.datetime.now() + def save(self): max = 1 << 32 if self.key == '': str = '%s%s%d' % (self.user, self.email, random.randint(0, max)) self.key = self._meta.get_field('key').construct(str).hexdigest() - super(UserPersonConfirmation, self).save() + super(EmailConfirmation, self).save() diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index 68fe563..9618d1f 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -23,3 +23,4 @@ from patchwork.tests.bundles import * from patchwork.tests.mboxviews import * from patchwork.tests.updates import * from patchwork.tests.filters import * +from patchwork.tests.confirm import * diff --git a/apps/patchwork/tests/confirm.py b/apps/patchwork/tests/confirm.py new file mode 100644 index 0000000..fad5125 --- /dev/null +++ b/apps/patchwork/tests/confirm.py @@ -0,0 +1,67 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2011 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +from django.test import TestCase +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from patchwork.models import EmailConfirmation, Person + +def _confirmation_url(conf): + return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + +class TestUser(object): + username = 'testuser' + email = 'test@example.com' + secondary_email = 'test2@example.com' + password = None + + def __init__(self): + self.password = User.objects.make_random_password() + self.user = User.objects.create_user(self.username, + self.email, self.password) + +class InvalidConfirmationTest(TestCase): + def setUp(self): + EmailConfirmation.objects.all().delete() + Person.objects.all().delete() + self.user = TestUser() + self.conf = EmailConfirmation(type = 'userperson', + email = self.user.secondary_email, + user = self.user.user) + self.conf.save() + + def testInactiveConfirmation(self): + self.conf.active = False + self.conf.save() + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/confirm-error.html') + self.assertEqual(response.context['error'], 'inactive') + self.assertEqual(response.context['conf'], self.conf) + + def testExpiredConfirmation(self): + self.conf.date -= self.conf.validity + self.conf.save() + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/confirm-error.html') + self.assertEqual(response.context['error'], 'expired') + self.assertEqual(response.context['conf'], self.conf) + diff --git a/apps/patchwork/tests/user.py b/apps/patchwork/tests/user.py new file mode 100644 index 0000000..c9e5be3 --- /dev/null +++ b/apps/patchwork/tests/user.py @@ -0,0 +1,121 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2010 Jeremy Kerr +# +# This file is part of the Patchwork package. +# +# Patchwork is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# Patchwork is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +import unittest +from django.test import TestCase +from django.test.client import Client +from django.core import mail +from django.core.urlresolvers import reverse +from django.contrib.auth.models import User +from patchwork.models import EmailConfirmation, Person +from patchwork.utils import userprofile_register_callback + +def _confirmation_url(conf): + return reverse('patchwork.views.confirm', kwargs = {'key': conf.key}) + +class TestUser(object): + username = 'testuser' + email = 'test@example.com' + secondary_email = 'test2@example.com' + password = None + + def __init__(self): + self.password = User.objects.make_random_password() + self.user = User.objects.create_user(self.username, + self.email, self.password) + userprofile_register_callback(self.user) + +class UserPersonRequestTest(TestCase): + def setUp(self): + self.user = TestUser() + self.client.login(username = self.user.username, + password = self.user.password) + EmailConfirmation.objects.all().delete() + + def testUserPersonRequestForm(self): + response = self.client.get('/user/link/') + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + + def testUserPersonRequestEmpty(self): + response = self.client.post('/user/link/', {'email': ''}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + self.assertFormError(response, 'linkform', 'email', + 'This field is required.') + + def testUserPersonRequestInvalid(self): + response = self.client.post('/user/link/', {'email': 'foo'}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['linkform']) + self.assertFormError(response, 'linkform', 'email', + 'Enter a valid e-mail address.') + + def testUserPersonRequestValid(self): + response = self.client.post('/user/link/', + {'email': self.user.secondary_email}) + self.assertEquals(response.status_code, 200) + self.assertTrue(response.context['confirmation']) + + # check that we have a confirmation saved + self.assertEquals(EmailConfirmation.objects.count(), 1) + conf = EmailConfirmation.objects.all()[0] + self.assertEquals(conf.user, self.user.user) + self.assertEquals(conf.email, self.user.secondary_email) + self.assertEquals(conf.type, 'userperson') + + # check that an email has gone out... + self.assertEquals(len(mail.outbox), 1) + msg = mail.outbox[0] + self.assertEquals(msg.subject, 'Patchwork email address confirmation') + self.assertTrue(self.user.secondary_email in msg.to) + self.assertTrue(_confirmation_url(conf) in msg.body) + + # ...and that the URL is valid + response = self.client.get(_confirmation_url(conf)) + self.assertEquals(response.status_code, 200) + self.assertTemplateUsed(response, 'patchwork/user-link-confirm.html') + +class UserPersonConfirmTest(TestCase): + def setUp(self): + EmailConfirmation.objects.all().delete() + Person.objects.all().delete() + self.user = TestUser() + self.client.login(username = self.user.username, + password = self.user.password) + self.conf = EmailConfirmation(type = 'userperson', + email = self.user.secondary_email, + user = self.user.user) + self.conf.save() + + def testUserPersonConfirm(self): + self.assertEquals(Person.objects.count(), 1) + response = self.client.get(_confirmation_url(self.conf)) + self.assertEquals(response.status_code, 200) + + # check that the Person object has been created and linked + self.assertEquals(Person.objects.count(), 2) + person = Person.objects.get(email = self.user.secondary_email) + self.assertEquals(person.email, self.user.secondary_email) + self.assertEquals(person.user, self.user.user) + + # check that the confirmation has been marked as inactive. We + # need to reload the confirmation to check this. + conf = EmailConfirmation.objects.get(pk = self.conf.pk) + self.assertEquals(conf.active, False) diff --git a/apps/patchwork/urls.py b/apps/patchwork/urls.py index b49b4e1..27c79fd 100644 --- a/apps/patchwork/urls.py +++ b/apps/patchwork/urls.py @@ -44,13 +44,14 @@ urlpatterns = patterns('', 'patchwork.views.bundle.mbox'), (r'^user/link/$', 'patchwork.views.user.link'), - (r'^user/link/(?P[^/]+)/$', 'patchwork.views.user.link_confirm'), (r'^user/unlink/(?P[^/]+)/$', 'patchwork.views.user.unlink'), # public view for bundles (r'^bundle/(?P[^/]*)/(?P[^/]*)/$', 'patchwork.views.bundle.public'), + (r'^confirm/(?P[0-9a-f]+)/$', 'patchwork.views.confirm'), + # submitter autocomplete (r'^submitter/$', 'patchwork.views.submitter_complete'), diff --git a/apps/patchwork/views/base.py b/apps/patchwork/views/base.py index c0e68ed..1539472 100644 --- a/apps/patchwork/views/base.py +++ b/apps/patchwork/views/base.py @@ -18,7 +18,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -from patchwork.models import Patch, Project, Person +from patchwork.models import Patch, Project, Person, EmailConfirmation from django.shortcuts import render_to_response, get_object_or_404 from django.http import HttpResponse, HttpResponseRedirect, Http404 from patchwork.requestcontext import PatchworkRequestContext @@ -58,6 +58,28 @@ def pwclient(request): response.write(render_to_string('patchwork/pwclient', context)) return response +def confirm(request, key): + import patchwork.views.user + views = { + 'userperson': patchwork.views.user.link_confirm, + } + + conf = get_object_or_404(EmailConfirmation, key = key) + if conf.type not in views: + raise Http404 + + if conf.active and conf.is_valid(): + return views[conf.type](request, conf) + + context = PatchworkRequestContext(request) + context['conf'] = conf + if not conf.active: + context['error'] = 'inactive' + elif not conf.is_valid(): + context['error'] = 'expired' + + return render_to_response('patchwork/confirm-error.html', context) + def submitter_complete(request): search = request.GET.get('q', '') response = HttpResponse(mimetype = "text/plain") diff --git a/apps/patchwork/views/user.py b/apps/patchwork/views/user.py index 1ae3c2d..759a6e3 100644 --- a/apps/patchwork/views/user.py +++ b/apps/patchwork/views/user.py @@ -22,8 +22,7 @@ from django.contrib.auth.decorators import login_required from patchwork.requestcontext import PatchworkRequestContext from django.shortcuts import render_to_response, get_object_or_404 from django.http import HttpResponseRedirect -from patchwork.models import Project, Bundle, Person, UserPersonConfirmation, \ - State +from patchwork.models import Project, Bundle, Person, EmailConfirmation, State from patchwork.forms import UserProfileForm, UserPersonLinkForm from patchwork.filters import DelegateFilter from patchwork.views import generic_list @@ -61,7 +60,8 @@ def link(request): if request.method == 'POST': form = UserPersonLinkForm(request.POST) if form.is_valid(): - conf = UserPersonConfirmation(user = request.user, + conf = EmailConfirmation(type = 'userperson', + user = request.user, email = form.cleaned_data['email']) conf.save() context['confirmation'] = conf @@ -83,15 +83,19 @@ def link(request): return render_to_response('patchwork/user-link.html', context) @login_required -def link_confirm(request, key): +def link_confirm(request, conf): context = PatchworkRequestContext(request) - confirmation = get_object_or_404(UserPersonConfirmation, key = key) - errors = confirmation.confirm() - if errors: - context['errors'] = errors - else: - context['person'] = Person.objects.get(email = confirmation.email) + try: + person = Person.objects.get(email__iexact = conf.email) + except Person.DoesNotExist: + person = Person(email = conf.email) + + person.link_to_user(conf.user) + person.save() + conf.deactivate() + + context['person'] = person return render_to_response('patchwork/user-link-confirm.html', context) diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index 4dd6efb..f60c6b8 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -12,7 +12,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON auth_user_groups TO 'www-data'@localhost GRANT SELECT, UPDATE, INSERT, DELETE ON auth_group TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON auth_user_user_permissions TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON auth_permission TO 'www-data'@localhost; -GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_userpersonconfirmation TO 'www-data'@localhost; +GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_emailconfirmation TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_state TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_comment TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_person TO 'www-data'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 6a1a47d..47c4ad3 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -13,7 +13,7 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON auth_group, auth_user_user_permissions, auth_permission, - patchwork_userpersonconfirmation, + patchwork_emailconfirmation, patchwork_state, patchwork_comment, patchwork_person, @@ -43,7 +43,7 @@ GRANT SELECT, UPDATE ON patchwork_person_id_seq, patchwork_project_id_seq, patchwork_state_id_seq, - patchwork_userpersonconfirmation_id_seq, + patchwork_emailconfirmation_id_seq, patchwork_userprofile_id_seq, patchwork_userprofile_maintainer_projects_id_seq, registration_registrationprofile_id_seq diff --git a/lib/sql/migration/008-confirmations.sql b/lib/sql/migration/008-confirmations.sql new file mode 100644 index 0000000..89437a2 --- /dev/null +++ b/lib/sql/migration/008-confirmations.sql @@ -0,0 +1,11 @@ +BEGIN; +ALTER TABLE "patchwork_userpersonconfirmation" + RENAME TO "patchwork_emailconfirmation"; +ALTER SEQUENCE "patchwork_userpersonconfirmation_id_seq" + RENAME TO "patchwork_emailconfirmation_id_seq"; +ALTER TABLE "patchwork_emailconfirmation" + ALTER COLUMN "user_id" DROP NOT NULL, + ADD COLUMN "type" varchar(20) NOT NULL DEFAULT 'userperson'; +ALTER TABLE "patchwork_emailconfirmation" + ALTER COLUMN "type" DROP DEFAULT; +COMMIT; diff --git a/templates/patchwork/confirm-error.html b/templates/patchwork/confirm-error.html new file mode 100644 index 0000000..81292e2 --- /dev/null +++ b/templates/patchwork/confirm-error.html @@ -0,0 +1,19 @@ +{% extends "base.html" %} + +{% block title %}Confirmation{% endblock %} +{% block heading %}Confirmation{% endblock %} + + +{% block body %} + +{% if error == 'inactive' %} +

This confirmation has already been processed; you've probably visited this +page before.

+{% endif %} + +{% if error == 'expired' %} +

The confirmation has expired. If you'd still like to perform the +{{conf.get_type_display}} process, you'll need to resubmit the request.

+{% endif %} + +{% endblock %} diff --git a/templates/patchwork/user-link.mail b/templates/patchwork/user-link.mail index 5f74d3b..c483181 100644 --- a/templates/patchwork/user-link.mail +++ b/templates/patchwork/user-link.mail @@ -7,6 +7,6 @@ This email is to confirm that you own the email address: So that you can add it to your patchwork profile. You can confirm this email address by visiting the url: - http://{{site.domain}}{% url patchwork.views.user.link_confirm key=confirmation.key %} + http://{{site.domain}}{% url patchwork.views.confirm key=confirmation.key %} Happy patchworking. -- 2.39.2