Stricter email validation (#15030)

Gemfile

@@ -68,6 +68,7 @@ gem 'gibbon',                  '1.1.4'
 gem 'virtus',                   '1.0.5'
 gem 'uuidtools',                '2.1.5'
 gem 'cartodb-common',           git: 'https://github.com/cartodb/cartodb-common.git', branch: 'master'
+gem 'email_address',            '~> 0.1.11'
 
 # Markdown
 gem 'redcarpet', '3.3.3'

Gemfile.lock

@@ -135,6 +135,9 @@ GEM
       faraday (~> 0.9, ~> 0.8)
       oauth2 (~> 1.1)
     ejs (1.1.1)
+    email_address (0.1.11)
+      netaddr (~> 2.0)
+      simpleidn
     equalizer (0.0.11)
     erubis (2.7.0)
     ethon (0.8.1)
@@ -230,6 +233,7 @@ GEM
     multipart-post (2.0.0)
     net-ldap (0.16.0)
     net-telnet (0.1.1)
+    netaddr (2.0.3)
     nokogiri (1.8.5)
       mini_portile2 (~> 2.3.0)
     oauth (0.4.7)
@@ -367,6 +371,8 @@ GEM
       simplecov
     simplecov-rcov (0.2.3)
       simplecov (>= 0.4.1)
+    simpleidn (0.1.1)
+      unf (~> 0.1.4)
     sinatra (1.4.6)
       rack (~> 1.4)
       rack-protection (~> 1.4)
@@ -403,6 +409,9 @@ GEM
     tzinfo (1.2.5)
       thread_safe (~> 0.1)
     uber (0.1.0)
+    unf (0.1.4)
+      unf_ext
+    unf_ext (0.0.7.6)
     unicorn (4.8.2)
       kgio (~> 2.6)
       rack
@@ -448,6 +457,7 @@ DEPENDENCIES
   delorean
   dropbox_api (= 0.1.6)
   ejs (~> 1.1.1)
+  email_address (~> 0.1.11)
   execjs (~> 0.4)
   factory_girl_rails (~> 4.0.0)
   fake_net_ldap!

NEWS.md

@@ -14,6 +14,7 @@ sudo make install
 - Add number of employees and use case to user profile ([#14966](https://github.com/CartoDB/cartodb/pull/14966))
 
 ### Bug fixes / enhancements
+- Stricter email domain validation ([#15030](https://github.com/CartoDB/cartodb/pull/15030))
 - Add more columns to oauth_app ([#15015](https://github.com/CartoDB/cartodb/issues/15015))
 - Redirect viewer users to shared visualizations page, and show shared visualizations in Home ([CartoDB/support#2032](https://github.com/CartoDB/support/issues/2032))
 - Fix user presenter ([#15033](https://github.com/CartoDB/cartodb/pull/15033))

app/models/user.rb

@@ -1,6 +1,7 @@
 # encoding: UTF-8
 require 'cartodb/per_request_sequel_cache'
 require 'cartodb-common'
+require 'email_address'
 require_relative './user/user_decorator'
 require_relative './user/oauths'
 require_relative './synchronization/synchronization_oauth'
@@ -159,6 +160,14 @@ class User < Sequel::Model
   ## Validations
   def validate
     super
+    validate_username
+    validate_email
+    validate_password
+    validate_organization
+    validate_quotas
+  end
+
+  def validate_username
     validates_presence :username
     validates_unique   :username
     validates_format /\A[a-z0-9\-]+\z/, :username, message: "must only contain lowercase letters, numbers and the dash (-) symbol"
@@ -166,24 +175,32 @@ class User < Sequel::Model
     validates_format /[a-z0-9]{1}\z/, :username, message: "must end with alphanumeric chars"
     validates_max_length 63, :username
     errors.add(:name, 'is taken') if name_exists_in_organizations?
+  end
 
+  def validate_email
     validates_presence :email
-    validates_unique   :email, :message => 'is already taken'
-    validates_format EmailAddressValidator::Regexp::ADDR_SPEC, :email, :message => 'is not a valid address'
+    validates_unique   :email, message: 'is already taken'
+    errors.add(:email, EmailAddress.error(email)) unless EmailAddress.valid?(email)
+  end
 
+  def validate_password
     validates_presence :password if new? && crypted_password.blank?
 
     if new? || (password.present? && !@new_password.present?)
       errors.add(:password, "is not confirmed") unless password == password_confirmation
     end
     validate_password_change
+  end
 
+  def validate_organization
     if organization.present?
       organization_validation
     elsif org_admin
       errors.add(:org_admin, "cannot be set for non-organization user")
     end
+  end
 
+  def validate_quotas
     errors.add(:geocoding_quota, "cannot be nil") if geocoding_quota.nil?
     errors.add(:here_isolines_quota, "cannot be nil") if here_isolines_quota.nil?
     errors.add(:obs_snapshot_quota, "cannot be nil") if obs_snapshot_quota.nil?

config/initializers/email_address.rb

@@ -0,0 +1,21 @@
+EmailAddress::Config.configure(local_format: :conventional)
+
+class EmailValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return unless value
+
+    if value.is_a?(Array)
+      value.each { |v| validate_value(record, attribute, v) }
+    else
+      validate_value(record, attribute, value)
+    end
+  end
+
+  private
+
+  def validate_value(record, attribute, value)
+    unless EmailAddress.valid?(value)
+      record.errors[attribute] << (options[:message] || "#{value} is not a valid email")
+    end
+  end
+end

config/initializers/rfc822.rb

@@ -1,44 +0,0 @@
-# coding: UTF-8
-
-module EmailAddressValidator
-  module Regexp
-    ATEXT = /[A-Za-z0-9!#\$%&'\*\+\-\/=\?\^_`\{\|\}\~]/
-    DOT_ATOM = /(?:#{ATEXT})+(?:\.(?:#{ATEXT})+)*/
-
-    TEXT = /[\x01-\x09\x0B\x0C\x0E-\x7F]/
-    QTEXT = /[\x01-\x08\x0B\x0C\x0E-\x1F\x21\x23-\x5B\x5D-\x7E]/
-    QUOTED_PAIR = /\\#{TEXT}/
-    QCONTENT = /(?:#{QTEXT}|#{QUOTED_PAIR})/
-    QUOTED_STRING = /"(?:\s*#{QCONTENT})*\s*"/
-
-    DTEXT = /[\x01-\x08\x0B\x0C\x0E-\x1F\x21-\x5A\x5E-\x7E]/
-    DCONTENT = /(?:#{DTEXT}|#{QUOTED_PAIR})/
-    DOMAIN_LITERAL = /\[(?:\s*#{DCONTENT})*\s*\]/
-    DOMAIN = /(?:#{DOT_ATOM}|#{DOMAIN_LITERAL})/
-
-    LOCAL_PART = /(?:#{DOT_ATOM}|#{QUOTED_STRING})/
-
-    ADDR_SPEC = /^(#{LOCAL_PART})@(#{DOMAIN})$/
-  end
-end
-
-# Taken from Rails examples, replacing with ADDR_SPEC
-# the original regexp: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
-class EmailValidator < ActiveModel::EachValidator
-  def validate_each(record, attribute, value)
-    return unless value
-    if value.is_a?(Array)
-      value.each { |v| validate_value(record, attribute, v) }
-    else
-      validate_value(record, attribute, value)
-    end
-  end
-
-  private
-
-  def validate_value(record, attribute, value)
-    unless value =~ EmailAddressValidator::Regexp::ADDR_SPEC
-      record.errors[attribute] << (options[:message] || "#{value} is not an email")
-    end
-  end
-end

lib/sql

@@ -1 +1 @@
-Subproject commit 32db4fd81efcba0be0686a0bf67f8cdb62f1133c
+Subproject commit afa52aa92b1c27f6aaf12565fcbe5f79faae9917

spec/models/user_spec.rb

@@ -433,6 +433,34 @@ describe User do
     user.dashboard_viewed?.should be_true
   end
 
+  describe "email validation" do
+    before(:all) do
+      EmailAddress::Config.configure(local_format: :conventional, host_validation: :mx)
+    end
+
+    after(:all) do
+      EmailAddress::Config.configure(local_format: :conventional, host_validation: :syntax)
+    end
+
+    it "disallows wrong domains" do
+      invalid_emails = ['pimpam@example.com',
+                        'pimpam@ageval.dr',
+                        'pimpam@qq.ocm',
+                        'pimpam@aa.ww',
+                        'pimpam@iu.eduy',
+                        'pimpam@gmail.como',
+                        'pimpam@namr.cim',
+                        'pimpam@buffalo.edi']
+
+      invalid_emails.each do |email|
+        user = ::User.new(email: email)
+
+        user.valid?.should be_false
+        user.errors.should include :email
+      end
+    end
+  end
+
   it "should validate that password is present if record is new and crypted_password is blank" do
     user = ::User.new
     user.username = "adminipop"

spec/requests/signup_controller_spec.rb

@@ -152,7 +152,6 @@ describe SignupController do
     DEFAULT_QUOTA_IN_BYTES = 1000
 
     before(:all) do
-      @organization.whitelisted_email_domains = ['carto.com']
       @organization.default_quota_in_bytes = DEFAULT_QUOTA_IN_BYTES
       @organization.save
 
@@ -160,6 +159,7 @@ describe SignupController do
     end
 
     before(:each) do
+      @organization.whitelisted_email_domains = ['carto.com']      
       @organization.auth_username_password_enabled = true
       @organization.auth_google_enabled = true
       @organization.strong_passwords_enabled = true

spec/spec_helper.rb

@@ -21,6 +21,10 @@ Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }
 # Inline Resque for queue handling
 Resque.inline = true
 
+# host_validation is set to support `example.com` emails in specs
+# in production we do check for the existance of mx records associated to the domain
+EmailAddress::Config.configure(local_format: :conventional, host_validation: :syntax)
+
 RSpec.configure do |config|
   config.include SpecHelperHelpers
   config.include CartoDB::Factories