Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Pro] Site email handling for pro users #3706

Merged
merged 17 commits into from
Feb 6, 2017
Merged

[Pro] Site email handling for pro users #3706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a5b88df
Send admin contact emails from pros to pro address
Jan 19, 2017
c539073
Set from: address to pro contact when admins email pros
Jan 19, 2017
b09ff28
Set @contact_email appropriately on contact form for pros
Jan 19, 2017
f99ec30
Move mail_user and auto_generated_headers helpers into application_ma…
Jan 27, 2017
ed051c5
Make new mailer helpers use correct from/reply-to for pros
Jan 27, 2017
b2e2a25
Make auto_generated_headers accept a dict of headers to use
Jan 30, 2017
b1def0c
Separate auto-generated and reply-to mail header setting
Feb 1, 2017
cb0db99
Use headers helper in contact_mailer where possible
Jan 30, 2017
9fd1996
Use mail_user helper in info_request_batch_mailer
Jan 30, 2017
cce7113
Use new application_mailer helper methods in request_mailer
Jan 30, 2017
c30338b
Move custom contact email to top of track_mailer
Jan 30, 2017
63fbfde
Use new mailer helper methods in user_mailer
Jan 30, 2017
b657053
Add a config setting for pro email to be forwarded to
Jan 27, 2017
a496759
Add fixture emails for explicitly testing non-bounce replies
Jan 27, 2017
c10e60d
Test that reply forwarding actually calls sendmail
Jan 27, 2017
e17c830
Forward pro and normal non-bounce replies to different addresses
Jan 27, 2017
8bb6946
Refactor reply-handling into a module in /lib
Jan 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/controllers/help_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ def unhappy

def contact
@contact_email = AlaveteliConfiguration::contact_email
if feature_enabled?(:alaveteli_pro) && @user && @user.pro?
@contact_email = AlaveteliConfiguration::pro_contact_email
end

# if they clicked remove for link to request/body, remove it
if params[:remove]
Expand Down
48 changes: 48 additions & 0 deletions app/mailers/application_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class ApplicationMailer < ActionMailer::Base
# Include all the functions views get, as emails call similar things.
helper :application
include MailerHelper
include AlaveteliFeatures::Helpers

# This really should be the default - otherwise you lose any information
# about the errors, and have to do error checking on return codes.
Expand All @@ -19,6 +20,53 @@ def blackhole_email
AlaveteliConfiguration::blackhole_prefix+"@"+AlaveteliConfiguration::incoming_email_domain
end

def mail_user(user, subject)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is going to set autogenerated headers, would be good to have the name of the function reflect that.

mail({
:from => contact_for_user(user),
:to => user.name_and_email,
:subject => subject,
})
end

def contact_for_user(user = nil)
if feature_enabled?(:alaveteli_pro) and user and user.pro?
pro_contact_from_name_and_email
else
contact_from_name_and_email
end
end

# Set headers that mark an email as being auto-generated and suppress out of
# office responses to them
def set_auto_generated_headers(opts = {})
headers({
'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834
'X-Auto-Response-Suppress' => 'OOF',
})
end

# Set Return-Path and Reply-To headers
#
# Note:
# - We set Return-Path, so you should always set Reply-To to be different
# from From, since some email clients seem to erroneously use the envelope
# from when they shouldn't, and this might help. (Have had mysterious
# cases of a reply coming in duplicate from a public body to both From and
# envelope from).
# - Return-Path is a special address we control so that SPF checks are done
# on it.
# - When sending emails from one user to another, do not set envelope from
# address to the from_user, so they can't get someone's email addresses
# from transitory bounce messages.
def set_reply_to_headers(user = nil, opts = {})
default_opts = {
'Return-Path' => blackhole_email,
'Reply-To' => contact_for_user(user)
}
default_opts.merge!(opts)
headers(default_opts)
end

# URL generating functions are needed by all controllers (for redirects),
# views (for links) and mailers (for use in emails), so include them into
# all of all.
Expand Down
50 changes: 20 additions & 30 deletions app/mailers/contact_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,44 +6,41 @@
# Email: hello@mysociety.org; WWW: http://www.mysociety.org/

class ContactMailer < ApplicationMailer
include AlaveteliFeatures::Helpers

# Send message to administrator
def to_admin_message(name, email, subject, message, logged_in_user, last_request, last_body)
@message, @logged_in_user, @last_request, @last_body = message, logged_in_user, last_request, last_body

# Return path is an address we control so that SPF checks are done on it.
headers('Return-Path' => blackhole_email,
'Reply-To' => MailHandler.address_from_name_and_email(name, email))
reply_to_address = MailHandler.address_from_name_and_email(name, email)
set_reply_to_headers(nil, 'Reply-To' => reply_to_address)

# From is an address we control so that strict DMARC senders don't get refused
mail(:from => MailHandler.address_from_name_and_email(name, blackhole_email),
:to => contact_from_name_and_email,
:to => contact_for_user(@logged_in_user),
:subject => subject)
end

