diff --git a/newsletter/admin.py b/newsletter/admin.py index 20157f36..6f7e50a3 100644 --- a/newsletter/admin.py +++ b/newsletter/admin.py @@ -360,7 +360,7 @@ class SubscriptionAdmin(NewsletterAdminLinkMixin, ExtendibleModelAdminMixin, 'user__email' ) readonly_fields = ( - 'ip', 'subscribe_date', 'unsubscribe_date', 'activation_code' + 'ip', 'subscribe_date', 'unsubscribe_date', 'activation_code', 'activation_uuid', ) date_hierarchy = 'subscribe_date' actions = ['make_subscribed', 'make_unsubscribed'] diff --git a/newsletter/forms.py b/newsletter/forms.py index 49514cfd..16cc5b5f 100644 --- a/newsletter/forms.py +++ b/newsletter/forms.py @@ -137,7 +137,7 @@ class UpdateForm(NewsletterForm): def clean_user_activation_code(self): data = self.cleaned_data['user_activation_code'] - if data != self.instance.activation_code: + if not self.instance.valid_activation(data): raise ValidationError( _('The validation code supplied by you does not match.') ) diff --git a/newsletter/migrations/0005_auto_20190208_2353.py b/newsletter/migrations/0005_auto_20190208_2353.py new file mode 100644 index 00000000..12cd7b27 --- /dev/null +++ b/newsletter/migrations/0005_auto_20190208_2353.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.5 on 2019-02-08 23:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('newsletter', '0004_auto_20180407_1043'), + ] + + operations = [ + migrations.AlterField( + model_name='submission', + name='subscriptions', + field=models.ManyToManyField(blank=True, db_index=True, help_text='If you select none, the system will automatically find the subscribers for you.', limit_choices_to={'subscribed': True}, to='newsletter.Subscription', verbose_name='recipients'), + ), + ] diff --git a/newsletter/migrations/0006_auto_20190317_0259.py b/newsletter/migrations/0006_auto_20190317_0259.py new file mode 100644 index 00000000..20da7dd7 --- /dev/null +++ b/newsletter/migrations/0006_auto_20190317_0259.py @@ -0,0 +1,24 @@ +# Generated by Django 2.1.7 on 2019-03-17 02:59 + +from django.db import migrations, models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('newsletter', '0005_auto_20190208_2353'), + ] + + operations = [ + migrations.AddField( + model_name='subscription', + name='activation_uuid', + field=models.UUIDField(default=uuid.uuid4, verbose_name='activation code'), + ), + migrations.AlterField( + model_name='subscription', + name='activation_code', + field=models.CharField(blank=True, max_length=40, verbose_name='activation code legacy'), + ), + ] diff --git a/newsletter/models.py b/newsletter/models.py index 9d2a7cd4..2616fd19 100644 --- a/newsletter/models.py +++ b/newsletter/models.py @@ -1,4 +1,5 @@ import logging +import uuid import time import django @@ -20,7 +21,7 @@ from .compat import get_context, reverse from .utils import ( - make_activation_code, get_default_sites, ACTIONS + get_default_sites, ACTIONS ) logger = logging.getLogger(__name__) @@ -283,11 +284,43 @@ def save(self, *args, **kwargs): create_date = models.DateTimeField(editable=False, default=now) + # activation_code = models.CharField( - verbose_name=_('activation code'), max_length=40, - default=make_activation_code + verbose_name=_('activation code legacy'), max_length=40, + blank=True, ) + activation_uuid = models.UUIDField( + verbose_name=_('activation code'), + default=uuid.uuid4 + ) + + def get_activation_code(self): + return self.activation_code or self.activation_uuid + + def valid_activation(self, data): + """ + Compare the data to legacy or new activation code. + + If legacy code is not blank, we compare against that first, othersise + use the uuid. + + If the data is not a UUID, we will attempt to coerce it. If that fails, + we will return false + """ + if self.activation_code: + # Legacy code is set, use that + return data == self.activation_code + + # + if not isinstance(data, uuid.UUID): + try: + data = uuid.UUID(data) + except ValueError: + return False + + return data == self.activation_uuid + subscribed = models.BooleanField( default=False, verbose_name=_('subscribed'), db_index=True ) @@ -363,34 +396,31 @@ def send_activation_email(self, action): logger.debug( u'Activation email sent for action "%(action)s" to %(subscriber)s ' u'with activation code "%(action_code)s".', { - 'action_code': self.activation_code, + 'action_code': self.get_activation_code(), 'action': action, 'subscriber': self } ) def subscribe_activate_url(self): - return reverse('newsletter_update_activate', kwargs={ + return reverse('newsletter_update_activate_uuid', kwargs={ 'newsletter_slug': self.newsletter.slug, - 'email': self.email, 'action': 'subscribe', - 'activation_code': self.activation_code + 'activation_code': self.get_activation_code() }) def unsubscribe_activate_url(self): - return reverse('newsletter_update_activate', kwargs={ + return reverse('newsletter_update_activate_uuid', kwargs={ 'newsletter_slug': self.newsletter.slug, - 'email': self.email, 'action': 'unsubscribe', - 'activation_code': self.activation_code + 'activation_code': self.get_activation_code() }) def update_activate_url(self): - return reverse('newsletter_update_activate', kwargs={ + return reverse('newsletter_update_activate_uuid', kwargs={ 'newsletter_slug': self.newsletter.slug, - 'email': self.email, 'action': 'update', - 'activation_code': self.activation_code + 'activation_code': self.get_activation_code() }) diff --git a/newsletter/urls.py b/newsletter/urls.py index 8f95b159..dea022b7 100644 --- a/newsletter/urls.py +++ b/newsletter/urls.py @@ -63,6 +63,13 @@ UpdateSubscriptionView.as_view(), name='newsletter_update' ), + # Action confirmation views - no email + surl( + '^/subscription/' + '/activate//$', + UpdateSubscriptionView.as_view(), name='newsletter_update_activate_uuid' + ), + # Action activation completed view surl( '^//' diff --git a/newsletter/views.py b/newsletter/views.py index 304fbd0a..16f9ac9b 100644 --- a/newsletter/views.py +++ b/newsletter/views.py @@ -2,6 +2,7 @@ import datetime import socket +import uuid from smtplib import SMTPException @@ -259,7 +260,7 @@ class ActionFormView(NewsletterMixin, ActionMixin, FormView): def get_url_from_viewname(self, viewname): """ - Return url for given `viename` + Return url for given `viewname` and associated with this view newsletter and action. """ @@ -494,16 +495,29 @@ def process_url_data(self, *args, **kwargs): Add email, subscription and activation_code to instance attributes. """ - assert 'email' in kwargs super(UpdateSubscriptionView, self).process_url_data(*args, **kwargs) - self.subscription = get_object_or_404( - Subscription, newsletter=self.newsletter, - email_field__exact=kwargs['email'] - ) - # activation_code is optional kwarg which defaults to None + # If activation code is a valid UUID, search using that code self.activation_code = kwargs.get('activation_code') + try: + code_uuid = uuid.UUID(self.activation_code) + self.subscription = get_object_or_404( + Subscription, newsletter=self.newsletter, + activation_uuid=code_uuid + ) + + except (TypeError, ValueError)as e: + # Code isn't a valid uuid, so use email + if 'email' in kwargs: + self.subscription = get_object_or_404( + Subscription, newsletter=self.newsletter, + email_field__exact=kwargs['email'] + ) + else: + raise Http404(ugettext( + 'Confirmation not found' + )) def get_initial(self): """ Returns the initial data to use for forms on this view. """ @@ -625,3 +639,19 @@ def render_to_response(self, context, **response_kwargs): context=context, **response_kwargs ) + + def _get_allow_future(self): + """ + BaseDateDetailView does a comparison of the date an object + was created with + from django.utils.timezone import now + and + datetime.date.today() + + These will be wrong at some times of the day, depending on timzeone + + This is here if needed, but won't be used. + :return: + """ + return True + diff --git a/setup.py b/setup.py index 8c604dcc..cce6b352 100755 --- a/setup.py +++ b/setup.py @@ -45,7 +45,7 @@ setup( name='django-newsletter', - version="0.8b1", + version="0.8b2", description=( 'Django app for managing multiple mass-mailing lists with both ' 'plaintext as well as HTML templates (and pluggable WYSIWYG editors ' diff --git a/tests/test_web.py b/tests/test_web.py index 49949827..49e06d2c 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -733,7 +733,7 @@ def test_subscribe_unsubscribed(self): { 'name_field': subscription.name, 'email_field': subscription.email, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() } ) @@ -768,7 +768,7 @@ def test_subscribe_unsubscribed(self): { 'name_field': subscription.name, 'email_field': subscription.email, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() } ) @@ -826,13 +826,13 @@ def test_subscribe_request_activate(self): response = self.client.get(activate_url) self.assertInContext(response, 'form', UpdateForm) - self.assertContains(response, subscription.activation_code) + self.assertContains(response, subscription.get_activation_code()) response = self.client.post( activate_url, { 'name_field': 'Test Name', 'email_field': self.testemail, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() } ) @@ -863,14 +863,14 @@ def test_subscribe_request_activate_form_loophole(self): response = self.client.get(activate_url) self.assertInContext(response, 'form', UpdateForm) - self.assertContains(response, subscription.activation_code) + self.assertContains(response, subscription.get_activation_code()) testname2 = 'Test Name2' testemail2 = 'test2@email.com' response = self.client.post(activate_url, { 'name_field': testname2, 'email_field': testemail2, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() }) # Assure we are redirected to "update activated" page. @@ -989,12 +989,12 @@ def test_unsubscribe_request_activate(self): response = self.client.get(activate_url) self.assertInContext(response, 'form', UpdateForm) - self.assertContains(response, subscription.activation_code) + self.assertContains(response, subscription.get_activation_code()) testname2 = 'Test Name2' response = self.client.post(activate_url, { 'name_field': testname2, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() }) # Assure we are redirected to "unsubscribe activated" page. @@ -1172,12 +1172,12 @@ def test_update_request_activate(self): response = self.client.get(activate_url) self.assertInContext(response, 'form', UpdateForm) - self.assertContains(response, subscription.activation_code) + self.assertContains(response, subscription.get_activation_code()) testname2 = 'Test Name2' response = self.client.post(activate_url, { 'name_field': testname2, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() }) # Assure we are redirected to "update activated" page. @@ -1196,6 +1196,8 @@ def test_update_request_activate_form(self): """ Test requesting a form for activating an update without activation code in the URL. + + NOTE: This is no longer valid, code is required. """ subscription = Subscription(newsletter=self.n, @@ -1203,10 +1205,10 @@ def test_update_request_activate_form(self): email=self.testemail) subscription.save() - activate_url = reverse('newsletter_update', kwargs={ + activate_url = reverse('newsletter_update_activate_uuid', kwargs={ 'newsletter_slug': self.n.slug, + 'activation_code': subscription.get_activation_code(), 'action': 'update', - 'email': subscription.email }) response = self.client.get(activate_url) @@ -1228,14 +1230,14 @@ def test_update_request_activate_form_loophole(self): response = self.client.get(activate_url) self.assertInContext(response, 'form', UpdateForm) - self.assertContains(response, subscription.activation_code) + self.assertContains(response, subscription.get_activation_code()) testname2 = 'Test Name2' testemail2 = 'test2@email.com' response = self.client.post(activate_url, { 'name_field': testname2, 'email_field': testemail2, - 'user_activation_code': subscription.activation_code + 'user_activation_code': subscription.get_activation_code() }) # Assure we are redirected to "update activated" page.