mirror of
https://github.com/discourse/discourse.git
synced 2026-07-31 08:38:27 -05:00
DEV: Strip unsubscribe links when sending digest previews to arbitrary addresses (#38298)
# Problem When an admin sends a digest preview to an arbitrary email address, the email contained functional unsubscribe links belonging to the target user, allowing the recipient to unsubscribe that user without their consent. # Solution Pass `skip_unsubscribe_links: true` when generating preview digests, which skips `UnsubscribeKey` creation and suppresses unsubscribe content from both email headers and body templates at the source.
This commit is contained in:
@@ -45,12 +45,19 @@ class Admin::EmailController < Admin::AdminController
|
||||
params.require(:username)
|
||||
params.require(:email)
|
||||
user = User.find_by_username(params[:username])
|
||||
raise Discourse::InvalidParameters unless user
|
||||
|
||||
message, skip_reason =
|
||||
UserNotifications.public_send(:digest, user, since: params[:last_seen_at])
|
||||
UserNotifications.public_send(
|
||||
:digest,
|
||||
user,
|
||||
since: params[:last_seen_at],
|
||||
skip_unsubscribe_links: true,
|
||||
)
|
||||
|
||||
if message
|
||||
message.to = params[:email]
|
||||
|
||||
begin
|
||||
Email::Sender.new(message, :digest).send
|
||||
render json: success_json
|
||||
|
||||
@@ -251,7 +251,9 @@ class UserNotifications < ActionMailer::Base
|
||||
|
||||
def digest(user, opts = {})
|
||||
build_summary_for(user)
|
||||
@unsubscribe_key = UnsubscribeKey.create_key_for(@user, UnsubscribeKey::DIGEST_TYPE)
|
||||
if !opts[:skip_unsubscribe_links]
|
||||
@unsubscribe_key = UnsubscribeKey.create_key_for(@user, UnsubscribeKey::DIGEST_TYPE)
|
||||
end
|
||||
|
||||
@since = opts[:since].presence
|
||||
@since ||= [user.last_seen_at, user.user_stat&.digest_attempted_at, 1.month.ago].compact.max
|
||||
@@ -371,7 +373,7 @@ class UserNotifications < ActionMailer::Base
|
||||
email_prefix: @email_prefix,
|
||||
date: short_date(Time.now),
|
||||
),
|
||||
add_unsubscribe_link: true,
|
||||
add_unsubscribe_link: !opts[:skip_unsubscribe_links],
|
||||
unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}",
|
||||
topic_ids: topics_for_digest.pluck(:id),
|
||||
post_ids:
|
||||
|
||||
@@ -3,10 +3,12 @@
|
||||
<!-- Empty cells pad either side of the email content -->
|
||||
<td></td>
|
||||
<td width="650" align="center">
|
||||
<%=raw(t 'user_notifications.digest.unsubscribe',
|
||||
site_link: html_site_link,
|
||||
email_preferences_link: link_to(t('user_notifications.digest.your_email_settings'), Discourse.base_url + '/my/preferences/emails'),
|
||||
unsubscribe_link: link_to(t('user_notifications.digest.click_here'), "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}")) %>
|
||||
<% if @unsubscribe_key %>
|
||||
<%=raw(t 'user_notifications.digest.unsubscribe',
|
||||
site_link: html_site_link,
|
||||
email_preferences_link: link_to(t('user_notifications.digest.your_email_settings'), Discourse.base_url + '/my/preferences/emails'),
|
||||
unsubscribe_link: link_to(t('user_notifications.digest.click_here'), "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}")) %>
|
||||
<% end %>
|
||||
</td>
|
||||
<td></td>
|
||||
</tr>
|
||||
|
||||
@@ -3,10 +3,12 @@
|
||||
<%= raw(@markdown_linker.references) %>
|
||||
|
||||
<%= digest_custom_text("above_footer") %>
|
||||
<%=raw(t :'user_notifications.digest.unsubscribe',
|
||||
site_link: site_link,
|
||||
email_preferences_link: raw(@markdown_linker.create(t('user_notifications.digest.your_email_settings'), '/my/preferences/emails')),
|
||||
unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), "/email/unsubscribe/#{@unsubscribe_key}"))) %>
|
||||
<%- if @unsubscribe_key %>
|
||||
<%=raw(t :'user_notifications.digest.unsubscribe',
|
||||
site_link: site_link,
|
||||
email_preferences_link: raw(@markdown_linker.create(t('user_notifications.digest.your_email_settings'), '/my/preferences/emails')),
|
||||
unsubscribe_link: raw(@markdown_linker.create(t('user_notifications.digest.click_here'), "/email/unsubscribe/#{@unsubscribe_key}"))) %>
|
||||
<%- end %>
|
||||
|
||||
<%= raw(@markdown_linker.references) %>
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ export default class AdminEmailPreviewDigestController extends Controller {
|
||||
|
||||
username = null;
|
||||
lastSeen = null;
|
||||
email = null;
|
||||
|
||||
@empty("email") emailEmpty;
|
||||
@or("emailEmpty", "sendingEmail") sendEmailDisabled;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { click, visit, waitUntil } from "@ember/test-helpers";
|
||||
import { click, fillIn, visit, waitUntil } from "@ember/test-helpers";
|
||||
import { test } from "qunit";
|
||||
import { acceptance } from "discourse/tests/helpers/qunit-helpers";
|
||||
|
||||
@@ -36,4 +36,29 @@ acceptance("Admin - email-preview", function (needs) {
|
||||
"text content is escaped correctly"
|
||||
);
|
||||
});
|
||||
|
||||
test("send digest form requires a destination email", async function (assert) {
|
||||
await visit("/admin/email/preview-digest");
|
||||
|
||||
assert
|
||||
.dom(".email-preview-digest .controls")
|
||||
.exists("send email form is shown");
|
||||
|
||||
assert
|
||||
.dom(".email-preview-digest .controls input[type='text']")
|
||||
.exists("an email text field is shown for sending this digest preview");
|
||||
|
||||
assert
|
||||
.dom(".email-preview-digest .controls .btn-default")
|
||||
.isDisabled("send button is disabled until an email address is entered");
|
||||
|
||||
await fillIn(
|
||||
".email-preview-digest .controls input[type='text']",
|
||||
"preview@example.com"
|
||||
);
|
||||
|
||||
assert
|
||||
.dom(".email-preview-digest .controls .btn-default")
|
||||
.isNotDisabled("send button is enabled once an email address is entered");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -238,10 +238,34 @@ RSpec.describe Admin::EmailController do
|
||||
params: {
|
||||
last_seen_at: 1.week.ago,
|
||||
username: admin.username,
|
||||
email: email("previous_replies"),
|
||||
email: admin.email,
|
||||
}
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
it "sends the digest to the requested email without unsubscribe links" do
|
||||
Fabricate(:topic, created_at: 1.day.ago)
|
||||
ActionMailer::Base.deliveries.clear
|
||||
|
||||
expect {
|
||||
post "/admin/email/send-digest.json",
|
||||
params: {
|
||||
last_seen_at: 1.month.ago,
|
||||
username: user.username,
|
||||
email: "attacker@evil.com",
|
||||
}
|
||||
}.not_to change { UnsubscribeKey.count }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.parsed_body["success"]).to eq("OK")
|
||||
|
||||
message = ActionMailer::Base.deliveries.last
|
||||
expect(message.to).to eq(["attacker@evil.com"])
|
||||
expect(message.html_part.body.to_s).not_to include("/email/unsubscribe/")
|
||||
expect(message.text_part.body.to_s).not_to include("/email/unsubscribe/")
|
||||
expect(message.header["List-Unsubscribe"].to_s).to be_blank
|
||||
expect(message.header["List-Unsubscribe-Post"].to_s).to be_blank
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user