emails: validate that email addresses are deliverable.

Reject email addresses that known to be undeliverable during signup.
Some users signup with invalid email addresses, which causes the welcome
email (which contains the email confirmation link) to bounce. Too many
bounces hurt our ability to send mail.

We check that an email address is undeliverable by checking if the
domain has a mail server and if the server returns an invalid address
error when attempting to send mail. This isn't foolproof since some
servers don't return an error if the address doesn't exist. If the
checks fail we know the address is bad, but if the checks pass that
doesn't guarantee the address is good. However, this is still good
enough to filter out bad addresses for popular providers like Gmail and
Microsoft that do return nonexistent address errors.

The address existence check requires being able to connect to mail
servers over port 25. This may fail if your network blocks port 25,
which many home ISPs and hosting providers do by default.

Gemfile

@@ -46,6 +46,7 @@ gem 'ipaddress'
 gem 'http'
 gem 'activerecord-hierarchical_query'
 gem 'pundit'
+gem 'mail'
 
 # needed for looser jpeg header compat
 gem 'ruby-imagespec', :require => "image_spec", :git => "https://github.com/r888888888/ruby-imagespec.git", :branch => "exif-fixes"

Gemfile.lock

@@ -436,6 +436,7 @@ DEPENDENCIES
   ipaddress
   jquery-rails
   listen
+  mail
   mechanize
   memoist
   memory_profiler

app/controllers/users_controller.rb

@@ -59,22 +59,31 @@ class UsersController < ApplicationController
   end
 
   def create
-    @user = authorize User.new(last_ip_addr: CurrentUser.ip_addr, **permitted_attributes(User))
+    @user = authorize User.new(
+      last_ip_addr: CurrentUser.ip_addr,
+      name: params[:user][:name],
+      password: params[:user][:password],
+      password_confirmation: params[:user][:password_confirmation]
+    )
 
-    if !Danbooru.config.enable_recaptcha? || verify_recaptcha(model: @user)
-      @user.save
-      if @user.errors.empty?
-        session[:user_id] = @user.id
-        UserMailer.welcome_user(@user).deliver_later
-      else
-        flash[:notice] = "Sign up failed: #{@user.errors.full_messages.join("; ")}"
-      end
-      set_current_user
-      respond_with(@user)
-    else
-      flash[:notice] = "Sign up failed"
-      redirect_to new_user_path
+    if params[:user][:email].present?
+      @user.email_address = EmailAddress.new(address: params[:user][:email])
     end
+
+    if Danbooru.config.enable_recaptcha? && !verify_recaptcha(model: @user)
+      flash[:notice] = "Sign up failed"
+    elsif @user.email_address&.invalid?(:deliverable)
+      flash[:notice] = "Sign up failed: email address is invalid or doesn't exist"
+      @user.errors[:base] << @user.email_address.errors.full_messages.join("; ")
+    elsif !@user.save
+      flash[:notice] = "Sign up failed: #{@user.errors.full_messages.join("; ")}"
+    else
+      session[:user_id] = @user.id
+      UserMailer.welcome_user(@user).deliver_later
+      set_current_user
+    end
+
+    respond_with(@user)
   end
 
   def update

app/logical/email_validator.rb

@@ -1,4 +1,6 @@
-module EmailNormalizer
+require 'resolv'
+
+module EmailValidator
   module_function
 
   IGNORE_DOTS = %w[gmail.com]
@@ -73,4 +75,41 @@ module EmailNormalizer
 
     "#{name}@#{domain}"
   end
+
+  def undeliverable?(to_address, from_address: Danbooru.config.contact_email, timeout: 3)
+    mail_server = mx_domain(to_address, timeout: timeout)
+    mail_server.nil? || rcpt_to_failed?(to_address, from_address, mail_server, timeout: timeout)
+  rescue
+    false
+  end
+
+  def rcpt_to_failed?(to_address, from_address, mail_server, timeout: nil)
+    return false unless smtp_enabled?
+
+    from_domain = Mail::Address.new(from_address).domain
+
+    smtp = Net::SMTP.new(mail_server)
+    smtp.read_timeout = timeout
+    smtp.open_timeout = timeout
+
+    smtp.start(from_domain) do |conn|
+      conn.mailfrom(from_address)
+
+      # Net::SMTPFatalError is raised if RCPT TO returns a 5xx error.
+      response = conn.rcptto(to_address) rescue $!
+      return response.is_a?(Net::SMTPFatalError)
+    end
+  end
+
+  def mx_domain(to_address, timeout: nil)
+    domain = Mail::Address.new(to_address).domain
+
+    dns = Resolv::DNS.new
+    dns.timeouts = timeout
+    response = dns.getresource(domain, Resolv::DNS::Resource::IN::MX)
+
+    response.exchange.to_s
+  rescue Resolv::ResolvError
+    nil
+  end
 end

