emails: Added placeholders strings in FormAddress.

We've had a bug for a while that if any ScheduledEmail objects get
created with the wrong email sender address, even after the sysadmin
corrects the problem, they'll still get errors because of the objects
stored with the wrong format.

We solve this by using FromAddress placeholders strings in
send_future_email function, so that ScheduledEmail objects end up
setting the final `from_address` value when mail is actually sent
using the setting in effect at that time.

Fixes #11008.
This commit is contained in:
arpit551
2020-03-13 00:58:05 +05:30
committed by Tim Abbott
parent 96eb1bcd9d
commit 8f7733cb20
5 changed files with 17 additions and 5 deletions

View File

@@ -212,7 +212,8 @@ def handle_digest_email(user_profile_id: int, cutoff: float,
logger.info("Sending digest email for user %s" % (user_profile.id,)) logger.info("Sending digest email for user %s" % (user_profile.id,))
# Send now, as a ScheduledEmail # Send now, as a ScheduledEmail
send_future_email('zerver/emails/digest', user_profile.realm, to_user_ids=[user_profile.id], send_future_email('zerver/emails/digest', user_profile.realm, to_user_ids=[user_profile.id],
from_name="Zulip Digest", from_address=FromAddress.NOREPLY, context=context) from_name="Zulip Digest", from_address=FromAddress.no_reply_placeholder,
context=context)
return None return None
def exclude_subscription_modified_streams(user_profile: UserProfile, def exclude_subscription_modified_streams(user_profile: UserProfile,

View File

@@ -546,7 +546,7 @@ def enqueue_welcome_emails(user: UserProfile, realm_creation: bool=False) -> Non
from_address = settings.WELCOME_EMAIL_SENDER['email'] from_address = settings.WELCOME_EMAIL_SENDER['email']
else: else:
from_name = None from_name = None
from_address = FromAddress.SUPPORT from_address = FromAddress.support_placeholder
other_account_count = UserProfile.objects.filter( other_account_count = UserProfile.objects.filter(
delivery_email__iexact=user.delivery_email).exclude(id=user.id).count() delivery_email__iexact=user.delivery_email).exclude(id=user.id).count()

View File

@@ -29,6 +29,10 @@ class FromAddress:
SUPPORT = parseaddr(settings.ZULIP_ADMINISTRATOR)[1] SUPPORT = parseaddr(settings.ZULIP_ADMINISTRATOR)[1]
NOREPLY = parseaddr(settings.NOREPLY_EMAIL_ADDRESS)[1] NOREPLY = parseaddr(settings.NOREPLY_EMAIL_ADDRESS)[1]
support_placeholder = "SUPPORT"
no_reply_placeholder = 'NO_REPLY'
tokenized_no_reply_placeholder = 'TOKENIZED_NO_REPLY'
# Generates an unpredictable noreply address. # Generates an unpredictable noreply address.
@staticmethod @staticmethod
def tokenized_no_reply_address() -> str: def tokenized_no_reply_address() -> str:
@@ -96,6 +100,13 @@ def build_email(template_prefix: str, to_user_ids: Optional[List[int]]=None,
from_name = "Zulip" from_name = "Zulip"
if from_address is None: if from_address is None:
from_address = FromAddress.NOREPLY from_address = FromAddress.NOREPLY
if from_address == FromAddress.tokenized_no_reply_placeholder:
from_address = FromAddress.tokenized_no_reply_address()
if from_address == FromAddress.no_reply_placeholder:
from_address = FromAddress.NOREPLY
if from_address == FromAddress.support_placeholder:
from_address = FromAddress.SUPPORT
from_email = formataddr((from_name, from_address)) from_email = formataddr((from_name, from_address))
reply_to = None reply_to = None
if reply_to_email is not None: if reply_to_email is not None:

View File

@@ -1399,7 +1399,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!"
email = data["email"] email = data["email"]
send_future_email( send_future_email(
"zerver/emails/invitation_reminder", referrer.realm, to_emails=[email], "zerver/emails/invitation_reminder", referrer.realm, to_emails=[email],
from_address=FromAddress.NOREPLY, context=context) from_address=FromAddress.no_reply_placeholder, context=context)
email_jobs_to_deliver = ScheduledEmail.objects.filter( email_jobs_to_deliver = ScheduledEmail.objects.filter(
scheduled_timestamp__lte=timezone_now()) scheduled_timestamp__lte=timezone_now())
self.assertEqual(len(email_jobs_to_deliver), 1) self.assertEqual(len(email_jobs_to_deliver), 1)
@@ -1414,7 +1414,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!"
email = data["email"] email = data["email"]
send_future_email( send_future_email(
"zerver/emails/invitation_reminder", referrer.realm, to_emails=[email], "zerver/emails/invitation_reminder", referrer.realm, to_emails=[email],
from_address=FromAddress.NOREPLY, context=context) from_address=FromAddress.no_reply_placeholder, context=context)
email_jobs_to_deliver = ScheduledEmail.objects.filter( email_jobs_to_deliver = ScheduledEmail.objects.filter(
scheduled_timestamp__lte=timezone_now(), type=ScheduledEmail.INVITATION_REMINDER) scheduled_timestamp__lte=timezone_now(), type=ScheduledEmail.INVITATION_REMINDER)

View File

@@ -241,7 +241,7 @@ class ConfirmationEmailWorker(QueueProcessingWorker):
"zerver/emails/invitation_reminder", "zerver/emails/invitation_reminder",
referrer.realm, referrer.realm,
to_emails=[invitee.email], to_emails=[invitee.email],
from_address=FromAddress.tokenized_no_reply_address(), from_address=FromAddress.tokenized_no_reply_placeholder,
language=referrer.realm.default_language, language=referrer.realm.default_language,
context=context, context=context,
delay=datetime.timedelta(days=settings.INVITATION_LINK_VALIDITY_DAYS - 2)) delay=datetime.timedelta(days=settings.INVITATION_LINK_VALIDITY_DAYS - 2))