diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb index a3e3f0fb3..1e2007555 100644 --- a/app/controllers/emails_controller.rb +++ b/app/controllers/emails_controller.rb @@ -58,7 +58,7 @@ class EmailsController < ApplicationController def send_confirmation @user = authorize User.find(params[:user_id]), policy_class: EmailAddressPolicy - UserMailer.welcome_user(@user).deliver_later + UserMailer.with_request(request).welcome_user(@user).deliver_later flash[:notice] = "Confirmation email sent to #{@user.email_address.address}. Check your email to confirm your address" redirect_to @user diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index a98c1c733..92fe1dc69 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -12,7 +12,7 @@ class PasswordResetsController < ApplicationController flash[:notice] = "That account does not exist" redirect_to password_reset_path elsif @user.can_receive_email?(require_verified_email: false) - UserMailer.password_reset(@user).deliver_later + UserMailer.with_request(request).password_reset(@user).deliver_later UserEvent.create_from_request!(@user, :password_reset, request) flash[:notice] = "Password reset email sent. Check your email" respond_with(@user, location: new_session_path) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 29b1eb17d..8986c1272 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -90,7 +90,7 @@ class UsersController < ApplicationController flash[:notice] = "Sign up failed: #{@user.errors.full_messages.join("; ")}" else session[:user_id] = @user.id - UserMailer.welcome_user(@user).deliver_later + UserMailer.with_request(request).welcome_user(@user).deliver_later set_current_user end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index c8cc1851c..612967b8b 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -34,7 +34,7 @@ class ApplicationJob < ActiveJob::Base PruneRateLimitsJob, ProcessUploadJob, RegeneratePostCountsJob, RegeneratePostJob, RetireTagRelationshipsJob, VacuumDatabaseJob, DiscordNotificationJob, BigqueryExportJob, ProcessBulkUpdateRequestJob, - PruneJobsJob, ActionMailer::MailDeliveryJob + PruneJobsJob, MailDeliveryJob ] end end diff --git a/app/jobs/mail_delivery_job.rb b/app/jobs/mail_delivery_job.rb new file mode 100644 index 000000000..63f4a977d --- /dev/null +++ b/app/jobs/mail_delivery_job.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# A replacement for the default ActionMailer::MailDeliveryJob that inherits from ApplicationJob, so +# it inherits the same behavior as other jobs. It also inserts the job ID into the mail headers +# for logging purposes. +# +# @see https://github.com/rails/rails/blob/main/actionmailer/lib/action_mailer/mail_delivery_job.rb +# @see https://guides.rubyonrails.org/configuring.html#config-action-mailer-delivery-job +# @see config/application.rb (config.action_mailer.delivery_job = "MailDeliveryJob") +class MailDeliveryJob < ApplicationJob + def perform(mailer, mail_method, delivery_method, args:, kwargs: nil, params: nil) + mailer_class = mailer.constantize.with(params.to_h) # mailer_class = UserMailer.with(params) + mail = mailer_class.public_send(mail_method, *args, **kwargs.to_h) # mail = UserMailer.welcome_user(user) + + mail.headers( + "X-Danbooru-Job-Id": job_id, + "X-Danbooru-Enqueued-At": enqueued_at, + ) + + mail.send(delivery_method) # mail.deliver_now + end +end diff --git a/app/logical/email_delivery_logger.rb b/app/logical/email_delivery_logger.rb new file mode 100644 index 000000000..5707865d1 --- /dev/null +++ b/app/logical/email_delivery_logger.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# This is called just before an email is sent out to log information about the email. +# +# @see config/application.rb (config.action_mailer.interceptors) +# @see https://guides.rubyonrails.org/action_mailer_basics.html#intercepting-emails +class EmailDeliveryLogger + def self.delivering_email(email) + DanbooruLogger.info("Delivering email to #{email.to}", headers: email.headers) + end +end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 5e0757ee9..077464051 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -3,12 +3,14 @@ # The base class for emails sent by Danbooru. # # @see https://guides.rubyonrails.org/action_mailer_basics.html +# @see app/logical/email_interceptor.rb class ApplicationMailer < ActionMailer::Base helper :application helper :users include UsersHelper default from: "#{Danbooru.config.canonical_app_name} <#{Danbooru.config.contact_email}>", content_type: "text/html" + default "Message-ID": -> { "<#{SecureRandom.uuid}@#{Danbooru.config.hostname}>" } def mail(user, require_verified_email:, **options) # https://www.rfc-editor.org/rfc/rfc8058#section-3.1 @@ -19,8 +21,29 @@ class ApplicationMailer < ActionMailer::Base headers["List-Unsubscribe"] = "<#{disable_email_notifications_url(user)}>" headers["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + headers["X-Danbooru-User"] = "#{user.name} <#{user_url(user)}>" + if params.to_h[:request] + headers["X-Danbooru-URL"] = params[:request][:url] + headers["X-Danbooru-IP"] = params[:request][:remote_ip] + headers["X-Danbooru-Session"] = params[:request][:session_id] + headers["X-Request-Id"] = params[:request][:request_id] + end + + headers(params.to_h[:headers].to_h) + message = super(to: user.email_address&.address, **options) message.perform_deliveries = user.can_receive_email?(require_verified_email: require_verified_email) message end + + def self.with_request(request) + with( + request: { + url: "#{request.method} #{request.url}", + remote_ip: request.remote_ip.to_s, + request_id: request.request_id.to_s, + session_id: request.session.id.to_s, + } + ) + end end diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 09d6e952a..de68e9bbf 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -146,7 +146,7 @@ class Dmail < ApplicationRecord def send_email if is_recipient? && !is_deleted? && to.receive_email_notifications? - UserMailer.dmail_notice(self).deliver_later + UserMailer.with(headers: { "X-Danbooru-Dmail": Routes.dmail_url(self) }).dmail_notice(self).deliver_later end end diff --git a/app/models/user.rb b/app/models/user.rb index b2b1fc426..06b06166c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -379,7 +379,7 @@ class User < ApplicationRecord if errors.none? UserEvent.create_from_request!(self, :email_change, request) - UserMailer.email_change_confirmation(self).deliver_later + UserMailer.with_request(request).email_change_confirmation(self).deliver_later end end end diff --git a/config/application.rb b/config/application.rb index 4279acfdc..3576f4a66 100644 --- a/config/application.rb +++ b/config/application.rb @@ -74,6 +74,14 @@ module Danbooru config.action_mailer.sendmail_settings = Danbooru.config.mail_settings end + # https://guides.rubyonrails.org/action_mailer_basics.html#intercepting-and-observing-emails + # app/logical/email_delivery_logger.rb + config.action_mailer.interceptors = ["EmailDeliveryLogger"] + + # https://guides.rubyonrails.org/configuring.html#config-action-mailer-delivery-job + # app/jobs/mail_delivery_job.rb + config.action_mailer.delivery_job = "MailDeliveryJob" + config.log_tags = [->(req) {"PID:#{Process.pid}"}] config.action_controller.action_on_unpermitted_parameters = :raise diff --git a/test/functional/dmails_controller_test.rb b/test/functional/dmails_controller_test.rb index f1dfca28c..ddded7eac 100644 --- a/test/functional/dmails_controller_test.rb +++ b/test/functional/dmails_controller_test.rb @@ -147,6 +147,9 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to Dmail.last assert_enqueued_emails 1 + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) end should "not allow banned users to send dmails" do diff --git a/test/functional/emails_controller_test.rb b/test/functional/emails_controller_test.rb index 117f86213..063f3668a 100644 --- a/test/functional/emails_controller_test.rb +++ b/test/functional/emails_controller_test.rb @@ -113,8 +113,11 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to(settings_path) assert_equal("abc@ogres.net", @user.reload.email_address.address) assert_equal(false, @user.email_address.is_verified) - assert_enqueued_email_with UserMailer, :email_change_confirmation, args: [@user], queue: "default" assert_equal(true, @user.user_events.email_change.exists?) + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) + # assert_enqueued_email_with UserMailer.with_request(request), :email_change_confirmation, args: [@user], queue: "default" end should "create a new address" do @@ -127,8 +130,11 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to(settings_path) assert_equal("abc@ogres.net", @user.reload.email_address.address) assert_equal(false, @user.reload.email_address.is_verified) - assert_enqueued_email_with UserMailer, :email_change_confirmation, args: [@user], queue: "default" assert_equal(true, @user.user_events.email_change.exists?) + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) + # assert_enqueued_email_with UserMailer.with_request(request), :email_change_confirmation, args: [@user], queue: "default" end should "not allow banned users to change their email address" do diff --git a/test/functional/password_resets_controller_test.rb b/test/functional/password_resets_controller_test.rb index c4ad7b123..78bd41068 100644 --- a/test/functional/password_resets_controller_test.rb +++ b/test/functional/password_resets_controller_test.rb @@ -15,8 +15,11 @@ class PasswordResetsControllerTest < ActionDispatch::IntegrationTest post password_reset_path, params: { user: { name: @user.name } } assert_redirected_to new_session_path - assert_enqueued_email_with UserMailer, :password_reset, args: [@user], queue: "default" assert_equal(true, @user.user_events.password_reset.exists?) + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) + #assert_enqueued_email_with UserMailer.with_request(request), :password_reset, args: [@user], queue: "default" end should "should fail if the user doesn't have a verified email address" do diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 55c574ac6..cf5fa3a8c 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -266,8 +266,11 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_equal(User::Levels::MEMBER, User.last.level) assert_equal(User.last, User.last.authenticate_password("xxxxx1")) assert_nil(User.last.email_address) - assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last], queue: "default" assert_equal(true, User.last.user_events.user_creation.exists?) + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) + # assert_enqueued_email_with UserMailer.with_request(request), :welcome_user, args: [User.last], queue: "default" end should "create a user with a valid email" do @@ -277,8 +280,11 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_equal("xxx", User.last.name) assert_equal(User.last, User.last.authenticate_password("xxxxx1")) assert_equal("webmaster@danbooru.donmai.us", User.last.email_address.address) - assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last], queue: "default" assert_equal(true, User.last.user_events.user_creation.exists?) + + perform_enqueued_jobs + assert_performed_jobs(1, only: MailDeliveryJob) + # assert_enqueued_email_with UserMailer.with_request(request), :welcome_user, args: [User.last], queue: "default" end should "not create a user with an invalid email" do