FEATURE: secure_email site setting to prevent data going out in email

This commit is contained in:
Robin Ward 2017-04-24 15:26:06 -04:00
parent b76674f640
commit bf9c4a7828
17 changed files with 281 additions and 89 deletions

View File

@ -0,0 +1,38 @@
module EmailHelper
def mailing_list_topic(topic, post_count)
render(
partial: partial_for("mailing_list_post"),
locals: { topic: topic, post_count: post_count }
)
end
def mailing_list_topic_text(topic)
url, title = extract_details(topic)
raw(@markdown_linker.create(title, url))
end
def private_topic_title(topic)
I18n.t("system_messages.secure_topic_title", id: topic.id)
end
def email_topic_link(topic)
url, title = extract_details(topic)
raw "<a href='#{Discourse.base_url}#{url}' style='color: ##{@anchor_color}'>#{title}</a>"
end
protected
def extract_details(topic)
if SiteSetting.private_email?
[topic.slugless_url, private_topic_title(topic)]
else
[topic.relative_url, format_topic_title(topic.title)]
end
end
def partial_for(name)
SiteSetting.private_email? ? "email/secure_#{name}" : "email/#{name}"
end
end

View File

@ -5,10 +5,9 @@ module Jobs
every 30.minutes
def execute(args)
unless SiteSetting.disable_digest_emails?
target_user_ids.each do |user_id|
Jobs.enqueue(:user_email, type: :digest, user_id: user_id)
end
return if SiteSetting.disable_digest_emails? || SiteSetting.private_email?
target_user_ids.each do |user_id|
Jobs.enqueue(:user_email, type: :digest, user_id: user_id)
end
end

View File