app/models/email_address.rb

@@ -7,12 +7,19 @@ class EmailAddress < ApplicationRecord
   validates :address, presence: true, confirmation: true, format: { with: EMAIL_REGEX }
   validates :normalized_address, uniqueness: true
   validates :user_id, uniqueness: true
+  validate :validate_deliverable, on: :deliverable
 
   def address=(value)
-    self.normalized_address = EmailNormalizer.normalize(value) || address
+    self.normalized_address = EmailValidator.normalize(value) || address
     super
   end
 
+  def validate_deliverable
+    if EmailValidator.undeliverable?(address)
+      errors[:address] << "is invalid or does not exist"
+    end
+  end
+
   concerning :VerificationMethods do
     def verifier
       @verifier ||= Danbooru::MessageVerifier.new(:email_verification_key)

app/views/users/new.html.erb

@@ -15,9 +15,7 @@
     <div id="p3">
       <%= edit_form_for(@user, html: { id: "signup-form" }) do |f| %>
         <%= f.input :name, as: :string %>
-        <%= f.simple_fields_for :email_address do |fe| %>
-          <%= fe.input :address, label: "Email", required: false, as: :email, hint: "Optional" %>
-        <% end %>
+        <%= f.input :email, label: "Email", required: false, as: :email, hint: "Optional" %>
         <%= f.input :password %>
         <%= f.input :password_confirmation %>
 

test/functional/users_controller_test.rb

@@ -114,29 +114,35 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     context "create action" do
       should "create a user" do
-        user_params = { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }
-        post users_path, params: { user: user_params }
+        post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
 
         assert_redirected_to User.last
         assert_equal("xxx", User.last.name)
+        assert_equal(nil, User.last.email_address)
         assert_no_emails
       end
 
       should "create a user with a valid email" do
-        user_params = { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1", email_address_attributes: { address: "test@gmail.com" } }
-        post users_path, params: { user: user_params }
+        post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1", email: "webmaster@danbooru.donmai.us" }}
 
         assert_redirected_to User.last
         assert_equal("xxx", User.last.name)
-        assert_equal("test@gmail.com", User.last.email_address.address)
+        assert_equal("webmaster@danbooru.donmai.us", User.last.email_address.address)
         assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last]
       end
 
       should "not create a user with an invalid email" do
-        user_params = { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1", email_address_attributes: { address: "test" } }
+        assert_no_difference(["User.count", "EmailAddress.count"]) do
+          post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1", email: "test" }}
 
-        assert_no_difference("User.count") do
-          post users_path, params: { user: user_params }
+          assert_response :success
+          assert_no_emails
+        end
+      end
+
+      should "not create a user with an undeliverable email address" do
+        assert_no_difference(["User.count", "EmailAddress.count"]) do
+          post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1", email: "nobody@nothing.donmai.us" } }
 
           assert_response :success
           assert_no_emails

test/unit/email_validator_test.rb

@@ -0,0 +1,23 @@
+require 'test_helper'
+
+class EmailValidatorTest < ActiveSupport::TestCase
+  context "EmailValidator" do
+    context "#undeliverable?" do
+      should "return good addresses as deliverable" do
+        assert_equal(false, EmailValidator.undeliverable?("webmaster@danbooru.donmai.us"))
+        assert_equal(false, EmailValidator.undeliverable?("noizave+spam@gmail.com"))
+      end
+
+      should "return nonexistent domains as undeliverable" do
+        assert_equal(true, EmailValidator.undeliverable?("nobody@does.not.exist.donmai.us"))
+      end
+
+      # XXX these tests are known to fail if your network blocks port 25.
+      should_eventually "return nonexistent addresses as undeliverable" do
+        assert_equal(true, EmailValidator.undeliverable?("does.not.exist.13yoigo34iy@gmail.com"))
+        assert_equal(true, EmailValidator.undeliverable?("does.not.exist.13yoigo34iy@outlook.com"))
+        assert_equal(true, EmailValidator.undeliverable?("does.not.exist.13yoigo34iy@hotmail.com"))
+      end
+    end
+  end
+end