diff --git a/src/postorius/models.py b/src/postorius/models.py index a974e43..1c6d6d0 100644 --- a/src/postorius/models.py +++ b/src/postorius/models.py @@ -20,7 +20,7 @@ import random -import hashlib +import uuid import logging from datetime import datetime, timedelta @@ -220,47 +220,19 @@ objects = MailmanRestManager('member', 'members') -class AddressConfirmationProfileManager(models.Manager): - """ - Manager class for AddressConfirmationProfile. - """ - - def create_profile(self, email, user): - # Create or update a profile - # Guarantee an email bytestr type that can be fed to hashlib. - email_str = email - if isinstance(email_str, unicode): - email_str = email_str.encode('utf-8') - activation_key = hashlib.sha1( - str(random.random())+email_str).hexdigest() - # Make now tz naive (we don't care about the timezone) - now = datetime.now().replace(tzinfo=None) - # Either update an existing profile record for the given email address - try: - profile = self.get(email=email) - profile.activation_key = activation_key - profile.created = now - profile.save() - # ... or create a new one. - except AddressConfirmationProfile.DoesNotExist: - profile = self.create(email=email, - activation_key=activation_key, - user=user, - created=now) - return profile - - class AddressConfirmationProfile(models.Model): """ Profile model for temporarily storing an activation key to register an email address. """ - email = models.EmailField() - activation_key = models.CharField(max_length=40) + email = models.EmailField(unique=True) + activation_key = models.CharField(max_length=32, unique=True) created = models.DateTimeField(auto_now=True) user = models.ForeignKey(User) - objects = AddressConfirmationProfileManager() + def save(self, *args, **kwargs): + self.activation_key = uuid.uuid4().hex + super(AddressConfirmationProfile, self).save(*args, **kwargs) def __unicode__(self): return u'Address Confirmation Profile for {0}'.format(self.email) diff --git a/src/postorius/tests/mailman_api_tests/test_address_activation.py b/src/postorius/tests/mailman_api_tests/test_address_activation.py index d67ae6e..601899f 100644 --- a/src/postorius/tests/mailman_api_tests/test_address_activation.py +++ b/src/postorius/tests/mailman_api_tests/test_address_activation.py @@ -32,10 +32,10 @@ def setUp(self): # Create a user and profile. self.user = User.objects.create_user('testuser', 'les@example.org', 'testpass') - self.profile = AddressConfirmationProfile.objects.create_profile('les2@example.org', - self.user) - self.expired = AddressConfirmationProfile.objects.create_profile('expired@example.org', - self.user) + self.profile = AddressConfirmationProfile.objects.create(email='les2@example.org', + user=self.user) + self.expired = AddressConfirmationProfile.objects.create(email='expired@example.org', + user=self.user) self.expired.created -= timedelta(weeks=100) self.expired.save() self.mm_user = get_client().create_user('subscribed@example.org', 'password') @@ -84,8 +84,8 @@ self.user = User.objects.create_user( username=u'ler_mm', email=u'ler@mailman.mostdesirable.org', password=u'pwd') - self.profile = AddressConfirmationProfile.objects.create_profile( - u'les@example.org', self.user) + self.profile = AddressConfirmationProfile.objects.create(email=u'les@example.org', + user=self.user) # Create a test request object self.request = RequestFactory().get('/') @@ -97,20 +97,9 @@ def test_profile_creation(self): # Profile is created and has all necessary properties. self.assertEqual(self.profile.email, u'les@example.org') - self.assertEqual(len(self.profile.activation_key), 40) + self.assertEqual(len(self.profile.activation_key), 32) self.assertTrue(type(self.profile.created), datetime) - def test_no_duplicate_profiles(self): - # Creating a new profile returns an existing updated record - # (if one exists), instead of creating a new one. - new_profile = AddressConfirmationProfile.objects.create_profile( - u'les@example.org', - User.objects.create(email=u'ler@mailman.mostdesirable.org')) - self.assertEqual(new_profile.user, self.profile.user) - self.assertEqual(new_profile.email, self.profile.email) - self.assertNotEqual(new_profile.created, self.profile.created) - self.assertNotEqual(new_profile.activation_key, self.profile.activation_key) - def test_unicode_representation(self): # Correct unicode representation? self.assertEqual(unicode(self.profile), @@ -130,6 +119,13 @@ self.profile.created = now - delta self.assertTrue(self.profile.is_expired) + def test_profile_is_updated_on_save(self): + key = self.profile.activation_key + created = self.profile.created + self.profile.save() + self.assertNotEqual(self.profile.activation_key, key) + self.assertNotEqual(self.profile.created, created) + @override_settings( EMAIL_CONFIRMATION_EXPIRATION_DELTA=timedelta(hours=5)) def test_profile_not_expired(self): @@ -145,9 +141,6 @@ EMAIL_CONFIRMATION_FROM='mailman@mostdesirable.org') def test_confirmation_link(self): # The profile obj can send out a confirmation email. - # set the activation key to a fixed string for testing - self.profile.activation_key = \ - '6323fba0097781fdb887cfc37a1122ee7c8bb0b0' # Simulate a VirtualHost with a different name self.request.META["HTTP_HOST"] = "another-virtualhost" # Now send the email @@ -169,10 +162,7 @@ self.user = User.objects.create_user( username='ler', email=u'ler@example.org', password='pwd') - self.profile = AddressConfirmationProfile.objects.create_profile( - u'les@example.org', self.user) - self.profile.activation_key = \ - u'6323fba0097781fdb887cfc37a1122ee7c8bb0b0' + self.profile = AddressConfirmationProfile.objects.create(email=u'les@example.org', user=self.user) self.profile.save() def tearDown(self): @@ -185,12 +175,12 @@ # to the user. request = RequestFactory().get(reverse( 'address_activation_link', kwargs={ - 'activation_key': '6323fba0097781fdb887cfc37a1122ee7c8bb0b0'})) + 'activation_key': self.profile.activation_key})) setattr(request, 'session', 'session') messages = FallbackStorage(request) setattr(request, '_messages', messages) address_activation_link( - request, '6323fba0097781fdb887cfc37a1122ee7c8bb0b0') + request, self.profile.activation_key) expected_calls = [call(request, u'ler@example.org', u'les@example.org')] self.assertEqual(_add_address_mock.mock_calls, expected_calls) diff --git a/src/postorius/urls.py b/src/postorius/urls.py index f401039..0d2f55c 100644 --- a/src/postorius/urls.py +++ b/src/postorius/urls.py @@ -83,7 +83,8 @@ url(r'^lists/$', list_views.list_index, name='list_index'), url(r'^lists/new/$', list_views.list_new, name='list_new'), url(r'^lists/(?P[^/]+)/', include(list_patterns)), - url(r'^users/address_activation/(?P[A-Za-z0-9]{40})/$', + # XXX This can be changed to limit the length of activation keys + url(r'^users/address_activation/(?P[A-Za-z0-9]+)/$', user_views.address_activation_link, name='address_activation_link'), ] diff --git a/src/postorius/views/user.py b/src/postorius/views/user.py index f1cb54a..745af2c 100644 --- a/src/postorius/views/user.py +++ b/src/postorius/views/user.py @@ -40,8 +40,7 @@ from smtplib import SMTPException from socket import error as socket_error import errno -import hashlib -import random +import uuid import datetime @@ -236,22 +235,17 @@ if request.method == 'POST': form = AddressActivationForm(request.POST) if form.is_valid(): - email_str = form.cleaned_data['email'].encode('utf-8') - activation_key = hashlib.sha1(str(random.random())+email_str).hexdigest() - # XXX Use the following when django 1.6 is dropped as a dependency # It is more efficient because it can be done in one database operation # - # defaults = {'activation_key': activation_key,} # profile, created = AddressConfirmationProfile.objects.update_or_create( - # email=form.cleaned_data['email'], user=request.user, defaults=defaults) + # email=form.cleaned_data['email'], user=request.user, defaults={ + # 'activation_key': uuid.uuid4().hex}) try: profile = AddressConfirmationProfile.objects.get(email=form.cleaned_data['email'], user=request.user) - profile.activation_key = activation_key - profile.created = datetime.datetime.now() profile.save() except AddressConfirmationProfile.DoesNotExist: - profile = AddressConfirmationProfile.objects.create_profile(email=form.cleaned_data['email'], user=request.user) + profile = AddressConfirmationProfile.objects.create(email=form.cleaned_data['email'], user=request.user) try: profile.send_confirmation_link(request) messages.success(request,