From 6076db74f17967e9b1a4d62d593d6c4b62deca0f Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 27 Nov 2011 15:27:14 +0000 Subject: [PATCH] Improved user creation from incoming email. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@7952 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/mail_handler.rb | 61 +++++++++++++++++++++++++--------- test/unit/mail_handler_test.rb | 40 ++++++++++++++++++++++ 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 5e8220c96..221b7885e 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -61,7 +61,7 @@ class MailHandler < ActionMailer::Base when 'accept' @user = User.anonymous when 'create' - @user = MailHandler.create_user_from_email(email) + @user = create_user_from_email(email) if @user logger.info "MailHandler: [#{@user.login}] account created" if logger && logger.info Mailer.deliver_account_information(@user, @user.password) @@ -322,22 +322,53 @@ class MailHandler < ActionMailer::Base @full_sanitizer ||= HTML::FullSanitizer.new end - # Creates a user account for the +email+ sender - def self.create_user_from_email(email) + def self.assign_string_attribute_with_limit(object, attribute, value) + limit = object.class.columns_hash[attribute.to_s].limit || 255 + value = value.to_s.slice(0, limit) + object.send("#{attribute}=", value) + end + + # Returns a User from an email address and a full name + def self.new_user_from_attributes(email_address, fullname=nil) + user = User.new + + # Truncating the email address would result in an invalid format + user.mail = email_address + assign_string_attribute_with_limit(user, 'login', email_address) + + names = fullname.blank? ? email_address.gsub(/@.*$/, '').split('.') : fullname.split + assign_string_attribute_with_limit(user, 'firstname', names.shift) + assign_string_attribute_with_limit(user, 'lastname', names.join(' ')) + user.lastname = '-' if user.lastname.blank? + + password_length = [Setting.password_min_length.to_i, 10].max + user.password = ActiveSupport::SecureRandom.hex(password_length / 2 + 1) + user.language = Setting.default_language + + unless user.valid? + user.login = "user#{ActiveSupport::SecureRandom.hex(6)}" if user.errors.on(:login) + user.firstname = "-" if user.errors.on(:firstname) + user.lastname = "-" if user.errors.on(:lastname) + end + + user + end + + # Creates a User for the +email+ sender + # Returns the user or nil if it could not be created + def create_user_from_email(email) addr = email.from_addrs.to_a.first if addr && !addr.spec.blank? - user = User.new - user.mail = addr.spec - - names = addr.name.blank? ? addr.spec.gsub(/@.*$/, '').split('.') : addr.name.split - user.firstname = names.shift - user.lastname = names.join(' ') - user.lastname = '-' if user.lastname.blank? - - user.login = user.mail - user.password = ActiveSupport::SecureRandom.hex(5) - user.language = Setting.default_language - user.save ? user : nil + user = self.class.new_user_from_attributes(addr.spec, addr.name) + if user.save + user + else + logger.error "MailHandler: failed to create User: #{user.errors.full_messages}" if logger + nil + end + else + logger.error "MailHandler: failed to create User: no FROM address found" if logger + nil end end diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index da4aa422c..a9e45d104 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -485,6 +485,46 @@ class MailHandlerTest < ActiveSupport::TestCase assert_equal issue.subject, 'New ticket on a given project with a very long subject line which exceeds 255 chars and should not be ignored but chopped off. And if the subject line is still not long enough, we just add more text. And more text. Wow, this is really annoying. Especially, if you have nothing to say...'[0,255] end + def test_new_user_from_attributes_should_return_valid_user + to_test = { + # [address, name] => [login, firstname, lastname] + ['jsmith@example.net', nil] => ['jsmith@example.net', 'jsmith', '-'], + ['jsmith@example.net', 'John'] => ['jsmith@example.net', 'John', '-'], + ['jsmith@example.net', 'John Smith'] => ['jsmith@example.net', 'John', 'Smith'], + ['jsmith@example.net', 'John Paul Smith'] => ['jsmith@example.net', 'John', 'Paul Smith'], + ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsTheMaximumLength Smith'] => ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsT', 'Smith'], + ['jsmith@example.net', 'John AVeryLongLastnameThatExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', 'AVeryLongLastnameThatExceedsTh'], + ['alongemailaddressthatexceedsloginlength@example.net', 'John Smith'] => ['alongemailaddressthatexceedslo', 'John', 'Smith'] + } + + to_test.each do |attrs, expected| + user = MailHandler.new_user_from_attributes(attrs.first, attrs.last) + + assert user.valid? + assert_equal attrs.first, user.mail + assert_equal expected[0], user.login + assert_equal expected[1], user.firstname + assert_equal expected[2], user.lastname + end + end + + def test_new_user_from_attributes_should_respect_minimum_password_length + with_settings :password_min_length => 15 do + user = MailHandler.new_user_from_attributes('jsmith@example.net') + assert user.valid? + assert user.password.length >= 15 + end + end + + def test_new_user_from_attributes_should_use_default_login_if_invalid + MailHandler.new_user_from_attributes('alongemailaddressthatexceedsloginlength-1@example.net').save! + + # another long address that would result in duplicate login + user = MailHandler.new_user_from_attributes('alongemailaddressthatexceedsloginlength-2@example.net') + assert user.valid? + assert user.login =~ /^user[a-f0-9]+$/ + end + private def submit_email(filename, options={})