From 7a9e5320914838379f7c626d21c826984a148ece Mon Sep 17 00:00:00 2001 From: Magnus Hagander Date: Thu, 10 Sep 2020 14:52:41 +0200 Subject: [PATCH] Track multiple email addresses for an organisation This allows organisation managers to add more than one email address to an organisation, and use this for sending news from. Sending news is the only thing that the email field is used for at this point. Adding an email will trigger a validation email sent to the address with a token to confirm it, so that we can actually trust the emails. Remove the previous registered emails on organisations. These addresses were never validated and thus cannot really be trusted, so it's better to remove them cleanly than to migrate them into the new system and be uncertain. Finally, in passing, remove the phone field on organisations. We've never used that for anything and there's not really any point in collecting the data. --- pgweb/account/urls.py | 1 + pgweb/account/views.py | 19 ++++++++ pgweb/core/forms.py | 45 +++++++++++++++++-- .../commands/cleanup_old_records.py | 2 + pgweb/core/migrations/0004_org_emails.py | 37 +++++++++++++++ pgweb/core/models.py | 23 ++++++++-- pgweb/news/admin.py | 12 +++++ pgweb/news/forms.py | 11 ++++- pgweb/news/migrations/0006_sending_email.py | 20 +++++++++ pgweb/news/models.py | 14 +++--- pgweb/news/util.py | 2 +- templates/core/org_add_email.txt | 13 ++++++ 12 files changed, 183 insertions(+), 16 deletions(-) create mode 100644 pgweb/core/migrations/0004_org_emails.py create mode 100644 pgweb/news/migrations/0006_sending_email.py create mode 100644 templates/core/org_add_email.txt diff --git a/pgweb/account/urls.py b/pgweb/account/urls.py index 3bd0c68f..25a01cad 100644 --- a/pgweb/account/urls.py +++ b/pgweb/account/urls.py @@ -26,6 +26,7 @@ urlpatterns = [ # Submitted items url(r'^(?Pnews)/(?P\d+)/(?Psubmit|withdraw)/$', pgweb.account.views.submitted_item_submitwithdraw), url(r'^(?Pnews|events|products|organisations|services)/(?P\d+|new)/$', pgweb.account.views.submitted_item_form), + url(r'^organisations/confirm/([0-9a-f]+)/$', pgweb.account.views.confirm_org_email), # Organisation information url(r'^orglist/$', pgweb.account.views.orglist), diff --git a/pgweb/account/views.py b/pgweb/account/views.py index 14d4bd7b..fb3b3ba1 100644 --- a/pgweb/account/views.py +++ b/pgweb/account/views.py @@ -30,6 +30,7 @@ from pgweb.util.moderation import ModerationState from pgweb.news.models import NewsArticle from pgweb.events.models import Event from pgweb.core.models import Organisation, UserProfile, ModerationNotification +from pgweb.core.models import OrganisationEmail from pgweb.contributors.models import Contributor from pgweb.downloads.models import Product from pgweb.profserv.models import ProfessionalService @@ -277,6 +278,24 @@ def submitted_item_form(request, objtype, item): extracontext=extracontext) +@login_required +@transaction.atomic +def confirm_org_email(request, token): + try: + email = OrganisationEmail.objects.get(token=token) + except OrganisationEmail.DoesNotExist: + raise Http404() + + if not email.org.managers.filter(pk=request.user.pk).exists(): + raise PermissionDenied("You are not a manager of the associated organisation") + + email.confirmed = True + email.token = None + email.save() + + return HttpResponseRedirect('/account/organisations/{}/'.format(email.org.id)) + + @content_sources('style', "'unsafe-inline'") def _submitted_item_submit(request, objtype, model, obj): if obj.modstate != ModerationState.CREATED: diff --git a/pgweb/core/forms.py b/pgweb/core/forms.py index 5406474f..9c2a7372 100644 --- a/pgweb/core/forms.py +++ b/pgweb/core/forms.py @@ -2,15 +2,18 @@ from django import forms from django.forms import ValidationError from django.conf import settings -from .models import Organisation +from .models import Organisation, OrganisationEmail from django.contrib.auth.models import User from pgweb.util.middleware import get_current_user from pgweb.util.moderation import ModerationState from pgweb.mailqueue.util import send_simple_mail +from pgweb.util.misc import send_template_mail, generate_random_token class OrganisationForm(forms.ModelForm): + remove_email = forms.ModelMultipleChoiceField(required=False, queryset=None, label="Current email addresses", help_text="Select one or more email addresses to remove") + add_email = forms.EmailField(required=False, help_text="Enter an email address to add") remove_manager = forms.ModelMultipleChoiceField(required=False, queryset=None, label="Current manager(s)", help_text="Select one or more managers to remove") add_manager = forms.EmailField(required=False) @@ -26,6 +29,19 @@ class OrganisationForm(forms.ModelForm): del self.fields['remove_manager'] del self.fields['add_manager'] + if self.instance and self.instance.pk and self.instance.is_approved: + # Only allow adding/removing emails on orgs that are actually approved + self.fields['remove_email'].queryset = OrganisationEmail.objects.filter(org=self.instance) + else: + del self.fields['remove_email'] + del self.fields['add_email'] + + def clean_add_email(self): + if self.cleaned_data['add_email']: + if OrganisationEmail.objects.filter(org=self.instance, address=self.cleaned_data['add_email'].lower()).exists(): + raise ValidationError("This email is already registered for your organisation.") + return self.cleaned_data['add_email'] + def clean_add_manager(self): if self.cleaned_data['add_manager']: # Something was added as manager - let's make sure the user exists @@ -49,7 +65,30 @@ class OrganisationForm(forms.ModelForm): def save(self, commit=True): model = super(OrganisationForm, self).save(commit=False) + ops = [] + if self.cleaned_data.get('add_email', None): + # Create the email record + e = OrganisationEmail(org=model, address=self.cleaned_data['add_email'].lower(), token=generate_random_token()) + e.save() + + # Send email for confirmation + send_template_mail( + settings.NOTIFICATION_FROM, + e.address, + "Email address added to postgresql.org organisation", + 'core/org_add_email.txt', + { + 'org': model, + 'email': e, + }, + ) + ops.append('Added email {}, confirmation request sent'.format(e.address)) + if self.cleaned_data.get('remove_email', None): + for e in self.cleaned_data['remove_email']: + ops.append('Removed email {}'.format(e.email)) + e.delete() + if 'add_manager' in self.cleaned_data and self.cleaned_data['add_manager']: u = User.objects.get(email=self.cleaned_data['add_manager'].lower()) model.managers.add(u) @@ -63,8 +102,8 @@ class OrganisationForm(forms.ModelForm): send_simple_mail( settings.NOTIFICATION_FROM, settings.NOTIFICATION_EMAIL, - "{0} modified managers of {1}".format(get_current_user().username, model), - "The following changes were made to managers:\n\n{0}".format("\n".join(ops)) + "{0} modified {1}".format(get_current_user().username, model), + "The following changes were made to {}:\n\n{}".format(model, "\n".join(ops)) ) return model diff --git a/pgweb/core/management/commands/cleanup_old_records.py b/pgweb/core/management/commands/cleanup_old_records.py index 70368a34..5a91a239 100644 --- a/pgweb/core/management/commands/cleanup_old_records.py +++ b/pgweb/core/management/commands/cleanup_old_records.py @@ -16,6 +16,7 @@ from django.db import connection, transaction from datetime import datetime, timedelta from pgweb.account.models import SecondaryEmail +from pgweb.core.models import OrganisationEmail class Command(BaseCommand): @@ -34,3 +35,4 @@ class Command(BaseCommand): # Clean up old email change tokens with transaction.atomic(): SecondaryEmail.objects.filter(confirmed=False, sentat__lt=datetime.now() - timedelta(hours=24)).delete() + OrganisationEmail.objects.filter(confirmed=False, added__lt=datetime.now() - timedelta(hours=72)).delete() diff --git a/pgweb/core/migrations/0004_org_emails.py b/pgweb/core/migrations/0004_org_emails.py new file mode 100644 index 00000000..ed22fb18 --- /dev/null +++ b/pgweb/core/migrations/0004_org_emails.py @@ -0,0 +1,37 @@ +# Generated by Django 2.2.11 on 2020-09-07 12:53 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0003_mailtemplate'), + ] + + operations = [ + migrations.RemoveField( + model_name='organisation', + name='email', + ), + migrations.RemoveField( + model_name='organisation', + name='phone', + ), + migrations.CreateModel( + name='OrganisationEmail', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('address', models.EmailField(max_length=254)), + ('confirmed', models.BooleanField(default=False)), + ('token', models.CharField(blank=True, max_length=100, null=True)), + ('org', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='core.Organisation')), + ('added', models.DateTimeField(null=False, auto_now_add=True)), + ], + options={ + 'ordering': ('org', 'address'), + 'unique_together': {('org', 'address')}, + }, + ), + ] diff --git a/pgweb/core/models.py b/pgweb/core/models.py index 7e3f3790..6131cb1c 100644 --- a/pgweb/core/models.py +++ b/pgweb/core/models.py @@ -133,8 +133,6 @@ class Organisation(TwostateModerateModel): name = models.CharField(max_length=100, null=False, blank=False, unique=True) address = models.TextField(null=False, blank=True) url = models.URLField(null=False, blank=False) - email = models.EmailField(null=False, blank=True) - phone = models.CharField(max_length=100, null=False, blank=True) orgtype = models.ForeignKey(OrganisationType, null=False, blank=False, verbose_name="Organisation type", on_delete=models.CASCADE) managers = models.ManyToManyField(User, blank=False) mailtemplate = models.CharField(max_length=50, null=False, blank=False, default='default', choices=_mail_template_choices, @@ -145,7 +143,7 @@ class Organisation(TwostateModerateModel): lastconfirmed = models.DateTimeField(null=False, blank=False, auto_now_add=True) account_edit_suburl = 'organisations' - moderation_fields = ['address', 'url', 'email', 'phone', 'orgtype', 'managers'] + moderation_fields = ['address', 'url', 'orgtype', 'managers'] def __str__(self): return self.name @@ -163,6 +161,25 @@ class Organisation(TwostateModerateModel): return OrganisationForm +class OrganisationEmail(models.Model): + org = models.ForeignKey(Organisation, null=False, blank=False, on_delete=models.CASCADE) + address = models.EmailField(null=False, blank=False) + confirmed = models.BooleanField(null=False, blank=False, default=False) + token = models.CharField(max_length=100, null=True, blank=True) + added = models.DateTimeField(null=False, blank=False, auto_now_add=True) + + class Meta: + ordering = ('org', 'address') + unique_together = ( + ('org', 'address', ), + ) + + def __str__(self): + if self.confirmed: + return self.address + return "{} (not confirmed yet)".format(self.address) + + # Basic classes for importing external RSS feeds, such as planet class ImportedRSSFeed(models.Model): internalname = models.CharField(max_length=32, null=False, blank=False, unique=True) diff --git a/pgweb/news/admin.py b/pgweb/news/admin.py index c68bd719..4c3c8e10 100644 --- a/pgweb/news/admin.py +++ b/pgweb/news/admin.py @@ -1,15 +1,27 @@ from django.contrib import admin +from django import forms from pgweb.util.admin import PgwebAdmin +from pgweb.core.models import OrganisationEmail from .models import NewsArticle, NewsTag +class NewsArticleAdminForm(forms.ModelForm): + model = NewsArticle + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance: + self.fields['email'].queryset = OrganisationEmail.objects.filter(org=self.instance.org, confirmed=True) + + class NewsArticleAdmin(PgwebAdmin): list_display = ('title', 'org', 'date', 'modstate', ) list_filter = ('modstate', ) filter_horizontal = ('tags', ) search_fields = ('content', 'title', ) exclude = ('modstate', 'firstmoderator', ) + form = NewsArticleAdminForm class NewsTagAdmin(PgwebAdmin): diff --git a/pgweb/news/forms.py b/pgweb/news/forms.py index 3d24bfbd..fc39581b 100644 --- a/pgweb/news/forms.py +++ b/pgweb/news/forms.py @@ -2,13 +2,18 @@ from django import forms from django.forms import ValidationError from pgweb.util.moderation import ModerationState -from pgweb.core.models import Organisation +from pgweb.core.models import Organisation, OrganisationEmail from .models import NewsArticle, NewsTag class NewsArticleForm(forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['email'].required = True + def filter_by_user(self, user): self.fields['org'].queryset = Organisation.objects.filter(managers=user, approved=True) + self.fields['email'].queryset = OrganisationEmail.objects.filter(org__managers=user, org__approved=True, confirmed=True) def clean_date(self): if self.instance.pk and self.instance.modstate != ModerationState.CREATED: @@ -25,6 +30,10 @@ class NewsArticleForm(forms.ModelForm): def clean(self): data = super().clean() + if data.get('email', None): + if data['email'].org != data['org']: + self.add_error('email', 'You must pick an email address associated with the organisation') + if 'tags' not in data: self.add_error('tags', 'Select one or more tags') else: diff --git a/pgweb/news/migrations/0006_sending_email.py b/pgweb/news/migrations/0006_sending_email.py new file mode 100644 index 00000000..b0f1de2c --- /dev/null +++ b/pgweb/news/migrations/0006_sending_email.py @@ -0,0 +1,20 @@ +# Generated by Django 2.2.11 on 2020-09-07 14:13 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0004_org_emails'), + ('news', '0005_modstate'), + ] + + operations = [ + migrations.AddField( + model_name='newsarticle', + name='email', + field=models.ForeignKey(blank=True, help_text='Pick a confirmed email associated with the organisation. This will be used as the reply address of posted news.', null=True, on_delete=django.db.models.deletion.CASCADE, to='core.OrganisationEmail', verbose_name='Reply email'), + ), + ] diff --git a/pgweb/news/models.py b/pgweb/news/models.py index 185cf157..95bcef75 100644 --- a/pgweb/news/models.py +++ b/pgweb/news/models.py @@ -1,6 +1,6 @@ from django.db import models from datetime import date -from pgweb.core.models import Organisation +from pgweb.core.models import Organisation, OrganisationEmail from pgweb.util.moderation import TristateModerateModel, ModerationState, TwoModeratorsMixin from .util import send_news_email, render_news_template, embed_images_in_html @@ -26,6 +26,7 @@ class NewsTag(models.Model): class NewsArticle(TwoModeratorsMixin, TristateModerateModel): org = models.ForeignKey(Organisation, null=False, blank=False, verbose_name="Organisation", help_text="If no organisations are listed, please check the organisation list and contact the organisation manager or webmaster@postgresql.org if none are listed.", on_delete=models.CASCADE) + email = models.ForeignKey(OrganisationEmail, null=True, blank=True, verbose_name="Reply email", help_text="Pick a confirmed email associated with the organisation. This will be used as the reply address of posted news.", on_delete=models.CASCADE) date = models.DateField(null=False, blank=False, default=date.today) title = models.CharField(max_length=200, null=False, blank=False) content = models.TextField(null=False, blank=False) @@ -34,8 +35,9 @@ class NewsArticle(TwoModeratorsMixin, TristateModerateModel): account_edit_suburl = 'news' markdown_fields = ('content',) - moderation_fields = ('org', 'sentfrom', 'replyto', 'date', 'title', 'content', 'taglist') - preview_fields = ('title', 'sentfrom', 'replyto', 'content', 'taglist') + moderation_fields = ('org', 'sentfrom', 'email', 'date', 'title', 'content', 'taglist') + preview_fields = ('title', 'sentfrom', 'email', 'content', 'taglist') + notify_fields = ('org', 'email', 'date', 'title', 'content', 'tags') rendered_preview_fields = ('content', ) extramodnotice = "In particular, note that news articles will be sent by email to subscribers, and therefor cannot be recalled in any way once sent." @@ -62,10 +64,6 @@ class NewsArticle(TwoModeratorsMixin, TristateModerateModel): def taglist(self): return ", ".join([t.name for t in self.tags.all()]) - @property - def replyto(self): - return self.org.email - @property def sentfrom(self): return self.org.fromnameoverride if self.org.fromnameoverride else '{} via PostgreSQL Announce'.format(self.org.name) @@ -100,7 +98,7 @@ class NewsArticle(TwoModeratorsMixin, TristateModerateModel): return 'Title/subject' elif f == 'sentfrom': return 'Sent from' - elif f == 'replyto': + elif f == 'email': return 'Direct replies to' elif f == 'taglist': return 'List of tags' diff --git a/pgweb/news/util.py b/pgweb/news/util.py index fbad07d8..067383aa 100644 --- a/pgweb/news/util.py +++ b/pgweb/news/util.py @@ -91,7 +91,7 @@ def send_news_email(news): settings.NEWS_MAIL_RECEIVER, news.title, news.content, - replyto=news.replyto, + replyto=news.email.address, sendername=news.sentfrom, receivername=settings.NEWS_MAIL_RECEIVER_NAME, messageid=messageid, diff --git a/templates/core/org_add_email.txt b/templates/core/org_add_email.txt new file mode 100644 index 00000000..73c6670e --- /dev/null +++ b/templates/core/org_add_email.txt @@ -0,0 +1,13 @@ +Hello! + +You are receiving this email because this address was added +to the organisation {{org}} +on www.postgresql.org. + +If this is not correct, please ignore this email and the +record of the address will be automatically deleted. + +If this is correct, please click the below link to confirm +this email address. + +{{link_root}}/account/organisations/confirm/{{email.token}}/ -- 2.39.5