# We always set Reply-To when we set Return-Path to be different from From,
# since some email clients seem to erroneously use the envelope from when
# they shouldn't, and this might help. (Have had mysterious cases of a
# reply coming in duplicate from a public body to both From and envelope
# from)

# Send message to another user
def user_message(from_user, recipient_user, from_user_url, subject, message)
@message, @from_user, @recipient_user, @from_user_url = message, from_user, recipient_user, from_user_url

# Do not set envelope from address to the from_user, so they can't get
# someone's email addresses from transitory bounce messages.
headers('Return-Path' => blackhole_email, 'Reply-To' => from_user.name_and_email)
set_reply_to_headers(nil, 'Reply-To' => from_user.name_and_email)

# From is an address we control so that strict DMARC senders don't get refused
mail(:from => MailHandler.address_from_name_and_email(from_user.name, blackhole_email),
:to => recipient_user.name_and_email,
:subject => subject)
end

# Send message to a user from the administrator
def from_admin_message(recipient_name, recipient_email, subject, message)
@message, @from_user = message, contact_from_name_and_email
@message = message
@recipient_name, @recipient_email = recipient_name, recipient_email
mail(:from => contact_from_name_and_email,

recipient_user = User.find_by_email(recipient_email)

mail(:from => contact_for_user(recipient_user),
:to => MailHandler.address_from_name_and_email(@recipient_name, @recipient_email),
:bcc => AlaveteliConfiguration::contact_email,
:subject => subject)
Expand All @@ -53,14 +50,10 @@ def from_admin_message(recipient_name, recipient_email, subject, message)
def add_public_body(change_request)
@change_request = change_request


# Return path is an address we control so that SPF checks are done on it.
headers('Return-Path' => blackhole_email,
'Reply-To' => MailHandler.address_from_name_and_email(
@change_request.get_user_name,
@change_request.get_user_email
)
)
reply_to_address = MailHandler.address_from_name_and_email(
@change_request.get_user_name,
@change_request.get_user_email)
set_reply_to_headers(nil, 'Reply-To' => reply_to_address)

# From is an address we control so that strict DMARC senders don't get refused
mail(:from => MailHandler.address_from_name_and_email(
Expand All @@ -77,13 +70,10 @@ def add_public_body(change_request)
def update_public_body_email(change_request)
@change_request = change_request

# Return path is an address we control so that SPF checks are done on it.
headers('Return-Path' => blackhole_email,
'Reply-To' => MailHandler.address_from_name_and_email(
@change_request.get_user_name,
@change_request.get_user_email
)
)
reply_to_address = MailHandler.address_from_name_and_email(
@change_request.get_user_name,
@change_request.get_user_email)
set_reply_to_headers(nil, 'Reply-To' => reply_to_address)

# From is an address we control so that strict DMARC senders don't get refused
mail(:from => MailHandler.address_from_name_and_email(
Expand Down
14 changes: 8 additions & 6 deletions app/mailers/info_request_batch_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,20 @@ class InfoRequestBatchMailer < ApplicationMailer

def batch_sent(info_request_batch, unrequestable, user)
@info_request_batch, @unrequestable = info_request_batch, unrequestable
headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email)

# Make a link going to the info request batch page, which logs the user in.
post_redirect = PostRedirect.new(
:uri => info_request_batch_url(@info_request_batch),
:user_id => info_request_batch.user_id)
:user_id => info_request_batch.user_id)
post_redirect.save!
@url = confirm_url(:email_token => post_redirect.email_token)

mail(:from => contact_from_name_and_email,
:to => user.name_and_email,
:subject => _("Your batch request \"{{title}}\" has been sent",
:title => info_request_batch.title.html_safe))
set_reply_to_headers(user)

mail_user(
user,
_("Your batch request \"{{title}}\" has been sent",
:title => info_request_batch.title.html_safe)
)
end
end
74 changes: 28 additions & 46 deletions app/mailers/request_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,20 @@ def stopped_responses(info_request, email, raw_email_data)
# An FOI response is outside the scope of the system, and needs admin attention
def requires_admin(info_request, set_by = nil, message = "")
user = set_by || info_request.user
to = if feature_enabled?(:alaveteli_pro) and user and user.pro?
pro_contact_from_name_and_email
else
contact_from_name_and_email
end
@reported_by = user
@url = request_url(info_request)
@admin_url = admin_request_url(info_request)
@info_request = info_request
@message = message

# Return path is an address we control so that SPF checks are done on it.
headers('Return-Path' => blackhole_email,
'Reply-To' => user.name_and_email)
set_reply_to_headers(nil, 'Reply-To' => user.name_and_email)

# From is an address we control so that strict DMARC senders don't get refused
mail(:from => MailHandler.address_from_name_and_email(
user.name,
blackhole_email
),
:to => to,
:to => contact_for_user(user),
:subject => _("FOI response requires admin ({{reason}}) - " \
"{{request_title}}",
:reason => info_request.described_state,
Expand All @@ -91,17 +84,16 @@ def new_response(info_request, incoming_message)
@url = incoming_message_url(incoming_message, :cachebust => true)
@incoming_message, @info_request = incoming_message, info_request

headers('Return-Path' => blackhole_email,
'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834
'X-Auto-Response-Suppress' => 'OOF')

mail(:from => contact_from_name_and_email,
:to => info_request.user.name_and_email,
:subject => _("New response to your FOI request - {{request_title}}",
:request_title => info_request.title.html_safe),
:charset => "UTF-8",
# not much we can do if the user's email is broken
:reply_to => contact_from_name_and_email)
set_reply_to_headers(info_request.user)
set_auto_generated_headers

mail(
:from => contact_for_user(info_request.user),
:to => info_request.user.name_and_email,
:subject => _("New response to your FOI request - {{request_title}}",
:request_title => info_request.title.html_safe),
:charset => "UTF-8"
)
end

# Tell the requester that the public body is late in replying
Expand All @@ -117,7 +109,9 @@ def overdue_alert(info_request, user)
@url = confirm_url(:email_token => post_redirect.email_token)
@info_request = info_request

auto_generated_headers
set_reply_to_headers(user)
set_auto_generated_headers

mail_user(
user,
_("Delayed response to your FOI request - {{request_title}}",
Expand All @@ -136,7 +130,9 @@ def very_overdue_alert(info_request, user)
@url = confirm_url(:email_token => post_redirect.email_token)
@info_request = info_request

auto_generated_headers
set_reply_to_headers(user)
set_auto_generated_headers

mail_user(
user,
_("You're long overdue a response to your FOI request - {{request_title}}",
Expand All @@ -157,7 +153,8 @@ def new_response_reminder_alert(info_request, incoming_message)
@incoming_message = incoming_message
@info_request = info_request

auto_generated_headers
set_reply_to_headers(info_request.user)
set_auto_generated_headers
mail_user(info_request.user, _("Was the response you got to your FOI " \
"request any good?"))
end
Expand All @@ -167,7 +164,8 @@ def old_unclassified_updated(info_request)
@url = request_url(info_request)
@info_request = info_request

auto_generated_headers
set_reply_to_headers(info_request.user)
set_auto_generated_headers
mail_user(info_request.user, _("Someone has updated the status of " \
"your request"))
end
Expand All @@ -185,7 +183,8 @@ def not_clarified_alert(info_request, incoming_message)
@incoming_message = incoming_message
@info_request = info_request

auto_generated_headers
set_reply_to_headers(info_request.user)
set_auto_generated_headers
mail_user(
info_request.user,
_("Clarify your FOI request - {{request_title}}",
Expand All @@ -198,7 +197,8 @@ def comment_on_alert(info_request, comment)
@comment, @info_request = comment, info_request
@url = comment_url(comment)

auto_generated_headers
set_reply_to_headers(info_request.user)
set_auto_generated_headers
mail_user(
info_request.user,
_("Somebody added a note to your FOI request - {{request_title}}",
Expand All @@ -212,7 +212,8 @@ def comment_on_alert_plural(info_request, count, earliest_unalerted_comment)
@count, @info_request = count, info_request
@url = comment_url(earliest_unalerted_comment)

auto_generated_headers
set_reply_to_headers(info_request.user)
set_auto_generated_headers
mail_user(
info_request.user,
_("Some notes have been added to your FOI request - {{request_title}}",
Expand Down Expand Up @@ -521,23 +522,4 @@ def self.alert_comment_on_request
end
end

private

def auto_generated_headers
headers({
'Return-Path' => blackhole_email,
'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken
'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834
'X-Auto-Response-Suppress' => 'OOF',
})
end

def mail_user(user, subject)
mail({
:from => contact_from_name_and_email,
:to => user.name_and_email,
:subject => subject,
})
end

end
10 changes: 6 additions & 4 deletions app/mailers/track_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
# Email: hello@mysociety.org; WWW: http://www.mysociety.org/

class TrackMailer < ApplicationMailer
# Note that this is different from all the other mailers, as tracks are
# sent from a different email address and have different bounce handling.
def contact_from_name_and_email
"#{AlaveteliConfiguration::track_sender_name} <#{AlaveteliConfiguration::track_sender_email}>"
end

def event_digest(user, email_about_things)
@user, @email_about_things = user, email_about_things

Expand All @@ -27,10 +33,6 @@ def event_digest(user, email_about_things)
:site_name => site_name.html_safe))
end

def contact_from_name_and_email
"#{AlaveteliConfiguration::track_sender_name} <#{AlaveteliConfiguration::track_sender_email}>"
end

# Send email alerts for tracked things. Never more than one email
# a day, nor about events which are more than a week old, nor
# events about which emails have been sent within the last two
Expand Down
Loading