diff --git a/app/models/email_address.rb b/app/models/email_address.rb index b27cf6e58..31bdf02fa 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -36,6 +36,7 @@ class EmailAddress < ActiveRecord::Base validates_length_of :address, :maximum => User::MAIL_LENGTH_LIMIT, :allow_nil => true validates_uniqueness_of :address, :case_sensitive => false, :if => Proc.new {|email| email.address_changed? && email.address.present?} + validate :validate_email_domain, :if => proc {|email| email.errors[:address].blank? && email.address.present?} safe_attributes 'address' @@ -51,6 +52,28 @@ class EmailAddress < ActiveRecord::Base end end + # Returns true if the email domain is allowed regarding allowed/denied + # domains defined in application settings, otherwise false + def self.valid_domain?(domain_or_email) + denied, allowed = + [:email_domains_denied, :email_domains_allowed].map do |setting| + Setting.__send__(setting) + end + domain = domain_or_email.split('@').last + return false if denied.present? && domain_in?(domain, denied) + return false if allowed.present? && !domain_in?(domain, allowed) + true + end + + # Returns true if domain belongs to domains list. + def self.domain_in?(domain, domains) + domain = domain.downcase + domains = domains.to_s.split(/[\s,]+/) unless domains.is_a?(Array) + domains.reject(&:blank?).map(&:downcase).any? do |s| + s.start_with?('.') ? domain.end_with?(s) : domain == s + end + end + private # send a security notification to user that a new email address was added @@ -117,4 +140,11 @@ class EmailAddress < ActiveRecord::Base Token.where(:user_id => user_id, :action => tokens).delete_all end end + + def validate_email_domain + domain = address.split('@').last + unless self.class.valid_domain?(domain) + errors.add(:address, :disallowed_domain, :domain => domain) + end + end end diff --git a/app/views/settings/_users.html.erb b/app/views/settings/_users.html.erb index ab61d7c21..846fe2586 100644 --- a/app/views/settings/_users.html.erb +++ b/app/views/settings/_users.html.erb @@ -4,6 +4,12 @@

<%= setting_text_field :max_additional_emails, :size => 6 %>

<%= setting_check_box :unsubscribe %>

+ +

<%= setting_text_area :email_domains_allowed %> + <%= l(:text_comma_separated) %> <%= l(:label_example) %>: example.com, example.org

+ +

<%= setting_text_area :email_domains_denied %> + <%= l(:text_comma_separated) %> <%= l(:label_example) %>: .example.com, foo.example.org, example.net

diff --git a/config/locales/en.yml b/config/locales/en.yml index efaea5252..bc9bf87f6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -136,6 +136,7 @@ en: must_contain_lowercase: "must contain lowercase letters (a-z)" must_contain_digits: "must contain digits (0-9)" must_contain_special_chars: "must contain special characters (!, $, %, ...)" + disallowed_domain: "contains a domain not allowed (%{domain})" actionview_instancetag_blank_option: Please select @@ -479,6 +480,8 @@ en: setting_force_default_language_for_loggedin: Force default language for logged-in users setting_link_copied_issue: Link issues on copy setting_max_additional_emails: Maximum number of additional email addresses + setting_email_domains_allowed: Allowed email domains + setting_email_domains_denied: Disallowed email domains setting_search_results_per_page: Search results per page setting_attachment_extensions_allowed: Allowed extensions setting_attachment_extensions_denied: Disallowed extensions diff --git a/config/settings.yml b/config/settings.yml index 5aaaaac28..7c0912232 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -53,6 +53,10 @@ password_max_age: max_additional_emails: format: int default: 5 +email_domains_allowed: + default: +email_domains_denied: + default: # Maximum lifetime of user sessions in minutes session_lifetime: format: int diff --git a/test/functional/email_addresses_controller_test.rb b/test/functional/email_addresses_controller_test.rb index 76ff0a561..eb7f0e8b0 100644 --- a/test/functional/email_addresses_controller_test.rb +++ b/test/functional/email_addresses_controller_test.rb @@ -118,6 +118,36 @@ class EmailAddressesControllerTest < Redmine::ControllerTest end end + def test_create_with_disallowed_domain_should_fail + @request.session[:user_id] = 2 + + with_settings :email_domains_denied => 'black.example' do + assert_no_difference 'EmailAddress.count' do + post :create, :params => { + :user_id => 2, + :email_address => { + :address => 'another@black.example' + } + } + assert_response :success + assert_select_error 'Email contains a domain not allowed (black.example)' + end + end + + with_settings :email_domains_allowed => 'white.example' do + assert_no_difference 'EmailAddress.count' do + post :create, :params => { + :user_id => 2, + :email_address => { + :address => 'something@example.fr' + } + } + assert_response :success + assert_select_error 'Email contains a domain not allowed (example.fr)' + end + end + end + def test_create_should_send_security_notification @request.session[:user_id] = 2 ActionMailer::Base.deliveries.clear diff --git a/test/unit/email_address_test.rb b/test/unit/email_address_test.rb index 3237d01b4..c3242e25b 100644 --- a/test/unit/email_address_test.rb +++ b/test/unit/email_address_test.rb @@ -30,4 +30,38 @@ class EmailAddressTest < ActiveSupport::TestCase email = EmailAddress.new(address: 'jsmith@example.xn--80akhbyknj4f') assert email.valid? end + + def test_address_should_be_validated_against_denied_domains + with_settings :email_domains_denied => "black.test\r\nBLACK.EXAMPLE, .subdomain.test" do + email = EmailAddress.new(address: 'user@black.test') + assert_not email.valid? + email = EmailAddress.new(address: 'user@notblack.test') + assert email.valid? + email = EmailAddress.new(address: 'user@BLACK.TEST') + assert_not email.valid? + email = EmailAddress.new(address: 'user@black.example') + assert_not email.valid? + email = EmailAddress.new(address: 'user@subdomain.test') + assert email.valid? + email = EmailAddress.new(address: 'user@foo.subdomain.test') + assert_not email.valid? + end + end + + def test_address_should_be_validated_against_allowed_domains + with_settings :email_domains_allowed => "white.test\r\nWHITE.EXAMPLE, .subdomain.test" do + email = EmailAddress.new(address: 'user@white.test') + assert email.valid? + email = EmailAddress.new(address: 'user@notwhite.test') + assert_not email.valid? + email = EmailAddress.new(address: 'user@WHITE.TEST') + assert email.valid? + email = EmailAddress.new(address: 'user@white.example') + assert email.valid? + email = EmailAddress.new(address: 'user@subdomain.test') + assert_not email.valid? + email = EmailAddress.new(address: 'user@foo.subdomain.test') + assert email.valid? + end + end end