@ -5,6 +5,7 @@ class InviteMailer < ActionMailer::Base
class UserNotificationRenderer < ActionView::Base
include UserNotificationsHelper
include EmailHelper
end
def send_invite(invite, custom_message=nil)
@ -30,12 +31,18 @@ class InviteMailer < ActionMailer::Base
template = 'custom_invite_mailer'
end
topic_title = first_topic.try(:title)
if SiteSetting.private_email?
topic_title = I18n.t("system_messages.secure_topic_title", id: first_topic.id)
topic_excerpt = ""
end
build_email(invite.email,
template: template,
invitee_name: invitee_name,
site_domain_name: Discourse.current_hostname,
invite_link: "#{Discourse.base_url}/invites/#{invite.invite_key}",
topic_title: first_topic.try(:title),
topic_title: topic_title,
topic_excerpt: topic_excerpt,
site_description: SiteSetting.site_description,
site_title: SiteSetting.title,

View File

@ -5,7 +5,7 @@ require_dependency 'age_words'
class UserNotifications < ActionMailer::Base
include UserNotificationsHelper
include ApplicationHelper
helper :application
helper :application, :email
default charset: 'UTF-8'
include Email::BuildEmailHelper
@ -284,10 +284,12 @@ class UserNotifications < ActionMailer::Base
class UserNotificationRenderer < ActionView::Base
include UserNotificationsHelper
include EmailHelper
end
def self.get_context_posts(post, topic_user, user)
if user.user_option.email_previous_replies == UserOption.previous_replies_type[:never]
if (user.user_option.email_previous_replies == UserOption.previous_replies_type[:never]) ||
SiteSetting.private_email?
return []
end
@ -349,6 +351,7 @@ class UserNotifications < ActionMailer::Base
def send_notification_email(opts)
post = opts[:post]
title = opts[:title]
allow_reply_by_email = opts[:allow_reply_by_email]
use_site_subject = opts[:use_site_subject]
add_re_to_subject = opts[:add_re_to_subject] && post.post_number > 1
@ -377,6 +380,10 @@ class UserNotifications < ActionMailer::Base
show_category_in_subject = nil
end
if SiteSetting.private_email?
title = I18n.t("system_messages.private_topic_title", id: post.topic_id)
end
context = ""
tu = TopicUser.get(post.topic_id, user)
context_posts = self.class.get_context_posts(post, tu, user)
@ -418,7 +425,12 @@ class UserNotifications < ActionMailer::Base
.count) >= (SiteSetting.max_emails_per_day_per_user-1)
in_reply_to_post = post.reply_to_post if user.user_option.email_in_reply_to
message = email_post_markdown(post) + (reached_limit ? "\n\n#{I18n.t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user}" : "");
if SiteSetting.private_email?
message = I18n.t('system_messages.contents_hidden')
else
message = email_post_markdown(post) + (reached_limit ? "\n\n#{I18n.t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user}" : "");
end
unless translation_override_exists
html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render(
@ -438,7 +450,7 @@ class UserNotifications < ActionMailer::Base
topic_title: gsub_emoji_to_unicode(title),
topic_title_url_encoded: title ? URI.encode(title) : title,
message: message,
url: post.url,
url: post.url(without_slug: SiteSetting.private_email?),
post_id: post.id,
topic_id: post.topic_id,
context: context,

View File

@ -400,9 +400,11 @@ class Post < ActiveRecord::Base
"#{Discourse.base_url}#{url}"
end
def url
def url(opts=nil)
opts ||= {}
if topic
Post.url(topic.slug, topic.id, post_number)
Post.url(topic.slug, topic.id, post_number, opts)
else
"/404"
end
@ -412,8 +414,13 @@ class Post < ActiveRecord::Base
"#{Discourse.base_url}/email/unsubscribe/#{UnsubscribeKey.create_key_for(user, self)}"
end
def self.url(slug, topic_id, post_number)
"/t/#{slug}/#{topic_id}/#{post_number}"
def self.url(slug, topic_id, post_number, opts=nil)
opts ||= {}
result = "/t/"
result << "#{slug}/" unless !!opts[:without_slug]
"#{result}#{topic_id}/#{post_number}"
end
def self.urls(post_ids)

View File

@ -890,11 +890,17 @@ SQL
end
def self.relative_url(id, slug, post_number=nil)
url = "#{Discourse.base_uri}/t/#{slug}/#{id}"
url = "#{Discourse.base_uri}/t/"
url << "#{slug}/" if slug.present?
url << id.to_s
url << "/#{post_number}" if post_number.to_i > 1
url
end
def slugless_url(post_number=nil)
Topic.relative_url(id, nil, post_number)
end
def relative_url(post_number=nil)
Topic.relative_url(id, slug, post_number)
end

View File

@ -0,0 +1,5 @@
<li>
<a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
<span><%= post_count %></span>
<%= category_badge(topic.category, inline_style: true, absolute_url: true) %>
</li>

View File

@ -0,0 +1,4 @@
<li>
<%= email_topic_link(topic) %>
<span><%= post_count %></span>
</li>

View File

@ -2,28 +2,32 @@
<div class='header-instructions'>%{header_instructions}</div>
<%= render partial: 'email/post', locals: { post: post, use_excerpt: false } %>
<%- if SiteSetting.private_email? %>
<p><%= t('system_messages.contents_hidden') %></p>
<% else %>
<%= render partial: 'email/post', locals: { post: post, use_excerpt: false } %>
<% if in_reply_to_post.present? || context_posts.present? %>
<div class='footer'>%{respond_instructions}</div>
<hr>
<% end %>
<% if in_reply_to_post.present? %>
<h4 class='previous-discussion'><%= t "user_notifications.in_reply_to" %></h4>
<%= render partial: 'email/post', locals: { post: in_reply_to_post, use_excerpt: true} %>
<% end %>
<% if context_posts.present? %>
<h4 class='previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
<% context_posts.each do |p| %>
<%= render partial: 'email/post', locals: { post: p, use_excerpt: false } %>
<% if in_reply_to_post.present? || context_posts.present? %>
<div class='footer'>%{respond_instructions}</div>
<hr>
<% end %>
<% end %>
<% if reached_limit %>
<hr>
<div class='footer'><%= t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user %></div>
<% if in_reply_to_post.present? %>
<h4 class='previous-discussion'><%= t "user_notifications.in_reply_to" %></h4>
<%= render partial: 'email/post', locals: { post: in_reply_to_post, use_excerpt: true} %>
<% end %>
<% if context_posts.present? %>
<h4 class='previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
<% context_posts.each do |p| %>
<%= render partial: 'email/post', locals: { post: p, use_excerpt: false } %>
<% end %>
<% end %>
<% if reached_limit %>
<hr>
<div class='footer'><%= t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user %></div>
<% end %>
<% end %>
<div class='footer'>%{respond_instructions}</div>
@ -33,7 +37,7 @@
<div itemscope itemtype="http://schema.org/EmailMessage" style="display:none">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="<%= Discourse.base_url %><%= post.url %>" />
<link itemprop="url" href="<%= Discourse.base_url %><%= post.url(without_slug: SiteSetting.private_email?) %>" />
<meta itemprop="name" content="<%= t 'read_full_topic' %>"/>
</div>
</div>

View File

@ -21,21 +21,13 @@
<h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.new_topics') %></h3>
<ul>
<% @new_topic_posts.keys.each do |topic| %>
<li>
<a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
<span><%= @new_topic_posts[topic].length %></span>
<%= category_badge(topic.category, inline_style: true, absolute_url: true) %>
</li>
<%= mailing_list_topic(topic, @new_topic_posts[topic].length) %>
<% end %>
</ul>
<h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.topic_updates') %></h3>
<ul>
<% @existing_topic_posts.keys.each do |topic| %>
<li>
<a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
<span><%= @existing_topic_posts[topic].length %></span>
<%= category_badge(topic.category, inline_style: true, absolute_url: true) %>
</li>
<%= mailing_list_topic(topic, @existing_topic_posts[topic].length) %>
<% end %>
</ul>
</td>
@ -46,40 +38,42 @@
<tr>
<td>
<h3 style='padding: 20px 20px 10px; margin: 0;'>
<a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
<%= email_topic_link(topic) %>
</h3>
</td>
</tr>
<tr>
<td style='border-left: 20px solid #eee; border-right: 20px solid #eee; border-bottom: 10px solid #eee; padding: 10px 10px; background: #fff;'>
<% @posts_by_topic[topic].each do |post| %>
<div>
<img style="float: left; width: 20px; padding-right: 5px;" src="<%= post.user.small_avatar_url %>" title="<%= post.user.username%>">
<p style="font-size: 15px; color: #888">
<a href='<%= "#{Discourse.base_url}/u/#{post.user.username}" %>'>
<%- if show_username_on_post(post) %>
<%= post.user.username %>
<% end %>
<%- unless SiteSetting.private_email? %>
<tr>
<td style='border-left: 20px solid #eee; border-right: 20px solid #eee; border-bottom: 10px solid #eee; padding: 10px 10px; background: #fff;'>
<% @posts_by_topic[topic].each do |post| %>
<div>
<img style="float: left; width: 20px; padding-right: 5px;" src="<%= post.user.small_avatar_url %>" title="<%= post.user.username%>">
<p style="font-size: 15px; color: #888">
<a href='<%= "#{Discourse.base_url}/u/#{post.user.username}" %>'>
<%- if show_username_on_post(post) %>
<%= post.user.username %>
<% end %>
<%- if show_name_on_post(post) %>
- <%= post.user.name %>
<% end %>
</a>
<%- if show_name_on_post(post) %>
- <%= post.user.name %>
<% end %>
</a>
<span> - </span>
<span><%= I18n.l(post.created_at, format: :long) %></span>
</p>
<%= raw format_for_email(post, false) %>
<hr />
</div>
<% end %>
<a style='font-size: 12px; float: right;' href='<%= Discourse.base_url + topic.relative_url %>'><%= t('user_notifications.mailing_list.view_this_topic') %></a>
</td>
</tr>
<tr>
<td style='padding: 0 20px; font-size: 12px;'>
<a href="#top"><%= t('user_notifications.mailing_list.back_to_top') %></a>
</td>
</tr>
<span> - </span>
<span><%= I18n.l(post.created_at, format: :long) %></span>
</p>
<%= raw format_for_email(post, false) %>
<hr />
</div>
<% end %>
<a style='font-size: 12px; float: right;' href='<%= Discourse.base_url + topic.relative_url %>'><%= t('user_notifications.mailing_list.view_this_topic') %></a>
</td>
</tr>
<tr>
<td style='padding: 0 20px; font-size: 12px;'>
<a href="#top"><%= t('user_notifications.mailing_list.back_to_top') %></a>
</td>
</tr>
<% end %>
<% end %>
</table>

View File

@ -4,28 +4,30 @@
<%- if @new_topic_posts.keys.present? -%>
### <%= t 'user_notifications.mailing_list.new_topics' %>
<%- @new_topic_posts.keys.each do |topic| %>
<%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
<%= mailing_list_topic_text(topic) %>
<%- end -%>
<%- end -%>
<%- if @existing_topic_posts.keys.present? -%>
### <%= t 'user_notifications.mailing_list.new_topics' %>
<%- @existing_topic_posts.keys.each do |topic| -%>
<%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
<%= mailing_list_topic_text(topic) %>
<%= @existing_topic_posts[topic].length %>
<%- end -%>
<%- end -%>
--------------------------------------------------------------------------------
<%- @posts_by_topic.keys.each do |topic| %>
### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
<%- unless SiteSetting.private_email? %>
<%- @posts_by_topic.keys.each do |topic| %>
### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
<%- @posts_by_topic[topic].each do |post| -%>
<%= post.user.name || post.user.username %> - <%= post.created_at %>
--------------------------------------------------------------------------------
<%= post.raw %>
<%- @posts_by_topic[topic].each do |post| -%>
<%= post.user.name || post.user.username %> - <%= post.created_at %>
--------------------------------------------------------------------------------
<%= post.raw %>
<%- end -%>
<%- end -%>
<%- end %>
<%- end %>

View File

@ -1336,6 +1336,7 @@ en:
enable_forwarded_emails: "[BETA] Allow users to create a topic by forwarding an email in."
always_show_trimmed_content: "Always show trimmed part of incoming emails. WARNING: might reveal email addresses."
private_email: "Don't include content from posts or topics in emails for extra privacy."
manual_polling_enabled: "Push emails using the API for email replies."
pop3_polling_enabled: "Poll via POP3 for email replies."
@ -1934,6 +1935,9 @@ en:
other: "This topic is temporarily closed for %{count} hours due to a large number of community flags."
system_messages:
secure_topic_title: "Topic #%{id}"
contents_hidden: "Please visit the post to see its contents."
post_hidden:
title: "Post Hidden"
subject_template: "Post hidden by community flags"

View File

@ -701,6 +701,7 @@ email:
default: "smime.p7s|signature.asc"
enable_forwarded_emails: false
always_show_trimmed_content: false
private_email: false
files:
max_image_size_kb:

View File

@ -130,7 +130,14 @@ module Email
# https://www.ietf.org/rfc/rfc3834.txt
@message.header['Precedence'] = 'list'
@message.header['List-ID'] = list_id
@message.header['List-Archive'] = topic.url if topic
if topic
if SiteSetting.private_email?
@message.header['List-Archive'] = "#{Discourse.base_url}#{topic.slugless_url}"
else
@message.header['List-Archive'] = topic.url
end
end
end
if reply_key.present? && @message.header['Reply-To'] =~ /\<([^\>]+)\>/

View File

@ -6,7 +6,7 @@ describe Jobs::EnqueueDigestEmails do
describe '#target_users' do
context 'disabled digests' do
before { SiteSetting.stubs(:default_email_digest_frequency).returns(0) }
before { SiteSetting.default_email_digest_frequency = 0 }
let!(:user_no_digests) { Fabricate(:active_user, last_emailed_at: 8.days.ago, last_seen_at: 10.days.ago) }
it "doesn't return users with email disabled" do
@ -129,13 +129,24 @@ describe Jobs::EnqueueDigestEmails do
end
end
context "private email" do
before do
Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never
SiteSetting.private_email = true
Jobs.expects(:enqueue).with(:user_email, type: :digest, user_id: user.id).never
end
it "doesn't return users with email disabled" do
Jobs::EnqueueDigestEmails.new.execute({})
end
end
context "digest emails are disabled" do
before do
Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never
SiteSetting.disable_digest_emails = true
end
it "does not enqueue the digest email job" do
SiteSetting.stubs(:disable_digest_emails?).returns(true)
Jobs.expects(:enqueue).with(:user_email, type: :digest, user_id: user.id).never
Jobs::EnqueueDigestEmails.new.execute({})
end

View File

@ -67,6 +67,7 @@ describe InviteMailer do
it 'renders invite link' do
expect(custom_invite_mail.body.encoded).to match("#{Discourse.base_url}/invites/#{invite.invite_key}")
end
end
end
end
@ -111,6 +112,14 @@ describe InviteMailer do
it 'renders topic title' do
expect(invite_mail.body.encoded).to match(topic.title)
end
it "respects the private_email setting" do
SiteSetting.private_email = true
message = invite_mail
expect(message.body.to_s).not_to include(topic.title)
expect(message.body.to_s).not_to include(topic.slug)
end
end
context "custom invite message" do

View File

@ -47,7 +47,10 @@ describe UserNotifications do
user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:always])
expect(UserNotifications.get_context_posts(post3, topic_user, user).count).to eq(2)
SiteSetting.private_email = true
expect(UserNotifications.get_context_posts(post3, topic_user, user).count).to eq(0)
end
end
describe ".signup" do
@ -152,6 +155,15 @@ describe UserNotifications do
expect(subject.html_part.body.to_s).to include old_topic.title
expect(subject.html_part.body.to_s).to include whisper.cooked
end
it "hides details for private email" do
SiteSetting.private_email = true
expect(subject.html_part.body.to_s).not_to include(topic.title)
expect(subject.html_part.body.to_s).not_to include(topic.slug)
expect(subject.text_part.body.to_s).not_to include(topic.title)
expect(subject.text_part.body.to_s).not_to include(topic.slug)
end
end
end
@ -291,8 +303,6 @@ describe UserNotifications do
expect(mail.html_part.to_s.scan(/In Reply To/).count).to eq(0)
SiteSetting.enable_names = true
SiteSetting.display_name_on_posts = true
SiteSetting.prioritize_username_in_ux = false
@ -324,6 +334,21 @@ describe UserNotifications do
expect(mail_html.scan(/>Bob Marley/).count).to eq(0)
expect(mail_html.scan(/>bobmarley/).count).to eq(1)
end
it "doesn't include details when private_email is enabled" do
SiteSetting.private_email = true
mail = UserNotifications.user_replied(
response.user,
post: response,
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash
)
expect(mail.html_part.to_s).to_not include(response.raw)
expect(mail.html_part.to_s).to_not include(topic.url)
expect(mail.text_part.to_s).to_not include(response.raw)
expect(mail.text_part.to_s).to_not include(topic.url)
end
end
describe '.user_posted' do
@ -360,6 +385,19 @@ describe UserNotifications do
tu = TopicUser.get(post.topic_id, response.user)
expect(tu.last_emailed_post_number).to eq(response.post_number)
end
it "doesn't include details when private_email is enabled" do
SiteSetting.private_email = true
mail = UserNotifications.user_posted(
response.user,
post: response,
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash
)
expect(mail.html_part.to_s).to_not include(response.raw)
expect(mail.text_part.to_s).to_not include(response.raw)
end
end
describe '.user_private_message' do
@ -397,6 +435,21 @@ describe UserNotifications do
tu = TopicUser.get(topic.id, response.user)
expect(tu.last_emailed_post_number).to eq(response.post_number)
end
it "doesn't include details when private_email is enabled" do
SiteSetting.private_email = true
mail = UserNotifications.user_private_message(
response.user,
post: response,
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash
)
expect(mail.html_part.to_s).to_not include(response.raw)
expect(mail.html_part.to_s).to_not include(topic.url)
expect(mail.text_part.to_s).to_not include(response.raw)
expect(mail.text_part.to_s).to_not include(topic.url)
end
end
@ -449,6 +502,28 @@ describe UserNotifications do
end
end
shared_examples "respect for private_email" do
context "private_email" do
it "doesn't support reply by email" do
SiteSetting.private_email = true
mailer = UserNotifications.send(
mail_type,
user,
notification_type: Notification.types[notification.notification_type],
notification_data_hash: notification.data_hash,
post: notification.post
)
message = mailer.message
topic = notification.post.topic
expect(message.html_part.body.to_s).not_to include(topic.title)
expect(message.html_part.body.to_s).not_to include(topic.slug)
expect(message.text_part.body.to_s).not_to include(topic.title)
expect(message.text_part.body.to_s).not_to include(topic.slug)
end
end
end
# The parts of emails that are derived from templates are translated
shared_examples "sets user locale" do
context "set locale for translating templates" do
@ -546,6 +621,7 @@ describe UserNotifications do
describe "user mentioned email" do
include_examples "notification email building" do
let(:notification_type) { :mentioned }
include_examples "respect for private_email"
include_examples "supports reply by email"
include_examples "sets user locale"
end
@ -554,6 +630,7 @@ describe UserNotifications do
describe "user replied" do
include_examples "notification email building" do
let(:notification_type) { :replied }
include_examples "respect for private_email"
include_examples "supports reply by email"
include_examples "sets user locale"
end
@ -562,6 +639,7 @@ describe UserNotifications do
describe "user quoted" do
include_examples "notification email building" do
let(:notification_type) { :quoted }
include_examples "respect for private_email"
include_examples "supports reply by email"
include_examples "sets user locale"
end
@ -570,6 +648,7 @@ describe UserNotifications do
describe "user posted" do
include_examples "notification email building" do
let(:notification_type) { :posted }
include_examples "respect for private_email"
include_examples "supports reply by email"
include_examples "sets user locale"
end
@ -578,6 +657,7 @@ describe UserNotifications do
describe "user invited to a private message" do
include_examples "notification email building" do
let(:notification_type) { :invited_to_private_message }
include_examples "respect for private_email"
include_examples "no reply by email"
include_examples "sets user locale"
end
@ -586,6 +666,7 @@ describe UserNotifications do
describe "user invited to a topic" do
include_examples "notification email building" do
let(:notification_type) { :invited_to_topic }
include_examples "respect for private_email"
include_examples "no reply by email"
include_examples "sets user locale"
end
@ -594,6 +675,7 @@ describe UserNotifications do
describe "watching first post" do
include_examples "notification email building" do
let(:notification_type) { :invited_to_topic }
include_examples "respect for private_email"
include_examples "no reply by email"
include_examples "sets user locale"
end