diff --git a/Gemfile b/Gemfile index 3f9c05419..99addc66b 100644 --- a/Gemfile +++ b/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" diff --git a/Gemfile.lock b/Gemfile.lock index 6218be49d..00191d539 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -436,6 +436,7 @@ DEPENDENCIES ipaddress jquery-rails listen + mail mechanize memoist memory_profiler diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6f8a54572..06dbc9e9e 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/logical/email_normalizer.rb b/app/logical/email_validator.rb similarity index 67% rename from app/logical/email_normalizer.rb rename to app/logical/email_validator.rb index 19bb6c439..ada3d638e 100644 --- a/app/logical/email_normalizer.rb +++ b/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 diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 7812d5131..d96470eff 100644 --- a/app/models/email_address.rb +++ b/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) diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 3081fdf0d..80ca23b11 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -15,9 +15,7 @@
<%= 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 %> diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 9ff3370f2..fa4c01a91 100644 --- a/test/functional/users_controller_test.rb +++ b/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 diff --git a/test/unit/email_validator_test.rb b/test/unit/email_validator_test.rb new file mode 100644 index 000000000..cf1bf0557 --- /dev/null +++ b/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