From ed9986def6531b2b7b743bf559470d459ae0154e Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 28 Sep 2022 19:44:29 -0500 Subject: [PATCH] emails: fix one-click unsubscription. Fix it so that emails are (hopefully) able to show the one-click unsubscribe button in Gmail and other mail providers that support the List-Unsubscribe header. This way users can unsubscribe instead of marking emails as spam. * Add the List-Unsubscribe-Post header. * Fix the disable email notifications endpoint to support POST as well as DELETE requests. * Fix the disable email notifications endpoint to disable XSRF protection (we don't need users to be logged in because we use a signed URL instead). https://www.rfc-editor.org/rfc/rfc8058#section-3.1 https://www.rfc-editor.org/rfc/rfc8058#section-8.1 --- .../user/email_notifications_controller.rb | 23 +++++++++++++------ app/mailers/application_mailer.rb | 9 +++++++- config/routes.rb | 2 +- .../user/email_notifications_controller.rb | 11 +++++++-- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/app/controllers/maintenance/user/email_notifications_controller.rb b/app/controllers/maintenance/user/email_notifications_controller.rb index 8b8d77cde..b6e52135f 100644 --- a/app/controllers/maintenance/user/email_notifications_controller.rb +++ b/app/controllers/maintenance/user/email_notifications_controller.rb @@ -7,8 +7,9 @@ module Maintenance respond_to :html, :json, :xml - before_action :validate_sig, :only => [:destroy] - rescue_from VerificationError, with: :render_verification_error + before_action :validate_sig, only: [:create, :destroy] + skip_forgery_protection only: [:create, :destroy] + rescue_with VerificationError, status: 403 def show end @@ -16,15 +17,23 @@ module Maintenance def destroy @user = ::User.find(params[:user_id]) @user.update!(receive_email_notifications: false) - respond_with(@user) + + # https://www.rfc-editor.org/rfc/rfc8058#section-3.1 + # + # A mail receiver can do a one-click unsubscription by performing an HTTPS POST to the HTTPS URI in the + # List-Unsubscribe header. It sends the key/value pair in the List-Unsubscribe-Post header as the request body. + # The List-Unsubscribe-Post header MUST contain the single key/value pair "List-Unsubscribe=One-Click". + # The mail sender MUST NOT return an HTTPS redirect + if params["List-Unsubscribe"] == "One-Click" + head 200 + else + respond_with(@user) + end end + alias_method :create, :destroy private - def render_verification_error - render plain: "", status: 403 - end - def validate_sig verifier = ActiveSupport::MessageVerifier.new(Danbooru.config.email_key, digest: "SHA256", serializer: JSON) calculated_sig = verifier.generate(params[:user_id].to_s) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 547d7f6cc..5e0757ee9 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -11,7 +11,14 @@ class ApplicationMailer < ActionMailer::Base default from: "#{Danbooru.config.canonical_app_name} <#{Danbooru.config.contact_email}>", content_type: "text/html" def mail(user, require_verified_email:, **options) - headers["List-Unsubscribe"] = disable_email_notifications_url(user) + # https://www.rfc-editor.org/rfc/rfc8058#section-3.1 + # + # A mail receiver can do a one-click unsubscription by performing an HTTPS POST to the HTTPS URI in the + # List-Unsubscribe header. It sends the key/value pair in the List-Unsubscribe-Post header as the request body. + # The List-Unsubscribe-Post header MUST contain the single key/value pair "List-Unsubscribe=One-Click". + headers["List-Unsubscribe"] = "<#{disable_email_notifications_url(user)}>" + headers["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + message = super(to: user.email_address&.address, **options) message.perform_deliveries = user.can_receive_email?(require_verified_email: require_verified_email) message diff --git a/config/routes.rb b/config/routes.rb index 034419dcb..7f80ec25d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,7 +49,7 @@ Rails.application.routes.draw do namespace :maintenance do namespace :user do resource :count_fixes, only: [:new, :create] - resource :email_notification, :only => [:show, :destroy] + resource :email_notification, only: [:show, :create, :destroy] resource :deletion, :only => [:show, :destroy] end end diff --git a/test/functional/maintenance/user/email_notifications_controller.rb b/test/functional/maintenance/user/email_notifications_controller.rb index e60526214..4cc7e7801 100644 --- a/test/functional/maintenance/user/email_notifications_controller.rb +++ b/test/functional/maintenance/user/email_notifications_controller.rb @@ -19,14 +19,21 @@ module Maintenance context "#destroy" do should "disable email notifications" do - delete_auth maintenance_user_email_notification_path(user_id: @user.id, sig: @sig), @user + delete maintenance_user_email_notification_path(user_id: @user.id, sig: @sig) + + assert_response :success + assert_equal(false, @user.reload.receive_email_notifications) + end + + should "disable email notifications from a one-click unsubscribe" do + post maintenance_user_email_notification_path(user_id: @user.id, sig: @sig), params: { "List-Unsubscribe": "One-Click" } assert_response :success assert_equal(false, @user.reload.receive_email_notifications) end should "not disable email notifications when given an incorrect signature" do - delete_auth maintenance_user_email_notification_path(user_id: @user.id, sig: "foo"), @user + delete maintenance_user_email_notification_path(user_id: @user.id, sig: "foo") assert_response 403 assert_equal(true, @user.reload.receive_email_notifications)