emails: fix sending emails to invalid addresses.

Fix mailers to not attempt deliveries to invalid or nonexistent email
addresses. This usually happened when someone changed their email, and
we tried to send a confirmation email to a nonexistent address.
This commit is contained in:
evazion
2022-01-02 12:23:01 -06:00
parent 9ffc269e43
commit 32613f9bb1
8 changed files with 30 additions and 13 deletions

View File

@@ -9,7 +9,7 @@ class PasswordResetsController < ApplicationController
if @user.blank?
flash[:notice] = "That account does not exist"
redirect_to password_reset_path
elsif @user.can_receive_email?(require_verification: false)
elsif @user.can_receive_email?(require_verified_email: false)
UserMailer.password_reset(@user).deliver_later
UserEvent.create_from_request!(@user, :password_reset, request)
flash[:notice] = "Password reset email sent. Check your email"

View File

@@ -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 if @user.can_receive_email?(require_verification: false)
UserMailer.welcome_user(@user).deliver_later
set_current_user
end

View File

@@ -5,4 +5,11 @@
# @see https://guides.rubyonrails.org/action_mailer_basics.html
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)
to = email_address_with_name(user.email_address&.address, user.name)
message = super(to: to, **options)
message.perform_deliveries = user.can_receive_email?(require_verified_email: require_verified_email)
message
end
end

View File

@@ -7,24 +7,24 @@ class UserMailer < ApplicationMailer
# The email sent when a user receives a DMail.
def dmail_notice(dmail)
@dmail = dmail
mail to: dmail.to.email_with_name, subject: "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}"
mail(dmail.to, require_verified_email: true, subject: "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}")
end
# The email sent when a user requests a password reset.
def password_reset(user)
@user = user
mail to: @user.email_with_name, subject: "#{Danbooru.config.app_name} password reset request"
mail(@user, require_verified_email: false, subject: "#{Danbooru.config.app_name} password reset request")
end
# The email sent when a user changes their email address.
def email_change_confirmation(user)
@user = user
mail to: @user.email_with_name, subject: "Confirm your email address"
mail(@user, require_verified_email: false, subject: "Confirm your email address")
end
# The email sent when a new user signs up with an email address.
def welcome_user(user)
@user = user
mail to: @user.email_with_name, subject: "Welcome to #{Danbooru.config.app_name}! Confirm your email address"
mail(@user, require_verified_email: false, subject: "Welcome to #{Danbooru.config.app_name}! Confirm your email address")
end
end

View File

@@ -141,7 +141,7 @@ class Dmail < ApplicationRecord
end
def send_email
if is_recipient? && !is_deleted? && to.receive_email_notifications? && to.can_receive_email?
if is_recipient? && !is_deleted? && to.receive_email_notifications?
UserMailer.dmail_notice(self).deliver_later
end
end

View File

@@ -361,11 +361,7 @@ class User < ApplicationRecord
end
module EmailMethods
def email_with_name
"#{name} <#{email_address.address}>"
end
def can_receive_email?(require_verification: true)
def can_receive_email?(require_verified_email: true)
email_address.present? && email_address.is_deliverable? && (email_address.is_verified? || !require_verification)
end

View File

@@ -259,7 +259,7 @@ 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_no_enqueued_emails
assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last], queue: "default"
assert_equal(true, User.last.user_events.user_creation.exists?)
end

View File

@@ -12,6 +12,20 @@ class UserMailerTest < ActionMailer::TestCase
mail = UserMailer.dmail_notice(@dmail)
assert_emails(1) { mail.deliver_now }
end
should "not send an email for a user without an email address" do
@user = create(:user)
@dmail = create(:dmail, owner: @user, to: @user)
mail = UserMailer.dmail_notice(@dmail)
assert_emails(0) { mail.deliver_now }
end
should "not send an email for a user with an undeliverable address" do
@user = create(:user, email_address: build(:email_address, is_deliverable: false))
@dmail = create(:dmail, owner: @user, to: @user)
mail = UserMailer.dmail_notice(@dmail)
assert_emails(0) { mail.deliver_now }
end
end
context "password_reset method" do