From 011ee687fda0d3baf66831279565c14e411eab11 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 22 Apr 2014 20:08:27 +0800 Subject: [PATCH] Add unconfirmed registration expiry Currently, unconfirmed registrations remain in the database. Although we have an expiry for the registrations, we don't actually remove rows from the database. This can clog the admin interface up with unnecessary registration spam. We currently have a patchwork cron script to send notifications on patch changes, so hook this into a new do_expiry function. Signed-off-by: Jeremy Kerr --- apps/patchwork/bin/patchwork-cron.py | 4 +- apps/patchwork/models.py | 3 +- apps/patchwork/tests/__init__.py | 1 + apps/patchwork/tests/expiry.py | 121 +++++++++++++++++++++++++++ apps/patchwork/utils.py | 26 +++++- 5 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 apps/patchwork/tests/expiry.py diff --git a/apps/patchwork/bin/patchwork-cron.py b/apps/patchwork/bin/patchwork-cron.py index e9bd0c1..148e97c 100755 --- a/apps/patchwork/bin/patchwork-cron.py +++ b/apps/patchwork/bin/patchwork-cron.py @@ -1,13 +1,15 @@ #!/usr/bin/env python import sys -from patchwork.utils import send_notifications +from patchwork.utils import send_notifications, do_expiry def main(args): errors = send_notifications() for (recipient, error) in errors: print "Failed sending to %s: %s" % (recipient.email, ex) + do_expiry() + if __name__ == '__main__': sys.exit(main(sys.argv)) diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index ec5727d..7371d8f 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -31,7 +31,8 @@ import random class Person(models.Model): email = models.CharField(max_length=255, unique = True) name = models.CharField(max_length=255, null = True, blank = True) - user = models.ForeignKey(User, null = True, blank = True) + user = models.ForeignKey(User, null = True, blank = True, + on_delete = models.SET_NULL) def __unicode__(self): if self.name: diff --git a/apps/patchwork/tests/__init__.py b/apps/patchwork/tests/__init__.py index e4bf42c..5dfcdee 100644 --- a/apps/patchwork/tests/__init__.py +++ b/apps/patchwork/tests/__init__.py @@ -30,3 +30,4 @@ from patchwork.tests.mail_settings import * from patchwork.tests.notifications import * from patchwork.tests.list import * from patchwork.tests.person import * +from patchwork.tests.expiry import * diff --git a/apps/patchwork/tests/expiry.py b/apps/patchwork/tests/expiry.py new file mode 100644 index 0000000..844ed4b --- /dev/null +++ b/apps/patchwork/tests/expiry.py @@ -0,0 +1,121 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2014 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 +import datetime +from django.test import TestCase +from django.contrib.auth.models import User +from patchwork.models import EmailConfirmation, Person, Patch +from patchwork.tests.utils import create_user, defaults +from patchwork.utils import do_expiry + +class TestRegistrationExpiry(TestCase): + + def register(self, date): + user = create_user() + user.is_active = False + user.date_joined = user.last_login = date + user.save() + + conf = EmailConfirmation(type='registration', user=user, + email=user.email) + conf.date = date + conf.save() + + return (user, conf) + + def testOldRegistrationExpiry(self): + date = ((datetime.datetime.now() - EmailConfirmation.validity) - + datetime.timedelta(hours = 1)) + (user, conf) = self.register(date) + + do_expiry() + + self.assertFalse(User.objects.filter(pk = user.pk).exists()) + self.assertFalse(EmailConfirmation.objects.filter(pk = conf.pk) + .exists()) + + + def testRecentRegistrationExpiry(self): + date = ((datetime.datetime.now() - EmailConfirmation.validity) + + datetime.timedelta(hours = 1)) + (user, conf) = self.register(date) + + do_expiry() + + self.assertTrue(User.objects.filter(pk = user.pk).exists()) + self.assertTrue(EmailConfirmation.objects.filter(pk = conf.pk) + .exists()) + + def testInactiveRegistrationExpiry(self): + (user, conf) = self.register(datetime.datetime.now()) + + # confirm registration + conf.user.is_active = True + conf.user.save() + conf.deactivate() + + do_expiry() + + self.assertTrue(User.objects.filter(pk = user.pk).exists()) + self.assertFalse(EmailConfirmation.objects.filter(pk = conf.pk) + .exists()) + + def testPatchSubmitterExpiry(self): + defaults.project.save() + defaults.patch_author_person.save() + + # someone submits a patch... + patch = Patch(project = defaults.project, + msgid = 'test@example.com', name = 'test patch', + submitter = defaults.patch_author_person, + content = defaults.patch) + patch.save() + + # ... then starts registration... + date = ((datetime.datetime.now() - EmailConfirmation.validity) - + datetime.timedelta(hours = 1)) + userid = 'test-user' + user = User.objects.create_user(userid, + defaults.patch_author_person.email, userid) + user.is_active = False + user.date_joined = user.last_login = date + user.save() + + self.assertEqual(user.email, patch.submitter.email) + + conf = EmailConfirmation(type='registration', user=user, + email=user.email) + conf.date = date + conf.save() + + # ... which expires + do_expiry() + + # we should see no matching user + self.assertFalse(User.objects.filter(email = patch.submitter.email) + .exists()) + # but the patch and person should still be present + self.assertTrue(Person.objects.filter( + pk = defaults.patch_author_person.pk).exists()) + self.assertTrue(Patch.objects.filter(pk = patch.pk).exists()) + + # and there should be no user associated with the person + self.assertEqual(Person.objects.get(pk = + defaults.patch_author_person.pk).user, None) diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index 37b85ce..9e1702e 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -22,14 +22,15 @@ import itertools import datetime from django.shortcuts import get_object_or_404 from django.template.loader import render_to_string +from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.conf import settings from django.core.mail import EmailMessage -from django.db.models import Max +from django.db.models import Max, Q, F from django.db.utils import IntegrityError from patchwork.forms import MultiplePatchForm from patchwork.models import Bundle, Project, BundlePatch, UserProfile, \ - PatchChangeNotification, EmailOptout + PatchChangeNotification, EmailOptout, EmailConfirmation def get_patch_ids(d, prefix = 'patch_id'): ids = [] @@ -224,3 +225,24 @@ def send_notifications(): delete_notifications() return errors + +def do_expiry(): + # expire any pending confirmations + q = (Q(date__lt = datetime.datetime.now() - EmailConfirmation.validity) | + Q(active = False)) + EmailConfirmation.objects.filter(q).delete() + + # expire inactive users with no pending confirmation + pending_confs = EmailConfirmation.objects.values('user') + users = User.objects.filter( + is_active = False, + last_login = F('date_joined') + ).exclude( + id__in = pending_confs + ) + + # delete users + users.delete() + + + -- 2.39.2