From ca342d48389de72e2c299c613a5a0e1deebf0093 Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Tue, 1 Aug 2023 19:34:40 +0200 Subject: [PATCH] Add List-Unsubscribe email header (#26085) --- .../mail_subscriptions_controller.rb | 5 +- app/mailers/notification_mailer.rb | 8 ++ app/views/layouts/mailer.html.haml | 4 +- spec/mailers/notification_mailer_spec.rb | 43 ++++++-- spec/requests/mail_subscriptions_spec.rb | 103 ++++++++++++++++++ 5 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 spec/requests/mail_subscriptions_spec.rb diff --git a/app/controllers/mail_subscriptions_controller.rb b/app/controllers/mail_subscriptions_controller.rb index b071a80605d..1caeaaacf4c 100644 --- a/app/controllers/mail_subscriptions_controller.rb +++ b/app/controllers/mail_subscriptions_controller.rb @@ -9,6 +9,8 @@ class MailSubscriptionsController < ApplicationController before_action :set_user before_action :set_type + protect_from_forgery with: :null_session + def show; end def create @@ -20,6 +22,7 @@ class MailSubscriptionsController < ApplicationController def set_user @user = GlobalID::Locator.locate_signed(params[:token], for: 'unsubscribe') + not_found unless @user end def set_body_classes @@ -35,7 +38,7 @@ class MailSubscriptionsController < ApplicationController when 'follow', 'reblog', 'favourite', 'mention', 'follow_request' "notification_emails.#{params[:type]}" else - raise ArgumentError + not_found end end end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 277612366b7..5eecfed1040 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -8,6 +8,7 @@ class NotificationMailer < ApplicationMailer before_action :process_params before_action :set_status, only: [:mention, :favourite, :reblog] before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request] + after_action :set_list_headers! default to: -> { email_address_with_name(@user.email, @me.username) } @@ -61,6 +62,7 @@ class NotificationMailer < ApplicationMailer @me = params[:recipient] @user = @me.user @type = action_name + @unsubscribe_url = unsubscribe_url(token: @user.to_sgid(for: 'unsubscribe').to_s, type: @type) end def set_status @@ -71,6 +73,12 @@ class NotificationMailer < ApplicationMailer @account = @notification.from_account end + def set_list_headers! + headers['List-ID'] = "<#{@type}.#{@me.username}.#{Rails.configuration.x.local_domain}>" + headers['List-Unsubscribe'] = "<#{@unsubscribe_url}>" + headers['List-Unsubscribe-Post'] = 'List-Unsubscribe=One-Click' + end + def thread_by_conversation(conversation) return if conversation.nil? diff --git a/app/views/layouts/mailer.html.haml b/app/views/layouts/mailer.html.haml index e39a09780e5..7fa344a9b7e 100644 --- a/app/views/layouts/mailer.html.haml +++ b/app/views/layouts/mailer.html.haml @@ -46,9 +46,9 @@ %p= t 'about.hosted_on', domain: site_hostname %p = link_to t('application_mailer.notification_preferences'), settings_preferences_notifications_url - - if defined?(@type) + - if defined?(@unsubscribe_url) ยท - = link_to t('application_mailer.unsubscribe'), unsubscribe_url(token: @user.to_sgid(for: 'unsubscribe').to_s, type: @type) + = link_to t('application_mailer.unsubscribe'), @unsubscribe_url %td.column-cell.text-right = link_to root_url do = image_tag full_pack_url('media/images/mailer/logo.png'), alt: 'Mastodon', height: 24 diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 636c2d42576..78a497c06bd 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -3,21 +3,42 @@ require 'rails_helper' RSpec.describe NotificationMailer do - let(:receiver) { Fabricate(:user) } + let(:receiver) { Fabricate(:user, account_attributes: { username: 'alice' }) } let(:sender) { Fabricate(:account, username: 'bob') } let(:foreign_status) { Fabricate(:status, account: sender, text: 'The body of the foreign status') } let(:own_status) { Fabricate(:status, account: receiver.account, text: 'The body of the own status') } + shared_examples 'headers' do |type, thread| + it 'renders the to and from headers' do + expect(mail[:to].value).to eq "#{receiver.account.username} <#{receiver.email}>" + expect(mail.from).to eq ['notifications@localhost'] + end + + it 'renders the list headers' do + expect(mail['List-ID'].value).to eq "<#{type}.alice.cb6e6126.ngrok.io>" + expect(mail['List-Unsubscribe'].value).to match(%r{}) + expect(mail['List-Unsubscribe'].value).to match("&type=#{type}") + expect(mail['List-Unsubscribe-Post'].value).to eq 'List-Unsubscribe=One-Click' + end + + if thread + it 'renders the thread headers' do + expect(mail['In-Reply-To'].value).to match(//) + expect(mail['References'].value).to match(//) + end + end + end + describe 'mention' do let(:mention) { Mention.create!(account: receiver.account, status: foreign_status) } let(:notification) { Notification.create!(account: receiver.account, activity: mention) } let(:mail) { prepared_mailer_for(receiver.account).mention } include_examples 'localized subject', 'notification_mailer.mention.subject', name: 'bob' + include_examples 'headers', 'mention', true - it 'renders the headers' do + it 'renders the subject' do expect(mail.subject).to eq('You were mentioned by bob') - expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -32,10 +53,10 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(receiver.account).follow } include_examples 'localized subject', 'notification_mailer.follow.subject', name: 'bob' + include_examples 'headers', 'follow', false - it 'renders the headers' do + it 'renders the subject' do expect(mail.subject).to eq('bob is now following you') - expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -49,10 +70,10 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(own_status.account).favourite } include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob' + include_examples 'headers', 'favourite', true - it 'renders the headers' do + it 'renders the subject' do expect(mail.subject).to eq('bob favorited your post') - expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -67,10 +88,10 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(own_status.account).reblog } include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob' + include_examples 'headers', 'reblog', true - it 'renders the headers' do + it 'renders the subject' do expect(mail.subject).to eq('bob boosted your post') - expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -85,10 +106,10 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(receiver.account).follow_request } include_examples 'localized subject', 'notification_mailer.follow_request.subject', name: 'bob' + include_examples 'headers', 'follow_request', false - it 'renders the headers' do + it 'renders the subject' do expect(mail.subject).to eq('Pending follower: bob') - expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do diff --git a/spec/requests/mail_subscriptions_spec.rb b/spec/requests/mail_subscriptions_spec.rb new file mode 100644 index 00000000000..cc6557cab01 --- /dev/null +++ b/spec/requests/mail_subscriptions_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'MailSubscriptionsController' do + let(:user) { Fabricate(:user) } + let(:token) { user.to_sgid(for: 'unsubscribe').to_s } + let(:type) { 'follow' } + + shared_examples 'not found with invalid token' do + context 'with invalid token' do + let(:token) { 'invalid-token' } + + it 'returns http not found' do + expect(response).to have_http_status(404) + end + end + end + + shared_examples 'not found with invalid type' do + context 'with invalid type' do + let(:type) { 'invalid_type' } + + it 'returns http not found' do + expect(response).to have_http_status(404) + end + end + end + + describe 'on the unsubscribe confirmation page' do + before do + get unsubscribe_url(token: token, type: type) + end + + it_behaves_like 'not found with invalid token' + it_behaves_like 'not found with invalid type' + + it 'shows unsubscribe form' do + expect(response).to have_http_status(200) + + expect(response.body).to include( + I18n.t('mail_subscriptions.unsubscribe.action') + ) + expect(response.body).to include(user.email) + end + end + + describe 'submitting the unsubscribe confirmation page' do + before do + user.settings.update('notification_emails.follow': true) + user.save! + + post unsubscribe_url, params: { token: token, type: type } + end + + it_behaves_like 'not found with invalid token' + it_behaves_like 'not found with invalid type' + + it 'shows confirmation page' do + expect(response).to have_http_status(200) + + expect(response.body).to include( + I18n.t('mail_subscriptions.unsubscribe.complete') + ) + expect(response.body).to include(user.email) + end + + it 'updates notification settings' do + user.reload + expect(user.settings['notification_emails.follow']).to be false + end + end + + describe 'unsubscribing with List-Unsubscribe-Post' do + around do |example| + old = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + + example.run + + ActionController::Base.allow_forgery_protection = old + end + + before do + user.settings.update('notification_emails.follow': true) + user.save! + + post unsubscribe_url(token: token, type: type), params: { 'List-Unsubscribe' => 'One-Click' } + end + + it_behaves_like 'not found with invalid token' + it_behaves_like 'not found with invalid type' + + it 'return http success' do + expect(response).to have_http_status(200) + end + + it 'updates notification settings' do + user.reload + expect(user.settings['notification_emails.follow']).to be false + end + end +end